diff mbox

drm: Lobotomize set_busid nonsense for !pci drivers

Message ID 1466510913-17958-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter June 21, 2016, 12:08 p.m. UTC
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).

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>
---
 Documentation/DocBook/gpu.tmpl                  |  4 ++
 drivers/gpu/drm/armada/armada_drv.c             |  1 -
 drivers/gpu/drm/drm_ioctl.c                     | 58 +++++++++++++++++++++++++
 drivers/gpu/drm/drm_platform.c                  | 18 --------
 drivers/gpu/drm/etnaviv/etnaviv_drv.c           |  1 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c         |  1 -
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  1 -
 drivers/gpu/drm/imx/imx-drm-core.c              |  1 -
 drivers/gpu/drm/msm/msm_drv.c                   |  1 -
 drivers/gpu/drm/nouveau/nouveau_drm.c           |  1 -
 drivers/gpu/drm/omapdrm/omap_drv.c              |  1 -
 drivers/gpu/drm/shmobile/shmob_drm_drv.c        |  1 -
 drivers/gpu/drm/tilcdc/tilcdc_drv.c             |  1 -
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c        | 10 -----
 drivers/gpu/drm/virtio/virtgpu_drv.c            |  1 -
 drivers/gpu/drm/virtio/virtgpu_drv.h            |  1 -
 include/drm/drmP.h                              |  1 -
 17 files changed, 62 insertions(+), 41 deletions(-)

Comments

Laszlo Ersek Sept. 30, 2016, 3:09 a.m. UTC | #1
Hello Daniel,

On 06/21/16 14:08, daniel.vetter at ffwll.ch (Daniel Vetter) wrote:
> 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).
> 
> Cc: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> Cc: Emil Velikov <emil.l.velikov at gmail.com>
> Tested-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk> (virt_gpu)
> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  Documentation/DocBook/gpu.tmpl                  |  4 ++
>  drivers/gpu/drm/armada/armada_drv.c             |  1 -
>  drivers/gpu/drm/drm_ioctl.c                     | 58 +++++++++++++++++++++++++
>  drivers/gpu/drm/drm_platform.c                  | 18 --------
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c           |  1 -
>  drivers/gpu/drm/exynos/exynos_drm_drv.c         |  1 -
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  1 -
>  drivers/gpu/drm/imx/imx-drm-core.c              |  1 -
>  drivers/gpu/drm/msm/msm_drv.c                   |  1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c           |  1 -
>  drivers/gpu/drm/omapdrm/omap_drv.c              |  1 -
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c        |  1 -
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             |  1 -
>  drivers/gpu/drm/virtio/virtgpu_drm_bus.c        | 10 -----
>  drivers/gpu/drm/virtio/virtgpu_drv.c            |  1 -
>  drivers/gpu/drm/virtio/virtgpu_drv.h            |  1 -
>  include/drm/drmP.h                              |  1 -
>  17 files changed, 62 insertions(+), 41 deletions(-)

This patch (commit a325725633c2) regresses X.org on QEMU's virtio-vga
device. Please see

  https://bugzilla.redhat.com/show_bug.cgi?id=1366842

complete with a bisection log under

  drivers/gpu/drm/virtio/

(comment 20).

Copying Thorsten so he can include this report in his next v4.8-rc8
regression report, if he chooses so. (Commit a325725633c2 is part of
v4.8-rc1, but we only managed to identify it now.) The last such report
I know of is archived e.g. at
<http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1239220.html>.

Reported-by: Joachim Frieben <jfrieben@hotmail.com>

Thanks
Laszlo
Hans de Goede Sept. 30, 2016, 8:28 a.m. UTC | #2
Hi,

