[PATCH v2 0/3] drm/nouveau: Support NVIDIA format modifiers

[PATCH v2 0/3] drm/nouveau: Support NVIDIA format modifiers

[James Jones]

This series modifies the NV5x+ nouveau display backends to advertise
appropriate format modifiers on their display planes in atomic mode
setting blobs.

Corresponding modifications to Mesa/userspace are available here:

https://gitlab.freedesktop.org/cubanismo/mesa/tree/nouveau_work

But those need a bit of cleanup before they're ready to submit.

I've tested this on Tesla, Kepler, Pascal, and Turing-class hardware
using various formats and all the exposed format modifiers, plus some
negative testing with invalid ones.

NOTE: this series depends on the "[PATCH v3] drm: Generalized NV Block
Linear DRM format mod" patch submitted to dri-devel.

v2: Used Tesla family instead of NV50 chipset compare to avoid treating
    oddly numbered NV4x-class chipsets as NV50+ GPUs.  Other instances
    of compares with chipset number in the series were audited, deemed
    safe, and left as-is for consistency with existing code.

James Jones (3):
  drm/nouveau: Add format mod prop to base/ovly/nvdisp
  drm/nouveau: Check framebuffer size against bo
  drm/nouveau: Support NVIDIA format modifiers

 drivers/gpu/drm/nouveau/dispnv50/base507c.c |   7 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c     |  59 ++++++++
 drivers/gpu/drm/nouveau/dispnv50/disp.h     |   4 +
 drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  35 ++++-
 drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  17 +++
 drivers/gpu/drm/nouveau/nouveau_display.c   | 154 ++++++++++++++++++++
 drivers/gpu/drm/nouveau/nouveau_display.h   |   4 +
 7 files changed, 272 insertions(+), 8 deletions(-)

-- 
2.17.1

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

[PATCH v2 1/3] drm/nouveau: Add format mod prop to base/ovly/nvdisp

[James Jones]

Advertise support for the full list of format
modifiers supported by each class of NVIDIA
desktop GPU display hardware.  Stash the array
of modifiers in the nouveau_display struct for
use when validating userspace framebuffer
creation requests, which will be supportd in
a subsequent change.

Signed-off-by: James Jones <jajones@nvidia.com>
---
 drivers/gpu/drm/nouveau/dispnv50/base507c.c |  7 +--
 drivers/gpu/drm/nouveau/dispnv50/disp.c     | 59 +++++++++++++++++++++
 drivers/gpu/drm/nouveau/dispnv50/disp.h     |  4 ++
 drivers/gpu/drm/nouveau/dispnv50/wndw.c     | 27 +++++++++-
 drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c | 17 ++++++
 drivers/gpu/drm/nouveau/nouveau_display.h   |  2 +
 6 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/base507c.c b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
index 00a85f1e1a4a..025b8f996a0a 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/base507c.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
@@ -262,7 +262,8 @@ base507c_new_(const struct nv50_wndw_func *func, const u32 *format,
 	struct nv50_disp_base_channel_dma_v0 args = {
 		.head = head,
 	};
-	struct nv50_disp *disp = nv50_disp(drm->dev);
+	struct nouveau_display *disp = nouveau_display(drm->dev);
+	struct nv50_disp *disp50 = nv50_disp(drm->dev);
 	struct nv50_wndw *wndw;
 	int ret;
 
@@ -272,9 +273,9 @@ base507c_new_(const struct nv50_wndw_func *func, const u32 *format,
 	if (*pwndw = wndw, ret)
 		return ret;
 
-	ret = nv50_dmac_create(&drm->client.device, &disp->disp->object,
+	ret = nv50_dmac_create(&drm->client.device, &disp->disp.object,
 			       &oclass, head, &args, sizeof(args),
-			       disp->sync->bo.offset, &wndw->wndw);
+			       disp50->sync->bo.offset, &wndw->wndw);
 	if (ret) {
 		NV_ERROR(drm, "base%04x allocation failed: %d\n", oclass, ret);
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 064a69d161e3..0956367d27a2 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2337,6 +2337,15 @@ nv50_display_create(struct drm_device *dev)
 	if (ret)
 		goto out;
 
+	/* Assign the correct format modifiers */
+	if (disp->disp->object.oclass >= TU102_DISP)
+		nouveau_display(dev)->format_modifiers = wndwc57e_modifiers;
+	else
+	if (disp->disp->object.oclass >= GF110_DISP)
+		nouveau_display(dev)->format_modifiers = disp90xx_modifiers;
+	else
+		nouveau_display(dev)->format_modifiers = disp50xx_modifiers;
+
 	/* create crtc objects to represent the hw heads */
 	if (disp->disp->object.oclass >= GV100_DISP)
 		crtcs = nvif_rd32(&device->object, 0x610060) & 0xff;
@@ -2404,3 +2413,53 @@ nv50_display_create(struct drm_device *dev)
 		nv50_display_destroy(dev);
 	return ret;
 }
+
+/******************************************************************************
+ * Format modifiers
+ *****************************************************************************/
+
+/****************************************************************
+ *            Log2(block height) ----------------------------+  *
+ *            Page Kind ----------------------------------+  |  *
+ *            Gob Height/Page Kind Generation ------+     |  |  *
+ *                          Sector layout -------+  |     |  |  *
+ *                          Compression ------+  |  |     |  |  */
+const u64 disp50xx_modifiers[] = { /*         |  |  |     |  |  */
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x7a, 0),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x7a, 1),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x7a, 2),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x7a, 3),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x7a, 4),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x7a, 5),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x78, 0),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x78, 1),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x78, 2),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x78, 3),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x78, 4),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x78, 5),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x70, 0),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x70, 1),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x70, 2),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x70, 3),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x70, 4),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 1, 0x70, 5),
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
+/****************************************************************
+ *            Log2(block height) ----------------------------+  *
+ *            Page Kind ----------------------------------+  |  *
+ *            Gob Height/Page Kind Generation ------+     |  |  *
+ *                          Sector layout -------+  |     |  |  *
+ *                          Compression ------+  |  |     |  |  */
+const u64 disp90xx_modifiers[] = { /*         |  |  |     |  |  */
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 0, 0xfe, 0),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 0, 0xfe, 1),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 0, 0xfe, 2),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 0, 0xfe, 3),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 0, 0xfe, 4),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 0, 0xfe, 5),
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h b/drivers/gpu/drm/nouveau/dispnv50/disp.h
index 7c41b0599d1a..49982b632f82 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h
@@ -76,6 +76,10 @@ void nv50_dmac_destroy(struct nv50_dmac *);
 u32 *evo_wait(struct nv50_dmac *, int nr);
 void evo_kick(u32 *, struct nv50_dmac *);
 
+extern const u64 disp50xx_modifiers[];
+extern const u64 disp90xx_modifiers[];
+extern const u64 wndwc57e_modifiers[];
+
 #define evo_mthd(p, m, s) do {						\
 	const u32 _m = (m), _s = (s);					\
 	if (drm_debug & DRM_UT_KMS)					\
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 2db029371c91..70ad64cb2d34 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -594,6 +594,29 @@ nv50_wndw_destroy(struct drm_plane *plane)
 	kfree(wndw);
 }
 
+/* This function assumes the format has already been validated against the plane
+ * and the modifier was validated against the device-wides modifier list at FB
+ * creation time.
+ */
+static bool nv50_plane_format_mod_supported(struct drm_plane *plane,
+					    u32 format, u64 modifier)
+{
+	struct nouveau_drm *drm = nouveau_drm(plane->dev);
+	uint8_t i;
+
+	if (drm->client.device.info.chipset < 0xc0) {
+		const struct drm_format_info *info = drm_format_info(format);
+		const uint8_t kind = (modifier >> 12) & 0xff;
+
+		if (!format) return false;
+
+		for (i = 0; i < info->num_planes; i++)
+			if ((info->cpp[i] != 4) && kind != 0x70) return false;
+	}
+
+	return true;
+}
+
 const struct drm_plane_funcs
 nv50_wndw = {
 	.update_plane = drm_atomic_helper_update_plane,
@@ -602,6 +625,7 @@ nv50_wndw = {
 	.reset = nv50_wndw_reset,
 	.atomic_duplicate_state = nv50_wndw_atomic_duplicate_state,
 	.atomic_destroy_state = nv50_wndw_atomic_destroy_state,
+	.format_mod_supported = nv50_plane_format_mod_supported,
 };
 
 static int
@@ -649,7 +673,8 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
 	for (nformat = 0; format[nformat]; nformat++);
 
 	ret = drm_universal_plane_init(dev, &wndw->plane, heads, &nv50_wndw,
-				       format, nformat, NULL,
+				       format, nformat,
+				       nouveau_display(dev)->format_modifiers,
 				       type, "%s-%d", name, index);
 	if (ret) {
 		kfree(*pwndw);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
index a311c79e5295..e887e44b54c9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
@@ -171,6 +171,23 @@ wndwc57e_ilut(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw)
 	asyw->xlut.i.load = wndwc57e_ilut_load;
 }
 
+/****************************************************************
+ *            Log2(block height) ----------------------------+  *
+ *            Page Kind ----------------------------------+  |  *
+ *            Gob Height/Page Kind Generation ------+     |  |  *
+ *                          Sector layout -------+  |     |  |  *
+ *                          Compression ------+  |  |     |  |  */
+const u64 wndwc57e_modifiers[] = { /*         |  |  |     |  |  */
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 2, 0x06, 0),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 2, 0x06, 1),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 2, 0x06, 2),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 2, 0x06, 3),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 2, 0x06, 4),
+	DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D(0, 1, 2, 0x06, 5),
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
 static const struct nv50_wndw_func
 wndwc57e = {
 	.acquire = wndwc37e_acquire,
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index 6e8e66882e45..c54682f00b01 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -47,6 +47,8 @@ struct nouveau_display {
 	struct drm_property *color_vibrance_property;
 
 	struct drm_atomic_state *suspend;
+
+	const u64 *format_modifiers;
 };
 
 static inline struct nouveau_display *
-- 
2.17.1

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

[PATCH v2 2/3] drm/nouveau: Check framebuffer size against bo

[James Jones]

Make sure framebuffer dimensions and tiling
parameters will not result in accesses beyond the
end of the GEM buffer they are bound to.

Signed-off-by: James Jones <jajones@nvidia.com>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 93 +++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 6f038511a03a..f1509392d7b7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -224,6 +224,72 @@ static const struct drm_framebuffer_funcs nouveau_framebuffer_funcs = {
 	.create_handle = nouveau_user_framebuffer_create_handle,
 };
 
+static inline uint32_t
+nouveau_get_width_in_blocks(uint32_t stride)
+{
+	/* GOBs per block in the x direction is always one, and GOBs are
+	 * 64 bytes wide
+	 */
+	static const uint32_t log_block_width = 6;
+
+	return (stride + (1 << log_block_width) - 1) >> log_block_width;
+}
+
+static inline uint32_t
+nouveau_get_height_in_blocks(struct nouveau_drm *drm,
+			     uint32_t height,
+			     uint32_t log_block_height_in_gobs)
+{
+	uint32_t log_gob_height;
+	uint32_t log_block_height;
+
+	BUG_ON(drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA);
+
+	if (drm->client.device.info.family < NV_DEVICE_INFO_V0_FERMI)
+		log_gob_height = 2;
+	else
+		log_gob_height = 3;
+
+	log_block_height = log_block_height_in_gobs + log_gob_height;
+
+	return (height + (1 << log_block_height) - 1) >> log_block_height;
+}
+
+static int
+nouveau_check_bl_size(struct nouveau_drm *drm, struct nouveau_bo *nvbo,
+		      uint32_t offset, uint32_t stride, uint32_t h,
+		      uint32_t tile_mode)
+{
+	uint32_t gob_size, bw, bh;
+	uint64_t bl_size;
+
+	BUG_ON(drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA);
+
+	if (drm->client.device.info.chipset >= 0xc0)
+		tile_mode >>= 4;
+
+	BUG_ON(tile_mode & 0xFFFFFFF0);
+
+	if (drm->client.device.info.family < NV_DEVICE_INFO_V0_FERMI)
+		gob_size = 256;
+	else
+		gob_size = 512;
+
+	bw = nouveau_get_width_in_blocks(stride);
+	bh = nouveau_get_height_in_blocks(drm, h, tile_mode);
+
+	bl_size = bw * bh * (1 << tile_mode) * gob_size;
+
+	DRM_DEBUG_KMS("offset=%u stride=%u h=%u tile_mode=0x%02x bw=%u bh=%u gob_size=%u bl_size=%llu size=%lu\n",
+		      offset, stride, h, tile_mode, bw, bh, gob_size, bl_size,
+		      nvbo->bo.mem.size);
+
+	if (bl_size + offset > nvbo->bo.mem.size)
+		return -ERANGE;
+
+	return 0;
+}
+
 int
 nouveau_framebuffer_new(struct drm_device *dev,
 			const struct drm_mode_fb_cmd2 *mode_cmd,
@@ -232,6 +298,8 @@ nouveau_framebuffer_new(struct drm_device *dev,
 {
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nouveau_framebuffer *fb;
+	const struct drm_format_info *info;
+	unsigned int width, height, i;
 	int ret;
 
         /* YUV overlays have special requirements pre-NV50 */
@@ -254,6 +322,31 @@ nouveau_framebuffer_new(struct drm_device *dev,
 		return -EINVAL;
 	}
 
+	info = drm_get_format_info(dev, mode_cmd);
+
+	for (i = 0; i < info->num_planes; i++) {
+		width = drm_format_info_plane_width(info,
+						    mode_cmd->width,
+						    i);
+		height = drm_format_info_plane_height(info,
+						      mode_cmd->height,
+						      i);
+
+		if (nvbo->kind) {
+			ret = nouveau_check_bl_size(drm, nvbo,
+						    mode_cmd->offsets[i],
+						    mode_cmd->pitches[i],
+						    height, nvbo->mode);
+			if (ret)
+				return ret;
+		} else {
+			uint32_t size = mode_cmd->pitches[i] * height;
+
+			if (size + mode_cmd->offsets[i] > nvbo->bo.mem.size)
+				return -ERANGE;
+		}
+	}
+
 	if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
 		return -ENOMEM;
 
-- 
2.17.1

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

[PATCH v2 3/3] drm/nouveau: Support NVIDIA format modifiers

[James Jones]

Allow setting the block layout of a nouveau FB
object using DRM format modifiers.  When
specified, the format modifier block layout and
kind overrides the GEM buffer's implicit layout
and kind.  The specified format modifier is
validated against he list of modifiers supported
by the target display hardware.

v2: Used Tesla family instead of NV50 chipset compare

Signed-off-by: James Jones <jajones@nvidia.com>
---
 drivers/gpu/drm/nouveau/dispnv50/wndw.c   |  8 +--
 drivers/gpu/drm/nouveau/nouveau_display.c | 65 ++++++++++++++++++++++-
 drivers/gpu/drm/nouveau/nouveau_display.h |  2 +
 3 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 70ad64cb2d34..06c1b18479c1 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -43,7 +43,7 @@ nv50_wndw_ctxdma_new(struct nv50_wndw *wndw, struct nouveau_framebuffer *fb)
 {
 	struct nouveau_drm *drm = nouveau_drm(fb->base.dev);
 	struct nv50_wndw_ctxdma *ctxdma;
-	const u8    kind = fb->nvbo->kind;
+	const u8    kind = fb->kind;
 	const u32 handle = 0xfb000000 | kind;
 	struct {
 		struct nv_dma_v0 base;
@@ -243,7 +243,7 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw, bool modeset,
 	if (asyw->state.fb != armw->state.fb || !armw->visible || modeset) {
 		asyw->image.w = fb->base.width;
 		asyw->image.h = fb->base.height;
-		asyw->image.kind = fb->nvbo->kind;
+		asyw->image.kind = fb->kind;
 
 		ret = nv50_wndw_atomic_check_acquire_rgb(asyw);
 		if (ret) {
@@ -255,9 +255,9 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw, bool modeset,
 		if (asyw->image.kind) {
 			asyw->image.layout = 0;
 			if (drm->client.device.info.chipset >= 0xc0)
-				asyw->image.blockh = fb->nvbo->mode >> 4;
+				asyw->image.blockh = fb->tile_mode >> 4;
 			else
-				asyw->image.blockh = fb->nvbo->mode;
+				asyw->image.blockh = fb->tile_mode;
 			asyw->image.blocks[0] = fb->base.pitches[0] / 64;
 			asyw->image.pitch[0] = 0;
 		} else {
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index f1509392d7b7..50e055adebd4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -224,6 +224,50 @@ static const struct drm_framebuffer_funcs nouveau_framebuffer_funcs = {
 	.create_handle = nouveau_user_framebuffer_create_handle,
 };
 
+static int
+nouveau_decode_mod(struct nouveau_drm *drm,
+		   uint64_t modifier,
+		   uint32_t *tile_mode,
+		   uint8_t *kind)
+{
+	struct nouveau_display *disp = nouveau_display(drm->dev);
+	int mod;
+
+	BUG_ON(!tile_mode || !kind);
+
+	if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) {
+		return -EINVAL;
+	}
+
+	BUG_ON(!disp->format_modifiers);
+
+	for (mod = 0;
+	     (disp->format_modifiers[mod] != DRM_FORMAT_MOD_INVALID) &&
+	     (disp->format_modifiers[mod] != modifier);
+	     mod++);
+
+	if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID)
+		return -EINVAL;
+
+	if (modifier == DRM_FORMAT_MOD_LINEAR) {
+		/* tile_mode will not be used in this case */
+		*tile_mode = 0;
+		*kind = 0;
+	} else {
+		/*
+		 * Extract the block height and kind from the corresponding
+		 * modifier fields.  See drm_fourcc.h for details.
+		 */
+		*tile_mode = (uint32_t)(modifier & 0xF);
+		*kind = (uint8_t)((modifier >> 12) & 0xFF);
+
+		if (drm->client.device.info.chipset >= 0xc0)
+			*tile_mode <<= 4;
+	}
+
+	return 0;
+}
+
 static inline uint32_t
 nouveau_get_width_in_blocks(uint32_t stride)
 {
@@ -300,6 +344,8 @@ nouveau_framebuffer_new(struct drm_device *dev,
 	struct nouveau_framebuffer *fb;
 	const struct drm_format_info *info;
 	unsigned int width, height, i;
+	uint32_t tile_mode;
+	uint8_t kind;
 	int ret;
 
         /* YUV overlays have special requirements pre-NV50 */
@@ -322,6 +368,18 @@ nouveau_framebuffer_new(struct drm_device *dev,
 		return -EINVAL;
 	}
 
+	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
+		if (nouveau_decode_mod(drm, mode_cmd->modifier[0], &tile_mode,
+				       &kind)) {
+			DRM_DEBUG_KMS("Unsupported modifier: 0x%llx\n",
+				      mode_cmd->modifier[0]);
+			return -EINVAL;
+		}
+	} else {
+		tile_mode = nvbo->mode;
+		kind = nvbo->kind;
+	}
+
 	info = drm_get_format_info(dev, mode_cmd);
 
 	for (i = 0; i < info->num_planes; i++) {
@@ -332,11 +390,11 @@ nouveau_framebuffer_new(struct drm_device *dev,
 						      mode_cmd->height,
 						      i);
 
-		if (nvbo->kind) {
+		if (kind) {
 			ret = nouveau_check_bl_size(drm, nvbo,
 						    mode_cmd->offsets[i],
 						    mode_cmd->pitches[i],
-						    height, nvbo->mode);
+						    height, tile_mode);
 			if (ret)
 				return ret;
 		} else {
@@ -352,6 +410,8 @@ nouveau_framebuffer_new(struct drm_device *dev,
 
 	drm_helper_mode_fill_fb_struct(dev, &fb->base, mode_cmd);
 	fb->nvbo = nvbo;
+	fb->tile_mode = tile_mode;
+	fb->kind = kind;
 
 	ret = drm_framebuffer_init(dev, &fb->base, &nouveau_framebuffer_funcs);
 	if (ret)
@@ -625,6 +685,7 @@ nouveau_display_create(struct drm_device *dev)
 
 	dev->mode_config.preferred_depth = 24;
 	dev->mode_config.prefer_shadow = 1;
+	dev->mode_config.allow_fb_modifiers = true;
 
 	if (drm->client.device.info.chipset < 0x11)
 		dev->mode_config.async_page_flip = false;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index c54682f00b01..0dad57b21983 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -15,6 +15,8 @@ struct nouveau_framebuffer {
 	u32 r_handle;
 	u32 r_format;
 	u32 r_pitch;
+	u32 tile_mode;
+	u8 kind;
 	struct nvif_object h_base[4];
 	struct nvif_object h_core;
 };
-- 
2.17.1

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

Re: [Nouveau] [PATCH v2 2/3] drm/nouveau: Check framebuffer size against bo

[Ben Skeggs]

On Tue, 17 Dec 2019 at 10:45, James Jones <jajones@nvidia.com> wrote:
>
> Make sure framebuffer dimensions and tiling
> parameters will not result in accesses beyond the
> end of the GEM buffer they are bound to.
>
> Signed-off-by: James Jones <jajones@nvidia.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_display.c | 93 +++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 6f038511a03a..f1509392d7b7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -224,6 +224,72 @@ static const struct drm_framebuffer_funcs nouveau_framebuffer_funcs = {
>         .create_handle = nouveau_user_framebuffer_create_handle,
>  };
>
> +static inline uint32_t
> +nouveau_get_width_in_blocks(uint32_t stride)
> +{
> +       /* GOBs per block in the x direction is always one, and GOBs are
> +        * 64 bytes wide
> +        */
> +       static const uint32_t log_block_width = 6;
> +
> +       return (stride + (1 << log_block_width) - 1) >> log_block_width;
> +}
> +
> +static inline uint32_t
> +nouveau_get_height_in_blocks(struct nouveau_drm *drm,
> +                            uint32_t height,
> +                            uint32_t log_block_height_in_gobs)
> +{
> +       uint32_t log_gob_height;
> +       uint32_t log_block_height;
> +
> +       BUG_ON(drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA);
> +
> +       if (drm->client.device.info.family < NV_DEVICE_INFO_V0_FERMI)
> +               log_gob_height = 2;
> +       else
> +               log_gob_height = 3;
> +
> +       log_block_height = log_block_height_in_gobs + log_gob_height;
> +
> +       return (height + (1 << log_block_height) - 1) >> log_block_height;
> +}
> +
> +static int
> +nouveau_check_bl_size(struct nouveau_drm *drm, struct nouveau_bo *nvbo,
> +                     uint32_t offset, uint32_t stride, uint32_t h,
> +                     uint32_t tile_mode)
> +{
> +       uint32_t gob_size, bw, bh;
> +       uint64_t bl_size;
> +
> +       BUG_ON(drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA);
> +
> +       if (drm->client.device.info.chipset >= 0xc0)
> +               tile_mode >>= 4;
> +
> +       BUG_ON(tile_mode & 0xFFFFFFF0);
As far as I can tell, tile_mode can be fed into this function
unsanitised from userspace, so we probably want something different to
a BUG_ON() here.

> +
> +       if (drm->client.device.info.family < NV_DEVICE_INFO_V0_FERMI)
> +               gob_size = 256;
> +       else
> +               gob_size = 512;
> +
> +       bw = nouveau_get_width_in_blocks(stride);
> +       bh = nouveau_get_height_in_blocks(drm, h, tile_mode);
> +
> +       bl_size = bw * bh * (1 << tile_mode) * gob_size;
> +
> +       DRM_DEBUG_KMS("offset=%u stride=%u h=%u tile_mode=0x%02x bw=%u bh=%u gob_size=%u bl_size=%llu size=%lu\n",
> +                     offset, stride, h, tile_mode, bw, bh, gob_size, bl_size,
> +                     nvbo->bo.mem.size);
> +
> +       if (bl_size + offset > nvbo->bo.mem.size)
> +               return -ERANGE;
> +
> +       return 0;
> +}
> +
>  int
>  nouveau_framebuffer_new(struct drm_device *dev,
>                         const struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -232,6 +298,8 @@ nouveau_framebuffer_new(struct drm_device *dev,
>  {
>         struct nouveau_drm *drm = nouveau_drm(dev);
>         struct nouveau_framebuffer *fb;
> +       const struct drm_format_info *info;
> +       unsigned int width, height, i;
>         int ret;
>
>          /* YUV overlays have special requirements pre-NV50 */
> @@ -254,6 +322,31 @@ nouveau_framebuffer_new(struct drm_device *dev,
>                 return -EINVAL;
>         }
>
> +       info = drm_get_format_info(dev, mode_cmd);
> +
> +       for (i = 0; i < info->num_planes; i++) {
> +               width = drm_format_info_plane_width(info,
> +                                                   mode_cmd->width,
> +                                                   i);
> +               height = drm_format_info_plane_height(info,
> +                                                     mode_cmd->height,
> +                                                     i);
> +
> +               if (nvbo->kind) {
> +                       ret = nouveau_check_bl_size(drm, nvbo,
> +                                                   mode_cmd->offsets[i],
> +                                                   mode_cmd->pitches[i],
> +                                                   height, nvbo->mode);
> +                       if (ret)
> +                               return ret;
> +               } else {
> +                       uint32_t size = mode_cmd->pitches[i] * height;
> +
> +                       if (size + mode_cmd->offsets[i] > nvbo->bo.mem.size)
> +                               return -ERANGE;
> +               }
> +       }
> +
>         if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
>                 return -ENOMEM;
>
> --
> 2.17.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH v2 0/3] drm/nouveau: Support NVIDIA format modifiers

[Ben Skeggs]

On Tue, 17 Dec 2019 at 10:44, James Jones <jajones@nvidia.com> wrote:
>
> This series modifies the NV5x+ nouveau display backends to advertise
> appropriate format modifiers on their display planes in atomic mode
> setting blobs.
>
> Corresponding modifications to Mesa/userspace are available here:
>
> https://gitlab.freedesktop.org/cubanismo/mesa/tree/nouveau_work
>
> But those need a bit of cleanup before they're ready to submit.
>
> I've tested this on Tesla, Kepler, Pascal, and Turing-class hardware
> using various formats and all the exposed format modifiers, plus some
> negative testing with invalid ones.
>
> NOTE: this series depends on the "[PATCH v3] drm: Generalized NV Block
> Linear DRM format mod" patch submitted to dri-devel.
>
> v2: Used Tesla family instead of NV50 chipset compare to avoid treating
>     oddly numbered NV4x-class chipsets as NV50+ GPUs.  Other instances
>     of compares with chipset number in the series were audited, deemed
>     safe, and left as-is for consistency with existing code.
Hey James,

These look OK to me, with the minor issue I mentioned on one of the
patches dealt with.  I'll hold off merging anything until I get the
go-ahead that the modifier definitions are definitely set in stone /
userspace is ready for inclusion.

Thanks,
Ben.

>
> James Jones (3):
>   drm/nouveau: Add format mod prop to base/ovly/nvdisp
>   drm/nouveau: Check framebuffer size against bo
>   drm/nouveau: Support NVIDIA format modifiers
>
>  drivers/gpu/drm/nouveau/dispnv50/base507c.c |   7 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c     |  59 ++++++++
>  drivers/gpu/drm/nouveau/dispnv50/disp.h     |   4 +
>  drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  35 ++++-
>  drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  17 +++
>  drivers/gpu/drm/nouveau/nouveau_display.c   | 154 ++++++++++++++++++++
>  drivers/gpu/drm/nouveau/nouveau_display.h   |   4 +
>  7 files changed, 272 insertions(+), 8 deletions(-)
>
> --
> 2.17.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH v2 2/3] drm/nouveau: Check framebuffer size against bo

[James Jones]

On 1/5/20 5:25 PM, Ben Skeggs wrote:
> On Tue, 17 Dec 2019 at 10:45, James Jones <jajones@nvidia.com> wrote:
>>
>> Make sure framebuffer dimensions and tiling
>> parameters will not result in accesses beyond the
>> end of the GEM buffer they are bound to.
>>
>> Signed-off-by: James Jones <jajones@nvidia.com>
>> ---
>>   drivers/gpu/drm/nouveau/nouveau_display.c | 93 +++++++++++++++++++++++
>>   1 file changed, 93 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
>> index 6f038511a03a..f1509392d7b7 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
>> @@ -224,6 +224,72 @@ static const struct drm_framebuffer_funcs nouveau_framebuffer_funcs = {
>>          .create_handle = nouveau_user_framebuffer_create_handle,
>>   };
>>
>> +static inline uint32_t
>> +nouveau_get_width_in_blocks(uint32_t stride)
>> +{
>> +       /* GOBs per block in the x direction is always one, and GOBs are
>> +        * 64 bytes wide
>> +        */
>> +       static const uint32_t log_block_width = 6;
>> +
>> +       return (stride + (1 << log_block_width) - 1) >> log_block_width;
>> +}
>> +
>> +static inline uint32_t
>> +nouveau_get_height_in_blocks(struct nouveau_drm *drm,
>> +                            uint32_t height,
>> +                            uint32_t log_block_height_in_gobs)
>> +{
>> +       uint32_t log_gob_height;
>> +       uint32_t log_block_height;
>> +
>> +       BUG_ON(drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA);
>> +
>> +       if (drm->client.device.info.family < NV_DEVICE_INFO_V0_FERMI)
>> +               log_gob_height = 2;
>> +       else
>> +               log_gob_height = 3;
>> +
>> +       log_block_height = log_block_height_in_gobs + log_gob_height;
>> +
>> +       return (height + (1 << log_block_height) - 1) >> log_block_height;
>> +}
>> +
>> +static int
>> +nouveau_check_bl_size(struct nouveau_drm *drm, struct nouveau_bo *nvbo,
>> +                     uint32_t offset, uint32_t stride, uint32_t h,
>> +                     uint32_t tile_mode)
>> +{
>> +       uint32_t gob_size, bw, bh;
>> +       uint64_t bl_size;
>> +
>> +       BUG_ON(drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA);
>> +
>> +       if (drm->client.device.info.chipset >= 0xc0)
>> +               tile_mode >>= 4;
>> +
>> +       BUG_ON(tile_mode & 0xFFFFFFF0);
> As far as I can tell, tile_mode can be fed into this function
> unsanitised from userspace, so we probably want something different to
> a BUG_ON() here.

Good catch.  I had assumed nouveau_bo::mode was validated at creation 
time.  I'll get this fixed up.

Thanks,
-James

>> +
>> +       if (drm->client.device.info.family < NV_DEVICE_INFO_V0_FERMI)
>> +               gob_size = 256;
>> +       else
>> +               gob_size = 512;
>> +
>> +       bw = nouveau_get_width_in_blocks(stride);
>> +       bh = nouveau_get_height_in_blocks(drm, h, tile_mode);
>> +
>> +       bl_size = bw * bh * (1 << tile_mode) * gob_size;
>> +
>> +       DRM_DEBUG_KMS("offset=%u stride=%u h=%u tile_mode=0x%02x bw=%u bh=%u gob_size=%u bl_size=%llu size=%lu\n",
>> +                     offset, stride, h, tile_mode, bw, bh, gob_size, bl_size,
>> +                     nvbo->bo.mem.size);
>> +
>> +       if (bl_size + offset > nvbo->bo.mem.size)
>> +               return -ERANGE;
>> +
>> +       return 0;
>> +}
>> +
>>   int
>>   nouveau_framebuffer_new(struct drm_device *dev,
>>                          const struct drm_mode_fb_cmd2 *mode_cmd,
>> @@ -232,6 +298,8 @@ nouveau_framebuffer_new(struct drm_device *dev,
>>   {
>>          struct nouveau_drm *drm = nouveau_drm(dev);
>>          struct nouveau_framebuffer *fb;
>> +       const struct drm_format_info *info;
>> +       unsigned int width, height, i;
>>          int ret;
>>
>>           /* YUV overlays have special requirements pre-NV50 */
>> @@ -254,6 +322,31 @@ nouveau_framebuffer_new(struct drm_device *dev,
>>                  return -EINVAL;
>>          }
>>
>> +       info = drm_get_format_info(dev, mode_cmd);
>> +
>> +       for (i = 0; i < info->num_planes; i++) {
>> +               width = drm_format_info_plane_width(info,
>> +                                                   mode_cmd->width,
>> +                                                   i);
>> +               height = drm_format_info_plane_height(info,
>> +                                                     mode_cmd->height,
>> +                                                     i);
>> +
>> +               if (nvbo->kind) {
>> +                       ret = nouveau_check_bl_size(drm, nvbo,
>> +                                                   mode_cmd->offsets[i],
>> +                                                   mode_cmd->pitches[i],
>> +                                                   height, nvbo->mode);
>> +                       if (ret)
>> +                               return ret;
>> +               } else {
>> +                       uint32_t size = mode_cmd->pitches[i] * height;
>> +
>> +                       if (size + mode_cmd->offsets[i] > nvbo->bo.mem.size)
>> +                               return -ERANGE;
>> +               }
>> +       }
>> +
>>          if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
>>                  return -ENOMEM;
>>
>> --
>> 2.17.1
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH v2 0/3] drm/nouveau: Support NVIDIA format modifiers

[James Jones]

On 1/5/20 5:30 PM, Ben Skeggs wrote:
> On Tue, 17 Dec 2019 at 10:44, James Jones <jajones@nvidia.com> wrote:
>>
>> This series modifies the NV5x+ nouveau display backends to advertise
>> appropriate format modifiers on their display planes in atomic mode
>> setting blobs.
>>
>> Corresponding modifications to Mesa/userspace are available here:
>>
>> https://gitlab.freedesktop.org/cubanismo/mesa/tree/nouveau_work
>>
>> But those need a bit of cleanup before they're ready to submit.
>>
>> I've tested this on Tesla, Kepler, Pascal, and Turing-class hardware
>> using various formats and all the exposed format modifiers, plus some
>> negative testing with invalid ones.
>>
>> NOTE: this series depends on the "[PATCH v3] drm: Generalized NV Block
>> Linear DRM format mod" patch submitted to dri-devel.
>>
>> v2: Used Tesla family instead of NV50 chipset compare to avoid treating
>>      oddly numbered NV4x-class chipsets as NV50+ GPUs.  Other instances
>>      of compares with chipset number in the series were audited, deemed
>>      safe, and left as-is for consistency with existing code.
> Hey James,
> 
> These look OK to me, with the minor issue I mentioned on one of the
> patches dealt with.  I'll hold off merging anything until I get the
> go-ahead that the modifier definitions are definitely set in stone /
> userspace is ready for inclusion.

Thanks for having a look.  I'll try to get the userspace changes 
finalized soon.  I think from the NV side, we consider the modifier 
definition itself (the v3 version of the patch) final, so if there's any 
stand-alone feedback from yourself or other drm/nouveau developers on 
that layout, we'd be eager to hear it.  I don't want it rushed in, but 
we do have several projects blocked on getting that approved & committed.

I assume the sequencing should be:

* Fix the minor issue you identified here/complete review of nouveau 
kernel patches
* Complete review of the related TegraDRM new modifier support patch
* Finalize and complete review of userspace/Mesa nouveau modifier 
support patches
* Get drm_fourcc.h updates committed
* Get these patches and TegraDRM patches committed
* Integrate final drm_fourcc.h to Mesa patches and get Mesa patches 
committed

Does that sound right to you?

Thanks,
-James

> Thanks,
> Ben.
> 
>>
>> James Jones (3):
>>    drm/nouveau: Add format mod prop to base/ovly/nvdisp
>>    drm/nouveau: Check framebuffer size against bo
>>    drm/nouveau: Support NVIDIA format modifiers
>>
>>   drivers/gpu/drm/nouveau/dispnv50/base507c.c |   7 +-
>>   drivers/gpu/drm/nouveau/dispnv50/disp.c     |  59 ++++++++
>>   drivers/gpu/drm/nouveau/dispnv50/disp.h     |   4 +
>>   drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  35 ++++-
>>   drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  17 +++
>>   drivers/gpu/drm/nouveau/nouveau_display.c   | 154 ++++++++++++++++++++
>>   drivers/gpu/drm/nouveau/nouveau_display.h   |   4 +
>>   7 files changed, 272 insertions(+), 8 deletions(-)
>>
>> --
>> 2.17.1
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH v2 0/3] drm/nouveau: Support NVIDIA format modifiers

[Ben Skeggs]

On Tue, 7 Jan 2020 at 05:17, James Jones <jajones@nvidia.com> wrote:
>
> On 1/5/20 5:30 PM, Ben Skeggs wrote:
> > On Tue, 17 Dec 2019 at 10:44, James Jones <jajones@nvidia.com> wrote:
> >>
> >> This series modifies the NV5x+ nouveau display backends to advertise
> >> appropriate format modifiers on their display planes in atomic mode
> >> setting blobs.
> >>
> >> Corresponding modifications to Mesa/userspace are available here:
> >>
> >> https://gitlab.freedesktop.org/cubanismo/mesa/tree/nouveau_work
> >>
> >> But those need a bit of cleanup before they're ready to submit.
> >>
> >> I've tested this on Tesla, Kepler, Pascal, and Turing-class hardware
> >> using various formats and all the exposed format modifiers, plus some
> >> negative testing with invalid ones.
> >>
> >> NOTE: this series depends on the "[PATCH v3] drm: Generalized NV Block
> >> Linear DRM format mod" patch submitted to dri-devel.
> >>
> >> v2: Used Tesla family instead of NV50 chipset compare to avoid treating
> >>      oddly numbered NV4x-class chipsets as NV50+ GPUs.  Other instances
> >>      of compares with chipset number in the series were audited, deemed
> >>      safe, and left as-is for consistency with existing code.
> > Hey James,
> >
> > These look OK to me, with the minor issue I mentioned on one of the
> > patches dealt with.  I'll hold off merging anything until I get the
> > go-ahead that the modifier definitions are definitely set in stone /
> > userspace is ready for inclusion.
>
> Thanks for having a look.  I'll try to get the userspace changes
> finalized soon.  I think from the NV side, we consider the modifier
> definition itself (the v3 version of the patch) final, so if there's any
> stand-alone feedback from yourself or other drm/nouveau developers on
> that layout, we'd be eager to hear it.  I don't want it rushed in, but
> we do have several projects blocked on getting that approved & committed.
>
> I assume the sequencing should be:
>
> * Fix the minor issue you identified here/complete review of nouveau
> kernel patches
> * Complete review of the related TegraDRM new modifier support patch
> * Finalize and complete review of userspace/Mesa nouveau modifier
> support patches
> * Get drm_fourcc.h updates committed
> * Get these patches and TegraDRM patches committed
> * Integrate final drm_fourcc.h to Mesa patches and get Mesa patches
> committed
>
> Does that sound right to you?
Seems very reasonable!

Ben.

>
> Thanks,
> -James
>
> > Thanks,
> > Ben.
> >
> >>
> >> James Jones (3):
> >>    drm/nouveau: Add format mod prop to base/ovly/nvdisp
> >>    drm/nouveau: Check framebuffer size against bo
> >>    drm/nouveau: Support NVIDIA format modifiers
> >>
> >>   drivers/gpu/drm/nouveau/dispnv50/base507c.c |   7 +-
> >>   drivers/gpu/drm/nouveau/dispnv50/disp.c     |  59 ++++++++
> >>   drivers/gpu/drm/nouveau/dispnv50/disp.h     |   4 +
> >>   drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  35 ++++-
> >>   drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  17 +++
> >>   drivers/gpu/drm/nouveau/nouveau_display.c   | 154 ++++++++++++++++++++
> >>   drivers/gpu/drm/nouveau/nouveau_display.h   |   4 +
> >>   7 files changed, 272 insertions(+), 8 deletions(-)
> >>
> >> --
> >> 2.17.1
> >>
> >> _______________________________________________
> >> Nouveau mailing list
> >> Nouveau@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH v2 0/3] drm/nouveau: Support NVIDIA format modifiers

[James Jones]

On 1/6/20 3:27 PM, Ben Skeggs wrote:
> On Tue, 7 Jan 2020 at 05:17, James Jones <jajones@nvidia.com> wrote:
>>
>> On 1/5/20 5:30 PM, Ben Skeggs wrote:
>>> On Tue, 17 Dec 2019 at 10:44, James Jones <jajones@nvidia.com> wrote:
>>>>
>>>> This series modifies the NV5x+ nouveau display backends to advertise
>>>> appropriate format modifiers on their display planes in atomic mode
>>>> setting blobs.
>>>>
>>>> Corresponding modifications to Mesa/userspace are available here:
>>>>
>>>> https://gitlab.freedesktop.org/cubanismo/mesa/tree/nouveau_work
>>>>
>>>> But those need a bit of cleanup before they're ready to submit.
>>>>
>>>> I've tested this on Tesla, Kepler, Pascal, and Turing-class hardware
>>>> using various formats and all the exposed format modifiers, plus some
>>>> negative testing with invalid ones.
>>>>
>>>> NOTE: this series depends on the "[PATCH v3] drm: Generalized NV Block
>>>> Linear DRM format mod" patch submitted to dri-devel.
>>>>
>>>> v2: Used Tesla family instead of NV50 chipset compare to avoid treating
>>>>       oddly numbered NV4x-class chipsets as NV50+ GPUs.  Other instances
>>>>       of compares with chipset number in the series were audited, deemed
>>>>       safe, and left as-is for consistency with existing code.
>>> Hey James,
>>>
>>> These look OK to me, with the minor issue I mentioned on one of the
>>> patches dealt with.  I'll hold off merging anything until I get the
>>> go-ahead that the modifier definitions are definitely set in stone /
>>> userspace is ready for inclusion.
>>
>> Thanks for having a look.  I'll try to get the userspace changes
>> finalized soon.  I think from the NV side, we consider the modifier
>> definition itself (the v3 version of the patch) final, so if there's any
>> stand-alone feedback from yourself or other drm/nouveau developers on
>> that layout, we'd be eager to hear it.  I don't want it rushed in, but
>> we do have several projects blocked on getting that approved & committed.
>>
>> I assume the sequencing should be:
>>
>> * Fix the minor issue you identified here/complete review of nouveau
>> kernel patches
>> * Complete review of the related TegraDRM new modifier support patch
>> * Finalize and complete review of userspace/Mesa nouveau modifier
>> support patches
>> * Get drm_fourcc.h updates committed
>> * Get these patches and TegraDRM patches committed
>> * Integrate final drm_fourcc.h to Mesa patches and get Mesa patches
>> committed
>>
>> Does that sound right to you?
> Seems very reasonable!

Thanks.  I needed to do more cleanup than I expected (a rewrite in the 
end), but the corresponding Mesa patches are out for review now, and 
I've sent out v3 of this patchset to address the remaining issue raised 
here.

Thanks,
-James

> Ben.
> 
>>
>> Thanks,
>> -James
>>
>>> Thanks,
>>> Ben.
>>>
>>>>
>>>> James Jones (3):
>>>>     drm/nouveau: Add format mod prop to base/ovly/nvdisp
>>>>     drm/nouveau: Check framebuffer size against bo
>>>>     drm/nouveau: Support NVIDIA format modifiers
>>>>
>>>>    drivers/gpu/drm/nouveau/dispnv50/base507c.c |   7 +-
>>>>    drivers/gpu/drm/nouveau/dispnv50/disp.c     |  59 ++++++++
>>>>    drivers/gpu/drm/nouveau/dispnv50/disp.h     |   4 +
>>>>    drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  35 ++++-
>>>>    drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  17 +++
>>>>    drivers/gpu/drm/nouveau/nouveau_display.c   | 154 ++++++++++++++++++++
>>>>    drivers/gpu/drm/nouveau/nouveau_display.h   |   4 +
>>>>    7 files changed, 272 insertions(+), 8 deletions(-)
>>>>
>>>> --
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> Nouveau mailing list
>>>> Nouveau@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel