On 03/14/2018 12:26 PM, Michal Wajdeczko wrote:
On Wed, 14 Mar 2018 19:44:43 +0100, Jackie Li <yaodong...@intel.com> wrote:

GuC Address Space and WOPCM Layout diagrams won't be generated correctly by
sphinx build if not using proper reST syntax.

This patch uses reST literal blocks to make sure GuC Address Space and
WOPCM Layout diagrams to be generated correctly, and it also corrects some
errors in the diagram description.

v2:
 - Fixed errors in diagram description

Signed-off-by: Jackie Li <yaodong...@intel.com>
Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kam...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c   | 52 ++++++++++++++++++++------------------  drivers/gpu/drm/i915/intel_wopcm.c | 44 +++++++++++++++++---------------
 2 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 3eb516e..6a4f36e 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -495,35 +495,37 @@ int intel_guc_resume(struct intel_guc *guc)
 /**
  * DOC: GuC Address Space
  *
- * The layout of GuC address space is shown as below:
+ * The layout of GuC address space is shown below:
  *
- *    +==============> +====================+ <== GUC_GGTT_TOP
- *    ^                |                    |
- *    |                |                    |
- *    |                |        DRAM        |
- *    |                |       Memory       |
- *    |                |                    |
- *   GuC               |                    |
- * Address  +========> +====================+ <== WOPCM Top
- *  Space   ^          |   HW contexts RSVD |
- *    |     |          |        WOPCM       |
- *    |     |     +==> +--------------------+ <== GuC WOPCM Top
- *    |    GuC    ^    |                    |
- *    |    GGTT   |    |                    |
- *    |    Pin   GuC   |        GuC         |
- *    |    Bias WOPCM  |       WOPCM        |
- *    |     |    Size  |                    |
- *    |     |     |    |                    |
- *    v     v     v    |                    |
- *    +=====+=====+==> +====================+ <== GuC WOPCM Base
- *                     |   Non-GuC WOPCM    |
- *                     |   (HuC/Reserved)   |
- *                     +====================+ <== WOPCM Base
+ * ::
+ *
+ *     +==============> +====================+ <== GUC_GGTT_TOP
+ *     ^                |                    |
+ *     |                |                    |
+ *     |                |        DRAM        |
+ *     |                |       Memory       |
+ *     |                |                    |
+ *    GuC               |                    |
+ *  Address  +========> +====================+ <== WOPCM Top
+ *   Space   ^          |   HW contexts RSVD |
+ *     |     |          |        WOPCM       |
+ *     |     |     +==> +--------------------+ <== GuC WOPCM Top
+ *     |    GuC    ^    |                    |
+ *     |    GGTT   |    |                    |
+ *     |    Pin   GuC   |        GuC         |
+ *     |    Bias WOPCM  |       WOPCM        |
+ *     |     |    Size  |                    |
+ *     |     |     |    |                    |
+ *     v     v     v    |                    |
+ *     +=====+=====+==> +====================+ <== GuC WOPCM Base
+ *                      |   Non-GuC WOPCM    |
+ *                      |   (HuC/Reserved)   |
+ *                      +====================+ <== WOPCM Base
  *
  * The lower part [0, GuC ggtt_pin_bias) is mapped to WOPCM which consists of
  * GuC WOPCM and WOPCM reserved for other usage (e.g.RC6 context). The

hmm, "lower part [...)" is little ambiguous here, as one may look for
"upper part [...)", so maybe better to be explicit:

    "The lower part of GuC Address Space ie. [0, ggtt_pin_bias)
    is mapped to WOPCM, while upper part of GuC Address Space ie.
    [ggtt_pin_bias, GUC_GGTT_TOP) is mapped to DRAM."

Agree. it's more clear in this way. but I think we should remove "ie." update it to "The lower part of GuC Address Space [0, ggtt_pin_bias) is mapped to WOPCM"?
value of
- * the GuC ggtt_pin_bias is determined by the actually GuC WOPCM size which is
- * set in GUC_WOPCM_SIZE register.
+ * the GuC ggtt_pin_bias is determined by the GuC WOPCM size which is set in
+ * GUC_WOPCM_SIZE register.
  */

hmm, I'm not sure that above statement is correct, compare your diagram
and calculation:

    guc->ggtt_pin_bias = i915->wopcm.size - i915->wopcm.guc.base;

also I would not mention registers here, as we don't read them while
calculating bias, so maybe something like this:

    "The value of ggtt_pin_bias is determined by the WOPCM size and
    actual GuC WOPCM base."

Thanks. Will update it.

Regards,
-Jackie

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

Reply via email to