Re: [Nouveau] [PATCH 3/3] drm/nouveau: Support NVIDIA format modifiers
On 12/12/19 6:51 PM, James Jones wrote:
On 12/11/19 1:13 PM, Ilia Mirkin wrote:
On Wed, Dec 11, 2019 at 4:04 PM James Jones wrote:
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.
Signed-off-by: James Jones
---
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 = 0xfb00 | 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..351b58410e1a 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.chipset < 0x50) {
Not a full review, but you want to go off the family (chip_class iirc?
something like that, should be obvious). Sadly 0x67/0x68 are higher
than 0x50 numerically, but are logically part of the nv4x generation.
Good catch. I'll get this fixed and send out an updated patchset.
I fixed this one instance in the v2 series, and I didn't see any other
potentially dangerous uses of chipset, so I left the others as-is, as
they seemed to better match surrounding code or existing checks used for
a given bit of functionality.
Thanks,
-James
+ 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;
Re: [Nouveau] [PATCH 3/3] drm/nouveau: Support NVIDIA format modifiers
On 12/11/19 1:13 PM, Ilia Mirkin wrote:
On Wed, Dec 11, 2019 at 4:04 PM James Jones wrote:
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.
Signed-off-by: James Jones
---
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 u8kind = fb->nvbo->kind;
+ const u8kind = fb->kind;
const u32 handle = 0xfb00 | 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..351b58410e1a 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.chipset < 0x50) {
Not a full review, but you want to go off the family (chip_class iirc?
something like that, should be obvious). Sadly 0x67/0x68 are higher
than 0x50 numerically, but are logically part of the nv4x generation.
Good catch. I'll get this fixed and send out an updated patchset.
Thanks,
-James
+ 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
Re: [Nouveau] [PATCH 3/3] drm/nouveau: Support NVIDIA format modifiers
On Wed, Dec 11, 2019 at 4:04 PM James Jones wrote:
>
> 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.
>
> Signed-off-by: James Jones
> ---
> 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 u8kind = fb->nvbo->kind;
> + const u8kind = fb->kind;
> const u32 handle = 0xfb00 | 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..351b58410e1a 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.chipset < 0x50) {
Not a full review, but you want to go off the family (chip_class iirc?
something like that, should be obvious). Sadly 0x67/0x68 are higher
than 0x50 numerically, but are logically part of the nv4x generation.
> + 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 @@
