On 02/14/2018 09:38 AM, Michal Wajdeczko wrote:
On Wed, 14 Feb 2018 03:26:12 +0100, Yaodong Li <yaodong...@intel.com> wrote:

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)
+------------------+ <=== 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:-)).

Ok, so we should program GUC_WOPCM_SIZE to wopcm->guc.size and then
use (wopcm->size - wopcm->guc.base) as offset to i915_vma_pin, right?

All values of size and top are relevant to guc.base (offsets from guc.base), and currently we are using guc.size + hole between guc.size and ctx reserved (if any) + ctx reserved size
as the offset to i915_vma_pin as it's the actual WOPCM_TOP.:-)


Intel-gfx mailing list

Reply via email to