dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] drm/omap: misc fixes
@ 2014-09-24 13:11 Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 1/9] drm/omap: fix encoder-crtc mapping Tomi Valkeinen
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

Hi,

This is a modified version of the series I sent earlier
(http://comments.gmane.org/gmane.comp.video.dri.devel/113812). I haven't had
time to work on the locking issues, so I've dropped the patches related to that
so that the rest could get merged.

I have also added three new small patches (the three last ones).

 Tomi

Tomi Valkeinen (9):
  drm/omap: fix encoder-crtc mapping
  drm/omap: page_flip: return -EBUSY if flip pending
  drm/omap: fix race issue with vsync irq and apply
  drm/omap: clear omap_obj->paddr in omap_gem_put_paddr()
  drm/omap: add pin refcounting to omap_framebuffer
  drm/omap: add a comment why locking is missing
  drm/omap: fix operation without fbdev
  drm/omap: fix error handling in omap_framebuffer_create()
  drm/omap: handle mismatching color format and buffer width

 drivers/gpu/drm/omapdrm/omap_crtc.c  | 19 ++++++++++++++++---
 drivers/gpu/drm/omapdrm/omap_drv.c   | 18 ++++++++++--------
 drivers/gpu/drm/omapdrm/omap_fb.c    | 26 +++++++++++++++++++++++---
 drivers/gpu/drm/omapdrm/omap_gem.c   |  1 +
 drivers/gpu/drm/omapdrm/omap_plane.c |  4 ++++
 5 files changed, 54 insertions(+), 14 deletions(-)

-- 
1.9.1

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

* [PATCH 1/9] drm/omap: fix encoder-crtc mapping
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 2/9] drm/omap: page_flip: return -EBUSY if flip pending Tomi Valkeinen
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

OMAP DSS hardware supports changing the output port to which an overlay
manager's video stream goes. For example, DPI video stream can come from
any of the four overlay managers on OMAP5.

However, as it's difficult to manage the change in the driver, the
omapdss driver does not support that at the moment, and has a hardcoded
overlay manager per output.

omapdrm, on the other hand, uses the hardware features to find out which
overlay manager to use for an output, which causes problems. For
example, on OMAP5, omapdrm tries to use DIGIT overlay manager for DPI
output, instead of the LCD3 required by the omapdss driver.

This patch changes the omapdrm to use the omapdss driver's hardcoded
overlay managers, which fixes the issue.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 002b9721e85a..26fda74c1e48 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -286,14 +286,13 @@ static int omap_modeset_init(struct drm_device *dev)
 		for (id = 0; id < priv->num_crtcs; id++) {
 			struct drm_crtc *crtc = priv->crtcs[id];
 			enum omap_channel crtc_channel;
-			enum omap_dss_output_id supported_outputs;
 
 			crtc_channel = omap_crtc_channel(crtc);
-			supported_outputs =
-				dss_feat_get_supported_outputs(crtc_channel);
 
-			if (supported_outputs & output->id)
+			if (output->dispc_channel == crtc_channel) {
 				encoder->possible_crtcs |= (1 << id);
+				break;
+			}
 		}
 
 		omap_dss_put_device(output);
-- 
1.9.1

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

* [PATCH 2/9] drm/omap: page_flip: return -EBUSY if flip pending
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 1/9] drm/omap: fix encoder-crtc mapping Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 3/9] drm/omap: fix race issue with vsync irq and apply Tomi Valkeinen
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

The DRM documentation says:

"If a page flip is already pending, the page_flip operation must return
-EBUSY."

Currently omapdrm returns -EINVAL instead. Fix omapdrm by returning
-EBUSY.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 2d28dc337cfb..0812b0f80167 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -360,7 +360,7 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
 	if (omap_crtc->old_fb) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		dev_err(dev->dev, "already a pending flip\n");
-		return -EINVAL;
+		return -EBUSY;
 	}
 
 	omap_crtc->event = event;
-- 
1.9.1

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

* [PATCH 3/9] drm/omap: fix race issue with vsync irq and apply
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 1/9] drm/omap: fix encoder-crtc mapping Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 2/9] drm/omap: page_flip: return -EBUSY if flip pending Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 4/9] drm/omap: clear omap_obj->paddr in omap_gem_put_paddr() Tomi Valkeinen
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

omap_crtc's apply_worker does:

	omap_irq_register(dev, &omap_crtc->apply_irq);
	dispc_mgr_go(channel);

