On Tue, Dec 01, 2015 at 05:16:54PM +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 now explicitly check for 0 instead of
> just subtracting one from an unsigned type.
>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>
> ---
>
> This is on top of
>
> commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33
> Author: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Date: Mon Nov 16 21:05:12 2015 +0200
>
> drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz
> clock difference
>
> in drm-intel/topic/drm-misc
> ---
> drivers/gpu/drm/drm_edid.c | 84
> ++++++++++++++++++++++++----------------------
> 1 file changed, 43 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index c214f1246cb4..aea09eae407e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
<snip>
> @@ -2713,16 +2716,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 (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {
Maybe add some sort of vic_is_valid() helpers to avoid duplicating
these in so many places?
Otherwise I like it.
> + 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 (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {
> + cea_mode = &edid_4k_modes[vic];
> clock2 = hdmi_mode_alternate_clock(cea_mode);
> }
> }
> @@ -2773,17 +2776,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 (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes))
> 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 +2881,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 (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) {
> DRM_ERROR("Unknown HDMI VIC: %d\n", vic);
> return 0;
> }
> @@ -3170,24 +3172,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 (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {
> 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 (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {
> 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 +3207,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
