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 nv_device_is_pci() function that allows to make
PCI-dependent code conditional,
- providing a nouveau_drm_platform_probe() 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@xxxxxxxxxx>
---
Changes since v1:
- Refactored nouveau_device_create_() to take an additional bus type
argument instead of having two versions of it that duplicate code.
- Fixed a typo when substituting pci_resource_* with nv_device_resource_*
- Check whether devices are PCI in relevant functions instead of
nouveau_drm_load().
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/nouveau_abi16.c | 13 +++-
drivers/gpu/drm/nouveau/nouveau_agp.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_bios.c | 4 ++
drivers/gpu/drm/nouveau/nouveau_bo.c | 22 +++---
drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_display.c | 3 +-
drivers/gpu/drm/nouveau/nouveau_drm.c | 59 ++++++++++++---
drivers/gpu/drm/nouveau/nouveau_sysfs.c | 8 ++-
drivers/gpu/drm/nouveau/nouveau_ttm.c | 31 ++++----
drivers/gpu/drm/nouveau/nouveau_vga.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 @@ _nouveau_mc_dtor(struct nouveau_object *object)
{
struct nouveau_device *device = nv_device(object);
struct nouveau_mc *pmc = (void *)object;
- free_irq(device->pdev->irq, pmc);
- if (pmc->use_msi)
+ free_irq(pmc->irq, pmc);
+ if (nv_device_is_pci(device) && pmc->use_msi)
You should be able to keep the conditional as is.
pci_disable_msi(device->pdev);
nouveau_subdev_destroy(&pmc->base);
}
@@ -114,22 +114,25 @@ nouveau_mc_create_(struct nouveau_object *parent, struct nouveau_object *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->use_msi = true;
+ if (nv_device_is_pci(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->use_msi = true;
+ break;
}
}
pmc->use_msi = nouveau_boolopt(device->cfgopt, "NvMSI", pmc->use_msi);
As you explicitly disable msi on platform devices you can move the
option parsing within the if (nv_device_is_pci()) 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 _nouveau_mc_dtor.
- if (pmc->use_msi && oclass->msi_rearm) {
+ if (nv_device_is_pci(device) && pmc->use_msi && oclass->msi_rearm) {
Will do that, rebase and post a v3.
Many thanks, and again, welcome to nouveau :-)
Thanks for the review and the welcome! :)
Alex.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
