[Mesa-dev] [PATCH 5/8] i965/msaa: Set SURFACE_STATE properly when CMS MSAA is in use.

[Paul Berry]

On 11 July 2012 09:50, Chad Versace <chad.versace at linux.intel.com> wrote:

> On 07/06/2012 03:29 PM, Paul Berry wrote:
> > When a buffer using Gen7's CMS MSAA layout is bound to a texture or a
> > render target, the SURFACE_STATE structure needs to point to the MCS
> > buffer and to indicate its pitch.  This patch updates the functions
> > that emit SURFACE_STATE to handle CMS layout properly.
> > ---
> >  src/mesa/drivers/dri/i965/brw_state.h             |    5 ++
> >  src/mesa/drivers/dri/i965/gen7_blorp.cpp          |    4 ++
> >  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |   45
> +++++++++++++++++++++
> >  3 files changed, 54 insertions(+), 0 deletions(-)
>
>
>
> >  void
> > +gen7_set_surface_mcs_info(struct brw_context *brw,
> > +                          struct gen7_surface_state *surf,
> > +                          uint32_t surf_offset,
> > +                          const struct intel_mipmap_tree *mcs_mt,
> > +                          bool is_render_target)
> > +{
> > +   /* From the Ivy Bridge PRM, Vol4 Part1 p76, "MCS Base Address":
> > +    *
> > +    *     "The MCS surface must be stored as Tile Y."
> > +    */
> > +   assert(mcs_mt->region->tiling == I915_TILING_Y);
> > +
> > +   /* Compute the pitch in units of tiles.  To do this we need to
> divide the
> > +    * pitch in bytes by 128, since a single Y-tile is 128 bytes wide.
> > +    */
> > +   unsigned pitch_bytes = mcs_mt->region->pitch * mcs_mt->cpp;
> > +   unsigned pitch_tiles = pitch_bytes / 128;
>
> Here, I first thought "A rounddown error will occur if pitch_bytes is not
> 128-aligned!" But I convinced myself that can't happen.
>
>
>
> > +
> > +   /* The upper 20 bits of surface state DWORD 6 are the upper 20 bits
> of the
> > +    * GPU address of the MCS buffer; the lower 12 bits contain other
> control
> > +    * information.  Since buffer addresses are always on 4k boundaries
> (and
> > +    * thus have their lower 12 bits zero), we can use an ordinary reloc
> to do
> > +    * the necessary address translation.
> > +    */
> > +   assert ((mcs_mt->region->bo->offset & 0xfff) == 0);
> > +   surf->ss6.mcs_enabled.mcs_enable = 1;
> > +   surf->ss6.mcs_enabled.mcs_surface_pitch = pitch_tiles - 1;
> > +   surf->ss6.mcs_enabled.mcs_base_address = mcs_mt->region->bo->offset
> >> 12;
> > +   drm_intel_bo_emit_reloc(brw->intel.batch.bo,
> > +                           surf_offset +
> > +                           offsetof(struct gen7_surface_state, ss6),
> > +                           mcs_mt->region->bo,
> > +                           surf->ss6.raw_data -
> mcs_mt->region->bo->offset,
> > +                           is_render_target ? I915_GEM_DOMAIN_RENDER
> > +                           : I915_GEM_DOMAIN_SAMPLER,
> > +                           is_render_target ? I915_GEM_DOMAIN_RENDER :
> 0);
> > +}
>
> I'm confused. ss6.raw_data is not an address nor an offset; it's lower
> bits are
> non-addressy things. So I see a unit conflict in the target_offset arg,
> `surf->ss6.raw_data - mcs_mt->region->bo->offset`.
>

Yeah, the hardware folks have made things difficult for us by squeezing
both an address and control information into a single dword.  They way they
got away with it is that the address is known by both Mesa and the GPU to
always lie on a page boundary; hence its lower 12 bits are always zero.
Therefore, we replace the lower 12 bits with control information.  So it
looks as though our address is some sort of crazy offset within the MCS
buffer, even though that "offset" is actually not referring to a memory
location.

If the MCS buffer doesn't get relocated, then everything works fine.  If
the MCS buffer *does* get relocated, then we need to tell the kernel
relocator to apply the same crazy offset to the new location of the MCS
buffer.  Fortunately the kernel always relocates things to page boundaries,
so the relocated address of the MCS buffer will still have its lower 12
bits zero.  That means that when the relocator applies the offset, the
lower 12 bits will still contain the same control information they had
before the relocation.

Would it be helpful to walk through an example?


>
> I tried to reconstruct what you may have intended, and I arrived at
> `(surf->ss6.mcs_enabled.mcs_base_address << 12) -
> mcs_mt->region->bo->offset`.
> But that just evaluates to a 0 offset, and I became confused again as to
> why you
> would want to calculate 0 in such roundabout way.
>

This value is the "crazy offset" I was referring to earlier; the one that
secretly contains the control information.  Would it help if I used the
alternative formulation "surf->ss6.raw_data & 0xfff"?  That might clarify
what I'm trying to achieve, and why the value is not expected to be zero.


>
> What's happening there?
>

> ----
> Chad Versace
> chad.versace at linux.intel.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120711/66a70fa8/attachment.html>