Re: [v2] drm/amd/display: Fall back to overlay cursor on dcn4x when top plane doesn't fill CRTC

Oops,
+ Harry

On 8/20/26 10:13, [email protected] wrote:
> From: James Lin <[email protected]>
> 
> [why]
> amdgpu_dm_crtc_get_cursor_mode() returns DM_CURSOR_NATIVE_MODE early and
> unconditionally for dcn4.x. That early return was added because these ASICs
> no longer have the cursor-on-scaled-plane or cursor-on-yuv-plane restrictions
> of older DCN, so native cursor is fine in those cases. However, it also
> bypasses the "does the top plane fill the CRTC?" (hole) evaluation further
> down. When the top/primary plane does not cover the whole CRTC, the cursor
> must fall back to an overlay plane so it is composited over the uncovered region;
> keeping the native cursor there produces an incorrect result. As a
> consequence igt@amdgpu/amd_cursor_overlay@non-full, which verifies exactly
> this native->overlay fallback, fails on dcn42.
> 
> [how]
> Replace the unconditional early return with a skip_fmt_scale_restrictions
> flag. For dcn4.x keep skipping the YUV / active-color-pipeline / different-
> scale overlay triggers (genuinely unnecessary on this hardware), but fall
> through to the existing entire_crtc_covered check so a top plane that does
> not fill the CRTC still selects DM_CURSOR_OVERLAY_MODE. Native mode is
> still chosen when the plane covers the whole CRTC.
> 
> Update the amdgpu_dm_cursor KUnit tests accordingly: exercise the dcn4x
> path with a full atomic state fixture (full coverage -> native) and add a
> dcn4x hole case (top plane does not fill the CRTC -> overlay).
> 
> Also remove DCN5/6 guards since they've been upstreamed.
> 
> v2:
> Move the check for disabled crtc to  separate early return, so a disabled
> CRTC always reports native mode on every ASIC, independently of the DCN4x
> fallback logic.
> 
> Signed-off-by: James Lin <[email protected]>
> Signed-off-by: Ivan Lipski <[email protected]>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_cursor.c  | 45 +++++++++++--------
>   .../amdgpu_dm/tests/amdgpu_dm_cursor_test.c   | 32 +++++++++----
>   2 files changed, 49 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_cursor.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_cursor.c
> index 9534848ed240..8204ad4018e6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_cursor.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_cursor.c
> @@ -87,12 +87,8 @@ STATIC_IFN_KUNIT int dm_check_cursor_fb(struct amdgpu_crtc *new_acrtc,
>   	 * check tiling flags when the FB doesn't have a modifier.
>   	 */
>   	if (!(fb->flags & DRM_MODE_FB_MODIFIERS)) {
> -#if defined(CONFIG_DRM_AMD_DC_DCN6_0) || defined(CONFIG_DRM_AMD_DC_DCN5_0)
>   		if (adev->family == AMDGPU_FAMILY_GC_12_0_0
>   		    || adev->family == AMDGPU_FAMILY_GC_13_0_1) {
> -#else
> -		if (adev->family == AMDGPU_FAMILY_GC_12_0_0) {
> -#endif
>   			linear = AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE) == 0;
>   		} else if (adev->family >= AMDGPU_FAMILY_AI) {
>   			linear = AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0;
> @@ -289,26 +285,34 @@ int amdgpu_dm_crtc_get_cursor_mode(struct amdgpu_device *adev,
>   	int underlying_scale_w, underlying_scale_h;
>   	int cursor_scale_w, cursor_scale_h;
>   	int i;
> +	bool skip_fmt_scale_restrictions = false;
>   
> -	/* Overlay cursor not supported on HW before DCN
> -	 * DCN401/420 does not have the cursor-on-scaled-plane or cursor-on-yuv-plane restrictions
> -	 * as previous DCN generations, so enable native mode on DCN401/420
> -	 *
> +	/*
>   	 * Always set native cursor mode when the CRTC is disabled,
>   	 * to make sure it doesn't cause atomic commits to fail when
>   	 * they are trying to disable the CRTC.
>   	 */
> +	if (!crtc_state->enable) {
> +		*cursor_mode = DM_CURSOR_NATIVE_MODE;
> +		return 0;
> +	}
> +
> +	/* Overlay cursor not supported on HW before DCN
> +	 * DCN401/420 does not have the cursor-on-scaled-plane or cursor-on-yuv-plane restrictions
> +	 * as previous DCN generations, so enable native mode on DCN401/420
> +	 */
>   	if (amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(4, 0, 1) ||
>   	    amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(4, 2, 0) ||
> -#if defined(CONFIG_DRM_AMD_DC_DCN6_0)
>   	    amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(4, 2, 1) ||
> -	    amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(6, 0, 0) ||
> -#else
> -	    amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(4, 2, 1) ||
> -#endif
> -	    !dm_crtc_state->base.enable) {
> -		*cursor_mode = DM_CURSOR_NATIVE_MODE;
> -		return 0;
> +	    amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(6, 0, 0)) {
> +		/*
> +		 * Newer DCN has no cursor-on-scaled/yuv-plane restriction, so
> +		 * skip those overlay triggers below. A plane that does not fill
> +		 * the CRTC still needs overlay mode so the cursor renders over
> +		 * the hole, so fall through to the coverage check instead of
> +		 * unconditionally forcing native mode here.
> +		 */
> +		skip_fmt_scale_restrictions = true;
>   	}
>   
>   	/* Init cursor_mode to be the same as current */
> @@ -407,13 +411,15 @@ int amdgpu_dm_crtc_get_cursor_mode(struct amdgpu_device *adev,
>   			continue;
>   
>   		/* Underlying plane is YUV format - use overlay cursor */
> -		if (amdgpu_dm_plane_is_video_format(plane_state->fb->format->format)) {
> +		if (!skip_fmt_scale_restrictions &&
> +		    amdgpu_dm_plane_is_video_format(plane_state->fb->format->format)) {
>   			*cursor_mode = DM_CURSOR_OVERLAY_MODE;
>   			return 0;
>   		}
>   
>   		/* Underlying plane has an active color pipeline - cursor would be transformed */
> -		if (dm_plane_color_pipeline_active(state, plane, false)) {
> +		if (!skip_fmt_scale_restrictions &&
> +		    dm_plane_color_pipeline_active(state, plane, false)) {
>   			*cursor_mode = DM_CURSOR_OVERLAY_MODE;
>   			return 0;
>   		}
> @@ -424,7 +430,8 @@ int amdgpu_dm_crtc_get_cursor_mode(struct amdgpu_device *adev,
>   				   &cursor_scale_w, &cursor_scale_h);
>   
>   		/* Underlying plane has different scale - use overlay cursor */
> -		if (cursor_scale_w != underlying_scale_w &&
> +		if (!skip_fmt_scale_restrictions &&
> +		    cursor_scale_w != underlying_scale_w &&
>   		    cursor_scale_h != underlying_scale_h) {
>   			*cursor_mode = DM_CURSOR_OVERLAY_MODE;
>   			return 0;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_cursor_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_cursor_test.c
> index efd16575896e..9026c6ce7336 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_cursor_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_cursor_test.c
> @@ -521,25 +521,38 @@ static void dm_test_crtc_get_cursor_mode_disabled_crtc(struct kunit *test)
>   }
>   
>   /**
> - * dm_test_crtc_get_cursor_mode_new_hardware - Test new hardware always uses native mode
> + * dm_test_crtc_get_cursor_mode_new_hardware - Test dcn4x uses native cursor when the top plane fills the CRTC
>    * @test: The KUnit test context
>    */
>   static void dm_test_crtc_get_cursor_mode_new_hardware(struct kunit *test)
>   {
> -	struct amdgpu_device *adev = dm_kunit_alloc_adev(test);
> -	struct dm_crtc_state *dm_crtc_state;
> +	struct dm_cursor_mode_fixture fixture = dm_test_alloc_cursor_mode_fixture(test);
>   	enum amdgpu_dm_cursor_mode cursor_mode = DM_CURSOR_OVERLAY_MODE;
> -	int ret;
>   
> -	dm_crtc_state = kunit_kzalloc(test, sizeof(*dm_crtc_state), GFP_KERNEL);
> -	KUNIT_ASSERT_NOT_NULL(test, dm_crtc_state);
> -	adev->ip_versions[DCE_HWIP][0] = IP_VERSION(4, 2, 0);
> +	fixture.adev->ip_versions[DCE_HWIP][0] = IP_VERSION(4, 2, 0);
>   
> -	ret = amdgpu_dm_crtc_get_cursor_mode(adev, NULL, dm_crtc_state, &cursor_mode);
> -	KUNIT_EXPECT_EQ(test, ret, 0);
> +	KUNIT_EXPECT_EQ(test, dm_test_get_cursor_mode(&fixture, &cursor_mode), 0);
>   	KUNIT_EXPECT_EQ(test, cursor_mode, DM_CURSOR_NATIVE_MODE);
>   }
>   
> +/**
> + * dm_test_crtc_get_cursor_mode_new_hardware_hole - Test dcn4x falls back to
> + * overlay cursor when the top plane does not fill the CRTC
> + * @test: The KUnit test context
> + */
> +static void dm_test_crtc_get_cursor_mode_new_hardware_hole(struct kunit *test)
> +{
> +	struct dm_cursor_mode_fixture fixture = dm_test_alloc_cursor_mode_fixture(test);
> +	enum amdgpu_dm_cursor_mode cursor_mode = DM_CURSOR_NATIVE_MODE;
> +
> +	fixture.adev->ip_versions[DCE_HWIP][0] = IP_VERSION(4, 2, 0);
> +	fixture.old_primary_state->crtc_w = 1280;
> +	fixture.primary_state->crtc_w = 1280;
> +
> +	KUNIT_EXPECT_EQ(test, dm_test_get_cursor_mode(&fixture, &cursor_mode), 0);
> +	KUNIT_EXPECT_EQ(test, cursor_mode, DM_CURSOR_OVERLAY_MODE);
> +}
> +
>   /**
>    * dm_test_crtc_get_cursor_mode_no_change - Test unchanged atomic state preserves cursor mode
>    * @test: The KUnit test context
> @@ -948,6 +961,7 @@ static struct kunit_case amdgpu_dm_cursor_tests[] = {
>   	/* amdgpu_dm_crtc_get_cursor_mode */
>   	KUNIT_CASE(dm_test_crtc_get_cursor_mode_disabled_crtc),
>   	KUNIT_CASE(dm_test_crtc_get_cursor_mode_new_hardware),
> +	KUNIT_CASE(dm_test_crtc_get_cursor_mode_new_hardware_hole),
>   	KUNIT_CASE(dm_test_crtc_get_cursor_mode_no_change),
>   	KUNIT_CASE(dm_test_crtc_get_cursor_mode_disabled_cursor),
>   	KUNIT_CASE(dm_test_crtc_get_cursor_mode_yuv_plane),

-- 
Thanks,
Ivan Lipski