drm: Lobotomize set_busid nonsense for !pci drivers

We already have a fallback in place to fill out the unique from
dev->unique, which is set to something reasonable in drm_dev_alloc.

Which means we only need to have a special set_busid for pci devices,
to be able to care the backwards compat code for drm 1.1 around, which
libdrm still needs.

While developing and testing this patch things blew up in really
interesting ways, and the code is rather confusing in naming things
between the kernel code, ioctl #defines and libdrm. For the next brave
dragon slayer, document all this madness properly in the userspace
interface section of gpu.tmpl.

v2: Make drm_dev_set_unique static and update kerneldoc.

v3: Entire rewrite, plus document what's going on for posterity in the
gpu docbook uapi section.

v4: Drop accidental amdgpu hunk (Emil).

v5: Drop accidental omapdrm vblank counter change (Emil).

v6: Rebase on top of the sphinx conversion.

Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Tested-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> (virt_gpu)
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 6da1e77e55fa1d7e0a5a87dc56724801bf19a269..a1c6f0103bc431545c56a0dc55ddddcc4bf020f9 100644 (file)
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -14,6 +14,12 @@ management, and output management.
 Cover generic ioctls and sysfs layout here. We only need high-level
 info, since man pages should cover the rest.
 
+libdrm Device Lookup
+====================
+
+.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c
+   :doc: getunique and setversion story
+
 Render nodes
 ============
 

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index cb21c0b6374a68d5640433246f5311528c749638..f5ebdd68144579c12dea0c8342ce8dd24e89e2e3 100644 (file)
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -189,7 +189,6 @@ static struct drm_driver armada_drm_driver = {
        .load                   = armada_drm_load,
        .lastclose              = armada_drm_lastclose,
        .unload                 = armada_drm_unload,
-       .set_busid              = drm_platform_set_busid,
        .get_vblank_counter     = drm_vblank_no_hw_counter,
        .enable_vblank          = armada_drm_enable_vblank,
        .disable_vblank         = armada_drm_disable_vblank,

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 1fa7619face3a7c91777042fd12b2ec714392b09..ffd6a27952300de56cb69b0ea22362f3916fdc1c 100644 (file)
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -37,6 +37,64 @@
 #include <linux/pci.h>
 #include <linux/export.h>
 
+/**
+ * DOC: getunique and setversion story
+ *
+ * BEWARE THE DRAGONS! MIND THE TRAPDOORS!
+ *
+ * In an attempt to warn anyone else who's trying to figure out what's going
+ * on here, I'll try to summarize the story. First things first, let's clear up
+ * the names, because the kernel internals, libdrm and the ioctls are all named
+ * differently:
+ *
+ *  - GET_UNIQUE ioctl, implemented by drm_getunique is wrapped up in libdrm
+ *    through the drmGetBusid function.
+ *  - The libdrm drmSetBusid function is backed by the SET_UNIQUE ioctl. All
+ *    that code is nerved in the kernel with drm_invalid_op().
+ *  - The internal set_busid kernel functions and driver callbacks are
+ *    exclusively use by the SET_VERSION ioctl, because only drm 1.0 (which is
+ *    nerved) allowed userspace to set the busid through the above ioctl.
+ *  - Other ioctls and functions involved are named consistently.
+ *
+ * For anyone wondering what's the difference between drm 1.1 and 1.4: Correctly
+ * handling pci domains in the busid on ppc. Doing this correctly was only
+ * implemented in libdrm in 2010, hence can't be nerved yet. No one knows what's
+ * special with drm 1.2 and 1.3.
+ *
+ * Now the actual horror story of how device lookup in drm works. At large,
+ * there's 2 different ways, either by busid, or by device driver name.
+ *
+ * Opening by busid is fairly simple:
+ *
+ * 1. First call SET_VERSION to make sure pci domains are handled properly. As a
+ *    side-effect this fills out the unique name in the master structure.
+ * 2. Call GET_UNIQUE to read out the unique name from the master structure,
+ *    which matches the busid thanks to step 1. If it doesn't, proceed to try
+ *    the next device node.
+ *
+ * Opening by name is slightly different:
+ *
+ * 1. Directly call VERSION to get the version and to match against the driver
+ *    name returned by that ioctl. Note that SET_VERSION is not called, which
+ *    means the the unique name for the master node just opening is _not_ filled
+ *    out. This despite that with current drm device nodes are always bound to
+ *    one device, and can't be runtime assigned like with drm 1.0.
+ * 2. Match driver name. If it mismatches, proceed to the next device node.
+ * 3. Call GET_UNIQUE, and check whether the unique name has length zero (by
+ *    checking that the first byte in the string is 0). If that's not the case
+ *    libdrm skips and proceeds to the next device node. Probably this is just
+ *    copypasta from drm 1.0 times where a set unique name meant that the driver
+ *    was in use already, but that's just conjecture.
+ *
+ * Long story short: To keep the open by name logic working, GET_UNIQUE must
+ * _not_ return a unique string when SET_VERSION hasn't been called yet,
+ * otherwise libdrm breaks. Even when that unique string can't ever change, and
+ * is totally irrelevant for actually opening the device because runtime
+ * assignable device instances were only support in drm 1.0, which is long dead.
+ * But the libdrm code in drmOpenByName somehow survived, hence this can't be
+ * broken.
+ */
+
 static int drm_version(struct drm_device *dev, void *data,
                       struct drm_file *file_priv);
 

diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index 644169e1a0296fe6fa07c91cc3c8520bf9588c4a..2c819ef90090dee3b8c154d5065fd77041e66f6b 100644 (file)
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -68,24 +68,6 @@ err_free:
        return ret;
 }
 
