Re: [PATCH] drm/amdgpu: rework resume handling for display (v2)

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 02.12.24 um 20:18 schrieb Christian König:
> Am 02.12.24 um 17:52 schrieb Alex Deucher:
> > Split resume into a 3rd step to handle displays when DCC is
> > enabled on DCN 4.0.1.  Move display after the buffer funcs
> > have been re-enabled so that the GPU will do the move and
> > properly set the DCC metadata for DCN.

BTW: We might want to add a WARN_ON() to the CPU copy path to make sure 
that we don't try to copy DCC buffers while the SDMA is turned off.

Christian.

> > v2: fix fence irq resume ordering
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
>
> Reviewed-by: Christian König <christian.koenig@xxxxxxx>
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 45 +++++++++++++++++++++-
> >   1 file changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 2a25e0742f8e7..d882d46de1416 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -3764,7 +3764,7 @@ static int 
> > amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev)
> >    *
> >    * @adev: amdgpu_device pointer
> >    *
> > - * First resume function for hardware IPs.  The list of all the 
> > hardware
> > + * Second resume function for hardware IPs.  The list of all the 
> > hardware
> >    * IPs that make up the asic is walked and the resume callbacks are 
> > run for
> >    * all blocks except COMMON, GMC, and IH.  resume puts the hardware 
> > into a
> >    * functional state after a suspend and updates the software state as
> > @@ -3782,6 +3782,7 @@ static int 
> > amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
> >           if (adev->ip_blocks[i].version->type == 
> > AMD_IP_BLOCK_TYPE_COMMON ||
> >               adev->ip_blocks[i].version->type == 
> > AMD_IP_BLOCK_TYPE_GMC ||
> >               adev->ip_blocks[i].version->type == 
> > AMD_IP_BLOCK_TYPE_IH ||
> > +            adev->ip_blocks[i].version->type == 
> > AMD_IP_BLOCK_TYPE_DCE ||
> >               adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_PSP)
> >               continue;
> >           r = amdgpu_ip_block_resume(&adev->ip_blocks[i]);
> > @@ -3792,6 +3793,36 @@ static int 
> > amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
> >       return 0;
> >   }
> >   +/**
> > + * amdgpu_device_ip_resume_phase3 - run resume for hardware IPs
> > + *
> > + * @adev: amdgpu_device pointer
> > + *
> > + * Third resume function for hardware IPs.  The list of all the 
> > hardware
> > + * IPs that make up the asic is walked and the resume callbacks are 
> > run for
> > + * all DCE.  resume puts the hardware into a functional state after 
> > a suspend
> > + * and updates the software state as necessary.  This function is 
> > also used
> > + * for restoring the GPU after a GPU reset.
> > + *
> > + * Returns 0 on success, negative error code on failure.
> > + */
> > +static int amdgpu_device_ip_resume_phase3(struct amdgpu_device *adev)
> > +{
> > +    int i, r;
> > +
> > +    for (i = 0; i < adev->num_ip_blocks; i++) {
> > +        if (!adev->ip_blocks[i].status.valid || 
> > adev->ip_blocks[i].status.hw)
> > +            continue;
> > +        if (adev->ip_blocks[i].version->type == 
> > AMD_IP_BLOCK_TYPE_DCE) {
> > +            r = amdgpu_ip_block_resume(&adev->ip_blocks[i]);
> > +            if (r)
> > +                return r;
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >   /**
> >    * amdgpu_device_ip_resume - run resume for hardware IPs
> >    *
> > @@ -3821,6 +3852,13 @@ static int amdgpu_device_ip_resume(struct 
> > amdgpu_device *adev)
> >       if (adev->mman.buffer_funcs_ring->sched.ready)
> >           amdgpu_ttm_set_buffer_funcs_status(adev, true);
> >   +    if (r)
> > +        return r;
> > +
> > +    amdgpu_fence_driver_hw_init(adev);
> > +
> > +    r = amdgpu_device_ip_resume_phase3(adev);
> > +
> >       return r;
> >   }
> >   @@ -4909,7 +4947,6 @@ int amdgpu_device_resume(struct drm_device 
> > *dev, bool fbcon)
> >           dev_err(adev->dev, "amdgpu_device_ip_resume failed 
> > (%d).\n", r);
> >           goto exit;
> >       }
> > -    amdgpu_fence_driver_hw_init(adev);
> >         if (!adev->in_s0ix) {
> >           r = amdgpu_amdkfd_resume(adev, adev->in_runpm);
> > @@ -5489,6 +5526,10 @@ int amdgpu_device_reinit_after_reset(struct 
> > amdgpu_reset_context *reset_context)
> >                   if (tmp_adev->mman.buffer_funcs_ring->sched.ready)
> > amdgpu_ttm_set_buffer_funcs_status(tmp_adev, true);
> >   +                r = amdgpu_device_ip_resume_phase3(tmp_adev);
> > +                if (r)
> > +                    goto out;
> > +
> >                   if (vram_lost)
> >                       amdgpu_device_fill_reset_magic(tmp_adev);