[PATCH 2/2] drm/amdgpu: stop/resume fb access when gpu reset

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of zhoucm1
> Sent: Tuesday, June 21, 2016 9:51 PM
> To: Alex Deucher; Deucher, Alexander
> Cc: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: stop/resume fb access when gpu
> reset
> 
> 
> 
> On 2016年06月21日 23:25, Alex Deucher wrote:
> > On Tue, Jun 21, 2016 at 9:21 AM, Deucher, Alexander
> > <Alexander.Deucher at amd.com> wrote:
> >>> -----Original Message-----
> >>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On
> Behalf
> >>> Of Chunming Zhou
> >>> Sent: Tuesday, June 21, 2016 4:12 AM
> >>> To: amd-gfx at lists.freedesktop.org
> >>> Cc: Zhou, David(ChunMing)
> >>> Subject: [PATCH 2/2] drm/amdgpu: stop/resume fb access when gpu
> reset
> >>>
> >>> Change-Id: I86b5c2b8c78a17ef43ade2a97ef8a33853650be0
> >>> Signed-off-by: Chunming Zhou <David1.Zhou at amd.com>
> >>> ---
> >>>   drivers/gpu/drm/amd/amdgpu/cik.c | 9 +++++++++
> >>>   drivers/gpu/drm/amd/amdgpu/vi.c  | 8 +++++++-
> >>>   2 files changed, 16 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c
> >>> b/drivers/gpu/drm/amd/amdgpu/cik.c
> >>> index 40f4fda..716efa8 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/cik.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/cik.c
> >>> @@ -1213,10 +1213,19 @@ static void
> >>> cik_set_bios_scratch_engine_hung(struct amdgpu_device *adev, bool
> hu
> >>>   static int cik_asic_reset(struct amdgpu_device *adev)
> >>>   {
> >>>        int r;
> >>> +     struct amdgpu_mode_mc_save save;
> >>> +
> >>>        cik_set_bios_scratch_engine_hung(adev, true);
> >>> +     /* Disable fb access */
> >>> +     if (adev->mode_info.num_crtc)
> >>> +             amdgpu_display_stop_mc_access(adev, &save);
> >> You don't need to check if there are crtcs or not before stopping the mc.
> The stop_mc_access and resume_mc_access callbacks will handle that
> automatically.  Also on chips with no DCE hardware, you still want to stop the
> mc.  Same for vi.c.
> > Nevermind, I was thinking of the actual gmc stop/resume mc functions.
> > So the code here with the num_crtc check is correct with respect to
> > disabling mc access in the dce block.  However, why do we need this?
> This sequence is actually by referring other drivers, they're doing it,
> the comment is:
>      // Disable VGA memory access if it is enabled and display requests
> to avoid any issue when resetting MC.

Ah, ok.  In that case, we should probably just call stop_mc_access.  That way the displays will stop requesting memory before the reset and the mc will be properly re-configured in gmc mc_program() which is called from gmc hw_init via gmc resume.

> 
> > The mc_stop sort of makes sense, but resuming after the reset doesn't
> > since the gpu state is reset after the reset so what's the point?
> > Wouldn't this already be taken care of by the ip specific suspend and
> > resume functions (e.g., gmc resume)?
> the amdgpu_display_stop/resume_mc_access are called by
> gmc_v8_0_mc_stop/resume, which are called in gmc_v8_0_soft_reset not in
> gmc_xxx_suspend/resume.
> do you mean adding gmc_xxx_mc_stop/resume to
> gmc_xxx_suspend/resume is
> making more sence?

I think all we need to do is stop the displays from accessing the mc.  No need call resume_mc_access() after the reset since the entire GPU will be reset and the dce block won't be back in shape until various atom tables as called.  Atom asic_init and amdgpu_resume() code should take care of that.

Alex

> 
> Regards,
> David Zhou
> >
> > Alex
> >
> >> Alex
> >>
> >>>        r = cik_gpu_pci_config_reset(adev);
> >>>
> >>> +     /* resume fb access */
> >>> +     if (adev->mode_info.num_crtc)
> >>> +             amdgpu_display_resume_mc_access(adev, &save);
> >>> +
> >>>        cik_set_bios_scratch_engine_hung(adev, false);
> >>>
> >>>        return r;
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> b/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> index 1ac0c91..497b817 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> @@ -633,10 +633,16 @@ static void
> vi_set_bios_scratch_engine_hung(struct
> >>> amdgpu_device *adev, bool hun
> >>>   static int vi_asic_reset(struct amdgpu_device *adev)
> >>>   {
> >>>        int r;
> >>> +     struct amdgpu_mode_mc_save save;
> >>>
> >>>        vi_set_bios_scratch_engine_hung(adev, true);
> >>> -
> >>> +     /* Disable fb access */
> >>> +     if (adev->mode_info.num_crtc)
> >>> +             amdgpu_display_stop_mc_access(adev, &save);
> >>>        r = vi_gpu_pci_config_reset(adev);
> >>> +     /* resume fb access */
> >>> +     if (adev->mode_info.num_crtc)
> >>> +             amdgpu_display_resume_mc_access(adev, &save);
> >>>
> >>>        vi_set_bios_scratch_engine_hung(adev, false);
> >>>
> >>> --
> >>> 1.9.1
> >>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx at lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx