Re: [RFC] drm: add support for tiled/compressed/etc modifier in addfb2

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Dec 12, 2014 at 04:33:38PM -0500, Rob Clark wrote:
> On Fri, Dec 12, 2014 at 3:57 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > Hi Rob,
> >
> > On Wednesday 10 December 2014 12:17:51 Rob Clark wrote:
> >> 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.
> >
> > But it will change the size of the structure, and thus the ioctl value. You
> > can't extend existing structures used in ioctls I'm afraid.
> 
> Actually, that is why it will work.  Old userspace passes smaller
> size, drm_ioctl() zero pads the difference..
> 
> The issue is (potentially) in the other direction (new userspace, old
> kernel) since the old kernel will ignore the new fields.  But that can
> be sorted w/ a cap/query

Or by using up one flag in the ioctl to specify whether the new fields
are valid or not.

> 
> BR,
> -R
> 
> 
> > By the way, is thus calls for an addfb3, I would add reserved fields at the
> > end of the structure to make future extensions possible without a new ioctl.
> >
> >> 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?
> >>
> >> TODO move definition of tokens to drm_fourcc.h?
> >>
> >> Signed-off-by: Rob Clark <robdclark@gmail.com>
> >> ---
> >>  include/uapi/drm/drm_mode.h | 30 ++++++++++++++++++++++++++++++
> >>  1 file changed, 30 insertions(+)
> >>
> >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> >> index aae71cb..c43648c 100644
> >> --- a/include/uapi/drm/drm_mode.h
> >> +++ b/include/uapi/drm/drm_mode.h
> >> @@ -330,6 +330,28 @@ struct drm_mode_fb_cmd {
> >>
> >>  #define DRM_MODE_FB_INTERLACED       (1<<0) /* for interlaced framebuffers */
> >>
> >> +/*
> >> + * 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 24 bits are assigned as vendor sees fit.
> >> + */
> >> +
> >> +/* Vendor Ids: */
> >> +#define DRM_FOURCC_MOD_VENDOR_SAMSUNG 0x03
> >> +/* ... more */
> >> +
> >> +#define DRM_FOURCC_MOD(vendor, name) (((DRM_FOURCC_MOD_VENDOR_## vendor) <<
> >> 24) | val)
> >> +
> >> +/* Modifier values: */
> >> +/* 64x32 macroblock, ie NV12MT: */
> >> +#define DRM_FOURCC_MOD_SAMSUNG_64_32_TILE DRM_FOURCC_MOD(SAMSUNG, 123)
> >> +/* ... more */
> >> +
> >>  struct drm_mode_fb_cmd2 {
> >>       __u32 fb_id;
> >>       __u32 width, height;
> >> @@ -349,10 +371,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_FOURCC_MOD_xxx.
> >>        */
> >>       __u32 handles[4];
> >>       __u32 pitches[4]; /* pitch for each plane */
> >>       __u32 offsets[4]; /* offset of each plane */
> >> +     __u32 modifier[4]; /* ie, tiling, compressed (per plane) */
> >>  };
> >>
> >>  #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel