[Mesa-dev] gallium-bind-sampler-states branch
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
Hi Brian, On Fri, Sep 13, 2013 at 8:46 AM, 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. 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
Re: [Mesa-dev] gallium-bind-sampler-states branch
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
Re: [Mesa-dev] gallium-bind-sampler-states branch
On 09/12/2013 09:06 PM, Chia-I Wu wrote: Hi Brian, On Fri, Sep 13, 2013 at 8:46 AM, 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. 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
On Sun, Sep 15, 2013 at 12:24 AM, Brian Paul 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 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
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 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
On 09/15/2013 09:31 AM, Chia-I Wu wrote: On Sun, Sep 15, 2013 at 12:24 AM, Brian Paul 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 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
On Tue, Sep 17, 2013 at 12:09 AM, Brian Paul wrote: > On 09/15/2013 09:31 AM, Chia-I Wu wrote: >> >> On Sun, Sep 15, 2013 at 12:24 AM, Brian Paul 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 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
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 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
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 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
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