Re: [Intel-gfx] [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup

2018-03-28 Thread Sagar Arun Kamble



On 3/28/2018 2:05 AM, Michal Wajdeczko wrote:

In commit 9192d4fb811e ("drm/i915/guc: Extract doorbell creation
from client allocation") we introduced asymmetry in doorbell cleanup
to avoid warnings due to failed communication with already reset GuC.
As we improved our reset/unload paths, we can restore symmetry in
doorbell cleanup, as GuC should be still active by now.

Suggested-by: Sagar Arun Kamble 
Signed-off-by: Michal Wajdeczko 
Cc: Sagar Arun Kamble 
Cc: Michal Winiarski 
Cc: Chris Wilson 

This looks good.
Reviewed-by: Sagar Arun Kamble 

We should extend this functionality further to consider cases when GuC 
doorbell deactivation flow is to be
invoked (when GuC is in sane state) and not to be invoked (when GuC is 
hung) as was prepared in patchset
https://patchwork.freedesktop.org/series/33209/ but that can be done 
after this series if we agree on need for such handling.


Thanks,
Sagar

---
  drivers/gpu/drm/i915/intel_guc_submission.c | 15 +++
  1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c 
b/drivers/gpu/drm/i915/intel_guc_submission.c
index 207cda0..26985d8 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -835,18 +835,9 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
  
  static void guc_clients_doorbell_fini(struct intel_guc *guc)

  {
-   /*
-* By the time we're here, GuC has already been reset.
-* Instead of trying (in vain) to communicate with it, let's just
-* cleanup the doorbell HW and our internal state.
-*/
-   if (guc->preempt_client) {
-   __destroy_doorbell(guc->preempt_client);
-   __update_doorbell_desc(guc->preempt_client,
-  GUC_DOORBELL_INVALID);
-   }
-   __destroy_doorbell(guc->execbuf_client);
-   __update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
+   if (guc->preempt_client)
+   destroy_doorbell(guc->preempt_client);
+   destroy_doorbell(guc->execbuf_client);
  }
  
  /**


--
Thanks,
Sagar

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup

2018-03-27 Thread Michal Wajdeczko
In commit 9192d4fb811e ("drm/i915/guc: Extract doorbell creation
from client allocation") we introduced asymmetry in doorbell cleanup
to avoid warnings due to failed communication with already reset GuC.
As we improved our reset/unload paths, we can restore symmetry in
doorbell cleanup, as GuC should be still active by now.

Suggested-by: Sagar Arun Kamble 
Signed-off-by: Michal Wajdeczko 
Cc: Sagar Arun Kamble 
Cc: Michal Winiarski 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c 
b/drivers/gpu/drm/i915/intel_guc_submission.c
index 207cda0..26985d8 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -835,18 +835,9 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
 
 static void guc_clients_doorbell_fini(struct intel_guc *guc)
 {
-   /*
-* By the time we're here, GuC has already been reset.
-* Instead of trying (in vain) to communicate with it, let's just
-* cleanup the doorbell HW and our internal state.
-*/
-   if (guc->preempt_client) {
-   __destroy_doorbell(guc->preempt_client);
-   __update_doorbell_desc(guc->preempt_client,
-  GUC_DOORBELL_INVALID);
-   }
-   __destroy_doorbell(guc->execbuf_client);
-   __update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
+   if (guc->preempt_client)
+   destroy_doorbell(guc->preempt_client);
+   destroy_doorbell(guc->execbuf_client);
 }
 
 /**
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx