diff mbox series

drm/amd/display: check attr flag before set cursor degamma on DCN3+

Message ID 20230731083505.1500965-1-mwen@igalia.com (mailing list archive)
State New, archived
Headers show
Series drm/amd/display: check attr flag before set cursor degamma on DCN3+ | expand

Commit Message

Melissa Wen July 31, 2023, 8:35 a.m. UTC
Don't set predefined degamma curve to cursor plane if the cursor
attribute flag is not set. Applying a degamma curve to the cursor by
default breaks userspace expectation. Checking the flag before
performing any color transformation prevents too dark cursor gamma in
DCN3+ on many Linux desktop environment (KDE Plasma, GNOME,
wlroots-based, etc.) as reported at:
- https://gitlab.freedesktop.org/drm/amd/-/issues/1513

This is the same approach followed by DCN2 drivers where the issue is
not present.

Fixes: 03f54d7d3448 ("drm/amd/display: Add DCN3 DPP")
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Harry Wentland July 31, 2023, 2:47 p.m. UTC | #1
On 2023-07-31 04:35, Melissa Wen wrote:
> Don't set predefined degamma curve to cursor plane if the cursor
> attribute flag is not set. Applying a degamma curve to the cursor by
> default breaks userspace expectation. Checking the flag before
> performing any color transformation prevents too dark cursor gamma in
> DCN3+ on many Linux desktop environment (KDE Plasma, GNOME,
> wlroots-based, etc.) as reported at:
> - https://gitlab.freedesktop.org/drm/amd/-/issues/1513
> 
> This is the same approach followed by DCN2 drivers where the issue is
> not present.
> 
> Fixes: 03f54d7d3448 ("drm/amd/display: Add DCN3 DPP")
> Signed-off-by: Melissa Wen <mwen@igalia.com>

Thanks. Good find.

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

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> index e5b7ef7422b8..50dc83404644 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> @@ -357,8 +357,11 @@ void dpp3_set_cursor_attributes(
>  	int cur_rom_en = 0;
>  
>  	if (color_format == CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA ||
> -		color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA)
> -		cur_rom_en = 1;
> +		color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA) {
> +		if (cursor_attributes->attribute_flags.bits.ENABLE_CURSOR_DEGAMMA) {
> +			cur_rom_en = 1;
> +		}
> +	}
>  
>  	REG_UPDATE_3(CURSOR0_CONTROL,
>  			CUR0_MODE, color_format,
Alex Hung Aug. 1, 2023, 4:07 a.m. UTC | #2
Tested-by: Alex Hung <alex.hung@amd.com>

On 2023-07-31 02:35, Melissa Wen wrote:
> Don't set predefined degamma curve to cursor plane if the cursor
> attribute flag is not set. Applying a degamma curve to the cursor by
> default breaks userspace expectation. Checking the flag before
> performing any color transformation prevents too dark cursor gamma in
> DCN3+ on many Linux desktop environment (KDE Plasma, GNOME,
> wlroots-based, etc.) as reported at:
> - https://gitlab.freedesktop.org/drm/amd/-/issues/1513
> 
> This is the same approach followed by DCN2 drivers where the issue is
> not present.
> 
> Fixes: 03f54d7d3448 ("drm/amd/display: Add DCN3 DPP")
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>   drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> index e5b7ef7422b8..50dc83404644 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> @@ -357,8 +357,11 @@ void dpp3_set_cursor_attributes(
>   	int cur_rom_en = 0;
>   
>   	if (color_format == CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA ||
> -		color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA)
> -		cur_rom_en = 1;
> +		color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA) {
> +		if (cursor_attributes->attribute_flags.bits.ENABLE_CURSOR_DEGAMMA) {
> +			cur_rom_en = 1;
> +		}
> +	}
>   
>   	REG_UPDATE_3(CURSOR0_CONTROL,
>   			CUR0_MODE, color_format,
Harry Wentland Aug. 2, 2023, 3:35 p.m. UTC | #3
Thanks. Change is merged to amd-staging-drm-next.

Harry

On 2023-08-01 00:07, Alex Hung wrote:
> Tested-by: Alex Hung <alex.hung@amd.com>
> 
> On 2023-07-31 02:35, Melissa Wen wrote:
>> Don't set predefined degamma curve to cursor plane if the cursor
>> attribute flag is not set. Applying a degamma curve to the cursor by
>> default breaks userspace expectation. Checking the flag before
>> performing any color transformation prevents too dark cursor gamma in
>> DCN3+ on many Linux desktop environment (KDE Plasma, GNOME,
>> wlroots-based, etc.) as reported at:
>> - https://gitlab.freedesktop.org/drm/amd/-/issues/1513
>>
>> This is the same approach followed by DCN2 drivers where the issue is
>> not present.
>>
>> Fixes: 03f54d7d3448 ("drm/amd/display: Add DCN3 DPP")
>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>> ---
>>   drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
>> index e5b7ef7422b8..50dc83404644 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
>> @@ -357,8 +357,11 @@ void dpp3_set_cursor_attributes(
>>       int cur_rom_en = 0;
>>         if (color_format == CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA ||
>> -        color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA)
>> -        cur_rom_en = 1;
>> +        color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA) {
>> +        if (cursor_attributes->attribute_flags.bits.ENABLE_CURSOR_DEGAMMA) {
>> +            cur_rom_en = 1;
>> +        }
>> +    }
>>         REG_UPDATE_3(CURSOR0_CONTROL,
>>               CUR0_MODE, color_format,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
index e5b7ef7422b8..50dc83404644 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
@@ -357,8 +357,11 @@  void dpp3_set_cursor_attributes(
 	int cur_rom_en = 0;
 
 	if (color_format == CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA ||
-		color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA)
-		cur_rom_en = 1;
+		color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA) {
+		if (cursor_attributes->attribute_flags.bits.ENABLE_CURSOR_DEGAMMA) {
+			cur_rom_en = 1;
+		}
+	}
 
 	REG_UPDATE_3(CURSOR0_CONTROL,
 			CUR0_MODE, color_format,