[CI 1/2] drm/xe/lnl: Apply Wa_22019338487

Daniele Ceraolo Spurio daniele.ceraolospurio at intel.com
Tue May 28 17:20:24 UTC 2024 


On 5/28/2024 6:45 AM, Lucas De Marchi wrote:
> On Mon, May 27, 2024 at 02:19:27PM GMT, Daniele Ceraolo Spurio wrote:
>> From: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>
>> 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/Makefile        |  2 ++
>> drivers/gpu/drm/xe/xe_device.c     |  8 ++++++++
>> drivers/gpu/drm/xe/xe_gsc.c        |  8 ++++++++
>> drivers/gpu/drm/xe/xe_guc_pc.c     | 31 ++++++++++++++++++++++++++++--
>> drivers/gpu/drm/xe/xe_guc_pc.h     |  2 ++
>> drivers/gpu/drm/xe/xe_wa_oob.rules |  1 +
>> 6 files changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index d5b137e762ed..3701d1a01e50 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -47,9 +47,11 @@ $(obj)/generated/%_wa_oob.c 
>> $(obj)/generated/%_wa_oob.h: $(obj)/xe_gen_wa_oob \
>>     $(call cmd,wa_oob)
>>
>> uses_generated_oob := \
>> +    $(obj)/xe_device.o \
>>     $(obj)/xe_gsc.o \
>>     $(obj)/xe_guc.o \
>>     $(obj)/xe_guc_ads.o \
>> +    $(obj)/xe_guc_pc.o \
>>     $(obj)/xe_migrate.o \
>>     $(obj)/xe_ring_ops.o \
>>     $(obj)/xe_vm.o \
>> diff --git a/drivers/gpu/drm/xe/xe_device.c 
>> b/drivers/gpu/drm/xe/xe_device.c
>> index f04b11e45c2d..85c1b0c406a6 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -16,6 +16,7 @@
>> #include <drm/drm_managed.h>
>> #include <drm/drm_print.h>
>> #include <drm/xe_drm.h>
>> +#include <generated/xe_wa_oob.h>
>>
>> #include "display/xe_display.h"
>> #include "instructions/xe_gpu_commands.h"
>> @@ -37,6 +38,7 @@
>> #include "xe_gt_printk.h"
>> #include "xe_gt_sriov_vf.h"
>> #include "xe_guc.h"
>> +#include "xe_guc_pc.h"
>> #include "xe_hwmon.h"
>> #include "xe_irq.h"
>> #include "xe_memirq.h"
>> @@ -51,6 +53,7 @@
>> #include "xe_ttm_stolen_mgr.h"
>> #include "xe_ttm_sys_mgr.h"
>> #include "xe_vm.h"
>> +#include "xe_wa.h"
>> #include "xe_wait_user_fence.h"
>>
>> static int xe_file_open(struct drm_device *dev, struct drm_file *file)
>> @@ -337,6 +340,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 = &gt->uc.guc.pc;
>>     int ret;
>>
>>     if (xe_mmio_read32(gt, GU_CNTL_PROTECTED) & DRIVERINT_FLR_DIS) {
>> @@ -344,6 +348,10 @@ static void xe_driver_flr(struct xe_device *xe)
>>         return;
>>     }
>>
>> +    /* Set requested freq to mert_freq_cap before FLR */
>> +    if (XE_WA(gt, 22019338487))
>> +        pc_set_cur_freq(pc, min(xe_guc_pc_mert_freq_cap(pc), 
>> pc->rpe_freq));
>
> it seems pc_set_cur_freq() is violating a layer here. At the minimum it
> should have the xe_guc_pc prefix, but it seems we actually want a higher
> level call here like  xe_guc_pc_prepare_for_flr(pc). Then the WA is
> handled internally to xe_guc_pc.

Note that this WA is being reviewed as its own patch here:
https://patchwork.freedesktop.org/series/134036/

Just mentioning it because I'm going to respin this series later today 
with a fix for the GSC proxy unload and I'm not going to include your 
suggested fix for this patch; I'm only including this patch in the 
series for testing, so it doesn't really matter if the patch is clean as 
long as it does the right thing :) . I'll leave it to Vinay to fix it 
properly in the other thread.

Daniele

