diff mbox series

drm/mgag200: Fix gamma lut not initialized.

Message ID 20230510085451.226546-1-jfalempe@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm/mgag200: Fix gamma lut not initialized. | expand

Commit Message

Jocelyn Falempe May 10, 2023, 8:54 a.m. UTC 
When mgag200 switched from simple KMS to regular atomic helpers,
the initialization of the gamma settings was lost.
This leads to a black screen, if the bios/uefi doesn't use the same
pixel color depth.

Link: https://bugzilla.redhat.com/show_bug.cgi?id=2171155
Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular atomic helpers")
Tested-by: Phil Oester <kernel@linuxace.com>
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +++++
 1 file changed, 5 insertions(+)


base-commit: 1baf9127c482a3a58aef81d92ae751798e2db202

Comments

Thomas Zimmermann May 10, 2023, 9:15 a.m. UTC | #1 
Hi,

oh great! Thank you for fixing this bug. And sorry that I broke it.

Am 10.05.23 um 10:54 schrieb Jocelyn Falempe:
> When mgag200 switched from simple KMS to regular atomic helpers,
> the initialization of the gamma settings was lost.
> This leads to a black screen, if the bios/uefi doesn't use the same
> pixel color depth.
> 
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2171155
> Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular atomic helpers")
> Tested-by: Phil Oester <kernel@linuxace.com>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>

Also needs:

Cc: <stable@vger.kernel.org> # v6.1+

In terms of what it does:

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

but the patch is apparently for against an old version. (v6.1?) The code 
in mgag200_crtc_helper_atomic_enable has changed quite a bit.

Best regards
Thomas

> ---
>   drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 461da1409fdf..911d46741e40 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -819,6 +819,11 @@ static void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>   	else if (mdev->type == G200_EV)
>   		mgag200_g200ev_set_hiprilvl(mdev);
>   
> +	if (crtc_state->gamma_lut)
> +		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
> +	else
> +		mgag200_crtc_set_gamma_linear(mdev, format);
> +
>   	mgag200_enable_display(mdev);
>   
>   	if (mdev->type == G200_WB || mdev->type == G200_EW3)
> 
> base-commit: 1baf9127c482a3a58aef81d92ae751798e2db202
Jocelyn Falempe May 10, 2023, 9:29 a.m. UTC | #2 
On 10/05/2023 11:15, Thomas Zimmermann wrote:
> Hi,
> 
> oh great! Thank you for fixing this bug. And sorry that I broke it.
> 
> Am 10.05.23 um 10:54 schrieb Jocelyn Falempe:
>> When mgag200 switched from simple KMS to regular atomic helpers,
>> the initialization of the gamma settings was lost.
>> This leads to a black screen, if the bios/uefi doesn't use the same
>> pixel color depth.
>>
>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2171155
>> Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular 
>> atomic helpers")
>> Tested-by: Phil Oester <kernel@linuxace.com>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Also needs:
> 
> Cc: <stable@vger.kernel.org> # v6.1+

Should I send a v2 with this added ?
> 
> In terms of what it does:
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> but the patch is apparently for against an old version. (v6.1?) The code 
> in mgag200_crtc_helper_atomic_enable has changed quite a bit.

Yes, I based it on the culprit commit 1baf9127c482, but it applies 
cleanly with git am -3 on top of v6.3

> 
> Best regards
> Thomas
> 
>> ---
>>   drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
>> b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 461da1409fdf..911d46741e40 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -819,6 +819,11 @@ static void 
>> mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>>       else if (mdev->type == G200_EV)
>>           mgag200_g200ev_set_hiprilvl(mdev);
>> +    if (crtc_state->gamma_lut)
>> +        mgag200_crtc_set_gamma(mdev, format, 
>> crtc_state->gamma_lut->data);
>> +    else
>> +        mgag200_crtc_set_gamma_linear(mdev, format);
>> +
>>       mgag200_enable_display(mdev);
>>       if (mdev->type == G200_WB || mdev->type == G200_EW3)
>>
>> base-commit: 1baf9127c482a3a58aef81d92ae751798e2db202
>
Thomas Zimmermann May 10, 2023, 10:13 a.m. UTC | #3 
Hi

Am 10.05.23 um 11:29 schrieb Jocelyn Falempe:
> On 10/05/2023 11:15, Thomas Zimmermann wrote:
>> Hi,
>>
>> oh great! Thank you for fixing this bug. And sorry that I broke it.
>>
>> Am 10.05.23 um 10:54 schrieb Jocelyn Falempe:
>>> When mgag200 switched from simple KMS to regular atomic helpers,
>>> the initialization of the gamma settings was lost.
>>> This leads to a black screen, if the bios/uefi doesn't use the same
>>> pixel color depth.
>>>
>>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2171155
>>> Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular 
>>> atomic helpers")
>>> Tested-by: Phil Oester <kernel@linuxace.com>
>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>
>> Also needs:
>>
>> Cc: <stable@vger.kernel.org> # v6.1+
> 
> Should I send a v2 with this added ?

Yes, please.

>>
>> In terms of what it does:
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>> but the patch is apparently for against an old version. (v6.1?) The 
>> code in mgag200_crtc_helper_atomic_enable has changed quite a bit.
> 
> Yes, I based it on the culprit commit 1baf9127c482, but it applies 
> cleanly with git am -3 on top of v6.3

Rather send a patch against drm-misc-fixes and let backporters sort it 
out. That's current practice AFAIK. You could also reply to your v2 
patch mail with a link to the v1 patch.

Best regards
Thomas

> 
>>
>> Best regards
>> Thomas
>>
>>> ---
>>>   drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
>>> b/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> index 461da1409fdf..911d46741e40 100644
>>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> @@ -819,6 +819,11 @@ static void 
>>> mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>>>       else if (mdev->type == G200_EV)
>>>           mgag200_g200ev_set_hiprilvl(mdev);
>>> +    if (crtc_state->gamma_lut)
>>> +        mgag200_crtc_set_gamma(mdev, format, 
>>> crtc_state->gamma_lut->data);
>>> +    else
>>> +        mgag200_crtc_set_gamma_linear(mdev, format);
>>> +
>>>       mgag200_enable_display(mdev);
>>>       if (mdev->type == G200_WB || mdev->type == G200_EW3)
>>>
>>> base-commit: 1baf9127c482a3a58aef81d92ae751798e2db202
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 461da1409fdf..911d46741e40 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -819,6 +819,11 @@  static void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 	else if (mdev->type == G200_EV)
 		mgag200_g200ev_set_hiprilvl(mdev);
 
+	if (crtc_state->gamma_lut)
+		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
+	else
+		mgag200_crtc_set_gamma_linear(mdev, format);
+
 	mgag200_enable_display(mdev);
 
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)