[PATCH v3 0/8] drm/omap: OMAP_BO flags

[PATCH v3 0/8] drm/omap: OMAP_BO flags

[Jean-Jacques Hiblot]

A first version of this work had been sent by Tomi Valkeinen in may 2017 
(https://www.spinics.net/lists/dri-devel/msg140663.html).

This series adds a few new OMAP_BO flags to help the userspace manage
situations where it needs to use lots of buffers, and would currently run
out of TILER space. The TILER space is limited to mapping 128MB at any given
time and some applications might need more.

This seres is also the opportunity to do some cleanup in the flags and
improve the comments describing them.

The user-space patches for libdrm, although ready, haven't been posted yet.
It will be be done when this series have been discussed and hopefully in
the process of getting merged.

JJ

changes in v3:
- rebase on top of v5.4-rc2
- Document omap_gem_new() and the new flags using the kernel-doc format

changes in v2:
- fixed build error that crept in during rebase before sending (sorry
  about that)
- rework the refcount part a bit.

Jean-Jacques Hiblot (1):
  drm/omap: use refcount API to track the number of users of dma_addr

Tomi Valkeinen (7):
  drm/omap: add omap_gem_unpin_locked()
  drm/omap: accept NULL for dma_addr in omap_gem_pin
  drm/omap: cleanup OMAP_BO flags
  drm/omap: remove OMAP_BO_TILED define
  drm/omap: cleanup OMAP_BO_SCANOUT use
  drm/omap: add omap_gem_validate_flags()
  drm/omap: add OMAP_BO flags to affect buffer allocation

 drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |   2 +-
 drivers/gpu/drm/omapdrm/omap_fb.c         |   6 +-
 drivers/gpu/drm/omapdrm/omap_gem.c        | 191 ++++++++++++++++------
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |   2 +-
 include/uapi/drm/omap_drm.h               |  27 ++-
 5 files changed, 164 insertions(+), 64 deletions(-)

-- 
2.17.1

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

[PATCH v3 1/8] drm/omap: use refcount API to track the number of users of dma_addr

[Jean-Jacques Hiblot]

This would give us a WARN_ON() if the pin/unpin calls are unbalanced.

Proposed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 45 +++++++++++++++---------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 08f539efddfb..51ede7777083 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -67,7 +67,7 @@ struct omap_gem_object {
 	/**
 	 * # of users of dma_addr
 	 */
-	u32 dma_addr_cnt;
+	refcount_t dma_addr_cnt;
 
 	/**
 	 * If the buffer has been imported from a dmabuf the OMAP_DB_DMABUF flag
@@ -773,13 +773,15 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
 	mutex_lock(&omap_obj->lock);
 
 	if (!omap_gem_is_contiguous(omap_obj) && priv->has_dmm) {
-		if (omap_obj->dma_addr_cnt == 0) {
+		if (refcount_read(&omap_obj->dma_addr_cnt) == 0) {
 			u32 npages = obj->size >> PAGE_SHIFT;
 			enum tiler_fmt fmt = gem2fmt(omap_obj->flags);
 			struct tiler_block *block;
 
 			BUG_ON(omap_obj->block);
 
+			refcount_set(&omap_obj->dma_addr_cnt, 1);
+
 			ret = omap_gem_attach_pages(obj);
 			if (ret)
 				goto fail;
@@ -813,10 +815,10 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
 			omap_obj->block = block;
 
 			DBG("got dma address: %pad", &omap_obj->dma_addr);
+		} else {
+			refcount_inc(&omap_obj->dma_addr_cnt);
 		}
 
-		omap_obj->dma_addr_cnt++;
-
 		*dma_addr = omap_obj->dma_addr;
 	} else if (omap_gem_is_contiguous(omap_obj)) {
 		*dma_addr = omap_obj->dma_addr;
@@ -846,23 +848,21 @@ void omap_gem_unpin(struct drm_gem_object *obj)
 
 	mutex_lock(&omap_obj->lock);
 
-	if (omap_obj->dma_addr_cnt > 0) {
-		omap_obj->dma_addr_cnt--;
-		if (omap_obj->dma_addr_cnt == 0) {
-			ret = tiler_unpin(omap_obj->block);
-			if (ret) {
-				dev_err(obj->dev->dev,
-					"could not unpin pages: %d\n", ret);
-			}
-			ret = tiler_release(omap_obj->block);
-			if (ret) {
-				dev_err(obj->dev->dev,
-					"could not release unmap: %d\n", ret);
-			}
-			omap_obj->dma_addr = 0;
-			omap_obj->block = NULL;
+	if (refcount_dec_and_test(&omap_obj->dma_addr_cnt)) {
+		ret = tiler_unpin(omap_obj->block);
+		if (ret) {
+			dev_err(obj->dev->dev,
+				"could not unpin pages: %d\n", ret);
 		}
+		ret = tiler_release(omap_obj->block);
+		if (ret) {
+			dev_err(obj->dev->dev,
+				"could not release unmap: %d\n", ret);
+		}
+		omap_obj->dma_addr = 0;
+		omap_obj->block = NULL;
 	}
+}
 
 	mutex_unlock(&omap_obj->lock);
 }
@@ -879,7 +879,7 @@ int omap_gem_rotated_dma_addr(struct drm_gem_object *obj, u32 orient,
 
 	mutex_lock(&omap_obj->lock);
 
-	if ((omap_obj->dma_addr_cnt > 0) && omap_obj->block &&
+	if ((refcount_read(&omap_obj->dma_addr_cnt) > 0) && omap_obj->block &&
 			(omap_obj->flags & OMAP_BO_TILED)) {
 		*dma_addr = tiler_tsptr(omap_obj->block, orient, x, y);
 		ret = 0;
@@ -1030,7 +1030,8 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
 
 	seq_printf(m, "%08x: %2d (%2d) %08llx %pad (%2d) %p %4d",
 			omap_obj->flags, obj->name, kref_read(&obj->refcount),
-			off, &omap_obj->dma_addr, omap_obj->dma_addr_cnt,
+			off, &omap_obj->dma_addr,
+			refcount_read(&omap_obj->dma_addr_cnt),
 			omap_obj->vaddr, omap_obj->roll);
 
 	if (omap_obj->flags & OMAP_BO_TILED) {
@@ -1093,7 +1094,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
 	mutex_lock(&omap_obj->lock);
 
 	/* The object should not be pinned. */
-	WARN_ON(omap_obj->dma_addr_cnt > 0);
+	WARN_ON(refcount_read(&omap_obj->dma_addr_cnt) > 0);
 
 	if (omap_obj->pages) {
 		if (omap_obj->flags & OMAP_BO_MEM_DMABUF)
-- 
2.17.1

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

[PATCH v3 2/8] drm/omap: add omap_gem_unpin_locked()

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

Add omap_gem_unpin_locked() which is a version of omap_gem_unpin() that
expects the caller to hold the omap_obj lock.

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

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 51ede7777083..9201c21e206f 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -834,20 +834,16 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
 }
 
 /**
- * omap_gem_unpin() - Unpin a GEM object from memory
+ * omap_gem_unpin_locked() - Unpin a GEM object from memory
  * @obj: the GEM object
  *
- * Unpin the given GEM object previously pinned with omap_gem_pin(). Pins are
- * reference-counted, the actualy unpin will only be performed when the number
- * of calls to this function matches the number of calls to omap_gem_pin().
+ * omap_gem_unpin() without locking.
  */
-void omap_gem_unpin(struct drm_gem_object *obj)
+static void omap_gem_unpin_locked(struct drm_gem_object *obj)
 {
 	struct omap_gem_object *omap_obj = to_omap_bo(obj);
 	int ret;
 
-	mutex_lock(&omap_obj->lock);
-
 	if (refcount_dec_and_test(&omap_obj->dma_addr_cnt)) {
 		ret = tiler_unpin(omap_obj->block);
 		if (ret) {
@@ -864,6 +860,20 @@ void omap_gem_unpin(struct drm_gem_object *obj)
 	}
 }
 
+/**
+ * omap_gem_unpin() - Unpin a GEM object from memory
+ * @obj: the GEM object
+ *
+ * Unpin the given GEM object previously pinned with omap_gem_pin(). Pins are
+ * reference-counted, the actual unpin will only be performed when the number
+ * of calls to this function matches the number of calls to omap_gem_pin().
+ */
+void omap_gem_unpin(struct drm_gem_object *obj)
+{
+	struct omap_gem_object *omap_obj = to_omap_bo(obj);
+
+	mutex_lock(&omap_obj->lock);
+	omap_gem_unpin_locked(obj);
 	mutex_unlock(&omap_obj->lock);
 }
 
-- 
2.17.1

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

[PATCH v3 3/8] drm/omap: accept NULL for dma_addr in omap_gem_pin

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

Allow NULL to be passed in 'dma_addr' for omap_gem_pin(), in case the
caller does not need the dma_addr.

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

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 9201c21e206f..a6562d23d314 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -819,9 +819,11 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
 			refcount_inc(&omap_obj->dma_addr_cnt);
 		}
 
-		*dma_addr = omap_obj->dma_addr;
+		if (dma_addr)
+			*dma_addr = omap_obj->dma_addr;
 	} else if (omap_gem_is_contiguous(omap_obj)) {
-		*dma_addr = omap_obj->dma_addr;
+		if (dma_addr)
+			*dma_addr = omap_obj->dma_addr;
 	} else {
 		ret = -EINVAL;
 		goto fail;
-- 
2.17.1

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

[PATCH v3 4/8] drm/omap: cleanup OMAP_BO flags

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reorder OMAP_BO flags and improve the comments.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 include/uapi/drm/omap_drm.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
index 1fccffef9e27..d8ee2f840697 100644
--- a/include/uapi/drm/omap_drm.h
+++ b/include/uapi/drm/omap_drm.h
@@ -38,19 +38,20 @@ struct drm_omap_param {
 	__u64 value;			/* in (set_param), out (get_param) */
 };
 
-#define OMAP_BO_SCANOUT		0x00000001	/* scanout capable (phys contiguous) */
-#define OMAP_BO_CACHE_MASK	0x00000006	/* cache type mask, see cache modes */
-#define OMAP_BO_TILED_MASK	0x00000f00	/* tiled mapping mask, see tiled modes */
+/* Scanout buffer, consumable by DSS */
+#define OMAP_BO_SCANOUT		0x00000001
 
-/* cache modes */
-#define OMAP_BO_CACHED		0x00000000	/* default */
-#define OMAP_BO_WC		0x00000002	/* write-combine */
-#define OMAP_BO_UNCACHED	0x00000004	/* strongly-ordered (uncached) */
+/* Buffer CPU caching mode: cached, write-combining or uncached. */
+#define OMAP_BO_CACHED		0x00000000
+#define OMAP_BO_WC		0x00000002
+#define OMAP_BO_UNCACHED	0x00000004
+#define OMAP_BO_CACHE_MASK	0x00000006
 
-/* tiled modes */
+/* Use TILER for the buffer. The TILER container unit can be 8, 16 or 32 bits. */
 #define OMAP_BO_TILED_8		0x00000100
 #define OMAP_BO_TILED_16	0x00000200
 #define OMAP_BO_TILED_32	0x00000300
+#define OMAP_BO_TILED_MASK	0x00000f00
 #define OMAP_BO_TILED		(OMAP_BO_TILED_8 | OMAP_BO_TILED_16 | OMAP_BO_TILED_32)
 
 union omap_gem_size {
-- 
2.17.1

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

[PATCH v3 5/8] drm/omap: remove OMAP_BO_TILED define

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

OMAP_BO_TILED does not make sense, as OMAP_BO_TILED_* values are not
bitmasks but normal values. As we already have OMAP_BO_TILED_MASK for
the mask, we can remove OMAP_BO_TILED and use OMAP_BO_TILED_MASK
instead.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |  2 +-
 drivers/gpu/drm/omapdrm/omap_fb.c         |  6 +++---
 drivers/gpu/drm/omapdrm/omap_gem.c        | 18 +++++++++---------
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  2 +-
 include/uapi/drm/omap_drm.h               |  1 -
 5 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
index 835e6654fa82..43c1d096b021 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
@@ -113,7 +113,7 @@ extern struct platform_driver omap_dmm_driver;
 /* GEM bo flags -> tiler fmt */
 static inline enum tiler_fmt gem2fmt(u32 flags)
 {
-	switch (flags & OMAP_BO_TILED) {
+	switch (flags & OMAP_BO_TILED_MASK) {
 	case OMAP_BO_TILED_8:
 		return TILFMT_8BIT;
 	case OMAP_BO_TILED_16:
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 1b8b5108caf8..7403316088b8 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -95,7 +95,7 @@ static u32 get_linear_addr(struct drm_framebuffer *fb,
 
 bool omap_framebuffer_supports_rotation(struct drm_framebuffer *fb)
 {
-	return omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED;
+	return omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED_MASK;
 }
 
 /* Note: DRM rotates counter-clockwise, TILER & DSS rotates clockwise */
@@ -154,7 +154,7 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
 	x = state->src_x >> 16;
 	y = state->src_y >> 16;
 
-	if (omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED) {
+	if (omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED_MASK) {
 		u32 w = state->src_w >> 16;
 		u32 h = state->src_h >> 16;
 
@@ -212,7 +212,7 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
 		plane = &omap_fb->planes[1];
 
 		if (info->rotation_type == OMAP_DSS_ROT_TILER) {
-			WARN_ON(!(omap_gem_flags(fb->obj[1]) & OMAP_BO_TILED));
+			WARN_ON(!(omap_gem_flags(fb->obj[1]) & OMAP_BO_TILED_MASK));
 			omap_gem_rotated_dma_addr(fb->obj[1], orient, x/2, y/2,
 						  &info->p_uv_addr);
 		} else {
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index a6562d23d314..4e8fcfdff3a0 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -196,7 +196,7 @@ static void omap_gem_evict(struct drm_gem_object *obj)
 	struct omap_gem_object *omap_obj = to_omap_bo(obj);
 	struct omap_drm_private *priv = obj->dev->dev_private;
 
-	if (omap_obj->flags & OMAP_BO_TILED) {
+	if (omap_obj->flags & OMAP_BO_TILED_MASK) {
 		enum tiler_fmt fmt = gem2fmt(omap_obj->flags);
 		int i;
 
@@ -324,7 +324,7 @@ size_t omap_gem_mmap_size(struct drm_gem_object *obj)
 	struct omap_gem_object *omap_obj = to_omap_bo(obj);
 	size_t size = obj->size;
 
-	if (omap_obj->flags & OMAP_BO_TILED) {
+	if (omap_obj->flags & OMAP_BO_TILED_MASK) {
 		/* for tiled buffers, the virtual size has stride rounded up
 		 * to 4kb.. (to hide the fact that row n+1 might start 16kb or
 		 * 32kb later!).  But we don't back the entire buffer with
@@ -513,7 +513,7 @@ vm_fault_t omap_gem_fault(struct vm_fault *vmf)
 	 * probably trigger put_pages()?
 	 */
 
-	if (omap_obj->flags & OMAP_BO_TILED)
+	if (omap_obj->flags & OMAP_BO_TILED_MASK)
 		ret = omap_gem_fault_2d(obj, vma, vmf);
 	else
 		ret = omap_gem_fault_1d(obj, vma, vmf);
@@ -786,7 +786,7 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
 			if (ret)
 				goto fail;
 
-			if (omap_obj->flags & OMAP_BO_TILED) {
+			if (omap_obj->flags & OMAP_BO_TILED_MASK) {
 				block = tiler_reserve_2d(fmt,
 						omap_obj->width,
 						omap_obj->height, 0);
@@ -892,7 +892,7 @@ int omap_gem_rotated_dma_addr(struct drm_gem_object *obj, u32 orient,
 	mutex_lock(&omap_obj->lock);
 
 	if ((refcount_read(&omap_obj->dma_addr_cnt) > 0) && omap_obj->block &&
-			(omap_obj->flags & OMAP_BO_TILED)) {
+			(omap_obj->flags & OMAP_BO_TILED_MASK)) {
 		*dma_addr = tiler_tsptr(omap_obj->block, orient, x, y);
 		ret = 0;
 	}
@@ -907,7 +907,7 @@ int omap_gem_tiled_stride(struct drm_gem_object *obj, u32 orient)
 {
 	struct omap_gem_object *omap_obj = to_omap_bo(obj);
 	int ret = -EINVAL;
-	if (omap_obj->flags & OMAP_BO_TILED)
+	if (omap_obj->flags & OMAP_BO_TILED_MASK)
 		ret = tiler_stride(gem2fmt(omap_obj->flags), orient);
 	return ret;
 }
@@ -1046,7 +1046,7 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
 			refcount_read(&omap_obj->dma_addr_cnt),
 			omap_obj->vaddr, omap_obj->roll);
 
-	if (omap_obj->flags & OMAP_BO_TILED) {
+	if (omap_obj->flags & OMAP_BO_TILED_MASK) {
 		seq_printf(m, " %dx%d", omap_obj->width, omap_obj->height);
 		if (omap_obj->block) {
 			struct tcm_area *area = &omap_obj->block->area;
@@ -1145,7 +1145,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 	int ret;
 
 	/* Validate the flags and compute the memory and cache flags. */
-	if (flags & OMAP_BO_TILED) {
+	if (flags & OMAP_BO_TILED_MASK) {
 		if (!priv->usergart) {
 			dev_err(dev->dev, "Tiled buffers require DMM\n");
 			return NULL;
@@ -1187,7 +1187,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 	omap_obj->flags = flags;
 	mutex_init(&omap_obj->lock);
 
-	if (flags & OMAP_BO_TILED) {
+	if (flags & OMAP_BO_TILED_MASK) {
 		/*
 		 * For tiled buffers align dimensions to slot boundaries and
 		 * calculate size based on aligned dimensions.
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index e8c3ae7ac77e..7344bb61936c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -67,7 +67,7 @@ static int omap_gem_dmabuf_begin_cpu_access(struct dma_buf *buffer,
 {
 	struct drm_gem_object *obj = buffer->priv;
 	struct page **pages;
-	if (omap_gem_flags(obj) & OMAP_BO_TILED) {
+	if (omap_gem_flags(obj) & OMAP_BO_TILED_MASK) {
 		/* TODO we would need to pin at least part of the buffer to
 		 * get de-tiled view.  For now just reject it.
 		 */
diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
index d8ee2f840697..5a142fad473c 100644
--- a/include/uapi/drm/omap_drm.h
+++ b/include/uapi/drm/omap_drm.h
@@ -52,7 +52,6 @@ struct drm_omap_param {
 #define OMAP_BO_TILED_16	0x00000200
 #define OMAP_BO_TILED_32	0x00000300
 #define OMAP_BO_TILED_MASK	0x00000f00
-#define OMAP_BO_TILED		(OMAP_BO_TILED_8 | OMAP_BO_TILED_16 | OMAP_BO_TILED_32)
 
 union omap_gem_size {
 	__u32 bytes;		/* (for non-tiled formats) */
-- 
2.17.1

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

[PATCH v3 6/8] drm/omap: cleanup OMAP_BO_SCANOUT use

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

omap_gem_new() has a comment about OMAP_BO_SCANOUT which does not make
sense. Also, for the TILER case, we drop OMAP_BO_SCANOUT flag for some
reason.

It's not clear what the original purpose of OMAP_BO_SCANOUT is, but
presuming it means "scanout buffer, something that can be consumed by
DSS", this patch cleans up the above issues.

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

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 4e8fcfdff3a0..27e0a2f8508a 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1155,7 +1155,6 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 		 * Tiled buffers are always shmem paged backed. When they are
 		 * scanned out, they are remapped into DMM/TILER.
 		 */
-		flags &= ~OMAP_BO_SCANOUT;
 		flags |= OMAP_BO_MEM_SHMEM;
 
 		/*
@@ -1166,9 +1165,8 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 		flags |= tiler_get_cpu_cache_flags();
 	} else if ((flags & OMAP_BO_SCANOUT) && !priv->has_dmm) {
 		/*
-		 * OMAP_BO_SCANOUT hints that the buffer doesn't need to be
-		 * tiled. However, to lower the pressure on memory allocation,
-		 * use contiguous memory only if no TILER is available.
+		 * If we don't have DMM, we must allocate scanout buffers
+		 * from contiguous DMA memory.
 		 */
 		flags |= OMAP_BO_MEM_DMA_API;
 	} else if (!(flags & OMAP_BO_MEM_DMABUF)) {
-- 
2.17.1

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

[PATCH v3 7/8] drm/omap: add omap_gem_validate_flags()

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

Add a helper function omap_gem_validate_flags() which validates the
omap_bo flags passed from the userspace.

Also drop the dev_err() message, as the userspace can cause that at
will.

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

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 27e0a2f8508a..e518d93ca6df 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1133,6 +1133,38 @@ void omap_gem_free_object(struct drm_gem_object *obj)
 	kfree(omap_obj);
 }
 
+static bool omap_gem_validate_flags(struct drm_device *dev, u32 flags)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+
+	switch (flags & OMAP_BO_CACHE_MASK) {
+	case OMAP_BO_CACHED:
+	case OMAP_BO_WC:
+	case OMAP_BO_CACHE_MASK:
+		break;
+
+	default:
+		return false;
+	}
+
+	if (flags & OMAP_BO_TILED_MASK) {
+		if (!priv->usergart)
+			return false;
+
+		switch (flags & OMAP_BO_TILED_MASK) {
+		case OMAP_BO_TILED_8:
+		case OMAP_BO_TILED_16:
+		case OMAP_BO_TILED_32:
+			break;
+
+		default:
+			return false;
+		}
+	}
+
+	return true;
+}
+
 /* GEM buffer object constructor */
 struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 		union omap_gem_size gsize, u32 flags)
@@ -1144,13 +1176,11 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 	size_t size;
 	int ret;
 
+	if (!omap_gem_validate_flags(dev, flags))
+		return NULL;
+
 	/* Validate the flags and compute the memory and cache flags. */
 	if (flags & OMAP_BO_TILED_MASK) {
-		if (!priv->usergart) {
-			dev_err(dev->dev, "Tiled buffers require DMM\n");
-			return NULL;
-		}
-
 		/*
 		 * Tiled buffers are always shmem paged backed. When they are
 		 * scanned out, they are remapped into DMM/TILER.
-- 
2.17.1

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

[PATCH v3 8/8] drm/omap: add OMAP_BO flags to affect buffer allocation

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

On SoCs with DMM/TILER, we have two ways to allocate buffers: normal
dma_alloc or via DMM (which basically functions as an IOMMU). DMM can
map 128MB at a time, and we only map the DMM buffers when they are used
(i.e. not at alloc time). If DMM is present, omapdrm always uses DMM.

There are use cases that require lots of big buffers that are being used
at the same time by different IPs. At the moment the userspace has a
hard maximum of 128MB.

This patch adds three new flags that can be used by the userspace to
solve the situation:

OMAP_BO_MEM_CONTIG: The driver will use dma_alloc to get the memory.
This can be used to avoid DMM if the userspace knows it needs more than
128M of memory at the same time.

OMAP_BO_MEM_DMM: The driver will use DMM to get the memory. There's not
much use for this flag at the moment, as on platforms with DMM it is
used by default, but it's here for completeness.

OMAP_BO_MEM_PIN: The driver will pin the memory at alloc time, and keep
it pinned. This can be used to 1) get an error at alloc time if DMM
space is full, and 2) get rid of the constant pin/unpin operations which
may have some effect on performance.

If none of the flags are given, the behavior is the same as currently.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 54 ++++++++++++++++++++++++++++--
 include/uapi/drm/omap_drm.h        |  9 +++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index e518d93ca6df..bf18dfe2b689 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1097,6 +1097,9 @@ void omap_gem_free_object(struct drm_gem_object *obj)
 	list_del(&omap_obj->mm_list);
 	mutex_unlock(&priv->list_lock);
 
+	if (omap_obj->flags & OMAP_BO_MEM_PIN)
+		omap_gem_unpin_locked(obj);
+
 	/*
 	 * We own the sole reference to the object at this point, but to keep
 	 * lockdep happy, we must still take the omap_obj_lock to call
@@ -1147,10 +1150,19 @@ static bool omap_gem_validate_flags(struct drm_device *dev, u32 flags)
 		return false;
 	}
 
+	if ((flags & OMAP_BO_MEM_CONTIG) && (flags & OMAP_BO_MEM_DMM))
+		return false;
+
+	if ((flags & OMAP_BO_MEM_DMM) && !priv->usergart)
+		return false;
+
 	if (flags & OMAP_BO_TILED_MASK) {
 		if (!priv->usergart)
 			return false;
 
+		if (flags & OMAP_BO_MEM_CONTIG)
+			return false;
+
 		switch (flags & OMAP_BO_TILED_MASK) {
 		case OMAP_BO_TILED_8:
 		case OMAP_BO_TILED_16:
@@ -1165,7 +1177,34 @@ static bool omap_gem_validate_flags(struct drm_device *dev, u32 flags)
 	return true;
 }
 
-/* GEM buffer object constructor */
+/**
+ * omap_gem_new() - Create a new GEM buffer
+ * @dev: The DRM device
+ * @gsize: The requested size for the GEM buffer. If the buffer is tiled
+ *         (2D buffer), the size is a pair of values: height and width
+ *         expressed in pixels. If the buffers is not tiled, it is expressed
+ *         in bytes.
+ * @flags: Flags give additionnal information about the allocation:
+ *         OMAP_BO_TILED_x: use the TILER (2D buffers). The TILER container
+ *              unit can be 8, 16 or 32 bits. Cache is always disabled for
+ *              tiled buffers.
+ *         OMAP_BO_SCANOUT: Scannout buffer, consummable by the DSS
+ *         OMAP_BO_CACHED: Buffer CPU caching mode: cached
+ *         OMAP_BO_WC: Buffer CPU caching mode: write-combined
+ *         OMAP_BO_UNCACHED: Buffer CPU caching mode: uncached
+ *         OMAP_BO_MEM_CONTIG: The driver will use dma_alloc to get the memory.
+ *              This can be used to avoid DMM if the userspace knows it needs
+ *              more than 128M of memory at the same time.
+ *         OMAP_BO_MEM_DMM: The driver will use DMM to get the memory. There's
+ *              not much use for this flag at the moment, as on platforms with
+ *              DMM it is used by default, but it's here for completeness.
+ *         OMAP_BO_MEM_PIN: The driver will pin the memory at alloc time, and
+ *              keep it pinned. This can be used to 1) get an error at alloc
+ *              time if DMM space is full, and 2) get rid of the constant
+ *              pin/unpin operations which may have some effect on performance.
+ *
+ * Return: The GEM buffer or NULL if the allocation failed
+ */
 struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 		union omap_gem_size gsize, u32 flags)
 {
@@ -1193,7 +1232,8 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 		 */
 		flags &= ~(OMAP_BO_CACHED|OMAP_BO_WC|OMAP_BO_UNCACHED);
 		flags |= tiler_get_cpu_cache_flags();
-	} else if ((flags & OMAP_BO_SCANOUT) && !priv->has_dmm) {
+	} else if ((flags & OMAP_BO_MEM_CONTIG) ||
+		((flags & OMAP_BO_SCANOUT) && !priv->has_dmm)) {
 		/*
 		 * If we don't have DMM, we must allocate scanout buffers
 		 * from contiguous DMA memory.
@@ -1253,12 +1293,22 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 			goto err_release;
 	}
 
+	if (flags & OMAP_BO_MEM_PIN) {
+		ret = omap_gem_pin(obj, NULL);
+		if (ret)
+			goto err_free_dma;
+	}
+
 	mutex_lock(&priv->list_lock);
 	list_add(&omap_obj->mm_list, &priv->obj_list);
 	mutex_unlock(&priv->list_lock);
 
 	return obj;
 
+err_free_dma:
+	if (flags & OMAP_BO_MEM_DMA_API)
+		dma_free_wc(dev->dev, size, omap_obj->vaddr,
+			    omap_obj->dma_addr);
 err_release:
 	drm_gem_object_release(obj);
 err_free:
diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
index 5a142fad473c..842d3180a442 100644
--- a/include/uapi/drm/omap_drm.h
+++ b/include/uapi/drm/omap_drm.h
@@ -47,6 +47,15 @@ struct drm_omap_param {
 #define OMAP_BO_UNCACHED	0x00000004
 #define OMAP_BO_CACHE_MASK	0x00000006
 
+/* Force allocation from contiguous DMA memory */
+#define OMAP_BO_MEM_CONTIG	0x00000008
+
+/* Force allocation via DMM */
+#define OMAP_BO_MEM_DMM		0x00000010
+
+/* Pin the buffer when allocating and keep pinned */
+#define OMAP_BO_MEM_PIN		0x00000020
+
 /* Use TILER for the buffer. The TILER container unit can be 8, 16 or 32 bits. */
 #define OMAP_BO_TILED_8		0x00000100
 #define OMAP_BO_TILED_16	0x00000200
-- 
2.17.1

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

Re: [PATCH v3 0/8] drm/omap: OMAP_BO flags

[Tomi Valkeinen]

On 07/10/2019 14:25, Jean-Jacques Hiblot wrote:
> A first version of this work had been sent by Tomi Valkeinen in may 2017
> (https://www.spinics.net/lists/dri-devel/msg140663.html).
> 
> This series adds a few new OMAP_BO flags to help the userspace manage
> situations where it needs to use lots of buffers, and would currently run
> out of TILER space. The TILER space is limited to mapping 128MB at any given
> time and some applications might need more.
> 
> This seres is also the opportunity to do some cleanup in the flags and
> improve the comments describing them.
> 
> The user-space patches for libdrm, although ready, haven't been posted yet.
> It will be be done when this series have been discussed and hopefully in
> the process of getting merged.
> 
> JJ
> 
> changes in v3:
> - rebase on top of v5.4-rc2
> - Document omap_gem_new() and the new flags using the kernel-doc format
> 
> changes in v2:
> - fixed build error that crept in during rebase before sending (sorry
>    about that)
> - rework the refcount part a bit.
> 
> Jean-Jacques Hiblot (1):
>    drm/omap: use refcount API to track the number of users of dma_addr
> 
> Tomi Valkeinen (7):
>    drm/omap: add omap_gem_unpin_locked()
>    drm/omap: accept NULL for dma_addr in omap_gem_pin
>    drm/omap: cleanup OMAP_BO flags
>    drm/omap: remove OMAP_BO_TILED define
>    drm/omap: cleanup OMAP_BO_SCANOUT use
>    drm/omap: add omap_gem_validate_flags()
>    drm/omap: add OMAP_BO flags to affect buffer allocation
> 
>   drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |   2 +-
>   drivers/gpu/drm/omapdrm/omap_fb.c         |   6 +-
>   drivers/gpu/drm/omapdrm/omap_gem.c        | 191 ++++++++++++++++------
>   drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |   2 +-
>   include/uapi/drm/omap_drm.h               |  27 ++-
>   5 files changed, 164 insertions(+), 64 deletions(-)

Thanks! This looks good to me. One comment, though:

Some people may have different opinions on how to manage other people's 
patches, but here's mine:

If you have made changes to a patch from someone else (me, in this 
case), other than really trivial typo fixes or such, you should add your 
signed-off-by.

Also, if you change the patch in a way that might make it behave 
differently than the original, you should change the ownership to 
yourself, drop the original signed-off-by, and perhaps say in the desc 
that the original was written by xyz. I don't want "my" patch to cause 
kernel crashes, if it's really not my code =).

Actually, I see we now have "Co-developed-by" documented in 
Documentation/process/5.Posting.rst. That may be suitable here.

And for the patches that you didn't touch, I'm sure you've still gone 
through those, so you could add your reviewed-by.

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,2/8] drm/omap: add omap_gem_unpin_locked()

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> Add omap_gem_unpin_locked() which is a version of omap_gem_unpin() that
> expects the caller to hold the omap_obj lock.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/gpu/drm/omapdrm/omap_gem.c | 24 +++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index 51ede7777083..9201c21e206f 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -834,20 +834,16 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
>   }
>   
>   /**
> - * omap_gem_unpin() - Unpin a GEM object from memory
> + * omap_gem_unpin_locked() - Unpin a GEM object from memory
>    * @obj: the GEM object
>    *
> - * Unpin the given GEM object previously pinned with omap_gem_pin(). Pins are
> - * reference-counted, the actualy unpin will only be performed when the number
> - * of calls to this function matches the number of calls to omap_gem_pin().
> + * omap_gem_unpin() without locking.
>    */
> -void omap_gem_unpin(struct drm_gem_object *obj)
> +static void omap_gem_unpin_locked(struct drm_gem_object *obj)
>   {
>   	struct omap_gem_object *omap_obj = to_omap_bo(obj);
>   	int ret;
>   
> -	mutex_lock(&omap_obj->lock);
> -
>   	if (refcount_dec_and_test(&omap_obj->dma_addr_cnt)) {
>   		ret = tiler_unpin(omap_obj->block);
>   		if (ret) {
> @@ -864,6 +860,20 @@ void omap_gem_unpin(struct drm_gem_object *obj)
>   	}
>   }
>   
> +/**
> + * omap_gem_unpin() - Unpin a GEM object from memory
> + * @obj: the GEM object
> + *
> + * Unpin the given GEM object previously pinned with omap_gem_pin(). Pins are
> + * reference-counted, the actual unpin will only be performed when the number
> + * of calls to this function matches the number of calls to omap_gem_pin().
> + */
> +void omap_gem_unpin(struct drm_gem_object *obj)
> +{
> +	struct omap_gem_object *omap_obj = to_omap_bo(obj);
> +
> +	mutex_lock(&omap_obj->lock);
> +	omap_gem_unpin_locked(obj);
>   	mutex_unlock(&omap_obj->lock);
>   }
>   

Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

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

Re: [v3,3/8] drm/omap: accept NULL for dma_addr in omap_gem_pin

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> Allow NULL to be passed in 'dma_addr' for omap_gem_pin(), in case the
> caller does not need the dma_addr.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/gpu/drm/omapdrm/omap_gem.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index 9201c21e206f..a6562d23d314 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -819,9 +819,11 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
>   			refcount_inc(&omap_obj->dma_addr_cnt);
>   		}
>   
> -		*dma_addr = omap_obj->dma_addr;
> +		if (dma_addr)
> +			*dma_addr = omap_obj->dma_addr;
>   	} else if (omap_gem_is_contiguous(omap_obj)) {
> -		*dma_addr = omap_obj->dma_addr;
> +		if (dma_addr)
> +			*dma_addr = omap_obj->dma_addr;
>   	} else {
>   		ret = -EINVAL;
>   		goto fail;
Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,4/8] drm/omap: cleanup OMAP_BO flags

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> Reorder OMAP_BO flags and improve the comments.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   include/uapi/drm/omap_drm.h | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
> index 1fccffef9e27..d8ee2f840697 100644
> --- a/include/uapi/drm/omap_drm.h
> +++ b/include/uapi/drm/omap_drm.h
> @@ -38,19 +38,20 @@ struct drm_omap_param {
>   	__u64 value;			/* in (set_param), out (get_param) */
>   };
>   
> -#define OMAP_BO_SCANOUT		0x00000001	/* scanout capable (phys contiguous) */
> -#define OMAP_BO_CACHE_MASK	0x00000006	/* cache type mask, see cache modes */
> -#define OMAP_BO_TILED_MASK	0x00000f00	/* tiled mapping mask, see tiled modes */
> +/* Scanout buffer, consumable by DSS */
> +#define OMAP_BO_SCANOUT		0x00000001
>   
> -/* cache modes */
> -#define OMAP_BO_CACHED		0x00000000	/* default */
> -#define OMAP_BO_WC		0x00000002	/* write-combine */
> -#define OMAP_BO_UNCACHED	0x00000004	/* strongly-ordered (uncached) */
> +/* Buffer CPU caching mode: cached, write-combining or uncached. */
> +#define OMAP_BO_CACHED		0x00000000
> +#define OMAP_BO_WC		0x00000002
> +#define OMAP_BO_UNCACHED	0x00000004
> +#define OMAP_BO_CACHE_MASK	0x00000006
>   
> -/* tiled modes */
> +/* Use TILER for the buffer. The TILER container unit can be 8, 16 or 32 bits. */
>   #define OMAP_BO_TILED_8		0x00000100
>   #define OMAP_BO_TILED_16	0x00000200
>   #define OMAP_BO_TILED_32	0x00000300
> +#define OMAP_BO_TILED_MASK	0x00000f00
>   #define OMAP_BO_TILED		(OMAP_BO_TILED_8 | OMAP_BO_TILED_16 | OMAP_BO_TILED_32)
>   
>   union omap_gem_size {
Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,5/8] drm/omap: remove OMAP_BO_TILED define

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> OMAP_BO_TILED does not make sense, as OMAP_BO_TILED_* values are not
> bitmasks but normal values. As we already have OMAP_BO_TILED_MASK for
> the mask, we can remove OMAP_BO_TILED and use OMAP_BO_TILED_MASK
> instead.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |  2 +-
>   drivers/gpu/drm/omapdrm/omap_fb.c         |  6 +++---
>   drivers/gpu/drm/omapdrm/omap_gem.c        | 18 +++++++++---------
>   drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  2 +-
>   include/uapi/drm/omap_drm.h               |  1 -
>   5 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
> index 835e6654fa82..43c1d096b021 100644
> --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
> +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
> @@ -113,7 +113,7 @@ extern struct platform_driver omap_dmm_driver;
>   /* GEM bo flags -> tiler fmt */
>   static inline enum tiler_fmt gem2fmt(u32 flags)
>   {
> -	switch (flags & OMAP_BO_TILED) {
> +	switch (flags & OMAP_BO_TILED_MASK) {
>   	case OMAP_BO_TILED_8:
>   		return TILFMT_8BIT;
>   	case OMAP_BO_TILED_16:
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> index 1b8b5108caf8..7403316088b8 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -95,7 +95,7 @@ static u32 get_linear_addr(struct drm_framebuffer *fb,
>   
>   bool omap_framebuffer_supports_rotation(struct drm_framebuffer *fb)
>   {
> -	return omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED;
> +	return omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED_MASK;
>   }
>   
>   /* Note: DRM rotates counter-clockwise, TILER & DSS rotates clockwise */
> @@ -154,7 +154,7 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
>   	x = state->src_x >> 16;
>   	y = state->src_y >> 16;
>   
> -	if (omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED) {
> +	if (omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED_MASK) {
>   		u32 w = state->src_w >> 16;
>   		u32 h = state->src_h >> 16;
>   
> @@ -212,7 +212,7 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
>   		plane = &omap_fb->planes[1];
>   
>   		if (info->rotation_type == OMAP_DSS_ROT_TILER) {
> -			WARN_ON(!(omap_gem_flags(fb->obj[1]) & OMAP_BO_TILED));
> +			WARN_ON(!(omap_gem_flags(fb->obj[1]) & OMAP_BO_TILED_MASK));
>   			omap_gem_rotated_dma_addr(fb->obj[1], orient, x/2, y/2,
>   						  &info->p_uv_addr);
>   		} else {
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index a6562d23d314..4e8fcfdff3a0 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -196,7 +196,7 @@ static void omap_gem_evict(struct drm_gem_object *obj)
>   	struct omap_gem_object *omap_obj = to_omap_bo(obj);
>   	struct omap_drm_private *priv = obj->dev->dev_private;
>   
> -	if (omap_obj->flags & OMAP_BO_TILED) {
> +	if (omap_obj->flags & OMAP_BO_TILED_MASK) {
>   		enum tiler_fmt fmt = gem2fmt(omap_obj->flags);
>   		int i;
>   
> @@ -324,7 +324,7 @@ size_t omap_gem_mmap_size(struct drm_gem_object *obj)
>   	struct omap_gem_object *omap_obj = to_omap_bo(obj);
>   	size_t size = obj->size;
>   
> -	if (omap_obj->flags & OMAP_BO_TILED) {
> +	if (omap_obj->flags & OMAP_BO_TILED_MASK) {
>   		/* for tiled buffers, the virtual size has stride rounded up
>   		 * to 4kb.. (to hide the fact that row n+1 might start 16kb or
>   		 * 32kb later!).  But we don't back the entire buffer with
> @@ -513,7 +513,7 @@ vm_fault_t omap_gem_fault(struct vm_fault *vmf)
>   	 * probably trigger put_pages()?
>   	 */
>   
> -	if (omap_obj->flags & OMAP_BO_TILED)
> +	if (omap_obj->flags & OMAP_BO_TILED_MASK)
>   		ret = omap_gem_fault_2d(obj, vma, vmf);
>   	else
>   		ret = omap_gem_fault_1d(obj, vma, vmf);
> @@ -786,7 +786,7 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t *dma_addr)
>   			if (ret)
>   				goto fail;
>   
> -			if (omap_obj->flags & OMAP_BO_TILED) {
> +			if (omap_obj->flags & OMAP_BO_TILED_MASK) {
>   				block = tiler_reserve_2d(fmt,
>   						omap_obj->width,
>   						omap_obj->height, 0);
> @@ -892,7 +892,7 @@ int omap_gem_rotated_dma_addr(struct drm_gem_object *obj, u32 orient,
>   	mutex_lock(&omap_obj->lock);
>   
>   	if ((refcount_read(&omap_obj->dma_addr_cnt) > 0) && omap_obj->block &&
> -			(omap_obj->flags & OMAP_BO_TILED)) {
> +			(omap_obj->flags & OMAP_BO_TILED_MASK)) {
>   		*dma_addr = tiler_tsptr(omap_obj->block, orient, x, y);
>   		ret = 0;
>   	}
> @@ -907,7 +907,7 @@ int omap_gem_tiled_stride(struct drm_gem_object *obj, u32 orient)
>   {
>   	struct omap_gem_object *omap_obj = to_omap_bo(obj);
>   	int ret = -EINVAL;
> -	if (omap_obj->flags & OMAP_BO_TILED)
> +	if (omap_obj->flags & OMAP_BO_TILED_MASK)
>   		ret = tiler_stride(gem2fmt(omap_obj->flags), orient);
>   	return ret;
>   }
> @@ -1046,7 +1046,7 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
>   			refcount_read(&omap_obj->dma_addr_cnt),
>   			omap_obj->vaddr, omap_obj->roll);
>   
> -	if (omap_obj->flags & OMAP_BO_TILED) {
> +	if (omap_obj->flags & OMAP_BO_TILED_MASK) {
>   		seq_printf(m, " %dx%d", omap_obj->width, omap_obj->height);
>   		if (omap_obj->block) {
>   			struct tcm_area *area = &omap_obj->block->area;
> @@ -1145,7 +1145,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   	int ret;
>   
>   	/* Validate the flags and compute the memory and cache flags. */
> -	if (flags & OMAP_BO_TILED) {
> +	if (flags & OMAP_BO_TILED_MASK) {
>   		if (!priv->usergart) {
>   			dev_err(dev->dev, "Tiled buffers require DMM\n");
>   			return NULL;
> @@ -1187,7 +1187,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   	omap_obj->flags = flags;
>   	mutex_init(&omap_obj->lock);
>   
> -	if (flags & OMAP_BO_TILED) {
> +	if (flags & OMAP_BO_TILED_MASK) {
>   		/*
>   		 * For tiled buffers align dimensions to slot boundaries and
>   		 * calculate size based on aligned dimensions.
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> index e8c3ae7ac77e..7344bb61936c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> @@ -67,7 +67,7 @@ static int omap_gem_dmabuf_begin_cpu_access(struct dma_buf *buffer,
>   {
>   	struct drm_gem_object *obj = buffer->priv;
>   	struct page **pages;
> -	if (omap_gem_flags(obj) & OMAP_BO_TILED) {
> +	if (omap_gem_flags(obj) & OMAP_BO_TILED_MASK) {
>   		/* TODO we would need to pin at least part of the buffer to
>   		 * get de-tiled view.  For now just reject it.
>   		 */
> diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
> index d8ee2f840697..5a142fad473c 100644
> --- a/include/uapi/drm/omap_drm.h
> +++ b/include/uapi/drm/omap_drm.h
> @@ -52,7 +52,6 @@ struct drm_omap_param {
>   #define OMAP_BO_TILED_16	0x00000200
>   #define OMAP_BO_TILED_32	0x00000300
>   #define OMAP_BO_TILED_MASK	0x00000f00
> -#define OMAP_BO_TILED		(OMAP_BO_TILED_8 | OMAP_BO_TILED_16 | OMAP_BO_TILED_32)
>   
>   union omap_gem_size {
>   	__u32 bytes;		/* (for non-tiled formats) */
Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,6/8] drm/omap: cleanup OMAP_BO_SCANOUT use

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> omap_gem_new() has a comment about OMAP_BO_SCANOUT which does not make
> sense. Also, for the TILER case, we drop OMAP_BO_SCANOUT flag for some
> reason.
>
> It's not clear what the original purpose of OMAP_BO_SCANOUT is, but
> presuming it means "scanout buffer, something that can be consumed by
> DSS", this patch cleans up the above issues.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/gpu/drm/omapdrm/omap_gem.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index 4e8fcfdff3a0..27e0a2f8508a 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -1155,7 +1155,6 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   		 * Tiled buffers are always shmem paged backed. When they are
>   		 * scanned out, they are remapped into DMM/TILER.
>   		 */
> -		flags &= ~OMAP_BO_SCANOUT;
>   		flags |= OMAP_BO_MEM_SHMEM;
>   
>   		/*
> @@ -1166,9 +1165,8 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   		flags |= tiler_get_cpu_cache_flags();
>   	} else if ((flags & OMAP_BO_SCANOUT) && !priv->has_dmm) {
>   		/*
> -		 * OMAP_BO_SCANOUT hints that the buffer doesn't need to be
> -		 * tiled. However, to lower the pressure on memory allocation,
> -		 * use contiguous memory only if no TILER is available.
> +		 * If we don't have DMM, we must allocate scanout buffers
> +		 * from contiguous DMA memory.
>   		 */
>   		flags |= OMAP_BO_MEM_DMA_API;
>   	} else if (!(flags & OMAP_BO_MEM_DMABUF)) {
Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,7/8] drm/omap: add omap_gem_validate_flags()

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> Add a helper function omap_gem_validate_flags() which validates the
> omap_bo flags passed from the userspace.
>
> Also drop the dev_err() message, as the userspace can cause that at
> will.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/gpu/drm/omapdrm/omap_gem.c | 40 ++++++++++++++++++++++++++----
>   1 file changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index 27e0a2f8508a..e518d93ca6df 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -1133,6 +1133,38 @@ void omap_gem_free_object(struct drm_gem_object *obj)
>   	kfree(omap_obj);
>   }
>   
> +static bool omap_gem_validate_flags(struct drm_device *dev, u32 flags)
> +{
> +	struct omap_drm_private *priv = dev->dev_private;
> +
> +	switch (flags & OMAP_BO_CACHE_MASK) {
> +	case OMAP_BO_CACHED:
> +	case OMAP_BO_WC:
> +	case OMAP_BO_CACHE_MASK:
> +		break;
> +
> +	default:
> +		return false;
> +	}
> +
> +	if (flags & OMAP_BO_TILED_MASK) {
> +		if (!priv->usergart)
> +			return false;
> +
> +		switch (flags & OMAP_BO_TILED_MASK) {
> +		case OMAP_BO_TILED_8:
> +		case OMAP_BO_TILED_16:
> +		case OMAP_BO_TILED_32:
> +			break;
> +
> +		default:
> +			return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
>   /* GEM buffer object constructor */
>   struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   		union omap_gem_size gsize, u32 flags)
> @@ -1144,13 +1176,11 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   	size_t size;
>   	int ret;
>   
> +	if (!omap_gem_validate_flags(dev, flags))
> +		return NULL;
> +
>   	/* Validate the flags and compute the memory and cache flags. */
>   	if (flags & OMAP_BO_TILED_MASK) {
> -		if (!priv->usergart) {
> -			dev_err(dev->dev, "Tiled buffers require DMM\n");
> -			return NULL;
> -		}
> -
>   		/*
>   		 * Tiled buffers are always shmem paged backed. When they are
>   		 * scanned out, they are remapped into DMM/TILER.
Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,8/8] drm/omap: add OMAP_BO flags to affect buffer allocation

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> On SoCs with DMM/TILER, we have two ways to allocate buffers: normal
> dma_alloc or via DMM (which basically functions as an IOMMU). DMM can
> map 128MB at a time, and we only map the DMM buffers when they are used
> (i.e. not at alloc time). If DMM is present, omapdrm always uses DMM.
>
> There are use cases that require lots of big buffers that are being used
> at the same time by different IPs. At the moment the userspace has a
> hard maximum of 128MB.
>
> This patch adds three new flags that can be used by the userspace to
> solve the situation:
>
> OMAP_BO_MEM_CONTIG: The driver will use dma_alloc to get the memory.
> This can be used to avoid DMM if the userspace knows it needs more than
> 128M of memory at the same time.
>
> OMAP_BO_MEM_DMM: The driver will use DMM to get the memory. There's not
> much use for this flag at the moment, as on platforms with DMM it is
> used by default, but it's here for completeness.
>
> OMAP_BO_MEM_PIN: The driver will pin the memory at alloc time, and keep
> it pinned. This can be used to 1) get an error at alloc time if DMM
> space is full, and 2) get rid of the constant pin/unpin operations which
> may have some effect on performance.
>
> If none of the flags are given, the behavior is the same as currently.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/gpu/drm/omapdrm/omap_gem.c | 54 ++++++++++++++++++++++++++++--
>   include/uapi/drm/omap_drm.h        |  9 +++++
>   2 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index e518d93ca6df..bf18dfe2b689 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -1097,6 +1097,9 @@ void omap_gem_free_object(struct drm_gem_object *obj)
>   	list_del(&omap_obj->mm_list);
>   	mutex_unlock(&priv->list_lock);
>   
> +	if (omap_obj->flags & OMAP_BO_MEM_PIN)
> +		omap_gem_unpin_locked(obj);
> +
>   	/*
>   	 * We own the sole reference to the object at this point, but to keep
>   	 * lockdep happy, we must still take the omap_obj_lock to call
> @@ -1147,10 +1150,19 @@ static bool omap_gem_validate_flags(struct drm_device *dev, u32 flags)
>   		return false;
>   	}
>   
> +	if ((flags & OMAP_BO_MEM_CONTIG) && (flags & OMAP_BO_MEM_DMM))
> +		return false;
> +
> +	if ((flags & OMAP_BO_MEM_DMM) && !priv->usergart)
> +		return false;
> +
>   	if (flags & OMAP_BO_TILED_MASK) {
>   		if (!priv->usergart)
>   			return false;
>   
> +		if (flags & OMAP_BO_MEM_CONTIG)
> +			return false;
> +
>   		switch (flags & OMAP_BO_TILED_MASK) {
>   		case OMAP_BO_TILED_8:
>   		case OMAP_BO_TILED_16:
> @@ -1165,7 +1177,34 @@ static bool omap_gem_validate_flags(struct drm_device *dev, u32 flags)
>   	return true;
>   }
>   
> -/* GEM buffer object constructor */
> +/**
> + * omap_gem_new() - Create a new GEM buffer
> + * @dev: The DRM device
> + * @gsize: The requested size for the GEM buffer. If the buffer is tiled
> + *         (2D buffer), the size is a pair of values: height and width
> + *         expressed in pixels. If the buffers is not tiled, it is expressed
> + *         in bytes.
> + * @flags: Flags give additionnal information about the allocation:
> + *         OMAP_BO_TILED_x: use the TILER (2D buffers). The TILER container
> + *              unit can be 8, 16 or 32 bits. Cache is always disabled for
> + *              tiled buffers.
> + *         OMAP_BO_SCANOUT: Scannout buffer, consummable by the DSS
> + *         OMAP_BO_CACHED: Buffer CPU caching mode: cached
> + *         OMAP_BO_WC: Buffer CPU caching mode: write-combined
> + *         OMAP_BO_UNCACHED: Buffer CPU caching mode: uncached
> + *         OMAP_BO_MEM_CONTIG: The driver will use dma_alloc to get the memory.
> + *              This can be used to avoid DMM if the userspace knows it needs
> + *              more than 128M of memory at the same time.
> + *         OMAP_BO_MEM_DMM: The driver will use DMM to get the memory. There's
> + *              not much use for this flag at the moment, as on platforms with
> + *              DMM it is used by default, but it's here for completeness.
> + *         OMAP_BO_MEM_PIN: The driver will pin the memory at alloc time, and
> + *              keep it pinned. This can be used to 1) get an error at alloc
> + *              time if DMM space is full, and 2) get rid of the constant
> + *              pin/unpin operations which may have some effect on performance.
> + *
> + * Return: The GEM buffer or NULL if the allocation failed
> + */
>   struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   		union omap_gem_size gsize, u32 flags)
>   {
> @@ -1193,7 +1232,8 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   		 */
>   		flags &= ~(OMAP_BO_CACHED|OMAP_BO_WC|OMAP_BO_UNCACHED);
>   		flags |= tiler_get_cpu_cache_flags();
> -	} else if ((flags & OMAP_BO_SCANOUT) && !priv->has_dmm) {
> +	} else if ((flags & OMAP_BO_MEM_CONTIG) ||
> +		((flags & OMAP_BO_SCANOUT) && !priv->has_dmm)) {
>   		/*
>   		 * If we don't have DMM, we must allocate scanout buffers
>   		 * from contiguous DMA memory.
> @@ -1253,12 +1293,22 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>   			goto err_release;
>   	}
>   
> +	if (flags & OMAP_BO_MEM_PIN) {
> +		ret = omap_gem_pin(obj, NULL);
> +		if (ret)
> +			goto err_free_dma;
> +	}
> +
>   	mutex_lock(&priv->list_lock);
>   	list_add(&omap_obj->mm_list, &priv->obj_list);
>   	mutex_unlock(&priv->list_lock);
>   
>   	return obj;
>   
> +err_free_dma:
> +	if (flags & OMAP_BO_MEM_DMA_API)
> +		dma_free_wc(dev->dev, size, omap_obj->vaddr,
> +			    omap_obj->dma_addr);
>   err_release:
>   	drm_gem_object_release(obj);
>   err_free:
> diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
> index 5a142fad473c..842d3180a442 100644
> --- a/include/uapi/drm/omap_drm.h
> +++ b/include/uapi/drm/omap_drm.h
> @@ -47,6 +47,15 @@ struct drm_omap_param {
>   #define OMAP_BO_UNCACHED	0x00000004
>   #define OMAP_BO_CACHE_MASK	0x00000006
>   
> +/* Force allocation from contiguous DMA memory */
> +#define OMAP_BO_MEM_CONTIG	0x00000008
> +
> +/* Force allocation via DMM */
> +#define OMAP_BO_MEM_DMM		0x00000010
> +
> +/* Pin the buffer when allocating and keep pinned */
> +#define OMAP_BO_MEM_PIN		0x00000020
> +
>   /* Use TILER for the buffer. The TILER container unit can be 8, 16 or 32 bits. */
>   #define OMAP_BO_TILED_8		0x00000100
>   #define OMAP_BO_TILED_16	0x00000200

I modified the patch to add comment following the kernel-doc style, so 
my Reviewed-by actually applies only to the code modifications, not the 
comment wording.

Reviewed-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

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

Re: [PATCH v3 0/8] drm/omap: OMAP_BO flags

[Jean-Jacques Hiblot]

On 07/10/2019 14:16, Tomi Valkeinen wrote:
> On 07/10/2019 14:25, Jean-Jacques Hiblot wrote:
>> A first version of this work had been sent by Tomi Valkeinen in may 2017
>> (https://www.spinics.net/lists/dri-devel/msg140663.html).
>>
>> This series adds a few new OMAP_BO flags to help the userspace manage
>> situations where it needs to use lots of buffers, and would currently 
>> run
>> out of TILER space. The TILER space is limited to mapping 128MB at 
>> any given
>> time and some applications might need more.
>>
>> This seres is also the opportunity to do some cleanup in the flags and
>> improve the comments describing them.
>>
>> The user-space patches for libdrm, although ready, haven't been 
>> posted yet.
>> It will be be done when this series have been discussed and hopefully in
>> the process of getting merged.
>>
>> JJ
>>
>> changes in v3:
>> - rebase on top of v5.4-rc2
>> - Document omap_gem_new() and the new flags using the kernel-doc format
>>
>> changes in v2:
>> - fixed build error that crept in during rebase before sending (sorry
>>    about that)
>> - rework the refcount part a bit.
>>
>> Jean-Jacques Hiblot (1):
>>    drm/omap: use refcount API to track the number of users of dma_addr
>>
>> Tomi Valkeinen (7):
>>    drm/omap: add omap_gem_unpin_locked()
>>    drm/omap: accept NULL for dma_addr in omap_gem_pin
>>    drm/omap: cleanup OMAP_BO flags
>>    drm/omap: remove OMAP_BO_TILED define
>>    drm/omap: cleanup OMAP_BO_SCANOUT use
>>    drm/omap: add omap_gem_validate_flags()
>>    drm/omap: add OMAP_BO flags to affect buffer allocation
>>
>>   drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |   2 +-
>>   drivers/gpu/drm/omapdrm/omap_fb.c         |   6 +-
>>   drivers/gpu/drm/omapdrm/omap_gem.c        | 191 ++++++++++++++++------
>>   drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |   2 +-
>>   include/uapi/drm/omap_drm.h               |  27 ++-
>>   5 files changed, 164 insertions(+), 64 deletions(-)
>
> Thanks! This looks good to me. One comment, though:
>
> Some people may have different opinions on how to manage other 
> people's patches, but here's mine:
>
> If you have made changes to a patch from someone else (me, in this 
> case), other than really trivial typo fixes or such, you should add 
> your signed-off-by.
>
> Also, if you change the patch in a way that might make it behave 
> differently than the original, you should change the ownership to 
> yourself, drop the original signed-off-by, and perhaps say in the desc 
> that the original was written by xyz. I don't want "my" patch to cause 
> kernel crashes, if it's really not my code =).
Actually I didn't touch those patches a lot. Apart from the first one 
(the atomic stuff) and the kernel-doc style comment in the last patch, 
it is pretty much the stuff that you authored and is now part of TI's tree.
>
> Actually, I see we now have "Co-developed-by" documented in 
> Documentation/process/5.Posting.rst. That may be suitable here.
>
> And for the patches that you didn't touch, I'm sure you've still gone 
> through those, so you could add your reviewed-by.

Done.

Thanks

JJ

>
>  Tomi
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 0/8] drm/omap: OMAP_BO flags

[Tomi Valkeinen]

On 08/10/2019 19:08, Jean-Jacques Hiblot wrote:
> 
> On 07/10/2019 14:16, Tomi Valkeinen wrote:
>> On 07/10/2019 14:25, Jean-Jacques Hiblot wrote:
>>> A first version of this work had been sent by Tomi Valkeinen in may 2017
>>> (https://www.spinics.net/lists/dri-devel/msg140663.html).
>>>
>>> This series adds a few new OMAP_BO flags to help the userspace manage
>>> situations where it needs to use lots of buffers, and would currently 
>>> run
>>> out of TILER space. The TILER space is limited to mapping 128MB at 
>>> any given
>>> time and some applications might need more.
>>>
>>> This seres is also the opportunity to do some cleanup in the flags and
>>> improve the comments describing them.
>>>
>>> The user-space patches for libdrm, although ready, haven't been 
>>> posted yet.
>>> It will be be done when this series have been discussed and hopefully in
>>> the process of getting merged.
>>>
>>> JJ
>>>
>>> changes in v3:
>>> - rebase on top of v5.4-rc2
>>> - Document omap_gem_new() and the new flags using the kernel-doc format
>>>
>>> changes in v2:
>>> - fixed build error that crept in during rebase before sending (sorry
>>>    about that)
>>> - rework the refcount part a bit.
>>>
>>> Jean-Jacques Hiblot (1):
>>>    drm/omap: use refcount API to track the number of users of dma_addr
>>>
>>> Tomi Valkeinen (7):
>>>    drm/omap: add omap_gem_unpin_locked()
>>>    drm/omap: accept NULL for dma_addr in omap_gem_pin
>>>    drm/omap: cleanup OMAP_BO flags
>>>    drm/omap: remove OMAP_BO_TILED define
>>>    drm/omap: cleanup OMAP_BO_SCANOUT use
>>>    drm/omap: add omap_gem_validate_flags()
>>>    drm/omap: add OMAP_BO flags to affect buffer allocation
>>>
>>>   drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |   2 +-
>>>   drivers/gpu/drm/omapdrm/omap_fb.c         |   6 +-
>>>   drivers/gpu/drm/omapdrm/omap_gem.c        | 191 ++++++++++++++++------
>>>   drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |   2 +-
>>>   include/uapi/drm/omap_drm.h               |  27 ++-
>>>   5 files changed, 164 insertions(+), 64 deletions(-)
>>
>> Thanks! This looks good to me. One comment, though:
>>
>> Some people may have different opinions on how to manage other 
>> people's patches, but here's mine:
>>
>> If you have made changes to a patch from someone else (me, in this 
>> case), other than really trivial typo fixes or such, you should add 
>> your signed-off-by.
>>
>> Also, if you change the patch in a way that might make it behave 
>> differently than the original, you should change the ownership to 
>> yourself, drop the original signed-off-by, and perhaps say in the desc 
>> that the original was written by xyz. I don't want "my" patch to cause 
>> kernel crashes, if it's really not my code =).
> Actually I didn't touch those patches a lot. Apart from the first one 
> (the atomic stuff) and the kernel-doc style comment in the last patch, 
> it is pretty much the stuff that you authored and is now part of TI's tree.
>>
>> Actually, I see we now have "Co-developed-by" documented in 
>> Documentation/process/5.Posting.rst. That may be suitable here.
>>
>> And for the patches that you didn't touch, I'm sure you've still gone 
>> through those, so you could add your reviewed-by.
> 
> Done.

Thanks! I'll push these to drm-misc-next.

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/8] drm/omap: use refcount API to track the number of users of dma_addr

[Tomi Valkeinen]

Hi JJ,

On 07/10/2019 14:25, Jean-Jacques Hiblot wrote:
> This would give us a WARN_ON() if the pin/unpin calls are unbalanced.
> 
> Proposed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---

This doesn't compile, but it gets fixed in some later patch in the series.

Can you fix this, and add your reviewed-bys, and resend one more time.

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel