Re: [PATCH 09/16] drm/amdgpu/si, cik, vi: Verify IP block when querying video codecs (v2)

[Alex Deucher <alexdeucher@gmail.com>]

On Tue, Nov 4, 2025 at 10:16 AM Timur Kristóf <timur.kristof@gmail.com> wrote:
>
> On Tue, 2025-11-04 at 14:44 +0100, Christian König wrote:
> > On 11/3/25 23:23, Timur Kristóf wrote:
> > > Some harvested chips may not have any IP blocks,
> > > or we may not have the firmware for the IP blocks.
> > > In these cases, the query should return that no video
> > > codec is supported.
> > >
> > > v2:
> > > - When codecs is NULL, treat that as empty codec list.
> >
> > I'm still not sure if returning an error instead wouldn't be better.
> >
> > @Alex and Leo what do you guys think?
> >
> > Regards,
> > Christian.
>
> Returning an error from this function would indicate to userspace that
> there was an error with querying the list of codecs.
>
> That is not what we want. We simply want to tell userspace that no
> codecs are supported.

If the IP is harvested or if it's been disabled, we wouldn't be
exposing any rings via drm_amdgpu_info_hw_ip so I don't think we need
this.

Alex

>
> Thanks & best regards,
> Timur
>
>
> >
> > >
> > > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +++++--
> > >  drivers/gpu/drm/amd/amdgpu/cik.c        | 6 ++++++
> > >  drivers/gpu/drm/amd/amdgpu/si.c         | 6 ++++++
> > >  drivers/gpu/drm/amd/amdgpu/vi.c         | 6 ++++++
> > >  4 files changed, 23 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > index b3e6b3fcdf2c..71eceac58fb6 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > @@ -1263,8 +1263,9 @@ int amdgpu_info_ioctl(struct drm_device *dev,
> > > void *data, struct drm_file *filp)
> > >                     -EFAULT : 0;
> > >     }
> > >     case AMDGPU_INFO_VIDEO_CAPS: {
> > > -           const struct amdgpu_video_codecs *codecs;
> > > +           const struct amdgpu_video_codecs *codecs = NULL;
> > >             struct drm_amdgpu_info_video_caps *caps;
> > > +           u32 codec_count;
> > >             int r;
> > >
> > >             if (!adev->asic_funcs->query_video_codecs)
> > > @@ -1291,7 +1292,9 @@ int amdgpu_info_ioctl(struct drm_device *dev,
> > > void *data, struct drm_file *filp)
> > >             if (!caps)
> > >                     return -ENOMEM;
> > >
> > > -           for (i = 0; i < codecs->codec_count; i++) {
> > > +           codec_count = codecs ? codecs->codec_count : 0;
> > > +
> > > +           for (i = 0; i < codec_count; i++) {
> > >                     int idx = codecs-
> > > >codec_array[i].codec_type;
> > >
> > >                     switch (idx) {
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c
> > > b/drivers/gpu/drm/amd/amdgpu/cik.c
> > > index 9cd63b4177bf..b755238c2c3d 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/cik.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/cik.c
> > > @@ -130,6 +130,12 @@ static const struct amdgpu_video_codecs
> > > cik_video_codecs_decode =
> > >  static int cik_query_video_codecs(struct amdgpu_device *adev, bool
> > > encode,
> > >                               const struct amdgpu_video_codecs
> > > **codecs)
> > >  {
> > > +   const enum amd_ip_block_type ip =
> > > +           encode ? AMD_IP_BLOCK_TYPE_VCE :
> > > AMD_IP_BLOCK_TYPE_UVD;
> > > +
> > > +   if (!amdgpu_device_ip_is_valid(adev, ip))
> > > +           return 0;
> > > +
> > >     switch (adev->asic_type) {
> > >     case CHIP_BONAIRE:
> > >     case CHIP_HAWAII:
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/si.c
> > > b/drivers/gpu/drm/amd/amdgpu/si.c
> > > index e0f139de7991..9468c03bdb1b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/si.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/si.c
> > > @@ -1003,6 +1003,12 @@ static const struct amdgpu_video_codecs
> > > hainan_video_codecs_decode =
> > >  static int si_query_video_codecs(struct amdgpu_device *adev, bool
> > > encode,
> > >                              const struct amdgpu_video_codecs
> > > **codecs)
> > >  {
> > > +   const enum amd_ip_block_type ip =
> > > +           encode ? AMD_IP_BLOCK_TYPE_VCE :
> > > AMD_IP_BLOCK_TYPE_UVD;
> > > +
> > > +   if (!amdgpu_device_ip_is_valid(adev, ip))
> > > +           return 0;
> > > +
> > >     switch (adev->asic_type) {
> > >     case CHIP_VERDE:
> > >     case CHIP_TAHITI:
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
> > > b/drivers/gpu/drm/amd/amdgpu/vi.c
> > > index a611a7345125..f0e4193cf722 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> > > @@ -256,6 +256,12 @@ static const struct amdgpu_video_codecs
> > > cz_video_codecs_decode =
> > >  static int vi_query_video_codecs(struct amdgpu_device *adev, bool
> > > encode,
> > >                              const struct amdgpu_video_codecs
> > > **codecs)
> > >  {
> > > +   const enum amd_ip_block_type ip =
> > > +           encode ? AMD_IP_BLOCK_TYPE_VCE :
> > > AMD_IP_BLOCK_TYPE_UVD;
> > > +
> > > +   if (!amdgpu_device_ip_is_valid(adev, ip))
> > > +           return 0;
> > > +
> > >     switch (adev->asic_type) {
> > >     case CHIP_TOPAZ:
> > >             if (encode)