On 03/13/2017 04:49 AM, Chris Wilson wrote:
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;
Fair enough, I'll move it. Thanks,
-- Oscar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
