Re: [PATCH] Revert "drm/amd/display: Hardware cursor changes color when switched to software cursor"

[Harry Wentland <harry.wentland@amd.com>]

On 2025-04-22 10:58, Melissa Wen wrote:
> This reverts commit 272e6aab14bbf98d7a06b2b1cd6308a02d4a10a1.
> 
> Applying degamma curve to the cursor by default breaks Linux userspace
> expectation.
> 
> On Linux, AMD display manager enables cursor degamma ROM just for
> implict sRGB on HW versions where degamma is split into two blocks:
> degamma ROM for pre-defined TFs and `gamma correction` for user/custom
> curves, and degamma ROM settings doesn't apply to cursor plane.
> 
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1513
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2803
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4144
> Reported-by: Michel Dänzer <michel.daenzer@mailbox.org>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> 
> Hi,
> 
> I suspect there is a conflict of interest between OSes here, because
> this is not the first time this mechanism has been removed from the
> DC shared-code and after reintroduced [1].
> 
> I'd suggest that other OSes set the `dc_cursor_attributes
> attribute_flags.bits.ENABLE_CURSOR_DEGAMMA` to true by default, rather
> than removing the mechanism that is valid for the Linux driver. Similar
> to what the Linux AMD DM does for the implicit sRGB [2][3], but in their
> case, they just need to initialize with 1.
> 

That's a good suggestion and I started that conversation with
Windows devs.

Is there an IGT test that would test for this behavior? Without
an IGT test I think we're apt to end back here again at some
point.

Harry

> Finally, thanks Michel for pointing this issue out to me and noticing
> the similarity to previous solution.
> 
> [1] https://gitlab.freedesktop.org/agd5f/linux/-/commit/d9fbd64e8e317
> [2] https://gitlab.freedesktop.org/agd5f/linux/-/commit/857b835f
> [3] https://gitlab.freedesktop.org/agd5f/linux/-/commit/66eba12a
> 
> Best Regards,
> 
> Melissa
> 
>  drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c
> index 1236e0f9a256..712aff7e17f7 100644
> --- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c
> +++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c
> @@ -120,10 +120,11 @@ void dpp401_set_cursor_attributes(
>  	enum dc_cursor_color_format color_format = cursor_attributes->color_format;
>  	int cur_rom_en = 0;
>  
> -	// DCN4 should always do Cursor degamma for Cursor Color modes
>  	if (color_format == CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA ||
>  		color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA) {
> -		cur_rom_en = 1;
> +		if (cursor_attributes->attribute_flags.bits.ENABLE_CURSOR_DEGAMMA) {
> +			cur_rom_en = 1;
> +		}
>  	}
>  
>  	REG_UPDATE_3(CURSOR0_CONTROL,