> -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf > Of Christian König > Sent: Wednesday, July 13, 2016 9:01 AM > To: Zhou, David(ChunMing); amd-gfx at lists.freedesktop.org > Subject: Re: [PATCH 01/10] drm/amdgpu: is_dislay_hung will be called by soft > reset > > The set looks good in general, but you use the same old approach we had > for radeon. Alex, what do you think about this? Should we stick with > that approach or rather start over? > > I think it would be better if we can get rid of the AMDGPU_RESET_ enum > and use a per IP block based function to figure out if a block is hung > or not. > > Then we can have a per block flag which tels us if a hung of this block > can be cleared by a soft reset or needs a hard reset. > > Then last but not least we reset only the hung blocks if possible. This should be implemented at the IP level, not as a new asic callback. We just never flushed out the design since it wasn't working at the time. I think we should add any new callbacks to the IP functions and then we should use either the IP idle checks or add a new check_soft_reset callback to set the flags. Then in amdgpu_device.c we could have a core soft_reset that just walks the IP list and calls the callbacks. Something like: +static int amdgpu_soft_reset(struct amdgpu_device *adev) +{ + int i, r, failied = 0; + + for (i = 0; i < adev->num_ip_blocks; i++) { + if (!adev->ip_block_status[i].valid) + continue; + if (adev->ip_blocks[i].funcs->check_soft_reset((void *)adev)) { + r = adev->ip_blocks[i].funcs->soft_reset((void *)adev); + if (r) { + DRM_ERROR("soft_reset %d failed %d\n", i, r); + failed = r; + } + } + } + + return failed; +} No need for pre/post callbacks, that call all be handled in the soft_reset callback directly. Alex > > Regards, > Christian. > > Am 13.07.2016 um 12:32 schrieb Chunming Zhou: > > Change-Id: Ie81f6b1124aeb4304f59f92282fe8b18fb06a693 > > Signed-off-by: Chunming Zhou <David1.Zhou at amd.com> > > --- > > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v10_0.h | 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v11_0.h | 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v8_0.h | 2 +- > > drivers/gpu/drm/amd/dal/amdgpu_dm/amdgpu_dm.c | 6 +++--- > > 7 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > > index 7554dd7..a592029 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > > @@ -532,7 +532,7 @@ static u32 dce_v10_0_hpd_get_gpio_reg(struct > amdgpu_device *adev) > > return mmDC_GPIO_HPD_A; > > } > > > > -static bool dce_v10_0_is_display_hung(struct amdgpu_device *adev) > > +bool dce_v10_0_is_display_hung(struct amdgpu_device *adev) > > { > > u32 crtc_hung = 0; > > u32 crtc_status[6]; > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.h > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.h > > index 3947956..77a732c 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.h > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.h > > @@ -32,5 +32,5 @@ void dce_v10_0_stop_mc_access(struct > amdgpu_device *adev, > > struct amdgpu_mode_mc_save *save); > > void dce_v10_0_resume_mc_access(struct amdgpu_device *adev, > > struct amdgpu_mode_mc_save *save); > > - > > +bool dce_v10_0_is_display_hung(struct amdgpu_device *adev); > > #endif > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > > index d9c9b88..e18bd20 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > > @@ -549,7 +549,7 @@ static u32 dce_v11_0_hpd_get_gpio_reg(struct > amdgpu_device *adev) > > return mmDC_GPIO_HPD_A; > > } > > > > -static bool dce_v11_0_is_display_hung(struct amdgpu_device *adev) > > +bool dce_v11_0_is_display_hung(struct amdgpu_device *adev) > > { > > u32 crtc_hung = 0; > > u32 crtc_status[6]; > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.h > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.h > > index dc6ff04..fa1884c 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.h > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.h > > @@ -32,5 +32,5 @@ void dce_v11_0_stop_mc_access(struct > amdgpu_device *adev, > > struct amdgpu_mode_mc_save *save); > > void dce_v11_0_resume_mc_access(struct amdgpu_device *adev, > > struct amdgpu_mode_mc_save *save); > > - > > +bool dce_v11_0_is_display_hung(struct amdgpu_device *adev); > > #endif > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > > index 14ac085..4863504 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > > @@ -478,7 +478,7 @@ static u32 dce_v8_0_hpd_get_gpio_reg(struct > amdgpu_device *adev) > > return mmDC_GPIO_HPD_A; > > } > > > > -static bool dce_v8_0_is_display_hung(struct amdgpu_device *adev) > > +bool dce_v8_0_is_display_hung(struct amdgpu_device *adev) > > { > > u32 crtc_hung = 0; > > u32 crtc_status[6]; > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.h > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.h > > index 4bb72ab..53f643c 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.h > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.h > > @@ -32,5 +32,5 @@ void dce_v8_0_stop_mc_access(struct > amdgpu_device *adev, > > struct amdgpu_mode_mc_save *save); > > void dce_v8_0_resume_mc_access(struct amdgpu_device *adev, > > struct amdgpu_mode_mc_save *save); > > - > > +bool dce_v8_0_is_display_hung(struct amdgpu_device *adev); > > #endif > > diff --git a/drivers/gpu/drm/amd/dal/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/dal/amdgpu_dm/amdgpu_dm.c > > index ca64158..0ea0956 100644 > > --- a/drivers/gpu/drm/amd/dal/amdgpu_dm/amdgpu_dm.c > > +++ b/drivers/gpu/drm/amd/dal/amdgpu_dm/amdgpu_dm.c > > @@ -1301,7 +1301,7 @@ static const struct amdgpu_display_funcs > dm_dce_v8_0_display_funcs = { > > .bandwidth_update = dm_bandwidth_update, /* called > unconditionally */ > > .vblank_get_counter = dm_vblank_get_counter,/* called > unconditionally */ > > .vblank_wait = NULL, > > - .is_display_hung = NULL, /* not called anywhere */ > > + .is_display_hung = dce_v8_0_is_display_hung, > > .backlight_set_level = > > dm_set_backlight_level,/* called unconditionally */ > > .backlight_get_level = > > @@ -1325,7 +1325,7 @@ static const struct amdgpu_display_funcs > dm_dce_v10_0_display_funcs = { > > .bandwidth_update = dm_bandwidth_update, /* called > unconditionally */ > > .vblank_get_counter = dm_vblank_get_counter,/* called > unconditionally */ > > .vblank_wait = NULL, > > - .is_display_hung = NULL, /* not called anywhere */ > > + .is_display_hung = dce_v10_0_is_display_hung, > > .backlight_set_level = > > dm_set_backlight_level,/* called unconditionally */ > > .backlight_get_level = > > @@ -1349,7 +1349,7 @@ static const struct amdgpu_display_funcs > dm_dce_v11_0_display_funcs = { > > .bandwidth_update = dm_bandwidth_update, /* called > unconditionally */ > > .vblank_get_counter = dm_vblank_get_counter,/* called > unconditionally */ > > .vblank_wait = NULL, > > - .is_display_hung = NULL, /* not called anywhere */ > > + .is_display_hung = dce_v11_0_is_display_hung, > > .backlight_set_level = > > dm_set_backlight_level,/* called unconditionally */ > > .backlight_get_level = > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
