On 11/9/2023 12:35 PM, Ville Syrjälä wrote:
On Thu, Nov 09, 2023 at 12:01:26PM -0800, Belgaumkar, Vinay wrote:
On 11/9/2023 11:30 AM, Ville Syrjälä wrote:
On Thu, Nov 09, 2023 at 11:21:48AM -0800, Vinay Belgaumkar wrote:
We read RENDER_HEAD as a part of the flush. If GT is in
deeper sleep states, this could lead to read errors since we are
not using a forcewake. Safer to read a shadowed register instead.
IIRC shadowing is only thing for writes, not reads.
Sure, but reading from a shadowed register does return the cached value
Does it? I suppose that would make some sense, but I don't recall that
ever being stated anywhere. At least before the shadow registers
existed reads would just give you zeroes when not awake.
(even though we don't care about the vakue here). When GT is in deeper
sleep states, it is better to read a shadowed (cached) value instead of
trying to attempt an mmio register read without a force wake anyways.
So you're saying reads from non-shadowed registers fails somehow
when not awake? How exactly do they fail? And when reading from
a shadowed register that failure never happens?
We could hit problems like the one being addressed here -
https://patchwork.freedesktop.org/series/125356/. ; Reading from a
shadowed register will avoid any needless references(without a wake) to
the MMIO space. Shouldn't hurt to make this change for all gens IMO.
Thanks,
Vinay.
Thanks,
Vinay.
Cc: John Harrison <john.c.harrison@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_gt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index ed32bf5b1546..ea814ea5f700 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -451,7 +451,7 @@ void intel_gt_flush_ggtt_writes(struct intel_gt *gt)
spin_lock_irqsave(&uncore->lock, flags);
intel_uncore_posting_read_fw(uncore,
- RING_HEAD(RENDER_RING_BASE));
+ RING_TAIL(RENDER_RING_BASE));
spin_unlock_irqrestore(&uncore->lock, flags);
}
}
--
2.38.1
