[Intel-gfx] [PATCH 1/2] drm: Introduce per-device driver_features

[Daniel Vetter]

On Thu, Sep 13, 2018 at 04:16:21PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> We wish to control certain driver_features flags on a per-device basis
> while still sharing a single drm_driver instance across all the
> devices. To that end introduce device.driver_features. By default
> it will be set to ~0 to not impose any limits beyond
> driver.driver_features. Drivers can then clear specific flags
> in the per-device bitmask to limit the capabilities of the device.
> 
> An alternative approach would be to copy the driver_features from
> the driver into the device in drm_dev_init(), however that would
> require verifying that no driver is currently changing
> driver.driver_features after drm_dev_init(). Hence the ~0 apporach
> was easier.
> 
> Ideally we'd also make drm_driver const but there is plenty of code
> left that wants to mutate it (eg. various vfunc assignments). We'll
> need to fix all that up before we can make it const.
> 
> And while at it fix up the type of the feature flag passed to
> drm_core_check_feature().
> 
> v2: Streamline the && vs. & (Chris)
>     s/int/u32/ in drm_core_check_feature() args
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

git grep DRIVER_ATOMIC -- drivers/gpu/drm/nouveau has a 2nd supporting
case for this. Exactly same problem as we have here. Would be good to also
convert that one, for a bit of OCD.

Irrespective of that, on the series:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Feel free to also add the r-b to the nouveau patch if you do it, it's
rather obvious.
-Daniel

> ---
>  drivers/gpu/drm/drm_drv.c |  3 +++
>  include/drm/drm_device.h  | 10 ++++++++++
>  include/drm/drm_drv.h     |  8 ++++----
>  3 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index ea4941da9b27..36e8e9cbec52 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -506,6 +506,9 @@ int drm_dev_init(struct drm_device *dev,
>  	dev->dev = parent;
>  	dev->driver = driver;
>  
> +	/* no per-device feature limits by default */
> +	dev->driver_features = ~0u;
> +
>  	INIT_LIST_HEAD(&dev->filelist);
>  	INIT_LIST_HEAD(&dev->filelist_internal);
>  	INIT_LIST_HEAD(&dev->clientlist);
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index f9c6e0e3aec7..42411b3ea0c8 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -45,6 +45,16 @@ struct drm_device {
>  	/* currently active master for this device. Protected by master_mutex */
>  	struct drm_master *master;
>  
> +	/**
> +	 * @driver_features: per-device driver features
> +	 *
> +	 * Drivers can clear specific flags here to disallow
> +	 * certain features on a per-device basis while still
> +	 * sharing a single &struct drm_driver instance across
> +	 * all devices.
> +	 */
> +	u32 driver_features;
> +
>  	/**
>  	 * @unplugged:
>  	 *
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 23b9678137a6..8830e3de3a86 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -653,14 +653,14 @@ static inline bool drm_dev_is_unplugged(struct drm_device *dev)
>   * @dev: DRM device to check
>   * @feature: feature flag
>   *
> - * This checks @dev for driver features, see &drm_driver.driver_features and the
> - * various DRIVER_\* flags.
> + * This checks @dev for driver features, see &drm_driver.driver_features,
> + * &drm_device.driver_features, and the various DRIVER_\* flags.
>   *
>   * Returns true if the @feature is supported, false otherwise.
>   */
> -static inline bool drm_core_check_feature(struct drm_device *dev, int feature)
> +static inline bool drm_core_check_feature(struct drm_device *dev, u32 feature)
>  {
> -	return dev->driver->driver_features & feature;
> +	return dev->driver->driver_features & dev->driver_features & feature;
>  }
>  
>  /**
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch