Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-27 Thread Brian Paul

On 09/12/2013 06:46 PM, Brian Paul wrote:


I just pushed a gallium-bind-sampler-states branch to my git repo at
git://people.freedesktop.org/~brianp/mesa

It replaces the four
pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
functions with a single bind_sampler_states() function:

  void (*bind_sampler_states)(struct pipe_context *,
  unsigned shader, unsigned start_slot,
  unsigned num_samplers, void **samplers);

At this point start_slot is always zero (at least for non-compute
shaders).  And as the updated gallium docs explain, at some point calls
to bind_sampler_states() will be used to updated sub-ranges, but that
never happens currently.

I've updated all the drivers, state trackers, utils, etc.

I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
texture subset of piglit pass w/out regressions.  I'd appreciate it if
other driver developers would test their favorite driver.


OK, I've gotten a few thumbs up on this change but no actual 
Reviewed-by.  I'd like to push this master in the next few days.  If 
there's no other feedback I'll do so on Monday or so.


-Brian

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


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-16 Thread Roland Scheidegger
Am 14.09.2013 18:24, schrieb Brian Paul:
 On 09/12/2013 09:06 PM, Chia-I Wu wrote:
 Hi Brian,

 On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:

 I just pushed a gallium-bind-sampler-states branch to my git repo at
 git://people.freedesktop.org/~brianp/mesa

 It replaces the four
 pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
 functions with a single bind_sampler_states() function:

   void (*bind_sampler_states)(struct pipe_context *,
   unsigned shader, unsigned start_slot,
   unsigned num_samplers, void **samplers);

 At this point start_slot is always zero (at least for non-compute
 shaders).
 And as the updated gallium docs explain, at some point calls to
 bind_sampler_states() will be used to updated sub-ranges, but that never
 happens currently.

 I've updated all the drivers, state trackers, utils, etc.

 I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
 texture subset of piglit pass w/out regressions.  I'd appreciate it
 if other
 driver developers would test their favorite driver.
 For ilo, the new code does not follow the doc and unbinds samplers not
 in range.
 
 I think that's OK.  The CSO module (used by the state tracker) currently
 always calls pipe_context::bind_sampler_states() with start=0 and count
 such that it sets/replaces all samplers, never a sub-range.  That
 could/should change in the future.
 
 See single_sampler_done() in cso_context.c.

Not all state trackers use cso though. While this can be fixed later, I
think I'd prefer if the docs would state this isn't really meant to be
that way and should be changed in the future in cso and certainly fixed
in the drivers.

Roland



 
 
 Is it fine if I implement the new bind_sampler_states as a helper
 function on master branch, so that you hook it up to
 pipe_context::bind_sampler_states in your branch and remove the old
 ones?
 
 I'm not quite sure that I understand what you mean.  Can you elaborate?
 
 -Brian
 
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-16 Thread Brian Paul

On 09/15/2013 09:31 AM, Chia-I Wu wrote:

On Sun, Sep 15, 2013 at 12:24 AM, Brian Paul bri...@vmware.com wrote:

On 09/12/2013 09:06 PM, Chia-I Wu wrote:


Hi Brian,

On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:



I just pushed a gallium-bind-sampler-states branch to my git repo at
git://people.freedesktop.org/~brianp/mesa

It replaces the four
pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
functions with a single bind_sampler_states() function:

   void (*bind_sampler_states)(struct pipe_context *,
   unsigned shader, unsigned start_slot,
   unsigned num_samplers, void **samplers);

At this point start_slot is always zero (at least for non-compute
shaders).
And as the updated gallium docs explain, at some point calls to
bind_sampler_states() will be used to updated sub-ranges, but that never
happens currently.

I've updated all the drivers, state trackers, utils, etc.

I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
texture subset of piglit pass w/out regressions.  I'd appreciate it if
other
driver developers would test their favorite driver.


For ilo, the new code does not follow the doc and unbinds samplers not in
range.



I think that's OK.  The CSO module (used by the state tracker) currently
always calls pipe_context::bind_sampler_states() with start=0 and count such
that it sets/replaces all samplers, never a sub-range.  That could/should
change in the future.

See single_sampler_done() in cso_context.c.




Is it fine if I implement the new bind_sampler_states as a helper
function on master branch, so that you hook it up to
pipe_context::bind_sampler_states in your branch and remove the old
ones?



I'm not quite sure that I understand what you mean.  Can you elaborate?

There is already ilo_bind_sampler_states that does what
pipe_context::bind_sampler_states expects, except that the function
returns a bool.  I can make it return void so that, in your branch,
you can initialize pipe_context::bind_sampler_states to it instead of
adding ilo_bind_sampler_states2.


Sure, feel free to refactor as you see fit.  I didn't do that in the 
first place because the existing ilo_bind_X_sampler_states() functions 
are calling ilo_bind_sampler_states() twice and setting dirty flags.  I 
didn't feel comfortable rewriting all that and likely breaking it so I 
just wrapped things up with ilo_bind_sampler_states2().


-Brian

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


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-16 Thread Chia-I Wu
On Tue, Sep 17, 2013 at 12:09 AM, Brian Paul bri...@vmware.com wrote:
 On 09/15/2013 09:31 AM, Chia-I Wu wrote:

 On Sun, Sep 15, 2013 at 12:24 AM, Brian Paul bri...@vmware.com wrote:

 On 09/12/2013 09:06 PM, Chia-I Wu wrote:


 Hi Brian,

 On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:



 I just pushed a gallium-bind-sampler-states branch to my git repo at
 git://people.freedesktop.org/~brianp/mesa

 It replaces the four
 pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
 functions with a single bind_sampler_states() function:

void (*bind_sampler_states)(struct pipe_context *,
unsigned shader, unsigned start_slot,
unsigned num_samplers, void **samplers);

 At this point start_slot is always zero (at least for non-compute
 shaders).
 And as the updated gallium docs explain, at some point calls to
 bind_sampler_states() will be used to updated sub-ranges, but that
 never
 happens currently.

 I've updated all the drivers, state trackers, utils, etc.

 I've tested the svga, llvmpipe and softpipe drivers.  'make check' and
 a
 texture subset of piglit pass w/out regressions.  I'd appreciate it if
 other
 driver developers would test their favorite driver.


 For ilo, the new code does not follow the doc and unbinds samplers not
 in
 range.



 I think that's OK.  The CSO module (used by the state tracker) currently
 always calls pipe_context::bind_sampler_states() with start=0 and count
 such
 that it sets/replaces all samplers, never a sub-range.  That could/should
 change in the future.

 See single_sampler_done() in cso_context.c.



 Is it fine if I implement the new bind_sampler_states as a helper
 function on master branch, so that you hook it up to
 pipe_context::bind_sampler_states in your branch and remove the old
 ones?



 I'm not quite sure that I understand what you mean.  Can you elaborate?

 There is already ilo_bind_sampler_states that does what
 pipe_context::bind_sampler_states expects, except that the function
 returns a bool.  I can make it return void so that, in your branch,
 you can initialize pipe_context::bind_sampler_states to it instead of
 adding ilo_bind_sampler_states2.


 Sure, feel free to refactor as you see fit.  I didn't do that in the first
 place because the existing ilo_bind_X_sampler_states() functions are calling
 ilo_bind_sampler_states() twice and setting dirty flags.  I didn't feel
 comfortable rewriting all that and likely breaking it so I just wrapped
 things up with ilo_bind_sampler_states2().
I've pushed the change.  It is called twice by
ilo_bind_X_sampler_states() needs to bind the samplers in the range,
and unbind samplers that are not.

It's great to see the interface change.  Thanks for the work.  When
you get to work on consolidating set_X_sampler_views, I believe there
is also ilo_set_sampler_views that you can hook up to :P



 -Brian




-- 
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-16 Thread Roland Scheidegger
Am 16.09.2013 18:18, schrieb Brian Paul:
 On 09/16/2013 08:56 AM, Roland Scheidegger wrote:
 Am 14.09.2013 18:24, schrieb Brian Paul:
 On 09/12/2013 09:06 PM, Chia-I Wu wrote:
 Hi Brian,

 On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:

 I just pushed a gallium-bind-sampler-states branch to my git repo at
 git://people.freedesktop.org/~brianp/mesa

 It replaces the four
 pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
 functions with a single bind_sampler_states() function:

void (*bind_sampler_states)(struct pipe_context *,
unsigned shader, unsigned start_slot,
unsigned num_samplers, void
 **samplers);

 At this point start_slot is always zero (at least for non-compute
 shaders).
 And as the updated gallium docs explain, at some point calls to
 bind_sampler_states() will be used to updated sub-ranges, but that
 never
 happens currently.

 I've updated all the drivers, state trackers, utils, etc.

 I've tested the svga, llvmpipe and softpipe drivers.  'make check'
 and a
 texture subset of piglit pass w/out regressions.  I'd appreciate it
 if other
 driver developers would test their favorite driver.
 For ilo, the new code does not follow the doc and unbinds samplers not
 in range.

 I think that's OK.  The CSO module (used by the state tracker) currently
 always calls pipe_context::bind_sampler_states() with start=0 and count
 such that it sets/replaces all samplers, never a sub-range.  That
 could/should change in the future.

 See single_sampler_done() in cso_context.c.

 Not all state trackers use cso though.
 
 AFAICT, all src/gallium/state_trackers/ use CSO, except for clover (and
 I did miss a function call change there).  I didn't look at any
 out-of-tree state trackers.
Yes, I was thinking about out-of-tree state trackers.

 
 
 While this can be fixed later, I
 think I'd prefer if the docs would state this isn't really meant to be
 that way and should be changed in the future in cso and certainly fixed
 in the drivers.
 
 As I mentioned earlier, I updated the docs to explain how things current
 work and how it should work in the future:
 
 
 * :ref:`Sampler`: Texture sampler states are bound separately for fragment,
   vertex, geometry and compute shaders with the ``bind_sampler_states``
   function.  The ``start`` and ``num_samplers`` parameters indicate a range
   of samplers to change.  NOTE: at this time, start is always zero and
   the CSO module will always replace all samplers at once (no sub-ranges).
   This may change in the future.
 
 
 Is that what you mean?

Yes, I don't particularly like the wording - it is not obvious that it
is really a bug if a driver can't cope with start != 0, and the start
is always zero etc. is just a guarantee from cso (and some other state
trackers) for now but not required already from the interface. No biggie
though.

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


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-16 Thread Brian Paul

On 09/16/2013 08:56 AM, Roland Scheidegger wrote:

Am 14.09.2013 18:24, schrieb Brian Paul:

On 09/12/2013 09:06 PM, Chia-I Wu wrote:

Hi Brian,

On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:


I just pushed a gallium-bind-sampler-states branch to my git repo at
git://people.freedesktop.org/~brianp/mesa

It replaces the four
pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
functions with a single bind_sampler_states() function:

   void (*bind_sampler_states)(struct pipe_context *,
   unsigned shader, unsigned start_slot,
   unsigned num_samplers, void **samplers);

At this point start_slot is always zero (at least for non-compute
shaders).
And as the updated gallium docs explain, at some point calls to
bind_sampler_states() will be used to updated sub-ranges, but that never
happens currently.

I've updated all the drivers, state trackers, utils, etc.

I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
texture subset of piglit pass w/out regressions.  I'd appreciate it
if other
driver developers would test their favorite driver.

For ilo, the new code does not follow the doc and unbinds samplers not
in range.


I think that's OK.  The CSO module (used by the state tracker) currently
always calls pipe_context::bind_sampler_states() with start=0 and count
such that it sets/replaces all samplers, never a sub-range.  That
could/should change in the future.

See single_sampler_done() in cso_context.c.


Not all state trackers use cso though.


AFAICT, all src/gallium/state_trackers/ use CSO, except for clover (and 
I did miss a function call change there).  I didn't look at any 
out-of-tree state trackers.



 While this can be fixed later, I
 think I'd prefer if the docs would state this isn't really meant to be
 that way and should be changed in the future in cso and certainly fixed
 in the drivers.

As I mentioned earlier, I updated the docs to explain how things current 
work and how it should work in the future:



* :ref:`Sampler`: Texture sampler states are bound separately for fragment,
  vertex, geometry and compute shaders with the ``bind_sampler_states``
  function.  The ``start`` and ``num_samplers`` parameters indicate a range
  of samplers to change.  NOTE: at this time, start is always zero and
  the CSO module will always replace all samplers at once (no sub-ranges).
  This may change in the future.


Is that what you mean?

-Brian

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


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-15 Thread Chia-I Wu
On Sun, Sep 15, 2013 at 12:24 AM, Brian Paul bri...@vmware.com wrote:
 On 09/12/2013 09:06 PM, Chia-I Wu wrote:

 Hi Brian,

 On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:


 I just pushed a gallium-bind-sampler-states branch to my git repo at
 git://people.freedesktop.org/~brianp/mesa

 It replaces the four
 pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
 functions with a single bind_sampler_states() function:

   void (*bind_sampler_states)(struct pipe_context *,
   unsigned shader, unsigned start_slot,
   unsigned num_samplers, void **samplers);

 At this point start_slot is always zero (at least for non-compute
 shaders).
 And as the updated gallium docs explain, at some point calls to
 bind_sampler_states() will be used to updated sub-ranges, but that never
 happens currently.

 I've updated all the drivers, state trackers, utils, etc.

 I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
 texture subset of piglit pass w/out regressions.  I'd appreciate it if
 other
 driver developers would test their favorite driver.

 For ilo, the new code does not follow the doc and unbinds samplers not in
 range.


 I think that's OK.  The CSO module (used by the state tracker) currently
 always calls pipe_context::bind_sampler_states() with start=0 and count such
 that it sets/replaces all samplers, never a sub-range.  That could/should
 change in the future.

 See single_sampler_done() in cso_context.c.



 Is it fine if I implement the new bind_sampler_states as a helper
 function on master branch, so that you hook it up to
 pipe_context::bind_sampler_states in your branch and remove the old
 ones?


 I'm not quite sure that I understand what you mean.  Can you elaborate?
There is already ilo_bind_sampler_states that does what
pipe_context::bind_sampler_states expects, except that the function
returns a bool.  I can make it return void so that, in your branch,
you can initialize pipe_context::bind_sampler_states to it instead of
adding ilo_bind_sampler_states2.


 -Brian




-- 
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-14 Thread Brian Paul

On 09/12/2013 09:06 PM, Chia-I Wu wrote:

Hi Brian,

On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:


I just pushed a gallium-bind-sampler-states branch to my git repo at
git://people.freedesktop.org/~brianp/mesa

It replaces the four
pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
functions with a single bind_sampler_states() function:

  void (*bind_sampler_states)(struct pipe_context *,
  unsigned shader, unsigned start_slot,
  unsigned num_samplers, void **samplers);

At this point start_slot is always zero (at least for non-compute shaders).
And as the updated gallium docs explain, at some point calls to
bind_sampler_states() will be used to updated sub-ranges, but that never
happens currently.

I've updated all the drivers, state trackers, utils, etc.

I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
texture subset of piglit pass w/out regressions.  I'd appreciate it if other
driver developers would test their favorite driver.

For ilo, the new code does not follow the doc and unbinds samplers not in range.


I think that's OK.  The CSO module (used by the state tracker) currently 
always calls pipe_context::bind_sampler_states() with start=0 and count 
such that it sets/replaces all samplers, never a sub-range.  That 
could/should change in the future.


See single_sampler_done() in cso_context.c.



Is it fine if I implement the new bind_sampler_states as a helper
function on master branch, so that you hook it up to
pipe_context::bind_sampler_states in your branch and remove the old
ones?


I'm not quite sure that I understand what you mean.  Can you elaborate?

-Brian

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


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-13 Thread Roland Scheidegger
Am 13.09.2013 02:46, schrieb Brian Paul:
 
 I just pushed a gallium-bind-sampler-states branch to my git repo at
 git://people.freedesktop.org/~brianp/mesa
 
 It replaces the four
 pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
 functions with a single bind_sampler_states() function:
 
  void (*bind_sampler_states)(struct pipe_context *,
  unsigned shader, unsigned start_slot,
  unsigned num_samplers, void **samplers);
 
 At this point start_slot is always zero (at least for non-compute
 shaders).  And as the updated gallium docs explain, at some point calls
 to bind_sampler_states() will be used to updated sub-ranges, but that
 never happens currently.
 
 I've updated all the drivers, state trackers, utils, etc.
 
 I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
 texture subset of piglit pass w/out regressions.  I'd appreciate it if
 other driver developers would test their favorite driver.
 
 
 Next, I'd like to consolidate the
 set_vertex/geometry/fragment/compute_sampler_views() functions with a
 single function.  But I have no idea when I'll get around to that.
 

This looks good to me. And I'm very much in favor of doing the same for
sampler_views() (even more so because for d3d10 we need to have 128 of
them not just 16 and I suspect they get changed more often too).

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


[Mesa-dev] gallium-bind-sampler-states branch

2013-09-12 Thread Brian Paul


I just pushed a gallium-bind-sampler-states branch to my git repo at 
git://people.freedesktop.org/~brianp/mesa


It replaces the four 
pipe_context::bind_fragment/vertex/geometry/compute_sampler_states() 
functions with a single bind_sampler_states() function:


 void (*bind_sampler_states)(struct pipe_context *,
 unsigned shader, unsigned start_slot,
 unsigned num_samplers, void **samplers);

At this point start_slot is always zero (at least for non-compute 
shaders).  And as the updated gallium docs explain, at some point calls 
to bind_sampler_states() will be used to updated sub-ranges, but that 
never happens currently.


I've updated all the drivers, state trackers, utils, etc.

I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a 
texture subset of piglit pass w/out regressions.  I'd appreciate it if 
other driver developers would test their favorite driver.



Next, I'd like to consolidate the 
set_vertex/geometry/fragment/compute_sampler_views() functions with a 
single function.  But I have no idea when I'll get around to that.


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


Re: [Mesa-dev] gallium-bind-sampler-states branch

2013-09-12 Thread Chia-I Wu
Hi Brian,

On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul bri...@vmware.com wrote:

 I just pushed a gallium-bind-sampler-states branch to my git repo at
 git://people.freedesktop.org/~brianp/mesa

 It replaces the four
 pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
 functions with a single bind_sampler_states() function:

  void (*bind_sampler_states)(struct pipe_context *,
  unsigned shader, unsigned start_slot,
  unsigned num_samplers, void **samplers);

 At this point start_slot is always zero (at least for non-compute shaders).
 And as the updated gallium docs explain, at some point calls to
 bind_sampler_states() will be used to updated sub-ranges, but that never
 happens currently.

 I've updated all the drivers, state trackers, utils, etc.

 I've tested the svga, llvmpipe and softpipe drivers.  'make check' and a
 texture subset of piglit pass w/out regressions.  I'd appreciate it if other
 driver developers would test their favorite driver.
For ilo, the new code does not follow the doc and unbinds samplers not in range.

Is it fine if I implement the new bind_sampler_states as a helper
function on master branch, so that you hook it up to
pipe_context::bind_sampler_states in your branch and remove the old
ones?


 Next, I'd like to consolidate the
 set_vertex/geometry/fragment/compute_sampler_views() functions with a single
 function.  But I have no idea when I'll get around to that.

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



-- 
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev