On 7/28/2020 3:04 AM, Daniel Vetter wrote:
On Mon, Jul 27, 2020 at 2:27 PM Michel Dänzer <michel@xxxxxxxxxxx> wrote:
On 2020-07-25 1:26 a.m., Paulo Zanoni wrote:
Em seg, 2020-07-20 às 17:01 +0530, Karthik B S escreveu:
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1fa67700d8f4..95953b393941 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -697,14 +697,24 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc)
return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xffffff;
}
+static u32 g4x_get_flip_counter(struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+ enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+ return I915_READ(PIPE_FLIPCOUNT_G4X(pipe));
+}
+
u32 g4x_get_vblank_counter(struct drm_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
enum pipe pipe = to_intel_crtc(crtc)->pipe;
+ if (crtc->state->async_flip)
+ return g4x_get_flip_counter(crtc);
+
return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
I don't understand the intention behind this, can you please clarify?
This goes back to my reply of the cover letter. It seems that here
we're going to alternate between two different counters in our vblank
count. So if user space alternates between sometimes using async flips
and sometimes using normal flip it's going to get some very weird
deltas, isn't it? At least this is what I remember from when I played
with these registers: FLIPCOUNT drifts away from FRMCOUNT when we start
using async flips.
This definitely looks wrong. The counter value returned by the
get_vblank_counter hook is supposed to increment when a vertical blank
period occurs; page flips are not supposed to affect this in any way.
Also you just flat out can't access crtc->state from interrupt
context. Anything you need in there needs to be protected by the right
irq-type spin_lock, updates correctly synchronized against both the
interrupt handler and atomic updates, and data copied over, not
pointers. Otherwise just crash&burn.
Thanks for the review.
I will be removing this change in the next revision based on the
feedback received, but I will keep this in mind whenever I'll have to
access something from the interrupt context.
Thanks,
Karthik.B.S
-Daniel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
