public inbox for dri-devel@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm: Introduce per-device driver_features
@ 2018-09-13 13:16 Ville Syrjala
  2018-09-13 13:16 ` [PATCH 2/2] drm/i915: Clear DRIVER_ATOMIC on a per-device basis Ville Syrjala
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ville Syrjala @ 2018-09-13 13:16 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

From: Ville Syrjälä <ville.syrjala@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@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] drm/i915: Clear DRIVER_ATOMIC on a per-device basis
  2018-09-13 13:16 [PATCH 1/2] drm: Introduce per-device driver_features Ville Syrjala
@ 2018-09-13 13:16 ` Ville Syrjala
  2018-09-13 13:28   ` Chris Wilson
  2018-09-13 13:27 ` [PATCH 1/2] drm: Introduce per-device driver_features Chris Wilson
  2018-09-13 13:50 ` [Intel-gfx] " Daniel Vetter
  2 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjala @ 2018-09-13 13:16 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we're clearing DRIVER_ATOMIC in driver.driver_features
for older platforms. This will not work correctly should we ever
have a system with and old and new GPU in it. While that is not
possible currently let's make the code more correct and use
the per-device driver_features instead.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5dd7fc582e6f..2ddf8538cb47 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1384,14 +1384,14 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct drm_i915_private *dev_priv;
 	int ret;
 
-	/* Enable nuclear pageflip on ILK+ */
-	if (!i915_modparams.nuclear_pageflip && match_info->gen < 5)
-		driver.driver_features &= ~DRIVER_ATOMIC;
-
 	dev_priv = i915_driver_create(pdev, ent);
 	if (!dev_priv)
 		return -ENOMEM;
 
+	/* Disable nuclear pageflip by default on pre-ILK */
+	if (!i915_modparams.nuclear_pageflip && match_info->gen < 5)
+		dev_priv->drm.driver_features &= ~DRIVER_ATOMIC;
+
 	ret = pci_enable_device(pdev);
 	if (ret)
 		goto out_fini;
-- 
2.16.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm: Introduce per-device driver_features
  2018-09-13 13:16 [PATCH 1/2] drm: Introduce per-device driver_features Ville Syrjala
  2018-09-13 13:16 ` [PATCH 2/2] drm/i915: Clear DRIVER_ATOMIC on a per-device basis Ville Syrjala
@ 2018-09-13 13:27 ` Chris Wilson
  2018-09-13 13:50 ` [Intel-gfx] " Daniel Vetter
  2 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-09-13 13:27 UTC (permalink / raw)
  To: Ville Syrjala, dri-devel; +Cc: intel-gfx

Quoting Ville Syrjala (2018-09-13 14:16:21)
> From: Ville Syrjälä <ville.syrjala@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@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

The gradual transition makes sense (less work!), as does being able to
deselect features on individual devices.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drm/i915: Clear DRIVER_ATOMIC on a per-device basis
  2018-09-13 13:16 ` [PATCH 2/2] drm/i915: Clear DRIVER_ATOMIC on a per-device basis Ville Syrjala
@ 2018-09-13 13:28   ` Chris Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-09-13 13:28 UTC (permalink / raw)
  To: Ville Syrjala, dri-devel; +Cc: intel-gfx

Quoting Ville Syrjala (2018-09-13 14:16:22)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we're clearing DRIVER_ATOMIC in driver.driver_features
> for older platforms. This will not work correctly should we ever
> have a system with and old and new GPU in it. While that is not
> possible currently let's make the code more correct and use
> the per-device driver_features instead.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

One day we will be able to have driver const again,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm: Introduce per-device driver_features
  2018-09-13 13:16 [PATCH 1/2] drm: Introduce per-device driver_features Ville Syrjala
  2018-09-13 13:16 ` [PATCH 2/2] drm/i915: Clear DRIVER_ATOMIC on a per-device basis Ville Syrjala
  2018-09-13 13:27 ` [PATCH 1/2] drm: Introduce per-device driver_features Chris Wilson
@ 2018-09-13 13:50 ` Daniel Vetter
  2018-09-13 14:29   ` Ville Syrjälä
  2 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2018-09-13 13:50 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx, dri-devel

On Thu, Sep 13, 2018 at 04:16:21PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@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@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm: Introduce per-device driver_features
  2018-09-13 13:50 ` [Intel-gfx] " Daniel Vetter
@ 2018-09-13 14:29   ` Ville Syrjälä
  2018-09-13 14:52     ` [Intel-gfx] " Michel Dänzer
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2018-09-13 14:29 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel

On Thu, Sep 13, 2018 at 03:50:01PM +0200, Daniel Vetter wrote:
> On Thu, Sep 13, 2018 at 04:16:21PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@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@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@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.

Thanks for pointing it out. I'll cook it up and send separately after
this lands.

> 
> Irrespective of that, on the series:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm: Introduce per-device driver_features
  2018-09-13 14:29   ` Ville Syrjälä
@ 2018-09-13 14:52     ` Michel Dänzer
  2018-09-13 15:06       ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Michel Dänzer @ 2018-09-13 14:52 UTC (permalink / raw)
  To: Ville Syrjälä, Daniel Vetter; +Cc: intel-gfx, dri-devel

On 2018-09-13 4:29 p.m., Ville Syrjälä wrote:
> On Thu, Sep 13, 2018 at 03:50:01PM +0200, Daniel Vetter wrote:
>> On Thu, Sep 13, 2018 at 04:16:21PM +0300, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@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@chris-wilson.co.uk>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@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.
> 
> Thanks for pointing it out. I'll cook it up and send separately after
> this lands.

I don't suppose you'd like to do amdgpu as well, while you're at it? :)

Either way, this is awesome stuff! This patch is

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm: Introduce per-device driver_features
  2018-09-13 14:52     ` [Intel-gfx] " Michel Dänzer
@ 2018-09-13 15:06       ` Ville Syrjälä
  2018-09-13 16:24         ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2018-09-13 15:06 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: intel-gfx, dri-devel

On Thu, Sep 13, 2018 at 04:52:34PM +0200, Michel Dänzer wrote:
> On 2018-09-13 4:29 p.m., Ville Syrjälä wrote:
> > On Thu, Sep 13, 2018 at 03:50:01PM +0200, Daniel Vetter wrote:
> >> On Thu, Sep 13, 2018 at 04:16:21PM +0300, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@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@chris-wilson.co.uk>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@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.
> > 
> > Thanks for pointing it out. I'll cook it up and send separately after
> > this lands.
> 
> I don't suppose you'd like to do amdgpu as well, while you're at it? :)

Sure. I'll take a gander at it as well.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm: Introduce per-device driver_features
  2018-09-13 15:06       ` Ville Syrjälä
@ 2018-09-13 16:24         ` Ville Syrjälä
  0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2018-09-13 16:24 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: intel-gfx, dri-devel

On Thu, Sep 13, 2018 at 06:06:01PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 13, 2018 at 04:52:34PM +0200, Michel Dänzer wrote:
> > On 2018-09-13 4:29 p.m., Ville Syrjälä wrote:
> > > On Thu, Sep 13, 2018 at 03:50:01PM +0200, Daniel Vetter wrote:
> > >> On Thu, Sep 13, 2018 at 04:16:21PM +0300, Ville Syrjala wrote:
> > >>> From: Ville Syrjälä <ville.syrjala@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@chris-wilson.co.uk>
> > >>> Signed-off-by: Ville Syrjälä <ville.syrjala@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.
> > > 
> > > Thanks for pointing it out. I'll cook it up and send separately after
> > > this lands.
> > 
> > I don't suppose you'd like to do amdgpu as well, while you're at it? :)
> 
> Sure. I'll take a gander at it as well.

Series pushed to drm-misc-next. Thanks for the reviews.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-09-13 16:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-13 13:16 [PATCH 1/2] drm: Introduce per-device driver_features Ville Syrjala
2018-09-13 13:16 ` [PATCH 2/2] drm/i915: Clear DRIVER_ATOMIC on a per-device basis Ville Syrjala
2018-09-13 13:28   ` Chris Wilson
2018-09-13 13:27 ` [PATCH 1/2] drm: Introduce per-device driver_features Chris Wilson
2018-09-13 13:50 ` [Intel-gfx] " Daniel Vetter
2018-09-13 14:29   ` Ville Syrjälä
2018-09-13 14:52     ` [Intel-gfx] " Michel Dänzer
2018-09-13 15:06       ` Ville Syrjälä
2018-09-13 16:24         ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox