On Wed, Mar 13, 2019 at 11:37 AM Roland Scheidegger <srol...@vmware.com> wrote: > > Am 12.03.19 um 22:48 schrieb Rob Clark: > > On Tue, Mar 12, 2019 at 1:59 PM Roland Scheidegger <srol...@vmware.com> > > wrote: > >> > >> Am 12.03.19 um 16:16 schrieb Rob Clark: > >>> This previously was not called out clearly, but based on a survey of the > >>> code, it seems the expected behavior is to release the reference to any > >>> sampler views beyond the new range being bound. > >> > >> That isn't really true. This was designed to work like d3d10, where > >> other views are unmodified. > >> The cso code will actually unset all views which previously were set and > >> are above the num_views in the call (this wouldn't be necessary if the > >> pipe function itself would work like this). > >> However, it will only do this for fragment textures, and pass through > >> the parameters unmodified otherwise. Which means behavior might not be > >> very consistent for the different stages... > > > > hmm, I did notice w/ deqp tests (which aren't so good at > > resetting/clearing state between tests), that I ended up w/ different > > # of sampler views bound (without changing freedreno to match the > > behavior of most of the other drivers).. I didn't really dig in that > > closely but it seemed like mesa/st wasn't clearing the additional > > previously bound textures. Maybe I overlooked something, but that > > seemed wrong. > > > > One way or another, I guess we should clarify and change the various > > drivers to have a common behavior because right now there two > > different behaviors and I guess it is at least confusing for new > > gallium driver writers (as it was for me and I've been at it for a > > while) > > Yes, I agree with that, the current state there doesn't help anyone.
I guess the best thing is that I should put together a patchset that documents the opposite behavior of what this patch suggests, followed by patches for the other drivers to change them to match the docs. BR, -R > Roland > > > > BR, > > -R > > > >> > >> > >>> > >>> I think radeonsi and freedreno were the only ones not doing this. Which > >>> could probably temporarily leak a bit of memory by holding on to the > >>> sampler view reference. > >> Not sure about other drivers, but llvmpipe will not do this neither. > >> > >> Roland > >> > >> > >>> > >>> Signed-off-by: Rob Clark <robdcl...@gmail.com> > >>> --- > >>> src/gallium/docs/source/context.rst | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/src/gallium/docs/source/context.rst > >>> b/src/gallium/docs/source/context.rst > >>> index f89d9e1005e..199d335f8f4 100644 > >>> --- a/src/gallium/docs/source/context.rst > >>> +++ b/src/gallium/docs/source/context.rst > >>> @@ -143,6 +143,9 @@ to the array index which is used for sampling. > >>> to a respective sampler view and releases a reference to the previous > >>> sampler view. > >>> > >>> + Previously bound samplers with index ``>= num_views`` are unbound > >>> rather > >>> + than unmodified. > >>> + > >>> * ``create_sampler_view`` creates a new sampler view. ``texture`` is > >>> associated > >>> with the sampler view which results in sampler view holding a reference > >>> to the texture. Format specified in template must be compatible > >>> > >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
