On Tue, Nov 4, 2025 at 10:29 AM Christian König <[email protected]> wrote: > > On 11/4/25 16:27, Alex Deucher wrote: > > On Tue, Nov 4, 2025 at 10:16 AM Timur Kristóf <[email protected]> > > 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. > > Oh good point as well. But should we then return an error? > > I think yes, because the HW isn't available at all and so querying the > available codecs doesn't make sense in the first place.
Either way works for me. Alex > > Regards, > Christian. > > > > > Alex > > > >> > >> Thanks & best regards, > >> Timur > >> > >> > >>> > >>>> > >>>> Signed-off-by: Timur Kristóf <[email protected]> > >>>> --- > >>>> 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) >
