Re: [RFC 2/6] drm/i915: Add tiled framebuffer modifiers

Daniel Vetter
2015-02-02T15:56:58Z

On Mon, Feb 02, 2015 at 10:23:57AM +0000, Tvrtko Ursulin wrote:
>
> On 02/02/2015 09:58 AM, Daniel Vetter wrote:
> >On Mon, Feb 02, 2015 at 10:41:24AM +0100, Daniel Vetter wrote:
> >>On Fri, Jan 30, 2015 at 05:36:54PM +0000, Tvrtko Ursulin wrote:
> >>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>
> >>>To be used from the new addfb2 extension.
> >>>
> >>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>---
> >>> include/uapi/drm/i915_drm.h | 13 +++++++++++++
> >>> 1 file changed, 13 insertions(+)
> >>>
> >>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> >>>index 6eed16b..a7327fd 100644
> >>>--- a/include/uapi/drm/i915_drm.h
> >>>+++ b/include/uapi/drm/i915_drm.h
> >>>@@ -28,6 +28,7 @@
> >>> #define _UAPI_I915_DRM_H_
> >>>
> >>> #include <drm/drm.h>
> >>>+#include <uapi/drm/drm_fourcc.h>
> >>>
> >>> /* Please note that modifications to all structs defined here are
> >>> * subject to backwards-compatibility constraints.
> >>>@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
> >>> __u64 value;
> >>> };
> >>>
> >>>+/** @{
> >>>+ * Intel framebuffer modifiers
> >>>+ *
> >>>+ * Tiling modes supported by the display hardware
> >>>+ * to be passed in via the DRM addfb2 ioctl.
> >>>+ */
> >>>+/** None */
> >>>+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00000000000000L)
> >>>+/** X tiling */
> >>>+#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL, 0x00000000000001L)
> >>
> >>One thing I wonder here is whether we should have a modifier for each
> >>physical layout (tiling modes do change slightly between hw) or whether we
> >>should just continue to assume that this is Intel-specific and add a
> >>disclaimer that the precise layout depends upon the actual intel box
> >>you're running on?
> >>
> >>Leaning towards your approach, worst case we get to write some code to
> >>de-alias layout modifiers with established cross-vendor layouts (if they
> >>ever happen). Just want to make sure that we've thought about this. Adding
> >>Rob&dri-devel for this.
> >
> >Something else to ponder: We also need layout modifiers for non-fb formats
> >in userspace so that clients and compositors can communicate about render
> >formats. Given that I think it'll make sense to enumerate all the other
> >tiling formats we have, too (i.e. Y-tiled and W-tiled).
>
> If we need fb modifiers for non-fb formats, although that sounds a bit funky
> to me, we can always add them in separate patches, no?

Oh and the explanation of why this makes sense: Userspace needs to agree
on some modifier numbers assignment too for its purposes of sharing
buffers between clients and compositor. And there's a lot of overlap with
buffers that can actually be scanned out (for the obvious reason called
fullscreen apps), so it makes sense to reuse those numbers instead of
everyone creating their own spec.

But then we need to make sure that non-fb modifiers of interest as used in
userspace aren't eventually used by the kernel for something else. Hence
they need to go into the kernel headers, just to reserve the numbers.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch