[PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD handles (v3)
Nath, Arindam
Arindam.Nath at amd.com
Thu Dec 15 11:47:11 UTC 2016
>-----Original Message-----
>From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
>Sent: Thursday, December 15, 2016 5:01 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 15 December 2016 at 07:30, Nath, Arindam <Arindam.Nath at amd.com>
>wrote:
>>>-----Original Message-----
>>>From: Emil Velikov [mailto:emil.l.velikov at 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 at amd.com> wrote:
>>>> From: Arindam Nath <arindam.nath at 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 at amd.com>
>>>> Reviewed-by: Christian König <christian.koenig at 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?
>>
>Unless I'm reading the code incorrectly - CHIP_POLARIS10 and older do
>not have support the query. Thus ENODEV is the one you want to use.
>Furthermore EINVAL is (mostly) used to indicate incorrect input
>(failed input validation) which userspace uses to check if kernel is
>too old/does not support X.
Actually, the code says only asics older than CHIP_POLARIS10 support the query, beyond it doesn’t.
>
>If in doubt I recommend checking how other drivers are handling
>things. From memory at least i915 uses the above.
I will check how other drivers handle this.
Thanks,
Arindam
>
>Thanks
>Emil
More information about the amd-gfx
mailing list