On 9/15/2020 7:49 PM, Ville Syrjälä wrote:
On Mon, Sep 14, 2020 at 11:26:31AM +0530, Karthik B S wrote:
In Gen 9 and Gen 10 platforms, async address update enable bit is
double buffered. Due to this, during the transition from async flip
to sync flip we have to wait until this bit is updated before continuing
with the normal commit for sync flip.
Signed-off-by: Karthik B S <karthik.b.s@xxxxxxxxx>
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_display.c | 44 ++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a0c17d94daf3..b7e24dff0772 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15360,6 +15360,42 @@ static void intel_enable_crtc(struct intel_atomic_state *state,
intel_crtc_enable_pipe_crc(crtc);
}
+static void skl_toggle_async_sync(struct intel_atomic_state *state,
skl_disable_async_flip_wa() maybe?
Thanks for the review.
I'll change the name.
+ struct intel_crtc *crtc,
+ struct intel_crtc_state *new_crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+ struct intel_plane *plane;
+ struct intel_plane_state *new_plane_state;
+ u32 update_mask = new_crtc_state->update_planes;
+ u32 plane_ctl, surf_addr;
+ enum plane_id plane_id;
+ unsigned long irqflags;
+ enum pipe pipe;
Most of these things are only needed within the loop, so that's where
the declarations should be.
Sure, I'll move it inside the loop.
+ int i;
+
+ for_each_new_intel_plane_in_state(state, plane, new_plane_state, i) {
+ if (crtc->pipe != plane->pipe ||
+ !(update_mask & BIT(plane->id)))
+ continue;
+
+ plane_id = plane->id;
+ pipe = plane->pipe;
+
I'd take the lock here so the rmw is fully protected.
Sure, I'll move it here.
+ plane_ctl = intel_de_read_fw(dev_priv, PLANE_CTL(pipe, plane_id));
+ surf_addr = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
+
+ plane_ctl &= ~PLANE_CTL_ASYNC_FLIP;
+
+ spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+ intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
+ intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), surf_addr);
+ spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+ }
+
+ intel_wait_for_vblank(dev_priv, crtc->pipe);
+}
+
static void intel_update_crtc(struct intel_atomic_state *state,
struct intel_crtc *crtc)
{
@@ -15387,6 +15423,14 @@ static void intel_update_crtc(struct intel_atomic_state *state,
else
intel_fbc_enable(state, crtc);
+ /* WA for older platforms where async address update enable bit
s/older//
Noted.
+ * is double buffered.
"is double buffered and only latched at start of vblank" or
something. Otherwise one is left wondering what the fuss is about.
Sure, I'll update this.
+ */
Multiline comment format should be
/*
* blah
* blah
*/
I'll fix this.
+ if (old_crtc_state->uapi.async_flip &&
+ !new_crtc_state->uapi.async_flip &&
+ INTEL_GEN(dev_priv) <= 10 && INTEL_GEN(dev_priv) >= 9)
IS_GEN_RANGE(9, 10) or whatever it's called.
Sure, I'll update this.
I guess we might want to put this call into intel_pre_plane_update()
since that's where all kinds of similar wait_for_vblank w/as live.
Sure, I'll move this to intel_pre_plane_update()
Thanks,
Karthik.B.S
+ skl_toggle_async_sync(state, crtc, new_crtc_state);
+
/* Perform vblank evasion around commit operation */
intel_pipe_update_start(new_crtc_state);
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
