[Mesa-dev] [RFC] New dma_buf -> EGLImage EGL extension - Final spec published!

[Rob Clark]

On Mon, Jun 20, 2016 at 8:37 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Fri, 17 Jun 2016 11:44:34 -0400
> Rob Clark <robdclark at gmail.com> wrote:
>
>> On Fri, Jun 17, 2016 at 9:31 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> > On Fri, 17 Jun 2016 08:26:04 -0400
>> > Rob Clark <robdclark at gmail.com> wrote:
>> >
>> >> On Fri, Jun 17, 2016 at 3:59 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> >> > On Thu, 16 Jun 2016 10:40:51 -0400
>> >> > Rob Clark <robdclark at gmail.com> wrote:
>> >> >
>> >> >> So, if we wanted to extend this to support the fourcc-modifiers that
>> >> >> we have on the kernel side for compressed/tiled/etc formats, what
>> >> >> would be the right approach?
>> >> >>
>> >> >> A new version of the existing extension or a new
>> >> >> EGL_EXT_image_dma_buf_import2 extension, or ??
>> >> >
>> >> > Hi Rob,
>> >> >
>> >> > there are actually several things it might be nice to add:
>> >> >
>> >> > - a fourth plane, to match what DRM AddFB2 supports
>> >> >
>> >> > - the 64-bit fb modifiers
>> >> >
>> >> > - queries for which pixel formats are supported by EGL, so a display
>> >> >   server can tell the applications that before the application goes and
>> >> >   tries with a random bunch of them, shooting in the dark
>> >> >
>> >> > - queries for which modifiers are supported for each pixel format, ditto
>> >> >
>> >> > I discussed these with Emil in the past, and it seems an appropriate
>> >> > approach might be the following.
>> >> >
>> >> > Adding the 4th plane can be done as revising the existing
>> >> > EGL_EXT_image_dma_buf_import extension. The plane count is tied to
>> >> > pixel formats (and modifiers?), so the user does not need to know
>> >> > specifically whether the EGL implementation could handle a 4th plane or
>> >> > not. It is implied by the pixel format.
>> >> >
>> >> > Adding the fb modifiers needs to be a new extension, so that users can
>> >> > tell if they are supported or not. This is to avoid the following false
>> >> > failure: if user assumes modifiers are always supported, it will (may?)
>> >> > provide zero modifiers explicitly. If EGL implementation does not
>> >> > handle modifiers this would be rejected as unrecognized attributes,
>> >> > while if the zero modifiers were not given explicitly, everything would
>> >> > just work.
>> >>
>> >> hmm, if we design it as "not passing modifier" == "zero modifier", and
>> >> "never explicitly pass a zero modifier" then modifiers could be added
>> >> without a new extension.  Although I agree that queries would need a
>> >> new extension.. so perhaps not worth being clever.
>> >
>> > Indeed.
>> >
>> >> > The queries obviously(?) need a new extension. It might make sense
>> >> > to bundle both modifier support and the queries in the same new
>> >> > extension.
>> >> >
>> >> > We have some rough old WIP code at
>> >> > https://git.collabora.com/cgit/user/lfrb/mesa.git/log/?h=T1410-modifiers
>> >> > https://git.collabora.com/cgit/user/lfrb/egl-specs.git/log/?h=T1410
>> >> >
>> >> >
>> >> >> On Mon, Feb 25, 2013 at 6:54 AM, Tom Cooksey <tom.cooksey at arm.com> wrote:
>> >> >> > Hi All,
>> >> >> >
>> >> >> > The final spec has had enum values assigned and been published on Khronos:
>> >> >> >
>> >> >> > http://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
>> >> >> >
>> >> >> > Thanks to all who've provided input.
>> >> >
>> >> > May I also pull your attention to a detail with the existing spec and
>> >> > Mesa behaviour I am asking about in
>> >> > https://lists.freedesktop.org/archives/mesa-dev/2016-June/120249.html
>> >> > "What is EGL_EXT_image_dma_buf_import image orientation as a GL texture?"
>> >> > Doing a dmabuf import seems to imply an y-flip AFAICT.
>> >>
>> >> I would have expected that *any* egl external image (dma-buf or
>> >> otherwise) should have native orientation rather than gl orientation.
>> >> It's somewhat useless otherwise.
>> >
>> > In that case importing dmabuf works differently than importing a
>> > wl_buffer (wl_drm), because for the latter, the y-invert flag is
>> > returned such that the orientation will match GL. And the direct
>> > scanout path goes through GBM since you have to import a wl_buffer, and
>> > I haven't looked what GBM does wrt. y-flip if anything.
>> >
>> >> I didn't read it carefully yet (would need caffeine first ;-)) but
>> >> EGL_KHR_image_base does say "This extension defines a new EGL resource
>> >> type that is suitable for sharing 2D arrays of image data between
>> >> client APIs" which to me implies native orientation.  So that just
>> >> sounds like a mesa bug somehow?
>> >
>> > That specific sentence implies nothing about orientation to me.
>> > Furthermore, the paragraph continues:
>> >
>> >         "Although the intended purpose is sharing 2D image data, the
>> >         underlying interface makes no assumptions about the format or
>> >         purpose of the resource being shared, leaving those decisions
>> >         to the application and associated client APIs."
>> >
>> > Might "format" include orientation?
>> >
>> > How does "native orientation" connect with "GL texture coordinates"?
>> > The latter have explicitly defined orientation and origin. For use in
>> > GL, the right way up image is having the origin in the bottom-left
>> > corner. An image right way up is an image right way up, regardless
>> > which corner is the origin. The problem comes when you start using
>> > coordinates.
>> >
>> >> Do you just get that w/ i965?  I know some linaro folks have been
>> >> using this extension to import buffers from video decoder with
>> >> freedreno/gallium and no one mentioned the video being upside down.
>> >
>> > Intel, yes, but since this happens *only* for the GL import path and
>> > direct scanout is fine without y-flipping, I bet people just flipped y
>> > and did not think twice, if there even was a problem. I just have a
>> > habit of asking "why". ;-)
>>
>> well, if possible, try with one of the gallium drivers?
>>
>> I'm honestly not 100% sure what it is supposed to be according to the
>> spec, but I do know some of the linaro folks were doing v4l2dec ->
>> glimagesink with dmabuf with both mali (I think some ST platform?) and
>> freedreno (snapdragon db410c), and no one complained to me that the
>> video was upside down for one or the other.  So I guess at least
>> gallium and mali are doing the same thing.  No idea if that is the
>> same thing that i965 does.
>
> Hi,
>
> Quentin did some tests for me, and the results are... not what I would
> have expected:
> https://phabricator.freedesktop.org/T7475#88454
>
> RadeonSI works like Intel, but Nouveau does the opposite. I think the
> Nouveau way is correct by the spec, which makes everyone else (intel,
> radeonsi, weston-simple-dmabuf-v4l) get it wrong. Unfortunately, two
> wrongs make a right, so when weston-simple-dmabuf-v4l hardcodes Y_INVERT
> as set, it'll come out the right way on intel and radeon.

oh, wow.. that seems quite odd that two different gallium drivers have
different results, since I'd have expected this to be completely
handled by mesa/st.. now I'm somewhat curious.

BR,
-R

> And it is wrong for weston-simple-dmabuf-v4l to set Y_INVERT, that I
> believe is clear. It is only there because of the "oops, it's
> upside-down" syndrome, AFAIK.
>
>> > After all, using GL with windows and FBOs and stuff you very often find
>> > yourself upside down, and I suspect people have got the habit of just
>> > flipping it if it does not look right the first time. See e.g. the
>> > row-order of data going into glTexImage2D...
>> >
>> > If the answer is "oops, well, dmabuf import is semantically y-flipping
>> > when it should not, but we cannot fix it because that would break
>> > everyone", I would be happy with that. I just want confirmation before
>> > flipping the flip flag. :-)
>
> So many bugs that accidentally counter each other...
>
>
> Thanks,
> pq