| Message ID | 1452252111-6439-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@linux.intel.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > 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@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@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
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@linux.intel.com> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > Reviewed-by: Ville Syrjälä <ville.syrjala@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@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@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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@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; }
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@linux.intel.com> Cc: Alex Deucher <alexander.deucher@amd.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/drm_edid.c | 94 ++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 41 deletions(-)