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@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
