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