Commit a3257256 authored by Daniel Vetter's avatar Daniel Vetter

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: 's avatarEmil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: 's avatarDaniel Vetter <daniel.vetter@intel.com>
parent 46bfdf9a
......@@ -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
============
......
......@@ -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,
......
......@@ -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);
......
......@@ -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
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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();
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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;
......
......@@ -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,
......
......@@ -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 {
......
......@@ -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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment