All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Nouveau on ARM fixes
@ 2013-08-28  0:00 Lucas Stach
  2013-08-28  0:00 ` [PATCH 1/6] drm/ttm: recognize ARM arch in ioprot handler Lucas Stach
                   ` (5 more replies)
  0 siblings, 6 replies; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  0:00 UTC (permalink / raw)
  To: nouveau; +Cc: dri-devel

This is the first set of patches to make Nouveau work
on Tegra. Those are only the obvious correctness fixes,
a lot of optimization work remains to be done, but at least
it's enough to get accel working and let the machine survive
a piglit run.

A new BO flag is introduced to allow userspace to hint the
kernel about possible optimizations.

Lucas Stach (6):
  drm/ttm: recognize ARM arch in ioprot handler
  drm/ttm: introduce dma cache sync helpers
  drm/nouveau: hook up cache sync functions
  drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS
  drm/nouveau: map IB write-combined
  drm/nouveau: use MSI interrupts

 drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
 drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 ++++++++++++++
 drivers/gpu/drm/nouveau/nouveau_bo.c             | 15 ++++++++++++-
 drivers/gpu/drm/nouveau/nouveau_bo.h             |  1 +
 drivers/gpu/drm/nouveau/nouveau_chan.c           |  3 ++-
 drivers/gpu/drm/nouveau/nouveau_gem.c            |  5 +++++
 drivers/gpu/drm/ttm/ttm_bo_util.c                |  2 +-
 drivers/gpu/drm/ttm/ttm_tt.c                     | 25 +++++++++++++++++++++
 include/drm/ttm/ttm_bo_driver.h                  | 28 ++++++++++++++++++++++++
 include/uapi/drm/nouveau_drm.h                   |  1 +
 10 files changed, 95 insertions(+), 3 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/6] drm/ttm: recognize ARM arch in ioprot handler
  2013-08-28  0:00 [PATCH 0/6] Nouveau on ARM fixes Lucas Stach
@ 2013-08-28  0:00 ` Lucas Stach
  2013-08-28  0:00 ` [PATCH 2/6] drm/ttm: introduce dma cache sync helpers Lucas Stach
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  0:00 UTC (permalink / raw)
  To: nouveau; +Cc: dri-devel

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 319cf41..db15687 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -487,7 +487,7 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp)
 			pgprot_val(tmp) |= _PAGE_GUARDED;
 	}
 #endif
-#if defined(__ia64__)
+#if defined(__ia64__) || defined(__arm__)
 	if (caching_flags & TTM_PL_FLAG_WC)
 		tmp = pgprot_writecombine(tmp);
 	else
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/6] drm/ttm: introduce dma cache sync helpers
  2013-08-28  0:00 [PATCH 0/6] Nouveau on ARM fixes Lucas Stach
  2013-08-28  0:00 ` [PATCH 1/6] drm/ttm: recognize ARM arch in ioprot handler Lucas Stach
@ 2013-08-28  0:00 ` Lucas Stach
  2013-08-28  0:00 ` [PATCH 4/6] drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS Lucas Stach
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  0:00 UTC (permalink / raw)
  To: nouveau; +Cc: dri-devel

On arches with non-coherent PCI, we need to flush caches ourselfes at
the appropriate places. Introduce two small helpers to make things easy
for TTM based drivers.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/gpu/drm/ttm/ttm_tt.c    | 25 +++++++++++++++++++++++++
 include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 5e93a52..935e121 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -38,6 +38,7 @@
 #include <linux/swap.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/dma-mapping.h>
 #include <drm/drm_cache.h>
 #include <drm/drm_mem_util.h>
 #include <drm/ttm/ttm_module.h>
@@ -249,6 +250,30 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
 }
 EXPORT_SYMBOL(ttm_dma_tt_fini);
 
+void ttm_dma_tt_cache_sync_for_device(struct ttm_dma_tt *ttm_dma,
+				      struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < ttm_dma->ttm.num_pages; i++) {
+		dma_sync_single_for_device(dev, ttm_dma->dma_address[i],
+					   PAGE_SIZE, DMA_TO_DEVICE);
+	}
+}
+EXPORT_SYMBOL(ttm_dma_tt_cache_sync_for_device);
+
+void ttm_dma_tt_cache_sync_for_cpu(struct ttm_dma_tt *ttm_dma,
+				   struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < ttm_dma->ttm.num_pages; i++) {
+		dma_sync_single_for_cpu(dev, ttm_dma->dma_address[i],
+					PAGE_SIZE, DMA_FROM_DEVICE);
+	}
+}
+EXPORT_SYMBOL(ttm_dma_tt_cache_sync_for_cpu);
+
 void ttm_tt_unbind(struct ttm_tt *ttm)
 {
 	int ret;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 984fc2d..db5f3b5 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -40,6 +40,7 @@
 #include <linux/fs.h>
 #include <linux/spinlock.h>
 #include <linux/reservation.h>
+#include <linux/device.h>
 
 struct ttm_backend_func {
 	/**
@@ -681,6 +682,33 @@ extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
 extern int ttm_tt_swapout(struct ttm_tt *ttm,
 			  struct file *persistent_swap_storage);
 
+/**
+ * ttm_dma_tt_cache_sync_for_device:
+ *
+ * @ttm A struct ttm_tt of the type returned by ttm_dma_tt_init.
+ * @dev A struct device representing the device to which to sync.
+ *
+ * This function will flush the CPU caches on arches where snooping in the
+ * TT is not available. On fully coherent arches this will turn into an (almost)
+ * noop. This makes sure that data written by the CPU is visible to the device.
+ */
+extern void ttm_dma_tt_cache_sync_for_device(struct ttm_dma_tt *ttm_dma,
+					     struct device *dev);
+
+/**
+ * ttm_dma_tt_cache_sync_for_cpu:
+ *
+ * @ttm A struct ttm_tt of the type returned by ttm_dma_tt_init.
+ * @dev A struct device representing the device from which to sync.
+ *
+ * This function will invalidate the CPU caches on arches where snooping in the
+ * TT is not available. On fully coherent arches this will turn into an (almost)
+ * noop. This makes sure that the CPU does not read any stale cached or
+ * prefetched data.
+ */
+extern void ttm_dma_tt_cache_sync_for_cpu(struct ttm_dma_tt *ttm_dma,
+					  struct device *dev);
+
 /*
  * ttm_bo.c
  */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/6] drm/nouveau: hook up cache sync functions
       [not found] ` <1377648050-6649-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
@ 2013-08-28  0:00   ` Lucas Stach
  2013-08-28 16:43     ` Konrad Rzeszutek Wilk
  2013-08-28  0:00   ` [PATCH 6/6] drm/nouveau: use MSI interrupts Lucas Stach
  1 sibling, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  0:00 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c  | 4 ++++
 drivers/gpu/drm/nouveau/nouveau_gem.c | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index af20fba..f4a2eb9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -411,6 +411,10 @@ nouveau_bo_validate(struct nouveau_bo *nvbo, bool interruptible,
 {
 	int ret;
 
+	if (nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
+		ttm_dma_tt_cache_sync_for_device((struct ttm_dma_tt *)nvbo->bo.ttm,
+			&nouveau_bdev(nvbo->bo.ttm->bdev)->dev->pdev->dev);
+
 	ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement,
 			      interruptible, no_wait_gpu);
 	if (ret)
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 830cb7b..f632b92 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -901,6 +901,11 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void *data,
 	ret = ttm_bo_wait(&nvbo->bo, true, true, no_wait);
 	spin_unlock(&nvbo->bo.bdev->fence_lock);
 	drm_gem_object_unreference_unlocked(gem);
+
+	if (!ret && nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
+		ttm_dma_tt_cache_sync_for_cpu((struct ttm_dma_tt *)nvbo->bo.ttm,
+					      &dev->pdev->dev);
+
 	return ret;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 4/6] drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS
  2013-08-28  0:00 [PATCH 0/6] Nouveau on ARM fixes Lucas Stach
  2013-08-28  0:00 ` [PATCH 1/6] drm/ttm: recognize ARM arch in ioprot handler Lucas Stach
  2013-08-28  0:00 ` [PATCH 2/6] drm/ttm: introduce dma cache sync helpers Lucas Stach
@ 2013-08-28  0:00 ` Lucas Stach
  2013-08-28  7:11   ` Ben Skeggs
  2013-08-28  0:00 ` [PATCH 5/6] drm/nouveau: map IB write-combined Lucas Stach
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  0:00 UTC (permalink / raw)
  To: nouveau; +Cc: dri-devel

This flag allows userspace to give the kernel a hint that it should use
a non-snooped resource. To guarantee coherency at all times mappings
into userspace are done write combined, so userspace should avoid
reading back from those resources.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
On x86 an optimized userspace can save up on snoop traffic in the
system, on ARM the benefits are potentially much larger, as we can save
the manual cache flush/invalidate.
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 11 ++++++++++-
 drivers/gpu/drm/nouveau/nouveau_bo.h |  1 +
 include/uapi/drm/nouveau_drm.h       |  1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index f4a2eb9..c5fcbcc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -231,6 +231,12 @@ nouveau_bo_new(struct drm_device *dev, int size, int align,
 
 	nouveau_bo_fixup_align(nvbo, flags, &align, &size);
 	nvbo->bo.mem.num_pages = size >> PAGE_SHIFT;
+
+	if (tile_flags & NOUVEAU_GEM_TILE_WCUS)
+		nvbo->valid_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
+	else
+		nvbo->valid_caching = TTM_PL_MASK_CACHING;
+
 	nouveau_bo_placement_set(nvbo, flags, 0);
 
 	acc_size = ttm_bo_dma_acc_size(&drm->ttm.bdev, size,
@@ -292,7 +298,7 @@ void
 nouveau_bo_placement_set(struct nouveau_bo *nvbo, uint32_t type, uint32_t busy)
 {
 	struct ttm_placement *pl = &nvbo->placement;
-	uint32_t flags = TTM_PL_MASK_CACHING |
+	uint32_t flags = nvbo->valid_caching |
 		(nvbo->pin_refcnt ? TTM_PL_FLAG_NO_EVICT : 0);
 
 	pl->placement = nvbo->placements;
@@ -1554,6 +1560,9 @@ nouveau_bo_vma_add(struct nouveau_bo *nvbo, struct nouveau_vm *vm,
 	if (nvbo->bo.mem.mem_type == TTM_PL_VRAM)
 		nouveau_vm_map(vma, nvbo->bo.mem.mm_node);
 	else if (nvbo->bo.mem.mem_type == TTM_PL_TT) {
+		if (!(nvbo->valid_caching & TTM_PL_FLAG_CACHED))
+			vma->access |= NV_MEM_ACCESS_NOSNOOP;
+
 		if (node->sg)
 			nouveau_vm_map_sg_table(vma, 0, size, node);
 		else
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h
index 653dbbb..2ecf8b7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -9,6 +9,7 @@ struct nouveau_bo {
 	struct ttm_buffer_object bo;
 	struct ttm_placement placement;
 	u32 valid_domains;
+	u32 valid_caching;
 	u32 placements[3];
 	u32 busy_placements[3];
 	struct ttm_bo_kmap_obj kmap;
diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index 2a5769f..4948eee2 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -36,6 +36,7 @@
 #define NOUVEAU_GEM_TILE_32BPP       0x00000002
 #define NOUVEAU_GEM_TILE_ZETA        0x00000004
 #define NOUVEAU_GEM_TILE_NONCONTIG   0x00000008
+#define NOUVEAU_GEM_TILE_WCUS        0x00000010 /* write-combined, unsnooped */
 
 struct drm_nouveau_gem_info {
 	uint32_t handle;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 5/6] drm/nouveau: map IB write-combined
  2013-08-28  0:00 [PATCH 0/6] Nouveau on ARM fixes Lucas Stach
                   ` (2 preceding siblings ...)
  2013-08-28  0:00 ` [PATCH 4/6] drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS Lucas Stach
@ 2013-08-28  0:00 ` Lucas Stach
       [not found] ` <1377648050-6649-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
  2013-08-28  7:50 ` [PATCH 0/6] Nouveau on ARM fixes Thierry Reding
  5 siblings, 0 replies; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  0:00 UTC (permalink / raw)
  To: nouveau; +Cc: dri-devel

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/gpu/drm/nouveau/nouveau_chan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
index e84f4c3..3b54e8f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -114,7 +114,8 @@ nouveau_channel_prep(struct nouveau_drm *drm, struct nouveau_cli *cli,
 	if (nouveau_vram_pushbuf)
 		target = TTM_PL_FLAG_VRAM;
 
-	ret = nouveau_bo_new(drm->dev, size, 0, target, 0, 0, NULL,
+	ret = nouveau_bo_new(drm->dev, size, 0, target, 0,
+			    NOUVEAU_GEM_TILE_WCUS, NULL,
 			    &chan->push.buffer);
 	if (ret == 0) {
 		ret = nouveau_bo_pin(chan->push.buffer, target);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 6/6] drm/nouveau: use MSI interrupts
       [not found] ` <1377648050-6649-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
  2013-08-28  0:00   ` [PATCH 3/6] drm/nouveau: hook up cache sync functions Lucas Stach
@ 2013-08-28  0:00   ` Lucas Stach
       [not found]     ` <1377648050-6649-7-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
  1 sibling, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  0:00 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

MSIs were only problematic on some old, broken chipsets. But now that we
already see systems where PCI legacy interrupts are somewhat flaky, it's
really time to move to MSIs.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
 drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
index 9d2cd20..ce6569f 100644
--- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
+++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
@@ -12,6 +12,7 @@ struct nouveau_mc_intr {
 struct nouveau_mc {
 	struct nouveau_subdev base;
 	const struct nouveau_mc_intr *intr_map;
+	bool use_msi;
 };
 
 static inline struct nouveau_mc *
diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
index ec9cd6f..02b337e 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
@@ -23,6 +23,7 @@
  */
 
 #include <subdev/mc.h>
+#include <core/option.h>
 
 static irqreturn_t
 nouveau_mc_intr(int irq, void *arg)
@@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
 		map++;
 	}
 
+	if (pmc->use_msi)
+		nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
+
 	if (intr) {
 		nv_error(pmc, "unknown intr 0x%08x\n", stat);
 	}
@@ -75,6 +79,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)
+		pci_disable_msi(device->pdev);
 	nouveau_subdev_destroy(&pmc->base);
 }
 
@@ -96,6 +102,17 @@ nouveau_mc_create_(struct nouveau_object *parent, struct nouveau_object *engine,
 
 	pmc->intr_map = intr_map;
 
+	pmc->use_msi = nouveau_boolopt(device->cfgopt, "NvMSI", true);
+	if (pmc->use_msi) {
+		ret = pci_enable_msi(device->pdev);
+		if (ret) {
+			pmc->use_msi = false;
+		} else {
+			nv_wr08(device, 0x00088068, 0xff);
+			nv_info(pmc, "MSI interrupts enabled\n");
+		}
+	}
+
 	ret = request_irq(device->pdev->irq, nouveau_mc_intr,
 			  IRQF_SHARED, "nouveau", pmc);
 	if (ret < 0)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
       [not found]     ` <1377648050-6649-7-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
@ 2013-08-28  7:09       ` Ben Skeggs
  2013-08-28  7:28         ` Lucas Stach
  0 siblings, 1 reply; 32+ messages in thread
From: Ben Skeggs @ 2013-08-28  7:09 UTC (permalink / raw)
  To: Lucas Stach
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
> MSIs were only problematic on some old, broken chipsets. But now that we
> already see systems where PCI legacy interrupts are somewhat flaky, it's
> really time to move to MSIs.
>
> Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> ---
>  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> index 9d2cd20..ce6569f 100644
> --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>  struct nouveau_mc {
>         struct nouveau_subdev base;
>         const struct nouveau_mc_intr *intr_map;
> +       bool use_msi;
>  };
>
>  static inline struct nouveau_mc *
> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> index ec9cd6f..02b337e 100644
> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> @@ -23,6 +23,7 @@
>   */
>
>  #include <subdev/mc.h>
> +#include <core/option.h>
>
>  static irqreturn_t
>  nouveau_mc_intr(int irq, void *arg)
> @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>                 map++;
>         }
>
> +       if (pmc->use_msi)
> +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
Register not present everywhere.

At the very least, the enabling of MSI should be disallowed on the
earlier chipsets where it's not supported.  Though, it's perhaps
possible that the pci_enable_msi() call will fail in all of these
cases anyway.. I'm not certain.

> +
>         if (intr) {
>                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
>         }
> @@ -75,6 +79,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)
> +               pci_disable_msi(device->pdev);
>         nouveau_subdev_destroy(&pmc->base);
>  }
>
> @@ -96,6 +102,17 @@ nouveau_mc_create_(struct nouveau_object *parent, struct nouveau_object *engine,
>
>         pmc->intr_map = intr_map;
>
> +       pmc->use_msi = nouveau_boolopt(device->cfgopt, "NvMSI", true);
> +       if (pmc->use_msi) {
> +               ret = pci_enable_msi(device->pdev);
> +               if (ret) {
> +                       pmc->use_msi = false;
> +               } else {
> +                       nv_wr08(device, 0x00088068, 0xff);
> +                       nv_info(pmc, "MSI interrupts enabled\n");
> +               }
> +       }
> +
>         ret = request_irq(device->pdev->irq, nouveau_mc_intr,
>                           IRQF_SHARED, "nouveau", pmc);
>         if (ret < 0)
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/6] drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS
  2013-08-28  0:00 ` [PATCH 4/6] drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS Lucas Stach
@ 2013-08-28  7:11   ` Ben Skeggs
  2013-08-28  7:39     ` Lucas Stach
  0 siblings, 1 reply; 32+ messages in thread
From: Ben Skeggs @ 2013-08-28  7:11 UTC (permalink / raw)
  To: Lucas Stach; +Cc: nouveau, dri-devel

On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
> This flag allows userspace to give the kernel a hint that it should use
> a non-snooped resource. To guarantee coherency at all times mappings
> into userspace are done write combined, so userspace should avoid
> reading back from those resources.
Do any other combinations of cached/uncached and snooped/non-snooped
make any sense?  If so, perhaps we want to split the flags.

>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
> On x86 an optimized userspace can save up on snoop traffic in the
> system, on ARM the benefits are potentially much larger, as we can save
> the manual cache flush/invalidate.
> ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c | 11 ++++++++++-
>  drivers/gpu/drm/nouveau/nouveau_bo.h |  1 +
>  include/uapi/drm/nouveau_drm.h       |  1 +
>  3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index f4a2eb9..c5fcbcc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -231,6 +231,12 @@ nouveau_bo_new(struct drm_device *dev, int size, int align,
>
>         nouveau_bo_fixup_align(nvbo, flags, &align, &size);
>         nvbo->bo.mem.num_pages = size >> PAGE_SHIFT;
> +
> +       if (tile_flags & NOUVEAU_GEM_TILE_WCUS)
> +               nvbo->valid_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> +       else
> +               nvbo->valid_caching = TTM_PL_MASK_CACHING;
> +
>         nouveau_bo_placement_set(nvbo, flags, 0);
>
>         acc_size = ttm_bo_dma_acc_size(&drm->ttm.bdev, size,
> @@ -292,7 +298,7 @@ void
>  nouveau_bo_placement_set(struct nouveau_bo *nvbo, uint32_t type, uint32_t busy)
>  {
>         struct ttm_placement *pl = &nvbo->placement;
> -       uint32_t flags = TTM_PL_MASK_CACHING |
> +       uint32_t flags = nvbo->valid_caching |
>                 (nvbo->pin_refcnt ? TTM_PL_FLAG_NO_EVICT : 0);
>
>         pl->placement = nvbo->placements;
> @@ -1554,6 +1560,9 @@ nouveau_bo_vma_add(struct nouveau_bo *nvbo, struct nouveau_vm *vm,
>         if (nvbo->bo.mem.mem_type == TTM_PL_VRAM)
>                 nouveau_vm_map(vma, nvbo->bo.mem.mm_node);
>         else if (nvbo->bo.mem.mem_type == TTM_PL_TT) {
> +               if (!(nvbo->valid_caching & TTM_PL_FLAG_CACHED))
> +                       vma->access |= NV_MEM_ACCESS_NOSNOOP;
> +
>                 if (node->sg)
>                         nouveau_vm_map_sg_table(vma, 0, size, node);
>                 else
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h
> index 653dbbb..2ecf8b7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
> @@ -9,6 +9,7 @@ struct nouveau_bo {
>         struct ttm_buffer_object bo;
>         struct ttm_placement placement;
>         u32 valid_domains;
> +       u32 valid_caching;
>         u32 placements[3];
>         u32 busy_placements[3];
>         struct ttm_bo_kmap_obj kmap;
> diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
> index 2a5769f..4948eee2 100644
> --- a/include/uapi/drm/nouveau_drm.h
> +++ b/include/uapi/drm/nouveau_drm.h
> @@ -36,6 +36,7 @@
>  #define NOUVEAU_GEM_TILE_32BPP       0x00000002
>  #define NOUVEAU_GEM_TILE_ZETA        0x00000004
>  #define NOUVEAU_GEM_TILE_NONCONTIG   0x00000008
> +#define NOUVEAU_GEM_TILE_WCUS        0x00000010 /* write-combined, unsnooped */
>
>  struct drm_nouveau_gem_info {
>         uint32_t handle;
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-28  7:09       ` Ben Skeggs
@ 2013-08-28  7:28         ` Lucas Stach
  2013-08-28 13:54           ` Ilia Mirkin
  2013-08-28 16:08           ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  7:28 UTC (permalink / raw)
  To: Ben Skeggs, Ilia Mirkin; +Cc: nouveau, dri-devel

Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
> > MSIs were only problematic on some old, broken chipsets. But now that we
> > already see systems where PCI legacy interrupts are somewhat flaky, it's
> > really time to move to MSIs.
> >
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> > index 9d2cd20..ce6569f 100644
> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
> >  struct nouveau_mc {
> >         struct nouveau_subdev base;
> >         const struct nouveau_mc_intr *intr_map;
> > +       bool use_msi;
> >  };
> >
> >  static inline struct nouveau_mc *
> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> > index ec9cd6f..02b337e 100644
> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> > @@ -23,6 +23,7 @@
> >   */
> >
> >  #include <subdev/mc.h>
> > +#include <core/option.h>
> >
> >  static irqreturn_t
> >  nouveau_mc_intr(int irq, void *arg)
> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
> >                 map++;
> >         }
> >
> > +       if (pmc->use_msi)
> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
> Register not present everywhere.
> 
> At the very least, the enabling of MSI should be disallowed on the
> earlier chipsets where it's not supported.  Though, it's perhaps
> possible that the pci_enable_msi() call will fail in all of these
> cases anyway.. I'm not certain.
> 
MSIs are required property for everything doing PCIe. So the only cases
where this should fail is plain PCI/AGP devices. I don't really have a
test system for those old cards set up.

But I remember Ilia having some legacy things plugged in, so maybe he
could test this patch and see how it goes?

> > +
> >         if (intr) {
> >                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
> >         }
> > @@ -75,6 +79,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)
> > +               pci_disable_msi(device->pdev);
> >         nouveau_subdev_destroy(&pmc->base);
> >  }
> >
> > @@ -96,6 +102,17 @@ nouveau_mc_create_(struct nouveau_object *parent, struct nouveau_object *engine,
> >
> >         pmc->intr_map = intr_map;
> >
> > +       pmc->use_msi = nouveau_boolopt(device->cfgopt, "NvMSI", true);
> > +       if (pmc->use_msi) {
> > +               ret = pci_enable_msi(device->pdev);
> > +               if (ret) {
> > +                       pmc->use_msi = false;
> > +               } else {
> > +                       nv_wr08(device, 0x00088068, 0xff);
> > +                       nv_info(pmc, "MSI interrupts enabled\n");
> > +               }
> > +       }
> > +
> >         ret = request_irq(device->pdev->irq, nouveau_mc_intr,
> >                           IRQF_SHARED, "nouveau", pmc);
> >         if (ret < 0)
> > --
> > 1.8.3.1
> >

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/6] drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS
  2013-08-28  7:11   ` Ben Skeggs
@ 2013-08-28  7:39     ` Lucas Stach
  0 siblings, 0 replies; 32+ messages in thread
