<div dir="ltr">On 12 November 2013 15:02, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=53077" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=53077</a> > --- > 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: > </div></div>Cc: <a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a> Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> However, you might want to just pass the miptree to the texture_alignment_unit() functions. That way, you can check mt->tiling too. </blockquote><div> </div><div>Yeah, that's reasonable. I'll post a better series.</div><div> </div></div> </div></div>