[Intel-gfx] [PATCH v2] drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams

[Michal Wajdeczko]

On Wed, 14 Mar 2018 19:44:43 +0100, Jackie Li <yaodong.li at 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.li at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at 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."

> 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."

/Michal