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

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Rob Clark <robdclark@gmail.com>
Cc: "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>
Subject: Re: [RFC] drm: add support for tiled/compressed/etc modifier in addfb2
Date: Sat, 20 Dec 2014 21:04:35 +0200	[thread overview]
Message-ID: <40548048.tOu43FSkQq@avalon> (raw)
In-Reply-To: <CAF6AEGuv6QPfdirykMi+v2yPOGQAR-xGbBSFTezqiRVx0AuNtQ@mail.gmail.com>

Hi Rob,

On Thursday 18 December 2014 19:55:24 Rob Clark wrote:
> On Thu, Dec 18, 2014 at 4:22 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >>>>>> TODO move definition of tokens to drm_fourcc.h?
> >>>>> 
> >>>>> Seems orthogonal imo. Another todo is to add checking to all
> >>>>> drivers to reject it if it's not 0 with -EINVAL. Otherwise we have yet
> >>>>> another case of an ioctl with fields that can't actually be used
> >>>>> everywhere.
> >>>> 
> >>>> Could we please add the check in core code instead of drivers ?
> >>> 
> >>> Nope since then no driver could ever use that extension. Defeats the
> >>> point ;-)
> >> 
> >> Except if we follow the proposal of adding a flag to tell whether a
> >> driver supports the extension ;-)
> > 
> > I'm not a terrible big fan of driver flags, mostly because I've seen too
> > much of the horrible stuff in dri1. Imo much better to pass everything to
> > drivers and help them out with helpers if needed. I might be going
> > overboard a bit with my bias against driver flags ;-)
> 
> I ventured a little ways down the thought path of adding list of
> supported modifier tokens per plane.. and then doing more complete
> checks in the core.  But then the question is, what about cases where
> some tiling format is only supported for UV but not Y, etc.. it
> quickly gets ugly.

From my experience with V4L2 expressing such constraints in a way that would 
be both simple and comprehensive isn't possible. We should aim for the common 
case, and I agree that finding out what the common case is would require 
implementing the feature first.

However, it would be pretty easy to flag whether a driver supports this new 
API at all. That could be used to zero the extra fields.

> I think for now better to let the driver do this (with a must_be_all_zeros()
> helper for what I expect will be the common case initially).  If common
> patterns emerge, then we refactor out a better helper..

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel