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

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

[Melissa Wen]

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.

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,
-- 
2.47.2

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

[Harry Wentland]

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,

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

[Melissa Wen]

On 24/04/2025 16:10, Harry Wentland wrote:
>
> 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.
Indeed, it's a good idea. AFAIK, there is no IGT color test for cursor.
I'll work on it then, as it might save everyone time.

Thanks

Melissa
>
> 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,

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

[Shengyu Qu]

[-- Attachment #1.1.1: Type: text/plain, Size: 3194 bytes --]

Hi,

Personally I prefer we always disable cursor HW degamma, and this is 
what color pipeline patch series have done for cursor plane(actually all 
colorops are disabled on cursor plane and its background plane). Since 
cursor plane shares colorops with cursor's background plane.

Also, degamma a sRGB image with sRGB curve is not a good choice, or we 
have to fake screen TRC to sRGB curve while the screen actually is 
calibrated to power 2.2 curve.

Best regards,
Shengyu

在 2025/4/22 22:58, Melissa Wen 写道:
> 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.
> 
> 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,


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6977 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

[Melissa Wen]

On 28/04/2025 06:21, Shengyu Qu wrote:
> Hi,
>
> Personally I prefer we always disable cursor HW degamma, and this is 
> what color pipeline patch series have done for cursor plane(actually 
> all colorops are disabled on cursor plane and its background plane). 
> Since cursor plane shares colorops with cursor's background plane.

Hi Shengyu,

This feature isn't related to under-discussion plane color mgmt, neither 
the current atomic CRTC color properties, where this cursor degamma is 
always disable. It's only needed for DRM legacy gamma because of the 
implict sRGB done by AMD display driver for this property.

Best Regards,

Melissa

>
> Also, degamma a sRGB image with sRGB curve is not a good choice, or we 
> have to fake screen TRC to sRGB curve while the screen actually is 
> calibrated to power 2.2 curve.
>
> Best regards,
> Shengyu
>
> 在 2025/4/22 22:58, Melissa Wen 写道:
>> 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.
>>
>> 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,
>

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

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 4/22/25 08: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.
> 
> 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,