| Message ID | 1521053083-2693-1-git-send-email-yaodong.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 14 Mar 2018 19:44:43 +0100, Jackie Li <yaodong.li@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@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@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
On 03/14/2018 12:26 PM, Michal Wajdeczko wrote: > On Wed, 14 Mar 2018 19:44:43 +0100, Jackie Li <yaodong.li@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@intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@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
Are we required to add reference to intel_guc.c and intel_wopcm.c in Documentation/gpu/i915.rst? On 3/15/2018 12:14 AM, Jackie Li 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@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@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 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. > */ > > /** > diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c > index 4117886..74bf76f 100644 > --- a/drivers/gpu/drm/i915/intel_wopcm.c > +++ b/drivers/gpu/drm/i915/intel_wopcm.c > @@ -11,28 +11,30 @@ > * DOC: WOPCM Layout > * > * The layout of the WOPCM will be fixed after writing to GuC WOPCM size and > - * offset registers whose are calculated are determined by size of HuC/GuC > - * firmware size and set of hw requirements/restrictions as shown below: > + * offset registers whose values are calculated and determined by HuC/GuC > + * firmware size and set of hardware requirements/restrictions as shown below: > * > - * +=========> +====================+ <== WOPCM Top > - * ^ | HW contexts RSVD | > - * | +===> +====================+ <== GuC WOPCM Top > - * | ^ | | > - * | | | | > - * | | | | > - * | GuC | | > - * | WOPCM | | > - * | Size +--------------------+ > - * WOPCM | | GuC FW RSVD | > - * | | +--------------------+ > - * | | | GuC Stack RSVD | > - * | | +------------------- + > - * | v | GuC WOPCM RSVD | > - * | +===> +====================+ <== GuC WOPCM base > - * | | WOPCM RSVD | > - * | +------------------- + <== HuC Firmware Top > - * v | HuC FW | > - * +=========> +====================+ <== WOPCM Base > + * :: > + * > + * +=========> +====================+ <== WOPCM Top > + * ^ | HW contexts RSVD | > + * | +===> +====================+ <== GuC WOPCM Top > + * | ^ | | > + * | | | | > + * | | | | > + * | GuC | | > + * | WOPCM | | > + * | Size +--------------------+ > + * WOPCM | | GuC FW RSVD | > + * | | +--------------------+ > + * | | | GuC Stack RSVD | > + * | | +------------------- + > + * | v | GuC WOPCM RSVD | > + * | +===> +====================+ <== GuC WOPCM base > + * | | WOPCM RSVD | > + * | +------------------- + <== HuC Firmware Top > + * v | HuC FW | > + * +=========> +====================+ <== WOPCM Base > * > * GuC accessible WOPCM starts at GuC WOPCM base and ends at GuC WOPCM top. > * The top part of the WOPCM is reserved for hardware contexts (e.g. RC6
On 03/14/2018 11:54 PM, Sagar Arun Kamble wrote: > Are we required to add reference to intel_guc.c and intel_wopcm.c in > Documentation/gpu/i915.rst? > hmm, I have the same question too:-). Can I modify the i915.rst to include kernel-doc from WOPCM and GuC WOPCM related changes? or someone would decide what contents should be included in i915 kernel doc? From my point of view, I think it would make the kernel-doc more comprehensible (at least for the wopcm part) if these diagrams were exported as a part of kernel-doc. Thanks, -Jackie
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 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. */ /** diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c index 4117886..74bf76f 100644 --- a/drivers/gpu/drm/i915/intel_wopcm.c +++ b/drivers/gpu/drm/i915/intel_wopcm.c @@ -11,28 +11,30 @@ * DOC: WOPCM Layout * * The layout of the WOPCM will be fixed after writing to GuC WOPCM size and - * offset registers whose are calculated are determined by size of HuC/GuC - * firmware size and set of hw requirements/restrictions as shown below: + * offset registers whose values are calculated and determined by HuC/GuC + * firmware size and set of hardware requirements/restrictions as shown below: * - * +=========> +====================+ <== WOPCM Top - * ^ | HW contexts RSVD | - * | +===> +====================+ <== GuC WOPCM Top - * | ^ | | - * | | | | - * | | | | - * | GuC | | - * | WOPCM | | - * | Size +--------------------+ - * WOPCM | | GuC FW RSVD | - * | | +--------------------+ - * | | | GuC Stack RSVD | - * | | +------------------- + - * | v | GuC WOPCM RSVD | - * | +===> +====================+ <== GuC WOPCM base - * | | WOPCM RSVD | - * | +------------------- + <== HuC Firmware Top - * v | HuC FW | - * +=========> +====================+ <== WOPCM Base + * :: + * + * +=========> +====================+ <== WOPCM Top + * ^ | HW contexts RSVD | + * | +===> +====================+ <== GuC WOPCM Top + * | ^ | | + * | | | | + * | | | | + * | GuC | | + * | WOPCM | | + * | Size +--------------------+ + * WOPCM | | GuC FW RSVD | + * | | +--------------------+ + * | | | GuC Stack RSVD | + * | | +------------------- + + * | v | GuC WOPCM RSVD | + * | +===> +====================+ <== GuC WOPCM base + * | | WOPCM RSVD | + * | +------------------- + <== HuC Firmware Top + * v | HuC FW | + * +=========> +====================+ <== WOPCM Base * * GuC accessible WOPCM starts at GuC WOPCM base and ends at GuC WOPCM top. * The top part of the WOPCM is reserved for hardware contexts (e.g. RC6
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@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@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(-)