On Mon 16 Nov 2015, Matt Turner wrote: > On Wed, Nov 11, 2015 at 2:06 PM, Ben Widawsky > <benjamin.widaw...@intel.com> wrote: > > Some of the information originally in this commit message is now in the > > patch > > before 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." > > > > The formats which are supported are defined in the table titled "Render > > Target > > Surface Types [SKL+]". There is no PRM yet to reference. The previously > > implemented helper function already does the right thing provided the table > > is > > correct. > > > > v2: Use better English in commit message (Matt) > > s/compressable/compressible/ (Matt) > > Don't compare bools to true (Matt) > > Use the helper function and don't increase the context size - this is mostly > > implemented in the patch just before this (Chad, Neil) > > Remove an "invalid" assert (Chad) > > Fix assertion to check num_samples > 1, instead of num_samples (Chad) > > > > Cc: Chad Versace <chad.vers...@linux.intel.com> > > Cc: Neil Roberts <n...@linux.intel.com> > > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > > --- > > src/mesa/drivers/dri/i965/brw_surface_formats.c | 52 > > ++++++++++++------------- > > src/mesa/drivers/dri/i965/gen8_surface_state.c | 7 +++- > > 2 files changed, 31 insertions(+), 28 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c > > b/src/mesa/drivers/dri/i965/brw_surface_formats.c > > index a7cdc13..a527f2f 100644 > > --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c > > +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c > > @@ -90,9 +90,9 @@ struct surface_format_info { > > */ > > const struct surface_format_info surface_formats[] = { > > /* smpl filt shad CK RT AB VB SO color ccs */ > > - SF( Y, 50, x, x, Y, Y, Y, Y, x, x, R32G32B32A32_FLOAT) > > - SF( Y, x, x, x, Y, x, Y, Y, x, x, R32G32B32A32_SINT) > > - SF( Y, x, x, x, Y, x, Y, Y, x, x, R32G32B32A32_UINT) > > + SF( Y, 50, x, x, Y, Y, Y, Y, x, 90, R32G32B32A32_FLOAT) > > + SF( Y, x, x, x, Y, x, Y, Y, x, 90, R32G32B32A32_SINT) > > + SF( Y, x, x, x, Y, x, Y, Y, x, 90, R32G32B32A32_UINT) > > SF( x, x, x, x, x, x, Y, x, x, x, R32G32B32A32_UNORM) > > SF( x, x, x, x, x, x, Y, x, x, x, R32G32B32A32_SNORM) > > SF( x, x, x, x, x, x, Y, x, x, x, R64G64_FLOAT) > > @@ -109,15 +109,15 @@ const struct surface_format_info surface_formats[] = { > > SF( x, x, x, x, x, x, Y, x, x, x, R32G32B32_SSCALED) > > SF( x, x, x, x, x, x, Y, x, x, x, R32G32B32_USCALED) > > SF( x, x, x, x, x, x, x, x, x, x, R32G32B32_SFIXED) > > - SF( Y, Y, x, x, Y, 45, Y, x, 60, x, R16G16B16A16_UNORM) > > - SF( Y, Y, x, x, Y, 60, Y, x, x, x, R16G16B16A16_SNORM) > > - SF( Y, x, x, x, Y, x, Y, x, x, x, R16G16B16A16_SINT) > > - SF( Y, x, x, x, Y, x, Y, x, x, x, R16G16B16A16_UINT) > > - SF( Y, Y, x, x, Y, Y, Y, x, x, x, R16G16B16A16_FLOAT) > > - SF( Y, 50, x, x, Y, Y, Y, Y, x, x, R32G32_FLOAT) > > + SF( Y, Y, x, x, Y, 45, Y, x, 60, 90, R16G16B16A16_UNORM) > > + SF( Y, Y, x, x, Y, 60, Y, x, x, 90, R16G16B16A16_SNORM) > > + SF( Y, x, x, x, Y, x, Y, x, x, 90, R16G16B16A16_SINT) > > + SF( Y, x, x, x, Y, x, Y, x, x, 90, R16G16B16A16_UINT) > > + SF( Y, Y, x, x, Y, Y, Y, x, x, 90, R16G16B16A16_FLOAT) > > + SF( Y, 50, x, x, Y, Y, Y, Y, x, 90, R32G32_FLOAT) > > SF( Y, 70, x, x, Y, Y, Y, Y, x, x, R32G32_FLOAT_LD) > > - SF( Y, x, x, x, Y, x, Y, Y, x, x, R32G32_SINT) > > - SF( Y, x, x, x, Y, x, Y, Y, x, x, R32G32_UINT) > > + SF( Y, x, x, x, Y, x, Y, Y, x, 90, R32G32_SINT) > > + SF( Y, x, x, x, Y, x, Y, Y, x, 90, R32G32_UINT) > > SF( Y, 50, Y, x, x, x, x, x, x, x, R32_FLOAT_X8X24_TYPELESS) > > SF( Y, x, x, x, x, x, x, x, x, x, X32_TYPELESS_G8X24_UINT) > > SF( Y, 50, x, x, x, x, x, x, x, x, L32A32_FLOAT) > > @@ -125,7 +125,7 @@ const struct surface_format_info surface_formats[] = { > > SF( x, x, x, x, x, x, Y, x, x, x, R32G32_SNORM) > > SF( x, x, x, x, x, x, Y, x, x, x, R64_FLOAT) > > SF( Y, Y, x, x, x, x, x, x, x, x, R16G16B16X16_UNORM) > > - SF( Y, Y, x, x, x, x, x, x, x, x, R16G16B16X16_FLOAT) > > + SF( Y, Y, x, x, x, x, x, x, x, 90, R16G16B16X16_FLOAT) > > SF( Y, 50, x, x, x, x, x, x, x, x, A32X32_FLOAT) > > SF( Y, 50, x, x, x, x, x, x, x, x, L32X32_FLOAT) > > SF( Y, 50, x, x, x, x, x, x, x, x, I32X32_FLOAT) > > @@ -135,29 +135,29 @@ const struct surface_format_info surface_formats[] = { > > SF( x, x, x, x, x, x, Y, x, x, x, R32G32_USCALED) > > SF( x, x, x, x, x, x, x, x, x, x, R32G32_SFIXED) > > SF( x, x, x, x, x, x, x, x, x, x, R64_PASSTHRU) > > - SF( Y, Y, x, Y, Y, Y, Y, x, 60, x, B8G8R8A8_UNORM) > > + SF( Y, Y, x, Y, Y, Y, Y, x, 60, 90, B8G8R8A8_UNORM) > > SF( Y, Y, x, x, Y, Y, x, x, x, x, B8G8R8A8_UNORM_SRGB) > > /* smpl filt shad CK RT AB VB SO color ccs */ > > SF( Y, Y, x, x, Y, Y, Y, x, 60, x, R10G10B10A2_UNORM) > > SF( Y, Y, x, x, x, x, x, x, 60, x, R10G10B10A2_UNORM_SRGB) > > SF( Y, x, x, x, Y, x, Y, x, x, x, R10G10B10A2_UINT) > > SF( Y, Y, x, x, x, Y, Y, x, x, x, R10G10B10_SNORM_A2_UNORM) > > - SF( Y, Y, x, x, Y, Y, Y, x, 60, x, R8G8B8A8_UNORM) > > + SF( Y, Y, x, x, Y, Y, Y, x, 60, 90, R8G8B8A8_UNORM) > > SF( Y, Y, x, x, Y, Y, x, x, 60, x, R8G8B8A8_UNORM_SRGB) > > - SF( Y, Y, x, x, Y, 60, Y, x, x, x, R8G8B8A8_SNORM) > > - SF( Y, x, x, x, Y, x, Y, x, x, x, R8G8B8A8_SINT) > > - SF( Y, x, x, x, Y, x, Y, x, x, x, R8G8B8A8_UINT) > > - SF( Y, Y, x, x, Y, 45, Y, x, x, x, R16G16_UNORM) > > - SF( Y, Y, x, x, Y, 60, Y, x, x, x, R16G16_SNORM) > > - SF( Y, x, x, x, Y, x, Y, x, x, x, R16G16_SINT) > > - SF( Y, x, x, x, Y, x, Y, x, x, x, R16G16_UINT) > > - SF( Y, Y, x, x, Y, Y, Y, x, x, x, R16G16_FLOAT) > > + SF( Y, Y, x, x, Y, 60, Y, x, x, 90, R8G8B8A8_SNORM) > > + SF( Y, x, x, x, Y, x, Y, x, x, 90, R8G8B8A8_SINT) > > + SF( Y, x, x, x, Y, x, Y, x, x, 90, R8G8B8A8_UINT) > > + SF( Y, Y, x, x, Y, 45, Y, x, x, 90, R16G16_UNORM) > > + SF( Y, Y, x, x, Y, 60, Y, x, x, 90, R16G16_SNORM) > > + SF( Y, x, x, x, Y, x, Y, x, x, 90, R16G16_SINT) > > + SF( Y, x, x, x, Y, x, Y, x, x, 90, R16G16_UINT) > > + SF( Y, Y, x, x, Y, Y, Y, x, x, 90, R16G16_FLOAT) > > SF( Y, Y, x, x, Y, Y, x, x, 60, x, B10G10R10A2_UNORM) > > SF( Y, Y, x, x, Y, Y, x, x, 60, x, B10G10R10A2_UNORM_SRGB) > > SF( Y, Y, x, x, Y, Y, Y, x, x, x, R11G11B10_FLOAT) > > - SF( Y, x, x, x, Y, x, Y, Y, x, x, R32_SINT) > > - SF( Y, x, x, x, Y, x, Y, Y, x, x, R32_UINT) > > - SF( Y, 50, Y, x, Y, Y, Y, Y, x, x, R32_FLOAT) > > + SF( Y, x, x, x, Y, x, Y, Y, x, 90, R32_SINT) > > + SF( Y, x, x, x, Y, x, Y, Y, x, 90, R32_UINT) > > + SF( Y, 50, Y, x, Y, Y, Y, Y, x, 90, R32_FLOAT) > > SF( Y, 50, Y, x, x, x, x, x, x, x, R24_UNORM_X8_TYPELESS) > > SF( Y, x, x, x, x, x, x, x, x, x, X24_TYPELESS_G8_UINT) > > SF( Y, Y, x, x, x, x, x, x, x, x, L16A16_UNORM) > > @@ -167,7 +167,7 @@ const struct surface_format_info surface_formats[] = { > > SF( Y, 50, Y, x, x, x, x, x, x, x, I32_FLOAT) > > SF( Y, 50, Y, x, x, x, x, x, x, x, L32_FLOAT) > > SF( Y, 50, Y, x, x, x, x, x, x, x, A32_FLOAT) > > - SF( Y, Y, x, Y, x, x, x, x, 60, x, B8G8R8X8_UNORM) > > + SF( Y, Y, x, Y, x, x, x, x, 60, 90, B8G8R8X8_UNORM) > > SF( Y, Y, x, x, x, x, x, x, x, x, B8G8R8X8_UNORM_SRGB) > > SF( Y, Y, x, x, x, x, x, x, x, x, R8G8B8X8_UNORM) > > SF( Y, Y, x, x, x, x, x, x, x, x, R8G8B8X8_UNORM_SRGB) > > diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c > > b/src/mesa/drivers/dri/i965/gen8_surface_state.c > > index 6909858..8fe480c 100644 > > --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c > > +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c > > @@ -222,6 +222,7 @@ gen8_emit_texture_surface_state(struct brw_context *brw, > > int surf_index = surf_offset - &brw->wm.base.surf_offset[0]; > > unsigned tiling_mode, pitch; > > const unsigned tr_mode = surface_tiling_resource_mode(mt->tr_mode); > > + const uint32_t surf_type = translate_tex_target(target); > > > > if (mt->format == MESA_FORMAT_S_UINT8) { > > tiling_mode = GEN8_SURFACE_TILING_W; > > @@ -243,11 +244,13 @@ 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 > 1 || > > + brw_losslessly_compressible_format(brw, surf_type)); > > + } > > Maybe splitting the assertions between two if-statements, like this, is > clearer? > > 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)); > } > > I think what was confusing was that the mt->num_samples > 1 in the > second assertion could only be reached by way of half of the if > conditional ("brw->gen >= 9").
Yes, that was the problem. Matt's suggested code is clear enough. I can't give an r-b yet, though, because there are outstanding issues in patch 2 that affect patch 3. Namely, the behavior of brw_losslessly_compressible_format(). _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev