Re: [PATCH v2] drm/amd/display: Reject non-zero src_y and src_x for video planes

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Good that this has been found. Just a curious guess, but have you guys 
checked if the src_x and src_y are a multiple of 2?

Might cause problems to try to access a subsampled surface if the 
coordinates doesn't make much sense.

Anyway patch is Acked-by: Christian König <christian.koenig@amd.com>

Regards,
Christian.

Am 23.04.21 um 16:09 schrieb Harry Wentland:
> [Why]
> This hasn't been well tested and leads to complete system hangs on DCN1
> based systems, possibly others.
>
> The system hang can be reproduced by gesturing the video on the YouTube
> Android app on ChromeOS into full screen.
>
> [How]
> Reject atomic commits with non-zero drm_plane_state.src_x or src_y values.
>
> v2:
>   - Add code comment describing the reason we're rejecting non-zero
>     src_x and src_y
>   - Drop gerrit Change-Id
>   - Add stable CC
>   - Based on amd-staging-drm-next
>
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> Cc: stable@vger.kernel.org
> Cc: nicholas.kazlauskas@amd.com
> Cc: amd-gfx@lists.freedesktop.org
> Cc: alexander.deucher@amd.com
> Cc: Roman.Li@amd.com
> Cc: hersenxs.wu@amd.com
> Cc: danny.wang@amd.com
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> 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 be1769d29742..b12469043e6b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4089,6 +4089,23 @@ static int fill_dc_scaling_info(const struct drm_plane_state *state,
>   	scaling_info->src_rect.x = state->src_x >> 16;
>   	scaling_info->src_rect.y = state->src_y >> 16;
>   
> +	/*
> +	 * 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)
> +	 * let's reject both non-zero src_x and src_y.
> +	 *
> +	 * We currently know of only one use-case to reproduce a
> +	 * scenario with non-zero src_x and src_y for NV12, which
> +	 * 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))
> +		return -EINVAL;
> +
>   	scaling_info->src_rect.width = state->src_w >> 16;
>   	if (scaling_info->src_rect.width == 0)
>   		return -EINVAL;