Re: [Mesa-dev] [PATCH 04/10] i965/skl: skip fast clears for certain surface formats

2015-11-10 Thread Ben Widawsky 
There is one thing I need a response on at the very bottom, the rest will be
addressed in v2.

Thanks.

On Mon, Nov 09, 2015 at 11:33:17AM -0800, Chad Versace wrote:
> On Tue 03 Nov 2015, Ben Widawsky wrote:
> > On Fri, Oct 16, 2015 at 04:05:22PM -0700, Chad Versace wrote:
> > > On Tue 13 Oct 2015, Ben Widawsky wrote:
> > > > Initially I had this planned as a patch to be squashed in to the 
> > > > enabling patch
> > > > because there is no point enabling fast clears without this. However, 
> > > > Chad
> > > > merged a patch which disables fast clears on gen9 explicitly, and so I 
> > > > can hide
> > > > this behind the revert of that patch. This is a nice I really wanted 
> > > > this patch
> > > > as a distinct patch for review. This is a new, weird, and poorly 
> > > > documented
> > > > restriction for SKL. (In fact, I am still not 100% certain the 
> > > > restriction is
> > > > entirely necessary, but there are around 30 piglit regressions without 
> > > > this).
> > > > 
> > > > SKL adds compressible render targets and as a result mutates some of the
> > > > programming for fast clears and resolves. There is a new internal 
> > > > surface type
> > > > called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary 
> > > > surface is
> > > > a CCS (Color Control Surface) with compression disabled or an MCS with
> > > > compression enabled, depending on number of multisamples. MCS 
> > > > (Multisample
> > > > Control Surface) is a special type of CCS."
> > > > 
> > > > Signed-off-by: Ben Widawsky 
> > > > ---
> > > >  src/mesa/drivers/dri/i965/brw_context.h |  1 +
> > > >  src/mesa/drivers/dri/i965/brw_surface_formats.c | 27 
> > > > +
> > > >  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 ++--
> > > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  3 +++
> > > >  4 files changed, 37 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> > > > b/src/mesa/drivers/dri/i965/brw_context.h
> > > > index e59478a..32b8250 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > > > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > > > @@ -1546,6 +1546,7 @@ struct brw_context
> > > >  
> > > > uint32_t render_target_format[MESA_FORMAT_COUNT];
> > > > bool format_supported_as_render_target[MESA_FORMAT_COUNT];
> > > > +   bool losslessly_compressable[MESA_FORMAT_COUNT];
> > > 
> > > I agree with Neil. It's a shame to increase the context size for static
> > > information. And there is already a static array in
> > > brw_surface_formats.c for exactly this type of information.
> > 
> > As I said in the mail with Neil, the formats aren't static and change per 
> > GEN. I
> > could do the base formats which are shared by all Gens, but I think you 
> > still
> > end up needing something like this. Or did I misunderstand what is being 
> > asked
> > of me?
> 
> One of us misunderstood each other, but I don't know who.
> 
> The table in brw_surface_formats.c does contain per-gen information. The
> table lists, for each surface format and surface format capability, the
> first gen that supports the capability.
> 
> So, you can avoid fattening brw_context by adding a new column in the
> brw_surface_formats table for losslessly_compressable, then setting each
> item in the column to either X (no gen supports the capability) or 90
> (Skylake).
> 
> Is there some reason why this wouldn't work?

I misunderstood something Neil said. You're (and Neil) totally right here.

> 
> > 
> > > >  
> > > > /* Interpolation modes, one byte per vue slot.
> > > >  * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
> > > > b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > > > index 97fff60..d706ecc 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > > > @@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
> > > >}
> > > > }
> > > >  
> > > > +   if (brw->gen >= 9) {
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
> > > > +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
> > >

Re: [Mesa-dev] [PATCH 04/10] i965/skl: skip fast clears for certain surface formats

2015-11-09 Thread Chad Versace 
On Tue 03 Nov 2015, Ben Widawsky wrote:
> On Fri, Oct 16, 2015 at 04:05:22PM -0700, Chad Versace wrote:
> > On Tue 13 Oct 2015, Ben Widawsky wrote:
> > > Initially I had this planned as a patch to be squashed in to the enabling 
> > > patch
> > > because there is no point enabling fast clears without this. However, Chad
> > > merged a patch which disables fast clears on gen9 explicitly, and so I 
> > > can hide
> > > this behind the revert of that patch. This is a nice I really wanted this 
> > > patch
> > > as a distinct patch for review. This is a new, weird, and poorly 
> > > documented
> > > restriction for SKL. (In fact, I am still not 100% certain the 
> > > restriction is
> > > entirely necessary, but there are around 30 piglit regressions without 
> > > this).
> > > 
> > > SKL adds compressible render targets and as a result mutates some of the
> > > programming for fast clears and resolves. There is a new internal surface 
> > > type
> > > called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary 
> > > surface is
> > > a CCS (Color Control Surface) with compression disabled or an MCS with
> > > compression enabled, depending on number of multisamples. MCS (Multisample
> > > Control Surface) is a special type of CCS."
> > > 
> > > Signed-off-by: Ben Widawsky 
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_context.h |  1 +
> > >  src/mesa/drivers/dri/i965/brw_surface_formats.c | 27 
> > > +
> > >  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 ++--
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  3 +++
> > >  4 files changed, 37 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> > > b/src/mesa/drivers/dri/i965/brw_context.h
> > > index e59478a..32b8250 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > > @@ -1546,6 +1546,7 @@ struct brw_context
> > >  
> > > uint32_t render_target_format[MESA_FORMAT_COUNT];
> > > bool format_supported_as_render_target[MESA_FORMAT_COUNT];
> > > +   bool losslessly_compressable[MESA_FORMAT_COUNT];
> > 
> > I agree with Neil. It's a shame to increase the context size for static
> > information. And there is already a static array in
> > brw_surface_formats.c for exactly this type of information.
> 
> As I said in the mail with Neil, the formats aren't static and change per 
> GEN. I
> could do the base formats which are shared by all Gens, but I think you still
> end up needing something like this. Or did I misunderstand what is being asked
> of me?

One of us misunderstood each other, but I don't know who.

The table in brw_surface_formats.c does contain per-gen information. The
table lists, for each surface format and surface format capability, the
first gen that supports the capability.

So, you can avoid fattening brw_context by adding a new column in the
brw_surface_formats table for losslessly_compressable, then setting each
item in the column to either X (no gen supports the capability) or 90
(Skylake).

Is there some reason why this wouldn't work?

> 
> > >  
> > > /* Interpolation modes, one byte per vue slot.
> > >  * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> > > diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
> > > b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > > index 97fff60..d706ecc 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > > @@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
> > >}
> > > }
> > >  
> > > +   if (brw->gen >= 9) {
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBX_FLOAT16] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8A8_UNORM] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_UNORM] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_SNORM] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT8] = true;
> > > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT8] = true;
> > >

Re: [Mesa-dev] [PATCH 04/10] i965/skl: skip fast clears for certain surface formats

2015-11-03 Thread Ben Widawsky 
On Fri, Oct 16, 2015 at 04:05:22PM -0700, Chad Versace wrote:
> On Tue 13 Oct 2015, Ben Widawsky wrote:
> > Initially I had this planned as a patch to be squashed in to the enabling 
> > patch
> > because there is no point enabling fast clears without this. However, Chad
> > merged a patch which disables fast clears on gen9 explicitly, and so I can 
> > hide
> > this behind the revert of that patch. This is a nice I really wanted this 
> > patch
> > as a distinct patch for review. This is a new, weird, and poorly documented
> > restriction for SKL. (In fact, I am still not 100% certain the restriction 
> > is
> > entirely necessary, but there are around 30 piglit regressions without 
> > this).
> > 
> > SKL adds compressible render targets and as a result mutates some of the
> > programming for fast clears and resolves. There is a new internal surface 
> > type
> > called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary 
> > surface is
> > a CCS (Color Control Surface) with compression disabled or an MCS with
> > compression enabled, depending on number of multisamples. MCS (Multisample
> > Control Surface) is a special type of CCS."
> > 
> > Signed-off-by: Ben Widawsky 
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.h |  1 +
> >  src/mesa/drivers/dri/i965/brw_surface_formats.c | 27 
> > +
> >  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 ++--
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  3 +++
> >  4 files changed, 37 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> > b/src/mesa/drivers/dri/i965/brw_context.h
> > index e59478a..32b8250 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -1546,6 +1546,7 @@ struct brw_context
> >  
> > uint32_t render_target_format[MESA_FORMAT_COUNT];
> > bool format_supported_as_render_target[MESA_FORMAT_COUNT];
> > +   bool losslessly_compressable[MESA_FORMAT_COUNT];
> 
> I agree with Neil. It's a shame to increase the context size for static
> information. And there is already a static array in
> brw_surface_formats.c for exactly this type of information.
> 

As I said in the mail with Neil, the formats aren't static and change per GEN. I
could do the base formats which are shared by all Gens, but I think you still
end up needing something like this. Or did I misunderstand what is being asked
of me?

> >  
> > /* Interpolation modes, one byte per vue slot.
> >  * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> > diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
> > b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > index 97fff60..d706ecc 100644
> > --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > @@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
> >}
> > }
> >  
> > +   if (brw->gen >= 9) {
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBX_FLOAT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8A8_UNORM] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_UNORM] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_SNORM] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT8] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT8] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R_UINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R_SINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R_FLOAT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8X8_UNORM] = true;
> > +   }
> 
> Properties of surface formats should go into the monster table that
> occurs earlier in the file. Then you can replace
> brw_context::losslessly_compressable with a query and keep brw_context
> at its current size.

Same

Re: [Mesa-dev] [PATCH 04/10] i965/skl: skip fast clears for certain surface formats

2015-10-16 Thread Chad Versace 
On Tue 13 Oct 2015, Ben Widawsky wrote:
> Initially I had this planned as a patch to be squashed in to the enabling 
> patch
> because there is no point enabling fast clears without this. However, Chad
> merged a patch which disables fast clears on gen9 explicitly, and so I can 
> hide
> this behind the revert of that patch. This is a nice I really wanted this 
> patch
> as a distinct patch for review. This is a new, weird, and poorly documented
> restriction for SKL. (In fact, I am still not 100% certain the restriction is
> entirely necessary, but there are around 30 piglit regressions without this).
> 
> SKL adds compressible render targets and as a result mutates some of the
> programming for fast clears and resolves. There is a new internal surface type
> called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary surface 
> is
> a CCS (Color Control Surface) with compression disabled or an MCS with
> compression enabled, depending on number of multisamples. MCS (Multisample
> Control Surface) is a special type of CCS."
> 
> Signed-off-by: Ben Widawsky 
> ---
>  src/mesa/drivers/dri/i965/brw_context.h |  1 +
>  src/mesa/drivers/dri/i965/brw_surface_formats.c | 27 
> +
>  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 ++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  3 +++
>  4 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index e59478a..32b8250 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1546,6 +1546,7 @@ struct brw_context
>  
> uint32_t render_target_format[MESA_FORMAT_COUNT];
> bool format_supported_as_render_target[MESA_FORMAT_COUNT];
> +   bool losslessly_compressable[MESA_FORMAT_COUNT];

I agree with Neil. It's a shame to increase the context size for static
information. And there is already a static array in
brw_surface_formats.c for exactly this type of information.

>  
> /* Interpolation modes, one byte per vue slot.
>  * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
> b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> index 97fff60..d706ecc 100644
> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> @@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
>}
> }
>  
> +   if (brw->gen >= 9) {
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBX_FLOAT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8A8_UNORM] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_UNORM] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_SNORM] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT8] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT8] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R_UINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R_SINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R_FLOAT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8X8_UNORM] = true;
> +   }

Properties of surface formats should go into the monster table that
occurs earlier in the file. Then you can replace
brw_context::losslessly_compressable with a query and keep brw_context
at its current size.

> +
> /* We will check this table for FBO completeness, but the surface format
>  * table above only covered color rendering.
>  */
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index 995b4dd..b19b492 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -243,8 +243,10 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> * "When

Re: [Mesa-dev] [PATCH 04/10] i965/skl: skip fast clears for certain surface formats

2015-10-14 Thread Ben Widawsky 
On Wed, Oct 14, 2015 at 01:10:23PM +0200, Neil Roberts wrote:
> It would be nice if you could give some indication of where this list of
> formats came from.
> 
> Unless we expect the list to change with future generations, maybe it
> would be better to make it a static const table? It's a shame to grow
> the context size unnecessarily.
> 
> Regards,
> - Neil
> 

You are correct, I should have referenced it. It is in the section titled
"Render Target Surface Types [SKL+] - Surface Formats for Render Target Messages
[SKL+]"

The supported formats do change, for now we could certainly assume that SKL is
the base set and future GENs add things.

> Ben Widawsky  writes:
> 
> > Initially I had this planned as a patch to be squashed in to the enabling 
> > patch
> > because there is no point enabling fast clears without this. However, Chad
> > merged a patch which disables fast clears on gen9 explicitly, and so I can 
> > hide
> > this behind the revert of that patch. This is a nice I really wanted this 
> > patch
> > as a distinct patch for review. This is a new, weird, and poorly documented
> > restriction for SKL. (In fact, I am still not 100% certain the restriction 
> > is
> > entirely necessary, but there are around 30 piglit regressions without 
> > this).
> >
> > SKL adds compressible render targets and as a result mutates some of the
> > programming for fast clears and resolves. There is a new internal surface 
> > type
> > called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary 
> > surface is
> > a CCS (Color Control Surface) with compression disabled or an MCS with
> > compression enabled, depending on number of multisamples. MCS (Multisample
> > Control Surface) is a special type of CCS."
> >
> > Signed-off-by: Ben Widawsky 
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.h |  1 +
> >  src/mesa/drivers/dri/i965/brw_surface_formats.c | 27 
> > +
> >  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 ++--
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  3 +++
> >  4 files changed, 37 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> > b/src/mesa/drivers/dri/i965/brw_context.h
> > index e59478a..32b8250 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -1546,6 +1546,7 @@ struct brw_context
> >  
> > uint32_t render_target_format[MESA_FORMAT_COUNT];
> > bool format_supported_as_render_target[MESA_FORMAT_COUNT];
> > +   bool losslessly_compressable[MESA_FORMAT_COUNT];
> >  
> > /* Interpolation modes, one byte per vue slot.
> >  * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> > diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
> > b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > index 97fff60..d706ecc 100644
> > --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> > @@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
> >}
> > }
> >  
> > +   if (brw->gen >= 9) {
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBX_FLOAT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8A8_UNORM] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_UNORM] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_SNORM] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT8] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT8] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT16] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R_UINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R_SINT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_R_FLOAT32] = true;
> > +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8X8_UNORM] = true;
> > +   }
> > +
> > /* We will check this table for FBO completeness, but

Re: [Mesa-dev] [PATCH 04/10] i965/skl: skip fast clears for certain surface formats

2015-10-14 Thread Neil Roberts 
It would be nice if you could give some indication of where this list of
formats came from.

Unless we expect the list to change with future generations, maybe it
would be better to make it a static const table? It's a shame to grow
the context size unnecessarily.

Regards,
- Neil

Ben Widawsky  writes:

> Initially I had this planned as a patch to be squashed in to the enabling 
> patch
> because there is no point enabling fast clears without this. However, Chad
> merged a patch which disables fast clears on gen9 explicitly, and so I can 
> hide
> this behind the revert of that patch. This is a nice I really wanted this 
> patch
> as a distinct patch for review. This is a new, weird, and poorly documented
> restriction for SKL. (In fact, I am still not 100% certain the restriction is
> entirely necessary, but there are around 30 piglit regressions without this).
>
> SKL adds compressible render targets and as a result mutates some of the
> programming for fast clears and resolves. There is a new internal surface type
> called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary surface 
> is
> a CCS (Color Control Surface) with compression disabled or an MCS with
> compression enabled, depending on number of multisamples. MCS (Multisample
> Control Surface) is a special type of CCS."
>
> Signed-off-by: Ben Widawsky 
> ---
>  src/mesa/drivers/dri/i965/brw_context.h |  1 +
>  src/mesa/drivers/dri/i965/brw_surface_formats.c | 27 
> +
>  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 ++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  3 +++
>  4 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index e59478a..32b8250 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1546,6 +1546,7 @@ struct brw_context
>  
> uint32_t render_target_format[MESA_FORMAT_COUNT];
> bool format_supported_as_render_target[MESA_FORMAT_COUNT];
> +   bool losslessly_compressable[MESA_FORMAT_COUNT];
>  
> /* Interpolation modes, one byte per vue slot.
>  * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
> b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> index 97fff60..d706ecc 100644
> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> @@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
>}
> }
>  
> +   if (brw->gen >= 9) {
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBX_FLOAT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8A8_UNORM] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_UNORM] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_SNORM] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT8] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT8] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_SINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_UINT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT16] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R_UINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R_SINT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_R_FLOAT32] = true;
> +  brw->losslessly_compressable[MESA_FORMAT_B8G8R8X8_UNORM] = true;
> +   }
> +
> /* We will check this table for FBO completeness, but the surface format
>  * table above only covered color rendering.
>  */
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index 995b4dd..b19b492 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -243,8 +243,10 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> * "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, 
> HALIGN
> *  16 must be used."
>

[Mesa-dev] [PATCH 04/10] i965/skl: skip fast clears for certain surface formats

2015-10-13 Thread Ben Widawsky 
Initially I had this planned as a patch to be squashed in to the enabling patch
because there is no point enabling fast clears without this. However, Chad
merged a patch which disables fast clears on gen9 explicitly, and so I can hide
this behind the revert of that patch. This is a nice I really wanted this patch
as a distinct patch for review. This is a new, weird, and poorly documented
restriction for SKL. (In fact, I am still not 100% certain the restriction is
entirely necessary, but there are around 30 piglit regressions without this).

SKL adds compressible render targets and as a result mutates some of the
programming for fast clears and resolves. There is a new internal surface type
called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary surface is
a CCS (Color Control Surface) with compression disabled or an MCS with
compression enabled, depending on number of multisamples. MCS (Multisample
Control Surface) is a special type of CCS."

Signed-off-by: Ben Widawsky 
---
 src/mesa/drivers/dri/i965/brw_context.h |  1 +
 src/mesa/drivers/dri/i965/brw_surface_formats.c | 27 +
 src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 ++--
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  3 +++
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index e59478a..32b8250 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1546,6 +1546,7 @@ struct brw_context
 
uint32_t render_target_format[MESA_FORMAT_COUNT];
bool format_supported_as_render_target[MESA_FORMAT_COUNT];
+   bool losslessly_compressable[MESA_FORMAT_COUNT];
 
