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.
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.
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) {
Many thanks, and again, welcome to nouveau :-)
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
