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

On Sat, 14 May 2022, Vinay Belgaumkar vinay.belgaumkar@intel.com wrote:

SLPC min/max frequency updates require H2G calls. We are seeing timeouts when GuC channel is backed up and it is unable to respond in a timely fashion causing warnings and affecting CI.

This is seen when waitboosting happens during a stress test. this patch updates the waitboost path to use a non-blocking H2G call instead, which returns as soon as the message is successfully transmitted.

v2: Use drm_notice to report any errors that might occur while sending the waitboost H2G request (Tvrtko)

Signed-off-by: Vinay Belgaumkar vinay.belgaumkar@intel.com

drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 44 +++++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c index 1db833da42df..e5e869c96262 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c @@ -98,6 +98,30 @@ static u32 slpc_get_state(struct intel_guc_slpc *slpc) return data->header.global_state; }

+static int guc_action_slpc_set_param_nb(struct intel_guc *guc, u8 id, u32 value) +{

• u32 request[] = {

static const

• GUC_ACTION_HOST2GUC_PC_SLPC_REQUEST,
  

• SLPC_EVENT(SLPC_EVENT_PARAMETER_SET, 2),
  

• id,
  

• value,
  

• };
• int ret;
• ret = intel_guc_send_nb(guc, request, ARRAY_SIZE(request), 0);
• return ret > 0 ? -EPROTO : ret;

+}

+static int slpc_set_param_nb(struct intel_guc_slpc *slpc, u8 id, u32 value) +{

• struct intel_guc *guc = slpc_to_guc(slpc);
• GEM_BUG_ON(id >= SLPC_MAX_PARAM);
• return guc_action_slpc_set_param_nb(guc, id, value);

+}

static int guc_action_slpc_set_param(struct intel_guc *guc, u8 id, u32 value) { u32 request[] = {

Ditto here, and the whole gt/uc directory seems to have tons of these u32 action/request array variables on stack, with the required initialization, that could be in rodata.

Please fix all of them.

BR, Jani.

@@ -208,12 +232,10 @@ 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);
  

• if (ret)
  

• 	i915_probe_error(i915, "Unable to force min freq to %u: %d",
  

• 			 freq, ret);
  

• /* Non-blocking request will avoid stalls */
  

• ret = slpc_set_param_nb(slpc,
  

• 			SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
  

• 			freq);
  

  }

  return ret;

@@ -222,6 +244,8 @@ static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq) static void slpc_boost_work(struct work_struct *work) { struct intel_guc_slpc *slpc = container_of(work, typeof(*slpc), boost_work);

• struct drm_i915_private *i915 = slpc_to_i915(slpc);

• int err;

  /*

  • Raise min freq to boost. It's possible that

@@ -231,8 +255,12 @@ static void slpc_boost_work(struct work_struct *work) */ mutex_lock(&slpc->lock); if (atomic_read(&slpc->num_waiters)) {

• slpc_force_min_freq(slpc, slpc->boost_freq);
  

• slpc->num_boosts++;
  

• err = slpc_force_min_freq(slpc, slpc->boost_freq);
  

• if (!err)
  

• 	slpc->num_boosts++;
  

• else
  

• 	drm_notice(&i915->drm, "Failed to send waitboost request (%d)\n",
  

• 		   err);
  

  } mutex_unlock(&slpc->lock);

}