On 12 November 2013 15:02, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 11/12/2013 01:29 PM, Paul Berry wrote: > > From the Sandy Bridge PRM, Vol 1 Part 1 7.18.3.4 (Alignment Unit > > Size): > > > > j [vertical alignment] = 4 for any render target surface is > > multisampled (4x) > > > > From the Ivy Bridge PRM, Vol 4 Part 1 2.12.2.1 (SURFACE_STATE for most > > messages), under the "Surface Vertical Alignment" heading: > > > > This field is intended to be set to VALIGN_4 if the surface was > > rendered as a depth buffer, for a multisampled (4x) render target, > > or for a multisampled (8x) render target, since these surfaces > > support only alignment of 4. > > > > Back in 2012 when we added multisampling support to the i965 driver, > > we forgot to update the logic for computing the vertical alignment, so > > we were often using a vertical alignment of 2 for multisampled > > buffers, leading to subtle rendering errors. > > > > Note that the specs also require a vertical alignment of 4 for all > > Y-tiled render target surfaces; I plan to address that in a separate > > patch. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53077 > > --- > > src/mesa/drivers/dri/i965/brw_tex_layout.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c > b/src/mesa/drivers/dri/i965/brw_tex_layout.c > > index d912862..d05dbeb 100644 > > --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c > > +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c > > @@ -86,7 +86,7 @@ intel_horizontal_texture_alignment_unit(struct > brw_context *brw, > > > > static unsigned int > > intel_vertical_texture_alignment_unit(struct brw_context *brw, > > - gl_format format) > > + gl_format format, bool > multisampled) > > { > > /** > > * From the "Alignment Unit Size" section of various specs, namely: > > @@ -110,8 +110,6 @@ intel_vertical_texture_alignment_unit(struct > brw_context *brw, > > * > > * On SNB+, non-special cases can be overridden by setting the > SURFACE_STATE > > * "Surface Vertical Alignment" field to VALIGN_2 or VALIGN_4. > > - * > > - * We currently don't support multisampling. > > */ > > if (_mesa_is_format_compressed(format)) > > return 4; > > @@ -119,6 +117,9 @@ intel_vertical_texture_alignment_unit(struct > brw_context *brw, > > if (format == MESA_FORMAT_S8) > > return brw->gen >= 7 ? 8 : 4; > > > > + if (multisampled) > > + return 4; > > + > > GLenum base_format = _mesa_get_format_base_format(format); > > > > if (brw->gen >= 6 && > > @@ -276,8 +277,10 @@ brw_miptree_layout_texture_3d(struct brw_context > *brw, > > void > > brw_miptree_layout(struct brw_context *brw, struct intel_mipmap_tree > *mt) > > { > > + bool multisampled = mt->num_samples > 1; > > mt->align_w = intel_horizontal_texture_alignment_unit(brw, > mt->format); > > - mt->align_h = intel_vertical_texture_alignment_unit(brw, mt->format); > > + mt->align_h = > > + intel_vertical_texture_alignment_unit(brw, mt->format, > multisampled); > > > > switch (mt->target) { > > case GL_TEXTURE_CUBE_MAP: > > > > Cc: mesa-sta...@lists.freedesktop.org > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> > > However, you might want to just pass the miptree to the > texture_alignment_unit() functions. That way, you can check mt->tiling > too. > Yeah, that's reasonable. I'll post a better series.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