On 30-09-16 05:09, Laszlo Ersek wrote:
> Hello Daniel,
>
> On 06/21/16 14:08, daniel.vetter at ffwll.ch (Daniel Vetter) wrote:
>> 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).
>>
>> Cc: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>> Cc: Emil Velikov <emil.l.velikov at gmail.com>
>> Tested-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk> (virt_gpu)
>> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>> ---
>>  Documentation/DocBook/gpu.tmpl                  |  4 ++
>>  drivers/gpu/drm/armada/armada_drv.c             |  1 -
>>  drivers/gpu/drm/drm_ioctl.c                     | 58 +++++++++++++++++++++++++
>>  drivers/gpu/drm/drm_platform.c                  | 18 --------
>>  drivers/gpu/drm/etnaviv/etnaviv_drv.c           |  1 -
>>  drivers/gpu/drm/exynos/exynos_drm_drv.c         |  1 -
>>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  1 -
>>  drivers/gpu/drm/imx/imx-drm-core.c              |  1 -
>>  drivers/gpu/drm/msm/msm_drv.c                   |  1 -
>>  drivers/gpu/drm/nouveau/nouveau_drm.c           |  1 -
>>  drivers/gpu/drm/omapdrm/omap_drv.c              |  1 -
>>  drivers/gpu/drm/shmobile/shmob_drm_drv.c        |  1 -
>>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             |  1 -
>>  drivers/gpu/drm/virtio/virtgpu_drm_bus.c        | 10 -----
>>  drivers/gpu/drm/virtio/virtgpu_drv.c            |  1 -
>>  drivers/gpu/drm/virtio/virtgpu_drv.h            |  1 -
>>  include/drm/drmP.h                              |  1 -
>>  17 files changed, 62 insertions(+), 41 deletions(-)
>
> This patch (commit a325725633c2) regresses X.org on QEMU's virtio-vga
> device. Please see
>
>   https://bugzilla.redhat.com/show_bug.cgi?id=1366842
>
> complete with a bisection log under
>
>   drivers/gpu/drm/virtio/
>
> (comment 20).
>
> Copying Thorsten so he can include this report in his next v4.8-rc8
> regression report, if he chooses so. (Commit a325725633c2 is part of
> v4.8-rc1, but we only managed to identify it now.) The last such report
> I know of is archived e.g. at
> <http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1239220.html>.
>
> Reported-by: Joachim Frieben <jfrieben@hotmail.com>

First of all Joachim thanks for bisecting this. I was thinking about this
bug / issue, while doing my laps in the swimming pool.

I wanted to add a comment to the bug to tell you that this is likely
a Xorg xserver issue and not a kernel issue and that there is no need to
bisect, but it is too late for that now.

Xorg when running without a Xorg.conf searches for what it considers
a "primary" gpu / video-card, basically it attempts to bring up the
right card in setups where there are multiple cards and if it does not
find one exits with an error.

The xserver has a 2 step process for finding the primary card:

1) It searches for is a card which has a vga-bios mapped,
as we've already determined in the mentioned Red Hat bug that works for
the classic qemu emulated video-cards, but not for qemu's virtio-vga.

2) If that does not work Xorg will fallback to any video class device
on pci-bus 1.

This fallback actually has been broken in the Xorg xserver for quite a
while now and only 2 days ago a patch from Laszlo was merged to fix this.

Only for things to break again due to this kernel patch.

Since the whole step 2) thingie is very much tied to x86 machines
where pci-bus 0 used to be the main bus and pci-bus 1 the agp,
which is sorta an obsolete assumption now a days and  since relying
on bus numbers / enumeration order is a bad idea in general I'm not
entirely sure if this counts as a regression.

I've discussed the problem of the xserver exiting with an error when
no primary device can be found with some people (ajax) at XDC last week
since there are other use-cases where the pci-bus 1 fallback does not
work.

As such I've been working on a xserver patch-set to make the xserver
try harder (pick the first available device) when both steps described
above fail to find one, which should make things work even with the
newest (broken / regressed) kernels.

Given this mail thread, I guess I'm working after all today (I had
planned a day off) and I'll try to wrap up this patch-set and reply
to this mail with the server patches attached for Joachim and/or
Laszlo to test.

Regards,

Hans



p.s.

It would be interesting to do a lspci on both a working and a
non-working kernel to see what exactly is going on here.
Emil Velikov Sept. 30, 2016, 9:55 a.m. UTC | #3
Hi all

On 30 September 2016 at 04:09, Laszlo Ersek <lersek@redhat.com> wrote:
> Hello Daniel,
>
> On 06/21/16 14:08, daniel.vetter at ffwll.ch (Daniel Vetter) wrote:
>> 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).
>>

> This patch (commit a325725633c2) regresses X.org on QEMU's virtio-vga
> device. Please see
>
>   https://bugzilla.redhat.com/show_bug.cgi?id=1366842
>
> complete with a bisection log under
>
>   drivers/gpu/drm/virtio/
>
Having gone through the xserver code as this patch was written, I'm
not entrirely sure how this can happen.

Since there is a very nice cleanup series that this patch is build
upon, I'd suggest trying to understand the issue, rather than a blind
revert.

Here we want to check the xserver/libdrm codepath before/after the
kernel patch, in particular the following drmOpen, drmAvailable,
drmSetInterfaceVersion and drmGetBusid. If the xserver is using open()
with drmSetInterfaceVersion then that is _not_ something it should be
doing and I'm against working around it in the kernel (fwiw).

