On Fri, Jan 08, 2016 at 01:30:29PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 08, 2016 at 01:21:51PM +0200, Jani Nikula wrote:
> > Add a dummy entry to CEA/HDMI mode tables so they can be indexed
> > directly using the VIC, avoiding a +1/-1 dance here and there. This adds
> > clarity to the error checking for various functions that return the VIC
> > on success and zero on failure; we can now explicitly check for 0
> > instead of just subtracting one from an unsigned type.
> >
> > Also add drm_valid_cea_vic() and drm_valid_hdmi_vic() helpers for
> > checking valid VICs.
> >
> > v2: add drm_valid_cea_vic and drm_valid_hdmi_vic helpers (Ville)
> > use { } instead of { 0 } for initializing the dummy modes
> >
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Applied to drm-misc, thanks.
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_edid.c | 94
> > ++++++++++++++++++++++++++--------------------
> > 1 file changed, 53 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index c214f1246cb4..04cb4877fabd 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -637,8 +637,12 @@ static const struct minimode extra_modes[] = {
> > /*
> > * Probably taken from CEA-861 spec.
> > * This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c.
> > + *
> > + * Index using the VIC.
> > */
> > static const struct drm_display_mode edid_cea_modes[] = {
> > + /* 0 - dummy, VICs start at 1 */
> > + { },
> > /* 1 - 640x480 at 60Hz */
> > { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656,
> > 752, 800, 0, 480, 490, 492, 525, 0,
> > @@ -987,9 +991,11 @@ static const struct drm_display_mode edid_cea_modes[]
> > = {
> > };
> >
> > /*
> > - * HDMI 1.4 4k modes.
> > + * HDMI 1.4 4k modes. Index using the VIC.
> > */
> > static const struct drm_display_mode edid_4k_modes[] = {
> > + /* 0 - dummy, VICs start at 1 */
> > + { },
> > /* 1 - 3840x2160 at 30Hz */
> > { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
> > 3840, 4016, 4104, 4400, 0,
> > @@ -2548,13 +2554,13 @@ cea_mode_alternate_clock(const struct
> > drm_display_mode *cea_mode)
> > static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode
> > *to_match,
> > unsigned int clock_tolerance)
> > {
> > - u8 mode;
> > + u8 vic;
> >
> > if (!to_match->clock)
> > return 0;
> >
> > - for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {
> > - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
> > + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];
> > unsigned int clock1, clock2;
> >
> > /* Check both 60Hz and 59.94Hz */
> > @@ -2566,7 +2572,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const
> > struct drm_display_mode *to_m
> > continue;
> >
> > if (drm_mode_equal_no_clocks(to_match, cea_mode))
> > - return mode + 1;
> > + return vic;
> > }
> >
> > return 0;
> > @@ -2581,13 +2587,13 @@ static u8 drm_match_cea_mode_clock_tolerance(const
> > struct drm_display_mode *to_m
> > */
> > u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
> > {
> > - u8 mode;
> > + u8 vic;
> >
> > if (!to_match->clock)
> > return 0;
> >
> > - for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {
> > - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
> > + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];
> > unsigned int clock1, clock2;
> >
> > /* Check both 60Hz and 59.94Hz */
> > @@ -2597,12 +2603,17 @@ u8 drm_match_cea_mode(const struct drm_display_mode
> > *to_match)
> > if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
> > KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
> > drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode))
> > - return mode + 1;
> > + return vic;
> > }
> > return 0;
> > }
> > EXPORT_SYMBOL(drm_match_cea_mode);
> >
> > +static bool drm_valid_cea_vic(u8 vic)
> > +{
> > + return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
> > +}
> > +
> > /**
> > * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to
> > * the input VIC from the CEA mode list
> > @@ -2612,10 +2623,7 @@ EXPORT_SYMBOL(drm_match_cea_mode);
> > */
> > enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
> > {
> > - /* return picture aspect ratio for video_code - 1 to access the
> > - * right array element
> > - */
> > - return edid_cea_modes[video_code-1].picture_aspect_ratio;
> > + return edid_cea_modes[video_code].picture_aspect_ratio;
> > }
> > EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
> >
> > @@ -2639,13 +2647,13 @@ hdmi_mode_alternate_clock(const struct
> > drm_display_mode *hdmi_mode)
> > static u8 drm_match_hdmi_mode_clock_tolerance(const struct
> > drm_display_mode *to_match,
> > unsigned int clock_tolerance)
> > {
> > - u8 mode;
> > + u8 vic;
> >
> > if (!to_match->clock)
> > return 0;
> >
> > - for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
> > - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
> > + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
> > + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
> > unsigned int clock1, clock2;
> >
> > /* Make sure to also match alternate clocks */
> > @@ -2657,7 +2665,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const
> > struct drm_display_mode *to_
> > continue;
> >
> > if (drm_mode_equal_no_clocks(to_match, hdmi_mode))
> > - return mode + 1;
> > + return vic;
> > }
> >
> > return 0;
> > @@ -2673,13 +2681,13 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const
> > struct drm_display_mode *to_
> > */
> > static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
> > {
> > - u8 mode;
> > + u8 vic;
> >
> > if (!to_match->clock)
> > return 0;
> >
> > - for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
> > - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
> > + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
> > + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
> > unsigned int clock1, clock2;
> >
> > /* Make sure to also match alternate clocks */
> > @@ -2689,11 +2697,16 @@ static u8 drm_match_hdmi_mode(const struct
> > drm_display_mode *to_match)
> > if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
> > KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
> > drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
> > - return mode + 1;
> > + return vic;
> > }
> > return 0;
> > }
> >
> > +static bool drm_valid_hdmi_vic(u8 vic)
> > +{
> > + return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes);
> > +}
> > +
> > static int
> > add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
> > {
> > @@ -2713,16 +2726,16 @@ add_alternate_cea_modes(struct drm_connector
> > *connector, struct edid *edid)
> > list_for_each_entry(mode, &connector->probed_modes, head) {
> > const struct drm_display_mode *cea_mode = NULL;
> > struct drm_display_mode *newmode;
> > - u8 mode_idx = drm_match_cea_mode(mode) - 1;
> > + u8 vic = drm_match_cea_mode(mode);
> > unsigned int clock1, clock2;
> >
> > - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
> > - cea_mode = &edid_cea_modes[mode_idx];
> > + if (drm_valid_cea_vic(vic)) {
> > + cea_mode = &edid_cea_modes[vic];
> > clock2 = cea_mode_alternate_clock(cea_mode);
> > } else {
> > - mode_idx = drm_match_hdmi_mode(mode) - 1;
> > - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
> > - cea_mode = &edid_4k_modes[mode_idx];
> > + vic = drm_match_hdmi_mode(mode);
> > + if (drm_valid_hdmi_vic(vic)) {
> > + cea_mode = &edid_4k_modes[vic];
> > clock2 = hdmi_mode_alternate_clock(cea_mode);
> > }
> > }
> > @@ -2773,17 +2786,17 @@ drm_display_mode_from_vic_index(struct
> > drm_connector *connector,
> > {
> > struct drm_device *dev = connector->dev;
> > struct drm_display_mode *newmode;
> > - u8 cea_mode;
> > + u8 vic;
> >
> > if (video_db == NULL || video_index >= video_len)
> > return NULL;
> >
> > /* CEA modes are numbered 1..127 */
> > - cea_mode = (video_db[video_index] & 127) - 1;
> > - if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
> > + vic = (video_db[video_index] & 127);
> > + if (!drm_valid_cea_vic(vic))
> > return NULL;
> >
> > - newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
> > + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
> > if (!newmode)
> > return NULL;
> >
> > @@ -2878,8 +2891,7 @@ static int add_hdmi_mode(struct drm_connector
> > *connector, u8 vic)
> > struct drm_device *dev = connector->dev;
> > struct drm_display_mode *newmode;
> >
> > - vic--; /* VICs start at 1 */
> > - if (vic >= ARRAY_SIZE(edid_4k_modes)) {
> > + if (!drm_valid_hdmi_vic(vic)) {
> > DRM_ERROR("Unknown HDMI VIC: %d\n", vic);
> > return 0;
> > }
> > @@ -3170,24 +3182,24 @@ static void fixup_detailed_cea_mode_clock(struct
> > drm_display_mode *mode)
> > {
> > const struct drm_display_mode *cea_mode;
> > int clock1, clock2, clock;
> > - u8 mode_idx;
> > + u8 vic;
> > const char *type;
> >
> > /*
> > * allow 5kHz clock difference either way to account for
> > * the 10kHz clock resolution limit of detailed timings.
> > */
> > - mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
> > - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
> > + vic = drm_match_cea_mode_clock_tolerance(mode, 5);
> > + if (drm_valid_cea_vic(vic)) {
> > type = "CEA";
> > - cea_mode = &edid_cea_modes[mode_idx];
> > + cea_mode = &edid_cea_modes[vic];
> > clock1 = cea_mode->clock;
> > clock2 = cea_mode_alternate_clock(cea_mode);
> > } else {
> > - mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;
> > - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
> > + vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);
> > + if (drm_valid_hdmi_vic(vic)) {
> > type = "HDMI";
> > - cea_mode = &edid_4k_modes[mode_idx];
> > + cea_mode = &edid_4k_modes[vic];
> > clock1 = cea_mode->clock;
> > clock2 = hdmi_mode_alternate_clock(cea_mode);
> > } else {
> > @@ -3205,7 +3217,7 @@ static void fixup_detailed_cea_mode_clock(struct
> > drm_display_mode *mode)
> > return;
> >
> > DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
> > - type, mode_idx + 1, mode->clock, clock);
> > + type, vic, mode->clock, clock);
> > mode->clock = clock;
> > }
> >
> > --
> > 2.1.4
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