-int drm_platform_set_busid(struct drm_device *dev, struct drm_master *master)
-{
-       int id;
-
-       id = dev->platformdev->id;
-       if (id < 0)
-               id = 0;
-
-       master->unique = kasprintf(GFP_KERNEL, "platform:%s:%02d",
-                                               dev->platformdev->name, id);
-       if (!master->unique)
-               return -ENOMEM;
-
-       master->unique_len = strlen(master->unique);
-       return 0;
-}
-EXPORT_SYMBOL(drm_platform_set_busid);
-
 /**
  * drm_platform_init - Register a platform device with the DRM subsystem
  * @driver: DRM device driver

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 3d4f56df8359e6ae81a77c574e9e9a42105b6963..340d390306d893d5a61898491b1778b0a647ac5b 100644 (file)
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -496,7 +496,6 @@ static struct drm_driver etnaviv_drm_driver = {
                                DRIVER_RENDER,
        .open               = etnaviv_open,
        .preclose           = etnaviv_preclose,
-       .set_busid          = drm_platform_set_busid,
        .gem_free_object_unlocked = etnaviv_gem_free_object,
        .gem_vm_ops         = &vm_ops,
        .prime_handle_to_fd = drm_gem_prime_handle_to_fd,

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 4a679fb9bb02126db687c89f902150b816b05916..13d28d4229e2b2263398f9970d26a5ef33223102 100644 (file)
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -407,7 +407,6 @@ static struct drm_driver exynos_drm_driver = {
        .preclose               = exynos_drm_preclose,
        .lastclose              = exynos_drm_lastclose,
        .postclose              = exynos_drm_postclose,
-       .set_busid              = drm_platform_set_busid,
        .get_vblank_counter     = drm_vblank_no_hw_counter,
        .enable_vblank          = exynos_drm_crtc_enable_vblank,
        .disable_vblank         = exynos_drm_crtc_disable_vblank,

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index ef2c32ec16168e51f4cdfc0c152a5e114bf837c6..1edd9bc802947f00cc1868e1e82a13f463c5b537 100644 (file)
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -171,7 +171,6 @@ static struct drm_driver kirin_drm_driver = {
        .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
                                  DRIVER_ATOMIC | DRIVER_HAVE_IRQ,
        .fops                   = &kirin_drm_fops,
-       .set_busid              = drm_platform_set_busid,
 
        .gem_free_object_unlocked = drm_gem_cma_free_object,
        .gem_vm_ops             = &drm_gem_cma_vm_ops,

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 82656654fb2181e2cbbb38e2b091727e9d97451e..7746418a4c08597a219e611500bb87200bd32135 100644 (file)
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -407,7 +407,6 @@ static struct drm_driver imx_drm_driver = {
        .load                   = imx_drm_driver_load,
        .unload                 = imx_drm_driver_unload,
        .lastclose              = imx_drm_driver_lastclose,
-       .set_busid              = drm_platform_set_busid,
        .gem_free_object_unlocked = drm_gem_cma_free_object,
        .gem_vm_ops             = &drm_gem_cma_vm_ops,
        .dumb_create            = drm_gem_cma_dumb_create,

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 568fcc328f27979ec933dc8e05dbc26d094a2900..a02dc2b27739cc85bd5a03f222a5e7ced1e6d65d 100644 (file)
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -722,7 +722,6 @@ static struct drm_driver msm_driver = {
        .open               = msm_open,
        .preclose           = msm_preclose,
        .lastclose          = msm_lastclose,
-       .set_busid          = drm_platform_set_busid,
        .irq_handler        = msm_irq,
        .irq_preinstall     = msm_irq_preinstall,
        .irq_postinstall    = msm_irq_postinstall,

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index c00ff6e786a350d9193f14d927bf16e8a106c810..295e7621cc686ff1a242203fcea83a350ebfcd6d 100644 (file)
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1070,7 +1070,6 @@ nouveau_drm_init(void)
        driver_pci = driver_stub;
        driver_pci.set_busid = drm_pci_set_busid;
        driver_platform = driver_stub;
-       driver_platform.set_busid = drm_platform_set_busid;
 
        nouveau_display_options();
 

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 6b97011154bf1a7cfb44798f3549f0d8871e2c9a..26c6134eb744075db77edaa279aa25cb47a56dc3 100644 (file)
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -801,7 +801,6 @@ static struct drm_driver omap_drm_driver = {
        .unload = dev_unload,
        .open = dev_open,
        .lastclose = dev_lastclose,
-       .set_busid = drm_platform_set_busid,
        .get_vblank_counter = drm_vblank_no_hw_counter,
        .enable_vblank = omap_irq_enable_vblank,
        .disable_vblank = omap_irq_disable_vblank,

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index ee79264b5b6a269f55f019c1a11ae7acf4be63e7..f0492603ea885d0767a375d782913c9da391cb62 100644 (file)
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -259,7 +259,6 @@ static struct drm_driver shmob_drm_driver = {
                                | DRIVER_PRIME,
        .load                   = shmob_drm_load,
        .unload                 = shmob_drm_unload,
-       .set_busid              = drm_platform_set_busid,
        .irq_handler            = shmob_drm_irq,
        .get_vblank_counter     = drm_vblank_no_hw_counter,
        .enable_vblank          = shmob_drm_enable_vblank,

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 308e197908fc31289091ea48c4ff86e59752ee72..d27809372d54491f19534ae43fc5745c74ae1131 100644 (file)
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -541,7 +541,6 @@ static struct drm_driver tilcdc_driver = {
        .load               = tilcdc_load,
        .unload             = tilcdc_unload,
        .lastclose          = tilcdc_lastclose,
-       .set_busid          = drm_platform_set_busid,
        .irq_handler        = tilcdc_irq,
        .irq_preinstall     = tilcdc_irq_preinstall,
        .irq_postinstall    = tilcdc_irq_postinstall,

diff --git a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
index 88a39165edd50705d561ca8c5d23df2ac628f9a1..7f0e93f87a554ee8e93a81ee2f4d80f9745ad3bc 100644 (file)
--- a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
@@ -27,16 +27,6 @@
 
 #include "virtgpu_drv.h"
 
-int drm_virtio_set_busid(struct drm_device *dev, struct drm_master *master)
-{
-       struct pci_dev *pdev = dev->pdev;
-
-       if (pdev) {
-               return drm_pci_set_busid(dev, master);
-       }
-       return 0;
-}
-
 static void virtio_pci_kick_out_firmware_fb(struct pci_dev *pci_dev)
 {
        struct apertures_struct *ap;

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 5820b7020ae5ab07dad986ddf5fb91ac17a0ca43..c13f70cfc46130946cd41e1846101160d426439b 100644 (file)
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -117,7 +117,6 @@ static const struct file_operations virtio_gpu_driver_fops = {
 
 static struct drm_driver driver = {
        .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_RENDER | DRIVER_ATOMIC,
-       .set_busid = drm_virtio_set_busid,
        .load = virtio_gpu_driver_load,
        .unload = virtio_gpu_driver_unload,
        .open = virtio_gpu_driver_open,

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index acf556a35cb266835c9ecf430fe333ae22cab5e9..b18ef3111f0c40c054c33e66877927a6fbbd4d5b 100644 (file)
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -49,7 +49,6 @@
 #define DRIVER_PATCHLEVEL 1
 
 /* virtgpu_drm_bus.c */
-int drm_virtio_set_busid(struct drm_device *dev, struct drm_master *master);
 int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev);
 
 struct virtio_gpu_object {

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fd4e4391aee50a41261baaae0920e39a0858e3fb..babec2e80760dc3bf88cd1d1aa31d20c2f804a17 100644 (file)
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1132,7 +1132,6 @@ extern int drm_pcie_get_max_link_width(struct drm_device *dev, u32 *mlw);
 
 /* platform section */
 extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
-extern int drm_platform_set_busid(struct drm_device *d, struct drm_master *m);
 
 /* returns true if currently okay to sleep */
 static __inline__ bool drm_can_sleep(void)