Re: [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5651 bytes --]

This patch is causing failure of IGT test kms_3d. The kms_3d test 
expects the no. of 3d modes to be 13.

(The test has hard-coded value for expected no. of 3d modes as 13)

But due to the addition of "matching aspect_ratio" in drm_mode_equal in 
this patch, the total no. of

modes in the connector modelist is increased by 2, resulting in failure 
of assertion 'mode_count==13'.

Perhaps this need to be handled in the test.

-Regards,

Ankit


On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> From: "Sharma, Shashank" <shashank.sharma@intel.com>
>
> Current DRM layer functions don't parse aspect ratio information
> while converting a user mode->kernel mode or vice versa. This
> causes modeset to pick mode with wrong aspect ratio, eventually
> causing failures in HDMI compliance test cases, due to wrong VIC.
>
> This patch adds aspect ratio information in DRM's mode conversion
> and mode comparision functions, to make sure kernel picks mode
> with right aspect ratio (as per the VIC).
>
> Background:
> This patch was once reviewed and merged, and later reverted due to
> lack of DRM cap protection. This is a re-spin of this patch, this
> time with DRM cap protection, to avoid aspect ratio information, when
> the client doesn't request for it.
>
> Review link: https://pw-emeril.freedesktop.org/patch/104068/
> Background discussion: https://patchwork.kernel.org/patch/9379057/
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Lin, Jia <lin.a.jia@intel.com>
> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
> Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (V2)
> Reviewed-by: Jose Abreu <Jose.Abreu@synopsys.com> (V4)
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Jose Abreu <Jose.Abreu@synopsys.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>
> V3: modified the aspect-ratio check in drm_mode_equal as per new flags
>      provided by Ville. https://patchwork.freedesktop.org/patch/188043/
> V4: rebase
> V5: rebase
> V6: As recommended by Ville, avoided matching of aspect-ratio in
>      drm_fb_helper, while trying to find a common mode among connectors
>      for the target clone mode.
> V7: rebase
> V8: rebase
> V9: rebase
> V10: rebase
> ---
>   drivers/gpu/drm/drm_fb_helper.c | 12 ++++++++++--
>   drivers/gpu/drm/drm_modes.c     | 35 ++++++++++++++++++++++++++++++++++-
>   2 files changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0646b10..2ee1eaa 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper,
>   		for (j = 0; j < i; j++) {
>   			if (!enabled[j])
>   				continue;
> -			if (!drm_mode_equal(modes[j], modes[i]))
> +			if (!drm_mode_match(modes[j], modes[i],
> +					    DRM_MODE_MATCH_TIMINGS |
> +					    DRM_MODE_MATCH_CLOCK |
> +					    DRM_MODE_MATCH_FLAGS |
> +					    DRM_MODE_MATCH_3D_FLAGS))
>   				can_clone = false;
>   		}
>   	}
> @@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper,
>   
>   		fb_helper_conn = fb_helper->connector_info[i];
>   		list_for_each_entry(mode, &fb_helper_conn->connector->modes, head) {
> -			if (drm_mode_equal(mode, dmt_mode))
> +			if (drm_mode_match(mode, dmt_mode,
> +					   DRM_MODE_MATCH_TIMINGS |
> +					   DRM_MODE_MATCH_CLOCK |
> +					   DRM_MODE_MATCH_FLAGS |
> +					   DRM_MODE_MATCH_3D_FLAGS))
>   				modes[i] = mode;
>   		}
>   		if (!modes[i])
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index d6133e8..454f2ff 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1049,7 +1049,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1,
>   			      DRM_MODE_MATCH_TIMINGS |
>   			      DRM_MODE_MATCH_CLOCK |
>   			      DRM_MODE_MATCH_FLAGS |
> -			      DRM_MODE_MATCH_3D_FLAGS);
> +			      DRM_MODE_MATCH_3D_FLAGS|
> +			      DRM_MODE_MATCH_ASPECT_RATIO);
>   }
>   EXPORT_SYMBOL(drm_mode_equal);
>   
> @@ -1647,6 +1648,20 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
>   	out->vrefresh = in->vrefresh;
>   	out->flags = in->flags;
>   	out->type = in->type;
> +
> +	switch (in->picture_aspect_ratio) {
> +	case HDMI_PICTURE_ASPECT_4_3:
> +		out->flags |= DRM_MODE_FLAG_PIC_AR_4_3;
> +		break;
> +	case HDMI_PICTURE_ASPECT_16_9:
> +		out->flags |= DRM_MODE_FLAG_PIC_AR_16_9;
> +		break;
> +	case HDMI_PICTURE_ASPECT_RESERVED:
> +	default:
> +		out->flags |= DRM_MODE_FLAG_PIC_AR_NONE;
> +		break;
> +	}
> +
>   	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
>   	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
>   }
> @@ -1693,6 +1708,24 @@ int drm_mode_convert_umode(struct drm_device *dev,
>   	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
>   	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
>   
> +	/* Clearing picture aspect ratio bits from out flags,
> +	 * as the aspect-ratio information is not stored in
> +	 * flags for kernel-mode, but in picture_aspect_ratio.
> +	 */
> +	out->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
> +
> +	switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) {
> +	case DRM_MODE_FLAG_PIC_AR_4_3:
> +		out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3;
> +		break;
> +	case DRM_MODE_FLAG_PIC_AR_16_9:
> +		out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
> +		break;
> +	default:
> +		out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +		break;
> +	}
> +
>   	out->status = drm_mode_validate_driver(dev, out);
>   	if (out->status != MODE_OK)
>   		return -EINVAL;


[-- Attachment #1.2: Type: text/html, Size: 7219 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx