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

["Timur Kristóf" <timur.kristof@gmail.com>]

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.

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)