[Intel-gfx] [PATCH 06/10] drm/edid: Fix cea mode aspect ratio handling

[Sharma, Shashank]

Regards

Shashank


On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> 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 at intel.com>
> Cc: "Lin, Jia" <lin.a.jia at intel.com>
> Cc: Akashdeep Sharma <akashdeep.sharma at intel.com>
> Cc: Jim Bride <jim.bride at linux.intel.com>
> Cc: Jose Abreu <Jose.Abreu at synopsys.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Emil Velikov <emil.l.velikov at gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>   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.  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 ?
> +
>   	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