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

[Chad Versace]

On 07/11/2012 10:31 AM, Paul Berry wrote:
> On 11 July 2012 09:50, Chad Versace <chad.versace at linux.intel.com
> <mailto:chad.versace at linux.intel.com>> wrote:

>     > +
>     > +   /* 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 <http://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.

Ah, I get it now. And yes, `surf->ss6.raw_data & 0xfff` more clearly reveals the
intent, in my opinion. It says, with no ambiguity, "preserve the lower 12 bits!".

With 0xfff or not, this is now
Reviewed-by: Chad Versace <chad.versace at linux.intel.com>