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

O'Rourke, Tom
2015-07-28T22:46:33Z

On Tue, Jul 28, 2015 at 08:40:58PM +0100, Dave Gordon wrote:
> On 28/07/15 16:16, 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@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?
>
> Actually I've changed my mind (again). On rereading this stuff, I
> now think that GUC_WOPCM_TOP is the same as the value put into the
> SIZE register. The (physical) range between the (real) WOPCM BASE
> and that plus the GUC WOPCM OFFSET isn't part of the GuC address
> space at all, so GuC address 0 maps (would map) to (real WOPCM
> BASE+GUC WOPCM OFFSET) in physical addresses, except that the bottom
> 80k is shadowed by the bootrom and SRAM; and then the SIZE register
> defines the size of the range from (GuC address) 0 to GUC_WOPCM_TOP;
> and then higher addresses map through the GTT as expected.
>
> Or so I think. Does anyone know for sure? Please let me know ASAP as
> I want to submit an updated patchset tomorrow!
>
> Thanks,
> .Dave.

[TOR:] Hi Dave, Sorry, I did not see your message earlier.
Please see my other reply on this thread. I think you are
right here, but to be clear, I think by "SIZE" you mean
"GUC_WOPCM_SIZE_VALUE".

Also, this should not matter here, but the SKL guc SRAM
shadow is 128k, not 80k.

Thanks,
Tom