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

Dave Gordon
2015-07-28T19:41:00Z

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.