[Mesa-dev] [PATCH 10/22] intel/blorp: Add an entrypoint for clearing depth and stencil

[Pohjolainen, Topi]

On Sun, Oct 09, 2016 at 10:52:52PM -0700, Jason Ekstrand wrote:
>    On Oct 9, 2016 10:48 PM, "Pohjolainen, Topi"
>    <[1]topi.pohjolainen at gmail.com> wrote:
>    >
>    > On Fri, Oct 07, 2016 at 09:41:08PM -0700, Jason Ekstrand wrote:
>    > > Signed-off-by: Jason Ekstrand <[2]jason at jlekstrand.net>
>    > > ---
>    > >  src/intel/blorp/blorp.h       | 10 ++++++++
>    > >  src/intel/blorp/blorp_clear.c | 58
>    +++++++++++++++++++++++++++++++++++++++++++
>    > >  2 files changed, 68 insertions(+)
>    > >
>    > > diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
>    > > index 480f1bb..263d194 100644
>    > > --- a/src/intel/blorp/blorp.h
>    > > +++ b/src/intel/blorp/blorp.h
>    > > @@ -134,6 +134,16 @@ blorp_clear(struct blorp_batch *batch,
>    > >              const bool color_write_disable[4]);
>    > >
>    > >  void
>    > > +blorp_clear_depth_stencil(struct blorp_batch *batch,
>    > > +                          const struct blorp_surf *depth,
>    > > +                          const struct blorp_surf *stencil,
>    > > +                          uint32_t level, uint32_t start_layer,
>    > > +                          uint32_t num_layers,
>    > > +                          uint32_t x0, uint32_t y0, uint32_t x1,
>    uint32_t y1,
>    > > +                          bool clear_depth, float depth_value,
>    > > +                          bool clear_stencil, uint8_t
>    stencil_value);
>    > > +
>    > > +void
>    > >  blorp_ccs_resolve(struct blorp_batch *batch,
>    > >                    struct blorp_surf *surf, enum isl_format
>    format);
>    > >
>    > > diff --git a/src/intel/blorp/blorp_clear.c
>    b/src/intel/blorp/blorp_clear.c
>    > > index a8f29fd..1d6bf1c 100644
>    > > --- a/src/intel/blorp/blorp_clear.c
>    > > +++ b/src/intel/blorp/blorp_clear.c
>    > > @@ -306,6 +306,64 @@ blorp_clear(struct blorp_batch *batch,
>    > >  }
>    > >
>    > >  void
>    > > +blorp_clear_depth_stencil(struct blorp_batch *batch,
>    > > +                          const struct blorp_surf *depth,
>    > > +                          const struct blorp_surf *stencil,
>    > > +                          uint32_t level, uint32_t start_layer,
>    > > +                          uint32_t num_layers,
>    > > +                          uint32_t x0, uint32_t y0, uint32_t x1,
>    uint32_t y1,
>    > > +                          bool clear_depth, float depth_value,
>    > > +                          bool clear_stencil, uint8_t
>    stencil_value)
>    > > +{
>    > > +   struct blorp_params params;
>    > > +   blorp_params_init(&params);
>    > > +
>    > > +   params.x0 = x0;
>    > > +   params.y0 = y0;
>    > > +   params.x1 = x1;
>    > > +   params.y1 = y1;
>    > > +
>    > > +   while (num_layers > 0) {
>    > > +      params.num_layers = num_layers;
>    > > +
>    > > +      if (clear_stencil) {
>    > > +         brw_blorp_surface_info_init(batch->blorp,
>    &params.stencil, stencil,
>    > > +                                     level, start_layer,
>    > > +                                     ISL_FORMAT_UNSUPPORTED,
>    true);
>    > > +         params.stencil_ref = stencil_value;
>    > > +
>    > > +         params.dst.surf.samples = params.stencil.surf.samples;
>    > > +         params.dst.surf.logical_level0_px =
>    > > +            params.stencil.surf.logical_level0_px;
>    > > +
>    > > +         if (params.stencil.view.array_len < params.num_layers)
>    > > +            params.num_layers = params.stencil.view.array_len;
>    > > +      }
>    > > +
>    > > +      if (clear_depth) {
>    > > +         brw_blorp_surface_info_init(batch->blorp, &params.depth,
>    depth,
>    > > +                                     level, start_layer,
>    > > +                                     ISL_FORMAT_UNSUPPORTED,
>    true);
>    > > +         params.z = depth_value;
>    > > +         params.depth_format =
>    > > +            isl_format_get_depth_format(depth->surf->format,
>    false);
>    > > +
>    > > +         params.dst.surf.samples = params.depth.surf.samples;
>    > > +         params.dst.surf.logical_level0_px =
>    > > +            params.depth.surf.logical_level0_px;
>    > > +
>    > > +         if (params.depth.view.array_len < params.num_layers)
>    > > +            params.num_layers = params.depth.view.array_len;
>    >
>    > Stencil already does the same, could we add an assert here:
>    >
>    >                assert(!clear_stencil ||
>    >                       params.depth.view.array_len ==
>    >                       params.stencil.view.array_len)
> 
>    Sure.
> 
>    >
>    > Moreover, I thought that start_layer + num_layers <= view.array_len
>    should
>    > always apply. You seem to explicitly prepare for that not to hold.
>    Could you
>    > give an example?
> 
>    I had to do this recently with the other clears as well.  Sandy Bridge
>    has a max of 512 layers for rendering.  In the surface_info_init
>    function, we clamp as needed and this ensures that 16k-slice textures
>    get 32 draw calls.

Ah, right, that makes sense. How would you feel we added that as comment?

     /* Sandy Bridge has a max of 512 layers for rendering. In the
      * surface_info_init function, we clamp as needed and this ensures that
      * 16k-slice textures get 32 draw calls.
      */
     assert(start_layer + num_layers <= view.array_len || gen == 6);

Otherwise:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

> 
>    > > +      }
>    > > +
>    > > +      batch->blorp->exec(batch, &params);
>    > > +
>    > > +      start_layer += params.num_layers;
>    > > +      num_layers -= params.num_layers;
>    > > +   }
>    > > +}
>    > > +
>    > > +void
>    > >  blorp_ccs_resolve(struct blorp_batch *batch,
>    > >                    struct blorp_surf *surf, enum isl_format format)
>    > >  {
>    > > --
>    > > 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:topi.pohjolainen at gmail.com
>    2. mailto:jason at jlekstrand.net
>    3. mailto:mesa-dev at lists.freedesktop.org
>    4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev