| Message ID | 20180327203557.64484-3-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <sagar.a.kamble@intel.com> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Michal Winiarski <michal.winiarski@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> This looks good. Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> 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); > } > > /**
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); } /**
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 <sagar.a.kamble@intel.com> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Michal Winiarski <michal.winiarski@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_guc_submission.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)