[PATCH 1/2] drm/xe/lnl: Apply Wa_22019338487
Lucas De Marchi
lucas.demarchi at intel.com
Wed Apr 24 00:15:46 UTC 2024
On Tue, Apr 23, 2024 at 12:51:43PM GMT, Belgaumkar, Vinay wrote:
>
>On 4/23/2024 10:43 AM, Lucas De Marchi wrote:
>>On Mon, Apr 22, 2024 at 11:10:43AM GMT, Belgaumkar, Vinay wrote:
>>>
>>>On 4/22/2024 11:09 AM, Lucas De Marchi wrote:
>>>>On Fri, Apr 19, 2024 at 01:13:43PM GMT, Vinay Belgaumkar wrote:
>>>>>This WA requires us to limit media GT frequency requests to a certain
>>>>>cap value during driver load as well as after driver unload.
>>>>>Freq limits
>>>>>are restored after driver load completes, so perf will not be
>>>>>affected during normal operations.
>>>>>
>>>>>Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>>>>---
>>>>>drivers/gpu/drm/xe/xe_device.c | 6 +++++
>>>>>drivers/gpu/drm/xe/xe_gsc.c | 8 +++++++
>>>>>drivers/gpu/drm/xe/xe_guc_pc.c | 40 ++++++++++++++++++++++++++++++++--
>>>>>drivers/gpu/drm/xe/xe_guc_pc.h | 3 +++
>>>>>4 files changed, 55 insertions(+), 2 deletions(-)
>>>>>
>>>>>diff --git a/drivers/gpu/drm/xe/xe_device.c
>>>>>b/drivers/gpu/drm/xe/xe_device.c
>>>>>index d85a2ba0a057..e29c152a6c4e 100644
>>>>>--- a/drivers/gpu/drm/xe/xe_device.c
>>>>>+++ b/drivers/gpu/drm/xe/xe_device.c
>>>>>@@ -30,6 +30,7 @@
>>>>>#include "xe_gsc_proxy.h"
>>>>>#include "xe_gt.h"
>>>>>#include "xe_gt_mcr.h"
>>>>>+#include "xe_guc_pc.h"
>>>>>#include "xe_hwmon.h"
>>>>>#include "xe_irq.h"
>>>>>#include "xe_memirq.h"
>>>>>@@ -336,6 +337,7 @@ static void xe_driver_flr(struct xe_device *xe)
>>>>>{
>>>>> const unsigned int flr_timeout = 3 * MICRO; /* specs
>>>>>recommend a 3s wait */
>>>>> struct xe_gt *gt = xe_root_mmio_gt(xe);
>>>>>+ struct xe_guc_pc *pc = >->uc.guc.pc;
>>>>> int ret;
>>>>>
>>>>> if (xe_mmio_read32(gt, GU_CNTL_PROTECTED) & DRIVERINT_FLR_DIS) {
>>>>>@@ -343,6 +345,10 @@ static void xe_driver_flr(struct xe_device *xe)
>>>>> return;
>>>>> }
>>>>>
>>>>>+ /* Set requested freq to mert_freq_cap before FLR */
>>>>>+ if (xe_guc_pc_needs_wa_22019338487(pc))
>>>>>+ pc_set_cur_freq(pc, min(xe_guc_pc_mert_freq_cap(pc),
>>>>>pc->rpe_freq));
>>>>>+
>>>>> drm_dbg(&xe->drm, "Triggering Driver-FLR\n");
>>>>>
>>>>> /*
>>>>>diff --git a/drivers/gpu/drm/xe/xe_gsc.c b/drivers/gpu/drm/xe/xe_gsc.c
>>>>>index 60202b903687..556e73fca813 100644
>>>>>--- a/drivers/gpu/drm/xe/xe_gsc.c
>>>>>+++ b/drivers/gpu/drm/xe/xe_gsc.c
>>>>>@@ -19,6 +19,7 @@
>>>>>#include "xe_gt.h"
>>>>>#include "xe_gt_mcr.h"
>>>>>#include "xe_gt_printk.h"
>>>>>+#include "xe_guc_pc.h"
>>>>>#include "xe_huc.h"
>>>>>#include "xe_map.h"
>>>>>#include "xe_mmio.h"
>>>>>@@ -339,6 +340,7 @@ static void gsc_work(struct work_struct *work)
>>>>> struct xe_gsc *gsc = container_of(work, typeof(*gsc), work);
>>>>> struct xe_gt *gt = gsc_to_gt(gsc);
>>>>> struct xe_device *xe = gt_to_xe(gt);
>>>>>+ struct xe_guc_pc *pc = >->uc.guc.pc;
>>>>> u32 actions;
>>>>> int ret;
>>>>>
>>>>>@@ -367,6 +369,12 @@ static void gsc_work(struct work_struct *work)
>>>>> if (actions & GSC_ACTION_SW_PROXY)
>>>>> xe_gsc_proxy_request_handler(gsc);
>>>>>
>>>>>+ /* Revert the min/max freq limits as we're done with
>>>>>GSC/driver load */
>>>>>+ if (xe_guc_pc_needs_wa_22019338487(pc)) {
>>>>>+ xe_guc_pc_set_max_freq(pc, pc->rp0_freq);
>>>>>+ xe_guc_pc_set_min_freq(pc, pc->rpe_freq);
>>>>>+ }
>>>>>+
>>>>>out:
>>>>> xe_force_wake_put(gt_to_fw(gt), XE_FW_GSC);
>>>>> xe_pm_runtime_put(xe);
>>>>>diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c
>>>>>b/drivers/gpu/drm/xe/xe_guc_pc.c
>>>>>index 521ae24f2314..7f82b6c2ad3c 100644
>>>>>--- a/drivers/gpu/drm/xe/xe_guc_pc.c
>>>>>+++ b/drivers/gpu/drm/xe/xe_guc_pc.c
>>>>>@@ -40,6 +40,8 @@
>>>>>#define GT_FREQUENCY_MULTIPLIER 50
>>>>>#define GT_FREQUENCY_SCALER 3
>>>>>
>>>>>+#define LNL_MERT_FREQ_CAP 800
>>>>>+
>>>>>/**
>>>>> * DOC: GuC Power Conservation (PC)
>>>>> *
>>>>>@@ -237,7 +239,7 @@ static void pc_set_manual_rp_ctrl(struct
>>>>>xe_guc_pc *pc, bool enable)
>>>>> xe_mmio_write32(gt, RP_CONTROL, state);
>>>>>}
>>>>>
>>>>>-static void pc_set_cur_freq(struct xe_guc_pc *pc, u32 freq)
>>>>>+void pc_set_cur_freq(struct xe_guc_pc *pc, u32 freq)
>>>>>{
>>>>> struct xe_gt *gt = pc_to_gt(pc);
>>>>> u32 rpnswreq;
>>>>>@@ -673,6 +675,25 @@ static void
>>>>>pc_init_fused_rp_values(struct xe_guc_pc *pc)
>>>>> tgl_init_fused_rp_values(pc);
>>>>>}
>>>>>
>>>>>+bool xe_guc_pc_needs_wa_22019338487(struct xe_guc_pc *pc)
>>>>>+{
>>>>>+ struct xe_gt *gt = pc_to_gt(pc);
>>>>>+ struct xe_device *xe = gt_to_xe(gt);
>>>>>+
>>>>>+ if (MEDIA_VERx100(xe) == 2000 && xe_gt_is_media_type(gt))
>>>>
>>>>that's what xe_wa_oob.rules is for. Why are you adding it as a separate
>>>>function that won't allow debugfs/.../workarounds to match reality?
>>>
>>>The workarounds are not initialized at this point, this is early
>>>in the driver load process.
>>
>>ugh... the _early() variant was supposed to be the sw part with
>>minimal hw. Now we added a few super_early(),
>>no_I_mean_earlier_than_super_early() variants... but with other names.
>>
>>How do we call xe_force_wake_init_gt() and xe_pcode_init() passing the
>>gt without the gt even be initialized? Are we trying to create a house
>>of cards? This init order is in an awful state :(. We should stop
>>pretending we are getting the engine info from hwconfig and just fix the
>>ordering so we can call xe_gt_init_early() and it really happens before
>>things like loading guc.
>
>Yup, doesn't look like an easy fix here. the GMDID is populated
>somewhere in xe_gt_init() which comes way after the early_init().
>Cannot check for WAs unless we have that.
nah... if we were reading GMDID in xe_gt_init() then basically we'd have
not a single WA applied. xe_gt_init() only stashes the raw gmdid in the
gt for later use. But the place that we really read it is
xe_info_init(), which happens much earlier.
The current problem is: the xe_wa_*_process() are called in
xe_gt_init_early(), which used to be very early. But now we have other
things happening before that like xe_gt_init_hwconfig(),
xe_pcode_init(), xe_force_wake_init_gt(). None of these should really be
happening before xe_gt_init_early(). While you figure out the second
part of the WA, let me take a look if there's a not-so-ugly fix.
Lucas De Marchi
More information about the Intel-xe
mailing list