On Tue, Dec 15, 2020 at 10:31:24PM +0200, Laurent Pinchart wrote:
The drm_driver structure contains a single field (legacy_dev_list) that is modified by the DRM core, used to store a linked list of legacy DRM devices associated with the driver. In order to make the structure const, move the field out to a global variable. This requires locking access to the global where the local field didn't require serialization, but this only affects legacy drivers, and isn't in any hot path.
While at it, compile-out the legacy_dev_list field when DRM_LEGACY isn't defined.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Hah, I didn't notice my review here, read it again, still looks good :-) -Daniel
Reviewed-by: Emil Velikov emil.velikov@collabora.com
Changes since v1:
- Move the legacy_dev_list to the end of struct drm_device, in the existing DRM_LEGACY section
- Drop the kerneldoc comment for legacy_dev_list
drivers/gpu/drm/drm_pci.c | 25 +++++++++++++++++-------- include/drm/drm_device.h | 10 +++------- include/drm/drm_drv.h | 2 -- 3 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 6dba4b8ce4fe..dfb138aaccba 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -24,6 +24,8 @@
#include <linux/dma-mapping.h> #include <linux/export.h> +#include <linux/list.h> +#include <linux/mutex.h> #include <linux/pci.h> #include <linux/slab.h>
@@ -36,6 +38,9 @@ #include "drm_legacy.h"
#ifdef CONFIG_DRM_LEGACY +/* List of devices hanging off drivers with stealth attach. */ +static LIST_HEAD(legacy_dev_list); +static DEFINE_MUTEX(legacy_dev_list_lock);
/**
- drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
@@ -225,10 +230,11 @@ static int drm_get_pci_dev(struct pci_dev *pdev, if (ret) goto err_agp;
- /* 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);
if (drm_core_check_feature(dev, DRIVER_LEGACY)) {
mutex_lock(&legacy_dev_list_lock);list_add_tail(&dev->legacy_dev_list, &legacy_dev_list);mutex_unlock(&legacy_dev_list_lock);}
return 0;
@@ -261,7 +267,6 @@ 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); for (i = 0; pdriver->id_table[i].vendor != 0; i++) { pid = &pdriver->id_table[i];
@@ -304,11 +309,15 @@ 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,
mutex_lock(&legacy_dev_list_lock);list_for_each_entry_safe(dev, tmp, &legacy_dev_list, legacy_dev_list) {
list_del(&dev->legacy_dev_list);drm_put_dev(dev);
if (dev->driver == driver) {list_del(&dev->legacy_dev_list);drm_put_dev(dev);}mutex_unlock(&legacy_dev_list_lock);} diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 283a93ce4617..bd5abe7cd48f 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -51,13 +51,6 @@ enum switch_power_state {
- may contain multiple heads.
*/ struct drm_device {
- /**
* @legacy_dev_list:** List of devices per driver for stealth attach cleanup*/- struct list_head legacy_dev_list;
- /** @if_version: Highest interface version set */ int if_version;
@@ -336,6 +329,9 @@ struct drm_device { /* Everything below here is for legacy driver, never use! */ /* private: */ #if IS_ENABLED(CONFIG_DRM_LEGACY)
- /* List of devices per driver for stealth attach cleanup */
- struct list_head legacy_dev_list;
- /* Context handle management - linked list of context handles */ struct list_head ctxlist;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 02787319246a..827838e0a97e 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -499,8 +499,6 @@ struct drm_driver { /* Everything below here is for legacy driver, never use! */ /* 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);
-- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel