[Intel-gfx] [10/11] drm/i915/guc: Refactor the concept "GuC context descriptor" into "GuC stage descriptor"

[Chris Wilson]

On Fri, Mar 10, 2017 at 08:28:57AM -0800, Oscar Mateo wrote:
> A GuC context and a HW context are in no way related, so the name "GuC context descriptor"
> is very unfortunate, because a new reader of the code gets overwhelmed very quickly with
> a lot of things called "context" that refer to different things. We can improve legibility
> a lot by simply renaming a few objects in the GuC code.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> ---
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index a912ec4..e320008 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -35,16 +35,18 @@
>   * descriptor and a workqueue (all of them inside a single gem object that
>   * contains all required pages for these elements).
>   *
> - * GuC context descriptor:
> - * During initialization, the driver allocates a static pool of 1024 such
> - * descriptors, and shares them with the GuC.
> + * GuC stage descriptor:
> + * Technically, this is known as a "GuC Context" descriptor in the specs, but
> + * we use the term "stage" to avoid confusion with all the other things already
> + * named "context" in the driver. During initialization, the driver allocates
> + * a static pool of 1024 such descriptors, and shares them with the GuC.
>   * Currently, there exists a 1:1 mapping between a i915_guc_client and a
> - * guc_context_desc (via the client's context_index), so effectively only
> - * one guc_context_desc gets used. This context descriptor lets the GuC know
> - * about the doorbell, workqueue and process descriptor. Theoretically, it also
> - * lets the GuC know about our HW contexts (Context ID, etc...), but we actually
> + * guc_stage_desc (via the client's stage_id), so effectively only one
> + * gets used. This stage descriptor lets the GuC know about the doorbell,
> + * workqueue and process descriptor. Theoretically, it also lets the GuC
> + * know about our HW contexts (context ID, etc...), but we actually
>   * employ a kind of submission where the GuC uses the LRCA sent via the work
> - * item instead (the single guc_context_desc associated to execbuf client
> + * item instead (the single guc_stage_desc associated to execbuf client
>   * contains information about the default kernel context only, but this is
>   * essentially unused). This is called a "proxy" submission.

Some of the above (esp. "this is known as a "GuC Context" descriptor in
the specs") would be good here. Some might say this is where we expect
the kerneldoc for structs to be...

>  /*Context descriptor for communicating between uKernel and Driver*/
> -struct guc_context_desc {
> +struct guc_stage_desc {
>  	u32 sched_common_area;
> -	u32 context_id;
> +	u32 stage_id;
>  	u32 pas_id;
>  	u8 engines_used;
>  	u64 db_trigger_cpu;
> @@ -359,7 +359,7 @@ struct guc_policy {
>  } __packed;

-- 
Chris Wilson, Intel Open Source Technology Centre