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

zhoucm1 david1.zhou at amd.com
Wed Jun 22 06:01:39 UTC 2016



On 2016年06月22日 11:58, Deucher, Alexander wrote:
>> -----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.
Yeah, I see your mean, but stop_mc_access must be use by matching 
resume_mc_access(), since there is a parameter "struct 
amdgpu_mode_mc_save *save" in them. gmc mc_program has one pair of them.
So I feel we still need to add resume_mc_access().

in addition to that, I add the wait_for_mc_idle after stop_mc_access 
when I gone through code again.

I will send this patch again with V2.

Regards,
David Zhou
>
> 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



More information about the amd-gfx mailing list