[PATCH 0/8] drm/tegra: Fixes for v3.19-rc1

[PATCH 0/8] drm/tegra: Fixes for v3.19-rc1

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Hi,

This is a set of fixes for two regressions and one bug in the IOMMU
mapping code. It turns out that all of these issues turn up primarily
on Tegra30 hardware. The IOMMU mapping bug only manifests on buffers
that aren't multiples of the page size. I happened to be testing HDMI
with 1080p while writing the code and framebuffers for that happen to
fit exactly within 2025 pages of 4 KiB each.

One of the regressions is caused by the IOMMU code allocating pages from
shmem which can have associated cache lines. If the pages aren't flushed
then these cache lines may be flushed later on and cause framebuffer
corruption. I'm not sure why I didn't see this before. Perhaps the board
that I was using had enough RAM so that the pages shmem would hand out
had a better chance of being unused. Or maybe I didn't look too closely.
The fix for this, implementing drm_clflush_*() for ARM, has also been
tested by Rob. The long-term plan is to make architectures expose an API
to flush pages, but for now drm_clflush_*() provides exactly what we
need.

The second regression is caused by a mismatch between the hardware pipe
number and the CRTC's DRM index. These were used inconsistently, which
could cause one code location to call drm_vblank_get() with a different
pipe than the corresponding drm_vblank_put(), thereby causing the
reference count to become unbalanced. Alexandre also reported a possible
race condition related to this, which this series also fixes.

I'm hoping to get reviews on this, especially the drm/irq and drm/cache
patches, quickly so that I can send a pull request to Dave, hopefully to
get this included, given the size, in v3.19-rc1 still.

Thierry

Thierry Reding (8):
  drm/irq: Add drm_crtc_send_vblank_event()
  drm/irq: Add drm_crtc_handle_vblank()
  drm/irq: Add drm_crtc_vblank_count()
  drm/tegra: dc: Consistently use the same pipe
  drm/tegra: dc: Fix a potential race on page-flip completion
  drm/cache: Implement drm_clflush_*() for ARM
  drm/tegra: gem: Flush buffer objects upon allocation
  drm/tegra: gem: Use the proper size for GEM objects

 drivers/gpu/drm/drm_cache.c | 45 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_irq.c   | 60 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tegra/dc.c  | 24 ++++++++++--------
 drivers/gpu/drm/tegra/drm.c | 16 +++++++-----
 drivers/gpu/drm/tegra/gem.c | 16 ++++++------
 include/drm/drmP.h          |  4 +++
 6 files changed, 142 insertions(+), 23 deletions(-)

-- 
2.1.3

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

[PATCH 1/8] drm/irq: Add drm_crtc_send_vblank_event()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This function is the KMS native variant of drm_send_vblank_event(). It
takes a struct drm_crtc * instead of a struct drm_device * and an index
of the CRTC.

