Re: [Intel-gfx] [PATCH] drm/i915: Skip Bit12 fw domain reset for gen12+

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 25/08/2022 18:49, Sripada, Radhakrishna wrote:
> Hi G.G,
> 
>> -----Original Message-----
>> From: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>
>> Sent: Tuesday, August 23, 2022 11:14 PM
>> To: Roper, Matthew D <matthew.d.roper@intel.com>; Sripada, Radhakrishna
>> <radhakrishna.sripada@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Skip Bit12 fw domain reset for gen12+
>>
>>
>>
>> On 8/18/22 3:00 PM, Matt Roper wrote:
>>> On Wed, Aug 17, 2022 at 03:43:04PM -0700, Radhakrishna Sripada wrote:
>>>> Bit12 of the Forcewake request register should not be cleared post
>>>> gen12. Do not touch this bit while clearing during fw domain reset.
>>>>
>>>> Bspec: 52542
>>>>
>>>> Signed-off-by: Sushma Venkatesh Reddy
>> <sushma.venkatesh.reddy@intel.com>
>>>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_uncore.c | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> b/drivers/gpu/drm/i915/intel_uncore.c
>>>> index a852c471d1b3..c85e2b686c95 100644
>>>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>>>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>>>> @@ -113,7 +113,10 @@ fw_domain_reset(const struct
>> intel_uncore_forcewake_domain *d)
>>>>    	 * off in ICL+), so no waiting for acks
>>>>    	 */
>>>>    	/* WaRsClearFWBitsAtReset:bdw,skl */
>>>
>>> While we're at it, let's remove the "bdw,skl" from this comment since
>>> it's misleading and doesn't match the code.  We do still apply this
>>> workaround on other pre-gen12 platforms than just those two.
>>>
>>> Aside from the comment tweak,
>>>
>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>>>
>>>> -	fw_clear(d, 0xffff);
>>>> +	if (GRAPHICS_VER(d->uncore->i915) >= 12)
>> Hi Radhakrishna Sripada,
>>
>> In bspec 52542, there is an explanation that BIT12 should not be set for
>> address 0xA188 corresponding to FORCEWAKE_MT/FORCEWAKE_GT_GEN9, but
>> in
>> bspec 52466, there is no explanation that BIT12 should not be set for
>> address 0xA278, address of FORCEWAKE_RENDER_GEN9.
>>
>> I ask if fw_domain_reset() should perform fw_clear() by comparing not
>> only GRAPHICS_VER() >= 12 but also checking of FW_DOMAIN_ID_RENDER and
>> FW_DOMAIN_ID_GT values.
> Based on the note in 52542, all other WA notes are overridden by the comment. So unless stated
> otherwise, it should apply to this register as well.
> 
> Created a bspec issue to request for additional clarification just to be safe. Will send an additional
> patch if the comment contradicts our understanding.

How important was this patch - should it be sent to stable?

Regards,

Tvrtko