On Fri, 17 Jun 2016 11:44:34 -0400 Rob Clark <robdclark@xxxxxxxxx> wrote: > On Fri, Jun 17, 2016 at 9:31 AM, Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: > > On Fri, 17 Jun 2016 08:26:04 -0400 > > Rob Clark <robdclark@xxxxxxxxx> wrote: > > > >> On Fri, Jun 17, 2016 at 3:59 AM, Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: > >> > On Thu, 16 Jun 2016 10:40:51 -0400 > >> > Rob Clark <robdclark@xxxxxxxxx> 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@xxxxxxx> 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. 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
Attachment:
pgp9VuzQXZAmt.pgp
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
