<div dir="ltr">On Tue, Jun 11, 2013 at 1:10 PM, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">Frank Henigman <<a href="mailto:fjhenigman@google.com">fjhenigman@google.com</a>> writes:<br>
<br>
> Replace the one texture lock with a lock per texture.  This allows<br>
> uploading textures from one thread concurrently with drawing in another<br>
> thread.  _mesa_lock_context_textures() was used to check for texture<br>
> updates from other contexts and also to acquire the texture lock.<br>
> It's been replaced with _mesa_check_context_textures() which only does<br>
> the checking.  Code sections that were between<br>
> _mesa_lock_context_textures() and _mesa_unlock_context_textures()<br>
> have been updated to lock individual textures as needed.<br>
<br>
</div>When someone's doing something like glCopyTexSubImage() from an FBO<br>
backed by a texture to another texture, how is the locking supposed to<br>
work?  How about copies from one texture to the same texture?<br></blockquote><div><br></div><div style>Right now glCopyTexSubImage locks the destination texture before copying</div><div style>to it, but doesn't lock the source texture.  This was safe because locking any</div>
<div style>texture effectively locked them all.  With my change that's no longer true so</div><div style>now we're copying from an unlocked texture.  Is that your concern?</div><div style>So we just need to have it lock the source texture too?</div>
<div style>We'll have to check for source == destination so we don't try to lock twice.</div><div style><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

I'm pretty sure that our current locking doesn't cover nearly as much as<br>
it needs to if one wants to make thread-per-context shareCtx support<br>
actually work, so I'm really concerned that this change may make the<br>
locking unfixable.<br></blockquote><div><br></div><div style>I agree there probably are problems with locking currently, and there seems</div><div style>to be zero coverage for context sharing in piglit.  But I don't understand how</div>
<div style>my change makes anything unfixable.  Can you elaborate?</div><div style><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">
> No piglit regressions on pineview with intel driver.<br>
> Passes new piglit test glx-multithread-texture.<br>
<br>
</div>This test doesn't do any concurrent access of textures as far as I can<br>
see, so I don't think it really exercises this patch.<br></blockquote><div><br></div><div style>The idea behind the test was to mimic an actual use case and show</div><div style>that my patch increases concurrency.  I had thought to actually measure</div>
<div style>concurrency in the test (maybe by timing how long the threads wait)</div><div style>but realized this would fail on single core machines so I left that out,</div><div style>though I can still tell the patch helps because the test completes significantly</div>
<div style>faster with it.</div><div style><br></div><div style>There's plenty of room for more shared context tests, especially as there</div><div style>weren't any.  I'd like to start by considering likely uses.  I'm thinking any</div>
<div style>use of shared contexts will probably include some synchronization between</div><div style>the users.  If you think there's value in a test of concurrent access to a</div><div style>texture I'll write one (including your glCopyTexSubImage scenarios).</div>
<div style>I'm interested in any suggestions as to the most useful tests to have at</div><div style>this point.   Thanks.</div></div></div></div>