[Public]
Regards,
Prike
> -----Original Message-----
> From: Alex Deucher <[email protected]>
> Sent: Wednesday, July 16, 2025 11:20 PM
> To: Liang, Prike <[email protected]>
> Cc: Lazar, Lijo <[email protected]>; Deucher, Alexander
> <[email protected]>; [email protected]; Koenig, Christian
> <[email protected]>
> Subject: Re: [PATCH 2/3] drm/amdgpu/gfx11: set MQD as appriopriate for queue
> priv
>
> On Wed, Jul 16, 2025 at 5:58 AM Liang, Prike <[email protected]> wrote:
> >
> > [Public]
> >
> > Regards,
> > Prike
> >
> > > -----Original Message-----
> > > From: amd-gfx <[email protected]> On Behalf Of
> > > Lazar, Lijo
> > > Sent: Wednesday, July 16, 2025 12:18 PM
> > > To: Deucher, Alexander <[email protected]>; amd-
> > > [email protected]
> > > Cc: Koenig, Christian <[email protected]>
> > > Subject: Re: [PATCH 2/3] drm/amdgpu/gfx11: set MQD as appriopriate
> > > for queue priv
> > >
> > >
> > >
> > > On 7/12/2025 3:21 AM, Alex Deucher wrote:
> > > > Set the MQD as appropriate for the queue priv state.
> > > >
> > > > Acked-by: Christian König <[email protected]>
> > > > Signed-off-by: Alex Deucher <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 8 ++++++--
> > > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > > b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > > index 37dcec2d07841..b9ba8b22a1073 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > > @@ -4124,6 +4124,8 @@ static int gfx_v11_0_gfx_mqd_init(struct
> > > > amdgpu_device *adev, void *m, #endif
> > > > if (prop->tmz_queue)
> > > > tmp = REG_SET_FIELD(tmp, CP_GFX_HQD_CNTL, TMZ_MATCH,
> > > 1);
> > > > + if (!prop->priv_queue)
> > > > + tmp = REG_SET_FIELD(tmp, CP_GFX_HQD_CNTL,
> > > RB_NON_PRIV, 1);
> > > > mqd->cp_gfx_hqd_cntl = tmp;
> > > >
> > > > /* set up cp_doorbell_control */ @@ -4276,8 +4278,10 @@ static
> > > > int gfx_v11_0_compute_mqd_init(struct
> > > amdgpu_device *adev, void *m,
> > > > tmp = REG_SET_FIELD(tmp, CP_HQD_PQ_CONTROL,
> > > UNORD_DISPATCH, 1);
> > > > tmp = REG_SET_FIELD(tmp, CP_HQD_PQ_CONTROL,
> > > TUNNEL_DISPATCH,
> > > > prop->allow_tunneling);
> > > > - tmp = REG_SET_FIELD(tmp, CP_HQD_PQ_CONTROL, PRIV_STATE, 1);
> > > > - tmp = REG_SET_FIELD(tmp, CP_HQD_PQ_CONTROL, KMD_QUEUE,
> 1);
> > > > + if (prop->priv_queue) {
> > > > + tmp = REG_SET_FIELD(tmp, CP_HQD_PQ_CONTROL,
> > > PRIV_STATE, 1);
> > > > + tmp = REG_SET_FIELD(tmp, CP_HQD_PQ_CONTROL,
> > > KMD_QUEUE, 1);
> > > > + }
> > >
> > > As per above logic, only kernel mode queues are supposed to be
> > > privileged. If so, would suggest renaming the flag to kernel_q
> > According to the CP guys, the privileged bit can be applied both the user
> > queue
> and kernel queue. So, we may don't bound the privileged queue to the kernel
> queue.
> > Meanwhile, the KMD_QUEUE bit may only set for the kernel queue only enabled
> case.
>
> I think we want PRIV_STATE for only kernel queues. Why would you want it for
> user queues?
If the user queue only limits to be none privileged, then that will limit the
user queue some kind of scheduling capability. Also, it seems this register bit
may not be allowed to touch here if we want to avoid the wrong privileges
setting on the user queue. I will check those questions with HW team later.
> Alex
>
> >
> > > Thanks,
> > > Lijo
> > >
> > > > if (prop->tmz_queue)
> > > > tmp = REG_SET_FIELD(tmp, CP_HQD_PQ_CONTROL, TMZ, 1);
> > > > mqd->cp_hqd_pq_control = tmp;
> >
