[02/26] drm: Switch DRIVER_ flags to an enum
diff mbox series

Message ID 20190124165831.16427-3-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • some cleanups, mostly around fbdev emulation
Related show

Commit Message

Daniel Vetter Jan. 24, 2019, 4:58 p.m. UTC
And move the documenation we alreay have into kerneldoc, plus a bit of
polish while at it.

FIXME: Updates for drm_driver.driver_features are missing, need to get
Sam's patches landed first.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-internals.rst |  62 -------------
 include/drm/drm_drv.h               | 138 ++++++++++++++++++++++++----
 2 files changed, 121 insertions(+), 79 deletions(-)

Comments

Sam Ravnborg Jan. 24, 2019, 5:38 p.m. UTC | #1
Hi Daniel.

> +
> +	/**
> +	 * @DRIVER_HAVE_DMA:
> +	 *
> +	 * Driver supports DMA, the userspace DMA API will be supported. Only
> +	 * for legacy drivers. Do not use.
> +	 */
> +	DRIVER_HAVE_DMA			= BIT(4),

What about grouping all the "legacy, do not use" flags in the bottom.
Maybe counting backwards.

Then one does not have "noise" in-between when trying to rad and understand the
relevant flags for a new driver.
Unless I am mistaken nothing should break because we change the bit for a certain
function, but I see you kept the current order thus the current vlaues.


> @@ -662,7 +766,7 @@ static inline bool drm_dev_is_unplugged(struct drm_device *dev)
>   * @feature: feature flag
>   *
>   * This checks @dev for driver features, see &drm_driver.driver_features,
> - * &drm_device.driver_features, and the various DRIVER_\* flags.
> + * &drm_device.driver_features, and the various &enum drm_driver_feature flags.
>   *
>   * Returns true if the @feature is supported, false otherwise.
>   */

Thanks for fixing this - I had a patch floating to do the same.
But this fix is better than what I did.

With or without a change in ordering you can add:

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Daniel Vetter Jan. 25, 2019, 9:35 a.m. UTC | #2
On Thu, Jan 24, 2019 at 06:38:33PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
> 
> > +
> > +	/**
> > +	 * @DRIVER_HAVE_DMA:
> > +	 *
> > +	 * Driver supports DMA, the userspace DMA API will be supported. Only
> > +	 * for legacy drivers. Do not use.
> > +	 */
> > +	DRIVER_HAVE_DMA			= BIT(4),
> 
> What about grouping all the "legacy, do not use" flags in the bottom.
> Maybe counting backwards.
> 
> Then one does not have "noise" in-between when trying to rad and understand the
> relevant flags for a new driver.
> Unless I am mistaken nothing should break because we change the bit for a certain
> function, but I see you kept the current order thus the current vlaues.

Good idea, will do.

> > @@ -662,7 +766,7 @@ static inline bool drm_dev_is_unplugged(struct drm_device *dev)
> >   * @feature: feature flag
> >   *
> >   * This checks @dev for driver features, see &drm_driver.driver_features,
> > - * &drm_device.driver_features, and the various DRIVER_\* flags.
> > + * &drm_device.driver_features, and the various &enum drm_driver_feature flags.
> >   *
> >   * Returns true if the @feature is supported, false otherwise.
> >   */
> 
> Thanks for fixing this - I had a patch floating to do the same.
> But this fix is better than what I did.

Just realized I've forgotten to remove the FIXME from the commit message
about this.

> With or without a change in ordering you can add:
> 
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

I'll respind, thanks for reviewing. Can I motivate you to also review
patch 1, with that I could merge the first 2 in this series.

Thanks, Daniel

Patch
diff mbox series

diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index 2caf21effd28..3ae23a5454ac 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -39,68 +39,6 @@  sections.
 Driver Information
 ------------------
 
-Driver Features
-~~~~~~~~~~~~~~~
-
-Drivers inform the DRM core about their requirements and supported
-features by setting appropriate flags in the driver_features field.
-Since those flags influence the DRM core behaviour since registration
-time, most of them must be set to registering the :c:type:`struct
-drm_driver <drm_driver>` instance.
-
-u32 driver_features;
-
-DRIVER_USE_AGP
-    Driver uses AGP interface, the DRM core will manage AGP resources.
-
-DRIVER_LEGACY
-    Denote a legacy driver using shadow attach. Don't use.
-
-DRIVER_KMS_LEGACY_CONTEXT
-    Used only by nouveau for backwards compatibility with existing userspace.
-    Don't use.
-
-DRIVER_PCI_DMA
-    Driver is capable of PCI DMA, mapping of PCI DMA buffers to
-    userspace will be enabled. Deprecated.
-
-DRIVER_SG
-    Driver can perform scatter/gather DMA, allocation and mapping of
-    scatter/gather buffers will be enabled. Deprecated.
-
-DRIVER_HAVE_DMA
-    Driver supports DMA, the userspace DMA API will be supported.
-    Deprecated.
-
-DRIVER_HAVE_IRQ; DRIVER_IRQ_SHARED
-    DRIVER_HAVE_IRQ indicates whether the driver has an IRQ handler
-    managed by the DRM Core. The core will support simple IRQ handler
-    installation when the flag is set. The installation process is
-    described in ?.
-
-    DRIVER_IRQ_SHARED indicates whether the device & handler support
-    shared IRQs (note that this is required of PCI drivers).
-
-DRIVER_GEM
-    Driver use the GEM memory manager.
-
-DRIVER_MODESET
-    Driver supports mode setting interfaces (KMS).
-
-DRIVER_PRIME
-    Driver implements DRM PRIME buffer sharing.
-
-DRIVER_RENDER
-    Driver supports dedicated render nodes.
-
-DRIVER_ATOMIC
-    Driver supports atomic properties. In this case the driver must
-    implement appropriate obj->atomic_get_property() vfuncs for any
-    modeset objects with driver specific properties.
-
-DRIVER_SYNCOBJ
-    Driver support drm sync objects.
-
 Major, Minor and Patchlevel
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 35af23f5fa0d..fbbcd2887ea8 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -41,21 +41,120 @@  struct drm_display_mode;
 struct drm_mode_create_dumb;
 struct drm_printer;
 
-/* driver capabilities and requirements mask */
-#define DRIVER_USE_AGP			0x1
-#define DRIVER_LEGACY			0x2
-#define DRIVER_PCI_DMA			0x8
-#define DRIVER_SG			0x10
-#define DRIVER_HAVE_DMA			0x20
-#define DRIVER_HAVE_IRQ			0x40
-#define DRIVER_IRQ_SHARED		0x80
-#define DRIVER_GEM			0x1000
-#define DRIVER_MODESET			0x2000
-#define DRIVER_PRIME			0x4000
-#define DRIVER_RENDER			0x8000
-#define DRIVER_ATOMIC			0x10000
-#define DRIVER_KMS_LEGACY_CONTEXT	0x20000
-#define DRIVER_SYNCOBJ                  0x40000
+/**
+ * enum drm_driver_feature - feature flags
+ *
+ * See &drm_driver.driver_features, drm_device.driver_features and
+ * drm_core_check_feature().
+ */
+enum drm_driver_feature {
+	/**
+	 * @DRIVER_USE_AGP:
+	 *
+	 * Set up DRM AGP support, see drm_agp_init(), the DRM core will manage
+	 * AGP resources. New drivers don't need this.
+	 */
+	DRIVER_USE_AGP			= BIT(0),
+	/**
+	 * @DRIVER_LEGACY:
+	 *
+	 * Denote a legacy driver using shadow attach. Do not use.
+	 */
+	DRIVER_LEGACY			= BIT(1),
+	/**
+	 * @DRIVER_PCI_DMA:
+	 *
+	 * Driver is capable of PCI DMA, mapping of PCI DMA buffers to userspace
+	 * will be enabled. Only for legacy drivers. Do not use.
+	 */
+	DRIVER_PCI_DMA			= BIT(2),
+	/**
+	 * @DRIVER_SG:
+	 *
+	 * Driver can perform scatter/gather DMA, allocation and mapping of
+	 * scatter/gather buffers will be enabled. Only for legacy drivers. Do
+	 * not use.
+	 */
+	DRIVER_SG			= BIT(3),
+
+	/**
+	 * @DRIVER_HAVE_DMA:
+	 *
+	 * Driver supports DMA, the userspace DMA API will be supported. Only
+	 * for legacy drivers. Do not use.
+	 */
+	DRIVER_HAVE_DMA			= BIT(4),
+	/**
+	 * @DRIVER_HAVE_IRQ:
+	 *
+	 * Legacy irq support. Only for legacy drivers. Do not use.
+	 *
+	 * New drivers can either use the drm_irq_install() and
+	 * drm_irq_uninstall() helper functions, or roll their own irq support
+	 * code by calling request_irq() directly.
+	 */
+	DRIVER_HAVE_IRQ			= BIT(5),
+	/**
+	 * @DRIVER_IRQ_SHARED:
+	 *
+	 * Indicates to drm_irq_install() that a shared irq should be requested.
+	 *
+	 * FIXME: This should be an explicit argument for non-legacy drivers, or
+	 * at least the default for PCI devices (which would cover all current
+	 * users).
+	 */
+	DRIVER_IRQ_SHARED		= BIT(6),
+	/**
+	 * @DRIVER_GEM:
+	 *
+	 * Driver use the GEM memory manager. This should be set for all modern
+	 * drivers.
+	 */
+	DRIVER_GEM			= BIT(7),
+	/**
+	 * @DRIVER_MODESET:
+	 *
+	 * Driver supports mode setting interfaces (KMS).
+	 */
+	DRIVER_MODESET			= BIT(8),
+	/**
+	 * @DRIVER_PRIME:
+	 *
+	 * Driver implements DRM PRIME buffer sharing.
+	 */
+	DRIVER_PRIME			= BIT(9),
+	/**
+	 * @DRIVER_RENDER:
+	 *
+	 * Driver supports dedicated render nodes. See also the :ref:`section on
+	 * render nodes <drm_render_node>` for details.
+	 */
+	DRIVER_RENDER			= BIT(10),
+	/**
+	 * @DRIVER_ATOMIC:
+	 *
+	 * Driver supports the full atomic modesetting userspace API. Drivers
+	 * which only use atomic internally, but do not the support the full
+	 * userspace API (e.g. not all properties converted to atomic, or
+	 * multi-plane updates are not guaranteed to be tear-free) should not
+	 * set this flag.
+	 */
+	DRIVER_ATOMIC			= BIT(11),
+	/**
+	 * @DRIVER_KMS_LEGACY_CONTEXT:
+	 *
+	 * Used only by nouveau for backwards compatibility with existing
+	 * userspace.  Do not use.
+	 */
+	DRIVER_KMS_LEGACY_CONTEXT	= BIT(12),
+	/**
+	 * @DRIVER_SYNCOBJ:
+	 *
+	 * Driver supports &drm_syncobj for explicit synchronization of command
+	 * submission.
+	 */
+	DRIVER_SYNCOBJ                  = BIT(13),
+};
 
 /**
  * struct drm_driver - DRM driver structure
@@ -579,7 +678,12 @@  struct drm_driver {
 	/** @date: driver date */
 	char *date;
 
-	/** @driver_features: driver features */
+	/**
+	 * @driver_features:
+	 * Driver features, see &enum drm_driver_feature. Drivers can disable
+	 * some features on a per-instance basis using
+	 * &drm_device.driver_features.
+	 */
 	u32 driver_features;
 
 	/**
@@ -662,7 +766,7 @@  static inline bool drm_dev_is_unplugged(struct drm_device *dev)
  * @feature: feature flag
  *
  * This checks @dev for driver features, see &drm_driver.driver_features,
- * &drm_device.driver_features, and the various DRIVER_\* flags.
+ * &drm_device.driver_features, and the various &enum drm_driver_feature flags.
  *
  * Returns true if the @feature is supported, false otherwise.
  */