| Message ID | 1422639418-28845-3-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -Daniel > +/** @} */ > + > #endif /* _UAPI_I915_DRM_H_ */ > -- > 2.2.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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). -Daniel
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? Regards, Tvrtko
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? Yes and no - I think the aliasing with the I915_TILING_FOO defines would be nice, and if you reserve another number for the fancy new tiling you're working on and so block Y-tiled that would be unfortunate ... Otoh meh, we need to remap anyway sooner or later. Like I've said, just something to consider. -Daniel
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
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.. BR, -R > 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. > -Daniel > >> +/** @} */ >> + >> #endif /* _UAPI_I915_DRM_H_ */ >> -- >> 2.2.2 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Feb 2, 2015 at 10:58 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > 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. right.. the next logical step is to extend the egl dmabuf extension to take modifiers in the same way as addfb2 does. So it makes sense to reserve/enumerate any sharable modifier, even if it is not ever used for scanout. As w/ fourcc's, it will be nice to keep the egl extension to keep the same formats and modifiers. BR, -R > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
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? Regards, Tvrtko
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. 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. -Daniel
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
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) +/** @} */ + #endif /* _UAPI_I915_DRM_H_ */