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(-)
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 *
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;
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;elselog_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;elsegob_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
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;elselog_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;elsegob_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
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; };
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
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
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
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@lists.freedesktop.org
-
Ben Skeggs -
James Jones