Re: [Mesa-dev] [PATCH 11/23] i965/gen8: Remove dead assertion

[Pohjolainen, Topi]

On Tue, Feb 09, 2016 at 03:40:07PM -0800, Ben Widawsky wrote:
> On Mon, Feb 08, 2016 at 06:51:31PM +0200, Topi Pohjolainen wrote:
> > The assertion is inside a condition mandating num_samples > 1 and
> > therefore the first half of the constraint is always met. The
> > second half in turn would only be applicable for single sampled
> > case and moreover it is trying to falsely check against surface
> > type instead of format.
> 
> 
> Oops.
> 
> > Subsequent patches will introduce proper support for the lossless
> > compression and dropping this here makes the patches a little
> > simpler.
> > 
> > Signed-off-by: Topi Pohjolainen 
> > ---
> >  src/mesa/drivers/dri/i965/gen8_surface_state.c | 6 --
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
> > b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > index 0df25d2..fc8f701 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> > @@ -243,12 +243,6 @@ gen8_emit_texture_surface_state(struct brw_context 
> > *brw,
> > */
> >if (brw->gen >= 9 || mt->num_samples == 1)
> >   assert(mt->halign == 16);
> > -
> > -  if (brw->gen >= 9) {
> > - assert(mt->num_samples > 1 ||
> > -brw_losslessly_compressible_format(brw, surf_type));
> > -  }
> > -
> > }
> >  
> > uint32_t *surf = allocate_surface_state(brw, surf_offset, surf_index);
> 
> Seems fine to drop the assertion entirely instead of correcting it. 
> You could mention the two patches which touched this part of the code if you
> wanted, or perhaps you realized you reviewed both of them :P

I did :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 11/23] i965/gen8: Remove dead assertion

[Ben Widawsky]

On Mon, Feb 08, 2016 at 06:51:31PM +0200, Topi Pohjolainen wrote:
> The assertion is inside a condition mandating num_samples > 1 and
> therefore the first half of the constraint is always met. The
> second half in turn would only be applicable for single sampled
> case and moreover it is trying to falsely check against surface
> type instead of format.


Oops.

> Subsequent patches will introduce proper support for the lossless
> compression and dropping this here makes the patches a little
> simpler.
> 
> Signed-off-by: Topi Pohjolainen 
> ---
>  src/mesa/drivers/dri/i965/gen8_surface_state.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index 0df25d2..fc8f701 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -243,12 +243,6 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> */
>if (brw->gen >= 9 || mt->num_samples == 1)
>   assert(mt->halign == 16);
> -
> -  if (brw->gen >= 9) {
> - assert(mt->num_samples > 1 ||
> -brw_losslessly_compressible_format(brw, surf_type));
> -  }
> -
> }
>  
> uint32_t *surf = allocate_surface_state(brw, surf_offset, surf_index);

Seems fine to drop the assertion entirely instead of correcting it. 
You could mention the two patches which touched this part of the code if you
wanted, or perhaps you realized you reviewed both of them :P
Reviewed-by: Ben Widawsky 

The one that used type instead of format:
commit 6fa1130cd21926cdd4ae86aa12ee3f5c0bb5ba33
Author: Ben Widawsky 
Date:   Tue Oct 13 20:50:21 2015 -0700

i965/skl: skip fast clears for certain surface formats

The one that didn't drop the duplicated assertion:
commit eb291d7013eef64c33826f9cc0006c89adcf4e53
Author: Neil Roberts 
Date:   Tue Nov 24 17:59:28 2015 +0100

i965/gen8+: Don't upload the MCS buffer for single-sampled textures
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 11/23] i965/gen8: Remove dead assertion

[Topi Pohjolainen]

The assertion is inside a condition mandating num_samples > 1 and
therefore the first half of the constraint is always met. The
second half in turn would only be applicable for single sampled
case and moreover it is trying to falsely check against surface
type instead of format.
Subsequent patches will introduce proper support for the lossless
compression and dropping this here makes the patches a little
simpler.

Signed-off-by: Topi Pohjolainen 
---
 src/mesa/drivers/dri/i965/gen8_surface_state.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
b/src/mesa/drivers/dri/i965/gen8_surface_state.c
index 0df25d2..fc8f701 100644
--- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
@@ -243,12 +243,6 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
*/
   if (brw->gen >= 9 || mt->num_samples == 1)
  assert(mt->halign == 16);
-
-  if (brw->gen >= 9) {
- assert(mt->num_samples > 1 ||
-brw_losslessly_compressible_format(brw, surf_type));
-  }
-
}
 
uint32_t *surf = allocate_surface_state(brw, surf_offset, surf_index);
-- 
2.5.0

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