Eventually the goal is to access vblank data through the CRTC only so
that the per-CRTC data can be moved to struct drm_crtc.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_irq.c | 19 +++++++++++++++++++
 include/drm/drmP.h        |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 920cdb91e9d0..795cff890114 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -904,6 +904,8 @@ static void send_vblank_event(struct drm_device *dev,
  *
  * Updates sequence # and timestamp on event, and sends it to userspace.
  * Caller must hold event lock.
+ *
+ * This is the legacy version of drm_crtc_send_vblank_event().
  */
 void drm_send_vblank_event(struct drm_device *dev, int crtc,
 		struct drm_pending_vblank_event *e)
@@ -924,6 +926,23 @@ void drm_send_vblank_event(struct drm_device *dev, int crtc,
 EXPORT_SYMBOL(drm_send_vblank_event);
 
 /**
+ * drm_crtc_send_vblank_event - helper to send vblank event after pageflip
+ * @crtc: the source CRTC of the vblank event
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ * Caller must hold event lock.
+ *
+ * This is the native KMS version of drm_send_vblank_event().
+ */
+void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
+				struct drm_pending_vblank_event *e)
+{
+	drm_send_vblank_event(crtc->dev, drm_crtc_index(crtc), e);
+}
+EXPORT_SYMBOL(drm_crtc_send_vblank_event);
+
+/**
  * drm_vblank_enable - enable the vblank interrupt on a CRTC
  * @dev: DRM device
  * @crtc: CRTC in question
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8ba35c622e22..b78601bb7c46 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -905,6 +905,8 @@ extern u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc,
 				     struct timeval *vblanktime);
 extern void drm_send_vblank_event(struct drm_device *dev, int crtc,
 				     struct drm_pending_vblank_event *e);
+extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
+				       struct drm_pending_vblank_event *e);
 extern bool drm_handle_vblank(struct drm_device *dev, int crtc);
 extern int drm_vblank_get(struct drm_device *dev, int crtc);
 extern void drm_vblank_put(struct drm_device *dev, int crtc);
-- 
2.1.3

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

[PATCH 2/8] drm/irq: Add drm_crtc_handle_vblank()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This function is the KMS native variant of drm_handle_vblank(). It takes
a struct drm_crtc * instead of a struct drm_device * and an index of the
CRTC.

Eventually the goal is to access vblank data through the CRTC only so
that the per-CRTC data can be moved to struct drm_crtc.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_irq.c | 20 ++++++++++++++++++++
 include/drm/drmP.h        |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 795cff890114..f7f1b579526e 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1614,6 +1614,8 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
  *
  * Drivers should call this routine in their vblank interrupt handlers to
  * update the vblank counter and send any signals that may be pending.
+ *
+ * This is the legacy version of drm_crtc_handle_vblank().
  */
 bool drm_handle_vblank(struct drm_device *dev, int crtc)
 {
@@ -1690,3 +1692,21 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
 	return true;
 }
 EXPORT_SYMBOL(drm_handle_vblank);
+
+/**
+ * drm_crtc_handle_vblank - handle a vblank event
+ * @crtc: where this event occurred
+ *
+ * Drivers should call this routine in their vblank interrupt handlers to
+ * update the vblank counter and send any signals that may be pending.
+ *
+ * This is the native KMS version of drm_handle_vblank().
+ *
+ * Returns:
+ * True if the event was successfully handled, false on failure.
+ */
+bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
+{
+	return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
+}
+EXPORT_SYMBOL(drm_crtc_handle_vblank);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index b78601bb7c46..f1f7f15ce0f3 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -908,6 +908,7 @@ extern void drm_send_vblank_event(struct drm_device *dev, int crtc,
 extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 				       struct drm_pending_vblank_event *e);
 extern bool drm_handle_vblank(struct drm_device *dev, int crtc);
+extern bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
 extern int drm_vblank_get(struct drm_device *dev, int crtc);
 extern void drm_vblank_put(struct drm_device *dev, int crtc);
 extern int drm_crtc_vblank_get(struct drm_crtc *crtc);
-- 
2.1.3

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

[PATCH 3/8] drm/irq: Add drm_crtc_vblank_count()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This function is the KMS native variant of drm_vblank_count(). It takes
a struct drm_crtc * instead of a struct drm_device * and an index of the
CRTC.

Eventually the goal is to access vblank data through the CRTC only so
that the per-CRTC data can be moved to struct drm_crtc.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_irq.c | 21 +++++++++++++++++++++
 include/drm/drmP.h        |  1 +
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index f7f1b579526e..75647e7f012b 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -830,6 +830,8 @@ drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
  * vblank events since the system was booted, including lost events due to
  * modesetting activity.
  *
+ * This is the legacy version of drm_crtc_vblank_count().
+ *
  * Returns:
  * The software vblank counter.
  */
@@ -844,6 +846,25 @@ u32 drm_vblank_count(struct drm_device *dev, int crtc)
 EXPORT_SYMBOL(drm_vblank_count);
 
 /**
+ * drm_crtc_vblank_count - retrieve "cooked" vblank counter value
+ * @crtc: which counter to retrieve
+ *
+ * Fetches the "cooked" vblank count value that represents the number of
+ * vblank events since the system was booted, including lost events due to
+ * modesetting activity.
+ *
+ * This is the native KMS version of drm_vblank_count().
+ *
+ * Returns:
+ * The software vblank counter.
+ */
+u32 drm_crtc_vblank_count(struct drm_crtc *crtc)
+{
+	return drm_vblank_count(crtc->dev, drm_crtc_index(crtc));
+}
+EXPORT_SYMBOL(drm_crtc_vblank_count);
+
+/**
  * drm_vblank_count_and_time - retrieve "cooked" vblank counter value
  * and the system timestamp corresponding to that vblank counter value.
  *
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index f1f7f15ce0f3..e1b2e8b98af7 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -901,6 +901,7 @@ extern int drm_vblank_init(struct drm_device *dev, int num_crtcs);
 extern int drm_wait_vblank(struct drm_device *dev, void *data,
 			   struct drm_file *filp);
 extern u32 drm_vblank_count(struct drm_device *dev, int crtc);
+extern u32 drm_crtc_vblank_count(struct drm_crtc *crtc);
 extern u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc,
 				     struct timeval *vblanktime);
 extern void drm_send_vblank_event(struct drm_device *dev, int crtc,
-- 
2.1.3

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

[PATCH 4/8] drm/tegra: dc: Consistently use the same pipe

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The hardware pipe numbers don't always match the DRM CRTC indices. This
can happen for example if the first display controller defers probe,
causing it to be registered with DRM after the second display
controller. When that happens the hardware pipe numbers and DRM CRTC
indices become different. Make sure that the CRTC index is always used
when accessing per-CRTC VBLANK data. This can be ensured by using the
drm_crtc_vblank_*() API, which will do the right thing automatically
given a struct drm_crtc *.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dc.c  | 14 +++++++-------
 drivers/gpu/drm/tegra/drm.c | 16 ++++++++++------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 7fe7bb1227cf..4c6a8a828475 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -826,8 +826,8 @@ static void tegra_dc_finish_page_flip(struct tegra_dc *dc)
 
 	if (base == bo->paddr + crtc->primary->fb->offsets[0]) {
 		spin_lock_irqsave(&drm->event_lock, flags);
-		drm_send_vblank_event(drm, dc->pipe, dc->event);
-		drm_vblank_put(drm, dc->pipe);
+		drm_crtc_send_vblank_event(crtc, dc->event);
+		drm_crtc_vblank_put(crtc);
 		dc->event = NULL;
 		spin_unlock_irqrestore(&drm->event_lock, flags);
 	}
@@ -843,7 +843,7 @@ void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
 
 	if (dc->event && dc->event->base.file_priv == file) {
 		dc->event->base.destroy(&dc->event->base);
-		drm_vblank_put(drm, dc->pipe);
+		drm_crtc_vblank_put(crtc);
 		dc->event = NULL;
 	}
 
@@ -853,16 +853,16 @@ void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
 static int tegra_dc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 			      struct drm_pending_vblank_event *event, uint32_t page_flip_flags)
 {
+	unsigned int pipe = drm_crtc_index(crtc);
 	struct tegra_dc *dc = to_tegra_dc(crtc);
-	struct drm_device *drm = crtc->dev;
 
 	if (dc->event)
 		return -EBUSY;
 
 	if (event) {
-		event->pipe = dc->pipe;
+		event->pipe = pipe;
 		dc->event = event;
-		drm_vblank_get(drm, dc->pipe);
+		drm_crtc_vblank_get(crtc);
 	}
 
 	tegra_dc_set_base(dc, 0, 0, fb);
@@ -1122,7 +1122,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
 		/*
 		dev_dbg(dc->dev, "%s(): vertical blank\n", __func__);
 		*/
-		drm_handle_vblank(dc->base.dev, dc->pipe);
+		drm_crtc_handle_vblank(&dc->base);
 		tegra_dc_finish_page_flip(dc);
 	}
 
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index e549afeece1f..d4f827593dfa 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -694,24 +694,28 @@ static const struct file_operations tegra_drm_fops = {
 	.llseek = noop_llseek,
 };
 
-static struct drm_crtc *tegra_crtc_from_pipe(struct drm_device *drm, int pipe)
+static struct drm_crtc *tegra_crtc_from_pipe(struct drm_device *drm,
+					     unsigned int pipe)
 {
 	struct drm_crtc *crtc;
 
 	list_for_each_entry(crtc, &drm->mode_config.crtc_list, head) {
-		struct tegra_dc *dc = to_tegra_dc(crtc);
-
-		if (dc->pipe == pipe)
+		if (pipe == drm_crtc_index(crtc))
 			return crtc;
 	}
 
 	return NULL;
 }
 
-static u32 tegra_drm_get_vblank_counter(struct drm_device *dev, int crtc)
+static u32 tegra_drm_get_vblank_counter(struct drm_device *drm, int pipe)
 {
+	struct drm_crtc *crtc = tegra_crtc_from_pipe(drm, pipe);
+
+	if (!crtc)
+		return 0;
+
 	/* TODO: implement real hardware counter using syncpoints */
-	return drm_vblank_count(dev, crtc);
+	return drm_crtc_vblank_count(crtc);
 }
 
 static int tegra_drm_enable_vblank(struct drm_device *drm, int pipe)
-- 
2.1.3

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

[PATCH 5/8] drm/tegra: dc: Fix a potential race on page-flip completion

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Page-flip completion could race with page-flip submission, so extend the
critical section to include all accesses to page-flip related data.

Reported-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 4c6a8a828475..28040f7e4a43 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -814,8 +814,12 @@ static void tegra_dc_finish_page_flip(struct tegra_dc *dc)
 	unsigned long flags, base;
 	struct tegra_bo *bo;
 
-	if (!dc->event)
+	spin_lock_irqsave(&drm->event_lock, flags);
+
+	if (!dc->event) {
+		spin_unlock_irqrestore(&drm->event_lock, flags);
 		return;
+	}
 
 	bo = tegra_fb_get_plane(crtc->primary->fb, 0);
 
@@ -825,12 +829,12 @@ static void tegra_dc_finish_page_flip(struct tegra_dc *dc)
 	tegra_dc_writel(dc, 0, DC_CMD_STATE_ACCESS);
 
 	if (base == bo->paddr + crtc->primary->fb->offsets[0]) {
-		spin_lock_irqsave(&drm->event_lock, flags);
 		drm_crtc_send_vblank_event(crtc, dc->event);
 		drm_crtc_vblank_put(crtc);
 		dc->event = NULL;
-		spin_unlock_irqrestore(&drm->event_lock, flags);
 	}
+
+	spin_unlock_irqrestore(&drm->event_lock, flags);
 }
 
 void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
-- 
2.1.3

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

[PATCH 6/8] drm/cache: Implement drm_clflush_*() for ARM

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add implementations for drm_clflush_*() on ARM by borrowing code from
the DMA mapping API implementation. Unfortunately ARM doesn't export an
API to flush caches on a page by page basis, so this replicates most of
the code.

Reviewed--by: Rob Clark <robdclark@gmail.com>
Tested-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_cache.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index a6b690626a6b..fca0b8994c77 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -72,6 +72,41 @@ drm_clflush_ipi_handler(void *null)
 }
 #endif
 
+#if defined(CONFIG_ARM)
+
+#include <asm/cacheflush.h>
+#include <asm/cachetype.h>
+#include <asm/highmem.h>
+#include <asm/outercache.h>
+
+static void drm_clflush_page(struct page *page)
+{
+	enum dma_data_direction dir = DMA_TO_DEVICE;
+	phys_addr_t phys = page_to_phys(page);
+	size_t size = PAGE_SIZE;
+	void *virt;
+
+	if (PageHighMem(page)) {
+		if (cache_is_vipt_nonaliasing()) {
+			virt = kmap_atomic(page);
+			dmac_map_area(virt, size, dir);
+			kunmap_atomic(virt);
+		} else {
+			virt = kmap_high_get(page);
+			if (virt) {
+				dmac_map_area(virt, size, dir);
+				kunmap_high(page);
+			}
+		}
+	} else {
+		virt = page_address(page);
+		dmac_map_area(virt, size, dir);
+	}
+
+	outer_flush_range(phys, phys + PAGE_SIZE);
+}
+#endif
+
 void
 drm_clflush_pages(struct page *pages[], unsigned long num_pages)
 {
@@ -99,6 +134,11 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages)
 				   (unsigned long)page_virtual + PAGE_SIZE);
 		kunmap_atomic(page_virtual);
 	}
+#elif defined(CONFIG_ARM)
+	unsigned long i;
+
+	for (i = 0; i < num_pages; i++)
+		drm_clflush_page(pages[i]);
 #else
 	printk(KERN_ERR "Architecture has no drm_cache.c support\n");
 	WARN_ON_ONCE(1);
@@ -123,6 +163,11 @@ drm_clflush_sg(struct sg_table *st)
 
 	if (on_each_cpu(drm_clflush_ipi_handler, NULL, 1) != 0)
 		printk(KERN_ERR "Timed out waiting for cache flush.\n");
+#elif defined(CONFIG_ARM)
+	struct sg_page_iter sg_iter;
+
+	for_each_sg_page(st->sgl, &sg_iter, st->nents, 0)
+		drm_clflush_page(sg_page_iter_page(&sg_iter));
 #else
 	printk(KERN_ERR "Architecture has no drm_cache.c support\n");
 	WARN_ON_ONCE(1);
