Re: [igt-dev] [PATCH i-g-t v10 2/2] lib/igt_ioctl_wrappers: Add igt_has_drm_cap ioctl wrapper

[Karthik B S <karthik.b.s@intel.com>]

On 9/24/2020 10:39 PM, Ville Syrjälä wrote:
> On Wed, Sep 23, 2020 at 07:32:58PM +0530, Karthik B S wrote:
>> Add a generic helper for DRM_IOCTL_GET_CAP ioctl.
>>
>> v10: -No changes.
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   lib/ioctl_wrappers.c | 15 +++++++++++++++
>>   lib/ioctl_wrappers.h |  1 +
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>> index 3781286d..3ea60c61 100644
>> --- a/lib/ioctl_wrappers.c
>> +++ b/lib/ioctl_wrappers.c
>> @@ -1318,3 +1318,18 @@ int __kms_addfb(int fd, uint32_t handle,
>>   
>>   	return ret < 0 ? -errno : ret;
>>   }
>> +
>> +/**
>> + * igt_has_drm_cap:
>> + * @fd: Open DRM file descriptor.
>> + * @cap: drm_get_cap structure.
>> + *
>> + * This helper verifies if the passed cap is supported by the kernel
>> + *
>> + * Returns: Whether the cap is supported or not.
>> + */
>> +bool igt_has_drm_cap(int fd, struct drm_get_cap cap)
> 
> I don't see why the caller would want to pass in the whole struct.
> Also passing by value is a but nuts. Ie. I'd just the actual cap
> ID and keep the ioctl struct details hidden inside this function.
> 

Thanks for the review.
I will make this change.

> The order of the patches looks wrong. 1/2 won't build w/o this AFAICS.
> 'git rebase -x ...' before sending the patches is always a good idea.
> 

Sure. I'll fix this. And use 'git rebase -x' from the next time.

Thanks,
Karthik.B.S
>> +{
>> +	igt_assert(drmIoctl(fd, DRM_IOCTL_GET_CAP, &cap) == 0);
>> +	return cap.value;
>> +}
>> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
>> index 870ac8b7..4cf4004d 100644
>> --- a/lib/ioctl_wrappers.h
>> +++ b/lib/ioctl_wrappers.h
>> @@ -196,6 +196,7 @@ struct local_drm_mode_fb_cmd2 {
>>   
>>   bool igt_has_fb_modifiers(int fd);
>>   void igt_require_fb_modifiers(int fd);
>> +bool igt_has_drm_cap(int fd, struct drm_get_cap cap);
>>   
>>   /**
>>    * __kms_addfb:
>> -- 
>> 2.22.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev