<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 14, 2018 at 8:43 AM, Daniel Stone <span dir="ltr"><<a href="mailto:daniel@fooishbar.org" target="_blank">daniel@fooishbar.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 14 February 2018 at 16:27, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> On Wed, Feb 14, 2018 at 3:39 AM, Daniel Stone <<a href="mailto:daniel@fooishbar.org">daniel@fooishbar.org</a>> wrote:<br>
</span><span class="">>> > On Tue, Feb 13, 2018 at 10:48 AM, Daniel Stone <<a href="mailto:daniel@fooishbar.org">daniel@fooishbar.org</a>> wrote:<br>
</span><span class="">>> >> For actual scanout, the kernel very specifically demands that if the<br>
>> >> BO is X-tiled, then set_tiling be called with TILING_X. This applies<br>
>> >> even if we explicitly allocate with MOD_X_TILED and pass that in via<br>
>> >> drmModeAddFB2WithModifiers: there is an explicit check for<br>
>> >> !!(bo_tiling == TILING_X) == !!(modifier == MOD_X_TILED).<br>
>><br>
>> You missed this bit. Suggested fixup: <a href="https://hastebin.com/xikopobiza" rel="noreferrer" target="_blank">https://hastebin.com/<wbr>xikopobiza</a><br>
><br>
> I was really hoping I'd read wrong.  I'm going with "that's a kernel bug".<br>
> Modifiers is supposed to separate tiling from the BO.  Tying them back<br>
> together in drmModeAddFB2WithModifiers is wrong.  This is especially true<br>
> since SET_TILING is an inherently racy operation and we really need to stop<br>
> using it entirely.<br>
><br>
> If what you wrote above really is immutably true, then this patch is dead.<br>
<br>
</span>It is immutably true, but I think this patch (with ugly fixup) still<br>
makes sense.<br></blockquote><div><br></div><div>That's unfortunate.  Yeah, I agree that we should only set_tiling in the minimum case.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Linear is unchanged as it's implicit. X tiling has to take the<br>
workaround, in case anyone wants to display it. </blockquote><div><br></div><div>I'm not sure I buy that.  From a userspace perspective, you shouldn't be using modifiers unless everything supports them.  Why?  Because no userspace that isn't an Intel driver should look at I915_FORMAT_MOD_X_TILED and say "I can hand that off without modifiers just fine".<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Y tiling can skip it<br>
though, and future Yf/etc tiling modes don't need to either extend the<br>
I915_TILING_* enum (eww), or be explicitly set to linear when they're<br>
not. I think there's value in having the code clarify, rather than it<br>
doing set_tiling everywhere, so people assuming it's still required.<br></blockquote><div><br></div><div>Agreed.  We should set the minimum amount of tiling.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I can kind of see why that ABI makes sense though. Having userspace<br>
explicitly call set_tiling(X) had the kernel magically imply that the<br>
FB created from that BO should be X-tiled. Given that ABI still<br>
unfortunately exists, enforcing coherency between old and new ways to<br>
declare an FB should be tiled, doesn't seem unreasonable.<span class=""><br></span></blockquote><div><br></div><div>Eh.  The whole point of modifiers is as an alternate mechanism.  Insisting that they "match" is, to me, equivalent to insisting that you use both which is silly.  Of course, anyone is free to argue in the opposite direction.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> What's the failure mode here?  We just don't flip?  Or the compositor wigs<br>
> out and crashes?<br>
<br>
</span>AddFB never succeeds, so we can never flip.<br>
</blockquote></div><br></div><div class="gmail_extra">Ok.  That helps.<br></div></div>