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

[zhoucm1]

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.

> 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?

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