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. 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)