-- 
2.1.3

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

[PATCH 7/8] drm/tegra: gem: Flush buffer objects upon allocation

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Buffers obtained via shmem may still have associated cachelines. If they
aren't properly flushed they may cause framebuffer corruption if the
cache gets flushed after the application has drawn to it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index da32086cbeaf..9a12adf708db 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -231,6 +231,8 @@ static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo,
 		return PTR_ERR(bo->sgt);
 	}
 
+	drm_clflush_sg(bo->sgt);
+
 	return 0;
 }
 
-- 
2.1.3

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

[PATCH 8/8] drm/tegra: gem: Use the proper size for GEM objects

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If the requested buffer size wasn't a multiple of the page size, the
IOMMU code would round down the size to the next multiple of the page
size, thereby causing translation errors. To fix this we no longer pass
around the requested size but reuse the computed size of the GEM object.
This is already rounded to the next page boundary, so mapping that size
works out fine.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 9a12adf708db..676d9a9f8d70 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -216,14 +216,13 @@ static void tegra_bo_free(struct drm_device *drm, struct tegra_bo *bo)
 	}
 }
 
-static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo,
-			      size_t size)
+static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo)
 {
 	bo->pages = drm_gem_get_pages(&bo->gem);
 	if (IS_ERR(bo->pages))
 		return PTR_ERR(bo->pages);
 
-	bo->num_pages = size >> PAGE_SHIFT;
+	bo->num_pages = bo->gem.size >> PAGE_SHIFT;
 
 	bo->sgt = drm_prime_pages_to_sg(bo->pages, bo->num_pages);
 	if (IS_ERR(bo->sgt)) {
@@ -236,14 +235,13 @@ static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo,
 	return 0;
 }
 
-static int tegra_bo_alloc(struct drm_device *drm, struct tegra_bo *bo,
-			  size_t size)
+static int tegra_bo_alloc(struct drm_device *drm, struct tegra_bo *bo)
 {
 	struct tegra_drm *tegra = drm->dev_private;
 	int err;
 
 	if (tegra->domain) {
-		err = tegra_bo_get_pages(drm, bo, size);
+		err = tegra_bo_get_pages(drm, bo);
 		if (err < 0)
 			return err;
 
@@ -253,6 +251,8 @@ static int tegra_bo_alloc(struct drm_device *drm, struct tegra_bo *bo,
 			return err;
 		}
 	} else {
+		size_t size = bo->gem.size;
+
 		bo->vaddr = dma_alloc_writecombine(drm->dev, size, &bo->paddr,
 						   GFP_KERNEL | __GFP_NOWARN);
 		if (!bo->vaddr) {
@@ -276,7 +276,7 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, size_t size,
 	if (IS_ERR(bo))
 		return bo;
 
-	err = tegra_bo_alloc(drm, bo, size);
+	err = tegra_bo_alloc(drm, bo);
 	if (err < 0)
 		goto release;
 
-- 
2.1.3

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

Re: [PATCH 0/8] drm/tegra: Fixes for v3.19-rc1

[Sean Paul]

On Tue, Dec 16, 2014 at 11:15 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Hi,
>
> This is a set of fixes for two regressions and one bug in the IOMMU
> mapping code. It turns out that all of these issues turn up primarily
> on Tegra30 hardware. The IOMMU mapping bug only manifests on buffers
> that aren't multiples of the page size. I happened to be testing HDMI
> with 1080p while writing the code and framebuffers for that happen to
> fit exactly within 2025 pages of 4 KiB each.
>
> One of the regressions is caused by the IOMMU code allocating pages from
> shmem which can have associated cache lines. If the pages aren't flushed
> then these cache lines may be flushed later on and cause framebuffer
> corruption. I'm not sure why I didn't see this before. Perhaps the board
> that I was using had enough RAM so that the pages shmem would hand out
> had a better chance of being unused. Or maybe I didn't look too closely.
> The fix for this, implementing drm_clflush_*() for ARM, has also been
> tested by Rob. The long-term plan is to make architectures expose an API
> to flush pages, but for now drm_clflush_*() provides exactly what we
> need.
>
> The second regression is caused by a mismatch between the hardware pipe
> number and the CRTC's DRM index. These were used inconsistently, which
> could cause one code location to call drm_vblank_get() with a different
> pipe than the corresponding drm_vblank_put(), thereby causing the
> reference count to become unbalanced. Alexandre also reported a possible
> race condition related to this, which this series also fixes.
>
> I'm hoping to get reviews on this, especially the drm/irq and drm/cache
> patches, quickly so that I can send a pull request to Dave, hopefully to
> get this included, given the size, in v3.19-rc1 still.
>

Hi Thierry,
Have you made a determination on "drm/tegra: Select root window for
event dispatch"? The bug it fixes makes concurrent flips and overlay
updates impossible.

Sean


> Thierry
>
> Thierry Reding (8):
>   drm/irq: Add drm_crtc_send_vblank_event()
>   drm/irq: Add drm_crtc_handle_vblank()
>   drm/irq: Add drm_crtc_vblank_count()
>   drm/tegra: dc: Consistently use the same pipe
>   drm/tegra: dc: Fix a potential race on page-flip completion
>   drm/cache: Implement drm_clflush_*() for ARM
>   drm/tegra: gem: Flush buffer objects upon allocation
>   drm/tegra: gem: Use the proper size for GEM objects
>
>  drivers/gpu/drm/drm_cache.c | 45 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_irq.c   | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tegra/dc.c  | 24 ++++++++++--------
>  drivers/gpu/drm/tegra/drm.c | 16 +++++++-----
>  drivers/gpu/drm/tegra/gem.c | 16 ++++++------
>  include/drm/drmP.h          |  4 +++
>  6 files changed, 142 insertions(+), 23 deletions(-)
>
> --
> 2.1.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/8] drm/irq: Add drm_crtc_vblank_count()

[Daniel Vetter]

On Tue, Dec 16, 2014 at 05:15:10PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> This function is the KMS native variant of drm_vblank_count(). It takes
> a struct drm_crtc * instead of a struct drm_device * and an index of the
> CRTC.
> 
> Eventually the goal is to access vblank data through the CRTC only so
> that the per-CRTC data can be moved to struct drm_crtc.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

On the first 3 patches:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> ---
>  drivers/gpu/drm/drm_irq.c | 21 +++++++++++++++++++++
>  include/drm/drmP.h        |  1 +
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index f7f1b579526e..75647e7f012b 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -830,6 +830,8 @@ drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
>   * vblank events since the system was booted, including lost events due to
>   * modesetting activity.
>   *
> + * This is the legacy version of drm_crtc_vblank_count().
> + *
>   * Returns:
>   * The software vblank counter.
>   */
> @@ -844,6 +846,25 @@ u32 drm_vblank_count(struct drm_device *dev, int crtc)
>  EXPORT_SYMBOL(drm_vblank_count);
>  
>  /**
> + * drm_crtc_vblank_count - retrieve "cooked" vblank counter value
> + * @crtc: which counter to retrieve
> + *
> + * Fetches the "cooked" vblank count value that represents the number of
> + * vblank events since the system was booted, including lost events due to
> + * modesetting activity.
> + *
> + * This is the native KMS version of drm_vblank_count().
> + *
> + * Returns:
> + * The software vblank counter.
> + */
> +u32 drm_crtc_vblank_count(struct drm_crtc *crtc)
> +{
> +	return drm_vblank_count(crtc->dev, drm_crtc_index(crtc));
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_count);
> +
> +/**
>   * drm_vblank_count_and_time - retrieve "cooked" vblank counter value
>   * and the system timestamp corresponding to that vblank counter value.
>   *
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index f1f7f15ce0f3..e1b2e8b98af7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -901,6 +901,7 @@ extern int drm_vblank_init(struct drm_device *dev, int num_crtcs);
>  extern int drm_wait_vblank(struct drm_device *dev, void *data,
>  			   struct drm_file *filp);
>  extern u32 drm_vblank_count(struct drm_device *dev, int crtc);
> +extern u32 drm_crtc_vblank_count(struct drm_crtc *crtc);
>  extern u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc,
>  				     struct timeval *vblanktime);
>  extern void drm_send_vblank_event(struct drm_device *dev, int crtc,
> -- 
> 2.1.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/8] drm/tegra: dc: Fix a potential race on page-flip completion

[Alexandre Courbot]

On Wed, Dec 17, 2014 at 1:15 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Page-flip completion could race with page-flip submission, so extend the
> critical section to include all accesses to page-flip related data.
>
> Reported-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Thanks, now I can finally do ovenight testing! :)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel