On 11/16/2017 12:00 AM, 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.

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.
Planning to change this in upcoming series (allocate doorbells on resume when not needing uc_init_hw) and then we can do away with this validation. Another problem I see is, this is time consuming and leads to increase in the resume time (we also sanitize on resume hence this is unnecessary for all unused doorbells)
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.thie...@intel.com>
Change looks good to me.
Acked-by: Sagar Arun Kamble <sagar.a.kam...@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kam...@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundare...@intel.com>
Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
Cc: Oscar Mateo <oscar.ma...@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
---
  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to