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

[Jason Ekstrand]

On Tue, Oct 11, 2016 at 11:50 AM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:

> On Tue, Oct 11, 2016 at 09:46:41PM +0300, Pohjolainen, Topi wrote:
> > On Fri, Oct 07, 2016 at 09:41:08PM -0700, Jason Ekstrand wrote:
> > > Signed-off-by: Jason Ekstrand <ja...@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;
> >
> > Similarly to patch four, here you are thinking of having additionally:
> >
> >             params.dst.view = params.depth.view;
> > > +
> > > +         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;
> >
> > As well as here:
> >
> >             params.dst.view = params.depth.view;
> >
> > Destination render surface itself is a null surface, right? Why do we
> need
> > to specify the view?
>
> Is this because when performing layrered depth clear even null render
> surface
> has to have corresponding amount of layers set?
>

Yes, that's the reasoning.  I'm not sure how accurate the null render
target is supposed to be but I figure more accurate is better than less.


> >
> > > +
> > > +         if (params.depth.view.array_len < params.num_layers)
> > > +            params.num_layers = params.depth.view.array_len;
> > > +      }
> > > +
> > > +      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
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev