Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission

["Michal Wajdeczko" <michal.wajdeczko@intel.com>]

On Mon, 28 Oct 2019 22:25:27 +0100, <don.hiatt@intel.com> wrote:

> From: Don Hiatt <don.hiatt@intel.com>
>
> On some platforms (e.g. KBL) that do not support GuC submission, but
> the user enabled the GuC communication (e.g for HuC authentication)
> calling the GuC EXIT_S_STATE action results in lose of ability to
> enter RC6. We can remove the GuC suspend/remove entirely as we do
> not need to save the GuC submission status.
>
> v2: Do not suspend/resume the GuC on platforms that do not support
>     Guc Submission.
>
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c  
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 3fdbc935d155..04031564f0b1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -572,10 +572,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
>  	if (!intel_guc_is_running(guc))
>  		return;
> +	/*
> +	 * If GuC communciation is enabled but submission is not supported,

typo

> +	 * we do not need to suspend the GuC but we do need to disable the
> +	 * GuC communication on suspend.
> +	 */
> +	if (!guc->submission_supported)

Using submission_supported flag directly can be tricky, as today it
is always set to false, but in the future it may indicate either that
submission is supported by the driver/fw and/or enabled by modparam.

There is no guarantee that it will reflect actual runtime status,
as even supported/unblocked guc submission may fallback to execlists.

We may need something like intel_guc_submission_is_active() that will
reflect actual mode of submission currently used by the driver.

> +		goto guc_disable_comm;

and maybe we can move above logic to intel_guc_suspend()
to do not introduce extra goto's ?

> +
>  	err = intel_guc_suspend(guc);
>  	if (err)
>  		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
> +guc_disable_comm:
>  	guc_disable_communication(guc);
>  }
> @@ -605,6 +614,14 @@ static int __uc_resume(struct intel_uc *uc, bool  
> enable_communication)
>  	if (enable_communication)
>  		guc_enable_communication(guc);
> +	/*
> +	 * If GuC communciation is enabled but submission is not supported,

typo

> +	 * we do not need to resume the GuC but we do need to enable the
> +	 * GuC communication on resume (above).
> +	 */
> +	if (!guc->submission_supported)
> +		return 0;

see suspend case comment

> +
>  	err = intel_guc_resume(guc);
>  	if (err) {
>  		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);

Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx