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

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 9228 bytes --]

On Mon, 20 Jun 2016 09:45:26 -0400
Rob Clark <robdclark@gmail.com> wrote:

> On Mon, Jun 20, 2016 at 8:37 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > On Fri, 17 Jun 2016 11:44:34 -0400
> > Rob Clark <robdclark@gmail.com> wrote:
> >  
> >> On Fri, Jun 17, 2016 at 9:31 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:  
> >> > On Fri, 17 Jun 2016 08:26:04 -0400
> >> > Rob Clark <robdclark@gmail.com> wrote:
> >> >  
> >> >> On Fri, Jun 17, 2016 at 3:59 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:  
> >> >> > On Thu, 16 Jun 2016 10:40:51 -0400
> >> >> > Rob Clark <robdclark@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@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.

Hi,

after a head-scratching session on #dri-devel and a little bit on
#wayland too, I'm ready to punt that off as a bad test.

It is quite possible the machine running nouveau had a webcam that
really produces upside-down images. weston-simple-dmabuf-v4l does not
understand that. Apparently mpv OTOH does. How, maybe there is an
answer in V4L2 API, but I haven't looked it up yet.

If we look at the Vivid tests, the results are consistent and expected.
All intel, nouveau and radeon work the same way.

> > 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...

Seems like we can conclude this by saying that a dmabuf imported via
EGL as a GL texture will have the origin at top-left, instead of what
the GL spec says.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]