[Mesa-dev] [PATCH] gallium/docs: clarify set_sampler_views

[Rob Clark]

On Fri, Mar 15, 2019 at 8:21 AM Axel Davy <davyaxel0 at gmail.com> wrote:
>
> On 15/03/2019 13:12, Rob Clark wrote:
> > On Fri, Mar 15, 2019 at 3:49 AM Axel Davy <davyaxel0 at gmail.com> wrote:
>
> >> To my knowledge, the semantic of set_sampler_views was changed two years
> >> ago, and that caused some issues:
> >> https://github.com/iXit/Mesa-3D/issues/308
> >>
> >> Making drivers consistent (and complete the doc) is a good initiative.
> >>
> >> My understanding though is that some drivers may want different
> >> behaviors in order to reduce overhead. If I understood correctly,
> >> radeonsi prefers to minimize view changes to reduce cpu overhead (and
> >> thus prefers not to unbind things outside of the set_sampler_views call
> >> range),
> >> while nouveau prefers to have only what's needed by the call bound for
> >> better gpu performance. I think both are happy with the current code,
> >> because the latter preferred behavior can be emulated in the driver,
> >> while it would be more difficult for the former.
> > Hmm, but this is at odds with Roland's statement that sampler views
> > outside the specified range are unmodified.
> >
> > If we defined the API such that slots >= start+nr are unbound, then a
> > driver could choose not to touch them to minimize CPU overhead.  But
> > if defined that they are unmodified then the driver cannot silently
> > discard them.
> >
> > BR,
> > -R
>
>
> The sampler views outside the specified range are unmodified.
> This is compatible with what I said. Nouveau determines the minimum
> number of samplers actually needed
> by the current shader, and internally unbinds those above (and rebinds
> if needed by new shader).
>

Mmmm, unless there is something subtle that I'm overlooking, I'm not
sure that I agree.. nvc0_stage_set_sampler_views() unref's the
remaining sampler views.  The end result is what you describe, but
only because of an implementation detail of how mesa/st works (ie. it
rebinds them if needed for a new shader).

But as Ilia mentioned, I see the use in differentiating between active
and bound.  Thas might be a good argument to switch back to my initial
docs patch which defined slots >= nr+count as unbound.

(otoh, nouveau could just track a bitmask of samplers used by the
shader internally, I guess?)

BR,
-R