RE: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD handles (v3)

-----Original Message----- From: Emil Velikov [mailto:emil.l.velikov@gmail.com] Sent: Wednesday, December 14, 2016 9:26 PM To: Nath, Arindam Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel; Koenig, Christian Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD handles (v3)

On 12 December 2016 at 18:49, arindam.nath@amd.com wrote:

...

From: Arindam Nath arindam.nath@amd.com

Change History

v3: changes suggested by Christian

• Add a check for UVD IP block using AMDGPU_HW_IP_UVD query type.
• Add a check for asic_type to be less than CHIP_POLARIS10 since starting Polaris, we support unlimited UVD instances.
• Add kerneldoc style comment for amdgpu_uvd_used_handles().

v2: as suggested by Christian

• Add a new query AMDGPU_INFO_NUM_HANDLES
• Create a helper function to return the number of currently used UVD handles.
• Modify the logic to count the number of used UVD handles since handles can be freed in non-linear fashion.

v1:

• User might want to query the maximum number of UVD instances supported by firmware. In addition to that, if there are multiple applications using UVD handles at the same time, he might also want to query the currently used number of handles.

  For this we add two variables max_handles and used_handles inside drm_amdgpu_info_hw_ip. So now an application (or libdrm) can use AMDGPU_INFO IOCTL with AMDGPU_INFO_HW_IP_INFO query type to get these values.

Signed-off-by: Arindam Nath arindam.nath@amd.com Reviewed-by: Christian König christian.koenig@amd.com

---

drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21

+++++++++++++++++++++

...

drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25

+++++++++++++++++++++++++

...

drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h | 1 + include/uapi/drm/amdgpu_drm.h | 9 +++++++++ 4 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

...

index 174eb59..3273d8c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device

*dev, void *data, struct drm_file

...

                    return -EINVAL;
            }
    }

•   case AMDGPU_INFO_NUM_HANDLES: {
  

•           struct drm_amdgpu_info_num_handles handle;
  

•           switch (info->query_hw_ip.type) {
  

•           case AMDGPU_HW_IP_UVD:
  

•                   /* Starting Polaris, we support unlimited UVD handles */
  

•                   if (adev->asic_type < CHIP_POLARIS10) {
  

•                           handle.uvd_max_handles = adev->uvd.max_handles;
  

•                           handle.uvd_used_handles =
  

amdgpu_uvd_used_handles(adev);

...

•                           return copy_to_user(out, &handle,
  

•                                   min((size_t)size, sizeof(handle))) ? -EFAULT : 0;
  

•                   } else {
  

•                           return -EINVAL;
  

Using EINVAL doesn't seem right here. As per man 3 ioctl

 EINVAL The request or arg argument is not valid for this device.

A bit further down you can see the one you want.

 ENODEV The fildes argument refers to a valid STREAMS device, but

the corresponding device driver does not support the ioctl() function.

Emil, ENODEV would mean the driver does not support ioctl() itself. But in our case ioctl() is supported.

Since we extract the query type from arg passed to ioctl(), and it is this query AMDGPU_INFO_NUM_HANDLES which is not supported by driver (for > Polaris), doesn’t returning EINVAL make more sense here?

Thanks, Arindam

Worth checking through the existing code and correcting similar thinkos ?

Thanks Emil