Re: [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 1, 2015 at 10:16 AM, Jani Nikula <jani.nikula@xxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>

Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>

>
> ---
>
> This is on top of
>
> commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33
> Author: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 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
> @@ -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 */
> +       { 0 },
>         /* 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 */
> +       { 0 },
>         /* 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,7 +2603,7 @@ 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;
>  }
> @@ -2612,10 +2618,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 +2642,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 +2660,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 +2676,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,7 +2692,7 @@ 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;
>  }
> @@ -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)) {
> +                       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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux