Hi Daniel
On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> There's really no need for the drm core to keep a list of all
> devices of a given driver - the linux device model keeps perfect
> track of this already for us.
>
> The exception is old legacy ums drivers using pci shadow attaching.
> So rename the lists to make the use case clearer and rip out everything
> else.
>
> v2: Rebase on top of David Herrmann's drm device register changes.
> Also drop the bogus dev_set_drvdata for platform drivers that somehow
> crept into the original version - drivers really should be in full
> control of that field.
You didn't really change any dev_set_drvdata, did you? And I guess you
mean pci_set_drvdata()? I had to keep it in place in drm_pci.c as it
has been there before my device-registration changes. However, with
your series you added the pci_set_drvdata() everywhere yourself, so
yes, please remove it.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_pci.c | 12 ++++++++++--
> drivers/gpu/drm/drm_platform.c | 1 -
> drivers/gpu/drm/drm_stub.c | 4 ----
> drivers/gpu/drm/drm_usb.c | 1 -
> include/drm/drmP.h | 6 +++---
> 5 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index f00d7a9671ea..ec78fc805ed0 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -346,6 +346,11 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
> driver->name, driver->major, driver->minor, driver->patchlevel,
> driver->date, pci_name(pdev), dev->primary->index);
>
> + /* 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_MODESET))
> + list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list);
> +
> return 0;
>
> err_pci:
> @@ -375,7 +380,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
>
> DRM_DEBUG("\n");
>
> - INIT_LIST_HEAD(&driver->device_list);
> driver->kdriver.pci = pdriver;
> driver->bus = &drm_pci_bus;
>
> @@ -384,6 +388,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
>
> /* If not using KMS, fall back to stealth mode manual scanning. */
> for (i = 0; pdriver->id_table[i].vendor != 0; i++) {
> + INIT_LIST_HEAD(&driver->legacy_dev_list);
Why doing this inside of the loop? If the pci_get_dev() below
succeeds, you clear the list again?
Thanks
David
> pid = &pdriver->id_table[i];
>
> /* Loop around setting up a DRM device for each PCI device
> @@ -465,8 +470,11 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
> if (driver->driver_features & DRIVER_MODESET) {
> pci_unregister_driver(pdriver);
> } else {
> - list_for_each_entry_safe(dev, tmp, &driver->device_list, driver_item)
> + list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
> + legacy_dev_list) {
> drm_put_dev(dev);
> + list_del(&dev->legacy_dev_list);
> + }
> }
> DRM_INFO("Module unloaded\n");
> }
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index 56a48033eced..21fc82006b78 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -147,7 +147,6 @@ int drm_platform_init(struct drm_driver *driver, struct platform_device *platfor
>
> driver->kdriver.platform_device = platform_device;
> driver->bus = &drm_platform_bus;
> - INIT_LIST_HEAD(&driver->device_list);
> return drm_get_platform_dev(platform_device, driver);
> }
> EXPORT_SYMBOL(drm_platform_init);
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 26055abf94ee..047d1d1fb0e1 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -538,8 +538,6 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> goto err_unload;
> }
>
> - list_add_tail(&dev->driver_item, &dev->driver->device_list);
> -
> ret = 0;
> goto out_unlock;
>
> @@ -593,7 +591,5 @@ void drm_dev_unregister(struct drm_device *dev)
> if (dev->render)
> drm_put_minor(&dev->render);
> drm_put_minor(&dev->primary);
> -
> - list_del(&dev->driver_item);
> }
> EXPORT_SYMBOL(drm_dev_unregister);
> diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
> index b179b70e7853..21ae8d96880b 100644
> --- a/drivers/gpu/drm/drm_usb.c
> +++ b/drivers/gpu/drm/drm_usb.c
> @@ -63,7 +63,6 @@ int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver)
> int res;
> DRM_DEBUG("\n");
>
> - INIT_LIST_HEAD(&driver->device_list);
> driver->kdriver.usb = udriver;
> driver->bus = &drm_usb_bus;
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 54d6c2d36a5b..e3dfbae963a5 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -988,8 +988,8 @@ struct drm_driver {
> } kdriver;
> struct drm_bus *bus;
>
> - /* List of devices hanging off this driver */
> - struct list_head device_list;
> + /* List of devices hanging off this driver with stealth attach. */
> + struct list_head legacy_dev_list;
> };
>
> #define DRM_MINOR_UNASSIGNED 0
> @@ -1099,7 +1099,7 @@ struct drm_vblank_crtc {
> * may contain multiple heads.
> */
> struct drm_device {
> - struct list_head driver_item; /**< list of devices per driver */
> + struct list_head legacy_dev_list;/**< list of devices per driver for stealth attach cleanup */
> char *devname; /**< For /proc/interrupts */
> int if_version; /**< Highest interface version set */
>
> --
> 1.8.4.rc3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
