[PATCH v2 1/2] drm: support feature masks in drm_core_check_feature()

[Thomas Zimmermann]

Hi

Am 22.01.20 um 15:27 schrieb Jani Nikula:
> On Wed, 22 Jan 2020, Thomas Zimmermann <tzimmermann at suse.de> wrote:
>> Hi Jani
>>
>> Am 22.01.20 um 15:02 schrieb Jani Nikula:
>>> Allow a mask of features to be passed to drm_core_check_feature(). All
>>> features in the mask are required.
>>>
>>> v2:
>>> - fix kernel-doc (Ville)
>>> - add an extra variable for clarity (Ville)
>>>
>>> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>>> ---
>>>  include/drm/drm_drv.h | 12 ++++++++----
>>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>>> index cf13470810a5..f18e19f3f2d0 100644
>>> --- a/include/drm/drm_drv.h
>>> +++ b/include/drm/drm_drv.h
>>> @@ -826,16 +826,20 @@ static inline bool drm_dev_is_unplugged(struct drm_device *dev)
>>>  /**
>>>   * drm_core_check_feature - check driver feature flags
>>>   * @dev: DRM device to check
>>> - * @feature: feature flag
>>> + * @features: feature flag(s)
>>>   *
>>>   * This checks @dev for driver features, see &drm_driver.driver_features,
>>>   * &drm_device.driver_features, and the various &enum drm_driver_feature flags.
>>>   *
>>> - * Returns true if the @feature is supported, false otherwise.
>>> + * Returns true if all features in the @features mask are supported, false
>>> + * otherwise.
>>>   */
>>> -static inline bool drm_core_check_feature(const struct drm_device *dev, u32 feature)
>>> +static inline bool drm_core_check_feature(const struct drm_device *dev,
>>> +					  u32 features)
>>
>> It's misnamed now. I'd add a new function, say
>> drm_core_check_all_features(), which makes the purpose clear.
> 
> We don't really need another function. We need this one to check all the
> features. But I'd rather not do the mass rename of all call sites for no
> real benefit.

My point is: does the function check for all of the given features or
any combination of them? The function's name doesn't tell you.

Best regards
Thomas

> 
> BR,
> Jani.
> 
> 
>>
>> Best regards
>> Thomas
>>
>>>  {
>>> -	return dev->driver->driver_features & dev->driver_features & feature;
>>> +	u32 supported = dev->driver->driver_features & dev->driver_features;
>>> +
>>> +	return features && (supported & features) == features;
>>>  }
>>>  
>>>  /**
>>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200122/c4f44f90/attachment-0001.sig>