/* Interpolation modes, one byte per vue slot.
 * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
b/src/mesa/drivers/dri/i965/brw_surface_formats.c
index 97fff60..d706ecc 100644
--- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
+++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
@@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
   }
}
 
+   if (brw->gen >= 9) {
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBX_FLOAT16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_B8G8R8A8_UNORM] = true;
+  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_UNORM] = true;
+  brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_SNORM] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT8] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT8] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RG_SINT16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RG_UINT16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT16] = true;
+  brw->losslessly_compressable[MESA_FORMAT_R_UINT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_R_SINT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_R_FLOAT32] = true;
+  brw->losslessly_compressable[MESA_FORMAT_B8G8R8X8_UNORM] = true;
+   }
+
/* We will check this table for FBO completeness, but the surface format
 * table above only covered color rendering.
 */
diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
b/src/mesa/drivers/dri/i965/gen8_surface_state.c
index 995b4dd..b19b492 100644
--- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
@@ -243,8 +243,10 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
* "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN
*  16 must be used."
*/
-  if (brw->gen >= 9 || mt->num_samples == 1)
+  if (brw->gen >= 9 || mt->num_samples == 1) {
  assert(mt->halign == 16);
+ assert(mt->num_samples || brw->losslessly_compressable[mt->format] == 
true);
+  }
}
 
const uint32_t surf_type = translate_tex_target(target);
@@ -488,8 +490,10 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
* "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN
*  16 must be used."
*/
-  if