[PATCH 1/4] drm/tegra: Implement more tiling modes

[PATCH 1/4] drm/tegra: Implement more tiling modes

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Tegra124 supports a block-linear mode in addition to the regular pitch
linear and tiled modes. Add support for these by moving the internal
representation into a structure rather than a simple flag.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/dc.c  | 102 +++++++++++++++++++++++++++++++++++++-------
 drivers/gpu/drm/tegra/dc.h  |   5 +++
 drivers/gpu/drm/tegra/drm.h |   8 +++-
 drivers/gpu/drm/tegra/fb.c  |   9 ++--
 drivers/gpu/drm/tegra/gem.c |   2 +-
 drivers/gpu/drm/tegra/gem.h |  16 ++++++-
 6 files changed, 118 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index ef40381f3909..afcca04f5367 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -18,6 +18,7 @@
 struct tegra_dc_soc_info {
 	bool supports_interlacing;
 	bool supports_cursor;
+	bool supports_block_linear;
 };
 
 struct tegra_plane {
@@ -212,15 +213,44 @@ static int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 	tegra_dc_writel(dc, h_offset, DC_WINBUF_ADDR_H_OFFSET);
 	tegra_dc_writel(dc, v_offset, DC_WINBUF_ADDR_V_OFFSET);
 
-	if (window->tiled) {
-		value = DC_WIN_BUFFER_ADDR_MODE_TILE_UV |
-			DC_WIN_BUFFER_ADDR_MODE_TILE;
+	if (dc->soc->supports_block_linear) {
+		unsigned long height = window->tiling.value;
+
+		switch (window->tiling.mode) {
+		case TEGRA_BO_TILING_MODE_PITCH:
+			value = DC_WINBUF_SURFACE_KIND_PITCH;
+			break;
+
+		case TEGRA_BO_TILING_MODE_TILED:
+			value = DC_WINBUF_SURFACE_KIND_TILED;
+			break;
+
+		case TEGRA_BO_TILING_MODE_BLOCK:
+			value = DC_WINBUF_SURFACE_KIND_BLOCK_HEIGHT(height) |
+				DC_WINBUF_SURFACE_KIND_BLOCK;
+			break;
+		}
+
+		tegra_dc_writel(dc, value, DC_WINBUF_SURFACE_KIND);
 	} else {
-		value = DC_WIN_BUFFER_ADDR_MODE_LINEAR_UV |
-			DC_WIN_BUFFER_ADDR_MODE_LINEAR;
-	}
+		switch (window->tiling.mode) {
+		case TEGRA_BO_TILING_MODE_PITCH:
+			value = DC_WIN_BUFFER_ADDR_MODE_LINEAR_UV |
+				DC_WIN_BUFFER_ADDR_MODE_LINEAR;
+			break;
 
-	tegra_dc_writel(dc, value, DC_WIN_BUFFER_ADDR_MODE);
+		case TEGRA_BO_TILING_MODE_TILED:
+			value = DC_WIN_BUFFER_ADDR_MODE_TILE_UV |
+				DC_WIN_BUFFER_ADDR_MODE_TILE;
+			break;
+
+		case TEGRA_BO_TILING_MODE_BLOCK:
+			DRM_ERROR("hardware doesn't support block linear mode\n");
+			return -EINVAL;
+		}
+
+		tegra_dc_writel(dc, value, DC_WIN_BUFFER_ADDR_MODE);
+	}
 
 	value = WIN_ENABLE;
 
@@ -288,6 +318,7 @@ static int tegra_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
 	struct tegra_dc *dc = to_tegra_dc(crtc);
 	struct tegra_dc_window window;
 	unsigned int i;
+	int err;
 
 	memset(&window, 0, sizeof(window));
 	window.src.x = src_x >> 16;
@@ -301,7 +332,10 @@ static int tegra_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
 	window.format = tegra_dc_format(fb->pixel_format, &window.swap);
 	window.bits_per_pixel = fb->bits_per_pixel;
 	window.bottom_up = tegra_fb_is_bottom_up(fb);
-	window.tiled = tegra_fb_is_tiled(fb);
+
+	err = tegra_fb_get_tiling(fb, &window.tiling);
+	if (err < 0)
+		return err;
 
 	for (i = 0; i < drm_format_num_planes(fb->pixel_format); i++) {
 		struct tegra_bo *bo = tegra_fb_get_plane(fb, i);
@@ -402,8 +436,14 @@ static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
 {
 	struct tegra_bo *bo = tegra_fb_get_plane(fb, 0);
 	unsigned int h_offset = 0, v_offset = 0;
+	struct tegra_bo_tiling tiling;
 	unsigned int format, swap;
 	unsigned long value;
+	int err;
+
+	err = tegra_fb_get_tiling(fb, &tiling);
+	if (err < 0)
+		return err;
 
 	tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER);
 
@@ -417,15 +457,44 @@ static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
 	tegra_dc_writel(dc, format, DC_WIN_COLOR_DEPTH);
 	tegra_dc_writel(dc, swap, DC_WIN_BYTE_SWAP);
 
-	if (tegra_fb_is_tiled(fb)) {
-		value = DC_WIN_BUFFER_ADDR_MODE_TILE_UV |
-			DC_WIN_BUFFER_ADDR_MODE_TILE;
+	if (dc->soc->supports_block_linear) {
+		unsigned long height = tiling.value;
+
+		switch (tiling.mode) {
+		case TEGRA_BO_TILING_MODE_PITCH:
+			value = DC_WINBUF_SURFACE_KIND_PITCH;
+			break;
+
+		case TEGRA_BO_TILING_MODE_TILED:
+			value = DC_WINBUF_SURFACE_KIND_TILED;
+			break;
+
+		case TEGRA_BO_TILING_MODE_BLOCK:
+			value = DC_WINBUF_SURFACE_KIND_BLOCK_HEIGHT(height) |
+				DC_WINBUF_SURFACE_KIND_BLOCK;
+			break;
+		}
+
+		tegra_dc_writel(dc, value, DC_WINBUF_SURFACE_KIND);
 	} else {
-		value = DC_WIN_BUFFER_ADDR_MODE_LINEAR_UV |
-			DC_WIN_BUFFER_ADDR_MODE_LINEAR;
-	}
+		switch (tiling.mode) {
+		case TEGRA_BO_TILING_MODE_PITCH:
+			value = DC_WIN_BUFFER_ADDR_MODE_LINEAR_UV |
+				DC_WIN_BUFFER_ADDR_MODE_LINEAR;
+			break;
 
-	tegra_dc_writel(dc, value, DC_WIN_BUFFER_ADDR_MODE);
+		case TEGRA_BO_TILING_MODE_TILED:
+			value = DC_WIN_BUFFER_ADDR_MODE_TILE_UV |
+				DC_WIN_BUFFER_ADDR_MODE_TILE;
+			break;
+
+		case TEGRA_BO_TILING_MODE_BLOCK:
+			DRM_ERROR("hardware doesn't support block linear mode\n");
+			return -EINVAL;
+		}
+
+		tegra_dc_writel(dc, value, DC_WIN_BUFFER_ADDR_MODE);
+	}
 
 	/* make sure bottom-up buffers are properly displayed */
 	if (tegra_fb_is_bottom_up(fb)) {
@@ -1277,16 +1346,19 @@ static const struct host1x_client_ops dc_client_ops = {
 static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.supports_interlacing = false,
 	.supports_cursor = false,
+	.supports_block_linear = false,
 };
 
 static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.supports_interlacing = false,
 	.supports_cursor = false,
+	.supports_block_linear = false,
 };
 
 static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
 	.supports_interlacing = true,
 	.supports_cursor = true,
+	.supports_block_linear = true,
 };
 
 static const struct of_device_id tegra_dc_of_match[] = {
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 78c5feff95d2..705c93b00794 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -428,6 +428,11 @@
 #define DC_WINBUF_ADDR_V_OFFSET_NS		0x809
 
 #define DC_WINBUF_UFLOW_STATUS			0x80a
+#define DC_WINBUF_SURFACE_KIND			0x80b
+#define DC_WINBUF_SURFACE_KIND_PITCH	(0 << 0)
+#define DC_WINBUF_SURFACE_KIND_TILED	(1 << 0)
+#define DC_WINBUF_SURFACE_KIND_BLOCK	(2 << 0)
+#define DC_WINBUF_SURFACE_KIND_BLOCK_HEIGHT(x) (((x) & 0x7) << 4)
 
 #define DC_WINBUF_AD_UFLOW_STATUS		0xbca
 #define DC_WINBUF_BD_UFLOW_STATUS		0xdca
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 11f4049cebe3..10bd45c26abf 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -19,6 +19,8 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fixed.h>
 
+#include "gem.h"
+
 struct reset_control;
 
 struct tegra_fb {
@@ -160,7 +162,8 @@ struct tegra_dc_window {
 	unsigned int stride[2];
 	unsigned long base[3];
 	bool bottom_up;
-	bool tiled;
+
+	struct tegra_bo_tiling tiling;
 };
 
 /* from dc.c */
@@ -280,7 +283,8 @@ int tegra_dpaux_train(struct tegra_dpaux *dpaux, struct drm_dp_link *link,
 struct tegra_bo *tegra_fb_get_plane(struct drm_framebuffer *framebuffer,
 				    unsigned int index);
 bool tegra_fb_is_bottom_up(struct drm_framebuffer *framebuffer);
-bool tegra_fb_is_tiled(struct drm_framebuffer *framebuffer);
+int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
+			struct tegra_bo_tiling *tiling);
 int tegra_drm_fb_prepare(struct drm_device *drm);
 int tegra_drm_fb_init(struct drm_device *drm);
 void tegra_drm_fb_exit(struct drm_device *drm);
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index fc1528e0bda1..7790d43ad082 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -46,14 +46,15 @@ bool tegra_fb_is_bottom_up(struct drm_framebuffer *framebuffer)
 	return false;
 }
 
-bool tegra_fb_is_tiled(struct drm_framebuffer *framebuffer)
+int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
+			struct tegra_bo_tiling *tiling)
 {
 	struct tegra_fb *fb = to_tegra_fb(framebuffer);
 
-	if (fb->planes[0]->flags & TEGRA_BO_TILED)
-		return true;
+	/* TODO: handle YUV formats? */
+	*tiling = fb->planes[0]->tiling;
 
-	return false;
+	return 0;
 }
 
 static void tegra_fb_destroy(struct drm_framebuffer *framebuffer)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 78cc8143760a..103acd36c67e 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -126,7 +126,7 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size,
 		goto err_mmap;
 
 	if (flags & DRM_TEGRA_GEM_CREATE_TILED)
-		bo->flags |= TEGRA_BO_TILED;
+		bo->tiling.mode = TEGRA_BO_TILING_MODE_TILED;
 
 	if (flags & DRM_TEGRA_GEM_CREATE_BOTTOM_UP)
 		bo->flags |= TEGRA_BO_BOTTOM_UP;
diff --git a/drivers/gpu/drm/tegra/gem.h b/drivers/gpu/drm/tegra/gem.h
index 2f3fe96c5154..43a25c853357 100644
--- a/drivers/gpu/drm/tegra/gem.h
+++ b/drivers/gpu/drm/tegra/gem.h
@@ -16,8 +16,18 @@
 #include <drm/drm.h>
 #include <drm/drmP.h>
 
-#define TEGRA_BO_TILED     (1 << 0)
-#define TEGRA_BO_BOTTOM_UP (1 << 1)
+#define TEGRA_BO_BOTTOM_UP (1 << 0)
+
+enum tegra_bo_tiling_mode {
+	TEGRA_BO_TILING_MODE_PITCH,
+	TEGRA_BO_TILING_MODE_TILED,
+	TEGRA_BO_TILING_MODE_BLOCK,
+};
+
+struct tegra_bo_tiling {
+	enum tegra_bo_tiling_mode mode;
+	unsigned long value;
+};
 
 struct tegra_bo {
 	struct drm_gem_object gem;
@@ -26,6 +36,8 @@ struct tegra_bo {
 	struct sg_table *sgt;
 	dma_addr_t paddr;
 	void *vaddr;
+
+	struct tegra_bo_tiling tiling;
 };
 
 static inline struct tegra_bo *to_tegra_bo(struct drm_gem_object *gem)
-- 
1.9.2

[PATCH 2/4] drm/tegra: Add SET/GET_TILING IOCTLs

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Currently the tiling parameters of buffer objects can only be set at
allocation time, and only a single tiled mode is supported. This new
DRM_TEGRA_GEM_SET_TILING IOCTL allows more modes to be set and also
allows the tiling mode to be changed after the allocation. This will
enable the Tegra DRM driver to import buffers from a GPU and directly
scan them out by configuring the display controller appropriately.

To complement this, the DRM_TEGRA_GEM_GET_TILING IOCTL can query the
current tiling mode of a buffer object. This is necessary when importing
buffers via handle (as is done in Mesa for example) so that userspace
can determine the proper parameters for the 2D or 3D engines.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/drm.c  | 95 ++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/tegra_drm.h | 25 ++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index fd736efd14bd..5dca20982f3b 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -455,6 +455,99 @@ static int tegra_get_syncpt_base(struct drm_device *drm, void *data,
 
 	return 0;
 }
+
+static int tegra_gem_set_tiling(struct drm_device *drm, void *data,
+				struct drm_file *file)
+{
+	struct drm_tegra_gem_set_tiling *args = data;
+	enum tegra_bo_tiling_mode mode;
+	struct drm_gem_object *gem;
+	unsigned long value = 0;
+	struct tegra_bo *bo;
+
+	switch (args->mode) {
+	case DRM_TEGRA_GEM_TILING_MODE_PITCH:
+		mode = TEGRA_BO_TILING_MODE_PITCH;
+
+		if (args->value != 0)
+			return -EINVAL;
+
+		break;
+
+	case DRM_TEGRA_GEM_TILING_MODE_TILED:
+		mode = TEGRA_BO_TILING_MODE_TILED;
+
+		if (args->value != 0)
+			return -EINVAL;
+
+		break;
+
+	case DRM_TEGRA_GEM_TILING_MODE_BLOCK:
+		mode = TEGRA_BO_TILING_MODE_BLOCK;
+
+		if (args->value > 5)
+			return -EINVAL;
+
+		value = args->value;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	gem = drm_gem_object_lookup(drm, file, args->handle);
+	if (!gem)
+		return -EINVAL;
+
+	bo = to_tegra_bo(gem);
+
+	bo->tiling.mode = mode;
+	bo->tiling.value = value;
+
+	drm_gem_object_unreference(gem);
+
+	return 0;
+}
+
+static int tegra_gem_get_tiling(struct drm_device *drm, void *data,
+				struct drm_file *file)
+{
+	struct drm_tegra_gem_get_tiling *args = data;
+	struct drm_gem_object *gem;
+	struct tegra_bo *bo;
+	int err = 0;
+
+	gem = drm_gem_object_lookup(drm, file, args->handle);
+	if (!gem)
+		return -EINVAL;
+
+	bo = to_tegra_bo(gem);
+
+	switch (bo->tiling.mode) {
+	case TEGRA_BO_TILING_MODE_PITCH:
+		args->mode = DRM_TEGRA_GEM_TILING_MODE_PITCH;
+		args->value = 0;
+		break;
+
+	case TEGRA_BO_TILING_MODE_TILED:
+		args->mode = DRM_TEGRA_GEM_TILING_MODE_TILED;
+		args->value = 0;
+		break;
+
+	case TEGRA_BO_TILING_MODE_BLOCK:
+		args->mode = DRM_TEGRA_GEM_TILING_MODE_BLOCK;
+		args->value = bo->tiling.value;
+		break;
+
+	default:
+		err = -EINVAL;
+		break;
+	}
+
+	drm_gem_object_unreference(gem);
+
+	return err;
+}
 #endif
 
 static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
@@ -469,6 +562,8 @@ static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT, tegra_get_syncpt, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_SUBMIT, tegra_submit, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT_BASE, tegra_get_syncpt_base, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_TILING, tegra_gem_set_tiling, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, DRM_UNLOCKED),
 #endif
 };
 
diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index b75482112428..0829f75eb986 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -129,6 +129,27 @@ struct drm_tegra_submit {
 	__u32 reserved[5];	/* future expansion */
 };
 
+#define DRM_TEGRA_GEM_TILING_MODE_PITCH 0
+#define DRM_TEGRA_GEM_TILING_MODE_TILED 1
+#define DRM_TEGRA_GEM_TILING_MODE_BLOCK 2
+
+struct drm_tegra_gem_set_tiling {
+	/* input */
+	__u32 handle;
+	__u32 mode;
+	__u32 value;
+	__u32 pad;
+};
+
+struct drm_tegra_gem_get_tiling {
+	/* input */
+	__u32 handle;
+	/* output */
+	__u32 mode;
+	__u32 value;
+	__u32 pad;
+};
+
 #define DRM_TEGRA_GEM_CREATE		0x00
 #define DRM_TEGRA_GEM_MMAP		0x01
 #define DRM_TEGRA_SYNCPT_READ		0x02
@@ -139,6 +160,8 @@ struct drm_tegra_submit {
 #define DRM_TEGRA_GET_SYNCPT		0x07
 #define DRM_TEGRA_SUBMIT		0x08
 #define DRM_TEGRA_GET_SYNCPT_BASE	0x09
+#define DRM_TEGRA_GEM_SET_TILING	0x0a
+#define DRM_TEGRA_GEM_GET_TILING	0x0b
 
 #define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_CREATE, struct drm_tegra_gem_create)
 #define DRM_IOCTL_TEGRA_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP, struct drm_tegra_gem_mmap)
@@ -150,5 +173,7 @@ struct drm_tegra_submit {
 #define DRM_IOCTL_TEGRA_GET_SYNCPT DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GET_SYNCPT, struct drm_tegra_get_syncpt)
 #define DRM_IOCTL_TEGRA_SUBMIT DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_SUBMIT, struct drm_tegra_submit)
 #define DRM_IOCTL_TEGRA_GET_SYNCPT_BASE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GET_SYNCPT_BASE, struct drm_tegra_get_syncpt_base)
+#define DRM_IOCTL_TEGRA_GEM_SET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_TILING, struct drm_tegra_gem_set_tiling)
+#define DRM_IOCTL_TEGRA_GEM_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_TILING, struct drm_tegra_gem_get_tiling)
 
 #endif
-- 
1.9.2

[PATCH 3/4] drm/tegra: Add SET/GET_FLAGS IOCTLs

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The DRM_TEGRA_GEM_SET_FLAGS IOCTL can be used to set the flags of a
buffer object after it has been allocated or imported. Flags associated
with a buffer object can be queried using the DRM_TEGRA_GEM_GET_FLAGS
IOCTL.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/drm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/tegra_drm.h | 21 +++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 5dca20982f3b..f292c29ef62f 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -548,6 +548,52 @@ static int tegra_gem_get_tiling(struct drm_device *drm, void *data,
 
 	return err;
 }
+
+static int tegra_gem_set_flags(struct drm_device *drm, void *data,
+			       struct drm_file *file)
+{
+	struct drm_tegra_gem_set_flags *args = data;
+	struct drm_tegra_bo *bo;
+	unsigned long flags = 0;
+
+	if (args->flags & ~DRM_TEGRA_GEM_FLAGS)
+		return -EINVAL;
+
+	gem = drm_gem_object_lookup(drm, file, args->handle);
+	if (!gem)
+		return -EINVAL;
+
+	bo = to_tegra_bo(gem);
+	bo->flags = 0;
+
+	if (args->flags & DRM_TEGRA_GEM_BOTTOM_UP)
+		bo->flags |= TEGRA_BO_BOTTOM_UP;
+
+	drm_gem_object_unreference(gem);
+
+	return 0;
+}
+
+static int tegra_gem_get_flags(struct drm_device *drm, void *data,
+			       struct drm_file *file)
+{
+	struct drm_tegra_gem_get_flags *args = data;
+	struct drm_tegra_bo *bo;
+
+	gem = drm_gem_object_lookup(drm, file, args->handle);
+	if (!gem)
+		return -EINVAL;
+
+	bo = to_tegra_bo(gem);
+	args->flags = 0;
+
+	if (bo->flags & TEGRA_BO_BOTTOM_UP)
+		args->flags |= DRM_TEGRA_GEM_BOTTOM_UP;
+
+	drm_gem_object_unreference(gem);
+
+	return 0;
+}
 #endif
 
 static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
@@ -564,6 +610,8 @@ static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT_BASE, tegra_get_syncpt_base, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_TILING, tegra_gem_set_tiling, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_FLAGS, tegra_gem_set_flags, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_FLAGS, tegra_gem_get_flags, DRM_UNLOCKED),
 #endif
 };
 
diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index 0829f75eb986..c15d781ecc0f 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -150,6 +150,23 @@ struct drm_tegra_gem_get_tiling {
 	__u32 pad;
 };
 
+#define DRM_TEGRA_GEM_BOTTOM_UP		(1 << 0)
+#define DRM_TEGRA_GEM_FLAGS		(DRM_TEGRA_GEM_BOTTOM_UP)
+
+struct drm_tegra_gem_set_flags {
+	/* input */
+	__u32 handle;
+	/* output */
+	__u32 flags;
+};
+
+struct drm_tegra_gem_get_flags {
+	/* input */
+	__u32 handle;
+	/* output */
+	__u32 flags;
+};
+
 #define DRM_TEGRA_GEM_CREATE		0x00
 #define DRM_TEGRA_GEM_MMAP		0x01
 #define DRM_TEGRA_SYNCPT_READ		0x02
@@ -162,6 +179,8 @@ struct drm_tegra_gem_get_tiling {
 #define DRM_TEGRA_GET_SYNCPT_BASE	0x09
 #define DRM_TEGRA_GEM_SET_TILING	0x0a
 #define DRM_TEGRA_GEM_GET_TILING	0x0b
+#define DRM_TEGRA_GEM_SET_FLAGS		0x0c
+#define DRM_TEGRA_GEM_GET_FLAGS		0x0d
 
 #define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_CREATE, struct drm_tegra_gem_create)
 #define DRM_IOCTL_TEGRA_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP, struct drm_tegra_gem_mmap)
@@ -175,5 +194,7 @@ struct drm_tegra_gem_get_tiling {
 #define DRM_IOCTL_TEGRA_GET_SYNCPT_BASE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GET_SYNCPT_BASE, struct drm_tegra_get_syncpt_base)
 #define DRM_IOCTL_TEGRA_GEM_SET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_TILING, struct drm_tegra_gem_set_tiling)
 #define DRM_IOCTL_TEGRA_GEM_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_TILING, struct drm_tegra_gem_get_tiling)
+#define DRM_IOCTL_TEGRA_GEM_SET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_FLAGS, struct drm_tegra_gem_set_flags)
+#define DRM_IOCTL_TEGRA_GEM_GET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_FLAGS, struct drm_tegra_gem_get_flags)
 
 #endif
-- 
1.9.2

[PATCH 4/4] drm/tegra: Allow non-authenticated processes to create buffer objects

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This matches what other drivers do for equivalent IOCTLs.

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

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index f292c29ef62f..56ef81a6f28f 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -598,7 +598,7 @@ static int tegra_gem_get_flags(struct drm_device *drm, void *data,
 
 static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
 #ifdef CONFIG_DRM_TEGRA_STAGING
-	DRM_IOCTL_DEF_DRV(TEGRA_GEM_CREATE, tegra_gem_create, DRM_UNLOCKED | DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(TEGRA_GEM_CREATE, tegra_gem_create, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_MMAP, tegra_gem_mmap, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_READ, tegra_syncpt_read, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_INCR, tegra_syncpt_incr, DRM_UNLOCKED),
-- 
1.9.2

Re: [PATCH 4/4] drm/tegra: Allow non-authenticated processes to create buffer objects

[Stéphane Marchesin]

On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> This matches what other drivers do for equivalent IOCTLs.
>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Stéphane Marchesin <marcheu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

> ---
>  drivers/gpu/drm/tegra/drm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index f292c29ef62f..56ef81a6f28f 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -598,7 +598,7 @@ static int tegra_gem_get_flags(struct drm_device *drm, void *data,
>
>  static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
>  #ifdef CONFIG_DRM_TEGRA_STAGING
> -       DRM_IOCTL_DEF_DRV(TEGRA_GEM_CREATE, tegra_gem_create, DRM_UNLOCKED | DRM_AUTH),
> +       DRM_IOCTL_DEF_DRV(TEGRA_GEM_CREATE, tegra_gem_create, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_GEM_MMAP, tegra_gem_mmap, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_READ, tegra_syncpt_read, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_INCR, tegra_syncpt_incr, DRM_UNLOCKED),
> --
> 1.9.2
>

Re: [PATCH 3/4] drm/tegra: Add SET/GET_FLAGS IOCTLs

[Stéphane Marchesin]

On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> The DRM_TEGRA_GEM_SET_FLAGS IOCTL can be used to set the flags of a
> buffer object after it has been allocated or imported. Flags associated
> with a buffer object can be queried using the DRM_TEGRA_GEM_GET_FLAGS
> IOCTL.
>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/gpu/drm/tegra/drm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/tegra_drm.h | 21 +++++++++++++++++++
>  2 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 5dca20982f3b..f292c29ef62f 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -548,6 +548,52 @@ static int tegra_gem_get_tiling(struct drm_device *drm, void *data,
>
>         return err;
>  }
> +
> +static int tegra_gem_set_flags(struct drm_device *drm, void *data,
> +                              struct drm_file *file)
> +{
> +       struct drm_tegra_gem_set_flags *args = data;
> +       struct drm_tegra_bo *bo;
> +       unsigned long flags = 0;
> +
> +       if (args->flags & ~DRM_TEGRA_GEM_FLAGS)
> +               return -EINVAL;
> +
> +       gem = drm_gem_object_lookup(drm, file, args->handle);
> +       if (!gem)
> +               return -EINVAL;

Usually -ENOENT is returned for unknown objects I think?

> +
> +       bo = to_tegra_bo(gem);
> +       bo->flags = 0;
> +
> +       if (args->flags & DRM_TEGRA_GEM_BOTTOM_UP)
> +               bo->flags |= TEGRA_BO_BOTTOM_UP;
> +
> +       drm_gem_object_unreference(gem);
> +
> +       return 0;
> +}
> +
> +static int tegra_gem_get_flags(struct drm_device *drm, void *data,
> +                              struct drm_file *file)
> +{
> +       struct drm_tegra_gem_get_flags *args = data;
> +       struct drm_tegra_bo *bo;
> +
> +       gem = drm_gem_object_lookup(drm, file, args->handle);
> +       if (!gem)
> +               return -EINVAL;
> +
> +       bo = to_tegra_bo(gem);
> +       args->flags = 0;
> +
> +       if (bo->flags & TEGRA_BO_BOTTOM_UP)
> +               args->flags |= DRM_TEGRA_GEM_BOTTOM_UP;
> +
> +       drm_gem_object_unreference(gem);
> +
> +       return 0;
> +}
>  #endif
>
>  static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
> @@ -564,6 +610,8 @@ static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
>         DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT_BASE, tegra_get_syncpt_base, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_TILING, tegra_gem_set_tiling, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, DRM_UNLOCKED),
> +       DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_FLAGS, tegra_gem_set_flags, DRM_UNLOCKED),
> +       DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_FLAGS, tegra_gem_get_flags, DRM_UNLOCKED),
>  #endif
>  };
>
> diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> index 0829f75eb986..c15d781ecc0f 100644
> --- a/include/uapi/drm/tegra_drm.h
> +++ b/include/uapi/drm/tegra_drm.h
> @@ -150,6 +150,23 @@ struct drm_tegra_gem_get_tiling {
>         __u32 pad;
>  };
>
> +#define DRM_TEGRA_GEM_BOTTOM_UP                (1 << 0)
> +#define DRM_TEGRA_GEM_FLAGS            (DRM_TEGRA_GEM_BOTTOM_UP)
> +
> +struct drm_tegra_gem_set_flags {
> +       /* input */
> +       __u32 handle;
> +       /* output */
> +       __u32 flags;
> +};
> +
> +struct drm_tegra_gem_get_flags {
> +       /* input */
> +       __u32 handle;
> +       /* output */
> +       __u32 flags;
> +};
> +
>  #define DRM_TEGRA_GEM_CREATE           0x00
>  #define DRM_TEGRA_GEM_MMAP             0x01
>  #define DRM_TEGRA_SYNCPT_READ          0x02
> @@ -162,6 +179,8 @@ struct drm_tegra_gem_get_tiling {
>  #define DRM_TEGRA_GET_SYNCPT_BASE      0x09
>  #define DRM_TEGRA_GEM_SET_TILING       0x0a
>  #define DRM_TEGRA_GEM_GET_TILING       0x0b
> +#define DRM_TEGRA_GEM_SET_FLAGS                0x0c
> +#define DRM_TEGRA_GEM_GET_FLAGS                0x0d
>
>  #define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_CREATE, struct drm_tegra_gem_create)
>  #define DRM_IOCTL_TEGRA_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP, struct drm_tegra_gem_mmap)
> @@ -175,5 +194,7 @@ struct drm_tegra_gem_get_tiling {
>  #define DRM_IOCTL_TEGRA_GET_SYNCPT_BASE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GET_SYNCPT_BASE, struct drm_tegra_get_syncpt_base)
>  #define DRM_IOCTL_TEGRA_GEM_SET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_TILING, struct drm_tegra_gem_set_tiling)
>  #define DRM_IOCTL_TEGRA_GEM_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_TILING, struct drm_tegra_gem_get_tiling)
> +#define DRM_IOCTL_TEGRA_GEM_SET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_FLAGS, struct drm_tegra_gem_set_flags)
> +#define DRM_IOCTL_TEGRA_GEM_GET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_FLAGS, struct drm_tegra_gem_get_flags)
>
>  #endif
> --
> 1.9.2
>

Re: [PATCH 2/4] drm/tegra: Add SET/GET_TILING IOCTLs

[Stéphane Marchesin]

On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> Currently the tiling parameters of buffer objects can only be set at
> allocation time, and only a single tiled mode is supported. This new
> DRM_TEGRA_GEM_SET_TILING IOCTL allows more modes to be set and also
> allows the tiling mode to be changed after the allocation. This will
> enable the Tegra DRM driver to import buffers from a GPU and directly
> scan them out by configuring the display controller appropriately.

I was wondering why not support the new tiling modes on creation as
well, I guess because you don't have room for height in the creation
ioctl?

>
> To complement this, the DRM_TEGRA_GEM_GET_TILING IOCTL can query the
> current tiling mode of a buffer object. This is necessary when importing
> buffers via handle (as is done in Mesa for example) so that userspace
> can determine the proper parameters for the 2D or 3D engines.
>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/gpu/drm/tegra/drm.c  | 95 ++++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/tegra_drm.h | 25 ++++++++++++
>  2 files changed, 120 insertions(+)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index fd736efd14bd..5dca20982f3b 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -455,6 +455,99 @@ static int tegra_get_syncpt_base(struct drm_device *drm, void *data,
>
>         return 0;
>  }
> +
> +static int tegra_gem_set_tiling(struct drm_device *drm, void *data,
> +                               struct drm_file *file)
> +{
> +       struct drm_tegra_gem_set_tiling *args = data;
> +       enum tegra_bo_tiling_mode mode;
> +       struct drm_gem_object *gem;
> +       unsigned long value = 0;
> +       struct tegra_bo *bo;
> +
> +       switch (args->mode) {
> +       case DRM_TEGRA_GEM_TILING_MODE_PITCH:
> +               mode = TEGRA_BO_TILING_MODE_PITCH;
> +
> +               if (args->value != 0)
> +                       return -EINVAL;
> +
> +               break;
> +
> +       case DRM_TEGRA_GEM_TILING_MODE_TILED:
> +               mode = TEGRA_BO_TILING_MODE_TILED;
> +
> +               if (args->value != 0)
> +                       return -EINVAL;
> +
> +               break;
> +
> +       case DRM_TEGRA_GEM_TILING_MODE_BLOCK:
> +               mode = TEGRA_BO_TILING_MODE_BLOCK;
> +
> +               if (args->value > 5)
> +                       return -EINVAL;
> +
> +               value = args->value;
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       gem = drm_gem_object_lookup(drm, file, args->handle);
> +       if (!gem)
> +               return -EINVAL;
> +
> +       bo = to_tegra_bo(gem);
> +
> +       bo->tiling.mode = mode;
> +       bo->tiling.value = value;
> +
> +       drm_gem_object_unreference(gem);
> +
> +       return 0;
> +}
> +
> +static int tegra_gem_get_tiling(struct drm_device *drm, void *data,
> +                               struct drm_file *file)
> +{
> +       struct drm_tegra_gem_get_tiling *args = data;
> +       struct drm_gem_object *gem;
> +       struct tegra_bo *bo;
> +       int err = 0;
> +
> +       gem = drm_gem_object_lookup(drm, file, args->handle);
> +       if (!gem)
> +               return -EINVAL;
> +
> +       bo = to_tegra_bo(gem);
> +
> +       switch (bo->tiling.mode) {
> +       case TEGRA_BO_TILING_MODE_PITCH:
> +               args->mode = DRM_TEGRA_GEM_TILING_MODE_PITCH;
> +               args->value = 0;
> +               break;
> +
> +       case TEGRA_BO_TILING_MODE_TILED:
> +               args->mode = DRM_TEGRA_GEM_TILING_MODE_TILED;
> +               args->value = 0;
> +               break;
> +
> +       case TEGRA_BO_TILING_MODE_BLOCK:
> +               args->mode = DRM_TEGRA_GEM_TILING_MODE_BLOCK;
> +               args->value = bo->tiling.value;
> +               break;
> +
> +       default:
> +               err = -EINVAL;
> +               break;
> +       }
> +
> +       drm_gem_object_unreference(gem);
> +
> +       return err;
> +}
>  #endif
>
>  static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
> @@ -469,6 +562,8 @@ static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
>         DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT, tegra_get_syncpt, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_SUBMIT, tegra_submit, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT_BASE, tegra_get_syncpt_base, DRM_UNLOCKED),
> +       DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_TILING, tegra_gem_set_tiling, DRM_UNLOCKED),
> +       DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, DRM_UNLOCKED),
>  #endif
>  };
>
> diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> index b75482112428..0829f75eb986 100644
> --- a/include/uapi/drm/tegra_drm.h
> +++ b/include/uapi/drm/tegra_drm.h
> @@ -129,6 +129,27 @@ struct drm_tegra_submit {
>         __u32 reserved[5];      /* future expansion */
>  };
>
> +#define DRM_TEGRA_GEM_TILING_MODE_PITCH 0
> +#define DRM_TEGRA_GEM_TILING_MODE_TILED 1
> +#define DRM_TEGRA_GEM_TILING_MODE_BLOCK 2
> +
> +struct drm_tegra_gem_set_tiling {
> +       /* input */
> +       __u32 handle;
> +       __u32 mode;
> +       __u32 value;

This seems to only be used for height, if so should it be called height?

Stéphane


> +       __u32 pad;
> +};
> +
> +struct drm_tegra_gem_get_tiling {
> +       /* input */
> +       __u32 handle;
> +       /* output */
> +       __u32 mode;
> +       __u32 value;
> +       __u32 pad;
> +};
> +
>  #define DRM_TEGRA_GEM_CREATE           0x00
>  #define DRM_TEGRA_GEM_MMAP             0x01
>  #define DRM_TEGRA_SYNCPT_READ          0x02
> @@ -139,6 +160,8 @@ struct drm_tegra_submit {
>  #define DRM_TEGRA_GET_SYNCPT           0x07
>  #define DRM_TEGRA_SUBMIT               0x08
>  #define DRM_TEGRA_GET_SYNCPT_BASE      0x09
> +#define DRM_TEGRA_GEM_SET_TILING       0x0a
> +#define DRM_TEGRA_GEM_GET_TILING       0x0b
>
>  #define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_CREATE, struct drm_tegra_gem_create)
>  #define DRM_IOCTL_TEGRA_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP, struct drm_tegra_gem_mmap)
> @@ -150,5 +173,7 @@ struct drm_tegra_submit {
>  #define DRM_IOCTL_TEGRA_GET_SYNCPT DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GET_SYNCPT, struct drm_tegra_get_syncpt)
>  #define DRM_IOCTL_TEGRA_SUBMIT DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_SUBMIT, struct drm_tegra_submit)
>  #define DRM_IOCTL_TEGRA_GET_SYNCPT_BASE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GET_SYNCPT_BASE, struct drm_tegra_get_syncpt_base)
> +#define DRM_IOCTL_TEGRA_GEM_SET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_TILING, struct drm_tegra_gem_set_tiling)
> +#define DRM_IOCTL_TEGRA_GEM_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_TILING, struct drm_tegra_gem_get_tiling)
>
>  #endif
> --
> 1.9.2
>

Re: [PATCH 2/4] drm/tegra: Add SET/GET_TILING IOCTLs

[Thierry Reding]

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

On Wed, Jun 11, 2014 at 10:43:30AM -0700, Stéphane Marchesin wrote:
> On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Currently the tiling parameters of buffer objects can only be set at
> > allocation time, and only a single tiled mode is supported. This new
> > DRM_TEGRA_GEM_SET_TILING IOCTL allows more modes to be set and also
> > allows the tiling mode to be changed after the allocation. This will
> > enable the Tegra DRM driver to import buffers from a GPU and directly
> > scan them out by configuring the display controller appropriately.
> 
> I was wondering why not support the new tiling modes on creation as
> well, I guess because you don't have room for height in the creation
> ioctl?

Yes, back at the time when the GEM_CREATE IOCTL was added we only left
enough room for "flags", so it'll be difficult to add support for the
newer tiling modes.

One possibility that I see is to extend struct drm_tegra_gem_create in a
backwards-compatible way and make new fields dependent on some new flag,
somewhat like this:

	#define DRM_TEGRA_GEM_CREATE_BLOCK_LINEAR (1 << 3)

	struct drm_tegra_gem_create {
		__u64 size;
		__u32 flags;
		__u32 handle;
		__u32 height;
		__u32 pad;
	};

Then the DRM driver can parse the height field only if the BLOCK_LINEAR
flag is set.

To be honest, though, I'd prefer to deprecate the flags at creation if
these patches get merged so that we can have a standard way of setting
these rather than a few.

Do you see any particular advantages in specifying the tiling mode at
creation time?

> > diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> > index b75482112428..0829f75eb986 100644
> > --- a/include/uapi/drm/tegra_drm.h
> > +++ b/include/uapi/drm/tegra_drm.h
> > @@ -129,6 +129,27 @@ struct drm_tegra_submit {
> >         __u32 reserved[5];      /* future expansion */
> >  };
> >
> > +#define DRM_TEGRA_GEM_TILING_MODE_PITCH 0
> > +#define DRM_TEGRA_GEM_TILING_MODE_TILED 1
> > +#define DRM_TEGRA_GEM_TILING_MODE_BLOCK 2
> > +
> > +struct drm_tegra_gem_set_tiling {
> > +       /* input */
> > +       __u32 handle;
> > +       __u32 mode;
> > +       __u32 value;
> 
> This seems to only be used for height, if so should it be called height?

The idea here was to have a generic name here so that it could be more
easily extended if/when newer tiling modes are introduced.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 3/4] drm/tegra: Add SET/GET_FLAGS IOCTLs

[Thierry Reding]

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

On Wed, Jun 11, 2014 at 09:21:21AM -0700, Stéphane Marchesin wrote:
> On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The DRM_TEGRA_GEM_SET_FLAGS IOCTL can be used to set the flags of a
> > buffer object after it has been allocated or imported. Flags associated
> > with a buffer object can be queried using the DRM_TEGRA_GEM_GET_FLAGS
> > IOCTL.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/drm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++
> >  include/uapi/drm/tegra_drm.h | 21 +++++++++++++++++++
> >  2 files changed, 69 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > index 5dca20982f3b..f292c29ef62f 100644
> > --- a/drivers/gpu/drm/tegra/drm.c
> > +++ b/drivers/gpu/drm/tegra/drm.c
> > @@ -548,6 +548,52 @@ static int tegra_gem_get_tiling(struct drm_device *drm, void *data,
> >
> >         return err;
> >  }
> > +
> > +static int tegra_gem_set_flags(struct drm_device *drm, void *data,
> > +                              struct drm_file *file)
> > +{
> > +       struct drm_tegra_gem_set_flags *args = data;
> > +       struct drm_tegra_bo *bo;
> > +       unsigned long flags = 0;
> > +
> > +       if (args->flags & ~DRM_TEGRA_GEM_FLAGS)
> > +               return -EINVAL;
> > +
> > +       gem = drm_gem_object_lookup(drm, file, args->handle);
> > +       if (!gem)
> > +               return -EINVAL;
> 
> Usually -ENOENT is returned for unknown objects I think?

We've had that discussion on IRC a little while back. I came across a
few places where this was returned to userspace, and since I remember
a particular email[0] about this error code so well I thought it should
be discussed, but I can't remember the outcome (if any). Quoting from
that email:

	ENOENT is not a valid error return from an ioctl. Never has been,
	never will be. ENOENT means "No such file and directory", and is
	for path operations. ioctl's are done on files that have already
	been opened, there's no way in hell that ENOENT would ever be
	valid.

Given that -ENOENT is used *a lot* in DRM (many, if not most, of the
instances returning the error from an IOCTL), I wonder how this can be
resolved. Given that userspace may rely on this error code and that we
can't break userspace I don't see a way out.

Thierry

[0]: https://lkml.org/lkml/2012/12/23/75

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 3/4] drm/tegra: Add SET/GET_FLAGS IOCTLs

[Daniel Vetter]

On Thu, Jun 12, 2014 at 09:18:40AM +0200, Thierry Reding wrote:
> On Wed, Jun 11, 2014 at 09:21:21AM -0700, Stéphane Marchesin wrote:
> > On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
> > <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > >
> > > The DRM_TEGRA_GEM_SET_FLAGS IOCTL can be used to set the flags of a
> > > buffer object after it has been allocated or imported. Flags associated
> > > with a buffer object can be queried using the DRM_TEGRA_GEM_GET_FLAGS
> > > IOCTL.
> > >
> > > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  drivers/gpu/drm/tegra/drm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++
> > >  include/uapi/drm/tegra_drm.h | 21 +++++++++++++++++++
> > >  2 files changed, 69 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > > index 5dca20982f3b..f292c29ef62f 100644
> > > --- a/drivers/gpu/drm/tegra/drm.c
> > > +++ b/drivers/gpu/drm/tegra/drm.c
> > > @@ -548,6 +548,52 @@ static int tegra_gem_get_tiling(struct drm_device *drm, void *data,
> > >
> > >         return err;
> > >  }
> > > +
> > > +static int tegra_gem_set_flags(struct drm_device *drm, void *data,
> > > +                              struct drm_file *file)
> > > +{
> > > +       struct drm_tegra_gem_set_flags *args = data;
> > > +       struct drm_tegra_bo *bo;
> > > +       unsigned long flags = 0;
> > > +
> > > +       if (args->flags & ~DRM_TEGRA_GEM_FLAGS)
> > > +               return -EINVAL;
> > > +
> > > +       gem = drm_gem_object_lookup(drm, file, args->handle);
> > > +       if (!gem)
> > > +               return -EINVAL;
> > 
> > Usually -ENOENT is returned for unknown objects I think?
> 
> We've had that discussion on IRC a little while back. I came across a
> few places where this was returned to userspace, and since I remember
> a particular email[0] about this error code so well I thought it should
> be discussed, but I can't remember the outcome (if any). Quoting from
> that email:
> 
> 	ENOENT is not a valid error return from an ioctl. Never has been,
> 	never will be. ENOENT means "No such file and directory", and is
> 	for path operations. ioctl's are done on files that have already
> 	been opened, there's no way in hell that ENOENT would ever be
> 	valid.
> 
> Given that -ENOENT is used *a lot* in DRM (many, if not most, of the
> instances returning the error from an IOCTL), I wonder how this can be
> resolved. Given that userspace may rely on this error code and that we
> can't break userspace I don't see a way out.

Imo returning -ENOENT for lookup failures is perfectly ok, after all the
error code spelled out means "No entity" or so. So might as well extend
the meaning. Without slight bending of error codes the only thing we'd
ever be able to return is -EINVAL for ioctls, which is completely
pointless.

So my approach is to throw the vfs experts opinion into the wind and
continue with the well-established rules we have in drm.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/4] drm/tegra: Add SET/GET_FLAGS IOCTLs

[Thierry Reding]

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

On Thu, Jun 12, 2014 at 11:28:43AM +0200, Daniel Vetter wrote:
> On Thu, Jun 12, 2014 at 09:18:40AM +0200, Thierry Reding wrote:
> > On Wed, Jun 11, 2014 at 09:21:21AM -0700, Stéphane Marchesin wrote:
> > > On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
> > > <thierry.reding@gmail.com> wrote:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > >
> > > > The DRM_TEGRA_GEM_SET_FLAGS IOCTL can be used to set the flags of a
> > > > buffer object after it has been allocated or imported. Flags associated
> > > > with a buffer object can be queried using the DRM_TEGRA_GEM_GET_FLAGS
> > > > IOCTL.
> > > >
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > ---
> > > >  drivers/gpu/drm/tegra/drm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  include/uapi/drm/tegra_drm.h | 21 +++++++++++++++++++
> > > >  2 files changed, 69 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > > > index 5dca20982f3b..f292c29ef62f 100644
> > > > --- a/drivers/gpu/drm/tegra/drm.c
> > > > +++ b/drivers/gpu/drm/tegra/drm.c
> > > > @@ -548,6 +548,52 @@ static int tegra_gem_get_tiling(struct drm_device *drm, void *data,
> > > >
> > > >         return err;
> > > >  }
> > > > +
> > > > +static int tegra_gem_set_flags(struct drm_device *drm, void *data,
> > > > +                              struct drm_file *file)
> > > > +{
> > > > +       struct drm_tegra_gem_set_flags *args = data;
> > > > +       struct drm_tegra_bo *bo;
> > > > +       unsigned long flags = 0;
> > > > +
> > > > +       if (args->flags & ~DRM_TEGRA_GEM_FLAGS)
> > > > +               return -EINVAL;
> > > > +
> > > > +       gem = drm_gem_object_lookup(drm, file, args->handle);
> > > > +       if (!gem)
> > > > +               return -EINVAL;
> > > 
> > > Usually -ENOENT is returned for unknown objects I think?
> > 
> > We've had that discussion on IRC a little while back. I came across a
> > few places where this was returned to userspace, and since I remember
> > a particular email[0] about this error code so well I thought it should
> > be discussed, but I can't remember the outcome (if any). Quoting from
> > that email:
> > 
> > 	ENOENT is not a valid error return from an ioctl. Never has been,
> > 	never will be. ENOENT means "No such file and directory", and is
> > 	for path operations. ioctl's are done on files that have already
> > 	been opened, there's no way in hell that ENOENT would ever be
> > 	valid.
> > 
> > Given that -ENOENT is used *a lot* in DRM (many, if not most, of the
> > instances returning the error from an IOCTL), I wonder how this can be
> > resolved. Given that userspace may rely on this error code and that we
> > can't break userspace I don't see a way out.
> 
> Imo returning -ENOENT for lookup failures is perfectly ok, after all the
> error code spelled out means "No entity" or so. So might as well extend
> the meaning. Without slight bending of error codes the only thing we'd
> ever be able to return is -EINVAL for ioctls, which is completely
> pointless.

I disagree. -EINVAL makes perfect sense because one of the arguments
passed into the IOCTL (the handle) is invalid.

> So my approach is to throw the vfs experts opinion into the wind and
> continue with the well-established rules we have in drm.

The quoted comments was from Linus himself and I didn't feel like being
shouted at. Given his above explanation it doesn't seem like the thing
that he'd consider subsystem specific. But then again, ENOENT usage has
gone unnoticed in DRM for some time, so maybe we'll get away with it.

I'll update the patch to make it consistent with the rest of DRM.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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