On 12 November 2013 15:15, Paul Berry <stereotype...@gmail.com> wrote:
> 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. > > > After further discussion on IRC, Ken, Eric and I decided that the patch should be landed as is. I will post a follow-up series to deal with the tiling restrictions. This should be cherry-picked back to 10.0 and 9.2. Cc-ing stable.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
