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

[Jason Ekstrand]

On Wed, May 11, 2016 at 8:39 AM, Pohjolainen, Topi <
topi.pohjolainen at intel.com> wrote:

> 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]].
>

I added basically that exact comment.  Thanks!


>       */
>      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;
>

I also made the const change you suggested here.


> >    > > > +   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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160511/1fa85eb7/attachment.html>