[PATCH 2/3] drm/xe/bmg: Update Wa_22019338487
Rodrigo Vivi
rodrigo.vivi at intel.com
Fri May 30 13:19:33 UTC 2025
On Fri, May 30, 2025 at 01:20:46AM -0700, Vinay Belgaumkar wrote:
> Limit GT max frequency to 2600Mhz during the L2 flush. Also, ensure
> GT actual frequency is limited to that value before performing the
> cache flush.
>
> Fixes: aaa08078e725 ("drm/xe/bmg: Apply Wa_22019338487")
>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> ---
> drivers/gpu/drm/xe/xe_device.c | 11 +++++-
> drivers/gpu/drm/xe/xe_guc_pc.c | 68 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_guc_pc.h | 2 +
> 3 files changed, 79 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 660b0c5126dc..6a115bd77d1c 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -40,6 +40,7 @@
> #include "xe_gt_printk.h"
> #include "xe_gt_sriov_vf.h"
> #include "xe_guc.h"
> +#include "xe_guc_pc.h"
> #include "xe_hw_engine_group.h"
> #include "xe_hwmon.h"
> #include "xe_irq.h"
> @@ -1000,16 +1001,19 @@ void xe_device_wmb(struct xe_device *xe)
> */
> void xe_device_td_flush(struct xe_device *xe)
> {
> - struct xe_gt *gt;
> + struct xe_gt *gt = xe_root_mmio_gt(xe);
> unsigned int fw_ref;
> + u32 gt_max_freq;
> u8 id;
>
> if (!IS_DGFX(xe) || GRAPHICS_VER(xe) < 20)
> return;
>
> + xe_guc_pc_flush_wa_22019338487_start(>->uc.guc.pc, >_max_freq);
There are 2 things that I don't like here to start with:
1. The wa_1234 in the function name. Please make it as generic as possible.
Then check for its need inside with the WA_ check.
2. gt_max_freq as a cookie to be returned. For multiple reasons.
a. This is kind of ugly that flush memory operations should care about the
freq and then return it later.
b. What happens on a race condition?
process a - get max 2000 - set to 1200 temporarily
process b - get max 2600
process a - returns max to 2000
process b - returns max to 2600
Make it in a way that the desired max freq is saved with locking mechanisms
inside some freq or pc struct. Then while we have this max stashed we refuse
any write to max_freq with some new return error and debug msg... or perhaps
the set max function waits for that to be unset since flush is fast operation...
> +
> if (XE_WA(xe_root_mmio_gt(xe), 16023588340)) {
> xe_device_l2_flush(xe);
> - return;
> + goto done;
> }
>
> for_each_gt(gt, xe, id) {
> @@ -1034,6 +1038,9 @@ void xe_device_td_flush(struct xe_device *xe)
>
> xe_force_wake_put(gt_to_fw(gt), fw_ref);
> }
> +
> +done:
> + xe_guc_pc_flush_wa_22019338487_end(>->uc.guc.pc, gt_max_freq);
> }
>
> void xe_device_l2_flush(struct xe_device *xe)
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 513657154dbe..4c4c7498e5e7 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -52,9 +52,11 @@
> #define LNL_MERT_FREQ_CAP 800
> #define BMG_MERT_FREQ_CAP 2133
> #define MIN_FREQ_WA_14022085890 1200
> +#define BMG_MERT_FLUSH_FREQ_CAP 2600
>
> #define SLPC_RESET_TIMEOUT_MS 5 /* roughly 5ms, but no need for precision */
> #define SLPC_RESET_EXTENDED_TIMEOUT_MS 1000 /* To be used only at pc_start */
> +#define SLPC_ACT_FREQ_TIMEOUT_MS 100
>
> /**
> * DOC: GuC Power Conservation (PC)
> @@ -869,6 +871,71 @@ static int pc_adjust_requested_freq(struct xe_guc_pc *pc)
> return ret;
> }
>
> +static int wait_for_act_freq_limit(struct xe_guc_pc *pc, u32 freq)
> +{
> + int timeout_us = 1000 * SLPC_ACT_FREQ_TIMEOUT_MS;
> + int slept, wait = 10;
> +
> + for (slept = 0; slept < timeout_us;) {
> + if (xe_guc_pc_get_act_freq(pc) <= freq)
> + return 0;
> +
> + usleep_range(wait, wait << 1);
> + slept += wait;
> + wait <<= 1;
> + if (slept + wait > timeout_us)
> + wait = timeout_us - slept;
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static bool needs_flush_wa_22019338487(struct xe_guc_pc *pc)
> +{
> + struct xe_gt *gt = pc_to_gt(pc);
> +
> + if (XE_WA(gt, 22019338487)) {
> + if (pc->rp0_freq > BMG_MERT_FLUSH_FREQ_CAP)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +void xe_guc_pc_flush_wa_22019338487_start(struct xe_guc_pc *pc, u32 *gt_max_freq)
please document every export/public function.
> +{
> + struct xe_gt *gt = pc_to_gt(pc);
> + int ret;
> +
> + if (!needs_flush_wa_22019338487(pc))
> + return;
> +
> + ret = xe_guc_pc_get_max_freq(pc, gt_max_freq);
> + if (!ret) {
> + ret = xe_guc_pc_set_max_freq(>->uc.guc.pc, BMG_MERT_FLUSH_FREQ_CAP);
> + /* Wait for actual freq to go below the flush cap */
> + if (!ret) {
> + ret = wait_for_act_freq_limit(pc, BMG_MERT_FLUSH_FREQ_CAP);
> + if (ret)
> + xe_gt_err_once(gt, "Actual freq did not reduce to %u, %pe\n",
> + BMG_MERT_FLUSH_FREQ_CAP, ERR_PTR(ret));
> + }
> + }
> +}
> +
> +void xe_guc_pc_flush_wa_22019338487_end(struct xe_guc_pc *pc, u32 gt_max_freq)
> +{
> + struct xe_gt *gt = pc_to_gt(pc);
> + int ret;
> +
> + if (needs_flush_wa_22019338487(pc)) {
> + ret = xe_guc_pc_set_max_freq(>->uc.guc.pc, gt_max_freq);
> + if (ret)
> + xe_gt_err_once(gt, "Unable to restore max freq to %u:%pe",
> + gt_max_freq, ERR_PTR(ret));
> + }
> +}
> +
> static int pc_set_mert_freq_cap(struct xe_guc_pc *pc)
> {
> int ret = 0;
> @@ -880,6 +947,7 @@ static int pc_set_mert_freq_cap(struct xe_guc_pc *pc)
> ret = xe_guc_pc_get_min_freq(pc, &pc->stashed_min_freq);
> if (!ret)
> ret = xe_guc_pc_get_max_freq(pc, &pc->stashed_max_freq);
> +
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.h b/drivers/gpu/drm/xe/xe_guc_pc.h
> index 0a2664d5c811..8424c68cfb1d 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.h
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.h
> @@ -38,5 +38,7 @@ u64 xe_guc_pc_mc6_residency(struct xe_guc_pc *pc);
> void xe_guc_pc_init_early(struct xe_guc_pc *pc);
> int xe_guc_pc_restore_stashed_freq(struct xe_guc_pc *pc);
> void xe_guc_pc_raise_unslice(struct xe_guc_pc *pc);
> +void xe_guc_pc_flush_wa_22019338487_start(struct xe_guc_pc *pc, u32 *gt_max_freq);
> +void xe_guc_pc_flush_wa_22019338487_end(struct xe_guc_pc *pc, u32 gt_max_freq);
>
> #endif /* _XE_GUC_PC_H_ */
> --
> 2.38.1
>
More information about the Intel-xe
mailing list