[Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
Daniel Vetter
daniel.vetter at ffwll.ch
Wed Jan 28 09:37:52 PST 2015
From: Rob Clark <robdclark at gmail.com>
In DRM/KMS we are lacking a good way to deal with tiled/compressed
formats. Especially in the case of dmabuf/prime buffer sharing, where
we cannot always rely on under-the-hood flags passed to driver specific
gem-create ioctl to pass around these extra flags.
The proposal is to add a per-plane format modifier. This allows to, if
necessary, use different tiling patters for sub-sampled planes, etc.
The format modifiers are added at the end of the ioctl struct, so for
legacy userspace it will be zero padded.
TODO how best to deal with assignment of modifier token values? The
rough idea was to namespace things with an 8bit vendor-id, and then
beyond that it is treated as an opaque value. But that was a relatively
arbitrary choice. There are cases where same tiling pattern and/or
compression is supported by various different vendors. So we should
standardize to use the vendor-id and value of the first one who
documents the format?
v1: original
v1.5: increase modifier to 64b
v2: Incorporate review comments from the big thread, plus a few more.
- Add a getcap so that userspace doesn't have to jump through hoops.
- Allow modifiers only when a flag is set. That way drivers know when
they're dealing with old userspace and need to fish out e.g. tiling
from other information.
- After rolling out checks for ->modifier to all drivers I've decided
that this is way too fragile and needs an explicit opt-in flag. So
do that instead.
- Add a define (just for documentation really) for the "NONE"
modifier. Imo we don't need to add mask #defines since drivers
really should only do exact matches against values defined with
fourcc_mod_code.
- Drop the Samsung tiling modifier on Rob's request since he's not yet
sure whether that one is accurate.
Cc: Rob Clark <robdclark at gmail.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Cc: Daniel Stone <daniel at fooishbar.org>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Michel Dänzer <michel at daenzer.net>
Signed-off-by: Rob Clark <robdclark at gmail.com> (v1.5)
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
--
I've picked this up since we want to push out some fancy new tiling
modes soonish. No defines yet, but Tvrkto is working on the i915 parts
of this.
I think the only part I haven't done from the discussion is exposing a
list of supported modifiers. Not sure that's really useful though
since all this is highly hw specific.
And a note to driver writes: They need to check or the flag and in its
absence make a reasonable choice about the internal layet (e.g. for
i915 consult the tiling mode of the underlying bo).
-Daniel
---
drivers/gpu/drm/drm_crtc.c | 14 +++++++++++++-
drivers/gpu/drm/drm_ioctl.c | 3 +++
include/drm/drm_crtc.h | 3 +++
include/uapi/drm/drm.h | 1 +
include/uapi/drm/drm_fourcc.h | 26 ++++++++++++++++++++++++++
include/uapi/drm/drm_mode.h | 9 +++++++++
6 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 419f9d915c78..8090e3d64f67 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3324,6 +3324,12 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i);
return -EINVAL;
}
+
+ if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) {
+ DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",
+ r->modifier[i], i);
+ return -EINVAL;
+ }
}
return 0;
@@ -3337,7 +3343,7 @@ static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev,
struct drm_framebuffer *fb;
int ret;
- if (r->flags & ~DRM_MODE_FB_INTERLACED) {
+ if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
return ERR_PTR(-EINVAL);
}
@@ -3353,6 +3359,12 @@ static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev,
return ERR_PTR(-EINVAL);
}
+ if (r->flags & DRM_MODE_FB_MODIFIERS &&
+ !dev->mode_config.allow_fb_modifiers) {
+ DRM_DEBUG_KMS("driver does not support fb modifiers\n");
+ return ERR_PTR(-EINVAL);
+ }
+
ret = framebuffer_check(r);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 3785d66721f2..a6d773a61c2d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -321,6 +321,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
else
req->value = 64;
break;
+ case DRM_CAP_ADDFB2_MODIFIERS:
+ req->value = dev->mode_config.allow_fb_modifiers;
+ break;
default:
return -EINVAL;
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 019c9b562144..767f7d4cab63 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1153,6 +1153,9 @@ struct drm_mode_config {
/* whether async page flip is supported or not */
bool async_page_flip;
+ /* whether the driver supports fb modifiers */
+ bool allow_fb_modifiers;
+
/* cursor size */
uint32_t cursor_width, cursor_height;
};
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 01b2d6d0e355..ff6ef62d084b 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -630,6 +630,7 @@ struct drm_gem_open {
*/
#define DRM_CAP_CURSOR_WIDTH 0x8
#define DRM_CAP_CURSOR_HEIGHT 0x9
+#define DRM_CAP_ADDFB2_MODIFIERS 0x10
/** DRM_IOCTL_GET_CAP ioctl argument type */
struct drm_get_cap {
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 646ae5f39f42..bf99557f1624 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -132,4 +132,30 @@
#define DRM_FORMAT_YUV444 fourcc_code('Y', 'U', '2', '4') /* non-subsampled Cb (1) and Cr (2) planes */
#define DRM_FORMAT_YVU444 fourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */
+
+/*
+ * Format Modifiers:
+ *
+ * Format modifiers describe, typically, a re-ordering or modification
+ * of the data in a plane of an FB. This can be used to express tiled/
+ * swizzled formats, or compression, or a combination of the two.
+ *
+ * The upper 8 bits of the format modifier are a vendor-id as assigned
+ * below. The lower 56 bits are assigned as vendor sees fit.
+ */
+
+/* Vendor Ids: */
+#define DRM_FORMAT_MOD_NONE 0
+#define DRM_FORMAT_MOD_VENDOR_INTEL 0x01
+#define DRM_FORMAT_MOD_VENDOR_AMD 0x02
+#define DRM_FORMAT_MOD_VENDOR_NV 0x03
+#define DRM_FORMAT_MOD_VENDOR_SAMSUNG 0x04
+#define DRM_FORMAT_MOD_VENDOR_QCOM 0x05
+/* add more to the end as needed */
+
+#define fourcc_mod_code(vendor, val) \
+ ((((u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 0x00ffffffffffffffL)
+
+/* Format Modifier tokens: */
+
#endif /* DRM_FOURCC_H */
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index ca788e01dab2..dbeba949462a 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -336,6 +336,7 @@ struct drm_mode_fb_cmd {
};
#define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */
+#define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */
struct drm_mode_fb_cmd2 {
__u32 fb_id;
@@ -356,10 +357,18 @@ struct drm_mode_fb_cmd2 {
* So it would consist of Y as offsets[0] and UV as
* offsets[1]. Note that offsets[0] will generally
* be 0 (but this is not required).
+ *
+ * To accommodate tiled, compressed, etc formats, a per-plane
+ * modifier can be specified. The default value of zero
+ * indicates "native" format as specified by the fourcc.
+ * Vendor specific modifier token. This allows, for example,
+ * different tiling/swizzling pattern on different planes.
+ * See discussion above of DRM_FORMAT_MOD_xxx.
*/
__u32 handles[4];
__u32 pitches[4]; /* pitch for each plane */
__u32 offsets[4]; /* offset of each plane */
+ __u64 modifier[4]; /* ie, tiling, compressed (per plane) */
};
#define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01
--
2.1.4
More information about the Intel-gfx
mailing list