This is supposed to enable the vsync irq, and set the GO bit. The vsync
handler will later check if the HW has cleared the GO bit and queue
apply work.

However, what may happen is that the vsync irq is enabled, and it gets
ran before the GO bit is set by the apply_worker. In this case the vsync
handler will see the GO bit as cleared, and queues the apply work too
early.

This causes WARN'ings from dispc driver, and possibly other problems.

This patch fixes the issue by adding a new variable, 'go_bit_set' which
is used to track the supposed state of GO bit and helps the apply worker
and irq handler handle the job without a race.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 0812b0f80167..193979f97bdb 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -47,6 +47,9 @@ struct omap_crtc {
 	bool enabled;
 	bool full_update;
 
+	/* tracks the state of GO bit between irq handler and apply worker */
+	bool go_bit_set;
+
 	struct omap_drm_apply apply;
 
 	struct omap_drm_irq apply_irq;
@@ -442,11 +445,16 @@ static void omap_crtc_apply_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	if (!omap_crtc->error_irq.registered)
 		__omap_irq_register(crtc->dev, &omap_crtc->error_irq);
 
-	if (!dispc_mgr_go_busy(omap_crtc->channel)) {
+	/* make sure we see the most recent 'go_bit_set' */
+	rmb();
+	if (omap_crtc->go_bit_set && !dispc_mgr_go_busy(omap_crtc->channel)) {
 		struct omap_drm_private *priv =
 				crtc->dev->dev_private;
 		DBG("%s: apply done", omap_crtc->name);
 		__omap_irq_unregister(crtc->dev, &omap_crtc->apply_irq);
+		omap_crtc->go_bit_set = false;
+		/* make sure apple_worker sees 'go_bit_set = false' */
+		wmb();
 		queue_work(priv->wq, &omap_crtc->apply_work);
 	}
 }
@@ -472,7 +480,9 @@ static void apply_worker(struct work_struct *work)
 	 * If we are still pending a previous update, wait.. when the
 	 * pending update completes, we get kicked again.
 	 */
-	if (omap_crtc->apply_irq.registered)
+	/* make sure we see the most recent 'go_bit_set' */
+	rmb();
+	if (omap_crtc->go_bit_set)
 		goto out;
 
 	/* finish up previous apply's: */
@@ -502,6 +512,9 @@ static void apply_worker(struct work_struct *work)
 		if (dispc_mgr_is_enabled(channel)) {
 			omap_irq_register(dev, &omap_crtc->apply_irq);
 			dispc_mgr_go(channel);
+			omap_crtc->go_bit_set = true;
+			/* make sure the irq handler sees 'go_bit_set' */
+			wmb();
 		} else {
 			struct omap_drm_private *priv = dev->dev_private;
 			queue_work(priv->wq, &omap_crtc->apply_work);
-- 
1.9.1

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

* [PATCH 4/9] drm/omap: clear omap_obj->paddr in omap_gem_put_paddr()
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
                   ` (2 preceding siblings ...)
  2014-09-24 13:11 ` [PATCH 3/9] drm/omap: fix race issue with vsync irq and apply Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 5/9] drm/omap: add pin refcounting to omap_framebuffer Tomi Valkeinen
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

Clear omap_obj's paddr when unmapping the memory, so that it's easier to
catch bad use of the paddr.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index e4849413ee80..b342e7b3bf00 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -829,6 +829,7 @@ int omap_gem_put_paddr(struct drm_gem_object *obj)
 				dev_err(obj->dev->dev,
 					"could not release unmap: %d\n", ret);
 			}
+			omap_obj->paddr = 0;
 			omap_obj->block = NULL;
 		}
 	}
-- 
1.9.1

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

* [PATCH 5/9] drm/omap: add pin refcounting to omap_framebuffer
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
                   ` (3 preceding siblings ...)
  2014-09-24 13:11 ` [PATCH 4/9] drm/omap: clear omap_obj->paddr in omap_gem_put_paddr() Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 6/9] drm/omap: add a comment why locking is missing Tomi Valkeinen
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

omap_framebuffer_pin() and omap_framebuffer_unpin() are currently
broken, as they cannot be called multiple times (i.e. pin, pin, unpin,
unpin), which is what happens in certain cases. This issue causes the
driver to possibly use 0 as an address for a displayed buffer, leading
to OCP error from DSS.

This patch fixes the issue by adding a simple pin_count, used to track
the number of pins.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_fb.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 2a5cacdc344b..58f2af32ede8 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -86,6 +86,7 @@ struct plane {
 
 struct omap_framebuffer {
 	struct drm_framebuffer base;
+	int pin_count;
 	const struct format *format;
 	struct plane planes[4];
 };
@@ -261,6 +262,11 @@ int omap_framebuffer_pin(struct drm_framebuffer *fb)
 	struct omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
 	int ret, i, n = drm_format_num_planes(fb->pixel_format);
 
+	if (omap_fb->pin_count > 0) {
+		omap_fb->pin_count++;
+		return 0;
+	}
+
 	for (i = 0; i < n; i++) {
 		struct plane *plane = &omap_fb->planes[i];
 		ret = omap_gem_get_paddr(plane->bo, &plane->paddr, true);
@@ -269,6 +275,8 @@ int omap_framebuffer_pin(struct drm_framebuffer *fb)
 		omap_gem_dma_sync(plane->bo, DMA_TO_DEVICE);
 	}
 
+	omap_fb->pin_count++;
+
 	return 0;
 
 fail:
@@ -287,6 +295,11 @@ int omap_framebuffer_unpin(struct drm_framebuffer *fb)
 	struct omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
 	int ret, i, n = drm_format_num_planes(fb->pixel_format);
 
+	omap_fb->pin_count--;
+
+	if (omap_fb->pin_count > 0)
+		return 0;
+
 	for (i = 0; i < n; i++) {
 		struct plane *plane = &omap_fb->planes[i];
 		ret = omap_gem_put_paddr(plane->bo);
-- 
1.9.1

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

* [PATCH 6/9] drm/omap: add a comment why locking is missing
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
                   ` (4 preceding siblings ...)
  2014-09-24 13:11 ` [PATCH 5/9] drm/omap: add pin refcounting to omap_framebuffer Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 7/9] drm/omap: fix operation without fbdev Tomi Valkeinen
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

unpin_worker() calls omap_framebuffer_unpin() without any locks, which
looks very suspicious. However, both pin and unpin are always called via
the driver's private workqueue, so the access is synchronized that way.

Add a comment to make this clear.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 891a4dc608af..743d04981d71 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -71,6 +71,10 @@ static void unpin_worker(struct drm_flip_work *work, void *val)
 			container_of(work, struct omap_plane, unpin_work);
 	struct drm_device *dev = omap_plane->base.dev;
 
+	/*
+	 * omap_framebuffer_pin/unpin are always called from priv->wq,
+	 * so there's no need for locking here.
+	 */
 	omap_framebuffer_unpin(val);
 	mutex_lock(&dev->mode_config.mutex);
 	drm_framebuffer_unreference(val);
-- 
1.9.1

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

* [PATCH 7/9] drm/omap: fix operation without fbdev
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
                   ` (5 preceding siblings ...)
  2014-09-24 13:11 ` [PATCH 6/9] drm/omap: add a comment why locking is missing Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 8/9] drm/omap: fix error handling in omap_framebuffer_create() Tomi Valkeinen
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

omapdrm should work fine even if fbdev is missing. The current driver
crashes in that case, though, as it is missing checks for the fbdev.

Add the checks so that we don't free fbdev or restore fbdev mode when
there's no fbdev.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 26fda74c1e48..65d665ff965f 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -518,7 +518,8 @@ static int dev_unload(struct drm_device *dev)
 
 	drm_kms_helper_poll_fini(dev);
 
-	omap_fbdev_free(dev);
+	if (priv->fbdev)
+		omap_fbdev_free(dev);
 
 	/* flush crtcs so the fbs get released */
 	for (i = 0; i < priv->num_crtcs; i++)
@@ -587,9 +588,11 @@ static void dev_lastclose(struct drm_device *dev)
 		}
 	}
 
-	ret = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
-	if (ret)
-		DBG("failed to restore crtc mode");
+	if (priv->fbdev) {
+		ret = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
+		if (ret)
+			DBG("failed to restore crtc mode");
+	}
 }
 
 static void dev_preclose(struct drm_device *dev, struct drm_file *file)
-- 
1.9.1

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

* [PATCH 8/9] drm/omap: fix error handling in omap_framebuffer_create()
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
                   ` (6 preceding siblings ...)
  2014-09-24 13:11 ` [PATCH 7/9] drm/omap: fix operation without fbdev Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:11 ` [PATCH 9/9] drm/omap: handle mismatching color format and buffer width Tomi Valkeinen
  2014-09-24 13:48 ` [PATCH 10/10] drm/omap: fix TILER on OMAP5 Tomi Valkeinen
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

When an error happens in omap_framebuffer_create(),
omap_framebuffer_create() calls omap_framebuffer_destroy() if the fb
struct has been allocated. However, that crashes, as
omap_framebuffer_destroy(), which calls drm_framebuffer_cleanup(),
should only be called after drm_framebuffer_init()

Fix this by just calling kfree() for the allocated fb when an error
happens.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_fb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 58f2af32ede8..2975096abdf5 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -420,7 +420,7 @@ struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
 struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 		struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos)
 {
-	struct omap_framebuffer *omap_fb;
+	struct omap_framebuffer *omap_fb = NULL;
 	struct drm_framebuffer *fb = NULL;
 	const struct format *format = NULL;
 	int ret, i, n = drm_format_num_planes(mode_cmd->pixel_format);
@@ -491,8 +491,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 	return fb;
 
 fail:
-	if (fb)
-		omap_framebuffer_destroy(fb);
+	kfree(omap_fb);
 
 	return ERR_PTR(ret);
 }
-- 
1.9.1

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

* [PATCH 9/9] drm/omap: handle mismatching color format and buffer width
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
                   ` (7 preceding siblings ...)
  2014-09-24 13:11 ` [PATCH 8/9] drm/omap: fix error handling in omap_framebuffer_create() Tomi Valkeinen
@ 2014-09-24 13:11 ` Tomi Valkeinen
  2014-09-24 13:48 ` [PATCH 10/10] drm/omap: fix TILER on OMAP5 Tomi Valkeinen
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:11 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

omapdrm doesn't check if the width of the framebuffer and the color
format's bits-per-pixel match.

For example, using a display with a width of 1280, and a buffer
allocated with using 32 bits per pixel (i.e. 1280*4 = 5120 bytes), with
a 24 bits per pixel color format, leads to the following mismatch:
5120/3 = 1706.666... bytes. This causes bad colors and a tilt on the
screen.

Add a check into omapdrm to return an error if the user tries to use
such a combination.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_fb.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 2975096abdf5..bf98580223d0 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -463,6 +463,14 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 			goto fail;
 		}
 
+		if (mode_cmd->width % format->planes[i].stride_bpp != 0) {
+			dev_err(dev->dev,
+				"buffer width (%d) is not a multiple of pixel width (%d)\n",
+				mode_cmd->width, format->planes[i].stride_bpp);
+			ret = -EINVAL;
+			goto fail;
+		}
+
 		size = pitch * mode_cmd->height / format->planes[i].sub_y;
 
 		if (size > (omap_gem_mmap_size(bos[i]) - mode_cmd->offsets[i])) {
-- 
1.9.1

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

* [PATCH 10/10] drm/omap: fix TILER on OMAP5
  2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
                   ` (8 preceding siblings ...)
  2014-09-24 13:11 ` [PATCH 9/9] drm/omap: handle mismatching color format and buffer width Tomi Valkeinen
@ 2014-09-24 13:48 ` Tomi Valkeinen
  9 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2014-09-24 13:48 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: Tomi Valkeinen

On OMAP5 it is not possible to use TILER buffer with CPU when caching or
write-combining is used. Doing so leads to errors from the memory
manager.

However, on OMAP4, write-combining works fine.

This patch adds platform specific data for the TILER, and a function
tiler_get_cpu_cache_flags() which can be used to get the caching mode to
be used.

Note that without write-combining the use of the TILER buffer with CPU
is unusably slow. It's still good to have it operational for testing
purposes.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---

One more patch, which was left out from the series.

 Tomi

 drivers/gpu/drm/omapdrm/omap_dmm_priv.h  |  6 +++++
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 39 ++++++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.h |  1 +
 drivers/gpu/drm/omapdrm/omap_gem.c       |  4 ++--
 4 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
index 58bcd6ae0255..d96660573076 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
@@ -153,6 +153,10 @@ struct refill_engine {
 	struct list_head idle_node;
 };
 
+struct dmm_platform_data {
+	uint32_t cpu_cache_flags;
+};
+
 struct dmm {
 	struct device *dev;
 	void __iomem *base;
@@ -183,6 +187,8 @@ struct dmm {
 
 	/* allocation list and lock */
 	struct list_head alloc_head;
+
+	const struct dmm_platform_data *plat_data;
 };
 
 #endif
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 56c60552abba..ef3d14d9cfa1 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -39,6 +39,10 @@
 static struct tcm *containers[TILFMT_NFORMATS];
 static struct dmm *omap_dmm;
 
+#if defined(CONFIG_OF)
+static const struct of_device_id dmm_of_match[];
+#endif
+
 /* global spinlock for protecting lists */
 static DEFINE_SPINLOCK(list_lock);
 
@@ -529,6 +533,11 @@ size_t tiler_vsize(enum tiler_fmt fmt, uint16_t w, uint16_t h)
 	return round_up(geom[fmt].cpp * w, PAGE_SIZE) * h;
 }
 
+uint32_t tiler_get_cpu_cache_flags(void)
+{
+	return omap_dmm->plat_data->cpu_cache_flags;
+}
+
 bool dmm_is_available(void)
 {
 	return omap_dmm ? true : false;
@@ -592,6 +601,18 @@ static int omap_dmm_probe(struct platform_device *dev)
 
 	init_waitqueue_head(&omap_dmm->engine_queue);
 
+	if (dev->dev.of_node) {
+		const struct of_device_id *match;
+
+		match = of_match_node(dmm_of_match, dev->dev.of_node);
+		if (!match) {
+			dev_err(&dev->dev, "failed to find matching device node\n");
+			return -ENODEV;
+		}
+
+		omap_dmm->plat_data = match->data;
+	}
+
 	/* lookup hwmod data - base address and irq */
 	mem = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	if (!mem) {
@@ -972,9 +993,23 @@ static const struct dev_pm_ops omap_dmm_pm_ops = {
 #endif
 
 #if defined(CONFIG_OF)
+static const struct dmm_platform_data dmm_omap4_platform_data = {
+	.cpu_cache_flags = OMAP_BO_WC,
+};
+
+static const struct dmm_platform_data dmm_omap5_platform_data = {
+	.cpu_cache_flags = OMAP_BO_UNCACHED,
+};
+
 static const struct of_device_id dmm_of_match[] = {
-	{ .compatible = "ti,omap4-dmm", },
-	{ .compatible = "ti,omap5-dmm", },
+	{
+		.compatible = "ti,omap4-dmm",
+		.data = &dmm_omap4_platform_data,
+	},
+	{
+		.compatible = "ti,omap5-dmm",
+		.data = &dmm_omap5_platform_data,
+	},
 	{},
 };
 #endif
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
index 4fdd61e54bd2..e83c78372db8 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
@@ -106,6 +106,7 @@ uint32_t tiler_stride(enum tiler_fmt fmt, uint32_t orient);
 size_t tiler_size(enum tiler_fmt fmt, uint16_t w, uint16_t h);
 size_t tiler_vsize(enum tiler_fmt fmt, uint16_t w, uint16_t h);
 void tiler_align(enum tiler_fmt fmt, uint16_t *w, uint16_t *h);
+uint32_t tiler_get_cpu_cache_flags(void);
 bool dmm_is_available(void);
 
 extern struct platform_driver omap_dmm_driver;
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index b342e7b3bf00..3b5cbce70def 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1360,8 +1360,8 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 		/* currently don't allow cached buffers.. there is some caching
 		 * stuff that needs to be handled better
 		 */
-		flags &= ~(OMAP_BO_CACHED|OMAP_BO_UNCACHED);
-		flags |= OMAP_BO_WC;
+		flags &= ~(OMAP_BO_CACHED|OMAP_BO_WC|OMAP_BO_UNCACHED);
+		flags |= tiler_get_cpu_cache_flags();
 
 		/* align dimensions to slot boundaries... */
 		tiler_align(gem2fmt(flags),
-- 
1.9.1

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

end of thread, other threads:[~2014-09-24 13:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-24 13:11 [PATCH 0/9] drm/omap: misc fixes Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 1/9] drm/omap: fix encoder-crtc mapping Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 2/9] drm/omap: page_flip: return -EBUSY if flip pending Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 3/9] drm/omap: fix race issue with vsync irq and apply Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 4/9] drm/omap: clear omap_obj->paddr in omap_gem_put_paddr() Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 5/9] drm/omap: add pin refcounting to omap_framebuffer Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 6/9] drm/omap: add a comment why locking is missing Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 7/9] drm/omap: fix operation without fbdev Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 8/9] drm/omap: fix error handling in omap_framebuffer_create() Tomi Valkeinen
2014-09-24 13:11 ` [PATCH 9/9] drm/omap: handle mismatching color format and buffer width Tomi Valkeinen
2014-09-24 13:48 ` [PATCH 10/10] drm/omap: fix TILER on OMAP5 Tomi Valkeinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).