[Intel-gfx] [v2] drm/i915/gen11: Moving WAs to icl_gt_workarounds_init()

[Lucas De Marchi]

On Fri, Dec 10, 2021 at 05:41:46PM -0800, John Harrison wrote:
>On 12/10/2021 17:22, Matt Roper wrote:
>>On Thu, Dec 09, 2021 at 09:21:39AM -0800, Lucas De Marchi wrote:
>>>On Fri, Dec 03, 2021 at 08:26:03PM +0530, ravitejax.goud.talla at intel.com wrote:
>>>>From: Raviteja Goud Talla <ravitejax.goud.talla at intel.com>
>>>>
>>>>Bspec page says "Reset: BUS", Accordingly moving w/a's:
>>>>Wa_1407352427,Wa_1406680159 to proper function icl_gt_workarounds_init()
>>>>Which will resolve guc enabling error
>>>>
>>>>v2:
>>>>  - Previous patch rev2 was created by email client which caused the
>>>>    Build failure, This v2 is to resolve the previous broken series
>>>>
>>>>Reviewed-by: John Harrison <John.C.Harrison at Intel.com>
>>>>Signed-off-by: Raviteja Goud Talla <ravitejax.goud.talla at intel.com>
>>>It seems like this patch is needed to be able to load GuC in ICL:
>>>https://gitlab.freedesktop.org/drm/intel/-/issues/4067#note_1184951
>>>
>>>And that is failing on Linus' master branch as of
>>>2a987e65025e ("Merge tag 'perf-tools-fixes-for-v5.16-2021-12-07' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux")
>>>and (at least) 5.15.*. Looking at the appropriate "Fixes" tag I found these commits:
>>>
>>>   1) 1cd21a7c5679 ("drm/i915: Add Wa_1407352427:icl,ehl") - original
>>>      commit adding the WA
>>>   2) 3551ff928744 ("drm/i915/gen11: Moving WAs to rcs_engine_wa_init()")
>>>      moving the WA to rcs_engine_wa_init()
>>>
>>>However (2) is on v5.7-rc1 and (1) is on v5.6-rc1 and according to the
>>>bug report GuC loading was working on 5.13. So I suspect the move
>>>to GuC 62.0.0 made the checks more strict, or there is another patch
>>This is correct.  Having "GT workarounds" on the engine list by accident
>>used to be harmless (because i915 [with execlist submission] and the guc
>>[with guc submission]) would simply re-apply the register setting more
>>often than it really needed to.  However the more recent GuC versions
>>have become more picky about the set of registers they're willing to
>Actually, I think the GuC was always picky but we haven't supported 
>GuC submission upstream for quite some time. There was very old 
>support (never enabled by default) in the VLV timescale. And at that 
>time, we were not using GuC scheduling anyway, so no save/restore 
>list. I think by ICL it had already been removed as broken. All you 
>could do was load the GuC in order to load the HuC. It is only 
>recently with the ADL-P/DG1 support that GuC submission has been 
>re-implemented and the save/restore list added in.

as I said in my other reply to Matt, it's not GuC sumbmission that got
broken though:  it's enable_guc=2.

>
>
>>save/restore for workarounds and will fail to load if they see a
>>register on the save/restore list that isn't something they think is
>>appropriate for an engine reset.
>>
>>GuC submission isn't officially supported on ICL; you can force it via
>>module parameter, which taints the kernel and takes you through untested
>>codepaths, so you do so at your own risk.  Given that, I don't think
>>there's any real need to worry about getting this backported to specific
>>stable kernels; having the workaround in the wrong place doesn't
>>actually harm anything on the official and default non-GuC mode.
>As above, even with GuC enabled, I still don't think it is a problem 
>for any kernel using a GuC version earlier than 62.0.0.

so that rules out 5.10 and the only stable we need this on is 5.15 which
is pretty easy and applies cleanly.

Lucas De Marchi