All of lore.kernel.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
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ 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] 11+ 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
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ 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] 11+ 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 subsequent siblings)
  4 siblings, 0 replies; 11+ 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] 11+ 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; 11+ 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] 11+ 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ä
  2018-09-13 14:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
  2018-09-13 15:29 ` ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 1 reply; 11+ 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] 11+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm: Introduce per-device driver_features
  2018-09-13 13:16 [PATCH 1/2] drm: Introduce per-device driver_features Ville Syrjala
                   ` (2 preceding siblings ...)
  2018-09-13 13:50 ` [Intel-gfx] " Daniel Vetter
@ 2018-09-13 14:20 ` Patchwork
  2018-09-13 15:29 ` ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-09-13 14:20 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm: Introduce per-device driver_features
URL   : https://patchwork.freedesktop.org/series/49639/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4816 -> Patchwork_10168 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49639/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10168 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@amdgpu/amd_cs_nop@sync-fork-gfx0:
      fi-kbl-8809g:       PASS -> DMESG-WARN (fdo#107762) +1

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         PASS -> DMESG-FAIL (fdo#107164)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_psr@primary_mmap_gtt:
      {fi-cnl-u}:         NOTRUN -> FAIL (fdo#107383) +3

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           PASS -> FAIL (fdo#107336)

    igt@pm_rpm@basic-pci-d3-state:
      fi-glk-dsi:         PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (49 -> 45) ==

  Additional (2): fi-cnl-u fi-icl-u 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


== Build changes ==

    * Linux: CI_DRM_4816 -> Patchwork_10168

  CI_DRM_4816: 5bebc54ac552e3716bfe0f1f7eb0babfbda49f09 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10168: c466afab936d8b828149178fc0adcf72bfcefa9c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c466afab936d drm/i915: Clear DRIVER_ATOMIC on a per-device basis
ff6ab987a3f2 drm: Introduce per-device driver_features

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10168/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [1/2] drm: Introduce per-device driver_features
  2018-09-13 13:16 [PATCH 1/2] drm: Introduce per-device driver_features Ville Syrjala
                   ` (3 preceding siblings ...)
  2018-09-13 14:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2018-09-13 15:29 ` Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-09-13 15:29 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm: Introduce per-device driver_features
URL   : https://patchwork.freedesktop.org/series/49639/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4816_full -> Patchwork_10168_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

  Here are the changes found in Patchwork_10168_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          PASS -> FAIL (fdo#105767)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-glk:          FAIL (fdo#106680) -> PASS

    igt@kms_color@pipe-c-ctm-max:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +34

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4816 -> Patchwork_10168

  CI_DRM_4816: 5bebc54ac552e3716bfe0f1f7eb0babfbda49f09 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10168: c466afab936d8b828149178fc0adcf72bfcefa9c @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10168/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ 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; 11+ 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] 11+ messages in thread

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

Thread overview: 11+ 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ä
2018-09-13 14:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2018-09-13 15:29 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.