<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Sep 18, 2017 at 9:20 AM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu 14 Sep 2017, Eric Anholt wrote:<br>
> Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
><br>
> > The setTexBuffer2 hook from GLX is used to implement glxBindTexImageEXT<br>
> > which has tighter restrictions than just "it's shared".  In particular,<br>
> > it says that any rendering to the image while it is bound causes the<br>
> > contents to become undefined.  This means that we can do whatever aux<br>
> > tracking we want between glxBindTexImageEXT and glxReleaseTexImageEXT so<br>
> > long as we always transition from external in Bind and to external in<br>
> > Release.<br>
><br>
> The intent of the spec was to get at the hard-to-define "you get pixels<br>
> at least as new as the outstanding X11 rendering when you called<br>
> glxBindTexImageEXT(), but if X11 keeps on rendering to the thing then<br>
> you may get newer pixels, too."  With your CCS plan and X11 rendering in<br>
> parallel with you GL texturing from the X11 pixmap, will we always see<br>
> either old or new pixels but not anything else?<br></div></div></blockquote><div><br></div><div>That gets interesting... Yes, reading and writing to a CCS image at the same time is problematic.  However, the spec does say "undefined" and not "either new or old pixels".  :-)  I'm not sure how this translates into the real world though.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
</div></div>We have bigger problems if X11 today renders to any surface that has<br>
been externally shared with I915_FORMAT_MOD_Y_TILED_CCS. If I ran<br>
git-grep correctly, then xf86-video-intel nor xf86-video-modesetting is<br>
unaware of modifiers and hence unaware of CCS aux state.<br></blockquote><div><br></div><div>you're not grepping the right branches. :-)  We do have code floating around that makes modesetting modifiers-aware.  It's not aware of what the modifiers mean other than that multi-planar modifiers have racing trouble and it avoids using them for front-buffer rendering.  That said, I've run a variety of examples, and I have yet to see any corruption issues.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
So, with respect to reviewing this patch, I think it's safe to assume<br>
that X11 never renders to a CCS-enabled surface, and that this patch is<br>
the correct fix. If X11 does render to CCS-enabled surfaces, then we<br>
have more invasive problems that need fixing.<br></blockquote><div><br></div><div>Yes.  In any case, I think the old code was decidedly wrong in the face of modifiers.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewed-by: Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>><br>
</blockquote></div><br></div></div>