<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="appendonsend"></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Ville Syrjälä <ville.syrjala@linux.intel.com><br>
<b>Sent:</b> Wednesday, October 2, 2019 19:58<br>
<b>To:</b> Lin, Wayne <Wayne.Lin@amd.com><br>
<b>Cc:</b> dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com><br>
<b>Subject:</b> Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">On Tue, Sep 24, 2019 at 01:26:21PM +0800, Wayne Lin wrote:<br>
> In HDMI 1.4 defines 4k modes without specific aspect ratio.<br>
> However, in HDMI 2.0, adds aspect ratio attribute to distinguish different<br>
> 4k modes.<br>
> <br>
> According to Appendix E of HDMI 2.0 spec, source should use VSIF to<br>
> indicate VIC mode only when the mode is one defined in HDMI 1.4b 4K modes.<br>
> Otherwise, use AVI infoframes to convey VIC.<br>
> <br>
> eg: VIC_103 should use AVI infoframes and VIC_93 use VSIF<br>
> <br>
> When the sink is HDMI 2.0, current code in<br>
> drm_hdmi_avi_infoframe_from_display_mode will also force mode VIC_103 to<br>
> have VIC value 0. This violates the spec and needs to be corrected.<br>
<br>
> Where is that being done? We only set the AVI VIC to zero if we're going<br>
> to use the HDMI VIC instead.</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">Appreciate for your time and apologize for not explaining it clearly. </div>
<div class="PlainText">Current code in drm_hdmi_avi_infoframe_from_display_mode() will call </div>
<div class="PlainText">drm_match_hdmi_mode() to set up vendor_if_vic. By checking </div>
<div class="PlainText">drm_valid_hdmi_vic(vendor_if_vic) to see if the vic info should be conveyed by avi</div>
<div class="PlainText">or not.</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">But in drm_match_hdmi_mode(), code doesn't enable match_flags with </div>
<div class="PlainText">DRM_MODE_MATCH_ASPECT_RATIO. I think it's due to HDMI1.4b doesn't specify</div>
<div class="PlainText">4K mode conveyed by HDMI VIC with particular aspect ratio. But in <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif;">Appendix E of </span></div>
<div class="PlainText"><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif;">HDMI 2.0 spec, it specify only 4k modes with particular aspect ratio should use
 VSIF to convey.</span></div>
<div class="PlainText">Hence, when the sink support HDMI 2.0 and set the mode to be VIC_103, calling</div>
<div class="PlainText">drm_match_hdmi_mode(mode) will return <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">
vendor_if_vic =</span> 3 (VIC_93 and VIC_103 are having</div>
<div class="PlainText">the same timing but different aspect ratio). Thereafter will set the  frame->video_code to 0. </div>
<div class="PlainText">However, VIC_103 should use AVI VIC according to HDMI 2.0 spec (only VIC: 93, 94, 95 &</div>
<div class="PlainText">98 should use VSIF).</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">This patch try to revise that, when the sink support HDMI 2.0, <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">drm_match_hdmi_mode()</span></div>
<div class="PlainText"><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">should also take
 aspect ratio </span><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif;">into consideration. </span></div>
<div class="PlainText"><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif;">But for easy reading, I add another function "</span><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif;">drm_match_hdmi_1_4_mode</span><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif;">"
 to do so.</span></div>
<div class="PlainText"><br>
> The same situation occurs in drm_hdmi_vendor_infoframe_from_display_mode<br>
> and should set HDMI_VIC when the mode is one defined in HDMI 1.4b 4K<br>
> modes.<br>
<br>
> Yes, and we do that. "vic = drm_match_hdmi_mode(mode);"<br>
<br>
> Apart from adding the aspect ratios I don't really understand what<br>
> you're trying to achieve here.</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">For HDMI 2.0 sink, <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">drm_match_hdmi_mode()
 should also take aspect ratio into consideration.</span></div>
<div class="PlainText"><span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">Once again, very
 appreciate for your time.</span></div>
<div class="PlainText"><br>
> ---<br>
>  drivers/gpu/drm/drm_edid.c | 95 ++++++++++++++++++++++++++++++++++++--<br>
>  1 file changed, 92 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c<br>
> index 649cfd8b4200..0fea9bf4ec67 100644<br>
> --- a/drivers/gpu/drm/drm_edid.c<br>
> +++ b/drivers/gpu/drm/drm_edid.c<br>
> @@ -1306,6 +1306,37 @@ static const struct drm_display_mode edid_4k_modes[] = {<br>
>          .vrefresh = 24, },<br>
>  };<br>
>  <br>
> +/*<br>
> + * 4k modes of HDMI 1.4 defined in HDMI 2.0. Index using the VIC.<br>
> + */<br>
> +static const struct drm_display_mode hdmi_1_4_edid_4k_modes[] = {<br>
> +     /* 0 - dummy, VICs start at 1 */<br>
> +     { },<br>
> +     /* 1 - 3840x2160@30Hz */<br>
> +     { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,<br>
> +                3840, 4016, 4104, 4400, 0,<br>
> +                2160, 2168, 2178, 2250, 0,<br>
> +                DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),<br>
> +       .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },<br>
> +     /* 2 - 3840x2160@25Hz */<br>
> +     { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,<br>
> +                3840, 4896, 4984, 5280, 0,<br>
> +                2160, 2168, 2178, 2250, 0,<br>
> +                DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),<br>
> +       .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },<br>
> +     /* 3 - 3840x2160@24Hz */<br>
> +     { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,<br>
> +                3840, 5116, 5204, 5500, 0,<br>
> +                2160, 2168, 2178, 2250, 0,<br>
> +                DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),<br>
> +       .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },<br>
> +     /* 4 - 4096x2160@24Hz (SMPTE) */<br>
> +     { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000,<br>
> +                4096, 5116, 5204, 5500, 0,<br>
> +                2160, 2168, 2178, 2250, 0,<br>
> +                DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),<br>
> +       .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135, },<br>
> +};<br>
>  /*** DDC fetch and block validation ***/<br>
>  <br>
>  static const u8 edid_header[] = {<br>
> @@ -3061,6 +3092,19 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)<br>
>        return cea_mode_alternate_clock(hdmi_mode);<br>
>  }<br>
>  <br>
> +/*<br>
> + * Calculate the alternate clock for HDMI modes (those from the HDMI vendor<br>
> + * specific block).<br>
> + *<br>
> + * It's almost like hdmi_mode_alternate_clock(), but no exception for VIC 4 mode.<br>
> + * There is an alternate clock (23.98Hz) of VIC 4 mode (4096x2160@24Hz) in HDMI 2.0<br>
> + */<br>
> +static unsigned int<br>
> +hdmi_1_4_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)<br>
> +{<br>
> +     return cea_mode_alternate_clock(hdmi_mode);<br>
> +}<br>
> +<br>
>  static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match,<br>
>                                              unsigned int clock_tolerance)<br>
>  {<br>
> @@ -3121,11 +3165,53 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)<br>
>        return 0;<br>
>  }<br>
>  <br>
> +/*<br>
> + * drm_match_hdmi_1_4_mode - look for a HDMI 1.4 mode matching given mode<br>
> + * @to_match: display mode<br>
> + *<br>
> + * An HDMI mode is one defined in the HDMI vendor specific block.<br>
> + * In HDMI 2.0, only few 4k resolutions with specific aspect ratio should<br>
> + * utilize H14b VSIF.<br>
> + *<br>
> + * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.<br>
> + */<br>
> +static u8 drm_match_hdmi_1_4_mode(const struct drm_display_mode *to_match)<br>
> +{<br>
> +     unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS;<br>
> +     u8 vic;<br>
> +<br>
> +     if (!to_match->clock)<br>
> +             return 0;<br>
> +<br>
> +     if (to_match->picture_aspect_ratio)<br>
> +             match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;<br>
> +<br>
> +     for (vic = 1; vic < ARRAY_SIZE(hdmi_1_4_edid_4k_modes); vic++) {<br>
> +             const struct drm_display_mode *hdmi_1_4_mode = &hdmi_1_4_edid_4k_modes[vic];<br>
> +             unsigned int clock1, clock2;<br>
> +<br>
> +             /* Make sure to also match alternate clocks */<br>
> +             clock1 = hdmi_1_4_mode->clock;<br>
> +             clock2 = hdmi_1_4_mode_alternate_clock(hdmi_1_4_mode);<br>
> +<br>
> +             if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||<br>
> +                  KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&<br>
> +                 drm_mode_match(to_match, hdmi_1_4_mode, match_flags))<br>
> +                     return vic;<br>
> +     }<br>
> +     return 0;<br>
> +}<br>
> +<br>
>  static bool drm_valid_hdmi_vic(u8 vic)<br>
>  {<br>
>        return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes);<br>
>  }<br>
>  <br>
> +static bool drm_valid_hdmi_1_4_vic(u8 vic)<br>
> +{<br>
> +     return vic > 0 && vic < ARRAY_SIZE(hdmi_1_4_edid_4k_modes);<br>
> +}<br>
> +<br>
>  static int<br>
>  add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)<br>
>  {<br>
> @@ -4894,10 +4980,12 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,<br>
>         * HDMI 1.4b 4K modes<br>
>         */<br>
>        if (frame->video_code) {<br>
> -             u8 vendor_if_vic = drm_match_hdmi_mode(mode);<br>
> +             u8 vendor_if_vic = is_hdmi2_sink(connector) ?<br>
> +                     drm_match_hdmi_1_4_mode(mode) : drm_match_hdmi_mode(mode);<br>
>                bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;<br>
>  <br>
> -             if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)<br>
> +             if (!is_s3d && is_hdmi2_sink(connector) ?<br>
> +                     drm_valid_hdmi_1_4_vic(vendor_if_vic) : drm_valid_hdmi_vic(vendor_if_vic))<br>
>                        frame->video_code = 0;<br>
>        }<br>
>  <br>
> @@ -5125,7 +5213,8 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,<br>
>        if (!has_hdmi_infoframe)<br>
>                return -EINVAL;<br>
>  <br>
> -     vic = drm_match_hdmi_mode(mode);<br>
> +     vic = is_hdmi2_sink(connector) ?<br>
> +                     drm_match_hdmi_1_4_mode(mode) : drm_match_hdmi_mode(mode);<br>
>        s3d_flags = mode->flags & DRM_MODE_FLAG_3D_MASK;<br>
>  <br>
>        /*<br>
> -- <br>
> 2.17.1<br>
> <br>
> _______________________________________________<br>
> dri-devel mailing list<br>
> dri-devel@lists.freedesktop.org<br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/dri-devel">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
<br>
-- <br>
Ville Syrjälä<br>
Intel<br>
</div>
</span></font></div>
</body>
</html>