From: Gustavo Sousa <gustavo.sousa@intel.com>
To: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>,
"Lucas De Marchi" <lucas.demarchi@intel.com>
Cc: <intel-xe@lists.freedesktop.org>,
Matt Roper <matthew.d.roper@intel.com>,
Bommu Krishnaiah <krishnaiah.bommu@intel.com>,
Tejas Upadhyay <tejas.upadhyay@intel.com>
Subject: Re: [PATCH v3 08/11] drm/xe/xe2: Add workaround 18034896535
Date: Mon, 29 Apr 2024 10:51:36 -0300 [thread overview]
Message-ID: <171439869685.11427.366527084236681194@gjsousa-mobl2> (raw)
In-Reply-To: <sw42yhgzfjqdvkhwfzyrjh46ncle2zq5mzohnzxlria5xqsgns@li2vup2xf4ea>
Quoting Lucas De Marchi (2024-04-26 18:02:49-03:00)
>On Mon, Apr 08, 2024 at 10:35:42PM GMT, Balasubramani Vivekanandan wrote:
>>From: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
>>
>>Add 18034896535 as driver permanent workaround.
>>
>>v2: 18034896535 and 16021540221 are two independent workarounds
>>that just happen to have the same implementation, hence keeping it.
>
>that doesn't work though. Now we get an error in LNL:
>
> xe 0000:00:02.0: [drm] *ERROR* GT0: [GT OTHER] discarding save-restore reg e48c (clear: 00000200, set: 00000200, masked: yes, mcr: yes): ret=-22
>
>That's only visible on stepping A* in LNL. I'm seeing it on a LNL I
>have:
>
> xe 0000:00:02.0: [drm:xe_pci_probe [xe]] XE_LUNARLAKE 64a0:0001 dgfx:0 gfx:Xe2_LPG / Xe2_HPG (20.04) media:Xe2_LPM / Xe2_HPM (20.00) display:no dma_m_s:46 tc:1 gscfi:0 cscfi:0
> xe 0000:00:02.0: [drm:xe_pci_probe [xe]] Stepping = (G:A1, M:A0, D:**, B:**)
>
>It looks like in CI we have B0, so we don't see the issue.
>
> xe 0000:00:02.0: [drm:xe_pci_probe [xe]] XE_LUNARLAKE 64a0:0004 dgfx:0 gfx:Xe2_LPG / Xe2_HPG (20.04) media:Xe2_LPM / Xe2_HPM (20.00) display:yes dma_m_s:46 tc:1 gscfi:0
> xe 0000:00:02.0: [drm:xe_pci_probe [xe]] Stepping = (G:B0, M:B0, D:**, B:**)
>
>We may need to revisit 2 things in the RTP matching:
>
> 1) The stepping is non-inclusive, while the version is
> inclusive.... this is confusing
I believe that behavior comes from the way we did range checks in i915
and is intentional.
For temporary workarounds, we usually have the stepping that fixes the
related hardware bug, so the workaround usually applies to steps before
that one. An exclusive ending value is useful here, since we can just
use the stepping with the fix and "forget" about it. If the end of the
range was inclusive, we would need to keep track of latest applicable
stepping.
Now, even if the workaround is temporary, we usually do have a defined
range of IP releases for which it is applicable and it makes sense to
have a closed end for version ranges.
--
Gustavo Sousa
> 2) Maybe we should just log rather than giving an error if the
> value is exactly the same?
>
> Currently we do:
>
> /*
> * Don't allow overwriting values: clr_bits/set_bits should be disjoint
> * when operating in the same register
> */
> if (e1->clr_bits & e2->clr_bits || e1->set_bits & e2->set_bits ||
> e1->clr_bits & e2->set_bits || e1->set_bits & e2->clr_bits)
> return false;
>
> I remember this was useful to find duplicate workarounds though
> (or some that we were implementing both as WA and tuning)
>
>For now I'm thinking to just merge the entries and add a comment.
>
>$ git diff
>diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
>index 9d9b7fa7a8f0..db7c7c7875c5 100644
>--- a/drivers/gpu/drm/xe/xe_wa.c
>+++ b/drivers/gpu/drm/xe/xe_wa.c
>@@ -449,12 +449,7 @@ static const struct xe_rtp_entry_sr engine_was[] = {
> XE_RTP_RULES(GRAPHICS_VERSION(2004), FUNC(xe_rtp_match_first_render_or_compute)),
> XE_RTP_ACTIONS(SET(ROW_CHICKEN3, XE2_EUPEND_CHK_FLUSH_DIS))
> },
>- { XE_RTP_NAME("16021540221"),
>- XE_RTP_RULES(GRAPHICS_VERSION(2004), GRAPHICS_STEP(A0, B0),
>- FUNC(xe_rtp_match_first_render_or_compute)),
>- XE_RTP_ACTIONS(SET(ROW_CHICKEN4, DISABLE_TDL_PUSH))
>- },
>- { XE_RTP_NAME("18034896535"),
>+ { XE_RTP_NAME("18034896535, 16021540221"), /* 16021540221: GRAPHICS_STEP(A0, B0) */
> XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, 2004),
> FUNC(xe_rtp_match_first_render_or_compute)),
> XE_RTP_ACTIONS(SET(ROW_CHICKEN4, DISABLE_TDL_PUSH))
>
>
>Lucas De Marchi
>
>>
>>Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
>>Reviewed-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
>>Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
>>Cc: Matt Roper <matthew.d.roper@intel.com>
>>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>>---
>> drivers/gpu/drm/xe/xe_wa.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
>>index c904e55ced9c..43fac92e5d20 100644
>>--- a/drivers/gpu/drm/xe/xe_wa.c
>>+++ b/drivers/gpu/drm/xe/xe_wa.c
>>@@ -428,6 +428,11 @@ static const struct xe_rtp_entry_sr engine_was[] = {
>> FUNC(xe_rtp_match_first_render_or_compute)),
>> XE_RTP_ACTIONS(SET(ROW_CHICKEN4, DISABLE_TDL_PUSH))
>> },
>>+ { XE_RTP_NAME("18034896535"),
>>+ XE_RTP_RULES(GRAPHICS_VERSION(2004),
>>+ FUNC(xe_rtp_match_first_render_or_compute)),
>>+ XE_RTP_ACTIONS(SET(ROW_CHICKEN4, DISABLE_TDL_PUSH))
>>+ },
>> { XE_RTP_NAME("14019322943"),
>> XE_RTP_RULES(GRAPHICS_VERSION(2004), GRAPHICS_STEP(A0, B0),
>> FUNC(xe_rtp_match_first_render_or_compute)),
>>--
>>2.25.1
>>
next prev parent reply other threads:[~2024-04-29 13:51 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-08 17:05 [PATCH v3 00/11] Add Battlemage support Balasubramani Vivekanandan
2024-04-08 17:05 ` [PATCH v3 01/11] drm/xe/xe2: Recognize Xe2_HPG IP Balasubramani Vivekanandan
2024-04-08 17:05 ` [PATCH v3 02/11] drm/xe/xe2: Recognize Xe2_HPM IP Balasubramani Vivekanandan
2024-04-08 17:05 ` [PATCH v3 03/11] drm/xe/bmg: Add BMG platform definition Balasubramani Vivekanandan
2024-04-08 19:09 ` Ghimiray, Himal Prasad
2024-04-09 15:46 ` Lucas De Marchi
2024-04-08 17:05 ` [PATCH v3 04/11] drm/xe/bmg: Add BMG mocs table Balasubramani Vivekanandan
2024-04-08 17:05 ` [PATCH v3 05/11] drm/xe/bmg: Program an additional discrete-specific PAT setting Balasubramani Vivekanandan
2024-04-08 19:05 ` Ghimiray, Himal Prasad
2024-04-08 17:05 ` [PATCH v3 06/11] drm/xe/xe2hpg: Determine flat ccs offset for vram Balasubramani Vivekanandan
2024-04-08 21:44 ` Matt Roper
2024-04-08 17:05 ` [PATCH v3 07/11] drm/xe/xe2hpg: Remove extra allocation of CCS pages for dgfx Balasubramani Vivekanandan
2024-04-08 18:57 ` Ghimiray, Himal Prasad
2024-04-08 21:46 ` Matt Roper
2024-04-22 10:15 ` Thomas Hellström
2024-04-08 17:05 ` [PATCH v3 08/11] drm/xe/xe2: Add workaround 18034896535 Balasubramani Vivekanandan
2024-04-26 21:02 ` Lucas De Marchi
2024-04-29 13:51 ` Gustavo Sousa [this message]
2024-04-08 17:05 ` [PATCH v3 09/11] drm/xe/xe2hpg: Add initial GT workarounds Balasubramani Vivekanandan
2024-04-08 17:05 ` [PATCH v3 10/11] drm/xe/xe2hpg: Introduce performance tuning changes for Xe2_HPG Balasubramani Vivekanandan
2024-04-08 17:05 ` [PATCH v3 11/11] drm/xe/xe2hpm: Add initial set of workarounds Balasubramani Vivekanandan
2024-04-08 18:33 ` ✓ CI.Patch_applied: success for Add Battlemage support (rev3) Patchwork
2024-04-08 18:33 ` ✗ CI.checkpatch: warning " Patchwork
2024-04-08 18:34 ` ✓ CI.KUnit: success " Patchwork
2024-04-08 18:46 ` ✓ CI.Build: " Patchwork
2024-04-08 18:48 ` ✓ CI.Hooks: " Patchwork
2024-04-08 18:50 ` ✓ CI.checksparse: " Patchwork
2024-04-08 19:12 ` ✓ CI.BAT: " Patchwork
2024-04-09 0:34 ` ✓ CI.FULL: " Patchwork
2024-04-09 21:25 ` [PATCH v3 00/11] Add Battlemage support Matt Roper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=171439869685.11427.366527084236681194@gjsousa-mobl2 \
--to=gustavo.sousa@intel.com \
--cc=balasubramani.vivekanandan@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=krishnaiah.bommu@intel.com \
--cc=lucas.demarchi@intel.com \
--cc=matthew.d.roper@intel.com \
--cc=tejas.upadhyay@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.