[Mesa-dev] [PATCH 2/3] mesa: build up a mask of active shader stages in a pipeline

Timothy Arceri t_arceri at yahoo.com.au
Mon Dec 7 21:28:10 PST 2015


On Tue, 2015-12-08 at 07:09 +0200, Tapani Pälli wrote:
> On 12/08/2015 06:55 AM, Timothy Arceri wrote:
> > On Tue, 2015-12-08 at 15:21 +1100, Timothy Arceri wrote:
> > > On Tue, 2015-12-08 at 10:55 +1100, Timothy Arceri wrote:
> > > > On Mon, 2015-12-07 at 11:29 +0200, Tapani Pälli wrote:
> > > > > This will be used for validating SSO pipeline where all
> > > > > active
> > > > > stages
> > > > > in linked programs should be in use when rendering.
> > > > > 
> > > > > Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> > > > > ---
> > > > >   src/mesa/main/mtypes.h      |  2 ++
> > > > >   src/mesa/main/pipelineobj.c | 39
> > > > > +++++++++++++++++++++++++++++++++++++++
> > > > >   2 files changed, 41 insertions(+)
> > > > > 
> > > > > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> > > > > index fa7ead0..d5a22c9 100644
> > > > > --- a/src/mesa/main/mtypes.h
> > > > > +++ b/src/mesa/main/mtypes.h
> > > > > @@ -2824,6 +2824,8 @@ struct gl_pipeline_object
> > > > >      GLboolean Validated;                 /**< Pipeline
> > > > > Validation
> > > > > status */
> > > > >   
> > > > >      GLchar *InfoLog;
> > > > > +
> > > > > +   uint8_t ActiveStages;                /**< Stages used,
> > > > > (glUseProgramStages) */
> > > > >   };
> > > > >   
> > > > >   /**
> > > > > diff --git a/src/mesa/main/pipelineobj.c
> > > > > b/src/mesa/main/pipelineobj.c
> > > > > index 6710d0d..c510ee8 100644
> > > > > --- a/src/mesa/main/pipelineobj.c
> > > > > +++ b/src/mesa/main/pipelineobj.c
> > > > > @@ -218,6 +218,43 @@ _mesa_reference_pipeline_object_(struct
> > > > > gl_context *ctx,
> > > > >      }
> > > > >   }
> > > > >   
> > > > > +static GLenum
> > > > > +shader_bit_from_shader_stage(gl_shader_stage stage)
> > > > > +{
> > > > > +   switch (stage) {
> > > > > +   case MESA_SHADER_VERTEX:
> > > > > +      return GL_VERTEX_SHADER_BIT;
> > > > > +   case MESA_SHADER_FRAGMENT:
> > > > > +      return GL_FRAGMENT_SHADER_BIT;
> > > > > +   case MESA_SHADER_GEOMETRY:
> > > > > +      return GL_GEOMETRY_SHADER_BIT;
> > > > > +   case MESA_SHADER_TESS_CTRL:
> > > > > +      return GL_TESS_CONTROL_SHADER_BIT;
> > > > > +   case MESA_SHADER_TESS_EVAL:
> > > > > +      return GL_TESS_EVALUATION_SHADER_BIT;
> > > > > +   case MESA_SHADER_COMPUTE:
> > > > > +      return GL_COMPUTE_SHADER_BIT;
> > > > > +   default:
> > > > > +      unreachable("bad value in
> > > > > _mesa_shader_bit_from_shader_stage()");
> > > > > +   }
> > > > > +}
> > > > > +
> > > > > +static void
> > > > > +update_active_pipeline_stages(struct gl_pipeline_object
> > > > > *pipe,
> > > > > +                              struct gl_shader_program
> > > > > *shProg,
> > > > > +                              GLbitfield stages)
> > > > > +{
> > > > > +   unsigned i;
> > > > > +   for (i = 0; i < MESA_SHADER_STAGES; i++) {
> > > > > +      if ((stages & shader_bit_from_shader_stage(i)) != 0) {
> > > > > +         if (shProg && shProg->ActiveStages & (1 << i))
> > > > > +            pipe->ActiveStages |= (1 << i);
> > > > > +         else
> > > > > +            pipe->ActiveStages &= ~(1 << i);
> > > > > +      }
> > > > > +   }
> > > > I think you could do most of the validation at this point which
> > > > would
> > > > reduce the amount of work required during rendering. You also
> > > > have
> > > > forgotten about GL_ALL_SHADER_BITS.
> > > No you didn't I was just looking at to code for to long :P
> > > However
> > > unless I'm missing something I believe my other comments are
> > > still
> > > valid.
> > > 
> > > > For example could you do something like this?
> > > > 
> > > > Note: You would need to call this function after the
> > > >   _mesa_use_shader_program() calls.
> > > > 
> > > > unsigned i;
> > > > unsigned j;
> > > > 
> > > > for (i = 0; i < MESA_SHADER_STAGES; i++) {
> > > >     if (!pipe->CurrentProgram[i]) {
> > > >        struct gl_shader_program *shProg = pipe
> > > > ->CurrentProgram[i];
> > > >        for (j = 0; j < MESA_SHADER_STAGES; j++) {
> > > >           if (!shProg->_LinkedShaders[j])
> > > >           continue;
> > > > 
> > > >           /* Make sure that any shader stage is also used in
> > > > the
> > > > pipeline */
> > > >           if (shProg != pipe->CurrentProgram[j]) {
> > > >              pipe->ValidProgramUse = false;
> > > >              return;
> > > >           }
> > > >        }
> > > >     }
> > > > 
> > > >     pipe->ValidProgramUse = true;
> > > > }
> > Thinking about this some more you would also need to revalidate if
> > the
> > program has been relinked. I guess ValidProgramUse could be stored
> > for
> > each program object rather than just for the pipe, then you could
> > loop
> > over the stages in patch 3 and only do the full validation check
> > when
> >   ValidProgramUse is false (you would reset to false on a relink).
> 
> Relink is handled by having ActiveStages mask in gl_shader_program
> (1st 
> patch) that changes during linking and then this validation will
> fail.

Right but I don't think using a mask is going to be able to do
everything the validation requires. See my latest comment on patch 3.

> 
> There is also case where linking fails to program which is not set 
> active, I have a patch for this that uses the mask also here, did not
> include it yet in the series since it was not quite ready at that
> time:
> 
> http://cgit.freedesktop.org/~tpalli/mesa/commit/?h=fix_sso&id=cffd08b
> 93f56244ec7f7fbbca69d7f0d6d0a6243
> 
> > I'm also just about to add another comment to patch 3 as I think
> > your c
> > urrent patch will miss some use cases.
> > 
> > > > > +}
> > > > > +
> > > > >   /**
> > > > >    * Bound program to severals stages of the pipeline
> > > > >    */
> > > > > @@ -311,6 +348,8 @@ _mesa_UseProgramStages(GLuint pipeline,
> > > > > GLbitfield stages, GLuint program)
> > > > >         }
> > > > >      }
> > > > >   
> > > > > +   update_active_pipeline_stages(pipe, shProg, stages);
> > > > > +
> > > > >      /* Enable individual stages from the program as
> > > > > requested by
> > > > > the
> > > > >       * application.  If there is no shader for a requested
> > > > > stage
> > > > > in
> > > > > the
> > > > >       * program, _mesa_use_shader_program will enable fixed
> > > > > -function
> > > > > processing
> > > > _______________________________________________
> > > > mesa-dev mailing list
> > > > mesa-dev at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 


More information about the mesa-dev mailing list