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
