Re: [PATCH 07/10] drm/edid: Don't send bogus aspect ratios in AVI infoframes

From: "Sharma, Shashank" <shashank.sharma@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	dri-devel@lists.freedesktop.org
Cc: Jose Abreu <Jose.Abreu@synopsys.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org,
	Emil Velikov <emil.l.velikov@gmail.com>,
	Nautiyal Ankit <ankit.k.nautiyal@intel.com>
Subject: Re: [PATCH 07/10] drm/edid: Don't send bogus aspect ratios in AVI infoframes
Date: Thu, 16 Nov 2017 20:31:36 +0530	[thread overview]
Message-ID: <ce06a42b-d81e-f41b-991e-680d468aa1a3@intel.com> (raw)
In-Reply-To: <20171113170427.4150-8-ville.syrjala@linux.intel.com>

Regards

Shashank

On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If the user mode would specify an aspect ratio other than 4:3 or 16:9
> we now silently ignore it. Maybe a better apporoach is to return an
> error? Let's try that.
>
> Also we must be careful that we don't try to send illegal picture
> aspect in the infoframe as it's only capable of signalling none,
> 4:3, and 16:9. Currently we're sending these bogus infoframes
> whenever the cea mode specifies some other aspect ratio.
>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Jose Abreu <Jose.Abreu@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Emil Velikov <emil.l.velikov@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 00aa98f3e55d..bafb3ee4ea97 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>   					 const struct drm_display_mode *mode,
>   					 bool is_hdmi2_sink)
>   {
> +	enum hdmi_picture_aspect picture_aspect;
>   	int err;
>   
>   	if (!frame || !mode)
> @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>   	 * Populate picture aspect ratio from either
>   	 * user input (if specified) or from the CEA mode list.
>   	 */
> -	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
> -		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
> -		frame->picture_aspect = mode->picture_aspect_ratio;
> -	else if (frame->video_code > 0)
> -		frame->picture_aspect = drm_get_cea_aspect_ratio(
> -						frame->video_code);
> +	picture_aspect = mode->picture_aspect_ratio;
> +	if (picture_aspect == HDMI_PICTURE_ASPECT_NONE)
> +		picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
This is slightly going in the loop.
- During the modeset the driver cant specify the aspect ratio 
information, as DRM layer lacks this support.
- So we fill the VIC field, by comparing the mode with the 
DRM_CEA_MODES[] list. This will pick the first mode
   available in the list (regardless of its aspect ratio), and fill the 
VIC, as we don't consider aspect ratio while comparing timings.
- Again, now while sending the aspect ratio, we are picking up the VIC, 
which may not be correct.

So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is 
first in list, we will always pick 4:3 aspect ratio.

- Shashank
>   
> +	/*
> +	 * The infoframe can't convey anything but none, 4:3
> +	 * and 16:9, so if the user has asked for anything else
> +	 * we can only satisfy it by specifying the right VIC.
> +	 */
> +	if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) {
> +		if (picture_aspect !=
> +		    drm_get_cea_aspect_ratio(frame->video_code))
> +			return -EINVAL;
> +		picture_aspect = HDMI_PICTURE_ASPECT_NONE;
> +	}
> +
> +	frame->picture_aspect = picture_aspect;
>   	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
>   	frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
>   

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel