Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-13 Thread Brian Paul 

On 06/13/2016 10:04 AM, Marek Olšák wrote:

On Mon, Jun 13, 2016 at 5:17 PM, Roland Scheidegger  wrote:

Am 13.06.2016 um 14:53 schrieb Marek Olšák:

On Mon, Jun 13, 2016 at 1:14 PM, Jose Fonseca  wrote:

On 10/06/16 15:40, Roland Scheidegger wrote:


Am 10.06.2016 um 12:38 schrieb Marek Olšák:


On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger 
wrote:


Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:


On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger
 wrote:


Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:


Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:


On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin 
wrote:


On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund 
wrote:


On Wednesday 08 June 2016, Ilia Mirkin wrote:


Glancing at the code (I don't even have a piglit checkout here):

static void
set_ubo_binding(struct gl_context *ctx, ...)
...
 /* If this is a real buffer object, mark it has having been
used
  * at some point as a UBO.
  */
 if (size >= 0)
bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;

That seems bogus - what if the current size is 0 (unallocated),
the
buffer object gets bound to a UBO endpoint, and then someone goes
in
and does glBufferData()? Same for set_ssbo_binding.

-ilia



The test is greater than or equal to zero, so the UsageHistory
should
be set even when the buffer is unallocated.



Right, duh.



But the piglit test doesn't bind the buffer as a uniform buffer
before
it allocates it.  It allocates the buffer first with
glNamedBufferData(),
and then binds it.  The UsageHistory is still set to the default
value in
the glNamedBufferData() call, since the buffer has never been
bound
at that point.  But the uniform buffer state should still be
marked as
dirty in the glBindBufferRange() call.  I think this failure
suggests
that that doesn't happen for some reason.



I haven't looked in GREAT detail, but the test does pass on nv50,
nvc0, and softpipe. It only fails on llvmpipe.

Brian, this might be out of my comfort area to figure out... Given
that it's working on the other drivers, that seems more likely to
be a
failing of llvmpipe somehow.



Another observation is that the square sizes/shapes are all correct.
However the colors are all of the first square (red). So it seems
like
some issue is happening for the fragment shader, but not vertex.



I've looked at this briefly and I'm not sure who's to blame.
It goes something like this:
- we don't get any set_constant_buffer calls anymore when the
contents
of the buffer change. I don't think that's ok, looks like a bug to
me?
Granted it's still the same buffer, just modfying a bound one.

- when the contents are updated, it ends up in some transfer stuff as
expected, in the end llvmpipe_transfer_map(). This checks if the
resource is referenced in the current scene, if so it would flush -
however this is only done for textures and rts (because UBO contents
are
indeed copied to the scene, not referenced, this is quite ok here, we
don't want to flush).

- on the next draw, we'd check the dirty bits - we never got
set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do
anything,
so we don't pick up the changed contents, and continue to use the old
copied ones.

We could check if buffers are referenced in the scene and set the
LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is
doing
is really ok? (For vs, it doesn't matter that we miss the update - as
the offsets etc. are all the same the vs will just pick up the
changes
automatically as we don't copy anything since this stuff all runs
synchronously.)




Err actually this analysis is flawed.
llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
bound constant buffer is changed and hence set LP_NEW_CONSTANTS
accordingly.
But it doesn't because the buffer doesn't have the
PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
would be illegal to bind it as such, but this is never enforced). In
fact it doesn't have _any_ bind flag set.
So I blame the GL api, but I don't know how to fix this mess cleanly
(we
don't really want to ignore bind flags completely).



Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse
you. If you use it for anything after resource creation, that's a bug.



This was designed in gallium with dx10 in mind, which has a proper api
for this. But I suppose the only way to fix it in the mesa state tracker
would be to just set ALL possible bind flags for buffers always...



Or you can assume bind == 0 means all flags are set.



Yes in this case, but I don't think that really helps. Even when using
the old gl api for setting this, the target (and hence the bind flag)
may be something completely different than the actual usage.



I'd rather have GL renderer set all possible bind flags.

And

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-13 Thread Marek Olšák 
On Mon, Jun 13, 2016 at 5:17 PM, Roland Scheidegger  wrote:
> Am 13.06.2016 um 14:53 schrieb Marek Olšák:
>> On Mon, Jun 13, 2016 at 1:14 PM, Jose Fonseca  wrote:
>>> On 10/06/16 15:40, Roland Scheidegger wrote:

 Am 10.06.2016 um 12:38 schrieb Marek Olšák:
>
> On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger 
> wrote:
>>
>> Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
>>>
>>> On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger
>>>  wrote:

 Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
>
> Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
>>
>> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin 
>> wrote:
>>>
>>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund 
>>> wrote:

 On Wednesday 08 June 2016, Ilia Mirkin wrote:
>
> Glancing at the code (I don't even have a piglit checkout here):
>
> static void
> set_ubo_binding(struct gl_context *ctx, ...)
> ...
> /* If this is a real buffer object, mark it has having been
> used
>  * at some point as a UBO.
>  */
> if (size >= 0)
>bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>
> That seems bogus - what if the current size is 0 (unallocated),
> the
> buffer object gets bound to a UBO endpoint, and then someone goes
> in
> and does glBufferData()? Same for set_ssbo_binding.
>
>-ilia


 The test is greater than or equal to zero, so the UsageHistory
 should
 be set even when the buffer is unallocated.
>>>
>>>
>>> Right, duh.
>>>

 But the piglit test doesn't bind the buffer as a uniform buffer
 before
 it allocates it.  It allocates the buffer first with
 glNamedBufferData(),
 and then binds it.  The UsageHistory is still set to the default
 value in
 the glNamedBufferData() call, since the buffer has never been
 bound
 at that point.  But the uniform buffer state should still be
 marked as
 dirty in the glBindBufferRange() call.  I think this failure
 suggests
 that that doesn't happen for some reason.
>>>
>>>
>>> I haven't looked in GREAT detail, but the test does pass on nv50,
>>> nvc0, and softpipe. It only fails on llvmpipe.
>>>
>>> Brian, this might be out of my comfort area to figure out... Given
>>> that it's working on the other drivers, that seems more likely to
>>> be a
>>> failing of llvmpipe somehow.
>>
>>
>> Another observation is that the square sizes/shapes are all correct.
>> However the colors are all of the first square (red). So it seems
>> like
>> some issue is happening for the fragment shader, but not vertex.
>>
>
> I've looked at this briefly and I'm not sure who's to blame.
> It goes something like this:
> - we don't get any set_constant_buffer calls anymore when the
> contents
> of the buffer change. I don't think that's ok, looks like a bug to
> me?
> Granted it's still the same buffer, just modfying a bound one.
>
> - when the contents are updated, it ends up in some transfer stuff as
> expected, in the end llvmpipe_transfer_map(). This checks if the
> resource is referenced in the current scene, if so it would flush -
> however this is only done for textures and rts (because UBO contents
> are
> indeed copied to the scene, not referenced, this is quite ok here, we
> don't want to flush).
>
> - on the next draw, we'd check the dirty bits - we never got
> set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
> LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do
> anything,
> so we don't pick up the changed contents, and continue to use the old
> copied ones.
>
> We could check if buffers are referenced in the scene and set the
> LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is
> doing
> is really ok? (For vs, it doesn't matter that we miss the update - as
> the offsets etc. are all the same the vs will just pick up the
> changes
> automatically as we don't copy anything since this stuff all runs
> synchronously.)
>


 Err

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-13 Thread Roland Scheidegger 
Am 13.06.2016 um 14:53 schrieb Marek Olšák:
> On Mon, Jun 13, 2016 at 1:14 PM, Jose Fonseca  wrote:
>> On 10/06/16 15:40, Roland Scheidegger wrote:
>>>
>>> Am 10.06.2016 um 12:38 schrieb Marek Olšák:

 On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger 
 wrote:
>
> Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
>>
>> On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger
>>  wrote:
>>>
>>> Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:

 Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
>
> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin 
> wrote:
>>
>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund 
>> wrote:
>>>
>>> On Wednesday 08 June 2016, Ilia Mirkin wrote:

 Glancing at the code (I don't even have a piglit checkout here):

 static void
 set_ubo_binding(struct gl_context *ctx, ...)
 ...
 /* If this is a real buffer object, mark it has having been
 used
  * at some point as a UBO.
  */
 if (size >= 0)
bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;

 That seems bogus - what if the current size is 0 (unallocated),
 the
 buffer object gets bound to a UBO endpoint, and then someone goes
 in
 and does glBufferData()? Same for set_ssbo_binding.

-ilia
>>>
>>>
>>> The test is greater than or equal to zero, so the UsageHistory
>>> should
>>> be set even when the buffer is unallocated.
>>
>>
>> Right, duh.
>>
>>>
>>> But the piglit test doesn't bind the buffer as a uniform buffer
>>> before
>>> it allocates it.  It allocates the buffer first with
>>> glNamedBufferData(),
>>> and then binds it.  The UsageHistory is still set to the default
>>> value in
>>> the glNamedBufferData() call, since the buffer has never been
>>> bound
>>> at that point.  But the uniform buffer state should still be
>>> marked as
>>> dirty in the glBindBufferRange() call.  I think this failure
>>> suggests
>>> that that doesn't happen for some reason.
>>
>>
>> I haven't looked in GREAT detail, but the test does pass on nv50,
>> nvc0, and softpipe. It only fails on llvmpipe.
>>
>> Brian, this might be out of my comfort area to figure out... Given
>> that it's working on the other drivers, that seems more likely to
>> be a
>> failing of llvmpipe somehow.
>
>
> Another observation is that the square sizes/shapes are all correct.
> However the colors are all of the first square (red). So it seems
> like
> some issue is happening for the fragment shader, but not vertex.
>

 I've looked at this briefly and I'm not sure who's to blame.
 It goes something like this:
 - we don't get any set_constant_buffer calls anymore when the
 contents
 of the buffer change. I don't think that's ok, looks like a bug to
 me?
 Granted it's still the same buffer, just modfying a bound one.

 - when the contents are updated, it ends up in some transfer stuff as
 expected, in the end llvmpipe_transfer_map(). This checks if the
 resource is referenced in the current scene, if so it would flush -
 however this is only done for textures and rts (because UBO contents
 are
 indeed copied to the scene, not referenced, this is quite ok here, we
 don't want to flush).

 - on the next draw, we'd check the dirty bits - we never got
 set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
 LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do
 anything,
 so we don't pick up the changed contents, and continue to use the old
 copied ones.

 We could check if buffers are referenced in the scene and set the
 LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is
 doing
 is really ok? (For vs, it doesn't matter that we miss the update - as
 the offsets etc. are all the same the vs will just pick up the
 changes
 automatically as we don't copy anything since this stuff all runs
 synchronously.)

>>>
>>>
>>> Err actually this analysis is flawed.
>>> llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
>>> bound constant buffer is changed and hence set LP_NEW_CONSTANTS
>>>

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-13 Thread Marek Olšák 
On Mon, Jun 13, 2016 at 1:14 PM, Jose Fonseca  wrote:
> On 10/06/16 15:40, Roland Scheidegger wrote:
>>
>> Am 10.06.2016 um 12:38 schrieb Marek Olšák:
>>>
>>> On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger 
>>> wrote:

 Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
>
> On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger
>  wrote:
>>
>> Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
>>>
>>> Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:

 On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin 
 wrote:
>
> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund 
> wrote:
>>
>> On Wednesday 08 June 2016, Ilia Mirkin wrote:
>>>
>>> Glancing at the code (I don't even have a piglit checkout here):
>>>
>>> static void
>>> set_ubo_binding(struct gl_context *ctx, ...)
>>> ...
>>> /* If this is a real buffer object, mark it has having been
>>> used
>>>  * at some point as a UBO.
>>>  */
>>> if (size >= 0)
>>>bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>>>
>>> That seems bogus - what if the current size is 0 (unallocated),
>>> the
>>> buffer object gets bound to a UBO endpoint, and then someone goes
>>> in
>>> and does glBufferData()? Same for set_ssbo_binding.
>>>
>>>-ilia
>>
>>
>> The test is greater than or equal to zero, so the UsageHistory
>> should
>> be set even when the buffer is unallocated.
>
>
> Right, duh.
>
>>
>> But the piglit test doesn't bind the buffer as a uniform buffer
>> before
>> it allocates it.  It allocates the buffer first with
>> glNamedBufferData(),
>> and then binds it.  The UsageHistory is still set to the default
>> value in
>> the glNamedBufferData() call, since the buffer has never been
>> bound
>> at that point.  But the uniform buffer state should still be
>> marked as
>> dirty in the glBindBufferRange() call.  I think this failure
>> suggests
>> that that doesn't happen for some reason.
>
>
> I haven't looked in GREAT detail, but the test does pass on nv50,
> nvc0, and softpipe. It only fails on llvmpipe.
>
> Brian, this might be out of my comfort area to figure out... Given
> that it's working on the other drivers, that seems more likely to
> be a
> failing of llvmpipe somehow.


 Another observation is that the square sizes/shapes are all correct.
 However the colors are all of the first square (red). So it seems
 like
 some issue is happening for the fragment shader, but not vertex.

>>>
>>> I've looked at this briefly and I'm not sure who's to blame.
>>> It goes something like this:
>>> - we don't get any set_constant_buffer calls anymore when the
>>> contents
>>> of the buffer change. I don't think that's ok, looks like a bug to
>>> me?
>>> Granted it's still the same buffer, just modfying a bound one.
>>>
>>> - when the contents are updated, it ends up in some transfer stuff as
>>> expected, in the end llvmpipe_transfer_map(). This checks if the
>>> resource is referenced in the current scene, if so it would flush -
>>> however this is only done for textures and rts (because UBO contents
>>> are
>>> indeed copied to the scene, not referenced, this is quite ok here, we
>>> don't want to flush).
>>>
>>> - on the next draw, we'd check the dirty bits - we never got
>>> set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
>>> LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do
>>> anything,
>>> so we don't pick up the changed contents, and continue to use the old
>>> copied ones.
>>>
>>> We could check if buffers are referenced in the scene and set the
>>> LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is
>>> doing
>>> is really ok? (For vs, it doesn't matter that we miss the update - as
>>> the offsets etc. are all the same the vs will just pick up the
>>> changes
>>> automatically as we don't copy anything since this stuff all runs
>>> synchronously.)
>>>
>>
>>
>> Err actually this analysis is flawed.
>> llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
>> bound constant buffer is changed and hence set LP_NEW_CONSTANTS
>> accordingly.
>> But it doesn't because the buffer doesn't have the
>> PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
>> would be illegal to

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-13 Thread Jose Fonseca 

On 10/06/16 15:40, Roland Scheidegger wrote:

Am 10.06.2016 um 12:38 schrieb Marek Olšák:

On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger  wrote:

Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:

On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger  wrote:

Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:

Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:

On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:

On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:

On Wednesday 08 June 2016, Ilia Mirkin wrote:

Glancing at the code (I don't even have a piglit checkout here):

static void
set_ubo_binding(struct gl_context *ctx, ...)
...
/* If this is a real buffer object, mark it has having been used
 * at some point as a UBO.
 */
if (size >= 0)
   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;

That seems bogus - what if the current size is 0 (unallocated), the
buffer object gets bound to a UBO endpoint, and then someone goes in
and does glBufferData()? Same for set_ssbo_binding.

   -ilia


The test is greater than or equal to zero, so the UsageHistory should
be set even when the buffer is unallocated.


Right, duh.



But the piglit test doesn't bind the buffer as a uniform buffer before
it allocates it.  It allocates the buffer first with glNamedBufferData(),
and then binds it.  The UsageHistory is still set to the default value in
the glNamedBufferData() call, since the buffer has never been bound
at that point.  But the uniform buffer state should still be marked as
dirty in the glBindBufferRange() call.  I think this failure suggests
that that doesn't happen for some reason.


I haven't looked in GREAT detail, but the test does pass on nv50,
nvc0, and softpipe. It only fails on llvmpipe.

Brian, this might be out of my comfort area to figure out... Given
that it's working on the other drivers, that seems more likely to be a
failing of llvmpipe somehow.


Another observation is that the square sizes/shapes are all correct.
However the colors are all of the first square (red). So it seems like
some issue is happening for the fragment shader, but not vertex.



I've looked at this briefly and I'm not sure who's to blame.
It goes something like this:
- we don't get any set_constant_buffer calls anymore when the contents
of the buffer change. I don't think that's ok, looks like a bug to me?
Granted it's still the same buffer, just modfying a bound one.

- when the contents are updated, it ends up in some transfer stuff as
expected, in the end llvmpipe_transfer_map(). This checks if the
resource is referenced in the current scene, if so it would flush -
however this is only done for textures and rts (because UBO contents are
indeed copied to the scene, not referenced, this is quite ok here, we
don't want to flush).

- on the next draw, we'd check the dirty bits - we never got
set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
so we don't pick up the changed contents, and continue to use the old
copied ones.

We could check if buffers are referenced in the scene and set the
LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
is really ok? (For vs, it doesn't matter that we miss the update - as
the offsets etc. are all the same the vs will just pick up the changes
automatically as we don't copy anything since this stuff all runs
synchronously.)




Err actually this analysis is flawed.
llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
bound constant buffer is changed and hence set LP_NEW_CONSTANTS accordingly.
But it doesn't because the buffer doesn't have the
PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
would be illegal to bind it as such, but this is never enforced). In
fact it doesn't have _any_ bind flag set.
So I blame the GL api, but I don't know how to fix this mess cleanly (we
don't really want to ignore bind flags completely).


Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse
you. If you use it for anything after resource creation, that's a bug.



This was designed in gallium with dx10 in mind, which has a proper api
for this. But I suppose the only way to fix it in the mesa state tracker
would be to just set ALL possible bind flags for buffers always...


Or you can assume bind == 0 means all flags are set.


Yes in this case, but I don't think that really helps. Even when using
the old gl api for setting this, the target (and hence the bind flag)
may be something completely different than the actual usage.


I'd rather have GL renderer set all possible bind flags.

And if the excess of bind flags leads certain drivers to significant 
inefficiencies, then those drivers can internally track exactly where 
those buffers/resources have been bound (similar to Roland's recent 
proposed patch for llvmpipe.)


(And if it turns out that all

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-10 Thread Roland Scheidegger 
Am 10.06.2016 um 12:38 schrieb Marek Olšák:
> On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger  
> wrote:
>> Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
>>> On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger  
>>> wrote:
 Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
> Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
>> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
>>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
 On Wednesday 08 June 2016, Ilia Mirkin wrote:
> Glancing at the code (I don't even have a piglit checkout here):
>
> static void
> set_ubo_binding(struct gl_context *ctx, ...)
> ...
>/* If this is a real buffer object, mark it has having been used
> * at some point as a UBO.
> */
>if (size >= 0)
>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>
> That seems bogus - what if the current size is 0 (unallocated), the
> buffer object gets bound to a UBO endpoint, and then someone goes in
> and does glBufferData()? Same for set_ssbo_binding.
>
>   -ilia

 The test is greater than or equal to zero, so the UsageHistory should
 be set even when the buffer is unallocated.
>>>
>>> Right, duh.
>>>

 But the piglit test doesn't bind the buffer as a uniform buffer before
 it allocates it.  It allocates the buffer first with 
 glNamedBufferData(),
 and then binds it.  The UsageHistory is still set to the default value 
 in
 the glNamedBufferData() call, since the buffer has never been bound
 at that point.  But the uniform buffer state should still be marked as
 dirty in the glBindBufferRange() call.  I think this failure suggests
 that that doesn't happen for some reason.
>>>
>>> I haven't looked in GREAT detail, but the test does pass on nv50,
>>> nvc0, and softpipe. It only fails on llvmpipe.
>>>
>>> Brian, this might be out of my comfort area to figure out... Given
>>> that it's working on the other drivers, that seems more likely to be a
>>> failing of llvmpipe somehow.
>>
>> Another observation is that the square sizes/shapes are all correct.
>> However the colors are all of the first square (red). So it seems like
>> some issue is happening for the fragment shader, but not vertex.
>>
>
> I've looked at this briefly and I'm not sure who's to blame.
> It goes something like this:
> - we don't get any set_constant_buffer calls anymore when the contents
> of the buffer change. I don't think that's ok, looks like a bug to me?
> Granted it's still the same buffer, just modfying a bound one.
>
> - when the contents are updated, it ends up in some transfer stuff as
> expected, in the end llvmpipe_transfer_map(). This checks if the
> resource is referenced in the current scene, if so it would flush -
> however this is only done for textures and rts (because UBO contents are
> indeed copied to the scene, not referenced, this is quite ok here, we
> don't want to flush).
>
> - on the next draw, we'd check the dirty bits - we never got
> set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
> LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
> so we don't pick up the changed contents, and continue to use the old
> copied ones.
>
> We could check if buffers are referenced in the scene and set the
> LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
> is really ok? (For vs, it doesn't matter that we miss the update - as
> the offsets etc. are all the same the vs will just pick up the changes
> automatically as we don't copy anything since this stuff all runs
> synchronously.)
>


 Err actually this analysis is flawed.
 llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
 bound constant buffer is changed and hence set LP_NEW_CONSTANTS 
 accordingly.
 But it doesn't because the buffer doesn't have the
 PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
 would be illegal to bind it as such, but this is never enforced). In
 fact it doesn't have _any_ bind flag set.
 So I blame the GL api, but I don't know how to fix this mess cleanly (we
 don't really want to ignore bind flags completely).
>>>
>>> Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse
>>> you. If you use it for anything after resource creation, that's a bug.
>>>
>>
>> This was designed in gallium with dx10 in mind, which has a proper api
>> for this. But I suppose the only way to fix it in the mesa state tracker
>> would be to just set ALL possible bind flags for buffers

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-10 Thread Marek Olšák 
On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger  wrote:
> Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
>> On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger  
>> wrote:
>>> Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
 Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
>>> On Wednesday 08 June 2016, Ilia Mirkin wrote:
 Glancing at the code (I don't even have a piglit checkout here):

 static void
 set_ubo_binding(struct gl_context *ctx, ...)
 ...
/* If this is a real buffer object, mark it has having been used
 * at some point as a UBO.
 */
if (size >= 0)
   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;

 That seems bogus - what if the current size is 0 (unallocated), the
 buffer object gets bound to a UBO endpoint, and then someone goes in
 and does glBufferData()? Same for set_ssbo_binding.

   -ilia
>>>
>>> The test is greater than or equal to zero, so the UsageHistory should
>>> be set even when the buffer is unallocated.
>>
>> Right, duh.
>>
>>>
>>> But the piglit test doesn't bind the buffer as a uniform buffer before
>>> it allocates it.  It allocates the buffer first with 
>>> glNamedBufferData(),
>>> and then binds it.  The UsageHistory is still set to the default value 
>>> in
>>> the glNamedBufferData() call, since the buffer has never been bound
>>> at that point.  But the uniform buffer state should still be marked as
>>> dirty in the glBindBufferRange() call.  I think this failure suggests
>>> that that doesn't happen for some reason.
>>
>> I haven't looked in GREAT detail, but the test does pass on nv50,
>> nvc0, and softpipe. It only fails on llvmpipe.
>>
>> Brian, this might be out of my comfort area to figure out... Given
>> that it's working on the other drivers, that seems more likely to be a
>> failing of llvmpipe somehow.
>
> Another observation is that the square sizes/shapes are all correct.
> However the colors are all of the first square (red). So it seems like
> some issue is happening for the fragment shader, but not vertex.
>

 I've looked at this briefly and I'm not sure who's to blame.
 It goes something like this:
 - we don't get any set_constant_buffer calls anymore when the contents
 of the buffer change. I don't think that's ok, looks like a bug to me?
 Granted it's still the same buffer, just modfying a bound one.

 - when the contents are updated, it ends up in some transfer stuff as
 expected, in the end llvmpipe_transfer_map(). This checks if the
 resource is referenced in the current scene, if so it would flush -
 however this is only done for textures and rts (because UBO contents are
 indeed copied to the scene, not referenced, this is quite ok here, we
 don't want to flush).

 - on the next draw, we'd check the dirty bits - we never got
 set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
 LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
 so we don't pick up the changed contents, and continue to use the old
 copied ones.

 We could check if buffers are referenced in the scene and set the
 LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
 is really ok? (For vs, it doesn't matter that we miss the update - as
 the offsets etc. are all the same the vs will just pick up the changes
 automatically as we don't copy anything since this stuff all runs
 synchronously.)

>>>
>>>
>>> Err actually this analysis is flawed.
>>> llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
>>> bound constant buffer is changed and hence set LP_NEW_CONSTANTS accordingly.
>>> But it doesn't because the buffer doesn't have the
>>> PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
>>> would be illegal to bind it as such, but this is never enforced). In
>>> fact it doesn't have _any_ bind flag set.
>>> So I blame the GL api, but I don't know how to fix this mess cleanly (we
>>> don't really want to ignore bind flags completely).
>>
>> Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse
>> you. If you use it for anything after resource creation, that's a bug.
>>
>
> This was designed in gallium with dx10 in mind, which has a proper api
> for this. But I suppose the only way to fix it in the mesa state tracker
> would be to just set ALL possible bind flags for buffers always...

Or you can assume bind == 0 means all flags are set.

Marek
___
mesa-dev mailing list

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Ilia Mirkin 
On Fri, Jun 10, 2016 at 12:19 AM, Roland Scheidegger  wrote:
> Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
>> On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger  
>> wrote:
>>> Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
 Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
>>> On Wednesday 08 June 2016, Ilia Mirkin wrote:
 Glancing at the code (I don't even have a piglit checkout here):

 static void
 set_ubo_binding(struct gl_context *ctx, ...)
 ...
/* If this is a real buffer object, mark it has having been used
 * at some point as a UBO.
 */
if (size >= 0)
   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;

 That seems bogus - what if the current size is 0 (unallocated), the
 buffer object gets bound to a UBO endpoint, and then someone goes in
 and does glBufferData()? Same for set_ssbo_binding.

   -ilia
>>>
>>> The test is greater than or equal to zero, so the UsageHistory should
>>> be set even when the buffer is unallocated.
>>
>> Right, duh.
>>
>>>
>>> But the piglit test doesn't bind the buffer as a uniform buffer before
>>> it allocates it.  It allocates the buffer first with 
>>> glNamedBufferData(),
>>> and then binds it.  The UsageHistory is still set to the default value 
>>> in
>>> the glNamedBufferData() call, since the buffer has never been bound
>>> at that point.  But the uniform buffer state should still be marked as
>>> dirty in the glBindBufferRange() call.  I think this failure suggests
>>> that that doesn't happen for some reason.
>>
>> I haven't looked in GREAT detail, but the test does pass on nv50,
>> nvc0, and softpipe. It only fails on llvmpipe.
>>
>> Brian, this might be out of my comfort area to figure out... Given
>> that it's working on the other drivers, that seems more likely to be a
>> failing of llvmpipe somehow.
>
> Another observation is that the square sizes/shapes are all correct.
> However the colors are all of the first square (red). So it seems like
> some issue is happening for the fragment shader, but not vertex.
>

 I've looked at this briefly and I'm not sure who's to blame.
 It goes something like this:
 - we don't get any set_constant_buffer calls anymore when the contents
 of the buffer change. I don't think that's ok, looks like a bug to me?
 Granted it's still the same buffer, just modfying a bound one.

 - when the contents are updated, it ends up in some transfer stuff as
 expected, in the end llvmpipe_transfer_map(). This checks if the
 resource is referenced in the current scene, if so it would flush -
 however this is only done for textures and rts (because UBO contents are
 indeed copied to the scene, not referenced, this is quite ok here, we
 don't want to flush).

 - on the next draw, we'd check the dirty bits - we never got
 set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
 LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
 so we don't pick up the changed contents, and continue to use the old
 copied ones.

 We could check if buffers are referenced in the scene and set the
 LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
 is really ok? (For vs, it doesn't matter that we miss the update - as
 the offsets etc. are all the same the vs will just pick up the changes
 automatically as we don't copy anything since this stuff all runs
 synchronously.)

>>>
>>>
>>> Err actually this analysis is flawed.
>>> llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
>>> bound constant buffer is changed and hence set LP_NEW_CONSTANTS accordingly.
>>> But it doesn't because the buffer doesn't have the
>>> PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
>>> would be illegal to bind it as such, but this is never enforced). In
>>> fact it doesn't have _any_ bind flag set.
>>> So I blame the GL api, but I don't know how to fix this mess cleanly (we
>>> don't really want to ignore bind flags completely).
>>
>> Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse
>> you. If you use it for anything after resource creation, that's a bug.
>>
>
> This was designed in gallium with dx10 in mind, which has a proper api
> for this. But I suppose the only way to fix it in the mesa state tracker
> would be to just set ALL possible bind flags for buffers always...

FWIW nouveau has similar requirements around constbuf uploads. I stuck
a cb_bindings per-shader bitmap on each piipe_resource (well,

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Roland Scheidegger 
Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
> On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger  
> wrote:
>> Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
>>> Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
 On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
>> On Wednesday 08 June 2016, Ilia Mirkin wrote:
>>> Glancing at the code (I don't even have a piglit checkout here):
>>>
>>> static void
>>> set_ubo_binding(struct gl_context *ctx, ...)
>>> ...
>>>/* If this is a real buffer object, mark it has having been used
>>> * at some point as a UBO.
>>> */
>>>if (size >= 0)
>>>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>>>
>>> That seems bogus - what if the current size is 0 (unallocated), the
>>> buffer object gets bound to a UBO endpoint, and then someone goes in
>>> and does glBufferData()? Same for set_ssbo_binding.
>>>
>>>   -ilia
>>
>> The test is greater than or equal to zero, so the UsageHistory should
>> be set even when the buffer is unallocated.
>
> Right, duh.
>
>>
>> But the piglit test doesn't bind the buffer as a uniform buffer before
>> it allocates it.  It allocates the buffer first with glNamedBufferData(),
>> and then binds it.  The UsageHistory is still set to the default value in
>> the glNamedBufferData() call, since the buffer has never been bound
>> at that point.  But the uniform buffer state should still be marked as
>> dirty in the glBindBufferRange() call.  I think this failure suggests
>> that that doesn't happen for some reason.
>
> I haven't looked in GREAT detail, but the test does pass on nv50,
> nvc0, and softpipe. It only fails on llvmpipe.
>
> Brian, this might be out of my comfort area to figure out... Given
> that it's working on the other drivers, that seems more likely to be a
> failing of llvmpipe somehow.

 Another observation is that the square sizes/shapes are all correct.
 However the colors are all of the first square (red). So it seems like
 some issue is happening for the fragment shader, but not vertex.

>>>
>>> I've looked at this briefly and I'm not sure who's to blame.
>>> It goes something like this:
>>> - we don't get any set_constant_buffer calls anymore when the contents
>>> of the buffer change. I don't think that's ok, looks like a bug to me?
>>> Granted it's still the same buffer, just modfying a bound one.
>>>
>>> - when the contents are updated, it ends up in some transfer stuff as
>>> expected, in the end llvmpipe_transfer_map(). This checks if the
>>> resource is referenced in the current scene, if so it would flush -
>>> however this is only done for textures and rts (because UBO contents are
>>> indeed copied to the scene, not referenced, this is quite ok here, we
>>> don't want to flush).
>>>
>>> - on the next draw, we'd check the dirty bits - we never got
>>> set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
>>> LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
>>> so we don't pick up the changed contents, and continue to use the old
>>> copied ones.
>>>
>>> We could check if buffers are referenced in the scene and set the
>>> LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
>>> is really ok? (For vs, it doesn't matter that we miss the update - as
>>> the offsets etc. are all the same the vs will just pick up the changes
>>> automatically as we don't copy anything since this stuff all runs
>>> synchronously.)
>>>
>>
>>
>> Err actually this analysis is flawed.
>> llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
>> bound constant buffer is changed and hence set LP_NEW_CONSTANTS accordingly.
>> But it doesn't because the buffer doesn't have the
>> PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
>> would be illegal to bind it as such, but this is never enforced). In
>> fact it doesn't have _any_ bind flag set.
>> So I blame the GL api, but I don't know how to fix this mess cleanly (we
>> don't really want to ignore bind flags completely).
> 
> Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse
> you. If you use it for anything after resource creation, that's a bug.
> 

This was designed in gallium with dx10 in mind, which has a proper api
for this. But I suppose the only way to fix it in the mesa state tracker
would be to just set ALL possible bind flags for buffers always...

Roland

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Ilia Mirkin 
On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger  wrote:
> Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
>> Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
>>> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
 On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
> On Wednesday 08 June 2016, Ilia Mirkin wrote:
>> Glancing at the code (I don't even have a piglit checkout here):
>>
>> static void
>> set_ubo_binding(struct gl_context *ctx, ...)
>> ...
>>/* If this is a real buffer object, mark it has having been used
>> * at some point as a UBO.
>> */
>>if (size >= 0)
>>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>>
>> That seems bogus - what if the current size is 0 (unallocated), the
>> buffer object gets bound to a UBO endpoint, and then someone goes in
>> and does glBufferData()? Same for set_ssbo_binding.
>>
>>   -ilia
>
> The test is greater than or equal to zero, so the UsageHistory should
> be set even when the buffer is unallocated.

 Right, duh.

>
> But the piglit test doesn't bind the buffer as a uniform buffer before
> it allocates it.  It allocates the buffer first with glNamedBufferData(),
> and then binds it.  The UsageHistory is still set to the default value in
> the glNamedBufferData() call, since the buffer has never been bound
> at that point.  But the uniform buffer state should still be marked as
> dirty in the glBindBufferRange() call.  I think this failure suggests
> that that doesn't happen for some reason.

 I haven't looked in GREAT detail, but the test does pass on nv50,
 nvc0, and softpipe. It only fails on llvmpipe.

 Brian, this might be out of my comfort area to figure out... Given
 that it's working on the other drivers, that seems more likely to be a
 failing of llvmpipe somehow.
>>>
>>> Another observation is that the square sizes/shapes are all correct.
>>> However the colors are all of the first square (red). So it seems like
>>> some issue is happening for the fragment shader, but not vertex.
>>>
>>
>> I've looked at this briefly and I'm not sure who's to blame.
>> It goes something like this:
>> - we don't get any set_constant_buffer calls anymore when the contents
>> of the buffer change. I don't think that's ok, looks like a bug to me?
>> Granted it's still the same buffer, just modfying a bound one.
>>
>> - when the contents are updated, it ends up in some transfer stuff as
>> expected, in the end llvmpipe_transfer_map(). This checks if the
>> resource is referenced in the current scene, if so it would flush -
>> however this is only done for textures and rts (because UBO contents are
>> indeed copied to the scene, not referenced, this is quite ok here, we
>> don't want to flush).
>>
>> - on the next draw, we'd check the dirty bits - we never got
>> set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
>> LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
>> so we don't pick up the changed contents, and continue to use the old
>> copied ones.
>>
>> We could check if buffers are referenced in the scene and set the
>> LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
>> is really ok? (For vs, it doesn't matter that we miss the update - as
>> the offsets etc. are all the same the vs will just pick up the changes
>> automatically as we don't copy anything since this stuff all runs
>> synchronously.)
>>
>
>
> Err actually this analysis is flawed.
> llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
> bound constant buffer is changed and hence set LP_NEW_CONSTANTS accordingly.
> But it doesn't because the buffer doesn't have the
> PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
> would be illegal to bind it as such, but this is never enforced). In
> fact it doesn't have _any_ bind flag set.
> So I blame the GL api, but I don't know how to fix this mess cleanly (we
> don't really want to ignore bind flags completely).

Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse
you. If you use it for anything after resource creation, that's a bug.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Roland Scheidegger 
Am 10.06.2016 um 04:58 schrieb Roland Scheidegger:
> Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
>> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
>>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
 On Wednesday 08 June 2016, Ilia Mirkin wrote:
> Glancing at the code (I don't even have a piglit checkout here):
>
> static void
> set_ubo_binding(struct gl_context *ctx, ...)
> ...
>/* If this is a real buffer object, mark it has having been used
> * at some point as a UBO.
> */
>if (size >= 0)
>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>
> That seems bogus - what if the current size is 0 (unallocated), the
> buffer object gets bound to a UBO endpoint, and then someone goes in
> and does glBufferData()? Same for set_ssbo_binding.
>
>   -ilia

 The test is greater than or equal to zero, so the UsageHistory should
 be set even when the buffer is unallocated.
>>>
>>> Right, duh.
>>>

 But the piglit test doesn't bind the buffer as a uniform buffer before
 it allocates it.  It allocates the buffer first with glNamedBufferData(),
 and then binds it.  The UsageHistory is still set to the default value in
 the glNamedBufferData() call, since the buffer has never been bound
 at that point.  But the uniform buffer state should still be marked as
 dirty in the glBindBufferRange() call.  I think this failure suggests
 that that doesn't happen for some reason.
>>>
>>> I haven't looked in GREAT detail, but the test does pass on nv50,
>>> nvc0, and softpipe. It only fails on llvmpipe.
>>>
>>> Brian, this might be out of my comfort area to figure out... Given
>>> that it's working on the other drivers, that seems more likely to be a
>>> failing of llvmpipe somehow.
>>
>> Another observation is that the square sizes/shapes are all correct.
>> However the colors are all of the first square (red). So it seems like
>> some issue is happening for the fragment shader, but not vertex.
>>
> 
> I've looked at this briefly and I'm not sure who's to blame.
> It goes something like this:
> - we don't get any set_constant_buffer calls anymore when the contents
> of the buffer change. I don't think that's ok, looks like a bug to me?
> Granted it's still the same buffer, just modfying a bound one.
> 
> - when the contents are updated, it ends up in some transfer stuff as
> expected, in the end llvmpipe_transfer_map(). This checks if the
> resource is referenced in the current scene, if so it would flush -
> however this is only done for textures and rts (because UBO contents are
> indeed copied to the scene, not referenced, this is quite ok here, we
> don't want to flush).
> 
> - on the next draw, we'd check the dirty bits - we never got
> set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
> LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
> so we don't pick up the changed contents, and continue to use the old
> copied ones.
> 
> We could check if buffers are referenced in the scene and set the
> LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
> is really ok? (For vs, it doesn't matter that we miss the update - as
> the offsets etc. are all the same the vs will just pick up the changes
> automatically as we don't copy anything since this stuff all runs
> synchronously.)
> 


Err actually this analysis is flawed.
llvmpipe would indeed check in llvmpipe_transfer_map() if a currently
bound constant buffer is changed and hence set LP_NEW_CONSTANTS accordingly.
But it doesn't because the buffer doesn't have the
PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it
would be illegal to bind it as such, but this is never enforced). In
fact it doesn't have _any_ bind flag set.
So I blame the GL api, but I don't know how to fix this mess cleanly (we
don't really want to ignore bind flags completely).

Roland


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Ilia Mirkin 
On Thu, Jun 9, 2016 at 10:58 PM, Roland Scheidegger  wrote:
> Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
>> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
>>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
 On Wednesday 08 June 2016, Ilia Mirkin wrote:
> Glancing at the code (I don't even have a piglit checkout here):
>
> static void
> set_ubo_binding(struct gl_context *ctx, ...)
> ...
>/* If this is a real buffer object, mark it has having been used
> * at some point as a UBO.
> */
>if (size >= 0)
>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>
> That seems bogus - what if the current size is 0 (unallocated), the
> buffer object gets bound to a UBO endpoint, and then someone goes in
> and does glBufferData()? Same for set_ssbo_binding.
>
>   -ilia

 The test is greater than or equal to zero, so the UsageHistory should
 be set even when the buffer is unallocated.
>>>
>>> Right, duh.
>>>

 But the piglit test doesn't bind the buffer as a uniform buffer before
 it allocates it.  It allocates the buffer first with glNamedBufferData(),
 and then binds it.  The UsageHistory is still set to the default value in
 the glNamedBufferData() call, since the buffer has never been bound
 at that point.  But the uniform buffer state should still be marked as
 dirty in the glBindBufferRange() call.  I think this failure suggests
 that that doesn't happen for some reason.
>>>
>>> I haven't looked in GREAT detail, but the test does pass on nv50,
>>> nvc0, and softpipe. It only fails on llvmpipe.
>>>
>>> Brian, this might be out of my comfort area to figure out... Given
>>> that it's working on the other drivers, that seems more likely to be a
>>> failing of llvmpipe somehow.
>>
>> Another observation is that the square sizes/shapes are all correct.
>> However the colors are all of the first square (red). So it seems like
>> some issue is happening for the fragment shader, but not vertex.
>>
>
> I've looked at this briefly and I'm not sure who's to blame.
> It goes something like this:
> - we don't get any set_constant_buffer calls anymore when the contents
> of the buffer change. I don't think that's ok, looks like a bug to me?
> Granted it's still the same buffer, just modfying a bound one.

That's very much intentional, at least as I understand it. The buffer
hasn't changed, neither have the start/size. The fact that you were
getting a dirty bit set before was an accident of implementation - had
the test not been calling piglit_draw_rect(), glBufferData() would not
have been called, and you would not have seen the new bindings, which
would not have triggered the unnecessary dirtying of the UBO. It's
really unfortunate that piglit_draw_rect() ends up dirtying so much
state - this can end up papering over other issues.

>
> - when the contents are updated, it ends up in some transfer stuff as
> expected, in the end llvmpipe_transfer_map(). This checks if the
> resource is referenced in the current scene, if so it would flush -
> however this is only done for textures and rts (because UBO contents are
> indeed copied to the scene, not referenced, this is quite ok here, we
> don't want to flush).
>
> - on the next draw, we'd check the dirty bits - we never got
> set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
> LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
> so we don't pick up the changed contents, and continue to use the old
> copied ones.
>
> We could check if buffers are referenced in the scene and set the
> LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
> is really ok? (For vs, it doesn't matter that we miss the update - as
> the offsets etc. are all the same the vs will just pick up the changes
> automatically as we don't copy anything since this stuff all runs
> synchronously.)

I believe that your transfer_unmap/flush logic needs to become
smarter. Especially in the situation of persistently and
coherently-mapped buffers, you really need to keep track of this
stuff. (And you appear to support ARB_buffer_storage).

Cheers,

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Roland Scheidegger 
Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
>>> On Wednesday 08 June 2016, Ilia Mirkin wrote:
 Glancing at the code (I don't even have a piglit checkout here):

 static void
 set_ubo_binding(struct gl_context *ctx, ...)
 ...
/* If this is a real buffer object, mark it has having been used
 * at some point as a UBO.
 */
if (size >= 0)
   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;

 That seems bogus - what if the current size is 0 (unallocated), the
 buffer object gets bound to a UBO endpoint, and then someone goes in
 and does glBufferData()? Same for set_ssbo_binding.

   -ilia
>>>
>>> The test is greater than or equal to zero, so the UsageHistory should
>>> be set even when the buffer is unallocated.
>>
>> Right, duh.
>>
>>>
>>> But the piglit test doesn't bind the buffer as a uniform buffer before
>>> it allocates it.  It allocates the buffer first with glNamedBufferData(),
>>> and then binds it.  The UsageHistory is still set to the default value in
>>> the glNamedBufferData() call, since the buffer has never been bound
>>> at that point.  But the uniform buffer state should still be marked as
>>> dirty in the glBindBufferRange() call.  I think this failure suggests
>>> that that doesn't happen for some reason.
>>
>> I haven't looked in GREAT detail, but the test does pass on nv50,
>> nvc0, and softpipe. It only fails on llvmpipe.
>>
>> Brian, this might be out of my comfort area to figure out... Given
>> that it's working on the other drivers, that seems more likely to be a
>> failing of llvmpipe somehow.
> 
> Another observation is that the square sizes/shapes are all correct.
> However the colors are all of the first square (red). So it seems like
> some issue is happening for the fragment shader, but not vertex.
> 

I've looked at this briefly and I'm not sure who's to blame.
It goes something like this:
- we don't get any set_constant_buffer calls anymore when the contents
of the buffer change. I don't think that's ok, looks like a bug to me?
Granted it's still the same buffer, just modfying a bound one.

- when the contents are updated, it ends up in some transfer stuff as
expected, in the end llvmpipe_transfer_map(). This checks if the
resource is referenced in the current scene, if so it would flush -
however this is only done for textures and rts (because UBO contents are
indeed copied to the scene, not referenced, this is quite ok here, we
don't want to flush).

- on the next draw, we'd check the dirty bits - we never got
set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
so we don't pick up the changed contents, and continue to use the old
copied ones.

We could check if buffers are referenced in the scene and set the
LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
is really ok? (For vs, it doesn't matter that we miss the update - as
the offsets etc. are all the same the vs will just pick up the changes
automatically as we don't copy anything since this stuff all runs
synchronously.)

Roland


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Ilia Mirkin 
On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin  wrote:
> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
>> On Wednesday 08 June 2016, Ilia Mirkin wrote:
>>> Glancing at the code (I don't even have a piglit checkout here):
>>>
>>> static void
>>> set_ubo_binding(struct gl_context *ctx, ...)
>>> ...
>>>/* If this is a real buffer object, mark it has having been used
>>> * at some point as a UBO.
>>> */
>>>if (size >= 0)
>>>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>>>
>>> That seems bogus - what if the current size is 0 (unallocated), the
>>> buffer object gets bound to a UBO endpoint, and then someone goes in
>>> and does glBufferData()? Same for set_ssbo_binding.
>>>
>>>   -ilia
>>
>> The test is greater than or equal to zero, so the UsageHistory should
>> be set even when the buffer is unallocated.
>
> Right, duh.
>
>>
>> But the piglit test doesn't bind the buffer as a uniform buffer before
>> it allocates it.  It allocates the buffer first with glNamedBufferData(),
>> and then binds it.  The UsageHistory is still set to the default value in
>> the glNamedBufferData() call, since the buffer has never been bound
>> at that point.  But the uniform buffer state should still be marked as
>> dirty in the glBindBufferRange() call.  I think this failure suggests
>> that that doesn't happen for some reason.
>
> I haven't looked in GREAT detail, but the test does pass on nv50,
> nvc0, and softpipe. It only fails on llvmpipe.
>
> Brian, this might be out of my comfort area to figure out... Given
> that it's working on the other drivers, that seems more likely to be a
> failing of llvmpipe somehow.

Another observation is that the square sizes/shapes are all correct.
However the colors are all of the first square (red). So it seems like
some issue is happening for the fragment shader, but not vertex.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-09 Thread Ilia Mirkin 
On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund  wrote:
> On Wednesday 08 June 2016, Ilia Mirkin wrote:
>> Glancing at the code (I don't even have a piglit checkout here):
>>
>> static void
>> set_ubo_binding(struct gl_context *ctx, ...)
>> ...
>>/* If this is a real buffer object, mark it has having been used
>> * at some point as a UBO.
>> */
>>if (size >= 0)
>>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>>
>> That seems bogus - what if the current size is 0 (unallocated), the
>> buffer object gets bound to a UBO endpoint, and then someone goes in
>> and does glBufferData()? Same for set_ssbo_binding.
>>
>>   -ilia
>
> The test is greater than or equal to zero, so the UsageHistory should
> be set even when the buffer is unallocated.

Right, duh.

>
> But the piglit test doesn't bind the buffer as a uniform buffer before
> it allocates it.  It allocates the buffer first with glNamedBufferData(),
> and then binds it.  The UsageHistory is still set to the default value in
> the glNamedBufferData() call, since the buffer has never been bound
> at that point.  But the uniform buffer state should still be marked as
> dirty in the glBindBufferRange() call.  I think this failure suggests
> that that doesn't happen for some reason.

I haven't looked in GREAT detail, but the test does pass on nv50,
nvc0, and softpipe. It only fails on llvmpipe.

Brian, this might be out of my comfort area to figure out... Given
that it's working on the other drivers, that seems more likely to be a
failing of llvmpipe somehow.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-08 Thread Fredrik Höglund 
On Wednesday 08 June 2016, Ilia Mirkin wrote:
> Glancing at the code (I don't even have a piglit checkout here):
> 
> static void
> set_ubo_binding(struct gl_context *ctx, ...)
> ...
>/* If this is a real buffer object, mark it has having been used
> * at some point as a UBO.
> */
>if (size >= 0)
>   bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
> 
> That seems bogus - what if the current size is 0 (unallocated), the
> buffer object gets bound to a UBO endpoint, and then someone goes in
> and does glBufferData()? Same for set_ssbo_binding.
> 
>   -ilia

The test is greater than or equal to zero, so the UsageHistory should
be set even when the buffer is unallocated.

But the piglit test doesn't bind the buffer as a uniform buffer before
it allocates it.  It allocates the buffer first with glNamedBufferData(),
and then binds it.  The UsageHistory is still set to the default value in
the glNamedBufferData() call, since the buffer has never been bound
at that point.  But the uniform buffer state should still be marked as
dirty in the glBindBufferRange() call.  I think this failure suggests
that that doesn't happen for some reason.

Fredrik

> 
> 
> On Wed, Jun 8, 2016 at 2:28 PM, Ilia Mirkin  wrote:
> > Hm, that's odd. I guess the buffer usage doesn't get set properly? I
> > won't be able to look at this until tonight at the earliest, feel free
> > to revert the change in the meanwhile.
> >
> >   -ilia
> >
> > On Wed, Jun 8, 2016 at 1:25 PM, Brian Paul  wrote:
> >> Ilia, this patch causes a regression in the piglit
> >> arb_uniform_buffer_object-rendering-dsa test with llvmpipe (at least).
> >>
> >> I haven't debugged it at all.
> >>
> >> -Brian
> >>
> >> On 06/07/2016 08:29 PM, Ilia Mirkin wrote:
> >>>
> >>> Module: Mesa
> >>> Branch: master
> >>> Commit: 6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02
> >>> URL:
> >>> https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3D6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=zn9XJ75wNk6pxn5KNFcWfLu7sXurLGGcWfIvVtSMwEo=
> >>>
> >>> Author: Ilia Mirkin 
> >>> Date:   Sat Jun  4 13:26:46 2016 -0400
> >>>
> >>> st/mesa: use buffer usage history to set dirty flags for revalidation
> >>>
> >>> We were previously unconditionally doing this for arrays and ubo's, and
> >>> ignoring texture/storage/atomic buffers. Instead use the usage history
> >>> to determine which atoms need to be revalidated.
> >>>
> >>> Signed-off-by: Ilia Mirkin 
> >>> Reviewed-by: Nicolai Hähnle 
> >>> Cc: "12.0" 
> >>>
> >>> ---
> >>>
> >>>   src/mesa/state_tracker/st_cb_bufferobjects.c | 15 +--
> >>>   1 file changed, 13 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> b/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> index 8bbc2f0..1a8aea3 100644
> >>> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> @@ -332,8 +332,19 @@ st_bufferobj_data(struct gl_context *ctx,
> >>> }
> >>>  }
> >>>
> >>> -   /* BufferData may change an array or uniform buffer, need to update it
> >>> */
> >>> -   st->dirty.st |= ST_NEW_VERTEX_ARRAYS | ST_NEW_UNIFORM_BUFFER;
> >>> +   /* The current buffer may be bound, so we have to revalidate all atoms
> >>> that
> >>> +* might be using it.
> >>> +*/
> >>> +   /* TODO: Add arrays to usage history */
> >>> +   st->dirty.st |= ST_NEW_VERTEX_ARRAYS;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_UNIFORM_BUFFER)
> >>> +  st->dirty.st |= ST_NEW_UNIFORM_BUFFER;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_SHADER_STORAGE_BUFFER)
> >>> +  st->dirty.st |= ST_NEW_STORAGE_BUFFER;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_TEXTURE_BUFFER)
> >>> +  st->dirty.st |= ST_NEW_SAMPLER_VIEWS | ST_NEW_IMAGE_UNITS;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_ATOMIC_COUNTER_BUFFER)
> >>> +  st->dirty.st |= ST_NEW_ATOMIC_BUFFER;
> >>>
> >>>  return GL_TRUE;
> >>>   }
> >>>
> >>> ___
> >>> mesa-commit mailing list
> >>> mesa-com...@lists.freedesktop.org
> >>>
> >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=TUTsMchaJhWjCx5k6tptwVWWVHJs-zDtCtJVpPhM1pM=
> >>>
> >>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-08 Thread Ilia Mirkin 
Glancing at the code (I don't even have a piglit checkout here):

static void
set_ubo_binding(struct gl_context *ctx, ...)
...
   /* If this is a real buffer object, mark it has having been used
* at some point as a UBO.
*/
   if (size >= 0)
  bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;

That seems bogus - what if the current size is 0 (unallocated), the
buffer object gets bound to a UBO endpoint, and then someone goes in
and does glBufferData()? Same for set_ssbo_binding.

  -ilia


On Wed, Jun 8, 2016 at 2:28 PM, Ilia Mirkin  wrote:
> Hm, that's odd. I guess the buffer usage doesn't get set properly? I
> won't be able to look at this until tonight at the earliest, feel free
> to revert the change in the meanwhile.
>
>   -ilia
>
> On Wed, Jun 8, 2016 at 1:25 PM, Brian Paul  wrote:
>> Ilia, this patch causes a regression in the piglit
>> arb_uniform_buffer_object-rendering-dsa test with llvmpipe (at least).
>>
>> I haven't debugged it at all.
>>
>> -Brian
>>
>> On 06/07/2016 08:29 PM, Ilia Mirkin wrote:
>>>
>>> Module: Mesa
>>> Branch: master
>>> Commit: 6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02
>>> URL:
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3D6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=zn9XJ75wNk6pxn5KNFcWfLu7sXurLGGcWfIvVtSMwEo=
>>>
>>> Author: Ilia Mirkin 
>>> Date:   Sat Jun  4 13:26:46 2016 -0400
>>>
>>> st/mesa: use buffer usage history to set dirty flags for revalidation
>>>
>>> We were previously unconditionally doing this for arrays and ubo's, and
>>> ignoring texture/storage/atomic buffers. Instead use the usage history
>>> to determine which atoms need to be revalidated.
>>>
>>> Signed-off-by: Ilia Mirkin 
>>> Reviewed-by: Nicolai Hähnle 
>>> Cc: "12.0" 
>>>
>>> ---
>>>
>>>   src/mesa/state_tracker/st_cb_bufferobjects.c | 15 +--
>>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
>>> b/src/mesa/state_tracker/st_cb_bufferobjects.c
>>> index 8bbc2f0..1a8aea3 100644
>>> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
>>> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
>>> @@ -332,8 +332,19 @@ st_bufferobj_data(struct gl_context *ctx,
>>> }
>>>  }
>>>
>>> -   /* BufferData may change an array or uniform buffer, need to update it
>>> */
>>> -   st->dirty.st |= ST_NEW_VERTEX_ARRAYS | ST_NEW_UNIFORM_BUFFER;
>>> +   /* The current buffer may be bound, so we have to revalidate all atoms
>>> that
>>> +* might be using it.
>>> +*/
>>> +   /* TODO: Add arrays to usage history */
>>> +   st->dirty.st |= ST_NEW_VERTEX_ARRAYS;
>>> +   if (st_obj->Base.UsageHistory & USAGE_UNIFORM_BUFFER)
>>> +  st->dirty.st |= ST_NEW_UNIFORM_BUFFER;
>>> +   if (st_obj->Base.UsageHistory & USAGE_SHADER_STORAGE_BUFFER)
>>> +  st->dirty.st |= ST_NEW_STORAGE_BUFFER;
>>> +   if (st_obj->Base.UsageHistory & USAGE_TEXTURE_BUFFER)
>>> +  st->dirty.st |= ST_NEW_SAMPLER_VIEWS | ST_NEW_IMAGE_UNITS;
>>> +   if (st_obj->Base.UsageHistory & USAGE_ATOMIC_COUNTER_BUFFER)
>>> +  st->dirty.st |= ST_NEW_ATOMIC_BUFFER;
>>>
>>>  return GL_TRUE;
>>>   }
>>>
>>> ___
>>> mesa-commit mailing list
>>> mesa-com...@lists.freedesktop.org
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=TUTsMchaJhWjCx5k6tptwVWWVHJs-zDtCtJVpPhM1pM=
>>>
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-08 Thread Ilia Mirkin 
Hm, that's odd. I guess the buffer usage doesn't get set properly? I
won't be able to look at this until tonight at the earliest, feel free
to revert the change in the meanwhile.

  -ilia

On Wed, Jun 8, 2016 at 1:25 PM, Brian Paul  wrote:
> Ilia, this patch causes a regression in the piglit
> arb_uniform_buffer_object-rendering-dsa test with llvmpipe (at least).
>
> I haven't debugged it at all.
>
> -Brian
>
> On 06/07/2016 08:29 PM, Ilia Mirkin wrote:
>>
>> Module: Mesa
>> Branch: master
>> Commit: 6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02
>> URL:
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3D6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=zn9XJ75wNk6pxn5KNFcWfLu7sXurLGGcWfIvVtSMwEo=
>>
>> Author: Ilia Mirkin 
>> Date:   Sat Jun  4 13:26:46 2016 -0400
>>
>> st/mesa: use buffer usage history to set dirty flags for revalidation
>>
>> We were previously unconditionally doing this for arrays and ubo's, and
>> ignoring texture/storage/atomic buffers. Instead use the usage history
>> to determine which atoms need to be revalidated.
>>
>> Signed-off-by: Ilia Mirkin 
>> Reviewed-by: Nicolai Hähnle 
>> Cc: "12.0" 
>>
>> ---
>>
>>   src/mesa/state_tracker/st_cb_bufferobjects.c | 15 +--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
>> b/src/mesa/state_tracker/st_cb_bufferobjects.c
>> index 8bbc2f0..1a8aea3 100644
>> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
>> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
>> @@ -332,8 +332,19 @@ st_bufferobj_data(struct gl_context *ctx,
>> }
>>  }
>>
>> -   /* BufferData may change an array or uniform buffer, need to update it
>> */
>> -   st->dirty.st |= ST_NEW_VERTEX_ARRAYS | ST_NEW_UNIFORM_BUFFER;
>> +   /* The current buffer may be bound, so we have to revalidate all atoms
>> that
>> +* might be using it.
>> +*/
>> +   /* TODO: Add arrays to usage history */
>> +   st->dirty.st |= ST_NEW_VERTEX_ARRAYS;
>> +   if (st_obj->Base.UsageHistory & USAGE_UNIFORM_BUFFER)
>> +  st->dirty.st |= ST_NEW_UNIFORM_BUFFER;
>> +   if (st_obj->Base.UsageHistory & USAGE_SHADER_STORAGE_BUFFER)
>> +  st->dirty.st |= ST_NEW_STORAGE_BUFFER;
>> +   if (st_obj->Base.UsageHistory & USAGE_TEXTURE_BUFFER)
>> +  st->dirty.st |= ST_NEW_SAMPLER_VIEWS | ST_NEW_IMAGE_UNITS;
>> +   if (st_obj->Base.UsageHistory & USAGE_ATOMIC_COUNTER_BUFFER)
>> +  st->dirty.st |= ST_NEW_ATOMIC_BUFFER;
>>
>>  return GL_TRUE;
>>   }
>>
>> ___
>> mesa-commit mailing list
>> mesa-com...@lists.freedesktop.org
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=TUTsMchaJhWjCx5k6tptwVWWVHJs-zDtCtJVpPhM1pM=
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

2016-06-08 Thread Brian Paul 
Ilia, this patch causes a regression in the piglit 
arb_uniform_buffer_object-rendering-dsa test with llvmpipe (at least).


I haven't debugged it at all.

-Brian

On 06/07/2016 08:29 PM, Ilia Mirkin wrote:

Module: Mesa
Branch: master
Commit: 6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02
URL:
https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3D6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=zn9XJ75wNk6pxn5KNFcWfLu7sXurLGGcWfIvVtSMwEo=

Author: Ilia Mirkin 
Date:   Sat Jun  4 13:26:46 2016 -0400

st/mesa: use buffer usage history to set dirty flags for revalidation

We were previously unconditionally doing this for arrays and ubo's, and
ignoring texture/storage/atomic buffers. Instead use the usage history
to determine which atoms need to be revalidated.

Signed-off-by: Ilia Mirkin 
Reviewed-by: Nicolai Hähnle 
Cc: "12.0" 

---

  src/mesa/state_tracker/st_cb_bufferobjects.c | 15 +--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c 
b/src/mesa/state_tracker/st_cb_bufferobjects.c
index 8bbc2f0..1a8aea3 100644
--- a/src/mesa/state_tracker/st_cb_bufferobjects.c
+++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
@@ -332,8 +332,19 @@ st_bufferobj_data(struct gl_context *ctx,
}
 }

-   /* BufferData may change an array or uniform buffer, need to update it */
-   st->dirty.st |= ST_NEW_VERTEX_ARRAYS | ST_NEW_UNIFORM_BUFFER;
+   /* The current buffer may be bound, so we have to revalidate all atoms that
+* might be using it.
+*/
+   /* TODO: Add arrays to usage history */
+   st->dirty.st |= ST_NEW_VERTEX_ARRAYS;
+   if (st_obj->Base.UsageHistory & USAGE_UNIFORM_BUFFER)
+  st->dirty.st |= ST_NEW_UNIFORM_BUFFER;
+   if (st_obj->Base.UsageHistory & USAGE_SHADER_STORAGE_BUFFER)
+  st->dirty.st |= ST_NEW_STORAGE_BUFFER;
+   if (st_obj->Base.UsageHistory & USAGE_TEXTURE_BUFFER)
+  st->dirty.st |= ST_NEW_SAMPLER_VIEWS | ST_NEW_IMAGE_UNITS;
+   if (st_obj->Base.UsageHistory & USAGE_ATOMIC_COUNTER_BUFFER)
+  st->dirty.st |= ST_NEW_ATOMIC_BUFFER;

 return GL_TRUE;
  }

___
mesa-commit mailing list
mesa-com...@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY=TUTsMchaJhWjCx5k6tptwVWWVHJs-zDtCtJVpPhM1pM=



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev