[PATCH] drm/i915/mtl: Update workaround 14018778641

[PATCH] drm/i915/mtl: Update workaround 14018778641

[Chen, Angus]

The WA should be extended to cover VDBOX engine. We found that
28-channels 1080p VP9 encoding may hit this issue.

Signed-off-by: Chen, Angus <angus.chen@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index d1ab560fcdfc..bf14749f5792 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1586,6 +1586,8 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
 	 */
 	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
 
+	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
+
 	/* Wa_22016670082 */
 	wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE);
 
-- 
2.34.1

✗ Fi.CI.BAT: failure for drm/i915/mtl: Update workaround 14018778641 (rev4)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 8069 bytes --]

== Series Details ==

Series: drm/i915/mtl: Update workaround 14018778641 (rev4)
URL   : https://patchwork.freedesktop.org/series/119517/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14662 -> Patchwork_119517v4
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_119517v4 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_119517v4, please notify your bug team ('I915-ci-infra@lists.freedesktop.org') to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/index.html

Participating hosts (42 -> 34)
------------------------------

  Additional (1): bat-dg2-11 
  Missing    (9): fi-kbl-7567u bat-kbl-2 fi-snb-2520m fi-glk-j4005 fi-kbl-8809g fi-cfl-8109u fi-elk-e7500 bat-jsl-1 bat-mtlp-6 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_119517v4:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gt_timelines:
    - bat-arls-2:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14662/bat-arls-2/igt@i915_selftest@live@gt_timelines.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-arls-2/igt@i915_selftest@live@gt_timelines.html

  
Known issues
------------

  Here are the changes found in Patchwork_119517v4 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap@basic:
    - bat-dg2-11:         NOTRUN -> [SKIP][3] ([i915#4083])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@gem_mmap@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg2-11:         NOTRUN -> [SKIP][4] ([i915#4077]) +2 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@gem_tiled_fence_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-dg2-11:         NOTRUN -> [SKIP][5] ([i915#4079]) +1 other test skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-11:         NOTRUN -> [SKIP][6] ([i915#6621])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-nick:        [PASS][7] -> [ABORT][8] ([i915#10594])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14662/fi-bsw-nick/igt@i915_selftest@live@execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/fi-bsw-nick/igt@i915_selftest@live@execlists.html

  * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy:
    - bat-dg2-11:         NOTRUN -> [SKIP][9] ([i915#4212]) +7 other tests skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-dg2-11:         NOTRUN -> [SKIP][10] ([i915#5190])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg2-11:         NOTRUN -> [SKIP][11] ([i915#4215] / [i915#5190])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - bat-dg2-11:         NOTRUN -> [SKIP][12] ([i915#4103] / [i915#4213]) +1 other test skip
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_dsc@dsc-basic:
    - bat-dg2-11:         NOTRUN -> [SKIP][13] ([i915#3555] / [i915#3840])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-dg2-11:         NOTRUN -> [SKIP][14]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-dg2-11:         NOTRUN -> [SKIP][15] ([i915#5274])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-dg2-11:         NOTRUN -> [SKIP][16] ([i915#5354])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_psr@psr-sprite-plane-onoff:
    - bat-dg2-11:         NOTRUN -> [SKIP][17] ([i915#1072] / [i915#9732]) +3 other tests skip
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_psr@psr-sprite-plane-onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg2-11:         NOTRUN -> [SKIP][18] ([i915#3555])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-dg2-11:         NOTRUN -> [SKIP][19] ([i915#3708])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-dg2-11:         NOTRUN -> [SKIP][20] ([i915#3708] / [i915#4077]) +1 other test skip
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-read:
    - bat-dg2-11:         NOTRUN -> [SKIP][21] ([i915#3291] / [i915#3708]) +2 other tests skip
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/bat-dg2-11/igt@prime_vgem@basic-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#10436]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10436
  [i915#10594]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10594
  [i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
  [i915#3291]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3291
  [i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3840
  [i915#4077]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4215
  [i915#5190]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/6621
  [i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732


Build changes
-------------

  * Linux: CI_DRM_14662 -> Patchwork_119517v4

  CI-20190529: 20190529
  CI_DRM_14662: f39ba481e5873b7617afc2e8cf618ac9dc85123f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7824: 7824
  Patchwork_119517v4: f39ba481e5873b7617afc2e8cf618ac9dc85123f @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119517v4/index.html

[-- Attachment #2: Type: text/html, Size: 9459 bytes --]

Re: [PATCH] drm/i915/mtl: Update workaround 14018778641

[Andi Shyti]

Hi Angus,

...

> @@ -1586,6 +1586,8 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  	 */
>  	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
>  
> +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);

Can you please add the reference of the workaround in a comment
here?

Thanks,
Andi

> +

Re: [PATCH] drm/i915/mtl: Update workaround 14018778641

[Chen, Angus]

The original workaround was from 
https://patchwork.freedesktop.org/patch/533574/
Then, we don't apply WA to VDBOX, VEBOX and RENDER engine with
the following patch 
https://patchwork.freedesktop.org/patch/543117/?series=119517&rev=1
But now we need this WA to cover VDBOX.

On 4/29/2024 5:53 PM, Andi Shyti wrote:
> Hi Angus,
>
> ...
>
>> @@ -1586,6 +1586,8 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>>   	 */
>>   	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
>>   
>> +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> Can you please add the reference of the workaround in a comment
> here?
>
> Thanks,
> Andi
>
>> +

RE: [PATCH] drm/i915/mtl: Update workaround 14018778641

[Upadhyay, Tejas]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Chen,
> Angus
> Sent: Tuesday, April 30, 2024 4:50 PM
> To: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/mtl: Update workaround 14018778641
> 
> The original workaround was from
> https://patchwork.freedesktop.org/patch/533574/
> Then, we don't apply WA to VDBOX, VEBOX and RENDER engine with the
> following patch
> https://patchwork.freedesktop.org/patch/543117/?series=119517&rev=1
> But now we need this WA to cover VDBOX.
> 
> On 4/29/2024 5:53 PM, Andi Shyti wrote:
> > Hi Angus,
> >
> > ...
> >
> >> @@ -1586,6 +1586,8 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt,
> struct i915_wa_list *wal)
> >>   	 */
> >>   	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
> >>
> >> +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> > Can you please add the reference of the workaround in a comment here?

As far as I know media team confirmed that there is no perf impact and this WA can be removed. As Andi said, reference would be help here.

Thanks,
Tejas
> >
> > Thanks,
> > Andi
> >
> >> +

[PATCH v2] drm/i915/mtl: Update workaround 14018778641

[Chen, Angus]

From: Angus Chen <angus.chen@intel.com>

Applying it to VDBOX after recent performance data on MTL

Signed-off-by: Angus Chen <angus.chen@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index f3332bc55b8f..8432fb4fd28d 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1695,6 +1695,9 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
 	 */
 	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
 
+	/* Wa_14018778641 */
+	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
+
 	debug_dump_steering(gt);
 }
 
-- 
2.34.1

Re: [PATCH v2] drm/i915/mtl: Update workaround 14018778641

[Chen, Angus]

Please ignore this patch. (Wrong branch)

On 5/13/2024 9:29 PM, Chen, Angus wrote:
> From: Angus Chen <angus.chen@intel.com>
>
> Applying it to VDBOX after recent performance data on MTL
>
> Signed-off-by: Angus Chen <angus.chen@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index f3332bc55b8f..8432fb4fd28d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1695,6 +1695,9 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>   	 */
>   	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
>   
> +	/* Wa_14018778641 */
> +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> +
>   	debug_dump_steering(gt);
>   }
>   

[PATCH v2] drm/i915/mtl: Update workaround 14018778641

[Chen, Angus]

The WA should be extended to cover VDBOX engine. We found that
28-channels 1080p VP9 encoding may hit this issue.

Signed-off-by: Chen, Angus <angus.chen@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index d1ab560fcdfc..da0a481a375e 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1586,6 +1586,9 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
 	 */
 	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
 
+	/* Wa_14018778641 */
+	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
+
 	/* Wa_22016670082 */
 	wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE);
 
-- 
2.34.1

Re: [PATCH v2] drm/i915/mtl: Update workaround 14018778641

[Andi Shyti]

Hi Angus,

On Mon, May 13, 2024 at 02:19:17PM +0000, Chen, Angus wrote:
> The WA should be extended to cover VDBOX engine. We found that
> 28-channels 1080p VP9 encoding may hit this issue.
> 
> Signed-off-by: Chen, Angus <angus.chen@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index d1ab560fcdfc..da0a481a375e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1586,6 +1586,9 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  	 */
>  	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
>  
> +	/* Wa_14018778641 */
> +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);

Wa_14018778641 says that we need to disable the FTLB for Compute,
Render, GSC, VDBox and VEBox engines, but here we are doing it
only for GSC and VDBox, why?

Besides, in MTL we have the media GT where the MOD_CTRL family
has address 0x38cf34. Should this be checked and included, as
well?

Thanks,
Andi

>  	/* Wa_22016670082 */
>  	wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE);
>  
> -- 
> 2.34.1

Re: [PATCH v2] drm/i915/mtl: Update workaround 14018778641

[Matt Roper]

On Fri, May 24, 2024 at 12:31:26AM +0200, Andi Shyti wrote:
> Hi Angus,
> 
> On Mon, May 13, 2024 at 02:19:17PM +0000, Chen, Angus wrote:
> > The WA should be extended to cover VDBOX engine. We found that
> > 28-channels 1080p VP9 encoding may hit this issue.
> > 
> > Signed-off-by: Chen, Angus <angus.chen@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index d1ab560fcdfc..da0a481a375e 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -1586,6 +1586,9 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> >  	 */
> >  	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
> >  
> > +	/* Wa_14018778641 */

I realize that the comment farther up in the code is wrong, but there's
no such workaround as "Wa_14018778641."  14018778641 is just an internal
database ID that isn't meaningful for tracking workarounds in code.
Workarounds are always identified by their "lineage" number, which is
the number that will identify the workaround in a consistent manner
across multiple platforms.  In this case it sounds like the expected
workaround number was actually Wa_14018575942.

> > +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> 
> Wa_14018778641 says that we need to disable the FTLB for Compute,
> Render, GSC, VDBox and VEBox engines, but here we are doing it
> only for GSC and VDBox, why?

Wa_14018575942 (which is a follow-up to an older Wa_18018781329), was
originally supposed to apply to all engines.  But after some
investigation, the hardware teams decided that it was _probably_ only
needed on the CCS engines so they suggested dropping the workaround from
other engine types to reclaim performance unless we started seeing
functional issues when doing so.  At some point someone did report some
functional issues with the RCS engine, so the workaround got restored
there.  Based on this patch, it sounds like the media team is now
reporting that they also see functional failures on the VD engines
without the workaround, so it also needs to be restored there now.

> 
> Besides, in MTL we have the media GT where the MOD_CTRL family
> has address 0x38cf34. Should this be checked and included, as
> well?

The gt pointer passed into xelpmp_gt_workarounds_init() is always the
media GT.  And the GSI offset of 0x380000 gets added into the register
offset automatically so you don't need to worry about doing so manually.


Matt

> 
> Thanks,
> Andi
> 
> >  	/* Wa_22016670082 */
> >  	wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE);
> >  
> > -- 
> > 2.34.1

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH v2] drm/i915/mtl: Update workaround 14018778641

[Andi Shyti]

Hi,

> > On Mon, May 13, 2024 at 02:19:17PM +0000, Chen, Angus wrote:
> > > The WA should be extended to cover VDBOX engine. We found that
> > > 28-channels 1080p VP9 encoding may hit this issue.
> > > 
> > > Signed-off-by: Chen, Angus <angus.chen@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > index d1ab560fcdfc..da0a481a375e 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > @@ -1586,6 +1586,9 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> > >  	 */
> > >  	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
> > >  
> > > +	/* Wa_14018778641 */
> 
> I realize that the comment farther up in the code is wrong, but there's
> no such workaround as "Wa_14018778641."  14018778641 is just an internal
> database ID that isn't meaningful for tracking workarounds in code.
> Workarounds are always identified by their "lineage" number, which is
> the number that will identify the workaround in a consistent manner
> across multiple platforms.  In this case it sounds like the expected
> workaround number was actually Wa_14018575942.

Indeed... should this be updated accordingly?

> > > +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> > 
> > Wa_14018778641 says that we need to disable the FTLB for Compute,
> > Render, GSC, VDBox and VEBox engines, but here we are doing it
> > only for GSC and VDBox, why?
> 
> Wa_14018575942 (which is a follow-up to an older Wa_18018781329), was
> originally supposed to apply to all engines.  But after some
> investigation, the hardware teams decided that it was _probably_ only
> needed on the CCS engines so they suggested dropping the workaround from
> other engine types to reclaim performance unless we started seeing
> functional issues when doing so.  At some point someone did report some
> functional issues with the RCS engine, so the workaround got restored
> there.  Based on this patch, it sounds like the media team is now
> reporting that they also see functional failures on the VD engines
> without the workaround, so it also needs to be restored there now.

But I don't see it documented. Wa_14018575942 only speaks about
the GSC_MOD_CTL. We need it documented, otherwise we need a note
in the comment explaining why this workaround is needed here, as
well.

Otherwise, as it happened in other cases, someone might open this
workaround, think it's wrong and revert this patch.

Angus, can you please explain the reason why this workaround is
needed here and put it in a comment?

> > Besides, in MTL we have the media GT where the MOD_CTRL family
> > has address 0x38cf34. Should this be checked and included, as
> > well?
> 
> The gt pointer passed into xelpmp_gt_workarounds_init() is always the
> media GT.  And the GSI offset of 0x380000 gets added into the register
> offset automatically so you don't need to worry about doing so manually.

Correct, thanks!

Andi

Re: [PATCH v2] drm/i915/mtl: Update workaround 14018778641

[Matt Roper]

On Fri, May 24, 2024 at 01:26:41PM +0200, Andi Shyti wrote:
> Hi,
> 
> > > On Mon, May 13, 2024 at 02:19:17PM +0000, Chen, Angus wrote:
> > > > The WA should be extended to cover VDBOX engine. We found that
> > > > 28-channels 1080p VP9 encoding may hit this issue.
> > > > 
> > > > Signed-off-by: Chen, Angus <angus.chen@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > index d1ab560fcdfc..da0a481a375e 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > @@ -1586,6 +1586,9 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> > > >  	 */
> > > >  	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
> > > >  
> > > > +	/* Wa_14018778641 */
> > 
> > I realize that the comment farther up in the code is wrong, but there's
> > no such workaround as "Wa_14018778641."  14018778641 is just an internal
> > database ID that isn't meaningful for tracking workarounds in code.
> > Workarounds are always identified by their "lineage" number, which is
> > the number that will identify the workaround in a consistent manner
> > across multiple platforms.  In this case it sounds like the expected
> > workaround number was actually Wa_14018575942.
> 
> Indeed... should this be updated accordingly?

Yeah, it would definitely be good to update this since it's caused
confusion multiple times in the past as well.

> 
> > > > +	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
> > > 
> > > Wa_14018778641 says that we need to disable the FTLB for Compute,
> > > Render, GSC, VDBox and VEBox engines, but here we are doing it
> > > only for GSC and VDBox, why?
> > 
> > Wa_14018575942 (which is a follow-up to an older Wa_18018781329), was
> > originally supposed to apply to all engines.  But after some
> > investigation, the hardware teams decided that it was _probably_ only
> > needed on the CCS engines so they suggested dropping the workaround from
> > other engine types to reclaim performance unless we started seeing
> > functional issues when doing so.  At some point someone did report some
> > functional issues with the RCS engine, so the workaround got restored
> > there.  Based on this patch, it sounds like the media team is now
> > reporting that they also see functional failures on the VD engines
> > without the workaround, so it also needs to be restored there now.
> 
> But I don't see it documented. Wa_14018575942 only speaks about
> the GSC_MOD_CTL. We need it documented, otherwise we need a note
> in the comment explaining why this workaround is needed here, as
> well.

Documented in the workaround database you mean?  The description there
seems pretty clear to me --- they indicate that although the hardware
issue theoretically existed on all engines, their belief was that only
the CCS engines could generate the kind of memory traffic necessary to
cause problems, so the initial suggestion was to implement it only on
the CCS engine.  However the workaround also provides the details for
how to implement it on all the other engine types if necessary in case
real-world usage uncovers corner cases that wind up needing it.

Maybe we should try to get them to update the language a bit now that we
have reports that at least RCS and VCS do indeed seem to need the
workaround in real-world usage.


Matt

> 
> Otherwise, as it happened in other cases, someone might open this
> workaround, think it's wrong and revert this patch.
> 
> Angus, can you please explain the reason why this workaround is
> needed here and put it in a comment?
> 
> > > Besides, in MTL we have the media GT where the MOD_CTRL family
> > > has address 0x38cf34. Should this be checked and included, as
> > > well?
> > 
> > The gt pointer passed into xelpmp_gt_workarounds_init() is always the
> > media GT.  And the GSI offset of 0x380000 gets added into the register
> > offset automatically so you don't need to worry about doing so manually.
> 
> Correct, thanks!
> 
> Andi

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

[PATCH v3] drm/i915/mtl: Update workaround 14018575942

[Chen, Angus]

The WA should be extended to cover VDBOX engine. We found that
28-channels 1080p VP9 encoding may hit this issue.

v3: update the WA number and explain the reason why
    this workaround is needed
v2: add WA number
v1: initial version

Signed-off-by: Chen, Angus <angus.chen@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index d1ab560fcdfc..05d56103ddab 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1586,6 +1586,14 @@ xelpmp_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
 	 */
 	wa_write_or(wal, XELPMP_GSC_MOD_CTRL, FORCE_MISS_FTLB);
 
+	/*
+	 * Wa_14018575942
+	 *
+	 * Issue is seen on media KPI test running on VDBOX engine
+	 * especially VP9 encoding WLs
+	 */
+	wa_write_or(wal, XELPMP_VDBX_MOD_CTRL, FORCE_MISS_FTLB);
+
 	/* Wa_22016670082 */
 	wa_write_or(wal, GEN12_SQCNT1, GEN12_STRICT_RAR_ENABLE);
 
-- 
2.34.1

Re: [PATCH v3] drm/i915/mtl: Update workaround 14018575942

[Andi Shyti]

Hi Angus,

On Fri, May 24, 2024 at 05:33:49PM +0000, Chen, Angus wrote:
> The WA should be extended to cover VDBOX engine. We found that
> 28-channels 1080p VP9 encoding may hit this issue.
> 
> v3: update the WA number and explain the reason why
>     this workaround is needed
> v2: add WA number
> v1: initial version
> 
> Signed-off-by: Chen, Angus <angus.chen@intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks Matt for your feedback!

Andi

Re: [PATCH v3] drm/i915/mtl: Update workaround 14018575942

[Andi Shyti]

Hi Angus,

On Fri, May 24, 2024 at 05:33:49PM +0000, Chen, Angus wrote:
> The WA should be extended to cover VDBOX engine. We found that
> 28-channels 1080p VP9 encoding may hit this issue.
> 
> v3: update the WA number and explain the reason why
>     this workaround is needed
> v2: add WA number
> v1: initial version
> 
> Signed-off-by: Chen, Angus <angus.chen@intel.com>

I had to do some manual work here, including the name/surname
reordring, because patchwork couldn't handle the --in-reply-to.

In any case, merged to drm-intel-gt-next.

Thanks,
Andi