On pe, 2016-08-19 at 14:13 +0530, akash.goel@xxxxxxxxx wrote:
> From: Akash Goel <akash.goel@xxxxxxxxx>
>
> The GuC log buffer flush work item has to do a register access to send the
> ack to GuC and this work item, if not synced before suspend, can potentially
> get executed after the GFX device is suspended. This work item function uses
> rpm get/put calls around the Hw access, which covers the rpm suspend case
> but for system suspend a sync would be required as kernel can potentially
> schedule the work items even after some devices, including GFX, have been
> put to suspend. But sync has to be done only for the system suspend case,
> as sync along with rpm get/put can cause a deadlock for rpm suspend path.
> To have the sync, but like a NOOP, for rpm suspend path also this work
> item could have been queued from the irq handler only when the device is
> runtime active & kept active while that work item is pending or getting
> executed but an interrupt can come even after the device is out of use and
> so can potentially lead to missing of this work item.
>
> By marking the workqueue, dedicated for handling GuC log buffer flush
> interrupts, as freezable we don't have to bother about flushing of this
> work item from the suspend hooks, the pending work item if any will be
> either executed before the suspend or scheduled later on resume. This way
> the handling of log buffer flush work item can be kept same between system
> suspend & rpm suspend.
>
> Suggested-by: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
Thanks for taking the time, looks good to me:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index d4a1c84..bb25404 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1242,8 +1242,19 @@ static int guc_create_log_extras(struct intel_guc *guc)
>
> /* Need a dedicated wq to process log buffer flush interrupts
> * from GuC without much delay so as to avoid any loss of logs.
> + *
> + * GuC log buffer flush work item has to do register access to
> + * send the ack to GuC and this work item, if not synced before
> + * suspend, can potentially get executed after the GFX device is
> + * suspended.
> + * By marking the WQ as freezable, we don't have to bother about
> + * flushing of this work item from the suspend hooks, the pending
> + * work item if any will be either executed before the suspend
> + * or scheduled later on resume. This way the handling of work
> + * item can be kept same between system suspend & rpm suspend.
> */
> - guc->log.flush_wq = alloc_ordered_workqueue("i915-guc_log", 0);
> + guc->log.flush_wq =
> + alloc_ordered_workqueue("i915-guc_log", WQ_FREEZABLE);
> if (guc->log.flush_wq == NULL) {
> DRM_ERROR("Couldn't allocate the wq for GuC logging\n");
> return -ENOMEM;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
