Re: [Intel-gfx] [PATCH v1.1] drm/i915/mtl: Implement Wa_14019141245

["Kalvala, Haridhar" <haridhar.kalvala@intel.com>]

On 4/26/2023 9:41 PM, Sripada, Radhakrishna wrote:
>
>> -----Original Message-----
>> From: Kalvala, Haridhar <haridhar.kalvala@intel.com>
>> Sent: Wednesday, April 26, 2023 5:36 AM
>> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH v1.1] drm/i915/mtl: Implement
>> Wa_14019141245
>>
>>
>> On 4/26/2023 12:00 AM, Radhakrishna Sripada wrote:
>>> Enable strict RAR to prevent spurious GPU hangs.
>>>
>>> v1.1: Rebase
>>>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Reviewed-by:Haridhar Kalvala <haridhar.kalvala@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gt/intel_gt_regs.h     | 5 +++++
>>>    drivers/gpu/drm/i915/gt/intel_workarounds.c | 4 ++++
>>>    drivers/gpu/drm/i915/i915_perf_oa_regs.h    | 4 ----
>>>    3 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> index e8c3b762a92a..af80d2fe739b 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> @@ -529,6 +529,11 @@
>>>
>>>    #define GEN8_RC6_CTX_INFO			_MMIO(0x8504)
>>>
>>> +#define GEN12_SQCNT1				_MMIO(0x8718)
>>> +#define   GEN12_SQCNT1_PMON_ENABLE		REG_BIT(30)
>>> +#define   GEN12_SQCNT1_OABPC			REG_BIT(29)
>>> +#define   GEN12_STRICT_RAR_ENABLE		REG_BIT(23)
>>> +
>>>    #define XEHP_SQCM				MCR_REG(0x8724)
>>>    #define   EN_32B_ACCESS				REG_BIT(30)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> index de4f8e2e8e8c..ad9e7f49a6fa 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> @@ -1699,6 +1699,9 @@ xelpg_gt_workarounds_init(struct intel_gt *gt,
>> struct i915_wa_list *wal)
>>>    	wa_mcr_write_or(wal, RENDER_MOD_CTRL, FORCE_MISS_FTLB);
>>>    	wa_mcr_write_or(wal, COMP_MOD_CTRL, FORCE_MISS_FTLB);
>>>
>>> +	/* Wa_14019141245 */
>>> +	wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE);
>>> +
>> looks good to me.
>>>    	if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) ||
>>>    	    IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0)) {
>>>    		/* Wa_14014830051 */
>>> @@ -1707,6 +1710,7 @@ xelpg_gt_workarounds_init(struct intel_gt *gt,
>> struct i915_wa_list *wal)
>>>    		/* Wa_14015795083 */
>>>    		wa_write_clr(wal, GEN7_MISCCPCTL,
>> GEN12_DOP_CLOCK_GATE_RENDER_ENABLE);
>>>    	}
>>> +
>>>    	/*
>>>    	 * Unlike older platforms, we no longer setup implicit steering here;
>>>    	 * all MCR accesses are explicitly steered.
>>> diff --git a/drivers/gpu/drm/i915/i915_perf_oa_regs.h
>> b/drivers/gpu/drm/i915/i915_perf_oa_regs.h
>>> index ba103875e19f..e5ac7a8b5cb6 100644
>>> --- a/drivers/gpu/drm/i915/i915_perf_oa_regs.h
>>> +++ b/drivers/gpu/drm/i915/i915_perf_oa_regs.h
>>> @@ -134,10 +134,6 @@
>>>    #define GDT_CHICKEN_BITS    _MMIO(0x9840)
>>>    #define   GT_NOA_ENABLE	    0x00000080
>>>
>>> -#define GEN12_SQCNT1				_MMIO(0x8718)
>>> -#define   GEN12_SQCNT1_PMON_ENABLE		REG_BIT(30)
>>> -#define   GEN12_SQCNT1_OABPC			REG_BIT(29)
>> These two register bit and register(0x8718) moved to "
>> intel_gt_regs.h"not getting used elsewhere(I mean, in i915_perf.c) ?
> 1) i915_perf.c includes gt/intel_gt_regs.h so moving the register def. there should not cause any problem.
> Moreover,
> 2) intel_gt_regs.h is used across almost all the files under i915/gt.
> i915_perf_oa_regs.h do not have that kind of usage.
> 3) because of this bit, the usage of this register is not limited to perf subsystem.
> Hence the better place in intel_gt_regs.h.
> 4) we need not have all the i915_pref_oa_regs.h definitions included in intel_workarounds.c
>
>
> - Radhakrishna(RK) Sripada
>    
>>> -
>>>    /* Gen12 OAM unit */
>>>    #define GEN12_OAM_HEAD_POINTER_OFFSET   (0x1a0)
>>>    #define  GEN12_OAM_HEAD_POINTER_MASK    0xffffffc0
>> --
>> Regards,
>> Haridhar Kalvala

-- 
Regards,
Haridhar Kalvala