[Intel-xe] [PATCH 2/3] drm/xe: Add dbg messages for LRC WAs

[Lucas De Marchi]

On Tue, Sep 05, 2023 at 09:06:44AM -0700, Lucas De Marchi wrote:
>On Tue, Sep 05, 2023 at 05:04:08PM +0200, Michal Wajdeczko wrote:
>>
>>
>>On 05.09.2023 16:31, Lucas De Marchi wrote:
>>>Just like the GT and engine workarounds, add debug message with the
>>>final value being written to the register for easy debugging.
>>>
>>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>>---
>>> drivers/gpu/drm/xe/xe_gt.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>>diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>>>index 3d6a7c11bac1..bd307770a620 100644
>>>--- a/drivers/gpu/drm/xe/xe_gt.c
>>>+++ b/drivers/gpu/drm/xe/xe_gt.c
>>>@@ -116,6 +116,7 @@ static int emit_nop_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>>
>>> static int emit_wa_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>> {
>>>+	struct xe_device *xe = gt_to_xe(gt);
>>> 	struct xe_reg_sr *sr = &q->hwe->reg_lrc;
>>> 	struct xe_reg_sr_entry *entry;
>>> 	unsigned long reg;
>>>@@ -133,10 +134,14 @@ static int emit_wa_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>> 		++count;
>>>
>>> 	if (count) {
>>>+		drm_dbg(&xe->drm, "LRC WA %s save-restore MMIOs\n", sr->name);
>>
>>shouldn't we use xe_gt_dbg() here?
>
>I'm not sure here... maybe. The LRC WAs should be saved per engine
>class. Currently they are saved per gt, which is not entirely correct...
>we end up applying more workarounds than we really should.  This is
>another fix to be done on top.
>
>for the current situation, as long as we also migrate the gt/engines
>workaround application to use xe_gt_dbg(), then we can also change them
>here.

with xe_gt_dbg and s/MMIOs/batch/ since we are not really changing them via mmio:

xe 0000:00:02.0: [drm:xe_guc_ct_enable [xe]] GuC CT communication channel enabled
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: LRC WA rcs0 save-restore MMIOs
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x2580] = 0x00060002
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x6604] = 0xe0efff6f
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7018] = 0x20002000
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7300] = 0x00400040
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7304] = 0x02000200
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: LRC WA bcs0 save-restore MMIOs
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x2580] = 0x00060002
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x6604] = 0xe0efff6f
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7018] = 0x20002000
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7300] = 0x00400040
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7304] = 0x02000200
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x22204] = 0x00000606
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: LRC WA vcs0 save-restore MMIOs
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x2580] = 0x00060002
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x6604] = 0xe0efff6f
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7018] = 0x20002000
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7300] = 0x00400040
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7304] = 0x02000200
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: LRC WA vecs0 save-restore MMIOs
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x2580] = 0x00060002
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x6604] = 0xe0efff6f
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7018] = 0x20002000
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7300] = 0x00400040
xe 0000:00:02.0: [drm:xe_gt_record_default_lrcs [xe]] GT0: REG[0x7304] = 0x02000200

but will probably do that in the other workarounds first so they match
the format.

Lucas De Marchi

>
>thanks
>Lucas De Marchi
>
>>
>>>+
>>> 		bb->cs[bb->len++] = MI_LOAD_REGISTER_IMM(count);
>>> 		xa_for_each(&sr->xa, reg, entry) {
>>> 			bb->cs[bb->len++] = reg;
>>> 			bb->cs[bb->len++] = entry->set_bits;
>>>+			drm_dbg(&xe->drm, "REG[0x%lx] = 0x%08x", reg,
>>>+				entry->set_bits);
>>> 		}
>>> 	}
>>>