On Wed, Jul 15, 2026 at 5:30 PM Timur Kristóf <[email protected]> wrote:
>
> On 2026. július 15., szerda 23:10:02 közép-európai nyári idő Alex Deucher
> wrote:
> > On Wed, Jul 15, 2026 at 4:32 PM Timur Kristóf <[email protected]>
> wrote:
> > > On 2026. július 15., szerda 21:50:53 közép-európai nyári idő Alex Deucher
> > >
> > > wrote:
> > > > 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
> > >
> > > As far as I understood, Tvrtko's point was why the function is called
> > > gfx_v7_0_mqd_deactivate() when what it does is the same as
> > > gfx_v8_0_deactivate_hqd(), ie. why does one have MQD in the name while the
> > > other has HQD in the name.
> >
> > I don't remember why it's named like that, but deactivate_hqd() is
> > better aligned with what it actually does. This is effectively the
> > MMIO equivalent of submitting an unmap queue packet to the KIQ.
> >
> > Alex
>
> Would it be OK if I renamed it?
> gfx_v7_0_mqd_deactivate() --> gfx_v7_0_deactivate_hqd()
> to better align it with gfx8 and ease the confusion.
yeah, that would be better.
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
>
>
>
>