From: Lucas Stach @ 2013-08-28  7:39 UTC (permalink / raw)
  To: Ben Skeggs; +Cc: nouveau, dri-devel

Am Mittwoch, den 28.08.2013, 17:11 +1000 schrieb Ben Skeggs:
> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
> > This flag allows userspace to give the kernel a hint that it should use
> > a non-snooped resource. To guarantee coherency at all times mappings
> > into userspace are done write combined, so userspace should avoid
> > reading back from those resources.
> Do any other combinations of cached/uncached and snooped/non-snooped
> make any sense?  If so, perhaps we want to split the flags.
> 
Thought about that and I came to the conclusion that it isn't worth the
hassle. If we split it then things get more complicated on x86, were we
would have to invalidate caches manually with all the related
performance implications.

So I think it's a lot easier for userspace writers to just set the WCUS
flag on resources where the can promise no to touch the resource for
reading (AFAIR Christoph wanted this flag mostly for resources that the
driver isn't going to touch ever), or where it can happily live with
uncached reading.

> >
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> > On x86 an optimized userspace can save up on snoop traffic in the
> > system, on ARM the benefits are potentially much larger, as we can save
> > the manual cache flush/invalidate.
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_bo.c | 11 ++++++++++-
> >  drivers/gpu/drm/nouveau/nouveau_bo.h |  1 +
> >  include/uapi/drm/nouveau_drm.h       |  1 +
> >  3 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > index f4a2eb9..c5fcbcc 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > @@ -231,6 +231,12 @@ nouveau_bo_new(struct drm_device *dev, int size, int align,
> >
> >         nouveau_bo_fixup_align(nvbo, flags, &align, &size);
> >         nvbo->bo.mem.num_pages = size >> PAGE_SHIFT;
> > +
> > +       if (tile_flags & NOUVEAU_GEM_TILE_WCUS)
> > +               nvbo->valid_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> > +       else
> > +               nvbo->valid_caching = TTM_PL_MASK_CACHING;
> > +
> >         nouveau_bo_placement_set(nvbo, flags, 0);
> >
> >         acc_size = ttm_bo_dma_acc_size(&drm->ttm.bdev, size,
> > @@ -292,7 +298,7 @@ void
> >  nouveau_bo_placement_set(struct nouveau_bo *nvbo, uint32_t type, uint32_t busy)
> >  {
> >         struct ttm_placement *pl = &nvbo->placement;
> > -       uint32_t flags = TTM_PL_MASK_CACHING |
> > +       uint32_t flags = nvbo->valid_caching |
> >                 (nvbo->pin_refcnt ? TTM_PL_FLAG_NO_EVICT : 0);
> >
> >         pl->placement = nvbo->placements;
> > @@ -1554,6 +1560,9 @@ nouveau_bo_vma_add(struct nouveau_bo *nvbo, struct nouveau_vm *vm,
> >         if (nvbo->bo.mem.mem_type == TTM_PL_VRAM)
> >                 nouveau_vm_map(vma, nvbo->bo.mem.mm_node);
> >         else if (nvbo->bo.mem.mem_type == TTM_PL_TT) {
> > +               if (!(nvbo->valid_caching & TTM_PL_FLAG_CACHED))
> > +                       vma->access |= NV_MEM_ACCESS_NOSNOOP;
> > +
> >                 if (node->sg)
> >                         nouveau_vm_map_sg_table(vma, 0, size, node);
> >                 else
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h
> > index 653dbbb..2ecf8b7 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bo.h
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
> > @@ -9,6 +9,7 @@ struct nouveau_bo {
> >         struct ttm_buffer_object bo;
> >         struct ttm_placement placement;
> >         u32 valid_domains;
> > +       u32 valid_caching;
> >         u32 placements[3];
> >         u32 busy_placements[3];
> >         struct ttm_bo_kmap_obj kmap;
> > diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
> > index 2a5769f..4948eee2 100644
> > --- a/include/uapi/drm/nouveau_drm.h
> > +++ b/include/uapi/drm/nouveau_drm.h
> > @@ -36,6 +36,7 @@
> >  #define NOUVEAU_GEM_TILE_32BPP       0x00000002
> >  #define NOUVEAU_GEM_TILE_ZETA        0x00000004
> >  #define NOUVEAU_GEM_TILE_NONCONTIG   0x00000008
> > +#define NOUVEAU_GEM_TILE_WCUS        0x00000010 /* write-combined, unsnooped */
> >
> >  struct drm_nouveau_gem_info {
> >         uint32_t handle;
> > --
> > 1.8.3.1
> >

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/6] Nouveau on ARM fixes
  2013-08-28  0:00 [PATCH 0/6] Nouveau on ARM fixes Lucas Stach
                   ` (4 preceding siblings ...)
       [not found] ` <1377648050-6649-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
@ 2013-08-28  7:50 ` Thierry Reding
  2013-08-28  8:09   ` Ben Skeggs
  5 siblings, 1 reply; 32+ messages in thread
From: Thierry Reding @ 2013-08-28  7:50 UTC (permalink / raw)
  To: Lucas Stach; +Cc: nouveau, dri-devel


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

On Wed, Aug 28, 2013 at 02:00:44AM +0200, Lucas Stach wrote:
> This is the first set of patches to make Nouveau work
> on Tegra.

Perhaps you should clarify that this patch series allows discrete GPUs
to be used via Tegra's PCIe interface.

People might misinterpret...

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/6] Nouveau on ARM fixes
  2013-08-28  7:50 ` [PATCH 0/6] Nouveau on ARM fixes Thierry Reding
@ 2013-08-28  8:09   ` Ben Skeggs
  0 siblings, 0 replies; 32+ messages in thread
From: Ben Skeggs @ 2013-08-28  8:09 UTC (permalink / raw)
  To: Thierry Reding; +Cc: nouveau, dri-devel

On Wed, Aug 28, 2013 at 5:50 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Wed, Aug 28, 2013 at 02:00:44AM +0200, Lucas Stach wrote:
>> This is the first set of patches to make Nouveau work
>> on Tegra.
>
> Perhaps you should clarify that this patch series allows discrete GPUs
> to be used via Tegra's PCIe interface.
>
> People might misinterpret...
Hah!  Too late, quality journalism already hard at work ;)

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-28  7:28         ` Lucas Stach
@ 2013-08-28 13:54           ` Ilia Mirkin
  2013-08-29  0:07             ` Ben Skeggs
  2013-08-28 16:08           ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 32+ messages in thread
From: Ilia Mirkin @ 2013-08-28 13:54 UTC (permalink / raw)
  To: Lucas Stach; +Cc: nouveau, dri-devel

On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>> > MSIs were only problematic on some old, broken chipsets. But now that we
>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>> > really time to move to MSIs.
>> >
>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>> > ---
>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>> >  2 files changed, 18 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>> > index 9d2cd20..ce6569f 100644
>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>> >  struct nouveau_mc {
>> >         struct nouveau_subdev base;
>> >         const struct nouveau_mc_intr *intr_map;
>> > +       bool use_msi;
>> >  };
>> >
>> >  static inline struct nouveau_mc *
>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> > index ec9cd6f..02b337e 100644
>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> > @@ -23,6 +23,7 @@
>> >   */
>> >
>> >  #include <subdev/mc.h>
>> > +#include <core/option.h>
>> >
>> >  static irqreturn_t
>> >  nouveau_mc_intr(int irq, void *arg)
>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>> >                 map++;
>> >         }
>> >
>> > +       if (pmc->use_msi)
>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>> Register not present everywhere.
>>
>> At the very least, the enabling of MSI should be disallowed on the
>> earlier chipsets where it's not supported.  Though, it's perhaps
>> possible that the pci_enable_msi() call will fail in all of these
>> cases anyway.. I'm not certain.
>>
> MSIs are required property for everything doing PCIe. So the only cases
> where this should fail is plain PCI/AGP devices. I don't really have a
> test system for those old cards set up.
>
> But I remember Ilia having some legacy things plugged in, so maybe he
> could test this patch and see how it goes?

Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
that it's not native PCIe, but some sort of bridge thing IIRC), nv42
PCIe, nv44 PCIe, nv96 PCIe.

Can I just apply this patch, or do I need to do the whole series? What
constitutes a success?

  -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-28  7:28         ` Lucas Stach
  2013-08-28 13:54           ` Ilia Mirkin
@ 2013-08-28 16:08           ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 32+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-08-28 16:08 UTC (permalink / raw)
  To: Lucas Stach; +Cc: nouveau, dri-devel

On Wed, Aug 28, 2013 at 09:28:57AM +0200, Lucas Stach wrote:
> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
> > On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
> > > MSIs were only problematic on some old, broken chipsets. But now that we
> > > already see systems where PCI legacy interrupts are somewhat flaky, it's
> > > really time to move to MSIs.
> > >
> > > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > > ---
> > >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
> > >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
> > >  2 files changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> > > index 9d2cd20..ce6569f 100644
> > > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> > > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> > > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
> > >  struct nouveau_mc {
> > >         struct nouveau_subdev base;
> > >         const struct nouveau_mc_intr *intr_map;
> > > +       bool use_msi;
> > >  };
> > >
> > >  static inline struct nouveau_mc *
> > > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> > > index ec9cd6f..02b337e 100644
> > > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> > > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> > > @@ -23,6 +23,7 @@
> > >   */
> > >
> > >  #include <subdev/mc.h>
> > > +#include <core/option.h>
> > >
> > >  static irqreturn_t
> > >  nouveau_mc_intr(int irq, void *arg)
> > > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
> > >                 map++;
> > >         }
> > >
> > > +       if (pmc->use_msi)
> > > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
> > Register not present everywhere.
> > 
> > At the very least, the enabling of MSI should be disallowed on the
> > earlier chipsets where it's not supported.  Though, it's perhaps
> > possible that the pci_enable_msi() call will fail in all of these
> > cases anyway.. I'm not certain.
> > 
> MSIs are required property for everything doing PCIe. So the only cases
> where this should fail is plain PCI/AGP devices. I don't really have a
> test system for those old cards set up.

That is not true. You can boot a machine with pci=nomsi that has PCIe and
it will work. Legacy interrupts still work on PCIe

> 
> But I remember Ilia having some legacy things plugged in, so maybe he
> could test this patch and see how it goes?
> 
> > > +
> > >         if (intr) {
> > >                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
> > >         }
> > > @@ -75,6 +79,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)
> > > +               pci_disable_msi(device->pdev);
> > >         nouveau_subdev_destroy(&pmc->base);
> > >  }
> > >
> > > @@ -96,6 +102,17 @@ nouveau_mc_create_(struct nouveau_object *parent, struct nouveau_object *engine,
> > >
> > >         pmc->intr_map = intr_map;
> > >
> > > +       pmc->use_msi = nouveau_boolopt(device->cfgopt, "NvMSI", true);
> > > +       if (pmc->use_msi) {
> > > +               ret = pci_enable_msi(device->pdev);
> > > +               if (ret) {
> > > +                       pmc->use_msi = false;
> > > +               } else {
> > > +                       nv_wr08(device, 0x00088068, 0xff);
> > > +                       nv_info(pmc, "MSI interrupts enabled\n");
> > > +               }
> > > +       }
> > > +
> > >         ret = request_irq(device->pdev->irq, nouveau_mc_intr,
> > >                           IRQF_SHARED, "nouveau", pmc);
> > >         if (ret < 0)
> > > --
> > > 1.8.3.1
> > >
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/6] drm/nouveau: hook up cache sync functions
  2013-08-28  0:00   ` [PATCH 3/6] drm/nouveau: hook up cache sync functions Lucas Stach
@ 2013-08-28 16:43     ` Konrad Rzeszutek Wilk
       [not found]       ` <20130828164357.GB27172-6K5HmflnPlqSPmnEAIUT9EEOCMrvLtNR@public.gmane.org>
  0 siblings, 1 reply; 32+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-08-28 16:43 UTC (permalink / raw)
  To: Lucas Stach; +Cc: nouveau, dri-devel

On Wed, Aug 28, 2013 at 02:00:47AM +0200, Lucas Stach wrote:
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c  | 4 ++++
>  drivers/gpu/drm/nouveau/nouveau_gem.c | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index af20fba..f4a2eb9 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -411,6 +411,10 @@ nouveau_bo_validate(struct nouveau_bo *nvbo, bool interruptible,
>  {
>  	int ret;
>  
> +	if (nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)

You don't want to do it also for tt_wc ?

> +		ttm_dma_tt_cache_sync_for_device((struct ttm_dma_tt *)nvbo->bo.ttm,
> +			&nouveau_bdev(nvbo->bo.ttm->bdev)->dev->pdev->dev);
> +
>  	ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement,
>  			      interruptible, no_wait_gpu);
>  	if (ret)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index 830cb7b..f632b92 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -901,6 +901,11 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void *data,
>  	ret = ttm_bo_wait(&nvbo->bo, true, true, no_wait);
>  	spin_unlock(&nvbo->bo.bdev->fence_lock);
>  	drm_gem_object_unreference_unlocked(gem);
> +
> +	if (!ret && nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)

Ditto? 
> +		ttm_dma_tt_cache_sync_for_cpu((struct ttm_dma_tt *)nvbo->bo.ttm,
> +					      &dev->pdev->dev);
> +
>  	return ret;
>  }
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/6] drm/nouveau: hook up cache sync functions
       [not found]       ` <20130828164357.GB27172-6K5HmflnPlqSPmnEAIUT9EEOCMrvLtNR@public.gmane.org>
@ 2013-08-28 16:58         ` Lucas Stach
  2013-08-28 18:21           ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2013-08-28 16:58 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am Mittwoch, den 28.08.2013, 12:43 -0400 schrieb Konrad Rzeszutek Wilk:
> On Wed, Aug 28, 2013 at 02:00:47AM +0200, Lucas Stach wrote:
> > Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_bo.c  | 4 ++++
> >  drivers/gpu/drm/nouveau/nouveau_gem.c | 5 +++++
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > index af20fba..f4a2eb9 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > @@ -411,6 +411,10 @@ nouveau_bo_validate(struct nouveau_bo *nvbo, bool interruptible,
> >  {
> >  	int ret;
> >  
> > +	if (nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
> 
> You don't want to do it also for tt_wc ?
> 
No the point of using writecombined memory for BOs is to explicitly
avoid the need for this cache sync. An uncached MMIO read from the
device should already flush out all writecombining buffers and this read
is always happening when submitting a pushbuf.

> > +		ttm_dma_tt_cache_sync_for_device((struct ttm_dma_tt *)nvbo->bo.ttm,
> > +			&nouveau_bdev(nvbo->bo.ttm->bdev)->dev->pdev->dev);
> > +
> >  	ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement,
> >  			      interruptible, no_wait_gpu);
> >  	if (ret)
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> > index 830cb7b..f632b92 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> > @@ -901,6 +901,11 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void *data,
> >  	ret = ttm_bo_wait(&nvbo->bo, true, true, no_wait);
> >  	spin_unlock(&nvbo->bo.bdev->fence_lock);
> >  	drm_gem_object_unreference_unlocked(gem);
> > +
> > +	if (!ret && nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
> 
> Ditto?
cpu_prep is used to make the kernel aware of a following userspace read.
Writecombined mappings are essentially uncached from the read
perspective.

>  
> > +		ttm_dma_tt_cache_sync_for_cpu((struct ttm_dma_tt *)nvbo->bo.ttm,
> > +					      &dev->pdev->dev);
> > +
> >  	return ret;
> >  }
> >  
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/6] drm/nouveau: hook up cache sync functions
  2013-08-28 16:58         ` Lucas Stach
@ 2013-08-28 18:21           ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 32+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-08-28 18:21 UTC (permalink / raw)
  To: Lucas Stach; +Cc: nouveau, dri-devel

On Wed, Aug 28, 2013 at 06:58:37PM +0200, Lucas Stach wrote:
> Am Mittwoch, den 28.08.2013, 12:43 -0400 schrieb Konrad Rzeszutek Wilk:
> > On Wed, Aug 28, 2013 at 02:00:47AM +0200, Lucas Stach wrote:
> > > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > > ---
> > >  drivers/gpu/drm/nouveau/nouveau_bo.c  | 4 ++++
> > >  drivers/gpu/drm/nouveau/nouveau_gem.c | 5 +++++
> > >  2 files changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > index af20fba..f4a2eb9 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > @@ -411,6 +411,10 @@ nouveau_bo_validate(struct nouveau_bo *nvbo, bool interruptible,
> > >  {
> > >  	int ret;
> > >  
> > > +	if (nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
> > 
> > You don't want to do it also for tt_wc ?
> > 
> No the point of using writecombined memory for BOs is to explicitly
> avoid the need for this cache sync. An uncached MMIO read from the
> device should already flush out all writecombining buffers and this read
> is always happening when submitting a pushbuf.


Could you include this explanation in the git commit description please?

> 
> > > +		ttm_dma_tt_cache_sync_for_device((struct ttm_dma_tt *)nvbo->bo.ttm,
> > > +			&nouveau_bdev(nvbo->bo.ttm->bdev)->dev->pdev->dev);
> > > +
> > >  	ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement,
> > >  			      interruptible, no_wait_gpu);
> > >  	if (ret)
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> > > index 830cb7b..f632b92 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> > > @@ -901,6 +901,11 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void *data,
> > >  	ret = ttm_bo_wait(&nvbo->bo, true, true, no_wait);
> > >  	spin_unlock(&nvbo->bo.bdev->fence_lock);
> > >  	drm_gem_object_unreference_unlocked(gem);
> > > +
> > > +	if (!ret && nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
> > 
> > Ditto?
> cpu_prep is used to make the kernel aware of a following userspace read.
> Writecombined mappings are essentially uncached from the read
> perspective.
> 
> >  
> > > +		ttm_dma_tt_cache_sync_for_cpu((struct ttm_dma_tt *)nvbo->bo.ttm,
> > > +					      &dev->pdev->dev);
> > > +
> > >  	return ret;
> > >  }
> > >  
> > > -- 
> > > 1.8.3.1
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-28 13:54           ` Ilia Mirkin
@ 2013-08-29  0:07             ` Ben Skeggs
       [not found]               ` <CACAvsv7Ew+0icWEq6ixdtP9Vpux4zeWjV5Lpih94YZWgs_jx4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 32+ messages in thread
From: Ben Skeggs @ 2013-08-29  0:07 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: nouveau, dri-devel

On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>> > really time to move to MSIs.
>>> >
>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>> > ---
>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>> >  2 files changed, 18 insertions(+)
>>> >
>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>> > index 9d2cd20..ce6569f 100644
>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>> >  struct nouveau_mc {
>>> >         struct nouveau_subdev base;
>>> >         const struct nouveau_mc_intr *intr_map;
>>> > +       bool use_msi;
>>> >  };
>>> >
>>> >  static inline struct nouveau_mc *
>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>> > index ec9cd6f..02b337e 100644
>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>> > @@ -23,6 +23,7 @@
>>> >   */
>>> >
>>> >  #include <subdev/mc.h>
>>> > +#include <core/option.h>
>>> >
>>> >  static irqreturn_t
>>> >  nouveau_mc_intr(int irq, void *arg)
>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>> >                 map++;
>>> >         }
>>> >
>>> > +       if (pmc->use_msi)
>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>> Register not present everywhere.
>>>
>>> At the very least, the enabling of MSI should be disallowed on the
>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>> possible that the pci_enable_msi() call will fail in all of these
>>> cases anyway.. I'm not certain.
>>>
>> MSIs are required property for everything doing PCIe. So the only cases
>> where this should fail is plain PCI/AGP devices. I don't really have a
>> test system for those old cards set up.
>>
>> But I remember Ilia having some legacy things plugged in, so maybe he
>> could test this patch and see how it goes?
>
> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
> that it's not native PCIe, but some sort of bridge thing IIRC),
Cases like the nv34 here (i think there's some nv4x that aren't native
pcie too) are what I'm wondering about primarily.

> nv42
> PCIe, nv44 PCIe, nv96 PCIe.
>
> Can I just apply this patch, or do I need to do the whole series? What
> constitutes a success?
>
>   -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
       [not found]               ` <CACAvsv7Ew+0icWEq6ixdtP9Vpux4zeWjV5Lpih94YZWgs_jx4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2013-08-29  2:20                 ` Ilia Mirkin
  2013-08-29  4:45                   ` Ben Skeggs
  0 siblings, 1 reply; 32+ messages in thread
From: Ilia Mirkin @ 2013-08-29  2:20 UTC (permalink / raw)
  To: Ben Skeggs
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>> > really time to move to MSIs.
>>>> >
>>>> > Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
>>>> > ---
>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>> >  2 files changed, 18 insertions(+)
>>>> >
>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>> > index 9d2cd20..ce6569f 100644
>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>> >  struct nouveau_mc {
>>>> >         struct nouveau_subdev base;
>>>> >         const struct nouveau_mc_intr *intr_map;
>>>> > +       bool use_msi;
>>>> >  };
>>>> >
>>>> >  static inline struct nouveau_mc *
>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>> > index ec9cd6f..02b337e 100644
>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>> > @@ -23,6 +23,7 @@
>>>> >   */
>>>> >
>>>> >  #include <subdev/mc.h>
>>>> > +#include <core/option.h>
>>>> >
>>>> >  static irqreturn_t
>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>> >                 map++;
>>>> >         }
>>>> >
>>>> > +       if (pmc->use_msi)
>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>> Register not present everywhere.
>>>>
>>>> At the very least, the enabling of MSI should be disallowed on the
>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>> possible that the pci_enable_msi() call will fail in all of these
>>>> cases anyway.. I'm not certain.
>>>>
>>> MSIs are required property for everything doing PCIe. So the only cases
>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>> test system for those old cards set up.
>>>
>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>> could test this patch and see how it goes?
>>
>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>> that it's not native PCIe, but some sort of bridge thing IIRC),
> Cases like the nv34 here (i think there's some nv4x that aren't native
> pcie too) are what I'm wondering about primarily.

And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
it (with no clients connecting to said X), causes a "failed to idle
channel" message in dmesg, which apparently never rectifies itself, so
X is hung forever. FTR, there were no displays connected either, but I
tried the exact same procedure without the MSI patch and it worked
fine. Here is the init sequence with the MSI patch:

[  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
[  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
[  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
[  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
[  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
[  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
[  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
[  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
[  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
[  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
[  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
[  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
[  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
[  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
[  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
[  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
[  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
[  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
[  307.215653] nouveau  [     DRM] VRAM: 63 MiB
[  307.215656] nouveau  [     DRM] GART: 128 MiB
[  307.215659] nouveau  [     DRM] BMP version 5.41
[  307.215662] nouveau  [     DRM] DCB version 2.2
[  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
[  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
[  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
[  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
[  307.215686] nouveau  [     DRM] Adaptor not initialised, running
VBIOS init tables.
[  307.215964] nouveau  [     DRM] Saving VGA fonts
[  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[  307.310087] [drm] No driver support for vblank timestamp query.
[  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
[  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
[  307.410799] nouveau  [     DRM] 0 available performance level(s)
[  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
[  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
[  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
[  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
[  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
ffff8801c73c3800
[...]
[  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
[  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
[  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]

In case it's of interest, this is a Quadro NVS 280 card, here is the
lspci output:

04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
[Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
controller])
        Subsystem: NVIDIA Corporation Device [10de:0215]
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx+
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0, Cache Line Size: 64 bytes
        Interrupt: pin A routed to IRQ 47
        Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
        Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
        Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
        [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
        Capabilities: [60] Power Management version 2
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
                Address: 00000000feeff00c  Data: 4162
        Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
                DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
<512ns, L1 <4us
                        ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
                DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
Unsupported-
                        RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                        MaxPayload 128 bytes, MaxReadReq 512 bytes
                DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
AuxPwr- TransPend-
                LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
Latency L0 <2us, L1 <16us
                        ClockPM- Surprise- LLActRep- BwNot-
                LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
                LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
SlotClk+ DLActive- BWMgmt- ABWMgmt-
        Capabilities: [100 v1] Virtual Channel
                Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
                Arb:    Fixed- WRR32- WRR64- WRR128-
                Ctrl:   ArbSelect=Fixed
                Status: InProgress-
                VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
                        Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
                        Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
                        Status: NegoPending- InProgress-
        Capabilities: [128 v1] Power Budgeting <?>
        Kernel driver in use: nouveau
        Kernel modules: nouveau


Let me know if you have any questions about my setup.

  -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-29  2:20                 ` Ilia Mirkin
@ 2013-08-29  4:45                   ` Ben Skeggs
  2013-08-29  5:00                     ` Ilia Mirkin
  0 siblings, 1 reply; 32+ messages in thread
From: Ben Skeggs @ 2013-08-29  4:45 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: nouveau, dri-devel

On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>> > really time to move to MSIs.
>>>>> >
>>>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>>> > ---
>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>> >  2 files changed, 18 insertions(+)
>>>>> >
>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>> > index 9d2cd20..ce6569f 100644
>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>> >  struct nouveau_mc {
>>>>> >         struct nouveau_subdev base;
>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>> > +       bool use_msi;
>>>>> >  };
>>>>> >
>>>>> >  static inline struct nouveau_mc *
>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>> > index ec9cd6f..02b337e 100644
>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>> > @@ -23,6 +23,7 @@
>>>>> >   */
>>>>> >
>>>>> >  #include <subdev/mc.h>
>>>>> > +#include <core/option.h>
>>>>> >
>>>>> >  static irqreturn_t
>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>> >                 map++;
>>>>> >         }
>>>>> >
>>>>> > +       if (pmc->use_msi)
>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>> Register not present everywhere.
>>>>>
>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>> cases anyway.. I'm not certain.
>>>>>
>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>> test system for those old cards set up.
>>>>
>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>> could test this patch and see how it goes?
>>>
>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>> Cases like the nv34 here (i think there's some nv4x that aren't native
>> pcie too) are what I'm wondering about primarily.
>
> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
> it (with no clients connecting to said X), causes a "failed to idle
> channel" message in dmesg, which apparently never rectifies itself, so
> X is hung forever. FTR, there were no displays connected either, but I
> tried the exact same procedure without the MSI patch and it worked
> fine. Here is the init sequence with the MSI patch:
I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
next thing would be to mmiotrace the binary driver and see if you can
make it enable+use MSI on it.  I doubt the current legacy driver does
it by default, but there was some magic to enable it that you can
probably find if you google around.

>
> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
> [  307.215656] nouveau  [     DRM] GART: 128 MiB
> [  307.215659] nouveau  [     DRM] BMP version 5.41
> [  307.215662] nouveau  [     DRM] DCB version 2.2
> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
> VBIOS init tables.
> [  307.215964] nouveau  [     DRM] Saving VGA fonts
> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
> [  307.310087] [drm] No driver support for vblank timestamp query.
> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
> ffff8801c73c3800
> [...]
> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>
> In case it's of interest, this is a Quadro NVS 280 card, here is the
> lspci output:
>
> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
> controller])
>         Subsystem: NVIDIA Corporation Device [10de:0215]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
> ParErr- Stepping- SERR- FastB2B- DisINTx+
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 64 bytes
>         Interrupt: pin A routed to IRQ 47
>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>         Capabilities: [60] Power Management version 2
>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>                 Address: 00000000feeff00c  Data: 4162
>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
> <512ns, L1 <4us
>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
> Unsupported-
>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
> AuxPwr- TransPend-
>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
> Latency L0 <2us, L1 <16us
>                         ClockPM- Surprise- LLActRep- BwNot-
>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>         Capabilities: [100 v1] Virtual Channel
>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>                 Ctrl:   ArbSelect=Fixed
>                 Status: InProgress-
>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>                         Status: NegoPending- InProgress-
>         Capabilities: [128 v1] Power Budgeting <?>
>         Kernel driver in use: nouveau
>         Kernel modules: nouveau
>
>
> Let me know if you have any questions about my setup.
>
>   -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-29  4:45                   ` Ben Skeggs
@ 2013-08-29  5:00                     ` Ilia Mirkin
  2013-08-29  5:07                       ` Ben Skeggs
  0 siblings, 1 reply; 32+ messages in thread
From: Ilia Mirkin @ 2013-08-29  5:00 UTC (permalink / raw)
  To: Ben Skeggs; +Cc: nouveau, dri-devel

On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>> > really time to move to MSIs.
>>>>>> >
>>>>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>>>> > ---
>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>> >  2 files changed, 18 insertions(+)
>>>>>> >
>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>> > index 9d2cd20..ce6569f 100644
>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>> >  struct nouveau_mc {
>>>>>> >         struct nouveau_subdev base;
>>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>>> > +       bool use_msi;
>>>>>> >  };
>>>>>> >
>>>>>> >  static inline struct nouveau_mc *
>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>> > index ec9cd6f..02b337e 100644
>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>> > @@ -23,6 +23,7 @@
>>>>>> >   */
>>>>>> >
>>>>>> >  #include <subdev/mc.h>
>>>>>> > +#include <core/option.h>
>>>>>> >
>>>>>> >  static irqreturn_t
>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>> >                 map++;
>>>>>> >         }
>>>>>> >
>>>>>> > +       if (pmc->use_msi)
>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>> Register not present everywhere.
>>>>>>
>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>> cases anyway.. I'm not certain.
>>>>>>
>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>> test system for those old cards set up.
>>>>>
>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>> could test this patch and see how it goes?
>>>>
>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>> pcie too) are what I'm wondering about primarily.
>>
>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>> it (with no clients connecting to said X), causes a "failed to idle
>> channel" message in dmesg, which apparently never rectifies itself, so
>> X is hung forever. FTR, there were no displays connected either, but I
>> tried the exact same procedure without the MSI patch and it worked
>> fine. Here is the init sequence with the MSI patch:
> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,

Should that work on the NV42 as well?

> next thing would be to mmiotrace the binary driver and see if you can
> make it enable+use MSI on it.  I doubt the current legacy driver does
> it by default, but there was some magic to enable it that you can
> probably find if you google around.

I've yet to set up the legacy driver... I bet it doesn't compile on
3.11, so I'll have to patch it to nuke procfs/i2c...

>
>>
>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>> VBIOS init tables.
>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>> [  307.310087] [drm] No driver support for vblank timestamp query.
>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>> ffff8801c73c3800
>> [...]
>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>
>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>> lspci output:
>>
>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>> controller])
>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>         Latency: 0, Cache Line Size: 64 bytes
>>         Interrupt: pin A routed to IRQ 47
>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>         Capabilities: [60] Power Management version 2
>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>                 Address: 00000000feeff00c  Data: 4162
>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>> <512ns, L1 <4us
>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>> Unsupported-
>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>> AuxPwr- TransPend-
>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>> Latency L0 <2us, L1 <16us
>>                         ClockPM- Surprise- LLActRep- BwNot-
>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>         Capabilities: [100 v1] Virtual Channel
>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>>                 Ctrl:   ArbSelect=Fixed
>>                 Status: InProgress-
>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>                         Status: NegoPending- InProgress-
>>         Capabilities: [128 v1] Power Budgeting <?>
>>         Kernel driver in use: nouveau
>>         Kernel modules: nouveau
>>
>>
>> Let me know if you have any questions about my setup.
>>
>>   -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-29  5:00                     ` Ilia Mirkin
@ 2013-08-29  5:07                       ` Ben Skeggs
       [not found]                         ` <CACAvsv4AZo-B3MtTh3oN946YAC=vPiR=S3TsEg7R2-hEma57tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 32+ messages in thread
From: Ben Skeggs @ 2013-08-29  5:07 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: nouveau, dri-devel

On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>>> > really time to move to MSIs.
>>>>>>> >
>>>>>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>>>>> > ---
>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>>> >  2 files changed, 18 insertions(+)
>>>>>>> >
>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>> > index 9d2cd20..ce6569f 100644
>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>>> >  struct nouveau_mc {
>>>>>>> >         struct nouveau_subdev base;
>>>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>>>> > +       bool use_msi;
>>>>>>> >  };
>>>>>>> >
>>>>>>> >  static inline struct nouveau_mc *
>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>> > index ec9cd6f..02b337e 100644
>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>> > @@ -23,6 +23,7 @@
>>>>>>> >   */
>>>>>>> >
>>>>>>> >  #include <subdev/mc.h>
>>>>>>> > +#include <core/option.h>
>>>>>>> >
>>>>>>> >  static irqreturn_t
>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>> >                 map++;
>>>>>>> >         }
>>>>>>> >
>>>>>>> > +       if (pmc->use_msi)
>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>>> Register not present everywhere.
>>>>>>>
>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>>> cases anyway.. I'm not certain.
>>>>>>>
>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>>> test system for those old cards set up.
>>>>>>
>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>>> could test this patch and see how it goes?
>>>>>
>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>>> pcie too) are what I'm wondering about primarily.
>>>
>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>>> it (with no clients connecting to said X), causes a "failed to idle
>>> channel" message in dmesg, which apparently never rectifies itself, so
>>> X is hung forever. FTR, there were no displays connected either, but I
>>> tried the exact same procedure without the MSI patch and it worked
>>> fine. Here is the init sequence with the MSI patch:
>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>
> Should that work on the NV42 as well?
I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.

>
>> next thing would be to mmiotrace the binary driver and see if you can
>> make it enable+use MSI on it.  I doubt the current legacy driver does
>> it by default, but there was some magic to enable it that you can
>> probably find if you google around.
>
> I've yet to set up the legacy driver... I bet it doesn't compile on
> 3.11, so I'll have to patch it to nuke procfs/i2c...
>
>>
>>>
>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>>> VBIOS init tables.
>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>>> ffff8801c73c3800
>>> [...]
>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>
>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>>> lspci output:
>>>
>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>>> controller])
>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>         Latency: 0, Cache Line Size: 64 bytes
>>>         Interrupt: pin A routed to IRQ 47
>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>>         Capabilities: [60] Power Management version 2
>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>                 Address: 00000000feeff00c  Data: 4162
>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>>> <512ns, L1 <4us
>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>>> Unsupported-
>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>>> AuxPwr- TransPend-
>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>>> Latency L0 <2us, L1 <16us
>>>                         ClockPM- Surprise- LLActRep- BwNot-
>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>         Capabilities: [100 v1] Virtual Channel
>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>>>                 Ctrl:   ArbSelect=Fixed
>>>                 Status: InProgress-
>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>                         Status: NegoPending- InProgress-
>>>         Capabilities: [128 v1] Power Budgeting <?>
>>>         Kernel driver in use: nouveau
>>>         Kernel modules: nouveau
>>>
>>>
>>> Let me know if you have any questions about my setup.
>>>
>>>   -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
       [not found]                         ` <CACAvsv4AZo-B3MtTh3oN946YAC=vPiR=S3TsEg7R2-hEma57tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2013-08-30  1:10                           ` Ilia Mirkin
  2013-08-30  1:58                             ` Ben Skeggs
  0 siblings, 1 reply; 32+ messages in thread
From: Ilia Mirkin @ 2013-08-30  1:10 UTC (permalink / raw)
  To: Ben Skeggs
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>>>> > really time to move to MSIs.
>>>>>>>> >
>>>>>>>> > Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
>>>>>>>> > ---
>>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>>>> >  2 files changed, 18 insertions(+)
>>>>>>>> >
>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>> > index 9d2cd20..ce6569f 100644
>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>>>> >  struct nouveau_mc {
>>>>>>>> >         struct nouveau_subdev base;
>>>>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>>>>> > +       bool use_msi;
>>>>>>>> >  };
>>>>>>>> >
>>>>>>>> >  static inline struct nouveau_mc *
>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>> > index ec9cd6f..02b337e 100644
>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>> > @@ -23,6 +23,7 @@
>>>>>>>> >   */
>>>>>>>> >
>>>>>>>> >  #include <subdev/mc.h>
>>>>>>>> > +#include <core/option.h>
>>>>>>>> >
>>>>>>>> >  static irqreturn_t
>>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>>> >                 map++;
>>>>>>>> >         }
>>>>>>>> >
>>>>>>>> > +       if (pmc->use_msi)
>>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>>>> Register not present everywhere.
>>>>>>>>
>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>>>> cases anyway.. I'm not certain.
>>>>>>>>
>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>>>> test system for those old cards set up.
>>>>>>>
>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>>>> could test this patch and see how it goes?
>>>>>>
>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>>>> pcie too) are what I'm wondering about primarily.
>>>>
>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>>>> it (with no clients connecting to said X), causes a "failed to idle
>>>> channel" message in dmesg, which apparently never rectifies itself, so
>>>> X is hung forever. FTR, there were no displays connected either, but I
>>>> tried the exact same procedure without the MSI patch and it worked
>>>> fine. Here is the init sequence with the MSI patch:
>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>>
>> Should that work on the NV42 as well?
> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
>
>>
>>> next thing would be to mmiotrace the binary driver and see if you can
>>> make it enable+use MSI on it.  I doubt the current legacy driver does
>>> it by default, but there was some magic to enable it that you can
>>> probably find if you google around.
>>
>> I've yet to set up the legacy driver... I bet it doesn't compile on
>> 3.11, so I'll have to patch it to nuke procfs/i2c...
>>
>>>
>>>>
>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>>>> VBIOS init tables.
>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>>>> ffff8801c73c3800
>>>> [...]
>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>
>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>>>> lspci output:
>>>>
>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>>>> controller])
>>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>>         Latency: 0, Cache Line Size: 64 bytes
>>>>         Interrupt: pin A routed to IRQ 47
>>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>>>         Capabilities: [60] Power Management version 2
>>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>                 Address: 00000000feeff00c  Data: 4162
>>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>>>> <512ns, L1 <4us
>>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>>>> Unsupported-
>>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>>>> AuxPwr- TransPend-
>>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>>>> Latency L0 <2us, L1 <16us
>>>>                         ClockPM- Surprise- LLActRep- BwNot-
>>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>>         Capabilities: [100 v1] Virtual Channel
>>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>>>>                 Ctrl:   ArbSelect=Fixed
>>>>                 Status: InProgress-
>>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>>                         Status: NegoPending- InProgress-
>>>>         Capabilities: [128 v1] Power Budgeting <?>
>>>>         Kernel driver in use: nouveau
>>>>         Kernel modules: nouveau
>>>>
>>>>
>>>> Let me know if you have any questions about my setup.
>>>>
>>>>   -ilia

Same problem with the following (whitespace-damanged) diff applied on top:

diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
index 02b337e..68a51d4 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
@@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
        }

        if (pmc->use_msi)
-               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
+               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);

        if (intr) {
                nv_error(pmc, "unknown intr 0x%08x\n", stat);
@@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
struct nouveau_object *engine,
                if (ret) {
                        pmc->use_msi = false;
                } else {
-                       nv_wr08(device, 0x00088068, 0xff);
+                       nv_wr08(device, 0x1868, 0xff);
                        nv_info(pmc, "MSI interrupts enabled\n");
                }
        }

I guess this needs a way of telling whether it has "for real" MSI or
not. That 1800 range is on NV41:NV50 according to rnndb, which
probably means that it's safe to use msi on nv41+ (via the 88068
address, since the 1800 stuff disappears on nv50+). [Based purely on
speculation, btw, not on hardware experimentation. I assume
pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
which is an agp version of nv44, and all the pci versions of the 6200
(and later) cards... i think there are some 8-series pci cards too.]

  -ilia

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-30  1:10                           ` Ilia Mirkin
@ 2013-08-30  1:58                             ` Ben Skeggs
  2013-08-30  2:00                               ` Ben Skeggs
       [not found]                               ` <CACAvsv5f3vfP1Fs41N=L8Sc_ih32_h5Vz44mjMTJ-WXH9mDe0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 2 replies; 32+ messages in thread
From: Ben Skeggs @ 2013-08-30  1:58 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: nouveau, dri-devel

On Fri, Aug 30, 2013 at 11:10 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>>>>> > really time to move to MSIs.
>>>>>>>>> >
>>>>>>>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>>>>>>> > ---
>>>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>>>>> >  2 files changed, 18 insertions(+)
>>>>>>>>> >
>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>> > index 9d2cd20..ce6569f 100644
>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>>>>> >  struct nouveau_mc {
>>>>>>>>> >         struct nouveau_subdev base;
>>>>>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>>>>>> > +       bool use_msi;
>>>>>>>>> >  };
>>>>>>>>> >
>>>>>>>>> >  static inline struct nouveau_mc *
>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>> > index ec9cd6f..02b337e 100644
>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>> > @@ -23,6 +23,7 @@
>>>>>>>>> >   */
>>>>>>>>> >
>>>>>>>>> >  #include <subdev/mc.h>
>>>>>>>>> > +#include <core/option.h>
>>>>>>>>> >
>>>>>>>>> >  static irqreturn_t
>>>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>>>> >                 map++;
>>>>>>>>> >         }
>>>>>>>>> >
>>>>>>>>> > +       if (pmc->use_msi)
>>>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>>>>> Register not present everywhere.
>>>>>>>>>
>>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>>>>> cases anyway.. I'm not certain.
>>>>>>>>>
>>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>>>>> test system for those old cards set up.
>>>>>>>>
>>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>>>>> could test this patch and see how it goes?
>>>>>>>
>>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>>>>> pcie too) are what I'm wondering about primarily.
>>>>>
>>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>>>>> it (with no clients connecting to said X), causes a "failed to idle
>>>>> channel" message in dmesg, which apparently never rectifies itself, so
>>>>> X is hung forever. FTR, there were no displays connected either, but I
>>>>> tried the exact same procedure without the MSI patch and it worked
>>>>> fine. Here is the init sequence with the MSI patch:
>>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>>>
>>> Should that work on the NV42 as well?
>> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
>>
>>>
>>>> next thing would be to mmiotrace the binary driver and see if you can
>>>> make it enable+use MSI on it.  I doubt the current legacy driver does
>>>> it by default, but there was some magic to enable it that you can
>>>> probably find if you google around.
>>>
>>> I've yet to set up the legacy driver... I bet it doesn't compile on
>>> 3.11, so I'll have to patch it to nuke procfs/i2c...
>>>
>>>>
>>>>>
>>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>>>>> VBIOS init tables.
>>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>>>>> ffff8801c73c3800
>>>>> [...]
>>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>
>>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>>>>> lspci output:
>>>>>
>>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>>>>> controller])
>>>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>>>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>>>         Latency: 0, Cache Line Size: 64 bytes
>>>>>         Interrupt: pin A routed to IRQ 47
>>>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>>>>         Capabilities: [60] Power Management version 2
>>>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>>                 Address: 00000000feeff00c  Data: 4162
>>>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>>>>> <512ns, L1 <4us
>>>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>>>>> Unsupported-
>>>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>>>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>>>>> AuxPwr- TransPend-
>>>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>>>>> Latency L0 <2us, L1 <16us
>>>>>                         ClockPM- Surprise- LLActRep- BwNot-
>>>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>>>         Capabilities: [100 v1] Virtual Channel
>>>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>>>>>                 Ctrl:   ArbSelect=Fixed
>>>>>                 Status: InProgress-
>>>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>>>                         Status: NegoPending- InProgress-
>>>>>         Capabilities: [128 v1] Power Budgeting <?>
>>>>>         Kernel driver in use: nouveau
>>>>>         Kernel modules: nouveau
>>>>>
>>>>>
>>>>> Let me know if you have any questions about my setup.
>>>>>
>>>>>   -ilia
>
> Same problem with the following (whitespace-damanged) diff applied on top:
>
> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> index 02b337e..68a51d4 100644
> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> @@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
>         }
>
>         if (pmc->use_msi)
> -               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
> +               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);
>
>         if (intr) {
>                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
> @@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
> struct nouveau_object *engine,
>                 if (ret) {
>                         pmc->use_msi = false;
>                 } else {
> -                       nv_wr08(device, 0x00088068, 0xff);
> +                       nv_wr08(device, 0x1868, 0xff);
>                         nv_info(pmc, "MSI interrupts enabled\n");
>                 }
>         }
>
> I guess this needs a way of telling whether it has "for real" MSI or
> not. That 1800 range is on NV41:NV50 according to rnndb, which
> probably means that it's safe to use msi on nv41+ (via the 88068
> address, since the 1800 stuff disappears on nv50+). [Based purely on
> speculation, btw, not on hardware experimentation. I assume
> pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
> which is an agp version of nv44, and all the pci versions of the 6200
> (and later) cards... i think there are some 8-series pci cards too.]
Yeah, I suspect the only case we need to explicitly detect is the
BR02-using pretend PCIE cards.

What's the pccid of your nv3x board?

>
>   -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-30  1:58                             ` Ben Skeggs
@ 2013-08-30  2:00                               ` Ben Skeggs
       [not found]                               ` <CACAvsv5f3vfP1Fs41N=L8Sc_ih32_h5Vz44mjMTJ-WXH9mDe0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 0 replies; 32+ messages in thread
From: Ben Skeggs @ 2013-08-30  2:00 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: nouveau, dri-devel

On Fri, Aug 30, 2013 at 11:58 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
> On Fri, Aug 30, 2013 at 11:10 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>>>>>> > really time to move to MSIs.
>>>>>>>>>> >
>>>>>>>>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>>>>>>>> > ---
>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>>>>>> >  2 files changed, 18 insertions(+)
>>>>>>>>>> >
>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>> > index 9d2cd20..ce6569f 100644
>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>>>>>> >  struct nouveau_mc {
>>>>>>>>>> >         struct nouveau_subdev base;
>>>>>>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>>>>>>> > +       bool use_msi;
>>>>>>>>>> >  };
>>>>>>>>>> >
>>>>>>>>>> >  static inline struct nouveau_mc *
>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>> > index ec9cd6f..02b337e 100644
>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>> > @@ -23,6 +23,7 @@
>>>>>>>>>> >   */
>>>>>>>>>> >
>>>>>>>>>> >  #include <subdev/mc.h>
>>>>>>>>>> > +#include <core/option.h>
>>>>>>>>>> >
>>>>>>>>>> >  static irqreturn_t
>>>>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>> >                 map++;
>>>>>>>>>> >         }
>>>>>>>>>> >
>>>>>>>>>> > +       if (pmc->use_msi)
>>>>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>>>>>> Register not present everywhere.
>>>>>>>>>>
>>>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>>>>>> cases anyway.. I'm not certain.
>>>>>>>>>>
>>>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>>>>>> test system for those old cards set up.
>>>>>>>>>
>>>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>>>>>> could test this patch and see how it goes?
>>>>>>>>
>>>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>>>>>> pcie too) are what I'm wondering about primarily.
>>>>>>
>>>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>>>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>>>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>>>>>> it (with no clients connecting to said X), causes a "failed to idle
>>>>>> channel" message in dmesg, which apparently never rectifies itself, so
>>>>>> X is hung forever. FTR, there were no displays connected either, but I
>>>>>> tried the exact same procedure without the MSI patch and it worked
>>>>>> fine. Here is the init sequence with the MSI patch:
>>>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>>>>
>>>> Should that work on the NV42 as well?
>>> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
>>>
>>>>
>>>>> next thing would be to mmiotrace the binary driver and see if you can
>>>>> make it enable+use MSI on it.  I doubt the current legacy driver does
>>>>> it by default, but there was some magic to enable it that you can
>>>>> probably find if you google around.
>>>>
>>>> I've yet to set up the legacy driver... I bet it doesn't compile on
>>>> 3.11, so I'll have to patch it to nuke procfs/i2c...
>>>>
>>>>>
>>>>>>
>>>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>>>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>>>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>>>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>>>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>>>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>>>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>>>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>>>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>>>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>>>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>>>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>>>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>>>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>>>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>>>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>>>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>>>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>>>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>>>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>>>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>>>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>>>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>>>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>>>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>>>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>>>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>>>>>> VBIOS init tables.
>>>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>>>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>>>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>>>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>>>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>>>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>>>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>>>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>>>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>>>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>>>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>>>>>> ffff8801c73c3800
>>>>>> [...]
>>>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>
>>>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>>>>>> lspci output:
>>>>>>
>>>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>>>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>>>>>> controller])
>>>>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>>>>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>>>>         Latency: 0, Cache Line Size: 64 bytes
>>>>>>         Interrupt: pin A routed to IRQ 47
>>>>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>>>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>>>>>         Capabilities: [60] Power Management version 2
>>>>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>>>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>>>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>>>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>>>                 Address: 00000000feeff00c  Data: 4162
>>>>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>>>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>>>>>> <512ns, L1 <4us
>>>>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>>>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>>>>>> Unsupported-
>>>>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>>>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>>>>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>>>>>> AuxPwr- TransPend-
>>>>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>>>>>> Latency L0 <2us, L1 <16us
>>>>>>                         ClockPM- Surprise- LLActRep- BwNot-
>>>>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>>>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>>>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>>>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>>>>         Capabilities: [100 v1] Virtual Channel
>>>>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>>>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>>>>>>                 Ctrl:   ArbSelect=Fixed
>>>>>>                 Status: InProgress-
>>>>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>>>>                         Status: NegoPending- InProgress-
>>>>>>         Capabilities: [128 v1] Power Budgeting <?>
>>>>>>         Kernel driver in use: nouveau
>>>>>>         Kernel modules: nouveau
>>>>>>
>>>>>>
>>>>>> Let me know if you have any questions about my setup.
>>>>>>
>>>>>>   -ilia
>>
>> Same problem with the following (whitespace-damanged) diff applied on top:
>>
>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> index 02b337e..68a51d4 100644
>> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> @@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
>>         }
>>
>>         if (pmc->use_msi)
>> -               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>> +               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);
>>
>>         if (intr) {
>>                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
>> @@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
>> struct nouveau_object *engine,
>>                 if (ret) {
>>                         pmc->use_msi = false;
>>                 } else {
>> -                       nv_wr08(device, 0x00088068, 0xff);
>> +                       nv_wr08(device, 0x1868, 0xff);
>>                         nv_info(pmc, "MSI interrupts enabled\n");
>>                 }
>>         }
>>
>> I guess this needs a way of telling whether it has "for real" MSI or
>> not. That 1800 range is on NV41:NV50 according to rnndb, which
>> probably means that it's safe to use msi on nv41+ (via the 88068
>> address, since the 1800 stuff disappears on nv50+). [Based purely on
>> speculation, btw, not on hardware experimentation. I assume
>> pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
>> which is an agp version of nv44, and all the pci versions of the 6200
>> (and later) cards... i think there are some 8-series pci cards too.]
> Yeah, I suspect the only case we need to explicitly detect is the
> BR02-using pretend PCIE cards.
>
> What's the pccid of your nv3x board?
Err, ignore that.  It's in the lspci output of an earlier mail!

>
>>
>>   -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
       [not found]                               ` <CACAvsv5f3vfP1Fs41N=L8Sc_ih32_h5Vz44mjMTJ-WXH9mDe0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2013-08-30  2:01                                 ` Ilia Mirkin
  2013-08-30  5:36                                   ` Ben Skeggs
  0 siblings, 1 reply; 32+ messages in thread
From: Ilia Mirkin @ 2013-08-30  2:01 UTC (permalink / raw)
  To: Ben Skeggs
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Thu, Aug 29, 2013 at 9:58 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Aug 30, 2013 at 11:10 AM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>>>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>>>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>>>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>>>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>>>>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>>>>>> > really time to move to MSIs.
>>>>>>>>>> >
>>>>>>>>>> > Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
>>>>>>>>>> > ---
>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>>>>>> >  2 files changed, 18 insertions(+)
>>>>>>>>>> >
>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>> > index 9d2cd20..ce6569f 100644
>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>>>>>> >  struct nouveau_mc {
>>>>>>>>>> >         struct nouveau_subdev base;
>>>>>>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>>>>>>> > +       bool use_msi;
>>>>>>>>>> >  };
>>>>>>>>>> >
>>>>>>>>>> >  static inline struct nouveau_mc *
>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>> > index ec9cd6f..02b337e 100644
>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>> > @@ -23,6 +23,7 @@
>>>>>>>>>> >   */
>>>>>>>>>> >
>>>>>>>>>> >  #include <subdev/mc.h>
>>>>>>>>>> > +#include <core/option.h>
>>>>>>>>>> >
>>>>>>>>>> >  static irqreturn_t
>>>>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>> >                 map++;
>>>>>>>>>> >         }
>>>>>>>>>> >
>>>>>>>>>> > +       if (pmc->use_msi)
>>>>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>>>>>> Register not present everywhere.
>>>>>>>>>>
>>>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>>>>>> cases anyway.. I'm not certain.
>>>>>>>>>>
>>>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>>>>>> test system for those old cards set up.
>>>>>>>>>
>>>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>>>>>> could test this patch and see how it goes?
>>>>>>>>
>>>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>>>>>> pcie too) are what I'm wondering about primarily.
>>>>>>
>>>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>>>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>>>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>>>>>> it (with no clients connecting to said X), causes a "failed to idle
>>>>>> channel" message in dmesg, which apparently never rectifies itself, so
>>>>>> X is hung forever. FTR, there were no displays connected either, but I
>>>>>> tried the exact same procedure without the MSI patch and it worked
>>>>>> fine. Here is the init sequence with the MSI patch:
>>>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>>>>
>>>> Should that work on the NV42 as well?
>>> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
>>>
>>>>
>>>>> next thing would be to mmiotrace the binary driver and see if you can
>>>>> make it enable+use MSI on it.  I doubt the current legacy driver does
>>>>> it by default, but there was some magic to enable it that you can
>>>>> probably find if you google around.
>>>>
>>>> I've yet to set up the legacy driver... I bet it doesn't compile on
>>>> 3.11, so I'll have to patch it to nuke procfs/i2c...
>>>>
>>>>>
>>>>>>
>>>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>>>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>>>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>>>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>>>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>>>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>>>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>>>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>>>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>>>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>>>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>>>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>>>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>>>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>>>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>>>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>>>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>>>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>>>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>>>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>>>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>>>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>>>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>>>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>>>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>>>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>>>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>>>>>> VBIOS init tables.
>>>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>>>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>>>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>>>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>>>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>>>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>>>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>>>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>>>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>>>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>>>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>>>>>> ffff8801c73c3800
>>>>>> [...]
>>>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>
>>>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>>>>>> lspci output:
>>>>>>
>>>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>>>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>>>>>> controller])
>>>>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>>>>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>>>>         Latency: 0, Cache Line Size: 64 bytes
>>>>>>         Interrupt: pin A routed to IRQ 47
>>>>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>>>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>>>>>         Capabilities: [60] Power Management version 2
>>>>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>>>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>>>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>>>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>>>                 Address: 00000000feeff00c  Data: 4162
>>>>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>>>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>>>>>> <512ns, L1 <4us
>>>>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>>>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>>>>>> Unsupported-
>>>>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>>>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>>>>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>>>>>> AuxPwr- TransPend-
>>>>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>>>>>> Latency L0 <2us, L1 <16us
>>>>>>                         ClockPM- Surprise- LLActRep- BwNot-
>>>>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>>>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>>>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>>>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>>>>         Capabilities: [100 v1] Virtual Channel
>>>>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>>>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>>>>>>                 Ctrl:   ArbSelect=Fixed
>>>>>>                 Status: InProgress-
>>>>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>>>>                         Status: NegoPending- InProgress-
>>>>>>         Capabilities: [128 v1] Power Budgeting <?>
>>>>>>         Kernel driver in use: nouveau
>>>>>>         Kernel modules: nouveau
>>>>>>
>>>>>>
>>>>>> Let me know if you have any questions about my setup.
>>>>>>
>>>>>>   -ilia
>>
>> Same problem with the following (whitespace-damanged) diff applied on top:
>>
>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> index 02b337e..68a51d4 100644
>> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> @@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
>>         }
>>
>>         if (pmc->use_msi)
>> -               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>> +               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);
>>
>>         if (intr) {
>>                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
>> @@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
>> struct nouveau_object *engine,
>>                 if (ret) {
>>                         pmc->use_msi = false;
>>                 } else {
>> -                       nv_wr08(device, 0x00088068, 0xff);
>> +                       nv_wr08(device, 0x1868, 0xff);
>>                         nv_info(pmc, "MSI interrupts enabled\n");
>>                 }
>>         }
>>
>> I guess this needs a way of telling whether it has "for real" MSI or
>> not. That 1800 range is on NV41:NV50 according to rnndb, which
>> probably means that it's safe to use msi on nv41+ (via the 88068
>> address, since the 1800 stuff disappears on nv50+). [Based purely on
>> speculation, btw, not on hardware experimentation. I assume
>> pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
>> which is an agp version of nv44, and all the pci versions of the 6200
>> (and later) cards... i think there are some 8-series pci cards too.]
> Yeah, I suspect the only case we need to explicitly detect is the
> BR02-using pretend PCIE cards.
>
> What's the pccid of your nv3x board?

I assume that was a typo and you meant "pci id"? If so, 10de:00fd. You
can see the full info (including ids) in the quoted part above.

  -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-30  2:01                                 ` Ilia Mirkin
@ 2013-08-30  5:36                                   ` Ben Skeggs
  2013-08-30  7:11                                     ` Lucas Stach
  0 siblings, 1 reply; 32+ messages in thread
From: Ben Skeggs @ 2013-08-30  5:36 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: nouveau, dri-devel

