On Thu, Feb 27, 2025 at 2:01 PM Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
>
> On Thu, Feb 27, 2025 at 1:52 PM Alexandre Demers
> <alexandre.f.demers@xxxxxxxxx> wrote:
> >
> > On Thu, Feb 27, 2025 at 9:23 AM Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
> > >
> > > On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
> > > <alexandre.f.demers@xxxxxxxxx> wrote:
> > > >
> > > > DCE6 was missing soft reset, but it was easily identifiable under radeon.
> > > > This should be it, pretty much as it is done under DCE8 and DCE10.
> > > >
> > > > Signed-off-by: Alexandre Demers <alexandre.f.demers@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> > > > 1 file changed, 57 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > index bd763fde1c50..254cb73324c6 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> > > > return mmDC_GPIO_HPD_A;
> > > > }
> > > >
> > > > +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> > > > +{
> > > > + u32 crtc_hung = 0;
> > > > + u32 crtc_status[6];
> > > > + u32 i, j, tmp;
> > > > +
> > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> > > > + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > + crtc_hung |= (1 << i);
> > > > + }
> > > > + }
> > > > +
> > > > + for (j = 0; j < 10; j++) {
> > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > + if (crtc_hung & (1 << i)) {
> > > > + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > + if (tmp != crtc_status[i])
> > > > + crtc_hung &= ~(1 << i);
> > > > + }
> > > > + }
> > > > + if (crtc_hung == 0)
> > > > + return false;
> > > > + udelay(100);
> > > > + }
> > > > +
> > > > + return true;
> > > > +}
> > > > +
> > > > static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> > > > bool render)
> > > > {
> > > > if (!render)
> > > > WREG32(mmVGA_RENDER_CONTROL,
> > > > RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> > > > -
> > > > }
> > > >
> > > > static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> > > > {
> > > > + int num_crtc = 0;
> > > > +
> > > > switch (adev->asic_type) {
> > > > case CHIP_TAHITI:
> > > > case CHIP_PITCAIRN:
> > > > case CHIP_VERDE:
> > > > - return 6;
> > > > + num_crtc = 6;
> > > > case CHIP_OLAND:
> > > > - return 2;
> > > > + num_crtc = 2;
> > > > default:
> > > > - return 0;
> > > > + num_crtc = 0;
> > > > }
> > > > + return num_crtc;
> > >
> > > Any particular reason for this change? It just adds an extra variable.
> > >
> > > Alex
> >
> > Just for uniformisation with DCE8 and DCE10. We could also remove the
> > variable and use returns everywhere.
> >
> > Any preferences?
>
> ah, ok. I think the direct returns are cleaner.
>
> Alex
Ok. Should I submit a new version or should this patch go through and
be changed everywhere in a different patch?
Alexandre
>
> > Alexandre
> >
> > >
> > > > }
> > > >
> > > > void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> > > > @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
> > > >
> > > > static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> > > > {
> > > > - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> > > > + u32 srbm_soft_reset = 0, tmp;
> > > > + struct amdgpu_device *adev = ip_block->adev;
> > > > +
> > > > + if (dce_v6_0_is_display_hung(adev))
> > > > + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> > > > +
> > > > + if (srbm_soft_reset) {
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > + tmp |= srbm_soft_reset;
> > > > + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > +
> > > > + udelay(50);
> > > > +
> > > > + tmp &= ~srbm_soft_reset;
> > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > +
> > > > + /* Wait a little for things to settle down */
> > > > + udelay(50);
> > > > + }
> > > > return 0;
> > > > }
> > > >
> > > > --
> > > > 2.48.1
> > > >
