On 2026. július 14., kedd 20:47:14 közép-európai nyári idő Tvrtko Ursulin
wrote:
> On 13/07/2026 13:58, Timur Kristóf wrote:
> > Call amdgpu_gfx_mqd_sw_init()/_fini() on GFX7 to initialize and
> > finalize the MQD, just like GFX8 and newer; instead of doing
> > an ad-hoc BO allocation. This introduces the possibility of
> > doing an MQD backup instead of trying to reinitialize the
> > MQD every time.
> >
> > This solves an issue with GFX IP block soft reset where
> > all compute rings would hang after the reset.
> >
> > Signed-off-by: Timur Kristóf <[email protected]>
> > ---
> >
> > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 106 +++++++++++++-------------
> > 1 file changed, 51 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c index 65b8497ad5f0..9c4b3ac27e1f
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -2698,25 +2698,6 @@ static int
> > gfx_v7_0_cp_compute_load_microcode(struct amdgpu_device *adev)>
> > return 0;
> >
> > }
> >
> > -/**
> > - * gfx_v7_0_cp_compute_fini - stop the compute queues
> > - *
> > - * @adev: amdgpu_device pointer
> > - *
> > - * Stop the compute queues and tear down the driver queue
> > - * info.
> > - */
> > -static void gfx_v7_0_cp_compute_fini(struct amdgpu_device *adev)
> > -{
> > - int i;
> > -
> > - for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> > - struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
> > -
> > - amdgpu_bo_free_kernel(&ring->mqd_obj, NULL, NULL);
> > - }
> > -}
> > -
> >
> > static void gfx_v7_0_mec_fini(struct amdgpu_device *adev)
> > {
> >
> > amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL);
> >
> > @@ -2788,28 +2769,29 @@ static void gfx_v7_0_compute_pipe_init(struct
> > amdgpu_device *adev,>
> > mutex_unlock(&adev->srbm_mutex);
> >
> > }
> >
> > -static int gfx_v7_0_mqd_deactivate(struct amdgpu_device *adev)
> > +static int gfx_v7_0_mqd_deactivate(struct amdgpu_device *adev, u32 req)
> >
> > {
> >
> > - int i;
> > + int i, r = 0;
> >
> > /* disable the queue if it's active */
> >
> > - if (RREG32(mmCP_HQD_ACTIVE) & 1) {
> > - WREG32(mmCP_HQD_DEQUEUE_REQUEST, 1);
> > + if (RREG32(mmCP_HQD_ACTIVE) & CP_HQD_ACTIVE__ACTIVE_MASK) {
> > + WREG32_FIELD(CP_HQD_DEQUEUE_REQUEST, DEQUEUE_REQ, req);
> >
> > for (i = 0; i < adev->usec_timeout; i++) {
> >
> > - if (!(RREG32(mmCP_HQD_ACTIVE) & 1))
> > + if (!(RREG32(mmCP_HQD_ACTIVE) &
CP_HQD_ACTIVE__ACTIVE_MASK))
> >
> > break;
> >
> > udelay(1);
> >
> > }
> >
> > if (i == adev->usec_timeout)
> >
> > - return -ETIMEDOUT;
> > + r = -ETIMEDOUT;
> >
> > - WREG32(mmCP_HQD_DEQUEUE_REQUEST, 0);
> > - WREG32(mmCP_HQD_PQ_RPTR, 0);
> > - WREG32(mmCP_HQD_PQ_WPTR, 0);
> >
> > }
> >
> > - return 0;
> > + WREG32(mmCP_HQD_DEQUEUE_REQUEST, 0);
> > + WREG32(mmCP_HQD_PQ_RPTR, 0);
> > + WREG32(mmCP_HQD_PQ_WPTR, 0);
> > +
> > + return r;
>
> I can see this matches gfx_v8_0_deactivate_hqd. If I am not missing
> anything only to replace the hardcoded 1 with CP_HQD_ACTIVE__ACTIVE_MASK?
There are two changes here:
- Replacing the hardcoded "1" with the define from the register definition
- When it times out, still write the CP_HQD_ registers afterwards like gfx8
> Is it okay to call the function mqd if the registers are hqd and is v7
> or v8 (which calls it hqd) more correct? Not saying either way, just
> observing a curiosity.
My best guess is that it's just that they used a different naming convention
and forgot to update the older code.
>
> > }
> >
> > static void gfx_v7_0_mqd_init(struct amdgpu_device *adev,
> >
> > @@ -2964,31 +2946,42 @@ static int gfx_v7_0_mqd_commit(struct
> > amdgpu_device *adev, struct cik_mqd *mqd)>
> > static int gfx_v7_0_compute_queue_init(struct amdgpu_device *adev, int
> > ring_id) {
> >
> > - int r;
> > - u64 mqd_gpu_addr;
> > - struct cik_mqd *mqd;
> >
> > struct amdgpu_ring *ring = &adev->gfx.compute_ring[ring_id];
> >
> > -
> > - r = amdgpu_bo_create_reserved(adev, sizeof(struct cik_mqd),
PAGE_SIZE,
> > - AMDGPU_GEM_DOMAIN_GTT,
&ring->mqd_obj,
> > - &mqd_gpu_addr, (void
**)&mqd);
> > - if (r) {
> > - dev_warn(adev->dev, "(%d) create MQD bo failed\n", r);
> > - return r;
> > + struct cik_mqd *mqd = ring->mqd_ptr;
> > + int mqd_idx = ring - &adev->gfx.compute_ring[0];
> > +
> > + if (!amdgpu_in_reset(adev) && !adev->in_suspend) {
> > + memset((void *)mqd, 0, ring->mqd_size);
> > + mutex_lock(&adev->srbm_mutex);
> > + cik_srbm_select(adev, ring->me, ring->pipe, ring-
>queue, 0);
> > + gfx_v7_0_mqd_init(adev, mqd, ring->mqd_gpu_addr, ring);
> > + gfx_v7_0_mqd_deactivate(adev, 1);
> > + gfx_v7_0_mqd_commit(adev, mqd);
> > + cik_srbm_select(adev, 0, 0, 0, 0);
> > + mutex_unlock(&adev->srbm_mutex);
> > +
> > + if (adev->gfx.mec.mqd_backup[mqd_idx])
> > + memcpy(adev->gfx.mec.mqd_backup[mqd_idx],
mqd, ring->mqd_size);
> > + } else {
> > + /* restore MQD to a clean status */
> > + if (adev->gfx.mec.mqd_backup[mqd_idx])
> > + memcpy(mqd, adev-
>gfx.mec.mqd_backup[mqd_idx], ring->mqd_size);
> > +
> > + /* Re-commit the restored backup */
> > + mutex_lock(&adev->srbm_mutex);
> > + cik_srbm_select(adev, ring->me, ring->pipe, ring-
>queue, 0);
> > + gfx_v7_0_mqd_deactivate(adev, 2);
> > + gfx_v7_0_mqd_commit(adev, mqd);
> > + cik_srbm_select(adev, 0, 0, 0, 0);
> > + mutex_unlock(&adev->srbm_mutex);
> > +
> > + /* reset ring buffer */
> > + ring->wptr = 0;
> > + atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
> > + atomic64_set((atomic64_t *)ring->rptr_cpu_addr, 0);
> > + amdgpu_ring_clear_ring(ring);
> >
> > }
> >
> > - mutex_lock(&adev->srbm_mutex);
> > - cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
> > -
> > - gfx_v7_0_mqd_init(adev, mqd, mqd_gpu_addr, ring);
> > - gfx_v7_0_mqd_deactivate(adev);
> > - gfx_v7_0_mqd_commit(adev, mqd);
> > -
> > - cik_srbm_select(adev, 0, 0, 0, 0);
> > - mutex_unlock(&adev->srbm_mutex);
> > -
> > - amdgpu_bo_kunmap(ring->mqd_obj);
> > - amdgpu_bo_unreserve(ring->mqd_obj);
> >
> > return 0;
> >
> > }
>
> I think I can follow this - only the wptr and rptr reset is a bit
> different than what v8 does it. Any specific reason? Gfx9 then reverts
> back to a single ring->wptr = 0. I guess v8 is somehow special?
>
> > @@ -3020,10 +3013,8 @@ static int gfx_v7_0_cp_compute_resume(struct
> > amdgpu_device *adev)>
> > /* init the queues */
> > for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> >
> > r = gfx_v7_0_compute_queue_init(adev, i);
> >
> > - if (r) {
> > - gfx_v7_0_cp_compute_fini(adev);
> > + if (r)
> >
> > return r;
> >
> > - }
> >
> > }
> >
> > gfx_v7_0_cp_compute_enable(adev, true);
> >
> > @@ -4430,6 +4421,11 @@ static int gfx_v7_0_sw_init(struct amdgpu_ip_block
> > *ip_block)>
> > }
> >
> > }
> >
> > + /* create MQD for all compute queues */
> > + r = amdgpu_gfx_mqd_sw_init(adev, sizeof(struct cik_mqd), 0);
> > + if (r)
> > + return r;
> > +
> >
> > adev->gfx.ce_ram_size = 0x8000;
> >
> > gfx_v7_0_gpu_early_init(adev);
> >
> > @@ -4452,7 +4448,7 @@ static int gfx_v7_0_sw_fini(struct amdgpu_ip_block
> > *ip_block)>
> > for (i = 0; i < adev->gfx.num_compute_rings; i++)
> >
> > amdgpu_ring_fini(&adev->gfx.compute_ring[i]);
> >
> > - gfx_v7_0_cp_compute_fini(adev);
> > + amdgpu_gfx_mqd_sw_fini(adev, 0);
> >
> > amdgpu_gfx_rlc_fini(adev);
> > gfx_v7_0_mec_fini(adev);
> > amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
>
> I am assuming all this applies only to compute because gfx is single
> instance on v7?
It applies only to compute because only compute has HQD/MQD
on these hardware generations.
> Anyway, it looks plausible to me so assuming you were able to exercise
> both paths
What do you mean by "both paths"?
Thanks,
Timur
