Re: [PATCH 17/20] drm: add driver->set_busid() callback

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2490 bytes --]

On Fri, Aug 29, 2014 at 12:12:43PM +0200, David Herrmann wrote:
> One step closer to dropping all the drm_bus_* code:
> Add a driver->set_busid() callback and make all drivers use the generic
> helpers. Nouveau is the only driver that uses two different bus-types with
> the same drm_driver. This is totally broken if both buses are available on
> the same machine (unlikely, but lets be safe).

It's not at all unlikely. There are quite a few people using nouveau to
drive an discrete GPU over PCIe on Tegra K1.

I've also been noticing lately (and I'm not sure why I didn't see it
earlier, possibly because the relevant patches weren't in nouveau yet)
that the nouveau kernel driver crashes when running X on Tegra K1 with
nouveau (and the gk20a GPU) enabled. The reason being that the device is
wrongfully marked as drm_pci_bus.

> Therefore, we create two
> different drivers for each platform during module_init() and set the
> set_busid() callback respectively.

From what I can tell this patch should fix the above-mentioned issue, so
thanks for that. =) I do have local changes that do mostly the same
thing, but slightly less elegantly.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
[...]
> @@ -1086,6 +1088,11 @@ EXPORT_SYMBOL(nouveau_platform_device_create_);
>  static int __init
>  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;

I think at some point we may also want to remove the DRIVER_MODESET flag
here. It probably won't be that easy because DRIVER_MODESET still
implies !legacy. I thought I had submitted a patch series at some point
to fix that (and got rid of the useless /dev/dri/cardX for render-only
devices), but I don't quite remember what became of it. I seem to
remember that Daniel didn't like it, but I can't recall the reason.

> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
[...]
> @@ -1507,6 +1509,7 @@ extern int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *speed_mask);
>  
>  /* 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);

One step closer to getting rid of this altogether.

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel