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

Rob Clark
2015-02-02T19:25:56Z

On Mon, Feb 2, 2015 at 11:59 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Feb 02, 2015 at 04:42:32PM +0000, Tvrtko Ursulin wrote:
>>
>> On 02/02/2015 04:32 PM, Rob Clark wrote:
>> >On Mon, Feb 2, 2015 at 4:41 AM, Daniel Vetter <daniel@ffwll.ch> 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?
>> >
>> >I'd kind of lean towards different modifiers per physical layout..
>> >that seems more useful for cases where nvidia/amd support some of the
>> >formats for buffer sharing..
>>
>> Hm.. we've got physical layout, alignment restrictions, geometry
>> restrictions, what are the odds this will be shareable or compatible, and
>> how will the token names even looks when one puts all of this into them?
>
> On top of that there's a _lot_ of different physical layouts for just X
> tiling. At least if you look at more than just modern platforms. And often
> userspace doesn't even know which precise variant it is.

hmm, if userspace doesn't know the format, that doesn't bode well for
sharing.. but in that case I915_FORMAT_MOD_DTRT alias might make
sense..

> I think if we eventually have a match with some other vendor format (the
> one with nvidia wasn't intentionally, it only works if you have swizzling
> enabled, not without swizzling) then we could do some aliasing: Define a
> new vendor neutral code which then all drivers supporting it would remap
> to the correct internal/vendor-specific representation.
>
> Of course integrated gpus are special, with plug-in pci devices you really
> have to spec the full thing.

the problem is if you are going to be sharing with another gpu, that
one is going to be plug-in ;-)

BR,
-R

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch