On Wed, 2025-10-29 at 11:35 +0100, Christian König wrote:
>
>
> On 10/28/25 23:06, 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.
> >
> > Signed-off-by: Timur Kristóf <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++-
> > 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, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index b3e6b3fcdf2c..42b5da59d00f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -1263,7 +1263,8 @@ 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;
> > + static const struct amdgpu_video_codecs no_codecs
> > = {0};
>
> No zero init for static variables please, that will raise you a
> constant checker warning.
>
> > + const struct amdgpu_video_codecs *codecs =
> > &no_codecs;
> > struct drm_amdgpu_info_video_caps *caps;
> > int r;
> >
> > 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;
>
> I'm wondering if returning EOPNOTSUPP is not more appropriate here
> than returning an empty cappability list.
I don't think so.
Returning EOPNOTSUPP would indicate that the operation of querying the
codec support is not supported, and not that the list of supported
codecs is empty.
>
> Anyway setting the codecs list to empty in the caller is rather bad
> coding style.
Sure, I'll come up with a better way to do this.
>
> Regards,
> Christian.
>
> > +
> > 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)
