From: Lucas De Marchi Date: Wed, 6 Sep 2023 01:20:53 +0000 (-0700) Subject: drm/xe: Fix LRC workarounds X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=278c35822d61ae53d3a1d162b29adda671b11e3b;p=linux.git drm/xe: Fix LRC workarounds Fix 2 issues when writing LRC workarounds by copying the same handling done when processing other RTP entries: For masked registers, it was not correctly setting the upper 16bits. Differently than i915, the entry itself doesn't set the upper bits for masked registers: this is done when applying them. Testing on ADL-P: Before: [drm:xe_gt_record_default_lrcs [xe]] LRC WA rcs0 save-restore MMIOs [drm:xe_gt_record_default_lrcs [xe]] REG[0x2580] = 0x00000002 ... [drm:xe_gt_record_default_lrcs [xe]] REG[0x7018] = 0x00002000 [drm:xe_gt_record_default_lrcs [xe]] REG[0x7300] = 0x00000040 [drm:xe_gt_record_default_lrcs [xe]] REG[0x7304] = 0x00000200 After: [drm:xe_gt_record_default_lrcs [xe]] LRC WA rcs0 save-restore MMIOs [drm:xe_gt_record_default_lrcs [xe]] REG[0x2580] = 0x00060002 ... [drm:xe_gt_record_default_lrcs [xe]] REG[0x7018] = 0x20002000 [drm:xe_gt_record_default_lrcs [xe]] REG[0x7300] = 0x00400040 [drm:xe_gt_record_default_lrcs [xe]] REG[0x7304] = 0x02000200 All of these registers are masked registers, so writing to them without the relevant bits in the upper 16b doesn't have any effect. Also, this adds support to regular registers; previously it was assumed that LRC entries would only contain masked registers. However this is not true. 0x6604 is not a masked register, but used in workarounds for e.g. ADL-P. See commit 28cf243a341a ("drm/i915/gt: Fix context workarounds with non-masked regs"). In the same test with ADL-P as above: Before: [drm:xe_gt_record_default_lrcs [xe]] REG[0x6604] = 0xe0000000 After: [drm:xe_gt_record_default_lrcs [xe]] REG[0x6604] = 0xe0efef6f As can be seen, now it will read what was in the register rather than completely overwrite the other bits. Reviewed-by: Matt Roper Link: https://lore.kernel.org/r/20230906012053.1733755-5-lucas.demarchi@intel.com Signed-off-by: Lucas De Marchi Signed-off-by: Rodrigo Vivi --- diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index 9e226b8a005af..678a276a25dc4 100644 --- a/drivers/gpu/drm/xe/xe_gt.c +++ b/drivers/gpu/drm/xe/xe_gt.c @@ -114,11 +114,20 @@ static int emit_nop_job(struct xe_gt *gt, struct xe_exec_queue *q) return 0; } +/* + * Convert back from encoded value to type-safe, only to be used when reg.mcr + * is true + */ +static struct xe_reg_mcr to_xe_reg_mcr(const struct xe_reg reg) +{ + return (const struct xe_reg_mcr){.__reg.raw = reg.raw }; +} + static int emit_wa_job(struct xe_gt *gt, struct xe_exec_queue *q) { struct xe_reg_sr *sr = &q->hwe->reg_lrc; struct xe_reg_sr_entry *entry; - unsigned long reg; + unsigned long idx; struct xe_sched_job *job; struct xe_bb *bb; struct dma_fence *fence; @@ -129,18 +138,36 @@ static int emit_wa_job(struct xe_gt *gt, struct xe_exec_queue *q) if (IS_ERR(bb)) return PTR_ERR(bb); - xa_for_each(&sr->xa, reg, entry) + xa_for_each(&sr->xa, idx, entry) ++count; if (count) { xe_gt_dbg(gt, "LRC WA %s save-restore batch\n", sr->name); 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; - xe_gt_dbg(gt, "REG[0x%lx] = 0x%08x", reg, - entry->set_bits); + + xa_for_each(&sr->xa, idx, entry) { + struct xe_reg reg = entry->reg; + struct xe_reg_mcr reg_mcr = to_xe_reg_mcr(reg); + u32 val; + + /* + * Skip reading the register if it's not really needed + */ + if (reg.masked) + val = entry->clr_bits << 16; + else if (entry->clr_bits + 1) + val = (reg.mcr ? + xe_gt_mcr_unicast_read_any(gt, reg_mcr) : + xe_mmio_read32(gt, reg)) & (~entry->clr_bits); + else + val = 0; + + val |= entry->set_bits; + + bb->cs[bb->len++] = reg.addr; + bb->cs[bb->len++] = val; + xe_gt_dbg(gt, "REG[0x%x] = 0x%08x", reg.addr, val); } }