Re: [PATCH v10 3/7] drm/i915/guc: Implement dynamic GuC WOPCM offset and size

[Yaodong Li <yaodong.li@intel.com>]

On 02/13/2018 07:56 AM, Michal Wajdeczko wrote:
>
> Also, comparing again your drawing with the spec I think it
> should look like this:
>
>    +-------- +====================+ <== WOPCM top
>   /|\        |     HW RESERVED    |
>    |         +------------------- +
>    |         |                    |
>    |     +-- +====================+ <== GuC WOPCM top
>    |    /|\  |                    |
>    |     |   |                    |
>    |    GuC  |                    |
>    |   WOPCM |    GuC firmware    |
>    |    size |                    |
>  WOPCM   |   +--------------------+
>   size  \|/  |    GuC reserved    |
>    |     +-- +====================+ <== GuC WOPCM base (offset from 
> WOPCM base)
>    |         |   WOPCM RESERVED   |
>    |         +--------------------+
>   \|/        |    HuC firmware    |
>    +-------- +====================+ <== WOPCM base
>
>
>> +
>> +/* GUC WOPCM offset needs to be 16KB aligned. */
>> +#define GUC_WOPCM_OFFSET_ALIGNMENT    (16 << 10)
>> +/* 16KB reserved at the beginning of GuC WOPCM. */
>> +#define GUC_WOPCM_RESERVED        (16 << 10)
>> +/* 8KB from GUC_WOPCM_RESERVED is reserved for GuC stack. */
>> +#define GUC_WOPCM_STACK_RESERVED    (8 << 10)
>> +/* 24KB at the end of GuC WOPCM is reserved for RC6 CTX on BXT. */
>> +#define BXT_GUC_WOPCM_RC6_CTX_RESERVED    (24 << 10)
>
> Hmm, I'm still not convinced that we correctly attach it to "GuC WOPCM".
>
> In the result, maybe we should stop focusing on "intel_guc_wopcm" and 
> define
> everything as "intel_wopcm" as it was earlier suggested by Joonas.
>
> Then we can define our struct to match diagram above as
>
> struct intel_wopcm {
>     u32 size;
>     struct {
>         u32 base;
>         u32 size;
>     } guc;
> };
>
> u32 intel_wopcm_guc_top(struct intel_wopcm *wopcm)
> {
>     return wopcm->guc.base + wopcm->guc.size;
> }
More comments about the *top*:-)

The *top* we used in driver code to verify where the non-wopcm guc memory
allocation starts is the offset value from guc.base to WOPCM_TOP. so we 
don't need to
add the guc.base to it.

 From GuC point of view the *top* (boundary between wopcm and non-wopcm 
memory)
is like:
+------------------+ <=== GuC memory end

    Non WOPCM

+------------------+ <=== *top* (This is actually in top of the whole WOPCM)
      CTX Rsvd
+------------------+ <=== *__top*  (wopcm->guc.size)
       WOPCM
+------------------+ <=== guc base (wopcm->guc.base)

actually, we had a discussion whether we should set the guc wopcm and 
non-wopcm
boundary to *__top* but the suggestion we got is to stick to the spec 
description which
says to allocate "non-wopcm memory starts from WOPCM TOP"(even if it seems
we could use *__top* as the boundary:-)). and it's because of this 
reason we called the
wopcm from guc_base to *top* as GuC WOPCM since we count the wopcm between
*__top* to *top* as a part of GuC address space while trying to allocate 
non-wopcm
for GuC (even though GuC won't access CTX Rsvd at allO:-)).

Regards,
-Jackie
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx