>-----Original Message----- >From: Emil Velikov [mailto:emil.l.velikov@xxxxxxxxx] >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@xxxxxxx> >wrote: >>>-----Original Message----- >>>From: Emil Velikov [mailto:emil.l.velikov@xxxxxxxxx] >>>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@xxxxxxx> wrote: >>>> From: Arindam Nath <arindam.nath@xxxxxxx> >>>> >>>> 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@xxxxxxx> >>>> Reviewed-by: Christian König <christian.koenig@xxxxxxx> >>>> --- >>>> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
