| Message ID | 20200122140259.12086-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] drm: support feature masks in drm_core_check_feature() | expand |
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@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@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. 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; > } > > /** >
On Wed, 22 Jan 2020, Thomas Zimmermann <tzimmermann@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@linux.intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@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. 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; >> } >> >> /** >>
Hi Am 22.01.20 um 15:27 schrieb Jani Nikula: > On Wed, 22 Jan 2020, Thomas Zimmermann <tzimmermann@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@linux.intel.com> >>> Signed-off-by: Jani Nikula <jani.nikula@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; >>> } >>> >>> /** >>> >
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) { - return dev->driver->driver_features & dev->driver_features & feature; + u32 supported = dev->driver->driver_features & dev->driver_features; + + return features && (supported & features) == features; } /**