>
>> +
>>     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 80a61934decc..4369e3ff19cf 100644
>> --- a/drivers/gpu/drm/xe/xe_gsc.c
>> +++ b/drivers/gpu/drm/xe/xe_gsc.c
>> @@ -22,6 +22,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"
>> @@ -342,6 +343,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 = &gt->uc.guc.pc;
>>     u32 actions;
>>     int ret;
>>
>> @@ -370,6 +372,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 */
>
> same thing... do we need a higher level call here rather than calling
> the set max/min freq?
>
>> +    if (XE_WA(gt, 22019338487)) {
>> +        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 8187dfb2ad6c..3ecfbaa75495 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
>> @@ -8,6 +8,7 @@
>> #include <linux/delay.h>
>>
>> #include <drm/drm_managed.h>
>> +#include <generated/xe_wa_oob.h>
>>
>> #include "abi/guc_actions_abi.h"
>> #include "abi/guc_actions_slpc_abi.h"
>> @@ -24,6 +25,7 @@
>> #include "xe_map.h"
>> #include "xe_mmio.h"
>> #include "xe_pcode.h"
>> +#include "xe_wa.h"
>>
>> #define MCHBAR_MIRROR_BASE_SNB    0x140000
>>
>> @@ -41,6 +43,8 @@
>> #define GT_FREQUENCY_MULTIPLIER    50
>> #define GT_FREQUENCY_SCALER    3
>>
>> +#define LNL_MERT_FREQ_CAP    800
>> +
>> /**
>>  * DOC: GuC Power Conservation (PC)
>>  *
>> @@ -238,7 +242,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;
>> @@ -674,6 +678,14 @@ static void pc_init_fused_rp_values(struct 
>> xe_guc_pc *pc)
>>         tgl_init_fused_rp_values(pc);
>> }
>>
>> +u32 xe_guc_pc_mert_freq_cap(struct xe_guc_pc *pc)
>> +{
>> +    if (MEDIA_VERx100(pc_to_xe(pc)) == 2000)
>> +        return LNL_MERT_FREQ_CAP;
>> +    else
>> +        return 0;
>> +}
>> +
>> /**
>>  * xe_guc_pc_init_early - Initialize RPx values and request a higher GT
>>  * frequency to allow faster GuC load times
>> @@ -685,7 +697,11 @@ void xe_guc_pc_init_early(struct xe_guc_pc *pc)
>>
>>     xe_force_wake_assert_held(gt_to_fw(gt), XE_FW_GT);
>>     pc_init_fused_rp_values(pc);
>> -    pc_set_cur_freq(pc, pc->rp0_freq);
>> +
>> +    if (XE_WA(gt, 22019338487))
>> +        pc_set_cur_freq(pc, min(xe_guc_pc_mert_freq_cap(pc), 
>> pc->rp0_freq));
>> +    else
>> +        pc_set_cur_freq(pc, pc->rp0_freq);
>> }
>>
>> static int pc_adjust_freq_bounds(struct xe_guc_pc *pc)
>> @@ -716,6 +732,17 @@ static int pc_adjust_freq_bounds(struct 
>> xe_guc_pc *pc)
>>     if (pc_get_min_freq(pc) > pc->rp0_freq)
>>         ret = pc_set_min_freq(pc, pc->rp0_freq);
>>
>> +    if (!ret && XE_WA(pc_to_gt(pc), 22019338487)) {
>> +        /*
>> +         * Setting min to RPn disables use of efficient freq
>> +         * which could otherwise interfere with this WA for media GT.
>> +         * We will also bind max to MERT_FREQ_CAP until driver loads.
>> +         */
>> +        ret = pc_set_min_freq(pc, pc->rpn_freq);
>> +        if (!ret)
>> +            ret = pc_set_max_freq(pc, min(pc->rp0_freq, 
>> xe_guc_pc_mert_freq_cap(pc)));
>> +    }
>> +
>> out:
>>     return ret;
>> }
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.h 
>> b/drivers/gpu/drm/xe/xe_guc_pc.h
>> index 532cac985a6d..4abdefa36b9a 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc.h
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc.h
>> @@ -29,5 +29,7 @@ enum xe_gt_idle_state xe_guc_pc_c_status(struct 
>> xe_guc_pc *pc);
>> u64 xe_guc_pc_rc6_residency(struct xe_guc_pc *pc);
>> u64 xe_guc_pc_mc6_residency(struct xe_guc_pc *pc);
>> void xe_guc_pc_init_early(struct xe_guc_pc *pc);
>> +void pc_set_cur_freq(struct xe_guc_pc *pc, u32 freq);
>
>        ^
>
> See the other functions in this header. We don't export functions to
> the other parts in the driver without a prefix for this component.
>
> Lucas De Marchi
>
>> +u32 xe_guc_pc_mert_freq_cap(struct xe_guc_pc *pc);
>>
>> #endif /* _XE_GUC_PC_H_ */
>> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules 
>> b/drivers/gpu/drm/xe/xe_wa_oob.rules
>> index 12fe88796a49..a6b897030fde 100644
>> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
>> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
>> @@ -27,3 +27,4 @@
>> 16022287689    GRAPHICS_VERSION(2001)
>>         GRAPHICS_VERSION(2004)
>> 13011645652    GRAPHICS_VERSION(2004)
>> +22019338487    MEDIA_VERSION(2000)
>> -- 
>> 2.43.0
>>

More information about the Intel-xe mailing list