On 15/11/17 10:30, Michel Thierry wrote:
During driver load we create the GuC clients and allocate their
doorbells just before executing guc_init_doorbell_hw; but since we just
created these doorbells, how can they be out of sync?
This code has had more than enough refactoring (2 more still in progress)
so I would not be surprised if calling guc_init_doorbell_hw made sense at
some point, but not anymore.
I think the idea was to clean up the unallocated doorbells on takeover
to be covered in case the previous occupant of the GPU didn't release
them when leaving the HW. We do a full gpu reset during i915 load now in
i915_gem_sanitize so the doorbell HW should be cleaned up by that, but
there is still a possible issue when i915.reset=0. However, with reset=0
this wouldn't be the only thing not sanitized and the only bad
consequence would be extra irqs to GuC (which would be ignored), so I
don't think it is worth worrying about that case.
Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Daniele
The resume path is different, in this case the driver doesn't
recreate clients, and it is still reasonable to validate/reallocate the
doorbells in order to confirm that they still belong to the clients.
And probably guc_init_doorbell_hw is no longer the right name, but I'll
leave that to someone else.
Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 5d6576e01a91..d6762ca42cf1 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1424,16 +1424,16 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
} else {
guc_reset_wq(guc->execbuf_client);
guc_reset_wq(guc->preempt_client);
+
+ err = guc_init_doorbell_hw(guc);
+ if (err)
+ goto err_free_clients;
}
err = intel_guc_sample_forcewake(guc);
if (err)
goto err_free_clients;
- err = guc_init_doorbell_hw(guc);
- if (err)
- goto err_free_clients;
-
/* Take over from manual control of ELSP (execlists) */
guc_interrupts_capture(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
