* [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()
@ 2022-10-20 14:36 Alex Deucher
2022-10-24 6:20 ` Quan, Evan
0 siblings, 1 reply; 6+ messages in thread
From: Alex Deucher @ 2022-10-20 14:36 UTC (permalink / raw)
To: amd-gfx, dri-devel; +Cc: Alex Deucher, Thomas Zimmermann
It's used to restore the fbdev console, but as amdgpu uses
generic fbdev emulation, the console is being restored by the
DRM client helpers already. See the call to drm_client_dev_restore()
in drm_lastclose().
Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of setting up AMD own's.")
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index fe23e09eec98..474b9f40f792 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
*/
void amdgpu_driver_lastclose_kms(struct drm_device *dev)
{
- drm_fb_helper_lastclose(dev);
vga_switcheroo_process_delayed_switch();
}
--
2.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()
2022-10-20 14:36 [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose() Alex Deucher
@ 2022-10-24 6:20 ` Quan, Evan
2022-10-24 7:32 ` Thomas Zimmermann
0 siblings, 1 reply; 6+ messages in thread
From: Quan, Evan @ 2022-10-24 6:20 UTC (permalink / raw)
To: Deucher, Alexander, amd-gfx, dri-devel
Cc: Deucher, Alexander, Thomas Zimmermann
[AMD Official Use Only - General]
Reviewed-by: Evan Quan <evan.quan@amd.com>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Thursday, October 20, 2022 10:36 PM
> To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Thomas
> Zimmermann <tzimmermann@suse.de>
> Subject: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in
> lastclose()
>
> It's used to restore the fbdev console, but as amdgpu uses
> generic fbdev emulation, the console is being restored by the
> DRM client helpers already. See the call to drm_client_dev_restore()
> in drm_lastclose().
>
> Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of
> setting up AMD own's.")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index fe23e09eec98..474b9f40f792 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev,
> void *data, struct drm_file *filp)
> */
> void amdgpu_driver_lastclose_kms(struct drm_device *dev)
> {
> - drm_fb_helper_lastclose(dev);
> vga_switcheroo_process_delayed_switch();
> }
>
> --
> 2.37.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()
2022-10-24 6:20 ` Quan, Evan
@ 2022-10-24 7:32 ` Thomas Zimmermann
2022-10-24 16:48 ` Alex Deucher
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Zimmermann @ 2022-10-24 7:32 UTC (permalink / raw)
To: Quan, Evan, Deucher, Alexander, amd-gfx, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2396 bytes --]
Hi
Am 24.10.22 um 08:20 schrieb Quan, Evan:
> [AMD Official Use Only - General]
>
> Reviewed-by: Evan Quan <evan.quan@amd.com>
>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
>> Deucher
>> Sent: Thursday, October 20, 2022 10:36 PM
>> To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Thomas
>> Zimmermann <tzimmermann@suse.de>
>> Subject: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in
>> lastclose()
>>
>> It's used to restore the fbdev console, but as amdgpu uses
>> generic fbdev emulation, the console is being restored by the
>> DRM client helpers already. See the call to drm_client_dev_restore()
>> in drm_lastclose().
>>
>> Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of
>> setting up AMD own's.")
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index fe23e09eec98..474b9f40f792 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev,
>> void *data, struct drm_file *filp)
>> */
>> void amdgpu_driver_lastclose_kms(struct drm_device *dev)
>> {
>> - drm_fb_helper_lastclose(dev);
>> vga_switcheroo_process_delayed_switch();
>> }
Without the call to drm_fb_helper_lastclose(), the console emulation
will be restored by drm_client_dev_restore() from drm_lastclose(). [1]
It means that it's now changing order with the call to
vga_switcheroo_process_delay_switch(). Can this become a problem?
I looked at the other callers of that function. Most restore the console
before doing the switcheroo. Nouveau doesn't seem to care about the
console at all.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.0.3/source/drivers/gpu/drm/drm_file.c#L467
>>
>> --
>> 2.37.3
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()
2022-10-24 7:32 ` Thomas Zimmermann
@ 2022-10-24 16:48 ` Alex Deucher
2022-10-25 7:25 ` Thomas Zimmermann
0 siblings, 1 reply; 6+ messages in thread
From: Alex Deucher @ 2022-10-24 16:48 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: Deucher, Alexander, Quan, Evan, dri-devel, amd-gfx
On Mon, Oct 24, 2022 at 3:33 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 24.10.22 um 08:20 schrieb Quan, Evan:
> > [AMD Official Use Only - General]
> >
> > Reviewed-by: Evan Quan <evan.quan@amd.com>
> >
> >> -----Original Message-----
> >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> >> Deucher
> >> Sent: Thursday, October 20, 2022 10:36 PM
> >> To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Thomas
> >> Zimmermann <tzimmermann@suse.de>
> >> Subject: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in
> >> lastclose()
> >>
> >> It's used to restore the fbdev console, but as amdgpu uses
> >> generic fbdev emulation, the console is being restored by the
> >> DRM client helpers already. See the call to drm_client_dev_restore()
> >> in drm_lastclose().
> >>
> >> Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of
> >> setting up AMD own's.")
> >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
> >> 1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> index fe23e09eec98..474b9f40f792 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> @@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev,
> >> void *data, struct drm_file *filp)
> >> */
> >> void amdgpu_driver_lastclose_kms(struct drm_device *dev)
> >> {
> >> - drm_fb_helper_lastclose(dev);
> >> vga_switcheroo_process_delayed_switch();
> >> }
>
> Without the call to drm_fb_helper_lastclose(), the console emulation
> will be restored by drm_client_dev_restore() from drm_lastclose(). [1]
> It means that it's now changing order with the call to
> vga_switcheroo_process_delay_switch(). Can this become a problem?
>
> I looked at the other callers of that function. Most restore the console
> before doing the switcheroo. Nouveau doesn't seem to care about the
> console at all.
I don't know off hand. I suppose if the switch powered down the GPU
and then we tried to restore it's console state that would be a
problem, but it looks like vga_switchto_stage2() just powers down the
GPU without going through suspend so I'm not sure if this actually
worked correctly? What are the potential problems with calling
drm_fb_helper_lastclose twice?
Alex
>
> Best regards
> Thomas
>
> [1]
> https://elixir.bootlin.com/linux/v6.0.3/source/drivers/gpu/drm/drm_file.c#L467
>
> >>
> >> --
> >> 2.37.3
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()
2022-10-24 16:48 ` Alex Deucher
@ 2022-10-25 7:25 ` Thomas Zimmermann
2022-10-27 18:15 ` Alex Deucher
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Zimmermann @ 2022-10-25 7:25 UTC (permalink / raw)
To: Alex Deucher; +Cc: Deucher, Alexander, Quan, Evan, amd-gfx, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 3753 bytes --]
Hi
Am 24.10.22 um 18:48 schrieb Alex Deucher:
> On Mon, Oct 24, 2022 at 3:33 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 24.10.22 um 08:20 schrieb Quan, Evan:
>>> [AMD Official Use Only - General]
>>>
>>> Reviewed-by: Evan Quan <evan.quan@amd.com>
>>>
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
>>>> Deucher
>>>> Sent: Thursday, October 20, 2022 10:36 PM
>>>> To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Thomas
>>>> Zimmermann <tzimmermann@suse.de>
>>>> Subject: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in
>>>> lastclose()
>>>>
>>>> It's used to restore the fbdev console, but as amdgpu uses
>>>> generic fbdev emulation, the console is being restored by the
>>>> DRM client helpers already. See the call to drm_client_dev_restore()
>>>> in drm_lastclose().
>>>>
>>>> Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of
>>>> setting up AMD own's.")
>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
>>>> 1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> index fe23e09eec98..474b9f40f792 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> @@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev,
>>>> void *data, struct drm_file *filp)
>>>> */
>>>> void amdgpu_driver_lastclose_kms(struct drm_device *dev)
>>>> {
>>>> - drm_fb_helper_lastclose(dev);
>>>> vga_switcheroo_process_delayed_switch();
>>>> }
>>
>> Without the call to drm_fb_helper_lastclose(), the console emulation
>> will be restored by drm_client_dev_restore() from drm_lastclose(). [1]
>> It means that it's now changing order with the call to
>> vga_switcheroo_process_delay_switch(). Can this become a problem?
>>
>> I looked at the other callers of that function. Most restore the console
>> before doing the switcheroo. Nouveau doesn't seem to care about the
>> console at all.
>
> I don't know off hand. I suppose if the switch powered down the GPU
> and then we tried to restore it's console state that would be a
> problem, but it looks like vga_switchto_stage2() just powers down the
> GPU without going through suspend so I'm not sure if this actually
> worked correctly? What are the potential problems with calling
> drm_fb_helper_lastclose twice?
It should do fbdev console modesetting twice; so no problem.
AFAIU calling vga switcheroo does nothing for devices without support.
So let's call vga_switcheroo_process_delayed_switch() at the very end of
drm_lastclose() and remove the amdgpu callback entirely. [1] This
should not be a problem and if, we can refactor the whole function.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_file.c#L468
>
> Alex
>
>>
>> Best regards
>> Thomas
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.0.3/source/drivers/gpu/drm/drm_file.c#L467
>>
>>>>
>>>> --
>>>> 2.37.3
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()
2022-10-25 7:25 ` Thomas Zimmermann
@ 2022-10-27 18:15 ` Alex Deucher
0 siblings, 0 replies; 6+ messages in thread
From: Alex Deucher @ 2022-10-27 18:15 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: Deucher, Alexander, Quan, Evan, amd-gfx, dri-devel
On Tue, Oct 25, 2022 at 3:25 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 24.10.22 um 18:48 schrieb Alex Deucher:
> > On Mon, Oct 24, 2022 at 3:33 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi
> >>
> >> Am 24.10.22 um 08:20 schrieb Quan, Evan:
> >>> [AMD Official Use Only - General]
> >>>
> >>> Reviewed-by: Evan Quan <evan.quan@amd.com>
> >>>
> >>>> -----Original Message-----
> >>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> >>>> Deucher
> >>>> Sent: Thursday, October 20, 2022 10:36 PM
> >>>> To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> >>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Thomas
> >>>> Zimmermann <tzimmermann@suse.de>
> >>>> Subject: [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in
> >>>> lastclose()
> >>>>
> >>>> It's used to restore the fbdev console, but as amdgpu uses
> >>>> generic fbdev emulation, the console is being restored by the
> >>>> DRM client helpers already. See the call to drm_client_dev_restore()
> >>>> in drm_lastclose().
> >>>>
> >>>> Fixes: 087451f372bf76 ("drm/amdgpu: use generic fb helpers instead of
> >>>> setting up AMD own's.")
> >>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>>> ---
> >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 -
> >>>> 1 file changed, 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>>> index fe23e09eec98..474b9f40f792 100644
> >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>>> @@ -1106,7 +1106,6 @@ int amdgpu_info_ioctl(struct drm_device *dev,
> >>>> void *data, struct drm_file *filp)
> >>>> */
> >>>> void amdgpu_driver_lastclose_kms(struct drm_device *dev)
> >>>> {
> >>>> - drm_fb_helper_lastclose(dev);
> >>>> vga_switcheroo_process_delayed_switch();
> >>>> }
> >>
> >> Without the call to drm_fb_helper_lastclose(), the console emulation
> >> will be restored by drm_client_dev_restore() from drm_lastclose(). [1]
> >> It means that it's now changing order with the call to
> >> vga_switcheroo_process_delay_switch(). Can this become a problem?
> >>
> >> I looked at the other callers of that function. Most restore the console
> >> before doing the switcheroo. Nouveau doesn't seem to care about the
> >> console at all.
> >
> > I don't know off hand. I suppose if the switch powered down the GPU
> > and then we tried to restore it's console state that would be a
> > problem, but it looks like vga_switchto_stage2() just powers down the
> > GPU without going through suspend so I'm not sure if this actually
> > worked correctly? What are the potential problems with calling
> > drm_fb_helper_lastclose twice?
>
> It should do fbdev console modesetting twice; so no problem.
>
> AFAIU calling vga switcheroo does nothing for devices without support.
> So let's call vga_switcheroo_process_delayed_switch() at the very end of
> drm_lastclose() and remove the amdgpu callback entirely. [1] This
> should not be a problem and if, we can refactor the whole function.
Sounds like a plan. Thanks!
Alex
>
> Best regards
> Thomas
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_file.c#L468
>
> >
> > Alex
> >
> >>
> >> Best regards
> >> Thomas
> >>
> >> [1]
> >> https://elixir.bootlin.com/linux/v6.0.3/source/drivers/gpu/drm/drm_file.c#L467
> >>
> >>>>
> >>>> --
> >>>> 2.37.3
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> (HRB 36809, AG Nürnberg)
> >> Geschäftsführer: Ivo Totev
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-27 18:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-20 14:36 [PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose() Alex Deucher
2022-10-24 6:20 ` Quan, Evan
2022-10-24 7:32 ` Thomas Zimmermann
2022-10-24 16:48 ` Alex Deucher
2022-10-25 7:25 ` Thomas Zimmermann
2022-10-27 18:15 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.