[PATCH] drm/amd/display: reject both non-zero src_x and src_y only for DCN1x

[Paul Menzel]

Dear Shirish,


Am 08.11.21 um 09:15 schrieb Shirish S:
> limit the MPO rejection only for DCN1x as its not required on later

it’s

> versions.

Where is it documented, that it’s not required for later versions?

Shortly describing the implementation is also useful. Something like: 
Require `fill_dc_scaling_info()` to receive the device to be able to 
check the version.

> Fixes: d89f6048bdcb ("drm/amd/display: Reject non-zero src_y and src_x for video planes")
> 

I’d remove the blank line.

> Signed-off-by: Shirish S <shirish.s at amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 ++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 1e26d9be8993..26b29d561919 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4572,7 +4572,8 @@ static void get_min_max_dc_plane_scaling(struct drm_device *dev,
>   }
>   
>   
> -static int fill_dc_scaling_info(const struct drm_plane_state *state,
> +static int fill_dc_scaling_info(struct amdgpu_device *adev,
> +				const struct drm_plane_state *state,
>   				struct dc_scaling_info *scaling_info)
>   {
>   	int scale_w, scale_h, min_downscale, max_upscale;
> @@ -4586,7 +4587,8 @@ static int fill_dc_scaling_info(const struct drm_plane_state *state,
>   	/*
>   	 * For reasons we don't (yet) fully understand a non-zero
>   	 * src_y coordinate into an NV12 buffer can cause a
> -	 * system hang. To avoid hangs (and maybe be overly cautious)
> +	 * system hang on DCN1x.
> +	 * To avoid hangs (and maybe be overly cautious)

I’d remove the added line break.

>   	 * let's reject both non-zero src_x and src_y.
>   	 *
>   	 * We currently know of only one use-case to reproduce a
> @@ -4594,10 +4596,10 @@ static int fill_dc_scaling_info(const struct drm_plane_state *state,
>   	 * is to gesture the YouTube Android app into full screen
>   	 * on ChromeOS.
>   	 */
> -	if (state->fb &&
> -	    state->fb->format->format == DRM_FORMAT_NV12 &&
> -	    (scaling_info->src_rect.x != 0 ||
> -	     scaling_info->src_rect.y != 0))
> +	if (((adev->ip_versions[DCE_HWIP][0] == IP_VERSION(1, 0, 0)) ||
> +	    (adev->ip_versions[DCE_HWIP][0] == IP_VERSION(1, 0, 1))) &&
> +	    (state->fb && state->fb->format->format == DRM_FORMAT_NV12 &&
> +	    (scaling_info->src_rect.x != 0 || scaling_info->src_rect.y != 0)))
>   		return -EINVAL;
>   
>   	scaling_info->src_rect.width = state->src_w >> 16;
> @@ -5503,7 +5505,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
>   	int ret;
>   	bool force_disable_dcc = false;
>   
> -	ret = fill_dc_scaling_info(plane_state, &scaling_info);
> +	ret = fill_dc_scaling_info(adev, plane_state, &scaling_info);
>   	if (ret)
>   		return ret;
>   
> @@ -7566,7 +7568,7 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
>   	if (ret)
>   		return ret;
>   
> -	ret = fill_dc_scaling_info(new_plane_state, &scaling_info);
> +	ret = fill_dc_scaling_info(adev, new_plane_state, &scaling_info);
>   	if (ret)
>   		return ret;
>   
> @@ -9014,7 +9016,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   			bundle->surface_updates[planes_count].gamut_remap_matrix = &dc_plane->gamut_remap_matrix;
>   		}
>   
> -		fill_dc_scaling_info(new_plane_state,
> +		fill_dc_scaling_info(dm->adev, new_plane_state,
>   				     &bundle->scaling_infos[planes_count]);
>   
>   		bundle->surface_updates[planes_count].scaling_info =
>