On Wed, Jul 15, 2026 at 11:54 AM Timur Kristóf <[email protected]> wrote:
>
> 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.
Technically, the MQD (Memory Queue Descriptor) and HQD (Hardware Queue
Descriptor) are separate things. The MQD is a structure stored in
memory which stores the state of a queue. The HQD is the hardware
state which stores the queue when it's mapped to hardware. The way
it's supposed to work is that the driver sets up MQDs in memory and
then asks KIQ to map those queues to an HQD or put them on a runlist
(for user queues) if there are more MQDs than HQDs. For gfx8 and
newer the driver sets up the MQD and then uses the KIQ to map the
queues. gfx7 could work that way as well, but we never got around to
porting the changes back to gfx7. Instead the driver maps the queues
to the HQD directly via MMIO rather than having KIQ do it on gfx7.
Alex
>
> >
> > > }
> > >
> > > 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
>
>
