[PATCH] drm/amdgpu: don't call drm_fb_helper_lastclose in lastclose()

[Alex Deucher]

On Tue, Oct 25, 2022 at 3:25 AM Thomas Zimmermann <tzimmermann at 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 at 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 at amd.com>
> >>>
> >>>> -----Original Message-----
> >>>> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Alex
> >>>> Deucher
> >>>> Sent: Thursday, October 20, 2022 10:36 PM
> >>>> To: amd-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> >>>> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Thomas
> >>>> Zimmermann <tzimmermann at 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 at suse.de>
> >>>> Signed-off-by: Alex Deucher <alexander.deucher at 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