| Message ID | 20220923201154.283784-15-umesh.nerlige.ramappa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DG2 OA support | expand |
On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: > > From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> Hi Umesh/Vinay, > @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, > intel_engine_pm_get(stream->engine); > intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); > > + /* > + * Wa_16011777198:dg2: GuC resets render as part of the Wa. This causes > + * OA to lose the configuration state. Prevent this by overriding GUCRC > + * mode. > + */ > + if (intel_guc_slpc_is_used(>->uc.guc) && > + intel_uc_uses_guc_rc(>->uc) && Is this condition above correct? E.g. what happens when: a. GuCRC is used but SLPC is not used? b. GuCRC is not used. Don't we need to disable RC6 in host based RC6 control? Do we need to worry about these cases? Or if we always expect both GuCRC and SLPC to be used on DG2 then I think let's get rid of these from the if condition and add a drm_err() if we see these not being used and OA is being enabled on DG2? Thanks. -- Ashutosh > + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || > + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { > + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, > + SLPC_GUCRC_MODE_GUCRC_NO_RC6); > + if (ret) { > + drm_dbg(&stream->perf->i915->drm, > + "Unable to override gucrc mode\n"); > + goto err_config; > + } > + } > + > ret = alloc_oa_buffer(stream); > if (ret) > goto err_oa_buf_alloc; > -- > 2.25.1 >
On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote: >On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: >> >> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > >Hi Umesh/Vinay, > >> @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, >> intel_engine_pm_get(stream->engine); >> intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); >> >> + /* >> + * Wa_16011777198:dg2: GuC resets render as part of the Wa. This causes >> + * OA to lose the configuration state. Prevent this by overriding GUCRC >> + * mode. >> + */ >> + if (intel_guc_slpc_is_used(>->uc.guc) && >> + intel_uc_uses_guc_rc(>->uc) && > >Is this condition above correct? E.g. what happens when: > >a. GuCRC is used but SLPC is not used? >b. GuCRC is not used. Don't we need to disable RC6 in host based RC6 > control? When using host based rc6, existing OA code is using forcewake and a reference to engine_pm to prevent rc6. Other questions, directing to @Vinay. Thanks, Umesh > >Do we need to worry about these cases? > >Or if we always expect both GuCRC and SLPC to be used on DG2 then I think >let's get rid of these from the if condition and add a drm_err() if we see >these not being used and OA is being enabled on DG2? > >Thanks. >-- >Ashutosh > >> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || >> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { >> + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, >> + SLPC_GUCRC_MODE_GUCRC_NO_RC6); >> + if (ret) { >> + drm_dbg(&stream->perf->i915->drm, >> + "Unable to override gucrc mode\n"); >> + goto err_config; >> + } >> + } >> + >> ret = alloc_oa_buffer(stream); >> if (ret) >> goto err_oa_buf_alloc; >> -- >> 2.25.1 >>
On 9/26/2022 11:19 AM, Umesh Nerlige Ramappa wrote: > On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote: >> On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: >>> >>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> >> >> Hi Umesh/Vinay, >> >>> @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct >>> i915_perf_stream *stream, >>> intel_engine_pm_get(stream->engine); >>> intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); >>> >>> + /* >>> + * Wa_16011777198:dg2: GuC resets render as part of the Wa. >>> This causes >>> + * OA to lose the configuration state. Prevent this by >>> overriding GUCRC >>> + * mode. >>> + */ >>> + if (intel_guc_slpc_is_used(>->uc.guc) && >>> + intel_uc_uses_guc_rc(>->uc) && >> >> Is this condition above correct? E.g. what happens when: >> >> a. GuCRC is used but SLPC is not used? Currently, we have no way to separate out GuC RC and SLPC. Both are on when guc submission is enabled/supported. So, the above check is kind of redundant anyways. Thanks, Vinay. >> b. GuCRC is not used. Don't we need to disable RC6 in host based RC6 >> control? > > When using host based rc6, existing OA code is using forcewake and a > reference to engine_pm to prevent rc6. Other questions, directing to > @Vinay. > > Thanks, > Umesh > >> >> Do we need to worry about these cases? >> >> Or if we always expect both GuCRC and SLPC to be used on DG2 then I >> think >> let's get rid of these from the if condition and add a drm_err() if >> we see >> these not being used and OA is being enabled on DG2? >> >> Thanks. >> -- >> Ashutosh >> >>> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || >>> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { >>> + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, >>> + SLPC_GUCRC_MODE_GUCRC_NO_RC6); >>> + if (ret) { >>> + drm_dbg(&stream->perf->i915->drm, >>> + "Unable to override gucrc mode\n"); >>> + goto err_config; >>> + } >>> + } >>> + >>> ret = alloc_oa_buffer(stream); >>> if (ret) >>> goto err_oa_buf_alloc; >>> -- >>> 2.25.1 >>>
On Mon, 26 Sep 2022 14:17:21 -0700, Belgaumkar, Vinay wrote: > > > On 9/26/2022 11:19 AM, Umesh Nerlige Ramappa wrote: > > On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote: > >> On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: > >>> > >>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > >> > >> Hi Umesh/Vinay, > >> > >>> @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct > >>> i915_perf_stream *stream, > >>> intel_engine_pm_get(stream->engine); > >>> intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); > >>> > >>> + /* > >>> + * Wa_16011777198:dg2: GuC resets render as part of the Wa. This > >>> causes > >>> + * OA to lose the configuration state. Prevent this by overriding > >>> GUCRC > >>> + * mode. > >>> + */ > >>> + if (intel_guc_slpc_is_used(>->uc.guc) && > >>> + intel_uc_uses_guc_rc(>->uc) && > >> > >> Is this condition above correct? E.g. what happens when: > >> > >> a. GuCRC is used but SLPC is not used? > > Currently, we have no way to separate out GuC RC and SLPC. Both are on when > guc submission is enabled/supported. So, the above check is kind of > redundant anyways. That is why I was suggesting changing the if check to an assert or drm_err. So looks like it will work with or without GuC RC, but if we are using GuC RC we should be using SLPC. So: if (GuC_RC && !SLPC) drm_err(); > Thanks, > > Vinay. > > >> b. GuCRC is not used. Don't we need to disable RC6 in host based RC6 > >> control? > > > > When using host based rc6, existing OA code is using forcewake and a > > reference to engine_pm to prevent rc6. Other questions, directing to > > @Vinay. > > > > Thanks, > > Umesh > > > >> > >> Do we need to worry about these cases? > >> > >> Or if we always expect both GuCRC and SLPC to be used on DG2 then I > >> think > >> let's get rid of these from the if condition and add a drm_err() if we > >> see > >> these not being used and OA is being enabled on DG2? > >> > >> Thanks. > >> -- > >> Ashutosh > >> > >>> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || > >>> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { > >>> + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, > >>> + SLPC_GUCRC_MODE_GUCRC_NO_RC6); > >>> + if (ret) { > >>> + drm_dbg(&stream->perf->i915->drm, > >>> + "Unable to override gucrc mode\n"); > >>> + goto err_config; > >>> + } > >>> + } > >>> + > >>> ret = alloc_oa_buffer(stream); > >>> if (ret) > >>> goto err_oa_buf_alloc; > >>> -- > >>> 2.25.1 > >>>
On Mon, Sep 26, 2022 at 04:28:44PM -0700, Dixit, Ashutosh wrote: >On Mon, 26 Sep 2022 14:17:21 -0700, Belgaumkar, Vinay wrote: >> >> >> On 9/26/2022 11:19 AM, Umesh Nerlige Ramappa wrote: >> > On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote: >> >> On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: >> >>> >> >>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> >> >> >> >> Hi Umesh/Vinay, >> >> >> >>> @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct >> >>> i915_perf_stream *stream, >> >>> intel_engine_pm_get(stream->engine); >> >>> intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); >> >>> >> >>> + /* >> >>> + * Wa_16011777198:dg2: GuC resets render as part of the Wa. This >> >>> causes >> >>> + * OA to lose the configuration state. Prevent this by overriding >> >>> GUCRC >> >>> + * mode. >> >>> + */ >> >>> + if (intel_guc_slpc_is_used(>->uc.guc) && >> >>> + intel_uc_uses_guc_rc(>->uc) && >> >> >> >> Is this condition above correct? E.g. what happens when: >> >> >> >> a. GuCRC is used but SLPC is not used? >> >> Currently, we have no way to separate out GuC RC and SLPC. Both are on when >> guc submission is enabled/supported. So, the above check is kind of >> redundant anyways. > >That is why I was suggesting changing the if check to an assert or >drm_err. So looks like it will work with or without GuC RC, but if we are >using GuC RC we should be using SLPC. So: > > if (GuC_RC && !SLPC) > drm_err(); I am little confused. What's the ask here? Should I just use one of these conditions? i.e. if (intel_guc_slpc_is_used(>->uc.guc)) ... Thanks, Umesh > >> Thanks, >> >> Vinay. >> >> >> b. GuCRC is not used. Don't we need to disable RC6 in host based RC6 >> >> control? >> > >> > When using host based rc6, existing OA code is using forcewake and a >> > reference to engine_pm to prevent rc6. Other questions, directing to >> > @Vinay. >> > >> > Thanks, >> > Umesh >> > >> >> >> >> Do we need to worry about these cases? >> >> >> >> Or if we always expect both GuCRC and SLPC to be used on DG2 then I >> >> think >> >> let's get rid of these from the if condition and add a drm_err() if we >> >> see >> >> these not being used and OA is being enabled on DG2? >> >> >> >> Thanks. >> >> -- >> >> Ashutosh >> >> >> >>> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || >> >>> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { >> >>> + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, >> >>> + SLPC_GUCRC_MODE_GUCRC_NO_RC6); >> >>> + if (ret) { >> >>> + drm_dbg(&stream->perf->i915->drm, >> >>> + "Unable to override gucrc mode\n"); >> >>> + goto err_config; >> >>> + } >> >>> + } >> >>> + >> >>> ret = alloc_oa_buffer(stream); >> >>> if (ret) >> >>> goto err_oa_buf_alloc; >> >>> -- >> >>> 2.25.1 >> >>>
On Tue, 27 Sep 2022 09:11:23 -0700, Umesh Nerlige Ramappa wrote: > > On Mon, Sep 26, 2022 at 04:28:44PM -0700, Dixit, Ashutosh wrote: > > On Mon, 26 Sep 2022 14:17:21 -0700, Belgaumkar, Vinay wrote: > >> > >> > >> On 9/26/2022 11:19 AM, Umesh Nerlige Ramappa wrote: > >> > On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote: > >> >> On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: > >> >>> > >> >>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > >> >> > >> >> Hi Umesh/Vinay, > >> >> > >> >>> @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct > >> >>> i915_perf_stream *stream, > >> >>> intel_engine_pm_get(stream->engine); > >> >>> intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); > >> >>> > >> >>> + /* > >> >>> + * Wa_16011777198:dg2: GuC resets render as part of the Wa. This > >> >>> causes > >> >>> + * OA to lose the configuration state. Prevent this by overriding > >> >>> GUCRC > >> >>> + * mode. > >> >>> + */ > >> >>> + if (intel_guc_slpc_is_used(>->uc.guc) && > >> >>> + intel_uc_uses_guc_rc(>->uc) && > >> >> > >> >> Is this condition above correct? E.g. what happens when: > >> >> > >> >> a. GuCRC is used but SLPC is not used? > >> > >> Currently, we have no way to separate out GuC RC and SLPC. Both are on when > >> guc submission is enabled/supported. So, the above check is kind of > >> redundant anyways. > > > > That is why I was suggesting changing the if check to an assert or > > drm_err. So looks like it will work with or without GuC RC, but if we are > > using GuC RC we should be using SLPC. So: > > > > if (GuC_RC && !SLPC) > > drm_err(); > > I am little confused. What's the ask here? Should I just use one of these > conditions? i.e. > > if (intel_guc_slpc_is_used(>->uc.guc)) I think let's just use intel_uc_uses_guc_rc and drop intel_guc_slpc_is_used. I am assuming if SLPC is somehow not on and we try to use SLPC set/unset param, some kind of error will be reported. > ... > > Thanks, > Umesh > > > > >> Thanks, > >> > >> Vinay. > >> > >> >> b. GuCRC is not used. Don't we need to disable RC6 in host based RC6 > >> >> control? > >> > > >> > When using host based rc6, existing OA code is using forcewake and a > >> > reference to engine_pm to prevent rc6. Other questions, directing to > >> > @Vinay. > >> > > >> > Thanks, > >> > Umesh > >> > > >> >> > >> >> Do we need to worry about these cases? > >> >> > >> >> Or if we always expect both GuCRC and SLPC to be used on DG2 then I > >> >> think > >> >> let's get rid of these from the if condition and add a drm_err() if we > >> >> see > >> >> these not being used and OA is being enabled on DG2? > >> >> > >> >> Thanks. > >> >> -- > >> >> Ashutosh > >> >> > >> >>> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || > >> >>> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { > >> >>> + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, > >> >>> + SLPC_GUCRC_MODE_GUCRC_NO_RC6); > >> >>> + if (ret) { > >> >>> + drm_dbg(&stream->perf->i915->drm, > >> >>> + "Unable to override gucrc mode\n"); > >> >>> + goto err_config; > >> >>> + } > >> >>> + } > >> >>> + > >> >>> ret = alloc_oa_buffer(stream); > >> >>> if (ret) > >> >>> goto err_oa_buf_alloc; > >> >>> -- > >> >>> 2.25.1 > >> >>>
On Tue, 27 Sep 2022 10:34:52 -0700, Dixit, Ashutosh wrote: > > On Tue, 27 Sep 2022 09:11:23 -0700, Umesh Nerlige Ramappa wrote: > > > > On Mon, Sep 26, 2022 at 04:28:44PM -0700, Dixit, Ashutosh wrote: > > > On Mon, 26 Sep 2022 14:17:21 -0700, Belgaumkar, Vinay wrote: > > >> > > >> > > >> On 9/26/2022 11:19 AM, Umesh Nerlige Ramappa wrote: > > >> > On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote: > > >> >> On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: > > >> >>> > > >> >>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > >> >> > > >> >> Hi Umesh/Vinay, > > >> >> > > >> >>> @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct > > >> >>> i915_perf_stream *stream, > > >> >>> intel_engine_pm_get(stream->engine); > > >> >>> intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); > > >> >>> > > >> >>> + /* > > >> >>> + * Wa_16011777198:dg2: GuC resets render as part of the Wa. This > > >> >>> causes > > >> >>> + * OA to lose the configuration state. Prevent this by overriding > > >> >>> GUCRC > > >> >>> + * mode. > > >> >>> + */ > > >> >>> + if (intel_guc_slpc_is_used(>->uc.guc) && > > >> >>> + intel_uc_uses_guc_rc(>->uc) && > > >> >> > > >> >> Is this condition above correct? E.g. what happens when: > > >> >> > > >> >> a. GuCRC is used but SLPC is not used? > > >> > > >> Currently, we have no way to separate out GuC RC and SLPC. Both are on when > > >> guc submission is enabled/supported. So, the above check is kind of > > >> redundant anyways. > > > > > > That is why I was suggesting changing the if check to an assert or > > > drm_err. So looks like it will work with or without GuC RC, but if we are > > > using GuC RC we should be using SLPC. So: > > > > > > if (GuC_RC && !SLPC) > > > drm_err(); > > > > I am little confused. What's the ask here? Should I just use one of these > > conditions? i.e. > > > > if (intel_guc_slpc_is_used(>->uc.guc)) > > I think let's just use intel_uc_uses_guc_rc and drop > intel_guc_slpc_is_used. I am assuming if SLPC is somehow not on and we try > to use SLPC set/unset param, some kind of error will be reported. With the above this patch is also: Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > > > > ... > > > > Thanks, > > Umesh > > > > > > > >> Thanks, > > >> > > >> Vinay. > > >> > > >> >> b. GuCRC is not used. Don't we need to disable RC6 in host based RC6 > > >> >> control? > > >> > > > >> > When using host based rc6, existing OA code is using forcewake and a > > >> > reference to engine_pm to prevent rc6. Other questions, directing to > > >> > @Vinay. > > >> > > > >> > Thanks, > > >> > Umesh > > >> > > > >> >> > > >> >> Do we need to worry about these cases? > > >> >> > > >> >> Or if we always expect both GuCRC and SLPC to be used on DG2 then I > > >> >> think > > >> >> let's get rid of these from the if condition and add a drm_err() if we > > >> >> see > > >> >> these not being used and OA is being enabled on DG2? > > >> >> > > >> >> Thanks. > > >> >> -- > > >> >> Ashutosh > > >> >> > > >> >>> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || > > >> >>> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { > > >> >>> + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, > > >> >>> + SLPC_GUCRC_MODE_GUCRC_NO_RC6); > > >> >>> + if (ret) { > > >> >>> + drm_dbg(&stream->perf->i915->drm, > > >> >>> + "Unable to override gucrc mode\n"); > > >> >>> + goto err_config; > > >> >>> + } > > >> >>> + } > > >> >>> + > > >> >>> ret = alloc_oa_buffer(stream); > > >> >>> if (ret) > > >> >>> goto err_oa_buf_alloc; > > >> >>> -- > > >> >>> 2.25.1 > > >> >>>
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 fdd895f73f9f..b3a4fb9e021f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c @@ -137,6 +137,17 @@ static int guc_action_slpc_set_param(struct intel_guc *guc, u8 id, u32 value) return ret > 0 ? -EPROTO : ret; } +static int guc_action_slpc_unset_param(struct intel_guc *guc, u8 id) +{ + u32 request[] = { + GUC_ACTION_HOST2GUC_PC_SLPC_REQUEST, + SLPC_EVENT(SLPC_EVENT_PARAMETER_UNSET, 1), + id, + }; + + return intel_guc_send(guc, request, ARRAY_SIZE(request)); +} + static bool slpc_is_running(struct intel_guc_slpc *slpc) { return slpc_get_state(slpc) == SLPC_GLOBAL_STATE_RUNNING; @@ -190,6 +201,15 @@ static int slpc_set_param(struct intel_guc_slpc *slpc, u8 id, u32 value) return ret; } +static int slpc_unset_param(struct intel_guc_slpc *slpc, u8 id) +{ + struct intel_guc *guc = slpc_to_guc(slpc); + + GEM_BUG_ON(id >= SLPC_MAX_PARAM); + + return guc_action_slpc_unset_param(guc, id); +} + static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq) { struct drm_i915_private *i915 = slpc_to_i915(slpc); @@ -610,6 +630,52 @@ 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; + + with_intel_runtime_pm(&i915->runtime_pm, wakeref) { + 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); + } + + return ret; +} + +int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc) +{ + struct drm_i915_private *i915 = slpc_to_i915(slpc); + intel_wakeref_t wakeref; + int ret = 0; + + with_intel_runtime_pm(&i915->runtime_pm, wakeref) { + ret = slpc_unset_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE); + 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 432d18b6cc0d..80016f860406 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" @@ -1569,6 +1570,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(>->uc.guc) && + intel_uc_uses_guc_rc(>->uc) && + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) + drm_WARN_ON(>->i915->drm, + intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc)); + intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL); intel_engine_pm_put(stream->engine); @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, intel_engine_pm_get(stream->engine); intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL); + /* + * Wa_16011777198:dg2: GuC resets render as part of the Wa. This causes + * OA to lose the configuration state. Prevent this by overriding GUCRC + * mode. + */ + if (intel_guc_slpc_is_used(>->uc.guc) && + intel_uc_uses_guc_rc(>->uc) && + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) { + ret = intel_guc_slpc_override_gucrc_mode(>->uc.guc.slpc, + SLPC_GUCRC_MODE_GUCRC_NO_RC6); + if (ret) { + drm_dbg(&stream->perf->i915->drm, + "Unable to override gucrc mode\n"); + goto err_config; + } + } + ret = alloc_oa_buffer(stream); if (ret) goto err_oa_buf_alloc;