Re: [Mesa-dev] [PATCH 02/13] intel/blorp: Assert on gen7 fast-cleared subresource

[Nanley Chery]

On Tue, Jun 20, 2017 at 02:50:33PM -0700, Jason Ekstrand wrote:
> On Tue, Jun 20, 2017 at 2:43 PM, Nanley Chery  wrote:
> 
> > On Mon, Jun 19, 2017 at 04:06:34PM -0700, Jason Ekstrand wrote:
> > > On Tue, Jun 13, 2017 at 11:41 AM, Nanley Chery 
> > > wrote:
> > >
> > > > Signed-off-by: Nanley Chery 
> > > > ---
> > > >  src/intel/blorp/blorp_clear.c | 4 
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/src/intel/blorp/blorp_clear.c
> > b/src/intel/blorp/blorp_clear.c
> > > > index 3d5c41cc71..f43d05ad19 100644
> > > > --- a/src/intel/blorp/blorp_clear.c
> > > > +++ b/src/intel/blorp/blorp_clear.c
> > > > @@ -308,6 +308,10 @@ blorp_fast_clear(struct blorp_batch *batch,
> > > >   uint32_t level, uint32_t start_layer, uint32_t
> > > > num_layers,
> > > >   uint32_t x0, uint32_t y0, uint32_t x1, uint32_t y1)
> > > >  {
> > > > +   /* Multi-layer and multi-LOD fast clear is only available gen8+. */
> > > > +   assert(ISL_DEV_GEN(batch->blorp->isl_dev) >= 8 ||
> > > > +  (level == 0 && start_layer + num_layers == 1));
> > > >
> > >
> > > Alternatively, you could just add code to patch 1 which asserts that the
> > > level/layer is in-bounds for the auxiliary surface if one exists.
> > >
> > >
> >
> > I think your suggestion complements, but doesn't replace this patch.
> > Your suggestion validates the level and start_layer parameters. This
> > patch validates those parameters as well as the num_layers parameter.
> >
> 
> Right but maybe we should check num_layers against the aux surface rather
> than having a gen check?
> 
> 

Sure, I could do that. There's a similar assertion in blorp_ccs_resolve
that we can get rid of by following your suggestion. I'm gonna run it
through jenkins to make sure it's alright.

> > > > +
> > > > struct blorp_params params;
> > > > blorp_params_init();
> > > > params.num_layers = num_layers;
> > > > --
> > > > 2.13.1
> > > >
> > > > ___
> > > > 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

Re: [Mesa-dev] [PATCH 02/13] intel/blorp: Assert on gen7 fast-cleared subresource

[Jason Ekstrand]

On Tue, Jun 20, 2017 at 2:43 PM, Nanley Chery  wrote:

> On Mon, Jun 19, 2017 at 04:06:34PM -0700, Jason Ekstrand wrote:
> > On Tue, Jun 13, 2017 at 11:41 AM, Nanley Chery 
> > wrote:
> >
> > > Signed-off-by: Nanley Chery 
> > > ---
> > >  src/intel/blorp/blorp_clear.c | 4 
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/src/intel/blorp/blorp_clear.c
> b/src/intel/blorp/blorp_clear.c
> > > index 3d5c41cc71..f43d05ad19 100644
> > > --- a/src/intel/blorp/blorp_clear.c
> > > +++ b/src/intel/blorp/blorp_clear.c
> > > @@ -308,6 +308,10 @@ blorp_fast_clear(struct blorp_batch *batch,
> > >   uint32_t level, uint32_t start_layer, uint32_t
> > > num_layers,
> > >   uint32_t x0, uint32_t y0, uint32_t x1, uint32_t y1)
> > >  {
> > > +   /* Multi-layer and multi-LOD fast clear is only available gen8+. */
> > > +   assert(ISL_DEV_GEN(batch->blorp->isl_dev) >= 8 ||
> > > +  (level == 0 && start_layer + num_layers == 1));
> > >
> >
> > Alternatively, you could just add code to patch 1 which asserts that the
> > level/layer is in-bounds for the auxiliary surface if one exists.
> >
> >
>
> I think your suggestion complements, but doesn't replace this patch.
> Your suggestion validates the level and start_layer parameters. This
> patch validates those parameters as well as the num_layers parameter.
>

Right but maybe we should check num_layers against the aux surface rather
than having a gen check?


> > > +
> > > struct blorp_params params;
> > > blorp_params_init();
> > > params.num_layers = num_layers;
> > > --
> > > 2.13.1
> > >
> > > ___
> > > 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

Re: [Mesa-dev] [PATCH 02/13] intel/blorp: Assert on gen7 fast-cleared subresource

[Nanley Chery]

On Mon, Jun 19, 2017 at 04:06:34PM -0700, Jason Ekstrand wrote:
> On Tue, Jun 13, 2017 at 11:41 AM, Nanley Chery 
> wrote:
> 
> > Signed-off-by: Nanley Chery 
> > ---
> >  src/intel/blorp/blorp_clear.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/src/intel/blorp/blorp_clear.c b/src/intel/blorp/blorp_clear.c
> > index 3d5c41cc71..f43d05ad19 100644
> > --- a/src/intel/blorp/blorp_clear.c
> > +++ b/src/intel/blorp/blorp_clear.c
> > @@ -308,6 +308,10 @@ blorp_fast_clear(struct blorp_batch *batch,
> >   uint32_t level, uint32_t start_layer, uint32_t
> > num_layers,
> >   uint32_t x0, uint32_t y0, uint32_t x1, uint32_t y1)
> >  {
> > +   /* Multi-layer and multi-LOD fast clear is only available gen8+. */
> > +   assert(ISL_DEV_GEN(batch->blorp->isl_dev) >= 8 ||
> > +  (level == 0 && start_layer + num_layers == 1));
> >
> 
> Alternatively, you could just add code to patch 1 which asserts that the
> level/layer is in-bounds for the auxiliary surface if one exists.
> 
> 

I think your suggestion complements, but doesn't replace this patch.
Your suggestion validates the level and start_layer parameters. This
patch validates those parameters as well as the num_layers parameter.

> > +
> > struct blorp_params params;
> > blorp_params_init();
> > params.num_layers = num_layers;
> > --
> > 2.13.1
> >
> > ___
> > 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

Re: [Mesa-dev] [PATCH 02/13] intel/blorp: Assert on gen7 fast-cleared subresource

[Jason Ekstrand]

On Tue, Jun 13, 2017 at 11:41 AM, Nanley Chery 
wrote:

> Signed-off-by: Nanley Chery 
> ---
>  src/intel/blorp/blorp_clear.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/intel/blorp/blorp_clear.c b/src/intel/blorp/blorp_clear.c
> index 3d5c41cc71..f43d05ad19 100644
> --- a/src/intel/blorp/blorp_clear.c
> +++ b/src/intel/blorp/blorp_clear.c
> @@ -308,6 +308,10 @@ blorp_fast_clear(struct blorp_batch *batch,
>   uint32_t level, uint32_t start_layer, uint32_t
> num_layers,
>   uint32_t x0, uint32_t y0, uint32_t x1, uint32_t y1)
>  {
> +   /* Multi-layer and multi-LOD fast clear is only available gen8+. */
> +   assert(ISL_DEV_GEN(batch->blorp->isl_dev) >= 8 ||
> +  (level == 0 && start_layer + num_layers == 1));
>

Alternatively, you could just add code to patch 1 which asserts that the
level/layer is in-bounds for the auxiliary surface if one exists.


> +
> struct blorp_params params;
> blorp_params_init();
> params.num_layers = num_layers;
> --
> 2.13.1
>
> ___
> 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