[Intel-gfx] [PATCH] drm/i915/overlay: Fully configure overlay before turning on

[Chris Wilson]

We seem to trigger an occasional error when the GPU tries to dereference
a PTE for an overlay plane prior to us populating the offsets in the
overlay registers. This is despite the Overlay Command being set to off.
Close this window delaying the switch on until after those registers are
initialised.

In conjunction with inspecting the state of how we switch on the
overlay, we review when we mark the overlay as active. The overlay
becomes active when we queue the commands to change state in the
ringbuffer, and not before a potential failure.

References: https://bugs.freedesktop.org/show_bug.cgi?id=48939
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_overlay.c |   47 +++++++++++++++-------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 22bb856..139506f 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -205,6 +205,7 @@ intel_overlay_map_regs(struct intel_overlay *overlay)
 static void intel_overlay_unmap_regs(struct intel_overlay *overlay,
 				     struct overlay_registers __iomem *regs)
 {
+	ioread32(regs); /* flush all writes */
 	if (!OVERLAY_NEEDS_PHYSICAL(overlay->dev))
 		io_mapping_unmap(regs);
 }
@@ -291,9 +292,6 @@ static int intel_overlay_on(struct intel_overlay *overlay)
 	int pipe_a_quirk = 0;
 	int ret;
 
-	BUG_ON(overlay->active);
-	overlay->active = 1;
-
 	if (IS_I830(dev)) {
 		pipe_a_quirk = i830_activate_pipe_a(dev);
 		if (pipe_a_quirk < 0)
@@ -318,6 +316,8 @@ static int intel_overlay_on(struct intel_overlay *overlay)
 	OUT_RING(MI_NOOP);
 	ADVANCE_LP_RING();
 
+	overlay->active = 1;
+
 	ret = intel_overlay_do_wait_request(overlay, request, NULL);
 out:
 	if (pipe_a_quirk)
@@ -337,8 +337,6 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
 	u32 tmp;
 	int ret;
 
-	BUG_ON(!overlay->active);
-
 	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
 		return -ENOMEM;
@@ -393,7 +391,6 @@ static void intel_overlay_off_tail(struct intel_overlay *overlay)
 
 	overlay->crtc->overlay = NULL;
 	overlay->crtc = NULL;
-	overlay->active = 0;
 }
 
 /* overlay needs to be disabled in OCMD reg */
@@ -432,6 +429,8 @@ static int intel_overlay_off(struct intel_overlay *overlay)
 	OUT_RING(MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
 	ADVANCE_LP_RING();
 
+	overlay->active = 0;
+
 	return intel_overlay_do_wait_request(overlay, request,
 					     intel_overlay_off_tail);
 }
@@ -788,26 +787,6 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 	if (ret)
 		goto out_unpin;
 
-	if (!overlay->active) {
-		u32 oconfig;
-		regs = intel_overlay_map_regs(overlay);
-		if (!regs) {
-			ret = -ENOMEM;
-			goto out_unpin;
-		}
-		oconfig = OCONF_CC_OUT_8BIT;
-		if (IS_GEN4(overlay->dev))
-			oconfig |= OCONF_CSC_MODE_BT709;
-		oconfig |= overlay->crtc->pipe == 0 ?
-			OCONF_PIPE_A : OCONF_PIPE_B;
-		iowrite32(oconfig, &regs->OCONFIG);
-		intel_overlay_unmap_regs(overlay, regs);
-
-		ret = intel_overlay_on(overlay);
-		if (ret != 0)
-			goto out_unpin;
-	}
-
 	regs = intel_overlay_map_regs(overlay);
 	if (!regs) {
 		ret = -ENOMEM;
@@ -855,9 +834,23 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 
 	iowrite32(overlay_cmd_reg(params), &regs->OCMD);
 
+	if (!overlay->active) {
+		u32 oconfig;
+
+		oconfig = OCONF_CC_OUT_8BIT;
+		if (IS_GEN4(overlay->dev))
+			oconfig |= OCONF_CSC_MODE_BT709;
+		if (overlay->crtc->pipe)
+			oconfig |= OCONF_PIPE_B;
+		iowrite32(oconfig, &regs->OCONFIG);
+	}
+
 	intel_overlay_unmap_regs(overlay, regs);
 
-	ret = intel_overlay_continue(overlay, scale_changed);
+	if (!overlay->active)
+		ret = intel_overlay_on(overlay);
+	else
+		ret = intel_overlay_continue(overlay, scale_changed);
 	if (ret)
 		goto out_unpin;
 
-- 
1.7.10