linux-kernel - Re: [PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <93602cd6-a03d-4124-8bb3-de21136d9589@amd.com>
Date:   Fri, 4 Aug 2023 14:24:28 -0400
From:   Harry Wentland <harry.wentland@....com>
To:     Hamza Mahfooz <hamza.mahfooz@....com>,
        amd-gfx@...ts.freedesktop.org
Cc:     stable@...r.kernel.org, Leo Li <sunpeng.li@....com>,
        Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Qingqing Zhuo <qingqing.zhuo@....com>,
        Aurabindo Pillai <aurabindo.pillai@....com>,
        Hersen Wu <hersenxs.wu@....com>,
        Srinivasan Shanmugam <srinivasan.shanmugam@....com>,
        Stylon Wang <stylon.wang@....com>,
        Wayne Lin <wayne.lin@....com>, Alan Liu <haoping.liu@....com>,
        Melissa Wen <mwen@...lia.com>, Simon Ser <contact@...rsion.fr>,
        David Tadokoro <davidbtadokoro@....br>,
        André Almeida <andrealmeid@...lia.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/amd/display: ensure async flips are only accepted
 for fast updates



On 2023-08-04 14:20, Hamza Mahfooz wrote:
> We should be checking to see if async flips are supported in
> amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
> async flipping isn't supported if a plane's framebuffer changes memory
> domains during an atomic commit. So, move the check from
> dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
> the memory domain has changed in amdgpu_dm_atomic_check().
> 
> Cc: stable@...r.kernel.org
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2733
> Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast updates")
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@....com>

Reviewed-by: Harry Wentland <harry.wentland@....com>

Harry

> ---
> v2: link issue and revert back to the old way of setting update_type.
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ++++++++++++++++---
>  .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    | 12 ----------
>  2 files changed, 21 insertions(+), 15 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 32fb551862b0..1d3afab5bc85 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8086,10 +8086,12 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>  		 * fast updates.
>  		 */
>  		if (crtc->state->async_flip &&
> -		    acrtc_state->update_type != UPDATE_TYPE_FAST)
> +		    (acrtc_state->update_type != UPDATE_TYPE_FAST ||
> +		     get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
>  			drm_warn_once(state->dev,
>  				      "[PLANE:%d:%s] async flip with non-fast update\n",
>  				      plane->base.id, plane->name);
> +
>  		bundle->flip_addrs[planes_count].flip_immediate =
>  			crtc->state->async_flip &&
>  			acrtc_state->update_type == UPDATE_TYPE_FAST &&
> @@ -10050,6 +10052,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>  
>  	/* Remove exiting planes if they are modified */
>  	for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) {
> +		if (old_plane_state->fb && new_plane_state->fb &&
> +		    get_mem_type(old_plane_state->fb) !=
> +		    get_mem_type(new_plane_state->fb))
> +			lock_and_validation_needed = true;
> +
>  		ret = dm_update_plane_state(dc, state, plane,
>  					    old_plane_state,
>  					    new_plane_state,
> @@ -10297,9 +10304,20 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>  		struct dm_crtc_state *dm_new_crtc_state =
>  			to_dm_crtc_state(new_crtc_state);
>  
> +		/*
> +		 * Only allow async flips for fast updates that don't change
> +		 * the FB pitch, the DCC state, rotation, etc.
> +		 */
> +		if (new_crtc_state->async_flip && lock_and_validation_needed) {
> +			drm_dbg_atomic(crtc->dev,
> +				       "[CRTC:%d:%s] async flips are only supported for fast updates\n",
> +				       crtc->base.id, crtc->name);
> +			ret = -EINVAL;
> +			goto fail;
> +		}
> +
>  		dm_new_crtc_state->update_type = lock_and_validation_needed ?
> -							 UPDATE_TYPE_FULL :
> -							 UPDATE_TYPE_FAST;
> +			UPDATE_TYPE_FULL : UPDATE_TYPE_FAST;
>  	}
>  
>  	/* Must be success */
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 30d4c6fd95f5..440fc0869a34 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -398,18 +398,6 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> -	/*
> -	 * Only allow async flips for fast updates that don't change the FB
> -	 * pitch, the DCC state, rotation, etc.
> -	 */
> -	if (crtc_state->async_flip &&
> -	    dm_crtc_state->update_type != UPDATE_TYPE_FAST) {
> -		drm_dbg_atomic(crtc->dev,
> -			       "[CRTC:%d:%s] async flips are only supported for fast updates\n",
> -			       crtc->base.id, crtc->name);
> -		return -EINVAL;
> -	}
> -
>  	/* In some use cases, like reset, no stream is attached */
>  	if (!dm_crtc_state->stream)
>  		return 0;