[Nouveau] [PATCH v2] drm/nouveau: support for platform devices (original) (raw)

Alexandre Courbot acourbot at nvidia.com
Wed Feb 12 18:02:00 PST 2014

Hi Emil,

On 02/12/2014 11:18 PM, Emil Velikov wrote:

On 12/02/14 05:38, Alexandre Courbot wrote:

Upcoming mobile Kepler GPUs (such as GK20A) use the platform bus instead of PCI to which Nouveau is tightly dependent. This patch allows Nouveau to handle platform devices by:

- abstracting PCI-dependent functions that were typically used for resource querying and page mapping, - introducing a nvdeviceispci() function that allows to make PCI-dependent code conditional, - providing a nouveaudrmplatformprobe() function that takes a GPU platform device to be probed. Core code as well as engine/subdev drivers are updated wherever possible to make use of these functions. Some older drivers are too dependent on PCI to be properly updated, but all newer code on which future chips may depend should at least be runnable with platform devices. Hi Alexandre I've tried really hard to find something wrong with this patch but it seems that you have it polished very nicely.

Great!

There is one quite minor nit in-line, but I'm not fussed either way.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com> --- Changes since v1: - Refactored nouveaudevicecreate() to take an additional bus type argument instead of having two versions of it that duplicate code. - Fixed a typo when substituting pciresource* with nvdeviceresource* - Check whether devices are PCI in relevant functions instead of nouveaudrmload().

drivers/gpu/drm/nouveau/core/engine/device/base.c | 83 ++++++++++++++++++++-- drivers/gpu/drm/nouveau/core/engine/falcon.c | 6 +- drivers/gpu/drm/nouveau/core/engine/fifo/base.c | 2 +- drivers/gpu/drm/nouveau/core/engine/graph/nv20.c | 2 +- drivers/gpu/drm/nouveau/core/engine/graph/nv40.c | 2 +- drivers/gpu/drm/nouveau/core/engine/graph/nvc0.c | 4 +- drivers/gpu/drm/nouveau/core/engine/xtensa.c | 2 +- drivers/gpu/drm/nouveau/core/include/core/device.h | 30 ++++++++ .../gpu/drm/nouveau/core/include/engine/device.h | 17 +++-- drivers/gpu/drm/nouveau/core/include/subdev/mc.h | 1 + drivers/gpu/drm/nouveau/core/os.h | 1 + drivers/gpu/drm/nouveau/core/subdev/bar/base.c | 4 +- drivers/gpu/drm/nouveau/core/subdev/bar/nv50.c | 4 +- drivers/gpu/drm/nouveau/core/subdev/bar/nvc0.c | 15 ++-- .../gpu/drm/nouveau/core/subdev/devinit/fbmem.h | 8 ++- drivers/gpu/drm/nouveau/core/subdev/devinit/nv04.c | 2 +- drivers/gpu/drm/nouveau/core/subdev/devinit/nv05.c | 2 +- drivers/gpu/drm/nouveau/core/subdev/devinit/nv10.c | 2 +- drivers/gpu/drm/nouveau/core/subdev/devinit/nv20.c | 2 +- drivers/gpu/drm/nouveau/core/subdev/fb/nv50.c | 9 +-- drivers/gpu/drm/nouveau/core/subdev/fb/nvc0.c | 9 +-- drivers/gpu/drm/nouveau/core/subdev/i2c/base.c | 2 +- drivers/gpu/drm/nouveau/core/subdev/instmem/nv40.c | 7 +- drivers/gpu/drm/nouveau/core/subdev/mc/base.c | 39 ++++++---- drivers/gpu/drm/nouveau/core/subdev/mxm/base.c | 2 +- drivers/gpu/drm/nouveau/nouveauabi16.c | 13 +++- drivers/gpu/drm/nouveau/nouveauagp.c | 2 +- drivers/gpu/drm/nouveau/nouveaubios.c | 4 ++ drivers/gpu/drm/nouveau/nouveaubo.c | 22 +++--- drivers/gpu/drm/nouveau/nouveauchan.c | 2 +- drivers/gpu/drm/nouveau/nouveaudisplay.c | 3 +- drivers/gpu/drm/nouveau/nouveaudrm.c | 59 ++++++++++++--- drivers/gpu/drm/nouveau/nouveausysfs.c | 8 ++- drivers/gpu/drm/nouveau/nouveauttm.c | 31 ++++---- drivers/gpu/drm/nouveau/nouveauvga.c | 5 ++ 35 files changed, 297 insertions(+), 109 deletions(-) [snip] diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c index b4b9943773bc..572190c8363b 100644 --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c @@ -93,8 +93,8 @@ nouveaumcdtor(struct nouveauobject *object) { struct nouveaudevice *device = nvdevice(object); struct nouveaumc *pmc = (void *)object; - freeirq(device->pdev->irq, pmc); - if (pmc->usemsi) + freeirq(pmc->irq, pmc); + if (nvdeviceispci(device) && pmc->usemsi) You should be able to keep the conditional as is. pcidisablemsi(device->pdev); nouveausubdevdestroy(&pmc->base); } @@ -114,22 +114,25 @@ nouveaumccreate(struct nouveauobject *parent, struct nouveauobject *engine, if (ret) return ret; - switch (device->pdev->device & 0x0ff0) { - case 0x00f0: - case 0x02e0: - /* BR02? NFI how these would be handled yet exactly */ - break; - default: - switch (device->chipset) { - case 0xaa: break; /* reported broken, nv also disable it */ - default: - pmc->usemsi = true; + if (nvdeviceispci(device)) + switch (device->pdev->device & 0x0ff0) { + case 0x00f0: + case 0x02e0: + /* BR02? NFI how these would be handled yet exactly */ break; + default: + switch (device->chipset) { + case 0xaa: + /* reported broken, nv also disable it */ + break; + default: + pmc->usemsi = true; + break; } } pmc->usemsi = nouveauboolopt(device->cfgopt, "NvMSI", pmc->usemsi); As you explicitly disable msi on platform devices you can move the option parsing within the if (nvdeviceispci()) block.

Yes, that's correct. Actually I think it would also make sense to move the next paragraph under the "if (nv_device_is_pci())" block as well, since it also deals with MSI.

This way you can drop the change in the following conditional and the similar one in nouveaumcdtor.

- if (pmc->usemsi && oclass->msirearm) { + if (nvdeviceispci(device) && pmc->usemsi && oclass->msirearm) {

Will do that, rebase and post a v3.

Many thanks, and again, welcome to nouveau :-)

Thanks for the review and the welcome! :)

Alex.

More information about the dri-devel mailing list