[Mesa-dev] [PATCH] nv50, nvc0: Fix gallium nine regression regarding sampler bindings

Karol Herbst kherbst at redhat.com
Sun Nov 25 22:43:24 UTC 2018


On Sun, Nov 25, 2018 at 5:48 PM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>
> Would it make sense to instead keep track of a samplers_bound[] like
> we do for textures? It's only ever used in a context of
>
> for (i = 0; i < num_samplers; i++)
>   if (samplers[i])
>      do stuff
>
> So having a mask would actually optimize that, and make this logic much simpler.

was trying to come up with something more meaningful, but we usually
loop over all samplers anyway, so we either loop until we had
num_samplers or we check the mask up to PIPE_NUM_SAMPLERS bits... I
doubt that we would win much at all. Anyway, in my local version of
that patch I dropped those "num_samplers[s] = 0;" assignments as they
weren't making much sense to begin with. We might be able to decrease
num_samplers if we remove the last one, but I don't want to change
much of the behavior here anyway. We could improve that code in future
patches though.

> On Sun, Nov 25, 2018 at 10:47 AM Karol Herbst <kherbst at redhat.com> wrote:
> >
> > The new approach is that samplers don't get unbound even if they won't be used
> > in a draw and we should just leave them be as well.
> >
> > Fixes a regression in multiple windows games using gallium nine and nouveau.
> >
> > v2: adjust num_samplers to keep track of the highest sampler bound
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106577
> > Fixes: 4d6fab245eec3880e2a59424a579851f44857ce8
> >        "cso: don't track the number of sampler states bound"
> > Signed-off-by: Karol Herbst <kherbst at redhat.com>
> > ---
> >  src/gallium/drivers/nouveau/nv50/nv50_state.c | 19 ++++++++++---------
> >  src/gallium/drivers/nouveau/nvc0/nvc0_state.c | 16 ++++++++++------
> >  2 files changed, 20 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c b/src/gallium/drivers/nouveau/nv50/nv50_state.c
> > index fb4a259ce16..286509d5fcd 100644
> > --- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
> > +++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
> > @@ -589,6 +589,7 @@ nv50_sampler_state_delete(struct pipe_context *pipe, void *hwcso)
> >        for (i = 0; i < nv50_context(pipe)->num_samplers[s]; ++i)
> >           if (nv50_context(pipe)->samplers[s][i] == hwcso)
> >              nv50_context(pipe)->samplers[s][i] = NULL;
> > +      nv50_context(pipe)->num_samplers[s] = 0;
> >     }
> >
> >     nv50_screen_tsc_free(nv50_context(pipe)->screen, nv50_tsc_entry(hwcso));
> > @@ -600,26 +601,26 @@ static inline void
> >  nv50_stage_sampler_states_bind(struct nv50_context *nv50, int s,
> >                                 unsigned nr, void **hwcso)
> >  {
> > +   assert(nr <= PIPE_MAX_SAMPLERS);
> > +   unsigned highest_found = 0;
> >     unsigned i;
> >
> > -   assert(nr <= PIPE_MAX_SAMPLERS);
> >     for (i = 0; i < nr; ++i) {
> >        struct nv50_tsc_entry *old = nv50->samplers[s][i];
> >
> > +      if (hwcso[i])
> > +         highest_found = i;
> > +
> >        nv50->samplers[s][i] = nv50_tsc_entry(hwcso[i]);
> >        if (old)
> >           nv50_screen_tsc_unlock(nv50->screen, old);
> >     }
> > -   assert(nv50->num_samplers[s] <= PIPE_MAX_SAMPLERS);
> > -   for (; i < nv50->num_samplers[s]; ++i) {
> > -      if (nv50->samplers[s][i]) {
> > -         nv50_screen_tsc_unlock(nv50->screen, nv50->samplers[s][i]);
> > -         nv50->samplers[s][i] = NULL;
> > -      }
> > +   for (;i < nv50->num_samplers[s]; ++i) {
> > +      if (nv50->samplers[s][i])
> > +         highest_found = i;
> >     }
> >
> > -   nv50->num_samplers[s] = nr;
> > -
> > +   nv50->num_samplers[s] = highest_found + 1;
> >     nv50->dirty_3d |= NV50_NEW_3D_SAMPLERS;
> >  }
> >
> > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> > index f2393cb27b5..756b828238e 100644
> > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> > @@ -449,10 +449,12 @@ nvc0_sampler_state_delete(struct pipe_context *pipe, void *hwcso)
> >  {
> >     unsigned s, i;
> >
> > -   for (s = 0; s < 6; ++s)
> > +   for (s = 0; s < 6; ++s) {
> >        for (i = 0; i < nvc0_context(pipe)->num_samplers[s]; ++i)
> >           if (nvc0_context(pipe)->samplers[s][i] == hwcso)
> >              nvc0_context(pipe)->samplers[s][i] = NULL;
> > +      nvc0_context(pipe)->num_samplers[s] = 0;
> > +   }
> >
> >     nvc0_screen_tsc_free(nvc0_context(pipe)->screen, nv50_tsc_entry(hwcso));
> >
> > @@ -464,11 +466,15 @@ nvc0_stage_sampler_states_bind(struct nvc0_context *nvc0,
> >                                 unsigned s,
> >                                 unsigned nr, void **hwcso)
> >  {
> > +   unsigned highest_found = 0;
> >     unsigned i;
> >
> >     for (i = 0; i < nr; ++i) {
> >        struct nv50_tsc_entry *old = nvc0->samplers[s][i];
> >
> > +      if (hwcso[i])
> > +         highest_found = i;
> > +
> >        if (hwcso[i] == old)
> >           continue;
> >        nvc0->samplers_dirty[s] |= 1 << i;
> > @@ -478,13 +484,11 @@ nvc0_stage_sampler_states_bind(struct nvc0_context *nvc0,
> >           nvc0_screen_tsc_unlock(nvc0->screen, old);
> >     }
> >     for (; i < nvc0->num_samplers[s]; ++i) {
> > -      if (nvc0->samplers[s][i]) {
> > -         nvc0_screen_tsc_unlock(nvc0->screen, nvc0->samplers[s][i]);
> > -         nvc0->samplers[s][i] = NULL;
> > -      }
> > +      if (nvc0->samplers[s][i])
> > +         highest_found = i;
> >     }
> >
> > -   nvc0->num_samplers[s] = nr;
> > +   nvc0->num_samplers[s] = highest_found + 1;
> >  }
> >
> >  static void
> > --
> > 2.19.1
> >


More information about the mesa-dev mailing list