Re: [PATCH] drm/i915/guc/slpc: Use non-blocking H2G for waitboost

From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/guc/slpc: Use non-blocking H2G for waitboost
Date: Wed, 22 Jun 2022 17:53:26 -0700	[thread overview]
Message-ID: <87edzgxks9.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <20220623003225.23301-1-vinay.belgaumkar@intel.com>

On Wed, 22 Jun 2022 17:32:25 -0700, Vinay Belgaumkar wrote:
>
> @@ -208,12 +232,14 @@ static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
>	 */
>
>	with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> -		ret = slpc_set_param(slpc,
> -				     SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> -				     freq);
> +		/* Non-blocking request will avoid stalls */
> +		ret = slpc_set_param_nb(slpc,
> +					SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> +					freq);
>		if (ret)
> -			i915_probe_error(i915, "Unable to force min freq to %u: %d",
> -					 freq, ret);
> +			drm_notice(&i915->drm,
> +				   "Failed to send set_param for min freq(%d): (%d)\n",
> +				   freq, ret);

I am still thinking if we should replace drm_notice() by i915_probe_error()
since drm_notice() will basically hide any issues of boost/de-boost's
getting dropped.

Another idea here might be to maintain a counter, say "slpc->failed_boosts"
which we increment each time slpc_set_param_nb() fails and dump that
counter via intel_guc_slpc_print_info().

Anyway for now this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>