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

[Rob Clark]

On Fri, Dec 12, 2014 at 10:11 AM, Daniel Stone <daniel at fooishbar.org> wrote:
> Hi,
>
> On 12 December 2014 at 14:56, Ville Syrjälä <ville.syrjala at linux.intel.com>
> wrote:
>>
>> On Fri, Dec 12, 2014 at 08:50:18AM -0500, Rob Clark wrote:
>>
>> > It does make me briefly think of just letting us set properties on fb
>>
>> > objects :-P (but maybe that is a bit overkill)
>>
>> Yeah I had the same idea at some point. But then I decided that we could
>> just have these as properties on the plane.
>
>
> Mm, it does seem a bit weird. Yes, they are relative to how the plane
> interprets things, but then again so is the format surely. Not to mention
> another thing to go wrong, if someone forgets to set and/or clear it when
> changing the plane. Keeping it in the fb eliminates that possibility.
>

yeah.. logically it seems nicer for them to be prop's on fb's.  The
drawback is having to invent some bit of infrastructure to support
that.  Avoidance of inheriting someone else's plane prop's might be
enough justification to invent that infrastructure.  But fb prop's
don't really help w/ the whole not-all-planes-are-the-same thing..

>>
>> > I suppose the semi-custom plane property approach is a bit easier to
>> > extend-as-we-go, and we already have a mechanism so userspace can
>> > query about what is actually supported.  But this does feel a bit more
>> > like attributes of the fb.  I'm interested if anyone has particularly
>> > good arguments one way or another.
>>
>> I guess we could have just specified offset/size/stride as part of the
>> fb and let pixel format and such as properties. That would be a fairly
>> natural line IMO since it would be enough data to do a blit, but not
>> enough to actually interpret the pixel data. But we already went beyond
>> that with pixel formats. So I'm not sure how far we want to go.
>>
>> Also all this chroma siting and colorspace stuff definitely runs into
>> hardware specific limitations so having some way to tell userspace what
>> is possible would be nice as you said. Properties seem a decent match for
>> that.
>
>
> Yeah, that's a good idea actually, especially since different planes do have
> different capabilities.
>
>>
>> > > It might be useful to make the interpretation modifiers bitmaskable,
>> > > so they
>> > > can be combined (e.g. wide-range/unclamped YUV | whatever chroma
>> > > siting),
>> > > but I can't think of a usecase for combining multiple layout modifiers
>> > > (e.g.
>> > > this tiling | that compression).
>> >
>> > Yeah, I think the vendor-range part of the token, the vendor would
>> > probably want to define as a bitmask or set of bitfields so that they
>> > could have things like tiled+compressed
>> >
>> > (otoh, if you try to organize it too nicely now, eventually enough hw
>> > generations in the future that scheme will break down.. so maybe a big
>> > switch of #define cases is better than trying to interpret the
>> > modifier token)
>
>
> Having them separated is still kinda nice though, for the same rationale as
> the EGLImage import extension having them as hints. If your hardware doesn't
> support the tiling/compression format you use, then you're going to be
> showing absolute garbage. But if it doesn't support your exact chroma-siting
> or YUV range request, it'll still be totally viewable, just not entirely
> perfect. So I don't see the logic in failing these.

oh, sorry, I was just referring to the 'modifier token' stuff..
chroma-siting and YUV range are common enough that I think they should
be something separate from the per-plane 'modifer token'

>>
>> > >> 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?
>> > >
>> > >
>> > > Yeah, I'd second all of danvet's comments here, as well as adding a
>> > > new
>> > > ADDFB2_MODIFIERS cap + query for supported modifiers. Makes life much
>> > > easier
>> > > for generic userspace.
>> >
>> > I've locally made a few tweaks (64b and move some stuff to
>> > drm_fourcc.h)..
>> >
>> > I was kicking around the idea of letting plane specify an array of
>> > supported format modifiers, and adding this to getplane ioctl, as an
>> > alternative to a cap.  That plus wiring up some checking to disallow
>> > addfb2 for a format + modifiers not supported by at least one plane.
>> > Although some hw could only support certain tiling patterns for
>> > certain layers of an fb (ie. luma vs chroma).  So I may scrap that
>> > idea and just go back to cap.
>>
>> Indeed the format specific limitations are problem. Properties can't
>> handle that. We'd need to have some kind of caps for each plane+format
>> combination if we want to deal with that. But I don't think we can
>> still make it handle all the hw limitations, so I'm not sure it's worth
>> going down this path.
>

Yeah, I guess in the end the driver will still have to do some of it's
own checks, the core can't do everything..

>
> Well, you don't have to solve literally everything at once. Just having a
> list of formats which could possibly be supported if you did the right
> thing, would be a hell of a lot better than punting to userspace, which
> either a) has to have hardware-specific knowledge in every component
> (compositor, media library, etc etc), or b) brute-force it. The lack of any
> format query in EGLImage dmabuf import is a serious, serious, serious, pain
> when trying to do generic userspace (e.g. compositor feeds GStreamer a list
> of formats which are supported by the hardware). I get that there are
> combinations that could fail, but that's true of everything. At least
> narrowing down the problem space a bit is an enormous help.
>

For sure, once we get the kms bit sorted we're going to want to take
another pass at the eglimg extension and try to spiff it up a bit
better.

(and then I guess some day teach gallium / mesa-st about multi-planer
external eglImages..)

> Cheers,
> Daniel
>