To ease things one can set LIBGL_DEBUG=verbose (yay fun name) and
libdrm will print out some debug output to stderr

Regards,
Emil
Laszlo Ersek Sept. 30, 2016, 10:03 a.m. UTC | #4
On 09/30/16 10:28, Hans de Goede wrote:
> Hi,
> 
> On 30-09-16 05:09, Laszlo Ersek wrote:
>> Hello Daniel,
>>
>> On 06/21/16 14:08, daniel.vetter at ffwll.ch (Daniel Vetter) wrote:
>>> 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).
>>>
>>> Cc: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>>> Cc: Emil Velikov <emil.l.velikov at gmail.com>
>>> Tested-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>>> (virt_gpu)
>>> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>>> ---
>>>  Documentation/DocBook/gpu.tmpl                  |  4 ++
>>>  drivers/gpu/drm/armada/armada_drv.c             |  1 -
>>>  drivers/gpu/drm/drm_ioctl.c                     | 58
>>> +++++++++++++++++++++++++
>>>  drivers/gpu/drm/drm_platform.c                  | 18 --------
>>>  drivers/gpu/drm/etnaviv/etnaviv_drv.c           |  1 -
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c         |  1 -
>>>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  1 -
>>>  drivers/gpu/drm/imx/imx-drm-core.c              |  1 -
>>>  drivers/gpu/drm/msm/msm_drv.c                   |  1 -
>>>  drivers/gpu/drm/nouveau/nouveau_drm.c           |  1 -
>>>  drivers/gpu/drm/omapdrm/omap_drv.c              |  1 -
>>>  drivers/gpu/drm/shmobile/shmob_drm_drv.c        |  1 -
>>>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             |  1 -
>>>  drivers/gpu/drm/virtio/virtgpu_drm_bus.c        | 10 -----
>>>  drivers/gpu/drm/virtio/virtgpu_drv.c            |  1 -
>>>  drivers/gpu/drm/virtio/virtgpu_drv.h            |  1 -
>>>  include/drm/drmP.h                              |  1 -
>>>  17 files changed, 62 insertions(+), 41 deletions(-)
>>
>> This patch (commit a325725633c2) regresses X.org on QEMU's virtio-vga
>> device. Please see
>>
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1366842
>>
>> complete with a bisection log under
>>
>>   drivers/gpu/drm/virtio/
>>
>> (comment 20).
>>
>> Copying Thorsten so he can include this report in his next v4.8-rc8
>> regression report, if he chooses so. (Commit a325725633c2 is part of
>> v4.8-rc1, but we only managed to identify it now.) The last such report
>> I know of is archived e.g. at
>> <http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1239220.html>.
>>
>>
>> Reported-by: Joachim Frieben <jfrieben@hotmail.com>
> 
> First of all Joachim thanks for bisecting this.

(Small correction: while Joachim reported the BZ, the bisection was done
by yours truly. The bisection was painful enough that I'd want to take
"credit" for it -- using the stock Fedora kernel config for the
bisection, I think my laptop must have burned through enough electricity
to power a small town from Christmas to New Year's Eve. I *literally*
took naps between the test cycles. (And I mean literally literally.) I
know about "localmodconfig" but it has broken on me before, so I opted
for the Fedora config.)

> I was thinking about this
> bug / issue, while doing my laps in the swimming pool.

If you do that, it's easy to lose count of your laps ;)

> I wanted to add a comment to the bug to tell you that this is likely
> a Xorg xserver issue and not a kernel issue and that there is no need to
> bisect, but it is too late for that now.

Ouch. :/

> Xorg when running without a Xorg.conf searches for what it considers
> a "primary" gpu / video-card, basically it attempts to bring up the
> right card in setups where there are multiple cards and if it does not
> find one exits with an error.
> 
> The xserver has a 2 step process for finding the primary card:
> 
> 1) It searches for is a card which has a vga-bios mapped,
> as we've already determined in the mentioned Red Hat bug that works for
> the classic qemu emulated video-cards, but not for qemu's virtio-vga.
> 
> 2) If that does not work Xorg will fallback to any video class device
> on pci-bus 1.
> 
> This fallback actually has been broken in the Xorg xserver for quite a
> while now and only 2 days ago a patch from Laszlo was merged to fix this.
> 
> Only for things to break again due to this kernel patch.
> 
> Since the whole step 2) thingie is very much tied to x86 machines
> where pci-bus 0 used to be the main bus and pci-bus 1 the agp,
> which is sorta an obsolete assumption now a days and  since relying
> on bus numbers / enumeration order is a bad idea in general I'm not
> entirely sure if this counts as a regression.
> 
> I've discussed the problem of the xserver exiting with an error when
> no primary device can be found with some people (ajax) at XDC last week
> since there are other use-cases where the pci-bus 1 fallback does not
> work.
> 
> As such I've been working on a xserver patch-set to make the xserver
> try harder (pick the first available device) when both steps described
> above fail to find one, which should make things work even with the
> newest (broken / regressed) kernels.
> 
> Given this mail thread, I guess I'm working after all today (I had
> planned a day off)

Apologies...

> and I'll try to wrap up this patch-set and reply
> to this mail with the server patches attached for Joachim and/or
> Laszlo to test.

Thank you Hans, that's very kind of you. (And I also greatly appreciate
your description of the primary card selection logic.)

> p.s.
> 
> It would be interesting to do a lspci on both a working and a
> non-working kernel to see what exactly is going on here.

I'll upload the outputs to the RHBZ soon.

Thanks!
Laszlo
Emil Velikov Sept. 30, 2016, 10:37 a.m. UTC | #5
On 30 September 2016 at 10:55, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> Hi all
>
> On 30 September 2016 at 04:09, Laszlo Ersek <lersek@redhat.com> wrote:
>> Hello Daniel,
>>
>> On 06/21/16 14:08, daniel.vetter at ffwll.ch (Daniel Vetter) wrote:
>>> 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).
>>>
>
>> This patch (commit a325725633c2) regresses X.org on QEMU's virtio-vga
>> device. Please see
>>
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1366842
>>
>> complete with a bisection log under
>>
>>   drivers/gpu/drm/virtio/
>>
> Having gone through the xserver code as this patch was written, I'm
> not entrirely sure how this can happen.
>
> Since there is a very nice cleanup series that this patch is build
> upon, I'd suggest trying to understand the issue, rather than a blind
> revert.
>
> Here we want to check the xserver/libdrm codepath before/after the
> kernel patch, in particular the following drmOpen, drmAvailable,
> drmSetInterfaceVersion and drmGetBusid. If the xserver is using open()
> with drmSetInterfaceVersion then that is _not_ something it should be
> doing and I'm against working around it in the kernel (fwiw).
>
Skimming through the xserver code, with the xserver patch/fix the
following comes up - before we would use open(card0) while now we'll
opt for drmOpen alongside the drmSetInterfaceVersion/drmGetBusid.

IMHO the proper way (tm) to fix this is to stop using drmOpen (replace
with open), drmAvailable, drmSetInterfaceVersion (drop these two, they
are legacy UMS code) and drmGetBusid (where needed use the drm*Device
API.

Ideally we'll get platform/usb devices support for drm*Device thus one
will be able to correctly query/enumerate all the available devices,
apply heuristics and open() the {card,render,control} node of the
required device.

On the kernel side I'm haven't looked too closely, although things
should be handled correctly -> drm_dev_set_unique, sets the
unique/busid which is called in virtio via
drm_dev_init/drm_dev_alloc/drm_virtio_init


I'll see if I can find some time to untangle this later on today, if
not I'll look into it tomorrow morning.

-Emil
diff mbox

Patch

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index a836a8bcd289..94c6bdee8080 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -3099,6 +3099,10 @@  int num_ioctls;</synopsis>
   <!-- External: render nodes -->
 
     <sect1>
+      <title>libdrm Device Lookup</title>
+!Pdrivers/gpu/drm/drm_vma_manager.c getunique and setversion story
+    </sect1>
+    <sect1>
       <title>Render nodes</title>
       <para>
         DRM core provides multiple character-devices for user-space to use.
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index cb21c0b6374a..f5ebdd681445 100644
--- 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 1fa7619face3..063775f7946e 100644
--- 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 and 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 644169e1a029..2c819ef90090 100644
--- 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 3d4f56df8359..340d390306d8 100644
--- 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 4a679fb9bb02..13d28d4229e2 100644
--- 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 ef2c32ec1616..1edd9bc80294 100644
--- 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 82656654fb21..7746418a4c08 100644
--- 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 568fcc328f27..a02dc2b27739 100644
--- 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 c00ff6e786a3..295e7621cc68 100644
--- 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 6b97011154bf..26c6134eb744 100644
--- 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 ee79264b5b6a..f0492603ea88 100644
--- 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 308e197908fc..d27809372d54 100644
--- 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 88a39165edd5..7f0e93f87a55 100644
--- 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 5820b7020ae5..c13f70cfc461 100644
--- 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 acf556a35cb2..b18ef3111f0c 100644
--- 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 dd07dc552f28..179bdb228ecb 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1128,7 +1128,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)