[Mesa-dev] [PATCH 15/28] i965/blorp: Add a param array to prog_data

[Pohjolainen, Topi]

On Wed, May 11, 2016 at 07:46:33AM -0700, Jason Ekstrand wrote:
>    On May 11, 2016 7:45 AM, "Jason Ekstrand" <[1]jason at jlekstrand.net>
>    wrote:
>    >
>    >
>    > On May 10, 2016 11:53 PM, "Pohjolainen, Topi"
>    <[2]topi.pohjolainen at intel.com> wrote:
>    > >
>    > > On Tue, May 10, 2016 at 04:16:35PM -0700, Jason Ekstrand wrote:
>    > > > This array allows the push constants to be re-arranged on
>    upload.  The
>    > > > actual arrangement will, eventually, come from the back-end
>    compiler.
>    > > > ---
>    > > >  src/mesa/drivers/dri/i965/brw_blorp.c  |  4 ++++
>    > > >  src/mesa/drivers/dri/i965/brw_blorp.h  |  6 ++++++
>    > > >  src/mesa/drivers/dri/i965/gen6_blorp.c | 12 +++++++-----
>    > > >  3 files changed, 17 insertions(+), 5 deletions(-)
>    > > >
>    > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>    b/src/mesa/drivers/dri/i965/brw_blorp.c
>    > > > index 4bbe45f..1379804 100644
>    > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>    > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>    > > > @@ -139,6 +139,10 @@ brw_blorp_prog_data_init(struct
>    brw_blorp_prog_data *prog_data)
>    > > >  {
>    > > >     prog_data->first_curbe_grf = 0;
>    > > >     prog_data->persample_msaa_dispatch = false;
>    > > > +
>    > > > +   prog_data->nr_params = BRW_BLORP_NUM_PUSH_CONSTANT_DWORDS;
>    > > > +   for (unsigned i = 0; i < BRW_BLORP_NUM_PUSH_CONSTANT_DWORDS;
>    i++)
>    > > > +      prog_data->param[i] = i;
>    > > >  }
>    > > >
>    > > >
>    > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
>    b/src/mesa/drivers/dri/i965/brw_blorp.h
>    > > > index 4a0e46e..c2f33a1 100644
>    > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.h
>    > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
>    > > > @@ -199,6 +199,9 @@ struct brw_blorp_wm_push_constants
>    > > >     uint32_t pad[5];
>    > > >  };
>    > > >
>    > > > +#define BRW_BLORP_NUM_PUSH_CONSTANT_DWORDS \
>    > > > +   (sizeof(struct brw_blorp_wm_push_constants) / 4)
>    > > > +
>    > > >  /* Every 32 bytes of push constant data constitutes one GEN
>    register. */
>    > > >  static const unsigned int BRW_BLORP_NUM_PUSH_CONST_REGS =
>    > > >     sizeof(struct brw_blorp_wm_push_constants) / 32;
>    > > > @@ -212,6 +215,9 @@ struct brw_blorp_prog_data
>    > > >      * than one sample per pixel.
>    > > >      */
>    > > >     bool persample_msaa_dispatch;
>    > > > +
>    > > > +   uint8_t nr_params;
>    > > > +   uint8_t param[BRW_BLORP_NUM_PUSH_CONSTANT_DWORDS];
>    > >
>    > > Do I read this correctly: this corresponds to push_contant_loc in
>    the scalar
>    > > backend?
>    >
>    > Sort-of.  The mapping actually goes in the other direction:  From
>    location to uniform number.
> 
>    Really, it's just a simplified version of peog_data->param.

Right. Could we add some description, "param" doesn't tell much, does it?
For example,

     /* Compiler will re-arrange push constants and store the upload order
      * here. Given an index 'i' in the final upload buffer, param[i] gives
      * the index in the uniform store. In other words, the value to be
      * uploaded can be found in brw_blorp_params::wm_push_consts[param[i]].
      */
     uint8_t param[BRW_BLORP_NUM_PUSH_CONSTANT_DWORDS];

> 
>    > > >  };
>    > > >
>    > > >  void brw_blorp_prog_data_init(struct brw_blorp_prog_data
>    *prog_data);
>    > > > diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.c
>    b/src/mesa/drivers/dri/i965/gen6_blorp.c
>    > > > index 1955811..950e2b9 100644
>    > > > --- a/src/mesa/drivers/dri/i965/gen6_blorp.c
>    > > > +++ b/src/mesa/drivers/dri/i965/gen6_blorp.c
>    > > > @@ -308,11 +308,13 @@ gen6_blorp_emit_wm_constants(struct
>    brw_context *brw,
>    > > >  {
>    > > >     uint32_t wm_push_const_offset;
>    > > >
>    > > > -   void *constants = brw_state_batch(brw,
>    AUB_TRACE_WM_CONSTANTS,
>    > > > -
>    sizeof(params->wm_push_consts),
>    > > > -                                     32, &wm_push_const_offset);
>    > > > -   memcpy(constants, &params->wm_push_consts,
>    > > > -          sizeof(params->wm_push_consts));
>    > > > +   uint32_t *constants = brw_state_batch(brw,
>    AUB_TRACE_WM_CONSTANTS,
>    > > > +
>    sizeof(params->wm_push_consts),
>    > > > +                                         32,
>    &wm_push_const_offset);
>    > > > +
>    > > > +   uint32_t *push_consts = (uint32_t *)&params->wm_push_consts;
>    > > > +   for (unsigned i = 0; i < params->wm_prog_data->nr_params;
>    i++)
>    > > > +      constants[i] =
>    push_consts[params->wm_prog_data->param[i]];
>    > > >
>    > > >     return wm_push_const_offset;
>    > > >  }
>    > > > --
>    > > > 2.5.0.400.gff86faf
>    > > >
>    > > > _______________________________________________
>    > > > mesa-dev mailing list
>    > > > [3]mesa-dev at lists.freedesktop.org
>    > > > [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>    1. mailto:jason at jlekstrand.net
>    2. mailto:topi.pohjolainen at intel.com
>    3. mailto:mesa-dev at lists.freedesktop.org
>    4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev