thr3ads.net - Nouveau - [Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers [Feb 2020]

If this information is useful, please help other people find it:
Share via:

Thomas Zimmermann

2020-Feb-25 15:58 UTC

[Nouveau] [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver

This patchset moves legacy, non-KMS driver state from struct drm_driver
into struct drm_legacy_state. Only non-KMS drivers provide an instance
of the latter structure. One special case is nouveau, which supports
legacy interfaces. It also provides an instance of the legacy state if
the legacy interfaces have been enabled (i.e., defines the config option
CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)

I reviewed all call sites of legacy state and functions to verify that
DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that
DRIVER_MODESET is not set.

With the mutable KMS state removed, instances of struct drm_driver can
be declared as constant. The patchset modifies the DRM core accordingly.
Individual drivers can follow later on.

Thomas Zimmermann (3):
  drm: Add separate state structure for legacy, non-KMS drivers
  drm: Move non-kms driver state into struct drm_legacy_state
  drm: Constify struct drm_driver in DRM core

 drivers/gpu/drm/drm_bufs.c            | 10 +++++-----
 drivers/gpu/drm/drm_context.c         |  9 +++++----
 drivers/gpu/drm/drm_drv.c             | 12 ++++++++----
 drivers/gpu/drm/drm_file.c            |  4 ++--
 drivers/gpu/drm/drm_legacy_misc.c     |  6 +++---
 drivers/gpu/drm/drm_lock.c            |  7 ++++---
 drivers/gpu/drm/drm_pci.c             | 16 ++++++++++------
 drivers/gpu/drm/drm_vblank.c          | 11 ++++++-----
 drivers/gpu/drm/i810/i810_drv.c       | 10 +++++++---
 drivers/gpu/drm/mga/mga_drv.c         | 16 ++++++++++------
 drivers/gpu/drm/nouveau/nouveau_drm.c |  8 ++++++++
 drivers/gpu/drm/r128/r128_drv.c       | 16 ++++++++++------
 drivers/gpu/drm/savage/savage_drv.c   | 12 ++++++++----
 drivers/gpu/drm/sis/sis_drv.c         |  8 ++++++--
 drivers/gpu/drm/tdfx/tdfx_drv.c       |  6 +++++-
 drivers/gpu/drm/via/via_drv.c         | 16 ++++++++++------
 include/drm/drm_device.h              |  2 +-
 include/drm/drm_drv.h                 | 21 +++++----------------
 include/drm/drm_legacy.h              | 27 +++++++++++++++++++++++----
 include/drm/drm_pci.h                 |  4 ++--
 20 files changed, 138 insertions(+), 83 deletions(-)

--
2.25.0

Thomas Zimmermann

2020-Feb-25 15:59 UTC

head link

[Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers

Non-KMS drivers store state in struct drm_driver. This bloats the
structure for KMS drivers and prevents it from being declared with
'static const' qualifiers. Moving the non-KMS state into a separate
data structure resolves this.

Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
---
 drivers/gpu/drm/drm_drv.c             | 4 ++++
 drivers/gpu/drm/i810/i810_drv.c       | 4 ++++
 drivers/gpu/drm/mga/mga_drv.c         | 4 ++++
 drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++
 drivers/gpu/drm/r128/r128_drv.c       | 4 ++++
 drivers/gpu/drm/savage/savage_drv.c   | 4 ++++
 drivers/gpu/drm/sis/sis_drv.c         | 4 ++++
 drivers/gpu/drm/tdfx/tdfx_drv.c       | 4 ++++
 drivers/gpu/drm/via/via_drv.c         | 4 ++++
 include/drm/drm_drv.h                 | 3 +++
 include/drm/drm_legacy.h              | 6 ++++++
 11 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7b1a628d1f6e..4ba0df097602 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev,
 	if (WARN_ON(!parent))
 		return -EINVAL;
 
+	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
+	    WARN_ON(!driver->legacy))
+		return -EINVAL;
+
 	kref_init(&dev->ref);
 	dev->dev = get_device(parent);
 	dev->driver = driver;
diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
index 0e53a066d4db..55f17f00bae9 100644
--- a/drivers/gpu/drm/i810/i810_drv.c
+++ b/drivers/gpu/drm/i810/i810_drv.c
@@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = {
 	.llseek = noop_llseek,
 };
 
+static struct drm_legacy_state = i810_legacy_state {
+};
+
 static struct drm_driver driver = {
 	.driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY,
 	.dev_priv_size = sizeof(drm_i810_buf_priv_t),
@@ -71,6 +74,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.legacy = &i810_legacy_state,
 };
 
 static struct pci_driver i810_pci_driver = {
diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
index 71128e6f6ae9..4865982d949c 100644
--- a/drivers/gpu/drm/mga/mga_drv.c
+++ b/drivers/gpu/drm/mga/mga_drv.c
@@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = {
 	.llseek = noop_llseek,
 };
 
+static struct drm_legacy_state = mga_legacy_state {
+};
+
 static struct drm_driver driver = {
 	.driver_features  	    DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY |
@@ -78,6 +81,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.legacy = &mga_legacy_state,
 };
 
 static struct pci_driver mga_pci_driver = {
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 6b1629c14dd7..c88cf32e521c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1103,6 +1103,11 @@ nouveau_driver_fops = {
 	.llseek = noop_llseek,
 };
 
+#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
+static struct drm_legacy_state nouveau_legacy_state{
+};
+#endif
+
 static struct drm_driver
 driver_stub = {
 	.driver_features @@ -1150,6 +1155,9 @@ driver_stub = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
+	.legacy = &nouveau_legacy_state,
+#endif
 };
 
 static struct pci_device_id
diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
index b7a5f162ebae..ecece3a48d93 100644
--- a/drivers/gpu/drm/r128/r128_drv.c
+++ b/drivers/gpu/drm/r128/r128_drv.c
@@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = {
 	.llseek = noop_llseek,
 };
 
+static struct drm_legacy_state = r128_legacy_state {
+};
+
 static struct drm_driver driver = {
 	.driver_features  	    DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG |
DRIVER_LEGACY |
@@ -81,6 +84,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.legacy = &r128_legacy_state,
 };
 
 int r128_driver_load(struct drm_device *dev, unsigned long flags)
diff --git a/drivers/gpu/drm/savage/savage_drv.c
b/drivers/gpu/drm/savage/savage_drv.c
index 799bd11adb9c..c0a7146fbde1 100644
--- a/drivers/gpu/drm/savage/savage_drv.c
+++ b/drivers/gpu/drm/savage/savage_drv.c
@@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops = {
 	.llseek = noop_llseek,
 };
 
+static struct drm_legacy_state = savage_legacy_state {
+};
+
 static struct drm_driver driver = {
 	.driver_features  	    DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA |
DRIVER_LEGACY,
@@ -65,6 +68,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.legacy = &savage_legacy_state,
 };
 
 static struct pci_driver savage_pci_driver = {
diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
index 2c54b33abb54..1c794a82c8e6 100644
--- a/drivers/gpu/drm/sis/sis_drv.c
+++ b/drivers/gpu/drm/sis/sis_drv.c
@@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev,
struct drm_file *file)
 	kfree(file_priv);
 }
 
+static struct drm_legacy_state = sis_legacy_state {
+};
+
 static struct drm_driver driver = {
 	.driver_features = DRIVER_USE_AGP | DRIVER_LEGACY,
 	.load = sis_driver_load,
@@ -116,6 +119,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.legacy = &sis_legacy_state,
 };
 
 static struct pci_driver sis_pci_driver = {
diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c
index ab699bf0ac5c..17aa93d82d6b 100644
--- a/drivers/gpu/drm/tdfx/tdfx_drv.c
+++ b/drivers/gpu/drm/tdfx/tdfx_drv.c
@@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = {
 	.llseek = noop_llseek,
 };
 
+static struct drm_legacy_state = tdfx_legacy_state {
+};
+
 static struct drm_driver driver = {
 	.driver_features = DRIVER_LEGACY,
 	.fops = &tdfx_driver_fops,
@@ -65,6 +68,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.legacy = &tdfx_legacy_state,
 };
 
 static struct pci_driver tdfx_pci_driver = {
diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c
index 5da38082821f..22113d93ecdd 100644
--- a/drivers/gpu/drm/via/via_drv.c
+++ b/drivers/gpu/drm/via/via_drv.c
@@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = {
 	.llseek = noop_llseek,
 };
 
+static struct drm_legacy_state = via_legacy_state {
+};
+
 static struct drm_driver driver = {
 	.driver_features  	    DRIVER_USE_AGP | DRIVER_HAVE_IRQ | DRIVER_LEGACY,
@@ -97,6 +100,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.legacy = &via_legacy_state,
 };
 
 static struct pci_driver via_pci_driver = {
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 97109df5beac..94078af0cc28 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -40,6 +40,7 @@ struct dma_buf_attachment;
 struct drm_display_mode;
 struct drm_mode_create_dumb;
 struct drm_printer;
+struct drm_legacy_state;
 
 /**
  * enum drm_driver_feature - feature flags
@@ -599,6 +600,8 @@ struct drm_driver {
 	const struct file_operations *fops;
 
 	/* Everything below here is for legacy driver, never use! */
+	struct drm_legacy_state *legacy;
+
 	/* private: */
 
 	/* List of devices hanging off this driver with stealth attach. */
diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
index dcef3598f49e..c98a29e1c2b3 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -49,6 +49,12 @@ struct pci_driver;
  * you're doing it terribly wrong.
  */
 
+/**
+ * State for non-KMS drivers.
+ */
+struct drm_legacy_state {
+};
+
 /**
  * DMA buffer.
  */
-- 
2.25.0

Thomas Zimmermann

2020-Feb-25 15:59 UTC

head link

[Nouveau] [PATCH 2/3] drm: Move non-kms driver state into struct drm_legacy_state

All non-kms driver fields are now located in struct drm_legacy_state. A
driver-wide instance is available via struct drm_driver.legacy. The call
sites test if the driver is marked with DRIVER_LEGACY before accessing
the fields.

Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
---
 drivers/gpu/drm/drm_bufs.c          | 10 +++++-----
 drivers/gpu/drm/drm_context.c       |  9 +++++----
 drivers/gpu/drm/drm_drv.c           |  8 ++++----
 drivers/gpu/drm/drm_file.c          |  4 ++--
 drivers/gpu/drm/drm_legacy_misc.c   |  6 +++---
 drivers/gpu/drm/drm_lock.c          |  7 ++++---
 drivers/gpu/drm/drm_pci.c           |  8 +++++---
 drivers/gpu/drm/drm_vblank.c        | 11 ++++++-----
 drivers/gpu/drm/i810/i810_drv.c     |  8 ++++----
 drivers/gpu/drm/mga/mga_drv.c       | 14 +++++++-------
 drivers/gpu/drm/r128/r128_drv.c     | 14 +++++++-------
 drivers/gpu/drm/savage/savage_drv.c | 10 +++++-----
 drivers/gpu/drm/sis/sis_drv.c       |  6 +++---
 drivers/gpu/drm/tdfx/tdfx_drv.c     |  2 +-
 drivers/gpu/drm/via/via_drv.c       | 14 +++++++-------
 include/drm/drm_drv.h               | 14 --------------
 include/drm/drm_legacy.h            | 11 +++++++++++
 17 files changed, 79 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 19297e58b232..fbbd5f5820a4 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -812,7 +812,7 @@ int drm_legacy_addbufs_agp(struct drm_device *dev,
 		buf->pending = 0;
 		buf->file_priv = NULL;
 
-		buf->dev_priv_size = dev->driver->dev_priv_size;
+		buf->dev_priv_size = dev->driver->legacy->dev_priv_size;
 		buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL);
 		if (!buf->dev_private) {
 			/* Set count correctly so we free the proper amount. */
@@ -1011,7 +1011,7 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
 			buf->pending = 0;
 			buf->file_priv = NULL;
 
-			buf->dev_priv_size = dev->driver->dev_priv_size;
+			buf->dev_priv_size = dev->driver->legacy->dev_priv_size;
 			buf->dev_private = kzalloc(buf->dev_priv_size,
 						GFP_KERNEL);
 			if (!buf->dev_private) {
@@ -1175,7 +1175,7 @@ static int drm_legacy_addbufs_sg(struct drm_device *dev,
 		buf->pending = 0;
 		buf->file_priv = NULL;
 
-		buf->dev_priv_size = dev->driver->dev_priv_size;
+		buf->dev_priv_size = dev->driver->legacy->dev_priv_size;
 		buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL);
 		if (!buf->dev_private) {
 			/* Set count correctly so we free the proper amount. */
@@ -1581,8 +1581,8 @@ int drm_legacy_dma_ioctl(struct drm_device *dev, void
*data,
 	if (!drm_core_check_feature(dev, DRIVER_LEGACY))
 		return -EOPNOTSUPP;
 
-	if (dev->driver->dma_ioctl)
-		return dev->driver->dma_ioctl(dev, data, file_priv);
+	if (dev->driver->legacy->dma_ioctl)
+		return dev->driver->legacy->dma_ioctl(dev, data, file_priv);
 	else
 		return -EINVAL;
 }
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
index 1f802d8e5681..b433f755d7c2 100644
--- a/drivers/gpu/drm/drm_context.c
+++ b/drivers/gpu/drm/drm_context.c
@@ -145,8 +145,9 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev,
struct drm_file *file)
 	list_for_each_entry_safe(pos, tmp, &dev->ctxlist, head) {
 		if (pos->tag == file &&
 		    pos->handle != DRM_KERNEL_CONTEXT) {
-			if (dev->driver->context_dtor)
-				dev->driver->context_dtor(dev, pos->handle);
+			if (dev->driver->legacy->context_dtor)
+				dev->driver->legacy->context_dtor(dev,
+								  pos->handle);
 
 			drm_legacy_ctxbitmap_free(dev, pos->handle);
 			list_del(&pos->head);
@@ -500,8 +501,8 @@ int drm_legacy_rmctx(struct drm_device *dev, void *data,
 
 	DRM_DEBUG("%d\n", ctx->handle);
 	if (ctx->handle != DRM_KERNEL_CONTEXT) {
-		if (dev->driver->context_dtor)
-			dev->driver->context_dtor(dev, ctx->handle);
+		if (dev->driver->legacy->context_dtor)
+			dev->driver->legacy->context_dtor(dev, ctx->handle);
 		drm_legacy_ctxbitmap_free(dev, ctx->handle);
 	}
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 4ba0df097602..4875b76e7656 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -625,14 +625,14 @@ int drm_dev_init(struct drm_device *dev,
 	if (WARN_ON(!parent))
 		return -EINVAL;
 
-	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
-	    WARN_ON(!driver->legacy))
-		return -EINVAL;
-
 	kref_init(&dev->ref);
 	dev->dev = get_device(parent);
 	dev->driver = driver;
 
+	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
+	    WARN_ON(!driver->legacy))
+		return -EINVAL;
+
 	/* no per-device feature limits by default */
 	dev->driver_features = ~0u;
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index c4c704e01961..e9cb7cf2eb48 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -254,8 +254,8 @@ void drm_file_free(struct drm_file *file)
 		  atomic_read(&dev->open_count));
 
 	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
-	    dev->driver->preclose)
-		dev->driver->preclose(dev, file);
+	    dev->driver->legacy->preclose)
+		dev->driver->legacy->preclose(dev, file);
 
 	if (drm_core_check_feature(dev, DRIVER_LEGACY))
 		drm_legacy_lock_release(dev, file->filp);
diff --git a/drivers/gpu/drm/drm_legacy_misc.c
b/drivers/gpu/drm/drm_legacy_misc.c
index 8f54e6a78b6f..07172d0a4f6f 100644
--- a/drivers/gpu/drm/drm_legacy_misc.c
+++ b/drivers/gpu/drm/drm_legacy_misc.c
@@ -60,9 +60,9 @@ int drm_legacy_setup(struct drm_device * dev)
 {
 	int ret;
 
-	if (dev->driver->firstopen &&
-	    drm_core_check_feature(dev, DRIVER_LEGACY)) {
-		ret = dev->driver->firstopen(dev);
+	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
+	    dev->driver->legacy->firstopen) {
+		ret = dev->driver->legacy->firstopen(dev);
 		if (ret != 0)
 			return ret;
 	}
diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index 2c79e8199e3c..f07bf74065ec 100644
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -223,7 +223,7 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
 		  ret ? "interrupted" : "has lock");
 	if (ret) return ret;
 
-	/* don't set the block all signals on the master process for now 
+	/* don't set the block all signals on the master process for now
 	 * really probably not the correct answer but lets us debug xkb
  	 * xserver for now */
 	if (!drm_is_current_master(file_priv)) {
@@ -231,9 +231,10 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
 		dev->sigdata.lock = master->lock.hw_lock;
 	}
 
-	if (dev->driver->dma_quiescent && (lock->flags &
_DRM_LOCK_QUIESCENT))
+	if (dev->driver->legacy->dma_quiescent &&
+	    (lock->flags & _DRM_LOCK_QUIESCENT))
 	{
-		if (dev->driver->dma_quiescent(dev)) {
+		if (dev->driver->legacy->dma_quiescent(dev)) {
 			DRM_DEBUG("%d waiting for DMA quiescent\n",
 				  lock->context);
 			return -EBUSY;
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index c6bb98729a26..24ed0e92fd66 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -239,7 +239,8 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
 	/* No locking needed since shadow-attach is single-threaded since it may
 	 * only be called from the per-driver module init hook. */
 	if (drm_core_check_feature(dev, DRIVER_LEGACY))
-		list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list);
+		list_add_tail(&dev->legacy_dev_list,
+			      &driver->legacy->legacy_dev_list);
 
 	return 0;
 
@@ -275,7 +276,7 @@ int drm_legacy_pci_init(struct drm_driver *driver, struct
pci_driver *pdriver)
 		return -EINVAL;
 
 	/* If not using KMS, fall back to stealth mode manual scanning. */
-	INIT_LIST_HEAD(&driver->legacy_dev_list);
+	INIT_LIST_HEAD(&driver->legacy->legacy_dev_list);
 	for (i = 0; pdriver->id_table[i].vendor != 0; i++) {
 		pid = &pdriver->id_table[i];
 
@@ -317,7 +318,8 @@ void drm_legacy_pci_exit(struct drm_driver *driver, struct
pci_driver *pdriver)
 	if (!(driver->driver_features & DRIVER_LEGACY)) {
 		WARN_ON(1);
 	} else {
-		list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
+		list_for_each_entry_safe(dev, tmp,
+					 &driver->legacy->legacy_dev_list,
 					 legacy_dev_list) {
 			list_del(&dev->legacy_dev_list);
 			drm_put_dev(dev);
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 47fc4339ec7f..8655cf04712b 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -30,6 +30,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_legacy.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_print.h>
 #include <drm/drm_vblank.h>
@@ -144,8 +145,8 @@ static u32 __get_vblank_counter(struct drm_device *dev,
unsigned int pipe)
 
 		if (crtc->funcs->get_vblank_counter)
 			return crtc->funcs->get_vblank_counter(crtc);
-	} else if (dev->driver->get_vblank_counter) {
-		return dev->driver->get_vblank_counter(dev, pipe);
+	} else if (dev->driver->legacy->get_vblank_counter) {
+		return dev->driver->legacy->get_vblank_counter(dev, pipe);
 	}
 
 	return drm_vblank_no_hw_counter(dev, pipe);
@@ -364,7 +365,7 @@ static void __disable_vblank(struct drm_device *dev,
unsigned int pipe)
 		if (crtc->funcs->disable_vblank)
 			crtc->funcs->disable_vblank(crtc);
 	} else {
-		dev->driver->disable_vblank(dev, pipe);
+		dev->driver->legacy->disable_vblank(dev, pipe);
 	}
 }
 
@@ -1033,8 +1034,8 @@ static int __enable_vblank(struct drm_device *dev,
unsigned int pipe)
 
 		if (crtc->funcs->enable_vblank)
 			return crtc->funcs->enable_vblank(crtc);
-	} else if (dev->driver->enable_vblank) {
-		return dev->driver->enable_vblank(dev, pipe);
+	} else if (dev->driver->legacy->enable_vblank) {
+		return dev->driver->legacy->enable_vblank(dev, pipe);
 	}
 
 	return -EINVAL;
diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
index 55f17f00bae9..dd036e3ceb3d 100644
--- a/drivers/gpu/drm/i810/i810_drv.c
+++ b/drivers/gpu/drm/i810/i810_drv.c
@@ -56,16 +56,16 @@ static const struct file_operations i810_driver_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct drm_legacy_state = i810_legacy_state {
+static struct drm_legacy_state i810_legacy_state = {
+	.dev_priv_size = sizeof(drm_i810_buf_priv_t),
+	.preclose = i810_driver_preclose,
+	.dma_quiescent = i810_driver_dma_quiescent,
 };
 
 static struct drm_driver driver = {
 	.driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY,
-	.dev_priv_size = sizeof(drm_i810_buf_priv_t),
 	.load = i810_driver_load,
 	.lastclose = i810_driver_lastclose,
-	.preclose = i810_driver_preclose,
-	.dma_quiescent = i810_driver_dma_quiescent,
 	.ioctls = i810_ioctls,
 	.fops = &i810_driver_fops,
 	.name = DRIVER_NAME,
diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
index 4865982d949c..2454c57f9370 100644
--- a/drivers/gpu/drm/mga/mga_drv.c
+++ b/drivers/gpu/drm/mga/mga_drv.c
@@ -53,27 +53,27 @@ static const struct file_operations mga_driver_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct drm_legacy_state = mga_legacy_state {
+static struct drm_legacy_state mga_legacy_state = {
+	.dev_priv_size = sizeof(drm_mga_buf_priv_t),
+	.dma_quiescent = mga_driver_dma_quiescent,
+	.get_vblank_counter = mga_get_vblank_counter,
+	.enable_vblank = mga_enable_vblank,
+	.disable_vblank = mga_disable_vblank,
+	.dma_ioctl = mga_dma_buffers,
 };
 
 static struct drm_driver driver = {
 	.driver_features  	    DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY |
 	    DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ,
-	.dev_priv_size = sizeof(drm_mga_buf_priv_t),
 	.load = mga_driver_load,
 	.unload = mga_driver_unload,
 	.lastclose = mga_driver_lastclose,
-	.dma_quiescent = mga_driver_dma_quiescent,
-	.get_vblank_counter = mga_get_vblank_counter,
-	.enable_vblank = mga_enable_vblank,
-	.disable_vblank = mga_disable_vblank,
 	.irq_preinstall = mga_driver_irq_preinstall,
 	.irq_postinstall = mga_driver_irq_postinstall,
 	.irq_uninstall = mga_driver_irq_uninstall,
 	.irq_handler = mga_driver_irq_handler,
 	.ioctls = mga_ioctls,
-	.dma_ioctl = mga_dma_buffers,
 	.fops = &mga_driver_fops,
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
index ecece3a48d93..9cfd86117e80 100644
--- a/drivers/gpu/drm/r128/r128_drv.c
+++ b/drivers/gpu/drm/r128/r128_drv.c
@@ -57,26 +57,26 @@ static const struct file_operations r128_driver_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct drm_legacy_state = r128_legacy_state {
+static struct drm_legacy_state r128_legacy_state = {
+	.dev_priv_size = sizeof(drm_r128_buf_priv_t),
+	.preclose = r128_driver_preclose,
+	.get_vblank_counter = r128_get_vblank_counter,
+	.enable_vblank = r128_enable_vblank,
+	.disable_vblank = r128_disable_vblank,
+	.dma_ioctl = r128_cce_buffers,
 };
 
 static struct drm_driver driver = {
 	.driver_features  	    DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG |
DRIVER_LEGACY |
 	    DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ,
-	.dev_priv_size = sizeof(drm_r128_buf_priv_t),
 	.load = r128_driver_load,
-	.preclose = r128_driver_preclose,
 	.lastclose = r128_driver_lastclose,
-	.get_vblank_counter = r128_get_vblank_counter,
-	.enable_vblank = r128_enable_vblank,
-	.disable_vblank = r128_disable_vblank,
 	.irq_preinstall = r128_driver_irq_preinstall,
 	.irq_postinstall = r128_driver_irq_postinstall,
 	.irq_uninstall = r128_driver_irq_uninstall,
 	.irq_handler = r128_driver_irq_handler,
 	.ioctls = r128_ioctls,
-	.dma_ioctl = r128_cce_buffers,
 	.fops = &r128_driver_fops,
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/savage/savage_drv.c
b/drivers/gpu/drm/savage/savage_drv.c
index c0a7146fbde1..2d2680b47eff 100644
--- a/drivers/gpu/drm/savage/savage_drv.c
+++ b/drivers/gpu/drm/savage/savage_drv.c
@@ -47,20 +47,20 @@ static const struct file_operations savage_driver_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct drm_legacy_state = savage_legacy_state {
+static struct drm_legacy_state savage_legacy_state = {
+	.dev_priv_size = sizeof(drm_savage_buf_priv_t),
+	.firstopen = savage_driver_firstopen,
+	.preclose = savage_reclaim_buffers,
+	.dma_ioctl = savage_bci_buffers,
 };
 
 static struct drm_driver driver = {
 	.driver_features  	    DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA |
DRIVER_LEGACY,
-	.dev_priv_size = sizeof(drm_savage_buf_priv_t),
 	.load = savage_driver_load,
-	.firstopen = savage_driver_firstopen,
-	.preclose = savage_reclaim_buffers,
 	.lastclose = savage_driver_lastclose,
 	.unload = savage_driver_unload,
 	.ioctls = savage_ioctls,
-	.dma_ioctl = savage_bci_buffers,
 	.fops = &savage_driver_fops,
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
index 1c794a82c8e6..c465776e3180 100644
--- a/drivers/gpu/drm/sis/sis_drv.c
+++ b/drivers/gpu/drm/sis/sis_drv.c
@@ -99,7 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev,
struct drm_file *file)
 	kfree(file_priv);
 }
 
-static struct drm_legacy_state = sis_legacy_state {
+static struct drm_legacy_state sis_legacy_state = {
+	.preclose = sis_reclaim_buffers_locked,
+	.dma_quiescent = sis_idle,
 };
 
 static struct drm_driver driver = {
@@ -107,9 +109,7 @@ static struct drm_driver driver = {
 	.load = sis_driver_load,
 	.unload = sis_driver_unload,
 	.open = sis_driver_open,
-	.preclose = sis_reclaim_buffers_locked,
 	.postclose = sis_driver_postclose,
-	.dma_quiescent = sis_idle,
 	.lastclose = sis_lastclose,
 	.ioctls = sis_ioctls,
 	.fops = &sis_driver_fops,
diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c
index 17aa93d82d6b..a8a6bce6afbe 100644
--- a/drivers/gpu/drm/tdfx/tdfx_drv.c
+++ b/drivers/gpu/drm/tdfx/tdfx_drv.c
@@ -56,7 +56,7 @@ static const struct file_operations tdfx_driver_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct drm_legacy_state = tdfx_legacy_state {
+static struct drm_legacy_state tdfx_legacy_state = {
 };
 
 static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c
index 22113d93ecdd..62f026da5958 100644
--- a/drivers/gpu/drm/via/via_drv.c
+++ b/drivers/gpu/drm/via/via_drv.c
@@ -71,7 +71,13 @@ static const struct file_operations via_driver_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct drm_legacy_state = via_legacy_state {
+static struct drm_legacy_state via_legacy_state = {
+	.preclose = via_reclaim_buffers_locked,
+	.context_dtor = via_final_context,
+	.get_vblank_counter = via_get_vblank_counter,
+	.enable_vblank = via_enable_vblank,
+	.disable_vblank = via_disable_vblank,
+	.dma_quiescent = via_driver_dma_quiescent,
 };
 
 static struct drm_driver driver = {
@@ -80,17 +86,11 @@ static struct drm_driver driver = {
 	.load = via_driver_load,
 	.unload = via_driver_unload,
 	.open = via_driver_open,
-	.preclose = via_reclaim_buffers_locked,
 	.postclose = via_driver_postclose,
-	.context_dtor = via_final_context,
-	.get_vblank_counter = via_get_vblank_counter,
-	.enable_vblank = via_enable_vblank,
-	.disable_vblank = via_disable_vblank,
 	.irq_preinstall = via_driver_irq_preinstall,
 	.irq_postinstall = via_driver_irq_postinstall,
 	.irq_uninstall = via_driver_irq_uninstall,
 	.irq_handler = via_driver_irq_handler,
-	.dma_quiescent = via_driver_dma_quiescent,
 	.lastclose = via_lastclose,
 	.ioctls = via_ioctls,
 	.fops = &via_driver_fops,
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 94078af0cc28..15dd9a179216 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -601,20 +601,6 @@ struct drm_driver {
 
 	/* Everything below here is for legacy driver, never use! */
 	struct drm_legacy_state *legacy;
-
-	/* private: */
-
-	/* List of devices hanging off this driver with stealth attach. */
-	struct list_head legacy_dev_list;
-	int (*firstopen) (struct drm_device *);
-	void (*preclose) (struct drm_device *, struct drm_file *file_priv);
-	int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
-	int (*dma_quiescent) (struct drm_device *);
-	int (*context_dtor) (struct drm_device *dev, int context);
-	u32 (*get_vblank_counter)(struct drm_device *dev, unsigned int pipe);
-	int (*enable_vblank)(struct drm_device *dev, unsigned int pipe);
-	void (*disable_vblank)(struct drm_device *dev, unsigned int pipe);
-	int dev_priv_size;
 };
 
 int drm_dev_init(struct drm_device *dev,
diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
index c98a29e1c2b3..7b5ee6a94a8c 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -53,6 +53,17 @@ struct pci_driver;
  * State for non-KMS drivers.
  */
 struct drm_legacy_state {
+	/* List of devices hanging off this driver with stealth attach. */
+	struct list_head legacy_dev_list;
+	int (*firstopen)(struct drm_device *dev);
+	void (*preclose)(struct drm_device *dev, struct drm_file *file_priv);
+	int (*dma_ioctl)(struct drm_device *dev, void *data, struct drm_file
*file_priv);
+	int (*dma_quiescent)(struct drm_device *dev);
+	int (*context_dtor)(struct drm_device *dev, int context);
+	u32 (*get_vblank_counter)(struct drm_device *dev, unsigned int pipe);
+	int (*enable_vblank)(struct drm_device *dev, unsigned int pipe);
+	void (*disable_vblank)(struct drm_device *dev, unsigned int pipe);
+	int dev_priv_size;
 };
 
 /**
-- 
2.25.0

Thomas Zimmermann

2020-Feb-25 15:59 UTC

head link

[Nouveau] [PATCH 3/3] drm: Constify struct drm_driver in DRM core

Once used for creatign a DRM device, instances of struct drm_driver
are not modified. So all related variables in the DRM core can be
declared 'const'.

In addition to the core interfaces, the patch also changes tdfx
accordingly. The other non-KMS drivers set.num_ioctls outside of
their instance's initialization, so they cannot have the instance
declared const yet.

Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
---
 drivers/gpu/drm/drm_drv.c       |  8 ++++----
 drivers/gpu/drm/drm_pci.c       |  8 +++++---
 drivers/gpu/drm/tdfx/tdfx_drv.c |  2 +-
 include/drm/drm_device.h        |  2 +-
 include/drm/drm_drv.h           |  6 +++---
 include/drm/drm_legacy.h        | 10 ++++++----
 include/drm/drm_pci.h           |  4 ++--
 7 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 4875b76e7656..98d1f4169e6c 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -612,7 +612,7 @@ static void drm_fs_inode_free(struct inode *inode)
  * 0 on success, or error code on failure.
  */
 int drm_dev_init(struct drm_device *dev,
-		 struct drm_driver *driver,
+		 const struct drm_driver *driver,
 		 struct device *parent)
 {
 	int ret;
@@ -726,7 +726,7 @@ static void devm_drm_dev_init_release(void *data)
  */
 int devm_drm_dev_init(struct device *parent,
 		      struct drm_device *dev,
-		      struct drm_driver *driver)
+		      const struct drm_driver *driver)
 {
 	int ret;
 
@@ -804,7 +804,7 @@ EXPORT_SYMBOL(drm_dev_fini);
  * RETURNS:
  * Pointer to new DRM device, or ERR_PTR on failure.
  */
-struct drm_device *drm_dev_alloc(struct drm_driver *driver,
+struct drm_device *drm_dev_alloc(const struct drm_driver *driver,
 				 struct device *parent)
 {
 	struct drm_device *dev;
@@ -947,7 +947,7 @@ static void remove_compat_control_link(struct drm_device
*dev)
  */
 int drm_dev_register(struct drm_device *dev, unsigned long flags)
 {
-	struct drm_driver *driver = dev->driver;
+	const struct drm_driver *driver = dev->driver;
 	int ret;
 
 	if (drm_dev_needs_global_mutex(dev))
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 24ed0e92fd66..f6b7b30f7c5e 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -207,7 +207,7 @@ void drm_pci_agp_destroy(struct drm_device *dev)
  * Return: 0 on success or a negative error code on failure.
  */
 int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
-		    struct drm_driver *driver)
+		    const struct drm_driver *driver)
 {
 	struct drm_device *dev;
 	int ret;
@@ -264,7 +264,8 @@ EXPORT_SYMBOL(drm_get_pci_dev);
  *
  * Return: 0 on success or a negative error code on failure.
  */
-int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
+int drm_legacy_pci_init(const struct drm_driver *driver,
+			struct pci_driver *pdriver)
 {
 	struct pci_dev *pdev = NULL;
 	const struct pci_device_id *pid;
@@ -310,7 +311,8 @@ EXPORT_SYMBOL(drm_legacy_pci_init);
  * Unregister a DRM driver shadow-attached through drm_legacy_pci_init(). This
  * is deprecated and only used by dri1 drivers.
  */
-void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
+void drm_legacy_pci_exit(const struct drm_driver *driver,
+			 struct pci_driver *pdriver)
 {
 	struct drm_device *dev, *tmp;
 	DRM_DEBUG("\n");
diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c
index a8a6bce6afbe..637c9352c1af 100644
--- a/drivers/gpu/drm/tdfx/tdfx_drv.c
+++ b/drivers/gpu/drm/tdfx/tdfx_drv.c
@@ -59,7 +59,7 @@ static const struct file_operations tdfx_driver_fops = {
 static struct drm_legacy_state tdfx_legacy_state = {
 };
 
-static struct drm_driver driver = {
+static const struct drm_driver driver = {
 	.driver_features = DRIVER_LEGACY,
 	.fops = &tdfx_driver_fops,
 	.name = DRIVER_NAME,
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index bb60a949f416..6b2e2799d83c 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -68,7 +68,7 @@ struct drm_device {
 	struct device *dev;
 
 	/** @driver: DRM driver managing the device */
-	struct drm_driver *driver;
+	const struct drm_driver *driver;
 
 	/**
 	 * @dev_private:
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 15dd9a179216..887728d28226 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -604,14 +604,14 @@ struct drm_driver {
 };
 
 int drm_dev_init(struct drm_device *dev,
-		 struct drm_driver *driver,
+		 const struct drm_driver *driver,
 		 struct device *parent);
 int devm_drm_dev_init(struct device *parent,
 		      struct drm_device *dev,
-		      struct drm_driver *driver);
+		      const struct drm_driver *driver);
 void drm_dev_fini(struct drm_device *dev);
 
-struct drm_device *drm_dev_alloc(struct drm_driver *driver,
+struct drm_device *drm_dev_alloc(const struct drm_driver *driver,
 				 struct device *parent);
 int drm_dev_register(struct drm_device *dev, unsigned long flags);
 void drm_dev_unregister(struct drm_device *dev);
diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
index 7b5ee6a94a8c..abc77cf84752 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -211,18 +211,20 @@ void drm_legacy_idlelock_release(struct drm_lock_data
*lock);
 
 #ifdef CONFIG_PCI
 
-int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
-void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver
*pdriver);
+int drm_legacy_pci_init(const struct drm_driver *driver,
+			struct pci_driver *pdriver);
+void drm_legacy_pci_exit(const struct drm_driver *driver,
+			 struct pci_driver *pdriver);
 
 #else
 
-static inline int drm_legacy_pci_init(struct drm_driver *driver,
+static inline int drm_legacy_pci_init(const struct drm_driver *driver,
 				      struct pci_driver *pdriver)
 {
 	return -EINVAL;
 }
 
-static inline void drm_legacy_pci_exit(struct drm_driver *driver,
+static inline void drm_legacy_pci_exit(const struct drm_driver *driver,
 				       struct pci_driver *pdriver)
 {
 }
diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h
index 9031e217b506..8f98ae8384c2 100644
--- a/include/drm/drm_pci.h
+++ b/include/drm/drm_pci.h
@@ -47,7 +47,7 @@ void drm_pci_free(struct drm_device *dev, struct
drm_dma_handle * dmah);
 
 int drm_get_pci_dev(struct pci_dev *pdev,
 		    const struct pci_device_id *ent,
-		    struct drm_driver *driver);
+		    const struct drm_driver *driver);
 
 #else
 
@@ -64,7 +64,7 @@ static inline void drm_pci_free(struct drm_device *dev,
 
 static inline int drm_get_pci_dev(struct pci_dev *pdev,
 				  const struct pci_device_id *ent,
-				  struct drm_driver *driver)
+				  const struct drm_driver *driver)
 {
 	return -ENOSYS;
 }
-- 
2.25.0

Ilia Mirkin

2020-Feb-25 16:06 UTC

head link

[Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers

On Tue, Feb 25, 2020 at 10:59 AM Thomas Zimmermann <tzimmermann at
suse.de> wrote:>
> Non-KMS drivers store state in struct drm_driver. This bloats the
> structure for KMS drivers and prevents it from being declared with
> 'static const' qualifiers. Moving the non-KMS state into a separate
> data structure resolves this.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
>  drivers/gpu/drm/drm_drv.c             | 4 ++++
>  drivers/gpu/drm/i810/i810_drv.c       | 4 ++++
>  drivers/gpu/drm/mga/mga_drv.c         | 4 ++++
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++
>  drivers/gpu/drm/r128/r128_drv.c       | 4 ++++
>  drivers/gpu/drm/savage/savage_drv.c   | 4 ++++
>  drivers/gpu/drm/sis/sis_drv.c         | 4 ++++
>  drivers/gpu/drm/tdfx/tdfx_drv.c       | 4 ++++
>  drivers/gpu/drm/via/via_drv.c         | 4 ++++
>  include/drm/drm_drv.h                 | 3 +++
>  include/drm/drm_legacy.h              | 6 ++++++
>  11 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7b1a628d1f6e..4ba0df097602 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev,
>         if (WARN_ON(!parent))
>                 return -EINVAL;
>
> +       if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
> +           WARN_ON(!driver->legacy))
> +               return -EINVAL;
> +
>         kref_init(&dev->ref);
>         dev->dev = get_device(parent);
>         dev->driver = driver;
> diff --git a/drivers/gpu/drm/i810/i810_drv.c
b/drivers/gpu/drm/i810/i810_drv.c
> index 0e53a066d4db..55f17f00bae9 100644
> --- a/drivers/gpu/drm/i810/i810_drv.c
> +++ b/drivers/gpu/drm/i810/i810_drv.c
> @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = {
>         .llseek = noop_llseek,
>  };
>
> +static struct drm_legacy_state = i810_legacy_state {
Does this compile? I might have assumed this would need to be

static struct drm_legacy_state i810_legacy_state = {
> +};
> +
>  static struct drm_driver driver = {
>         .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA |
DRIVER_LEGACY,
>         .dev_priv_size = sizeof(drm_i810_buf_priv_t),
> @@ -71,6 +74,7 @@ static struct drm_driver driver = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +       .legacy = &i810_legacy_state,
>  };
>
>  static struct pci_driver i810_pci_driver = {
> diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
> index 71128e6f6ae9..4865982d949c 100644
> --- a/drivers/gpu/drm/mga/mga_drv.c
> +++ b/drivers/gpu/drm/mga/mga_drv.c
> @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = {
>         .llseek = noop_llseek,
>  };
>
> +static struct drm_legacy_state = mga_legacy_state {
> +};
> +
>  static struct drm_driver driver = {
>         .driver_features >             DRIVER_USE_AGP | DRIVER_PCI_DMA |
DRIVER_LEGACY |
> @@ -78,6 +81,7 @@ static struct drm_driver driver = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +       .legacy = &mga_legacy_state,
>  };
>
>  static struct pci_driver mga_pci_driver = {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 6b1629c14dd7..c88cf32e521c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1103,6 +1103,11 @@ nouveau_driver_fops = {
>         .llseek = noop_llseek,
>  };
>
> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> +static struct drm_legacy_state nouveau_legacy_state{
> +};
> +#endif
> +
>  static struct drm_driver
>  driver_stub = {
>         .driver_features > @@ -1150,6 +1155,9 @@ driver_stub = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> +       .legacy = &nouveau_legacy_state,
> +#endif
>  };
>
>  static struct pci_device_id
> diff --git a/drivers/gpu/drm/r128/r128_drv.c
b/drivers/gpu/drm/r128/r128_drv.c
> index b7a5f162ebae..ecece3a48d93 100644
> --- a/drivers/gpu/drm/r128/r128_drv.c
> +++ b/drivers/gpu/drm/r128/r128_drv.c
> @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = {
>         .llseek = noop_llseek,
>  };
>
> +static struct drm_legacy_state = r128_legacy_state {
> +};
> +
>  static struct drm_driver driver = {
>         .driver_features >             DRIVER_USE_AGP | DRIVER_PCI_DMA |
DRIVER_SG | DRIVER_LEGACY |
> @@ -81,6 +84,7 @@ static struct drm_driver driver = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +       .legacy = &r128_legacy_state,
>  };
>
>  int r128_driver_load(struct drm_device *dev, unsigned long flags)
> diff --git a/drivers/gpu/drm/savage/savage_drv.c
b/drivers/gpu/drm/savage/savage_drv.c
> index 799bd11adb9c..c0a7146fbde1 100644
> --- a/drivers/gpu/drm/savage/savage_drv.c
> +++ b/drivers/gpu/drm/savage/savage_drv.c
> @@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops =
{
>         .llseek = noop_llseek,
>  };
>
> +static struct drm_legacy_state = savage_legacy_state {
> +};
> +
>  static struct drm_driver driver = {
>         .driver_features >             DRIVER_USE_AGP | DRIVER_HAVE_DMA
| DRIVER_PCI_DMA | DRIVER_LEGACY,
> @@ -65,6 +68,7 @@ static struct drm_driver driver = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +       .legacy = &savage_legacy_state,
>  };
>
>  static struct pci_driver savage_pci_driver = {
> diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
> index 2c54b33abb54..1c794a82c8e6 100644
> --- a/drivers/gpu/drm/sis/sis_drv.c
> +++ b/drivers/gpu/drm/sis/sis_drv.c
> @@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev,
struct drm_file *file)
>         kfree(file_priv);
>  }
>
> +static struct drm_legacy_state = sis_legacy_state {
> +};
> +
>  static struct drm_driver driver = {
>         .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY,
>         .load = sis_driver_load,
> @@ -116,6 +119,7 @@ static struct drm_driver driver = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +       .legacy = &sis_legacy_state,
>  };
>
>  static struct pci_driver sis_pci_driver = {
> diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c
b/drivers/gpu/drm/tdfx/tdfx_drv.c
> index ab699bf0ac5c..17aa93d82d6b 100644
> --- a/drivers/gpu/drm/tdfx/tdfx_drv.c
> +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c
> @@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = {
>         .llseek = noop_llseek,
>  };
>
> +static struct drm_legacy_state = tdfx_legacy_state {
> +};
> +
>  static struct drm_driver driver = {
>         .driver_features = DRIVER_LEGACY,
>         .fops = &tdfx_driver_fops,
> @@ -65,6 +68,7 @@ static struct drm_driver driver = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +       .legacy = &tdfx_legacy_state,
>  };
>
>  static struct pci_driver tdfx_pci_driver = {
> diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c
> index 5da38082821f..22113d93ecdd 100644
> --- a/drivers/gpu/drm/via/via_drv.c
> +++ b/drivers/gpu/drm/via/via_drv.c
> @@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = {
>         .llseek = noop_llseek,
>  };
>
> +static struct drm_legacy_state = via_legacy_state {
> +};
> +
>  static struct drm_driver driver = {
>         .driver_features >             DRIVER_USE_AGP | DRIVER_HAVE_IRQ
| DRIVER_LEGACY,
> @@ -97,6 +100,7 @@ static struct drm_driver driver = {
>         .major = DRIVER_MAJOR,
>         .minor = DRIVER_MINOR,
>         .patchlevel = DRIVER_PATCHLEVEL,
> +       .legacy = &via_legacy_state,
>  };
>
>  static struct pci_driver via_pci_driver = {
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 97109df5beac..94078af0cc28 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -40,6 +40,7 @@ struct dma_buf_attachment;
>  struct drm_display_mode;
>  struct drm_mode_create_dumb;
>  struct drm_printer;
> +struct drm_legacy_state;
>
>  /**
>   * enum drm_driver_feature - feature flags
> @@ -599,6 +600,8 @@ struct drm_driver {
>         const struct file_operations *fops;
>
>         /* Everything below here is for legacy driver, never use! */
> +       struct drm_legacy_state *legacy;
> +
>         /* private: */
>
>         /* List of devices hanging off this driver with stealth attach. */
> diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
> index dcef3598f49e..c98a29e1c2b3 100644
> --- a/include/drm/drm_legacy.h
> +++ b/include/drm/drm_legacy.h
> @@ -49,6 +49,12 @@ struct pci_driver;
>   * you're doing it terribly wrong.
>   */
>
> +/**
> + * State for non-KMS drivers.
> + */
> +struct drm_legacy_state {
> +};
> +
>  /**
>   * DMA buffer.
>   */
> --
> 2.25.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Emil Velikov

2020-Feb-25 17:37 UTC

head link

[Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers

Hi Thomas,

On Tuesday, 25 February 2020, Thomas Zimmermann <tzimmermann at suse.de>
wrote:
> Non-KMS drivers store state in struct drm_driver. This bloats the
> structure for KMS drivers and prevents it from being declared with
> 'static const' qualifiers. Moving the non-KMS state into a separate
> data structure resolves this.
>
>Have you considered subclassing the const drm_driver for legacy drivers.
This way the legacy (pointer) data does not need to be exposed to
non-legacy drivers.

Not to mention that const data (drm_driver), which has (const) pointer to
mutable data (legacy bits) is a trap IMHO

Emil
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20200225/d84d213c/attachment.htm>

Daniel Vetter

2020-Feb-25 17:44 UTC

head link

[Nouveau] [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver

On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann
wrote:> This patchset moves legacy, non-KMS driver state from struct drm_driver
> into struct drm_legacy_state. Only non-KMS drivers provide an instance
> of the latter structure. One special case is nouveau, which supports
> legacy interfaces. It also provides an instance of the legacy state if
> the legacy interfaces have been enabled (i.e., defines the config option
> CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> 
> I reviewed all call sites of legacy state and functions to verify that
> DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that
> DRIVER_MODESET is not set.
> 
> With the mutable KMS state removed, instances of struct drm_driver can
> be declared as constant. The patchset modifies the DRM core accordingly.
> Individual drivers can follow later on.
Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d,
panfrost, ...). non-KMS != legacy, which is what you're talking about
here.

Other thing, and it's a bit raining on your parade: I don't see the
point.
Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts
sounds like a reasonable idea. But this is a lot of churn for drivers
which are all pretty much dead, and just waiting for their eventual
removal. And from a compile-testing pov of making sure modern drivers
don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY
should be plenty enough.

And from a "make stuff const" I think Laurent's much more minimal
series
also gets us there for all the drivers we care about.
-Daniel
> 
> Thomas Zimmermann (3):
>   drm: Add separate state structure for legacy, non-KMS drivers
>   drm: Move non-kms driver state into struct drm_legacy_state
>   drm: Constify struct drm_driver in DRM core
> 
>  drivers/gpu/drm/drm_bufs.c            | 10 +++++-----
>  drivers/gpu/drm/drm_context.c         |  9 +++++----
>  drivers/gpu/drm/drm_drv.c             | 12 ++++++++----
>  drivers/gpu/drm/drm_file.c            |  4 ++--
>  drivers/gpu/drm/drm_legacy_misc.c     |  6 +++---
>  drivers/gpu/drm/drm_lock.c            |  7 ++++---
>  drivers/gpu/drm/drm_pci.c             | 16 ++++++++++------
>  drivers/gpu/drm/drm_vblank.c          | 11 ++++++-----
>  drivers/gpu/drm/i810/i810_drv.c       | 10 +++++++---
>  drivers/gpu/drm/mga/mga_drv.c         | 16 ++++++++++------
>  drivers/gpu/drm/nouveau/nouveau_drm.c |  8 ++++++++
>  drivers/gpu/drm/r128/r128_drv.c       | 16 ++++++++++------
>  drivers/gpu/drm/savage/savage_drv.c   | 12 ++++++++----
>  drivers/gpu/drm/sis/sis_drv.c         |  8 ++++++--
>  drivers/gpu/drm/tdfx/tdfx_drv.c       |  6 +++++-
>  drivers/gpu/drm/via/via_drv.c         | 16 ++++++++++------
>  include/drm/drm_device.h              |  2 +-
>  include/drm/drm_drv.h                 | 21 +++++----------------
>  include/drm/drm_legacy.h              | 27 +++++++++++++++++++++++----
>  include/drm/drm_pci.h                 |  4 ++--
>  20 files changed, 138 insertions(+), 83 deletions(-)
> 
> --
> 2.25.0
> 
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Thomas Zimmermann

2020-Feb-26 05:39 UTC

head link

[Nouveau] [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver

Hi

Am 25.02.20 um 18:44 schrieb Daniel Vetter:> On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann wrote:
>> This patchset moves legacy, non-KMS driver state from struct drm_driver
>> into struct drm_legacy_state. Only non-KMS drivers provide an instance
>> of the latter structure. One special case is nouveau, which supports
>> legacy interfaces. It also provides an instance of the legacy state if
>> the legacy interfaces have been enabled (i.e., defines the config
option
>> CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
>>
>> I reviewed all call sites of legacy state and functions to verify that
>> DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or
that
>> DRIVER_MODESET is not set.
>>
>> With the mutable KMS state removed, instances of struct drm_driver can
>> be declared as constant. The patchset modifies the DRM core
accordingly.
>> Individual drivers can follow later on.
> 
> Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d,
> panfrost, ...). non-KMS != legacy, which is what you're talking about
> here.
OK, probably needs to be more precise.
> 
> Other thing, and it's a bit raining on your parade: I don't see the
point.
> Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts
> sounds like a reasonable idea. But this is a lot of churn for drivers
> which are all pretty much dead, and just waiting for their eventual
> removal. And from a compile-testing pov of making sure modern drivers
> don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY
> should be plenty enough.
It's not about these old drivers, it's about having constant driver
structures and maybe saving a few bytes in the modern drivers. If these
old drivers hold back improvements to the drivers we care about, I don't
see we they are not to be touched.

This cannot be solved with CONFIG_DRM_LEGACY, unless you want to wrap
any reference in core code to legacy data. I tried at first and it
turned out to be an unreadable mess.

And from a complexity POV the patchset is trivial. It adds a data
structure to each old driver and moves a few initializers around. The
worst thing that can happen is that code tried to dereference the legacy
pointer when it's not set. This bug will happen with modern drivers, so
we should see it easily.

Best regards
Thomas
> 
> And from a "make stuff const" I think Laurent's much more
minimal series
> also gets us there for all the drivers we care about.
> -Daniel
> 
>>
>> Thomas Zimmermann (3):
>>   drm: Add separate state structure for legacy, non-KMS drivers
>>   drm: Move non-kms driver state into struct drm_legacy_state
>>   drm: Constify struct drm_driver in DRM core
>>
>>  drivers/gpu/drm/drm_bufs.c            | 10 +++++-----
>>  drivers/gpu/drm/drm_context.c         |  9 +++++----
>>  drivers/gpu/drm/drm_drv.c             | 12 ++++++++----
>>  drivers/gpu/drm/drm_file.c            |  4 ++--
>>  drivers/gpu/drm/drm_legacy_misc.c     |  6 +++---
>>  drivers/gpu/drm/drm_lock.c            |  7 ++++---
>>  drivers/gpu/drm/drm_pci.c             | 16 ++++++++++------
>>  drivers/gpu/drm/drm_vblank.c          | 11 ++++++-----
>>  drivers/gpu/drm/i810/i810_drv.c       | 10 +++++++---
>>  drivers/gpu/drm/mga/mga_drv.c         | 16 ++++++++++------
>>  drivers/gpu/drm/nouveau/nouveau_drm.c |  8 ++++++++
>>  drivers/gpu/drm/r128/r128_drv.c       | 16 ++++++++++------
>>  drivers/gpu/drm/savage/savage_drv.c   | 12 ++++++++----
>>  drivers/gpu/drm/sis/sis_drv.c         |  8 ++++++--
>>  drivers/gpu/drm/tdfx/tdfx_drv.c       |  6 +++++-
>>  drivers/gpu/drm/via/via_drv.c         | 16 ++++++++++------
>>  include/drm/drm_device.h              |  2 +-
>>  include/drm/drm_drv.h                 | 21 +++++----------------
>>  include/drm/drm_legacy.h              | 27 +++++++++++++++++++++++----
>>  include/drm/drm_pci.h                 |  4 ++--
>>  20 files changed, 138 insertions(+), 83 deletions(-)
>>
>> --
>> 2.25.0
>>
> 
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 N?rnberg, Germany
(HRB 36809, AG N?rnberg)
Gesch?ftsf?hrer: Felix Imend?rffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20200226/d1010f1f/attachment.sig>

Possibly Parallel Threads

Search for more maybe matching threads

Nouveau - Feb 2020 - [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers

[Nouveau] [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver

[Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers

[Nouveau] [PATCH 2/3] drm: Move non-kms driver state into struct drm_legacy_state

[Nouveau] [PATCH 3/3] drm: Constify struct drm_driver in DRM core

[Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers

[Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers

[Nouveau] [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver

[Nouveau] [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver

Possibly Parallel Threads