[Intel-gfx] [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1

[O'Rourke, Tom]

On Tue, Jul 28, 2015 at 04:16:03PM +0100, Dave Gordon wrote:
> On 28/07/15 00:12, O'Rourke, Tom wrote:
> >On Mon, Jul 27, 2015 at 03:41:31PM -0700, Yu Dai wrote:
> >>
> >>On 07/24/2015 03:31 PM, O'Rourke, Tom wrote:
> >>>[TOR:] When I see "phase 1" I also look for "phase 2".
> >>>A subject that better describes the change in this patch
> >>>would help.
> >>>
> >>>On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote:
> >>>>From: Alex Dai <yu.dai at intel.com>
> >>>>
> >>>>This adds the first of the data structures used to communicate with the
> >>>>GuC (the pool of guc_context structures).
> >>>>
> >>>>We create a GuC-specific wrapper round the GEM object allocator as all
> >>>>GEM objects shared with the GuC must be pinned into GGTT space at an
> >>>>address that is NOT in the range [0..WOPCM_SIZE), as that range of GGTT
> >>>>addresses is not accessible to the GuC (from the GuC's point of view,
> >>>>it's permanently reserved for other objects such as the BootROM & SRAM).
> >>>[TOR:] I would like a clarfication on the excluded range.
> >>>The excluded range should be 0 to "size for guc within
> >>>WOPCM area" and not 0 to "size of WOPCM area".
> >>
> >>Nope, GGTT range [0..WOPCM_SIZE) should be excluded from GuC usage.
> >>BSpec clearly says, from 0 to WOPCM_TOP-1 is for BootROM, SRAM and
> >>WOPCM. From WOPCM_TOP and above is GFX DRAM. Be note that, that GGTT
> >>space is still available to any gfx obj as long as it is not
> >>accessed by GuC (OK to pass through GuC).
> >>
> >[TOR:] Should we take a closer look at the pin offset bias
> >for guc objects?  GUC_WOPCM_SIZE_VALUE is not the full size
> >of WOPCM area.
> 
> I'm inclined to set the bias to GUC_WOPCM_TOP, and then define that
> as the sum of GUC_WOPCM_OFFSET_VALUE and GUC_WOPCM_SIZE_VALUE. That
> seems to be what the BSpec pages "WriteOnceProtectedContentMemory
> (WOPCM) Management" and "WOPCM Memory Map" suggest, although I think
> they're pretty unclear on the details :(
> 
> Do you (both) agree this would be the right value?

[TOR:] No, I do not think that is the right value.

I think the excluded range should be [0 ... GUC_WOPCM_SIZE_VALUE)
and that GUC_WOPCM_SIZE_VALUE should be used as the bias (as it
is now) for objects used by GuC.

The term "WOPCM_SIZE" is ambiguous since it could mean
GUC_WOPCM_SIZE (as in 0xc050) or it could mean "size of
WOPCM area" (as in 0x1082C0).  It gets used both ways
in the BSpec.

> 
> [snip]
[snip]