On Thu, Nov 16, 2017 at 08:21:44PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
> > cause us to not send out any VICs in the AVI infoframes. That commit
> > was since reverted, but if and when we add aspect ratio handing back
> > we need to be more careful.
> >
> > Let's handle this by considering the aspect ratio as a requirement
> > for cea mode matching only if the passed in mode actually has a
> > non-zero aspect ratio field. This will keep userspace that doesn't
> > provide an aspect ratio working as before by matching it to the
> > first otherwise equal cea mode. And once userspace starts to
> > provide the aspect ratio it will be considerd a hard requirement
> > for the match.
> >
> > Also change the hdmi mode matching to use drm_mode_match() for
> > consistency, but we don't match on aspect ratio there since the
> > spec doesn't list a specific aspect ratio for those modes.
> >
> > Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> > Cc: "Lin, Jia" <lin.a.jia@xxxxxxxxx>
> > Cc: Akashdeep Sharma <akashdeep.sharma@xxxxxxxxx>
> > Cc: Jim Bride <jim.bride@xxxxxxxxxxxxxxx>
> > Cc: Jose Abreu <Jose.Abreu@xxxxxxxxxxxx>
> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > Cc: Emil Velikov <emil.l.velikov@xxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++----
> > 1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 7220b8f9a7e8..00aa98f3e55d 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -2903,11 +2903,15 @@ cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
> > static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match,
> > unsigned int clock_tolerance)
> > {
> > + 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;
> This doesn't look right. This means we are expecting a CEA mode without
> a pic aspect ratio field,
> which is invalid.
No, it's perfectly valid. It's what we currently get from userspace.
> Ankit is going to publish the aspect ratio patch
> series again, with proper DRM cap and flags check. Would it be
> ok if we have a look that handling first ?
This patch will be needed by that work. Otherwise we're going to stop
sending a VIC for CEA modes with current userspace.
> > +
> > for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > struct drm_display_mode cea_mode = edid_cea_modes[vic];
> > unsigned int clock1, clock2;
> > @@ -2921,7 +2925,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m
> > continue;
> >
> > do {
> > - if (drm_mode_equal_no_clocks_no_stereo(to_match, &cea_mode))
> > + if (drm_mode_match(to_match, &cea_mode, match_flags))
> > return vic;
> > } while (cea_mode_alternate_timings(vic, &cea_mode));
> > }
> > @@ -2938,11 +2942,15 @@ 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)
> > {
> > + 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;
> same here, and probably in other CEA match functions.
> > +
> > for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > struct drm_display_mode cea_mode = edid_cea_modes[vic];
> > unsigned int clock1, clock2;
> > @@ -2956,7 +2964,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
> > continue;
> >
> > do {
> > - if (drm_mode_equal_no_clocks_no_stereo(to_match, &cea_mode))
> > + if (drm_mode_match(to_match, &cea_mode, match_flags))
> > return vic;
> > } while (cea_mode_alternate_timings(vic, &cea_mode));
> > }
> > @@ -3003,6 +3011,7 @@ 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)
> > {
> > + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS;
> > u8 vic;
> >
> > if (!to_match->clock)
> > @@ -3020,7 +3029,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_
> > abs(to_match->clock - clock2) > clock_tolerance)
> > continue;
> >
> > - if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
> > + if (drm_mode_match(to_match, hdmi_mode, match_flags))
> > return vic;
> > }
> >
> > @@ -3037,6 +3046,7 @@ 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)
> > {
> > + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS;
> > u8 vic;
> >
> > if (!to_match->clock)
> > @@ -3052,7 +3062,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))
> > + drm_mode_match(to_match, hdmi_mode, match_flags))
> > return vic;
> > }
> > return 0;
> - Shashank
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
