Re: [Intel-gfx] [PATCH 18/19] drm/i915/guc: Support OA when Wa_16011777198 is enabled

Dixit, Ashutosh
2022-09-16T21:41:10Z

On Tue, 23 Aug 2022 13:41:54 -0700, Umesh Nerlige Ramappa wrote:
>
> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
> There is a w/a to reset RCS/CCS before it goes into RC6. This breaks
> OA. Fix it by disabling RC6.

Need to mention DG2 in the commit message?

> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
> .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h | 9 ++++
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 45 +++++++++++++++++++
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 2 +
> drivers/gpu/drm/i915/i915_perf.c | 29 ++++++++++++
> 4 files changed, 85 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
> index 4c840a2639dc..811add10c30d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
> @@ -128,6 +128,15 @@ enum slpc_media_ratio_mode {
> SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO = 2,
> };
>
> +enum slpc_gucrc_mode {
> + SLPC_GUCRC_MODE_HW = 0,
> + SLPC_GUCRC_MODE_GUCRC_NO_RC6 = 1,
> + SLPC_GUCRC_MODE_GUCRC_STATIC_TIMEOUT = 2,
> + SLPC_GUCRC_MODE_GUCRC_DYNAMIC_HYSTERESIS = 3,
> +
> + SLPC_GUCRC_MODE_MAX,
> +};
> +
> enum slpc_event_id {
> SLPC_EVENT_RESET = 0,
> SLPC_EVENT_SHUTDOWN = 1,
> 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 e1fa1f32f29e..23989f5452a7 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -642,6 +642,51 @@ static void slpc_get_rp_values(struct intel_guc_slpc *slpc)
> slpc->boost_freq = slpc->rp0_freq;
> }
>
> +/**
> + * intel_guc_slpc_override_gucrc_mode() - override GUCRC mode
> + * @slpc: pointer to intel_guc_slpc.
> + * @mode: new value of the mode.
> + *
> + * This function will override the GUCRC mode.
> + *
> + * Return: 0 on success, non-zero error code on failure.
> + */
> +int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode)
> +{
> + int ret;
> + struct drm_i915_private *i915 = slpc_to_i915(slpc);
> + intel_wakeref_t wakeref;
> +
> + if (mode >= SLPC_GUCRC_MODE_MAX)
> + return -EINVAL;
> +
> + wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> + ret = slpc_set_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE, mode);
> + if (ret)
> + drm_err(&i915->drm,
> + "Override gucrc mode %d failed %d\n",
> + mode, ret);
> +
> + intel_runtime_pm_put(&i915->runtime_pm, wakeref);

nit but I think let's switch to with_intel_runtime_pm() since all other
slpc functions use that.

> +
> + return ret;
> +}
> +
> +int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc)
> +{
> + struct drm_i915_private *i915 = slpc_to_i915(slpc);
> + int ret = 0;
> +
> + ret = slpc_unset_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE);

Looks like slpc_unset_param() is not present so that needs to be added to
the patch too, otherwise probably doesn't even compile.

> + if (ret)
> + drm_err(&i915->drm,
> + "Unsetting gucrc mode failed %d\n",
> + ret);
> +
> + return ret;
> +}
> +
> /*
> * intel_guc_slpc_enable() - Start SLPC
> * @slpc: pointer to intel_guc_slpc.
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> index 82a98f78f96c..ccf483730d9d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -42,5 +42,7 @@ int intel_guc_slpc_set_media_ratio_mode(struct intel_guc_slpc *slpc, u32 val);
> void intel_guc_pm_intrmsk_enable(struct intel_gt *gt);
> void intel_guc_slpc_boost(struct intel_guc_slpc *slpc);
> void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc);
> +int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc);
> +int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 132c2ce8b33b..ce1b6ad4d107 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -208,6 +208,7 @@
> #include "gt/intel_lrc.h"
> #include "gt/intel_lrc_reg.h"
> #include "gt/intel_ring.h"
> +#include "gt/uc/intel_guc_slpc.h"
>
> #include "i915_drv.h"
> #include "i915_file_private.h"
> @@ -1651,6 +1652,16 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>
> free_oa_buffer(stream);
>
> + /*
> + * Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
> + */
> + if (intel_guc_slpc_is_used(&gt->uc.guc) &&
> + intel_uc_uses_guc_rc(&gt->uc) &&
> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)))

Do these steppings need to be tweaked, otherwise ok as is too.