On Fri, Aug 30, 2013 at 12:01 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Thu, Aug 29, 2013 at 9:58 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>> On Fri, Aug 30, 2013 at 11:10 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>> On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>>>>>>> > really time to move to MSIs.
>>>>>>>>>>> >
>>>>>>>>>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>>>>>>>>> > ---
>>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>>>>>>> >  2 files changed, 18 insertions(+)
>>>>>>>>>>> >
>>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>>> > index 9d2cd20..ce6569f 100644
>>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>>>>>>> >  struct nouveau_mc {
>>>>>>>>>>> >         struct nouveau_subdev base;
>>>>>>>>>>> >         const struct nouveau_mc_intr *intr_map;
>>>>>>>>>>> > +       bool use_msi;
>>>>>>>>>>> >  };
>>>>>>>>>>> >
>>>>>>>>>>> >  static inline struct nouveau_mc *
>>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>>> > index ec9cd6f..02b337e 100644
>>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>>> > @@ -23,6 +23,7 @@
>>>>>>>>>>> >   */
>>>>>>>>>>> >
>>>>>>>>>>> >  #include <subdev/mc.h>
>>>>>>>>>>> > +#include <core/option.h>
>>>>>>>>>>> >
>>>>>>>>>>> >  static irqreturn_t
>>>>>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>>> >                 map++;
>>>>>>>>>>> >         }
>>>>>>>>>>> >
>>>>>>>>>>> > +       if (pmc->use_msi)
>>>>>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>>>>>>> Register not present everywhere.
>>>>>>>>>>>
>>>>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>>>>>>> cases anyway.. I'm not certain.
>>>>>>>>>>>
>>>>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>>>>>>> test system for those old cards set up.
>>>>>>>>>>
>>>>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>>>>>>> could test this patch and see how it goes?
>>>>>>>>>
>>>>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>>>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>>>>>>> pcie too) are what I'm wondering about primarily.
>>>>>>>
>>>>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>>>>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>>>>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>>>>>>> it (with no clients connecting to said X), causes a "failed to idle
>>>>>>> channel" message in dmesg, which apparently never rectifies itself, so
>>>>>>> X is hung forever. FTR, there were no displays connected either, but I
>>>>>>> tried the exact same procedure without the MSI patch and it worked
>>>>>>> fine. Here is the init sequence with the MSI patch:
>>>>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>>>>>
>>>>> Should that work on the NV42 as well?
>>>> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
>>>>
>>>>>
>>>>>> next thing would be to mmiotrace the binary driver and see if you can
>>>>>> make it enable+use MSI on it.  I doubt the current legacy driver does
>>>>>> it by default, but there was some magic to enable it that you can
>>>>>> probably find if you google around.
>>>>>
>>>>> I've yet to set up the legacy driver... I bet it doesn't compile on
>>>>> 3.11, so I'll have to patch it to nuke procfs/i2c...
>>>>>
>>>>>>
>>>>>>>
>>>>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>>>>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>>>>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>>>>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>>>>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>>>>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>>>>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>>>>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>>>>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>>>>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>>>>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>>>>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>>>>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>>>>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>>>>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>>>>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>>>>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>>>>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>>>>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>>>>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>>>>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>>>>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>>>>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>>>>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>>>>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>>>>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>>>>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>>>>>>> VBIOS init tables.
>>>>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>>>>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>>>>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>>>>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>>>>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>>>>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>>>>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>>>>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>>>>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>>>>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>>>>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>>>>>>> ffff8801c73c3800
>>>>>>> [...]
>>>>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>>
>>>>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>>>>>>> lspci output:
>>>>>>>
>>>>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>>>>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>>>>>>> controller])
>>>>>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>>>>>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>>>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>>>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>>>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>>>>>         Latency: 0, Cache Line Size: 64 bytes
>>>>>>>         Interrupt: pin A routed to IRQ 47
>>>>>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>>>>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>>>>>>         Capabilities: [60] Power Management version 2
>>>>>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>>>>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>>>>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>>>>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>>>>                 Address: 00000000feeff00c  Data: 4162
>>>>>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>>>>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>>>>>>> <512ns, L1 <4us
>>>>>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>>>>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>>>>>>> Unsupported-
>>>>>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>>>>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>>>>>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>>>>>>> AuxPwr- TransPend-
>>>>>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>>>>>>> Latency L0 <2us, L1 <16us
>>>>>>>                         ClockPM- Surprise- LLActRep- BwNot-
>>>>>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>>>>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>>>>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>>>>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>>>>>         Capabilities: [100 v1] Virtual Channel
>>>>>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>>>>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>>>>>>>                 Ctrl:   ArbSelect=Fixed
>>>>>>>                 Status: InProgress-
>>>>>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>>>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>>>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>>>>>                         Status: NegoPending- InProgress-
>>>>>>>         Capabilities: [128 v1] Power Budgeting <?>
>>>>>>>         Kernel driver in use: nouveau
>>>>>>>         Kernel modules: nouveau
>>>>>>>
>>>>>>>
>>>>>>> Let me know if you have any questions about my setup.
>>>>>>>
>>>>>>>   -ilia
>>>
>>> Same problem with the following (whitespace-damanged) diff applied on top:
>>>
>>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>> b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>> index 02b337e..68a51d4 100644
>>> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>> @@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
>>>         }
>>>
>>>         if (pmc->use_msi)
>>> -               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>> +               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);
>>>
>>>         if (intr) {
>>>                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
>>> @@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
>>> struct nouveau_object *engine,
>>>                 if (ret) {
>>>                         pmc->use_msi = false;
>>>                 } else {
>>> -                       nv_wr08(device, 0x00088068, 0xff);
>>> +                       nv_wr08(device, 0x1868, 0xff);
>>>                         nv_info(pmc, "MSI interrupts enabled\n");
>>>                 }
>>>         }
>>>
>>> I guess this needs a way of telling whether it has "for real" MSI or
>>> not. That 1800 range is on NV41:NV50 according to rnndb, which
>>> probably means that it's safe to use msi on nv41+ (via the 88068
>>> address, since the 1800 stuff disappears on nv50+). [Based purely on
>>> speculation, btw, not on hardware experimentation. I assume
>>> pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
>>> which is an agp version of nv44, and all the pci versions of the 6200
>>> (and later) cards... i think there are some 8-series pci cards too.]
>> Yeah, I suspect the only case we need to explicitly detect is the
>> BR02-using pretend PCIE cards.
>>
>> What's the pccid of your nv3x board?
>
> I assume that was a typo and you meant "pci id"? If so, 10de:00fd. You
> can see the full info (including ids) in the quoted part above.
Lucas,

Would you be OK with the patch still, but blacklisting devices with
10de:00fX/10de:02eX pciids?

These seem to be the two ranges that indicate that a bridge device is
present (according to xf86-video-nv).

Ben.

>
>   -ilia

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-30  5:36                                   ` Ben Skeggs
@ 2013-08-30  7:11                                     ` Lucas Stach
  2013-09-04  1:45                                       ` Ben Skeggs
  0 siblings, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2013-08-30  7:11 UTC (permalink / raw)
  To: Ben Skeggs; +Cc: nouveau, dri-devel

Am Freitag, den 30.08.2013, 15:36 +1000 schrieb Ben Skeggs:
> On Fri, Aug 30, 2013 at 12:01 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> > On Thu, Aug 29, 2013 at 9:58 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
> >> On Fri, Aug 30, 2013 at 11:10 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> >>> On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
> >>>> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> >>>>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
> >>>>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> >>>>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
> >>>>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> >>>>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
> >>>>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
> >>>>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
> >>>>>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
> >>>>>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
> >>>>>>>>>>> > really time to move to MSIs.
> >>>>>>>>>>> >
> >>>>>>>>>>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> >>>>>>>>>>> > ---
> >>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
> >>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
> >>>>>>>>>>> >  2 files changed, 18 insertions(+)
> >>>>>>>>>>> >
> >>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> >>>>>>>>>>> > index 9d2cd20..ce6569f 100644
> >>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> >>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
> >>>>>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
> >>>>>>>>>>> >  struct nouveau_mc {
> >>>>>>>>>>> >         struct nouveau_subdev base;
> >>>>>>>>>>> >         const struct nouveau_mc_intr *intr_map;
> >>>>>>>>>>> > +       bool use_msi;
> >>>>>>>>>>> >  };
> >>>>>>>>>>> >
> >>>>>>>>>>> >  static inline struct nouveau_mc *
> >>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> >>>>>>>>>>> > index ec9cd6f..02b337e 100644
> >>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> >>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> >>>>>>>>>>> > @@ -23,6 +23,7 @@
> >>>>>>>>>>> >   */
> >>>>>>>>>>> >
> >>>>>>>>>>> >  #include <subdev/mc.h>
> >>>>>>>>>>> > +#include <core/option.h>
> >>>>>>>>>>> >
> >>>>>>>>>>> >  static irqreturn_t
> >>>>>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
> >>>>>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
> >>>>>>>>>>> >                 map++;
> >>>>>>>>>>> >         }
> >>>>>>>>>>> >
> >>>>>>>>>>> > +       if (pmc->use_msi)
> >>>>>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
> >>>>>>>>>>> Register not present everywhere.
> >>>>>>>>>>>
> >>>>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
> >>>>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
> >>>>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
> >>>>>>>>>>> cases anyway.. I'm not certain.
> >>>>>>>>>>>
> >>>>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
> >>>>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
> >>>>>>>>>> test system for those old cards set up.
> >>>>>>>>>>
> >>>>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
> >>>>>>>>>> could test this patch and see how it goes?
> >>>>>>>>>
> >>>>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
> >>>>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
> >>>>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
> >>>>>>>> pcie too) are what I'm wondering about primarily.
> >>>>>>>
> >>>>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
> >>>>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
> >>>>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
> >>>>>>> it (with no clients connecting to said X), causes a "failed to idle
> >>>>>>> channel" message in dmesg, which apparently never rectifies itself, so
> >>>>>>> X is hung forever. FTR, there were no displays connected either, but I
> >>>>>>> tried the exact same procedure without the MSI patch and it worked
> >>>>>>> fine. Here is the init sequence with the MSI patch:
> >>>>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
> >>>>>
> >>>>> Should that work on the NV42 as well?
> >>>> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
> >>>>
> >>>>>
> >>>>>> next thing would be to mmiotrace the binary driver and see if you can
> >>>>>> make it enable+use MSI on it.  I doubt the current legacy driver does
> >>>>>> it by default, but there was some magic to enable it that you can
> >>>>>> probably find if you google around.
> >>>>>
> >>>>> I've yet to set up the legacy driver... I bet it doesn't compile on
> >>>>> 3.11, so I'll have to patch it to nuke procfs/i2c...
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
> >>>>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
> >>>>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
> >>>>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
> >>>>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
> >>>>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
> >>>>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
> >>>>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
> >>>>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
> >>>>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
> >>>>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
> >>>>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
> >>>>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
> >>>>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
> >>>>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
> >>>>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
> >>>>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
> >>>>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
> >>>>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
> >>>>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
> >>>>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
> >>>>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
> >>>>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
> >>>>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
> >>>>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
> >>>>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
> >>>>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
> >>>>>>> VBIOS init tables.
> >>>>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
> >>>>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
> >>>>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
> >>>>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
> >>>>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
> >>>>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
> >>>>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
> >>>>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
> >>>>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
> >>>>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
> >>>>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
> >>>>>>> ffff8801c73c3800
> >>>>>>> [...]
> >>>>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
> >>>>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
> >>>>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
> >>>>>>>
> >>>>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
> >>>>>>> lspci output:
> >>>>>>>
> >>>>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
> >>>>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
> >>>>>>> controller])
> >>>>>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
> >>>>>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
> >>>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
> >>>>>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> >>>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
> >>>>>>>         Latency: 0, Cache Line Size: 64 bytes
> >>>>>>>         Interrupt: pin A routed to IRQ 47
> >>>>>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
> >>>>>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
> >>>>>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
> >>>>>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
> >>>>>>>         Capabilities: [60] Power Management version 2
> >>>>>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
> >>>>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
> >>>>>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
> >>>>>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
> >>>>>>>                 Address: 00000000feeff00c  Data: 4162
> >>>>>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
> >>>>>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
> >>>>>>> <512ns, L1 <4us
> >>>>>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
> >>>>>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
> >>>>>>> Unsupported-
> >>>>>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
> >>>>>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
> >>>>>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
> >>>>>>> AuxPwr- TransPend-
> >>>>>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
> >>>>>>> Latency L0 <2us, L1 <16us
> >>>>>>>                         ClockPM- Surprise- LLActRep- BwNot-
> >>>>>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
> >>>>>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
> >>>>>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
> >>>>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
> >>>>>>>         Capabilities: [100 v1] Virtual Channel
> >>>>>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
> >>>>>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
> >>>>>>>                 Ctrl:   ArbSelect=Fixed
> >>>>>>>                 Status: InProgress-
> >>>>>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
> >>>>>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
> >>>>>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
> >>>>>>>                         Status: NegoPending- InProgress-
> >>>>>>>         Capabilities: [128 v1] Power Budgeting <?>
> >>>>>>>         Kernel driver in use: nouveau
> >>>>>>>         Kernel modules: nouveau
> >>>>>>>
> >>>>>>>
> >>>>>>> Let me know if you have any questions about my setup.
> >>>>>>>
> >>>>>>>   -ilia
> >>>
> >>> Same problem with the following (whitespace-damanged) diff applied on top:
> >>>
> >>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> >>> b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> >>> index 02b337e..68a51d4 100644
> >>> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> >>> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
> >>> @@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
> >>>         }
> >>>
> >>>         if (pmc->use_msi)
> >>> -               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
> >>> +               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);
> >>>
> >>>         if (intr) {
> >>>                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
> >>> @@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
> >>> struct nouveau_object *engine,
> >>>                 if (ret) {
> >>>                         pmc->use_msi = false;
> >>>                 } else {
> >>> -                       nv_wr08(device, 0x00088068, 0xff);
> >>> +                       nv_wr08(device, 0x1868, 0xff);
> >>>                         nv_info(pmc, "MSI interrupts enabled\n");
> >>>                 }
> >>>         }
> >>>
> >>> I guess this needs a way of telling whether it has "for real" MSI or
> >>> not. That 1800 range is on NV41:NV50 according to rnndb, which
> >>> probably means that it's safe to use msi on nv41+ (via the 88068
> >>> address, since the 1800 stuff disappears on nv50+). [Based purely on
> >>> speculation, btw, not on hardware experimentation. I assume
> >>> pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
> >>> which is an agp version of nv44, and all the pci versions of the 6200
> >>> (and later) cards... i think there are some 8-series pci cards too.]
> >> Yeah, I suspect the only case we need to explicitly detect is the
> >> BR02-using pretend PCIE cards.
> >>
> >> What's the pccid of your nv3x board?
> >
> > I assume that was a typo and you meant "pci id"? If so, 10de:00fd. You
> > can see the full info (including ids) in the quoted part above.
> Lucas,
> 
> Would you be OK with the patch still, but blacklisting devices with
> 10de:00fX/10de:02eX pciids?
> 
> These seem to be the two ranges that indicate that a bridge device is
> present (according to xf86-video-nv).
> 
Ok, will put this in for a respin of the series. I personally don't care
too much about cards < NV50 at the moment, but if blacklisting those two
ranges helps to avoid any breakage it's obviously the right thing to do.

Lucas

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-08-30  7:11                                     ` Lucas Stach
@ 2013-09-04  1:45                                       ` Ben Skeggs
  2013-09-30 17:27                                         ` [Nouveau] " Peter Hurley
  0 siblings, 1 reply; 32+ messages in thread
From: Ben Skeggs @ 2013-09-04  1:45 UTC (permalink / raw)
  To: Lucas Stach
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Fri, Aug 30, 2013 at 5:11 PM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
> Am Freitag, den 30.08.2013, 15:36 +1000 schrieb Ben Skeggs:
>> On Fri, Aug 30, 2013 at 12:01 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> > On Thu, Aug 29, 2013 at 9:58 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >> On Fri, Aug 30, 2013 at 11:10 AM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> >>> On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >>>> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> >>>>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >>>>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> >>>>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >>>>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote:
>> >>>>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>> >>>>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>> >>>>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> wrote:
>> >>>>>>>>>>> > MSIs were only problematic on some old, broken chipsets. But now that we
>> >>>>>>>>>>> > already see systems where PCI legacy interrupts are somewhat flaky, it's
>> >>>>>>>>>>> > really time to move to MSIs.
>> >>>>>>>>>>> >
>> >>>>>>>>>>> > Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
>> >>>>>>>>>>> > ---
>> >>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>> >>>>>>>>>>> >  drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>> >>>>>>>>>>> >  2 files changed, 18 insertions(+)
>> >>>>>>>>>>> >
>> >>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>> >>>>>>>>>>> > index 9d2cd20..ce6569f 100644
>> >>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>> >>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>> >>>>>>>>>>> > @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>> >>>>>>>>>>> >  struct nouveau_mc {
>> >>>>>>>>>>> >         struct nouveau_subdev base;
>> >>>>>>>>>>> >         const struct nouveau_mc_intr *intr_map;
>> >>>>>>>>>>> > +       bool use_msi;
>> >>>>>>>>>>> >  };
>> >>>>>>>>>>> >
>> >>>>>>>>>>> >  static inline struct nouveau_mc *
>> >>>>>>>>>>> > diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> >>>>>>>>>>> > index ec9cd6f..02b337e 100644
>> >>>>>>>>>>> > --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> >>>>>>>>>>> > +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> >>>>>>>>>>> > @@ -23,6 +23,7 @@
>> >>>>>>>>>>> >   */
>> >>>>>>>>>>> >
>> >>>>>>>>>>> >  #include <subdev/mc.h>
>> >>>>>>>>>>> > +#include <core/option.h>
>> >>>>>>>>>>> >
>> >>>>>>>>>>> >  static irqreturn_t
>> >>>>>>>>>>> >  nouveau_mc_intr(int irq, void *arg)
>> >>>>>>>>>>> > @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>> >>>>>>>>>>> >                 map++;
>> >>>>>>>>>>> >         }
>> >>>>>>>>>>> >
>> >>>>>>>>>>> > +       if (pmc->use_msi)
>> >>>>>>>>>>> > +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>> >>>>>>>>>>> Register not present everywhere.
>> >>>>>>>>>>>
>> >>>>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>> >>>>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>> >>>>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>> >>>>>>>>>>> cases anyway.. I'm not certain.
>> >>>>>>>>>>>
>> >>>>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>> >>>>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>> >>>>>>>>>> test system for those old cards set up.
>> >>>>>>>>>>
>> >>>>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>> >>>>>>>>>> could test this patch and see how it goes?
>> >>>>>>>>>
>> >>>>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>> >>>>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>> >>>>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>> >>>>>>>> pcie too) are what I'm wondering about primarily.
>> >>>>>>>
>> >>>>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>> >>>>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>> >>>>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>> >>>>>>> it (with no clients connecting to said X), causes a "failed to idle
>> >>>>>>> channel" message in dmesg, which apparently never rectifies itself, so
>> >>>>>>> X is hung forever. FTR, there were no displays connected either, but I
>> >>>>>>> tried the exact same procedure without the MSI patch and it worked
>> >>>>>>> fine. Here is the init sequence with the MSI patch:
>> >>>>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>> >>>>>
>> >>>>> Should that work on the NV42 as well?
>> >>>> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
>> >>>>
>> >>>>>
>> >>>>>> next thing would be to mmiotrace the binary driver and see if you can
>> >>>>>> make it enable+use MSI on it.  I doubt the current legacy driver does
>> >>>>>> it by default, but there was some magic to enable it that you can
>> >>>>>> probably find if you google around.
>> >>>>>
>> >>>>> I've yet to set up the legacy driver... I bet it doesn't compile on
>> >>>>> 3.11, so I'll have to patch it to nuke procfs/i2c...
>> >>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>> >>>>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>> >>>>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>> >>>>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>> >>>>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>> >>>>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>> >>>>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>> >>>>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>> >>>>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>> >>>>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>> >>>>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>> >>>>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>> >>>>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>> >>>>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>> >>>>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>> >>>>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>> >>>>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>> >>>>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>> >>>>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>> >>>>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>> >>>>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>> >>>>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>> >>>>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>> >>>>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>> >>>>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>> >>>>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>> >>>>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>> >>>>>>> VBIOS init tables.
>> >>>>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>> >>>>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>> >>>>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>> >>>>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>> >>>>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>> >>>>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>> >>>>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>> >>>>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>> >>>>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>> >>>>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>> >>>>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>> >>>>>>> ffff8801c73c3800
>> >>>>>>> [...]
>> >>>>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>> >>>>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>> >>>>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>> >>>>>>>
>> >>>>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>> >>>>>>> lspci output:
>> >>>>>>>
>> >>>>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>> >>>>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>> >>>>>>> controller])
>> >>>>>>>         Subsystem: NVIDIA Corporation Device [10de:0215]
>> >>>>>>>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>> >>>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>> >>>>>>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>> >>>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>> >>>>>>>         Latency: 0, Cache Line Size: 64 bytes
>> >>>>>>>         Interrupt: pin A routed to IRQ 47
>> >>>>>>>         Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>> >>>>>>>         Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>> >>>>>>>         Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>> >>>>>>>         [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>> >>>>>>>         Capabilities: [60] Power Management version 2
>> >>>>>>>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>> >>>>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>> >>>>>>>                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>> >>>>>>>         Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>> >>>>>>>                 Address: 00000000feeff00c  Data: 4162
>> >>>>>>>         Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>> >>>>>>>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>> >>>>>>> <512ns, L1 <4us
>> >>>>>>>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>> >>>>>>>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>> >>>>>>> Unsupported-
>> >>>>>>>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>> >>>>>>>                         MaxPayload 128 bytes, MaxReadReq 512 bytes
>> >>>>>>>                 DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>> >>>>>>> AuxPwr- TransPend-
>> >>>>>>>                 LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>> >>>>>>> Latency L0 <2us, L1 <16us
>> >>>>>>>                         ClockPM- Surprise- LLActRep- BwNot-
>> >>>>>>>                 LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>> >>>>>>>                         ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>> >>>>>>>                 LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>> >>>>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>> >>>>>>>         Capabilities: [100 v1] Virtual Channel
>> >>>>>>>                 Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>> >>>>>>>                 Arb:    Fixed- WRR32- WRR64- WRR128-
>> >>>>>>>                 Ctrl:   ArbSelect=Fixed
>> >>>>>>>                 Status: InProgress-
>> >>>>>>>                 VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>> >>>>>>>                         Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>> >>>>>>>                         Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>> >>>>>>>                         Status: NegoPending- InProgress-
>> >>>>>>>         Capabilities: [128 v1] Power Budgeting <?>
>> >>>>>>>         Kernel driver in use: nouveau
>> >>>>>>>         Kernel modules: nouveau
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> Let me know if you have any questions about my setup.
>> >>>>>>>
>> >>>>>>>   -ilia
>> >>>
>> >>> Same problem with the following (whitespace-damanged) diff applied on top:
>> >>>
>> >>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> >>> b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> >>> index 02b337e..68a51d4 100644
>> >>> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> >>> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>> >>> @@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
>> >>>         }
>> >>>
>> >>>         if (pmc->use_msi)
>> >>> -               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>> >>> +               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);
>> >>>
>> >>>         if (intr) {
>> >>>                 nv_error(pmc, "unknown intr 0x%08x\n", stat);
>> >>> @@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
>> >>> struct nouveau_object *engine,
>> >>>                 if (ret) {
>> >>>                         pmc->use_msi = false;
>> >>>                 } else {
>> >>> -                       nv_wr08(device, 0x00088068, 0xff);
>> >>> +                       nv_wr08(device, 0x1868, 0xff);
>> >>>                         nv_info(pmc, "MSI interrupts enabled\n");
>> >>>                 }
>> >>>         }
>> >>>
>> >>> I guess this needs a way of telling whether it has "for real" MSI or
>> >>> not. That 1800 range is on NV41:NV50 according to rnndb, which
>> >>> probably means that it's safe to use msi on nv41+ (via the 88068
>> >>> address, since the 1800 stuff disappears on nv50+). [Based purely on
>> >>> speculation, btw, not on hardware experimentation. I assume
>> >>> pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
>> >>> which is an agp version of nv44, and all the pci versions of the 6200
>> >>> (and later) cards... i think there are some 8-series pci cards too.]
>> >> Yeah, I suspect the only case we need to explicitly detect is the
>> >> BR02-using pretend PCIE cards.
>> >>
>> >> What's the pccid of your nv3x board?
>> >
>> > I assume that was a typo and you meant "pci id"? If so, 10de:00fd. You
>> > can see the full info (including ids) in the quoted part above.
>> Lucas,
>>
>> Would you be OK with the patch still, but blacklisting devices with
>> 10de:00fX/10de:02eX pciids?
>>
>> These seem to be the two ranges that indicate that a bridge device is
>> present (according to xf86-video-nv).
>>
> Ok, will put this in for a respin of the series. I personally don't care
> too much about cards < NV50 at the moment, but if blacklisting those two
> ranges helps to avoid any breakage it's obviously the right thing to do.
Well, we can't just go around breaking stuff deliberately for the
people still using them!

I've blacklisted them myself and merged the patch.

Thanks,
Ben.

>
> Lucas
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Nouveau] [PATCH 6/6] drm/nouveau: use MSI interrupts
  2013-09-04  1:45                                       ` Ben Skeggs
@ 2013-09-30 17:27                                         ` Peter Hurley
       [not found]                                           ` <5249B494.5020500-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2013-09-30 17:27 UTC (permalink / raw)
  To: Ben Skeggs, Lucas Stach; +Cc: nouveau, dri-devel

On 09/03/2013 09:45 PM, Ben Skeggs wrote:
> On Fri, Aug 30, 2013 at 5:11 PM, Lucas Stach <dev@lynxeye.de> wrote:
>> Am Freitag, den 30.08.2013, 15:36 +1000 schrieb Ben Skeggs:
>>> On Fri, Aug 30, 2013 at 12:01 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>> On Thu, Aug 29, 2013 at 9:58 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>> On Fri, Aug 30, 2013 at 11:10 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>> On Thu, Aug 29, 2013 at 1:07 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>>>> On Thu, Aug 29, 2013 at 3:00 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>>>> On Thu, Aug 29, 2013 at 12:45 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>>>>>> On Thu, Aug 29, 2013 at 12:20 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>>>>>> On Wed, Aug 28, 2013 at 8:07 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>>>>>>>> On Wed, Aug 28, 2013 at 11:54 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>>>>>>>>> On Wed, Aug 28, 2013 at 3:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>>>>>>> Am Mittwoch, den 28.08.2013, 17:09 +1000 schrieb Ben Skeggs:
>>>>>>>>>>>>>> On Wed, Aug 28, 2013 at 10:00 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>>>>>>>>>>>> MSIs were only problematic on some old, broken chipsets. But now that we
>>>>>>>>>>>>>>> already see systems where PCI legacy interrupts are somewhat flaky, it's
>>>>>>>>>>>>>>> really time to move to MSIs.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>   drivers/gpu/drm/nouveau/core/include/subdev/mc.h |  1 +
>>>>>>>>>>>>>>>   drivers/gpu/drm/nouveau/core/subdev/mc/base.c    | 17 +++++++++++++++++
>>>>>>>>>>>>>>>   2 files changed, 18 insertions(+)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>>>>>>> index 9d2cd20..ce6569f 100644
>>>>>>>>>>>>>>> --- a/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>>>>>>> +++ b/drivers/gpu/drm/nouveau/core/include/subdev/mc.h
>>>>>>>>>>>>>>> @@ -12,6 +12,7 @@ struct nouveau_mc_intr {
>>>>>>>>>>>>>>>   struct nouveau_mc {
>>>>>>>>>>>>>>>          struct nouveau_subdev base;
>>>>>>>>>>>>>>>          const struct nouveau_mc_intr *intr_map;
>>>>>>>>>>>>>>> +       bool use_msi;
>>>>>>>>>>>>>>>   };
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   static inline struct nouveau_mc *
>>>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>>>>>>> index ec9cd6f..02b337e 100644
>>>>>>>>>>>>>>> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>>>>>>> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>>>>>>>>>>> @@ -23,6 +23,7 @@
>>>>>>>>>>>>>>>    */
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   #include <subdev/mc.h>
>>>>>>>>>>>>>>> +#include <core/option.h>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   static irqreturn_t
>>>>>>>>>>>>>>>   nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>>>>>>> @@ -43,6 +44,9 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>>>>>>>>>>                  map++;
>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> +       if (pmc->use_msi)
>>>>>>>>>>>>>>> +               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>>>>>>>>>> Register not present everywhere.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> At the very least, the enabling of MSI should be disallowed on the
>>>>>>>>>>>>>> earlier chipsets where it's not supported.  Though, it's perhaps
>>>>>>>>>>>>>> possible that the pci_enable_msi() call will fail in all of these
>>>>>>>>>>>>>> cases anyway.. I'm not certain.
>>>>>>>>>>>>>>
>>>>>>>>>>>>> MSIs are required property for everything doing PCIe. So the only cases
>>>>>>>>>>>>> where this should fail is plain PCI/AGP devices. I don't really have a
>>>>>>>>>>>>> test system for those old cards set up.
>>>>>>>>>>>>>
>>>>>>>>>>>>> But I remember Ilia having some legacy things plugged in, so maybe he
>>>>>>>>>>>>> could test this patch and see how it goes?
>>>>>>>>>>>>
>>>>>>>>>>>> Sure, let me know what you need -- I have nv18 PCI, nv34 PCIe (note
>>>>>>>>>>>> that it's not native PCIe, but some sort of bridge thing IIRC),
>>>>>>>>>>> Cases like the nv34 here (i think there's some nv4x that aren't native
>>>>>>>>>>> pcie too) are what I'm wondering about primarily.
>>>>>>>>>>
>>>>>>>>>> And rightly so. With the NV18 PCI, NV34 PCIe, NV42 PCIe plugged in,
>>>>>>>>>> with "AutoAddGPU" disabled the NV18 and NV42 seem fine. However merely
>>>>>>>>>> starting X (not xinit, not startx, not [gkx]dm) on the NV34 and ^C'ing
>>>>>>>>>> it (with no clients connecting to said X), causes a "failed to idle
>>>>>>>>>> channel" message in dmesg, which apparently never rectifies itself, so
>>>>>>>>>> X is hung forever. FTR, there were no displays connected either, but I
>>>>>>>>>> tried the exact same procedure without the MSI patch and it worked
>>>>>>>>>> fine. Here is the init sequence with the MSI patch:
>>>>>>>>> I don't suppose bashing 0x1868 instead of 0x88068 works here?  If not,
>>>>>>>>
>>>>>>>> Should that work on the NV42 as well?
>>>>>>> I believe so.  NV4x has both the 0x18xx and 0x88xxx apertures I believe.
>>>>>>>
>>>>>>>>
>>>>>>>>> next thing would be to mmiotrace the binary driver and see if you can
>>>>>>>>> make it enable+use MSI on it.  I doubt the current legacy driver does
>>>>>>>>> it by default, but there was some magic to enable it that you can
>>>>>>>>> probably find if you google around.
>>>>>>>>
>>>>>>>> I've yet to set up the legacy driver... I bet it doesn't compile on
>>>>>>>> 3.11, so I'll have to patch it to nuke procfs/i2c...
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> [  307.049812] nouveau  [  DEVICE][0000:04:00.0] BOOT0  : 0x034a00b1
>>>>>>>>>> [  307.049815] nouveau  [  DEVICE][0000:04:00.0] Chipset: NV34 (NV34)
>>>>>>>>>> [  307.049819] nouveau  [  DEVICE][0000:04:00.0] Family : NV30
>>>>>>>>>> [  307.050648] nouveau  [   VBIOS][0000:04:00.0] checking PRAMIN for image...
>>>>>>>>>> [  307.050652] nouveau  [   VBIOS][0000:04:00.0] ... signature not found
>>>>>>>>>> [  307.050653] nouveau  [   VBIOS][0000:04:00.0] checking PROM for image...
>>>>>>>>>> [  307.195201] nouveau  [   VBIOS][0000:04:00.0] ... appears to be valid
>>>>>>>>>> [  307.195205] nouveau  [   VBIOS][0000:04:00.0] using image from PROM
>>>>>>>>>> [  307.195209] nouveau  [   VBIOS][0000:04:00.0] BMP version 5.29
>>>>>>>>>> [  307.195429] nouveau  [   VBIOS][0000:04:00.0] version 04.34.20.79.00
>>>>>>>>>> [  307.195971] nouveau  [ DEVINIT][0000:04:00.0] adaptor not initialised
>>>>>>>>>> [  307.195979] nouveau  [   VBIOS][0000:04:00.0] running init tables
>>>>>>>>>> [  307.209253] nouveau 0000:04:00.0: irq 47 for MSI/MSI-X
>>>>>>>>>> [  307.209266] nouveau  [     PMC][0000:04:00.0] MSI interrupts enabled
>>>>>>>>>> [  307.209281] nouveau W[  PTIMER][0000:04:00.0] unknown input clock freq
>>>>>>>>>> [  307.209288] nouveau  [     PFB][0000:04:00.0] RAM type: DDR1
>>>>>>>>>> [  307.209290] nouveau  [     PFB][0000:04:00.0] RAM size: 64 MiB
>>>>>>>>>> [  307.209292] nouveau  [     PFB][0000:04:00.0]    ZCOMP: 0 tags
>>>>>>>>>> [  307.215653] nouveau  [     DRM] VRAM: 63 MiB
>>>>>>>>>> [  307.215656] nouveau  [     DRM] GART: 128 MiB
>>>>>>>>>> [  307.215659] nouveau  [     DRM] BMP version 5.41
>>>>>>>>>> [  307.215662] nouveau  [     DRM] DCB version 2.2
>>>>>>>>>> [  307.215666] nouveau  [     DRM] DCB outp 00: 01000300 000088b8
>>>>>>>>>> [  307.215669] nouveau  [     DRM] DCB outp 01: 02010310 000088b8
>>>>>>>>>> [  307.215672] nouveau  [     DRM] DCB outp 02: 01000302 00000000
>>>>>>>>>> [  307.215676] nouveau  [     DRM] DCB outp 03: 04010312 00000000
>>>>>>>>>> [  307.215686] nouveau  [     DRM] Adaptor not initialised, running
>>>>>>>>>> VBIOS init tables.
>>>>>>>>>> [  307.215964] nouveau  [     DRM] Saving VGA fonts
>>>>>>>>>> [  307.310084] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
>>>>>>>>>> [  307.310087] [drm] No driver support for vblank timestamp query.
>>>>>>>>>> [  307.310093] nouveau  [     DRM] 0xB61E: Parsing digital output script table
>>>>>>>>>> [  307.360111] nouveau  [     DRM] 0xB70B: Parsing digital output script table
>>>>>>>>>> [  307.410799] nouveau  [     DRM] 0 available performance level(s)
>>>>>>>>>> [  307.410804] nouveau  [     DRM] c: core 249MHz memory 405MHz
>>>>>>>>>> [  307.412062] nouveau  [     DRM] MM: using M2MF for buffer copies
>>>>>>>>>> [  307.442478] nouveau 0000:04:00.0: No connectors reported connected with modes
>>>>>>>>>> [  307.442483] [drm] Cannot find any crtc or sizes - going 1024x768
>>>>>>>>>> [  307.442669] nouveau  [     DRM] allocated 1024x768 fb: 0x9000, bo
>>>>>>>>>> ffff8801c73c3800
>>>>>>>>>> [...]
>>>>>>>>>> [  360.414044] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>>>>> [  375.403288] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>>>>> [  390.392407] nouveau E[ X[8294]] failed to idle channel 0xcccc0001 [X[8294]]
>>>>>>>>>>
>>>>>>>>>> In case it's of interest, this is a Quadro NVS 280 card, here is the
>>>>>>>>>> lspci output:
>>>>>>>>>>
>>>>>>>>>> 04:00.0 VGA compatible controller [0300]: NVIDIA Corporation NV37GL
>>>>>>>>>> [Quadro PCI-E Series] [10de:00fd] (rev a2) (prog-if 00 [VGA
>>>>>>>>>> controller])
>>>>>>>>>>          Subsystem: NVIDIA Corporation Device [10de:0215]
>>>>>>>>>>          Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>>>>>>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>>>>>>>>          Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>>>>>>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>>>>>>>>          Latency: 0, Cache Line Size: 64 bytes
>>>>>>>>>>          Interrupt: pin A routed to IRQ 47
>>>>>>>>>>          Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>>>>>          Region 1: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>>>>>>>>>>          Region 2: Memory at f5000000 (32-bit, non-prefetchable) [size=16M]
>>>>>>>>>>          [virtual] Expansion ROM at f6000000 [disabled] [size=128K]
>>>>>>>>>>          Capabilities: [60] Power Management version 2
>>>>>>>>>>                  Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
>>>>>>>>>> PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>>>>>>>>>                  Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>>>>>>>>>>          Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>>>>>>>>>                  Address: 00000000feeff00c  Data: 4162
>>>>>>>>>>          Capabilities: [78] Express (v1) Legacy Endpoint, MSI 00
>>>>>>>>>>                  DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
>>>>>>>>>> <512ns, L1 <4us
>>>>>>>>>>                          ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
>>>>>>>>>>                  DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
>>>>>>>>>> Unsupported-
>>>>>>>>>>                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>>>>>>>>>                          MaxPayload 128 bytes, MaxReadReq 512 bytes
>>>>>>>>>>                  DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
>>>>>>>>>> AuxPwr- TransPend-
>>>>>>>>>>                  LnkCap: Port #0, Speed 2.5GT/s, Width x16, ASPM L0s,
>>>>>>>>>> Latency L0 <2us, L1 <16us
>>>>>>>>>>                          ClockPM- Surprise- LLActRep- BwNot-
>>>>>>>>>>                  LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
>>>>>>>>>>                          ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>>>>>>>>>                  LnkSta: Speed 2.5GT/s, Width x16, TrErr- Train-
>>>>>>>>>> SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>>>>>>>>          Capabilities: [100 v1] Virtual Channel
>>>>>>>>>>                  Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
>>>>>>>>>>                  Arb:    Fixed- WRR32- WRR64- WRR128-
>>>>>>>>>>                  Ctrl:   ArbSelect=Fixed
>>>>>>>>>>                  Status: InProgress-
>>>>>>>>>>                  VC0:    Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>>>>>>>>                          Arb:    Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>>>>>>>>                          Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>>>>>>>>                          Status: NegoPending- InProgress-
>>>>>>>>>>          Capabilities: [128 v1] Power Budgeting <?>
>>>>>>>>>>          Kernel driver in use: nouveau
>>>>>>>>>>          Kernel modules: nouveau
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Let me know if you have any questions about my setup.
>>>>>>>>>>
>>>>>>>>>>    -ilia
>>>>>>
>>>>>> Same problem with the following (whitespace-damanged) diff applied on top:
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>> b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>> index 02b337e..68a51d4 100644
>>>>>> --- a/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>> +++ b/drivers/gpu/drm/nouveau/core/subdev/mc/base.c
>>>>>> @@ -45,7 +45,7 @@ nouveau_mc_intr(int irq, void *arg)
>>>>>>          }
>>>>>>
>>>>>>          if (pmc->use_msi)
>>>>>> -               nv_wr08(pmc->base.base.parent, 0x00088068, 0xff);
>>>>>> +               nv_wr08(pmc->base.base.parent, 0x1868, 0xff);
>>>>>>
>>>>>>          if (intr) {
>>>>>>                  nv_error(pmc, "unknown intr 0x%08x\n", stat);
>>>>>> @@ -108,7 +108,7 @@ nouveau_mc_create_(struct nouveau_object *parent,
>>>>>> struct nouveau_object *engine,
>>>>>>                  if (ret) {
>>>>>>                          pmc->use_msi = false;
>>>>>>                  } else {
>>>>>> -                       nv_wr08(device, 0x00088068, 0xff);
>>>>>> +                       nv_wr08(device, 0x1868, 0xff);
>>>>>>                          nv_info(pmc, "MSI interrupts enabled\n");
>>>>>>                  }
>>>>>>          }
>>>>>>
>>>>>> I guess this needs a way of telling whether it has "for real" MSI or
>>>>>> not. That 1800 range is on NV41:NV50 according to rnndb, which
>>>>>> probably means that it's safe to use msi on nv41+ (via the 88068
>>>>>> address, since the 1800 stuff disappears on nv50+). [Based purely on
>>>>>> speculation, btw, not on hardware experimentation. I assume
>>>>>> pci_enable_msi() would implicitly fail on any non-pcie card, e.g. nv4a
>>>>>> which is an agp version of nv44, and all the pci versions of the 6200
>>>>>> (and later) cards... i think there are some 8-series pci cards too.]
>>>>> Yeah, I suspect the only case we need to explicitly detect is the
>>>>> BR02-using pretend PCIE cards.
>>>>>
>>>>> What's the pccid of your nv3x board?
>>>>
>>>> I assume that was a typo and you meant "pci id"? If so, 10de:00fd. You
>>>> can see the full info (including ids) in the quoted part above.
>>> Lucas,
>>>
>>> Would you be OK with the patch still, but blacklisting devices with
>>> 10de:00fX/10de:02eX pciids?
>>>
>>> These seem to be the two ranges that indicate that a bridge device is
>>> present (according to xf86-video-nv).
>>>
>> Ok, will put this in for a respin of the series. I personally don't care
>> too much about cards < NV50 at the moment, but if blacklisting those two
>> ranges helps to avoid any breakage it's obviously the right thing to do.
> Well, we can't just go around breaking stuff deliberately for the
> people still using them!
>
> I've blacklisted them myself and merged the patch.

Ben,

This patch causes my dual-head Quadro FX570 (G84) to fail to idle when
dragging a window around.

It loops for the full timeout (15 sec.) in nouveau_gem_ioctl_cpu_prep() --
ie., never reaches required fence sequence #.


lspci -vvv -nn

02:00.0 VGA compatible controller [0300]: NVIDIA Corporation G84 [Quadro FX 570] [10de:040e] (rev a1) (prog-if 00 [VGA controller])
	Subsystem: NVIDIA Corporation Device [10de:0474]
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 52
	Region 0: Memory at de000000 (32-bit, non-prefetchable) [size=16M]
	Region 1: Memory at c0000000 (64-bit, prefetchable) [size=256M]
	Region 3: Memory at dc000000 (64-bit, non-prefetchable) [size=32M]
	Region 5: I/O ports at cc80 [size=128]
	Expansion ROM at dfc00000 [disabled] [size=128K]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [68] MSI: Enable- Count=1/1 Maskable- 64bit+
		Address: 0000000000000000  Data: 0000
	Capabilities: [78] Express (v1) Endpoint, MSI 00
		DevCap:	MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <4us
			ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal+ Fatal+ Unsupported-
			RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
			MaxPayload 128 bytes, MaxReadReq 512 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend-
		LnkCap:	Port #8, Speed 2.5GT/s, Width x16, ASPM L0s L1, Latency L0 <512ns, L1 <4us
			ClockPM- Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM L1 Enabled; RCB 64 bytes Disabled- Retrain- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x16, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
	Capabilities: [100 v1] Virtual Channel
		Caps:	LPEVC=0 RefClk=100ns PATEntryBits=1
		Arb:	Fixed- WRR32- WRR64- WRR128-
		Ctrl:	ArbSelect=Fixed
		Status:	InProgress-
		VC0:	Caps:	PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
			Arb:	Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
			Ctrl:	Enable+ ID=0 ArbSelect=Fixed TC/VC=01
			Status:	NegoPending- InProgress-
	Capabilities: [128 v1] Power Budgeting <?>
	Capabilities: [600 v1] Vendor Specific Information: ID=0001 Rev=1 Len=024 <?>
	Kernel driver in use: nouveau
	Kernel modules: nouveau, nvidiafb

Regards,
Peter Hurley

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] drm/nouveau: use MSI interrupts
       [not found]                                           ` <5249B494.5020500-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
@ 2013-10-01 17:32                                             ` Peter Hurley
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Hurley @ 2013-10-01 17:32 UTC (permalink / raw)
  To: Ben Skeggs, Lucas Stach
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 09/30/2013 01:27 PM, Peter Hurley wrote:
> On 09/03/2013 09:45 PM, Ben Skeggs wrote:
>> Well, we can't just go around breaking stuff deliberately for the
>> people still using them!
>>
>> I've blacklisted them myself and merged the patch.
>
> Ben,
>
> This patch causes my dual-head Quadro FX570 (G84) to fail to idle when
> dragging a window around.
>
> It loops for the full timeout (15 sec.) in nouveau_gem_ioctl_cpu_prep() --
> ie., never reaches required fence sequence #.

FWIW, I can get the binary 319.32 driver to run this hardware
with MSIs on [1] and not crash in similar circumstances.

But repeating the testing on 3.12.0-rc2/3 has proved challenging.
Although I have the binary driver running on 3.12.0-rc2, the
userspace won't fully come up and I haven't figured out why
(it's an old userspace that I don't use regularly, so it could
be some other problem).

Regards,
Peter Hurley

[1] lspci -vvv -nn on 3.2.x

02:00.0 VGA compatible controller [0300]: NVIDIA Corporation G84GL [Quadro FX 570] [10de:040e] (rev a1) (prog-if 00 [VGA controller])
	Subsystem: NVIDIA Corporation Device [10de:0474]
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 94
	Region 0: Memory at de000000 (32-bit, non-prefetchable) [size=16M]
	Region 1: Memory at c0000000 (64-bit, prefetchable) [size=256M]
	Region 3: Memory at dc000000 (64-bit, non-prefetchable) [size=32M]
	Region 5: I/O ports at cc80 [size=128]
	[virtual] Expansion ROM at dfc00000 [disabled] [size=128K]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+
		Address: 00000000fee1100c  Data: 41c9
	Capabilities: [78] Express (v2) Endpoint, MSI 00
		DevCap:	MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <4us
			ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal+ Fatal+ Unsupported-
			RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
			MaxPayload 128 bytes, MaxReadReq 512 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend-
		LnkCap:	Port #8, Speed 2.5GT/s, Width x16, ASPM L0s L1, Latency L0 <512ns, L1 <4us
			ClockPM- Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM L1 Enabled; RCB 64 bytes Disabled- Retrain- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x16, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
		DevCap2: Completion Timeout: Not Supported, TimeoutDis+
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
		LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-, Selectable De-emphasis: -6dB
			 Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
			 Compliance De-emphasis: -6dB
		LnkSta2: Current De-emphasis Level: -6dB
	Capabilities: [100 v1] Virtual Channel
		Caps:	LPEVC=0 RefClk=100ns PATEntryBits=1
		Arb:	Fixed- WRR32- WRR64- WRR128-
		Ctrl:	ArbSelect=Fixed
		Status:	InProgress-
		VC0:	Caps:	PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
			Arb:	Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
			Ctrl:	Enable+ ID=0 ArbSelect=Fixed TC/VC=01
			Status:	NegoPending- InProgress-
	Capabilities: [128 v1] Power Budgeting <?>
	Capabilities: [600 v1] Vendor Specific Information: ID=0001 Rev=1 Len=024 <?>
	Kernel driver in use: nvidia
	Kernel modules: nvidia_319_updates, nouveau, nvidiafb

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2013-10-01 17:32 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28  0:00 [PATCH 0/6] Nouveau on ARM fixes Lucas Stach
2013-08-28  0:00 ` [PATCH 1/6] drm/ttm: recognize ARM arch in ioprot handler Lucas Stach
2013-08-28  0:00 ` [PATCH 2/6] drm/ttm: introduce dma cache sync helpers Lucas Stach
2013-08-28  0:00 ` [PATCH 4/6] drm/nouveau: introduce NOUVEAU_GEM_TILE_WCUS Lucas Stach
2013-08-28  7:11   ` Ben Skeggs
2013-08-28  7:39     ` Lucas Stach
2013-08-28  0:00 ` [PATCH 5/6] drm/nouveau: map IB write-combined Lucas Stach
     [not found] ` <1377648050-6649-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
2013-08-28  0:00   ` [PATCH 3/6] drm/nouveau: hook up cache sync functions Lucas Stach
2013-08-28 16:43     ` Konrad Rzeszutek Wilk
     [not found]       ` <20130828164357.GB27172-6K5HmflnPlqSPmnEAIUT9EEOCMrvLtNR@public.gmane.org>
2013-08-28 16:58         ` Lucas Stach
2013-08-28 18:21           ` Konrad Rzeszutek Wilk
2013-08-28  0:00   ` [PATCH 6/6] drm/nouveau: use MSI interrupts Lucas Stach
     [not found]     ` <1377648050-6649-7-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
2013-08-28  7:09       ` Ben Skeggs
2013-08-28  7:28         ` Lucas Stach
2013-08-28 13:54           ` Ilia Mirkin
2013-08-29  0:07             ` Ben Skeggs
     [not found]               ` <CACAvsv7Ew+0icWEq6ixdtP9Vpux4zeWjV5Lpih94YZWgs_jx4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-08-29  2:20                 ` Ilia Mirkin
2013-08-29  4:45                   ` Ben Skeggs
2013-08-29  5:00                     ` Ilia Mirkin
2013-08-29  5:07                       ` Ben Skeggs
     [not found]                         ` <CACAvsv4AZo-B3MtTh3oN946YAC=vPiR=S3TsEg7R2-hEma57tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-08-30  1:10                           ` Ilia Mirkin
2013-08-30  1:58                             ` Ben Skeggs
2013-08-30  2:00                               ` Ben Skeggs
     [not found]                               ` <CACAvsv5f3vfP1Fs41N=L8Sc_ih32_h5Vz44mjMTJ-WXH9mDe0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-08-30  2:01                                 ` Ilia Mirkin
2013-08-30  5:36                                   ` Ben Skeggs
2013-08-30  7:11                                     ` Lucas Stach
2013-09-04  1:45                                       ` Ben Skeggs
2013-09-30 17:27                                         ` [Nouveau] " Peter Hurley
     [not found]                                           ` <5249B494.5020500-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
2013-10-01 17:32                                             ` Peter Hurley
2013-08-28 16:08           ` Konrad Rzeszutek Wilk
2013-08-28  7:50 ` [PATCH 0/6] Nouveau on ARM fixes Thierry Reding
2013-08-28  8:09   ` Ben Skeggs

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.