Re: [PATCH v9 14/14] drm/amdgpu: add kernel config for gfx-userqueue

["Sharma, Shashank" <shashank.sharma@xxxxxxx>]

On 02/05/2024 17:22, Christian König wrote:
> Am 26.04.24 um 15:48 schrieb Shashank Sharma:
> > This patch:
> > - adds a kernel config option "CONFIG_DRM_AMD_USERQ_GFX"
> > - moves the usequeue initialization code for all IPs under
> >    this flag
> >
> > so that the userqueue works only when the config is enabled.
> >
> > Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > Cc: Christian Koenig <christian.koenig@xxxxxxx>
> > Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/Kconfig     | 8 ++++++++
> >   drivers/gpu/drm/amd/amdgpu/Makefile    | 8 ++++++--
> >   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 4 ++++
> >   drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 3 +++
> >   4 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
> > b/drivers/gpu/drm/amd/amdgpu/Kconfig
> > index 22d88f8ef527..bba963527d22 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> > @@ -80,6 +80,14 @@ config DRM_AMDGPU_WERROR
> >         Add -Werror to the build flags for amdgpu.ko.
> >         Only enable this if you are warning code for amdgpu.ko.
> >   +config DRM_AMDGPU_USERQ_GFX
> > +    bool "Enable Navi 3x gfx usermode queues"
> > +    depends on DRM_AMDGPU
> > +    default n
> > +    help
> > +      Choose this option to enable usermode queue support for GFX
> > +          workload submission. This feature is supported on Navi 3X 
> > only.
>
> When this is for Navi 3x only I would name that 
> DRM_AMDGPU_NAVI3X_USERQ instead.
>
> And since we enable/disable GFX, Compute and SDMA I would drop "gfx" 
> from the comment and description.
>
> Apart from that the approach looks good to me.
Agree, both the review comments addressed in V10.

- Shashank

> Christian.
>
> > +
> >   source "drivers/gpu/drm/amd/acp/Kconfig"
> >   source "drivers/gpu/drm/amd/display/Kconfig"
> >   source "drivers/gpu/drm/amd/amdkfd/Kconfig"
> > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
> > b/drivers/gpu/drm/amd/amdgpu/Makefile
> > index a640bfa468ad..0b17fc1740a0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> > @@ -184,8 +184,12 @@ amdgpu-y += \
> >   amdgpu-y += \
> >       amdgpu_mes.o \
> >       mes_v10_1.o \
> > -    mes_v11_0.o \
> > -    mes_v11_0_userqueue.o
> > +    mes_v11_0.o
> > +
> > +# add GFX userqueue support
> > +ifneq ($(CONFIG_DRM_AMD_USERQ_GFX),)
> > +amdgpu-y += mes_v11_0_userqueue.o
> > +endif
> >     # add UVD block
> >   amdgpu-y += \
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > index 27b86f7fe949..8591aed9f9ab 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > @@ -1349,8 +1349,10 @@ static int gfx_v11_0_sw_init(void *handle)
> >           adev->gfx.mec.num_mec = 2;
> >           adev->gfx.mec.num_pipe_per_mec = 4;
> >           adev->gfx.mec.num_queue_per_pipe = 4;
> > +#ifdef CONFIG_DRM_AMD_USERQ_GFX
> >           adev->userq_funcs[AMDGPU_HW_IP_GFX] = &userq_mes_v11_0_funcs;
> >           adev->userq_funcs[AMDGPU_HW_IP_COMPUTE] = 
> > &userq_mes_v11_0_funcs;
> > +#endif
> >           break;
> >       case IP_VERSION(11, 0, 1):
> >       case IP_VERSION(11, 0, 4):
> > @@ -1362,8 +1364,10 @@ static int gfx_v11_0_sw_init(void *handle)
> >           adev->gfx.mec.num_mec = 1;
> >           adev->gfx.mec.num_pipe_per_mec = 4;
> >           adev->gfx.mec.num_queue_per_pipe = 4;
> > +#ifdef CONFIG_DRM_AMD_USERQ_GFX
> >           adev->userq_funcs[AMDGPU_HW_IP_GFX] = &userq_mes_v11_0_funcs;
> >           adev->userq_funcs[AMDGPU_HW_IP_COMPUTE] = 
> > &userq_mes_v11_0_funcs;
> > +#endif
> >           break;
> >       default:
> >           adev->gfx.me.num_me = 1;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> > index 90354a70c807..084059c95db6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> > @@ -1267,7 +1267,10 @@ static int sdma_v6_0_sw_init(void *handle)
> >           return -EINVAL;
> >       }
> >   +#ifdef CONFIG_DRM_AMD_USERQ_GFX
> >       adev->userq_funcs[AMDGPU_HW_IP_DMA] = &userq_mes_v11_0_funcs;
> > +#endif
> > +
> >       return r;
> >   }