[PATCH 00/11] drm/i915: Overlay fixes

[PATCH 00/11] drm/i915: Overlay fixes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Fixes for some recentish regressions in the overlay support (kernel
would oops as soon as you try to use the overlay), and I decided to
dig up a few older cleanups and fixes I had lying around in my
gen2 dungeon.

Entire series (with Chris's phys obj fix) here:
git://github.com/vsyrjala/linux.git overlay_fixes

Ville Syrjälä (11):
  drm/i915: Fix oops in overlay due to frontbuffer trakcing
  drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
  drm/i915: Restore dev_priv->mm.interruptible for overlay
  drm/i915: Fix the overlay frontbuffer tracking
  drm/i915: Use pipe_src_w in overlay code
  drm/i915: Kill intel_panel_fitter_pipe()
  drm/i915: Simplify SWIDTHSW calculation
  drm/i915: Reorganize overlay filter coeffs into a nicer form
  drm/i915: Use primary plane->state for overlay ckey setup
  drm/i915: Disable L2 cache clock gating on 830 when using the overlay
  drm/i915: Kill the 830 MI_OVERLAY_OFF workaround

 drivers/gpu/drm/i915/i915_gem_request.c |  36 +++++
 drivers/gpu/drm/i915/i915_gem_request.h |  35 +---
 drivers/gpu/drm/i915/i915_reg.h         |   4 +
 drivers/gpu/drm/i915/intel_display.c    |   2 +-
 drivers/gpu/drm/i915/intel_overlay.c    | 278 +++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_pm.c         |   2 -
 6 files changed, 193 insertions(+), 164 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/11] drm/i915: Fix oops in overlay due to frontbuffer trakcing

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The vma will be NULL if the overlay was previously off, so
dereferencing it will oops. Check for NULL before doing that.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Fixes: 9b3b7841b86d ("drm/i915/overlay: Use VMA as the primary tracker for images")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 354f8cec96bb..1cc963814224 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -839,8 +839,8 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 	if (ret)
 		goto out_unpin;
 
-	i915_gem_track_fb(overlay->vma->obj, new_bo,
-			  INTEL_FRONTBUFFER_OVERLAY(pipe));
+	i915_gem_track_fb(overlay->vma ? overlay->vma->obj : NULL,
+			  vma->obj, INTEL_FRONTBUFFER_OVERLAY(pipe));
 
 	overlay->old_vma = overlay->vma;
 	overlay->vma = vma;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/11] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The i915_gem_active stuff doesn't like a NULL ->retire hook, but
the overlay code can set it to NULL. That obviously ends up oopsing.
Fix it by setting the ->retire hook using init_request_active()
so that it'll do the NULL->i915_gem_retire_noop conversion for us.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 1cc963814224..786389dd5175 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -216,7 +216,7 @@ static void intel_overlay_submit_request(struct intel_overlay *overlay,
 {
 	GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
 					&overlay->i915->drm.struct_mutex));
-	overlay->last_flip.retire = retire;
+	init_request_active(&overlay->last_flip, retire);
 	i915_gem_active_set(&overlay->last_flip, req);
 	i915_add_request(req);
 }
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We need an uninterruptible wait for the overlay off request during
modeset. Restore the operation of dev_priv->mm.interruptible
sufficiently for that.

Toss in a WARN_ON() to make sure the request succeeds.

Fixes: 7da844c5c6fc ("drm/i915: Move the special case wait-request handling to its one caller")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 36 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_request.h | 35 ++------------------------------
 drivers/gpu/drm/i915/intel_display.c    |  2 +-
 3 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index fcf22b0e2967..1a7b88166c51 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1199,3 +1199,39 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
 	for_each_engine(engine, dev_priv, id)
 		engine_retire_requests(engine);
 }
+
+/**
+ * i915_gem_active_retire - waits until the request is retired
+ * @active - the active request on which to wait
+ *
+ * i915_gem_active_retire() waits until the request is completed,
+ * and then ensures that at least the retirement handler for this
+ * @active tracker is called before returning. If the @active
+ * tracker is idle, the function returns immediately.
+ */
+int __must_check
+i915_gem_active_retire(struct i915_gem_active *active,
+		       struct mutex *mutex)
+{
+	struct drm_i915_gem_request *request;
+	long ret;
+
+	request = i915_gem_active_raw(active, mutex);
+	if (!request)
+		return 0;
+
+	ret = i915_wait_request(request,
+				(request->i915->mm.interruptible ?
+				 I915_WAIT_INTERRUPTIBLE : 0) |
+				I915_WAIT_LOCKED,
+				MAX_SCHEDULE_TIMEOUT);
+	if (ret < 0)
+		return ret;
+
+	list_del_init(&active->link);
+	RCU_INIT_POINTER(active->request, NULL);
+
+	active->retire(active, request);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index e2b077df2da0..09add6b9cfc7 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -657,39 +657,8 @@ i915_gem_active_wait(const struct i915_gem_active *active, unsigned int flags)
 	return ret < 0 ? ret : 0;
 }
 
-/**
- * i915_gem_active_retire - waits until the request is retired
- * @active - the active request on which to wait
- *
- * i915_gem_active_retire() waits until the request is completed,
- * and then ensures that at least the retirement handler for this
- * @active tracker is called before returning. If the @active
- * tracker is idle, the function returns immediately.
- */
-static inline int __must_check
-i915_gem_active_retire(struct i915_gem_active *active,
-		       struct mutex *mutex)
-{
-	struct drm_i915_gem_request *request;
-	long ret;
-
-	request = i915_gem_active_raw(active, mutex);
-	if (!request)
-		return 0;
-
-	ret = i915_wait_request(request,
-				I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
-				MAX_SCHEDULE_TIMEOUT);
-	if (ret < 0)
-		return ret;
-
-	list_del_init(&active->link);
-	RCU_INIT_POINTER(active->request, NULL);
-
-	active->retire(active, request);
-
-	return 0;
-}
+int __must_check i915_gem_active_retire(struct i915_gem_active *active,
+					struct mutex *mutex);
 
 #define for_each_active(mask, idx) \
 	for (; mask ? idx = ffs(mask) - 1, 1 : 0; mask &= ~BIT(idx))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a41082e2750e..5a74991854e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4923,7 +4923,7 @@ static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc)
 
 		mutex_lock(&dev->struct_mutex);
 		dev_priv->mm.interruptible = false;
-		(void) intel_overlay_switch_off(intel_crtc->overlay);
+		WARN_ON(intel_overlay_switch_off(intel_crtc->overlay) != 0);
 		dev_priv->mm.interruptible = true;
 		mutex_unlock(&dev->struct_mutex);
 	}
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/11] drm/i915: Fix the overlay frontbuffer tracking

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do the overlay frontbuffer tracking properly so that it matches
the state of the overlay on/off/continue requests.

One slight problem is that intel_frontbuffer_flip_complete()
may get delayed by an arbitrarily liong time due to the fact that
the overlay code likes to bail out when a signal occurs. So the
flip may not get completed until the ioctl is restarted. But fixing
that would require bigger surgery, so I decided to ignore it for now.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 64 +++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 786389dd5175..0c5e82beda4a 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -271,8 +271,30 @@ static int intel_overlay_on(struct intel_overlay *overlay)
 	return intel_overlay_do_wait_request(overlay, req, NULL);
 }
 
+static void intel_overlay_flip_prepare(struct intel_overlay *overlay,
+				       struct i915_vma *vma)
+{
+	enum pipe pipe = overlay->crtc->pipe;
+
+	WARN_ON(overlay->old_vma);
+
+	i915_gem_track_fb(overlay->vma ? overlay->vma->obj : NULL,
+			  vma ? vma->obj : NULL,
+			  INTEL_FRONTBUFFER_OVERLAY(pipe));
+
+	intel_frontbuffer_flip_prepare(overlay->i915,
+				       INTEL_FRONTBUFFER_OVERLAY(pipe));
+
+	overlay->old_vma = overlay->vma;
+	if (vma)
+		overlay->vma = i915_vma_get(vma);
+	else
+		overlay->vma = NULL;
+}
+
 /* overlay needs to be enabled in OCMD reg */
 static int intel_overlay_continue(struct intel_overlay *overlay,
+				  struct i915_vma *vma,
 				  bool load_polyphase_filter)
 {
 	struct drm_i915_private *dev_priv = overlay->i915;
@@ -307,43 +329,44 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
 	intel_ring_emit(ring, flip_addr);
 	intel_ring_advance(ring);
 
+	intel_overlay_flip_prepare(overlay, vma);
+
 	intel_overlay_submit_request(overlay, req, NULL);
 
 	return 0;
 }
 
-static void intel_overlay_release_old_vid_tail(struct i915_gem_active *active,
-					       struct drm_i915_gem_request *req)
+static void intel_overlay_release_old_vma(struct intel_overlay *overlay)
 {
-	struct intel_overlay *overlay =
-		container_of(active, typeof(*overlay), last_flip);
 	struct i915_vma *vma;
 
 	vma = fetch_and_zero(&overlay->old_vma);
 	if (WARN_ON(!vma))
 		return;
 
-	i915_gem_track_fb(vma->obj, NULL,
-			  INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
+	intel_frontbuffer_flip_complete(overlay->i915,
+					INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
 
 	i915_gem_object_unpin_from_display_plane(vma);
 	i915_vma_put(vma);
 }
 
+static void intel_overlay_release_old_vid_tail(struct i915_gem_active *active,
+					       struct drm_i915_gem_request *req)
+{
+	struct intel_overlay *overlay =
+		container_of(active, typeof(*overlay), last_flip);
+
+	intel_overlay_release_old_vma(overlay);
+}
+
 static void intel_overlay_off_tail(struct i915_gem_active *active,
 				   struct drm_i915_gem_request *req)
 {
 	struct intel_overlay *overlay =
 		container_of(active, typeof(*overlay), last_flip);
-	struct i915_vma *vma;
 
-	/* never have the overlay hw on without showing a frame */
-	vma = fetch_and_zero(&overlay->vma);
-	if (WARN_ON(!vma))
-		return;
-
-	i915_gem_object_unpin_from_display_plane(vma);
-	i915_vma_put(vma);
+	intel_overlay_release_old_vma(overlay);
 
 	overlay->crtc->overlay = NULL;
 	overlay->crtc = NULL;
@@ -397,6 +420,8 @@ static int intel_overlay_off(struct intel_overlay *overlay)
 	}
 	intel_ring_advance(ring);
 
+	intel_overlay_flip_prepare(overlay, NULL);
+
 	return intel_overlay_do_wait_request(overlay, req,
 					     intel_overlay_off_tail);
 }
@@ -835,18 +860,10 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 
 	intel_overlay_unmap_regs(overlay, regs);
 
-	ret = intel_overlay_continue(overlay, scale_changed);
+	ret = intel_overlay_continue(overlay, vma, scale_changed);
 	if (ret)
 		goto out_unpin;
 
-	i915_gem_track_fb(overlay->vma ? overlay->vma->obj : NULL,
-			  vma->obj, INTEL_FRONTBUFFER_OVERLAY(pipe));
-
-	overlay->old_vma = overlay->vma;
-	overlay->vma = vma;
-
-	intel_frontbuffer_flip(dev_priv, INTEL_FRONTBUFFER_OVERLAY(pipe));
-
 	return 0;
 
 out_unpin:
@@ -1214,6 +1231,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 
 	mutex_unlock(&dev->struct_mutex);
 	drm_modeset_unlock_all(dev);
+	i915_gem_object_put(new_bo);
 
 	kfree(params);
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/11] drm/i915: Use pipe_src_w in overlay code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace the use of crtc->mode.h/vdisplay with the more appropriate
config->pipe_src_w/h.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 0c5e82beda4a..638933bfd8a0 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -937,12 +937,13 @@ static void update_pfit_vscale_ratio(struct intel_overlay *overlay)
 static int check_overlay_dst(struct intel_overlay *overlay,
 			     struct drm_intel_overlay_put_image *rec)
 {
-	struct drm_display_mode *mode = &overlay->crtc->base.mode;
+	const struct intel_crtc_state *pipe_config =
+		overlay->crtc->config;
 
-	if (rec->dst_x < mode->hdisplay &&
-	    rec->dst_x + rec->dst_width <= mode->hdisplay &&
-	    rec->dst_y < mode->vdisplay &&
-	    rec->dst_y + rec->dst_height <= mode->vdisplay)
+	if (rec->dst_x < pipe_config->pipe_src_w &&
+	    rec->dst_x + rec->dst_width <= pipe_config->pipe_src_w &&
+	    rec->dst_y < pipe_config->pipe_src_h &&
+	    rec->dst_y + rec->dst_height <= pipe_config->pipe_src_h)
 		return 0;
 	else
 		return -EINVAL;
@@ -1162,7 +1163,6 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 
 	if (overlay->crtc != crtc) {
-		struct drm_display_mode *mode = &crtc->base.mode;
 		ret = intel_overlay_switch_off(overlay);
 		if (ret != 0)
 			goto out_unlock;
@@ -1175,7 +1175,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 		crtc->overlay = overlay;
 
 		/* line too wide, i.e. one-line-mode */
-		if (mode->hdisplay > 1024 &&
+		if (crtc->config->pipe_src_w > 1024 &&
 		    intel_panel_fitter_pipe(dev_priv) == crtc->pipe) {
 			overlay->pfit_active = true;
 			update_pfit_vscale_ratio(overlay);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/11] drm/i915: Kill intel_panel_fitter_pipe()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Check pipe config gmch_pfit.control instead of using intel_panel_fitter_pipe()
to figure out if the pipe for the overlay is using the panel fitter.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 29 +----------------------------
 1 file changed, 1 insertion(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 638933bfd8a0..10e8d3b9a0c4 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1075,33 +1075,6 @@ static int check_overlay_src(struct drm_i915_private *dev_priv,
 	return 0;
 }
 
-/**
- * Return the pipe currently connected to the panel fitter,
- * or -1 if the panel fitter is not present or not in use
- */
-static int intel_panel_fitter_pipe(struct drm_i915_private *dev_priv)
-{
-	u32  pfit_control;
-
-	/* i830 doesn't have a panel fitter */
-	if (INTEL_GEN(dev_priv) <= 3 &&
-	    (IS_I830(dev_priv) || !IS_MOBILE(dev_priv)))
-		return -1;
-
-	pfit_control = I915_READ(PFIT_CONTROL);
-
-	/* See if the panel fitter is in use */
-	if ((pfit_control & PFIT_ENABLE) == 0)
-		return -1;
-
-	/* 965 can place panel fitter on either pipe */
-	if (IS_GEN4(dev_priv))
-		return (pfit_control >> 29) & 0x3;
-
-	/* older chips can only use pipe 1 */
-	return 1;
-}
-
 int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file_priv)
 {
@@ -1176,7 +1149,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 
 		/* line too wide, i.e. one-line-mode */
 		if (crtc->config->pipe_src_w > 1024 &&
-		    intel_panel_fitter_pipe(dev_priv) == crtc->pipe) {
+		    crtc->config->gmch_pfit.control & PFIT_ENABLE) {
 			overlay->pfit_active = true;
 			update_pfit_vscale_ratio(overlay);
 		} else
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/11] drm/i915: Simplify SWIDTHSW calculation

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The formula in Bspec for computing the overlay SWIDTHSW is overly
obfuscated. Simplify the formula to something that's easily parsed by
humans.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 10e8d3b9a0c4..19ff5d86f9f9 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -566,19 +566,17 @@ static int uv_vsubsampling(u32 format)
 
 static u32 calc_swidthsw(struct drm_i915_private *dev_priv, u32 offset, u32 width)
 {
-	u32 mask, shift, ret;
-	if (IS_GEN2(dev_priv)) {
-		mask = 0x1f;
-		shift = 5;
-	} else {
-		mask = 0x3f;
-		shift = 6;
-	}
-	ret = ((offset + width + mask) >> shift) - (offset >> shift);
-	if (!IS_GEN2(dev_priv))
-		ret <<= 1;
-	ret -= 1;
-	return ret << 2;
+	u32 sw;
+
+	if (IS_GEN2(dev_priv))
+		sw = ALIGN((offset & 31) + width, 32);
+	else
+		sw = ALIGN((offset & 63) + width, 64);
+
+	if (sw == 0)
+		return 0;
+
+	return (sw - 32) >> 3;
 }
 
 static const u16 y_static_hcoeffs[N_HORIZ_Y_TAPS * N_PHASES] = {
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/11] drm/i915: Reorganize overlay filter coeffs into a nicer form

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use two-dimensional arrays and named initializers to make the
overlay filter coefficient tables easier to parse for humans.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 64 ++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 19ff5d86f9f9..e4420b08f3ab 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -579,36 +579,44 @@ static u32 calc_swidthsw(struct drm_i915_private *dev_priv, u32 offset, u32 widt
 	return (sw - 32) >> 3;
 }
 
-static const u16 y_static_hcoeffs[N_HORIZ_Y_TAPS * N_PHASES] = {
-	0x3000, 0xb4a0, 0x1930, 0x1920, 0xb4a0,
-	0x3000, 0xb500, 0x19d0, 0x1880, 0xb440,
-	0x3000, 0xb540, 0x1a88, 0x2f80, 0xb3e0,
-	0x3000, 0xb580, 0x1b30, 0x2e20, 0xb380,
-	0x3000, 0xb5c0, 0x1bd8, 0x2cc0, 0xb320,
-	0x3020, 0xb5e0, 0x1c60, 0x2b80, 0xb2c0,
-	0x3020, 0xb5e0, 0x1cf8, 0x2a20, 0xb260,
-	0x3020, 0xb5e0, 0x1d80, 0x28e0, 0xb200,
-	0x3020, 0xb5c0, 0x1e08, 0x3f40, 0xb1c0,
-	0x3020, 0xb580, 0x1e78, 0x3ce0, 0xb160,
-	0x3040, 0xb520, 0x1ed8, 0x3aa0, 0xb120,
-	0x3040, 0xb4a0, 0x1f30, 0x3880, 0xb0e0,
-	0x3040, 0xb400, 0x1f78, 0x3680, 0xb0a0,
-	0x3020, 0xb340, 0x1fb8, 0x34a0, 0xb060,
-	0x3020, 0xb240, 0x1fe0, 0x32e0, 0xb040,
-	0x3020, 0xb140, 0x1ff8, 0x3160, 0xb020,
-	0xb000, 0x3000, 0x0800, 0x3000, 0xb000
+static const u16 y_static_hcoeffs[N_PHASES][N_HORIZ_Y_TAPS] = {
+	[ 0] = { 0x3000, 0xb4a0, 0x1930, 0x1920, 0xb4a0, },
+	[ 1] = { 0x3000, 0xb500, 0x19d0, 0x1880, 0xb440, },
+	[ 2] = { 0x3000, 0xb540, 0x1a88, 0x2f80, 0xb3e0, },
+	[ 3] = { 0x3000, 0xb580, 0x1b30, 0x2e20, 0xb380, },
+	[ 4] = { 0x3000, 0xb5c0, 0x1bd8, 0x2cc0, 0xb320, },
+	[ 5] = { 0x3020, 0xb5e0, 0x1c60, 0x2b80, 0xb2c0, },
+	[ 6] = { 0x3020, 0xb5e0, 0x1cf8, 0x2a20, 0xb260, },
+	[ 7] = { 0x3020, 0xb5e0, 0x1d80, 0x28e0, 0xb200, },
+	[ 8] = { 0x3020, 0xb5c0, 0x1e08, 0x3f40, 0xb1c0, },
+	[ 9] = { 0x3020, 0xb580, 0x1e78, 0x3ce0, 0xb160, },
+	[10] = { 0x3040, 0xb520, 0x1ed8, 0x3aa0, 0xb120, },
+	[11] = { 0x3040, 0xb4a0, 0x1f30, 0x3880, 0xb0e0, },
+	[12] = { 0x3040, 0xb400, 0x1f78, 0x3680, 0xb0a0, },
+	[13] = { 0x3020, 0xb340, 0x1fb8, 0x34a0, 0xb060, },
+	[14] = { 0x3020, 0xb240, 0x1fe0, 0x32e0, 0xb040, },
+	[15] = { 0x3020, 0xb140, 0x1ff8, 0x3160, 0xb020, },
+	[16] = { 0xb000, 0x3000, 0x0800, 0x3000, 0xb000, },
 };
 
-static const u16 uv_static_hcoeffs[N_HORIZ_UV_TAPS * N_PHASES] = {
-	0x3000, 0x1800, 0x1800, 0xb000, 0x18d0, 0x2e60,
-	0xb000, 0x1990, 0x2ce0, 0xb020, 0x1a68, 0x2b40,
-	0xb040, 0x1b20, 0x29e0, 0xb060, 0x1bd8, 0x2880,
-	0xb080, 0x1c88, 0x3e60, 0xb0a0, 0x1d28, 0x3c00,
-	0xb0c0, 0x1db8, 0x39e0, 0xb0e0, 0x1e40, 0x37e0,
-	0xb100, 0x1eb8, 0x3620, 0xb100, 0x1f18, 0x34a0,
-	0xb100, 0x1f68, 0x3360, 0xb0e0, 0x1fa8, 0x3240,
-	0xb0c0, 0x1fe0, 0x3140, 0xb060, 0x1ff0, 0x30a0,
-	0x3000, 0x0800, 0x3000
+static const u16 uv_static_hcoeffs[N_PHASES][N_HORIZ_UV_TAPS] = {
+	[ 0] = { 0x3000, 0x1800, 0x1800, },
+	[ 1] = { 0xb000, 0x18d0, 0x2e60, },
+	[ 2] = { 0xb000, 0x1990, 0x2ce0, },
+	[ 3] = { 0xb020, 0x1a68, 0x2b40, },
+	[ 4] = { 0xb040, 0x1b20, 0x29e0, },
+	[ 5] = { 0xb060, 0x1bd8, 0x2880, },
+	[ 6] = { 0xb080, 0x1c88, 0x3e60, },
+	[ 7] = { 0xb0a0, 0x1d28, 0x3c00, },
+	[ 8] = { 0xb0c0, 0x1db8, 0x39e0, },
+	[ 9] = { 0xb0e0, 0x1e40, 0x37e0, },
+	[10] = { 0xb100, 0x1eb8, 0x3620, },
+	[11] = { 0xb100, 0x1f18, 0x34a0, },
+	[12] = { 0xb100, 0x1f68, 0x3360, },
+	[13] = { 0xb0e0, 0x1fa8, 0x3240, },
+	[14] = { 0xb0c0, 0x1fe0, 0x3140, },
+	[15] = { 0xb060, 0x1ff0, 0x30a0, },
+	[16] = { 0x3000, 0x0800, 0x3000, },
 };
 
 static void update_polyphase_filter(struct overlay_registers __iomem *regs)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/11] drm/i915: Use primary plane->state for overlay ckey setup

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract the primary plane pixel format via plane state when setting up
the overlay colorkey.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index e4420b08f3ab..bc113ebc475f 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -689,31 +689,32 @@ static bool update_scaling_factors(struct intel_overlay *overlay,
 static void update_colorkey(struct intel_overlay *overlay,
 			    struct overlay_registers __iomem *regs)
 {
+	const struct intel_plane_state *state =
+		to_intel_plane_state(overlay->crtc->base.primary->state);
 	u32 key = overlay->color_key;
-	u32 flags;
+	u32 format = 0;
+	u32 flags = 0;
 
-	flags = 0;
 	if (overlay->color_key_enabled)
 		flags |= DST_KEY_ENABLE;
 
-	switch (overlay->crtc->base.primary->fb->bits_per_pixel) {
-	case 8:
+	if (state->base.visible)
+		format = state->base.fb->pixel_format;
+
+	switch (format) {
+	case DRM_FORMAT_C8:
 		key = 0;
 		flags |= CLK_RGB8I_MASK;
 		break;
-
-	case 16:
-		if (overlay->crtc->base.primary->fb->depth == 15) {
-			key = RGB15_TO_COLORKEY(key);
-			flags |= CLK_RGB15_MASK;
-		} else {
-			key = RGB16_TO_COLORKEY(key);
-			flags |= CLK_RGB16_MASK;
-		}
+	case DRM_FORMAT_XRGB1555:
+		key = RGB15_TO_COLORKEY(key);
+		flags |= CLK_RGB15_MASK;
 		break;
-
-	case 24:
-	case 32:
+	case DRM_FORMAT_RGB565:
+		key = RGB16_TO_COLORKEY(key);
+		flags |= CLK_RGB16_MASK;
+		break;
+	default:
 		flags |= CLK_RGB24_MASK;
 		break;
 	}
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/11] drm/i915: Disable L2 cache clock gating on 830 when using the overlay

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

BSpec says:
"Overlay Clock Gating Must be Disabled: Overlay & L2 Cache clock gating
must be disabled in order to prevent device hangs when turning off overlay.SW
must turn off Ovrunit clock gating (6200h) and L2 Cache clock gating (C8h)."

We only turned off the overlay clock gating (due to lack of docs I
presume). After a bit of experimentation it looks like the the magic
C8h register lives in the PCI config space of device 0, and the magic
bit appears to be bit 2. Or at the very least this eliminates the GPU
death after MI_OVERLAY_OFF.

L2 clock gating seems to save ~80mW, so let's keep it on unless we need
to actually use the overlay.

Also let's move the OVRUNIT clock gating to the same place since we can,
and 845 supposedly doesn't need it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
 drivers/gpu/drm/i915/intel_overlay.c | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c      |  2 --
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 90685d235410..e077e40f5005 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -110,6 +110,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   GRDOM_RESET_STATUS	(1 << 1)
 #define   GRDOM_RESET_ENABLE	(1 << 0)
 
+/* BSpec only has register offset, PCI device and bit found empirically */
+#define I830_CLOCK_GATE	0xc8 /* device 0 */
+#define   I830_L2_CACHE_CLOCK_GATE_DISABLE	(1 << 2)
+
 #define GCDGMBUS 0xcc
 
 #define GCFGC2	0xda
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index bc113ebc475f..9e8bbaa53a22 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -187,6 +187,29 @@ struct intel_overlay {
 	struct i915_gem_active last_flip;
 };
 
+static void i830_overlay_clock_gating(struct drm_i915_private *dev_priv,
+				      bool enable)
+{
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+	u8 val;
+
+	/* WA_OVERLAY_CLKGATE:alm */
+	if (enable)
+		I915_WRITE(DSPCLK_GATE_D, 0);
+	else
+		I915_WRITE(DSPCLK_GATE_D, OVRUNIT_CLOCK_GATE_DISABLE);
+
+	/* WA_DISABLE_L2CACHE_CLOCK_GATING:alm */
+	pci_bus_read_config_byte(pdev->bus,
+				 PCI_DEVFN(0, 0), I830_CLOCK_GATE, &val);
+	if (enable)
+		val &= ~I830_L2_CACHE_CLOCK_GATE_DISABLE;
+	else
+		val |= I830_L2_CACHE_CLOCK_GATE_DISABLE;
+	pci_bus_write_config_byte(pdev->bus,
+				  PCI_DEVFN(0, 0), I830_CLOCK_GATE, val);
+}
+
 static struct overlay_registers __iomem *
 intel_overlay_map_regs(struct intel_overlay *overlay)
 {
@@ -261,6 +284,9 @@ static int intel_overlay_on(struct intel_overlay *overlay)
 
 	overlay->active = true;
 
+	if (IS_I830(dev_priv))
+		i830_overlay_clock_gating(dev_priv, false);
+
 	ring = req->ring;
 	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_ON);
 	intel_ring_emit(ring, overlay->flip_addr | OFC_UPDATE);
@@ -365,12 +391,16 @@ static void intel_overlay_off_tail(struct i915_gem_active *active,
 {
 	struct intel_overlay *overlay =
 		container_of(active, typeof(*overlay), last_flip);
+	struct drm_i915_private *dev_priv = overlay->i915;
 
 	intel_overlay_release_old_vma(overlay);
 
 	overlay->crtc->overlay = NULL;
 	overlay->crtc = NULL;
 	overlay->active = false;
+
+	if (IS_I830(dev_priv))
+		i830_overlay_clock_gating(dev_priv, true);
 }
 
 /* overlay needs to be disabled in OCMD reg */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3ea7cf275be0..fd09ae08d86f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7593,8 +7593,6 @@ static void i85x_init_clock_gating(struct drm_i915_private *dev_priv)
 
 static void i830_init_clock_gating(struct drm_i915_private *dev_priv)
 {
-	I915_WRITE(DSPCLK_GATE_D, OVRUNIT_CLOCK_GATE_DISABLE);
-
 	I915_WRITE(MEM_MODE,
 		   _MASKED_BIT_ENABLE(MEM_DISPLAY_A_TRICKLE_FEED_DISABLE) |
 		   _MASKED_BIT_ENABLE(MEM_DISPLAY_B_TRICKLE_FEED_DISABLE));
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that we're disabling L2 clock gating MI_OVERLAY_OFF actually works
on 830, so let's use it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 9e8bbaa53a22..bfc9fa41c04e 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -431,23 +431,17 @@ static int intel_overlay_off(struct intel_overlay *overlay)
 	}
 
 	ring = req->ring;
+
 	/* wait for overlay to go idle */
 	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_CONTINUE);
 	intel_ring_emit(ring, flip_addr);
 	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+
 	/* turn overlay off */
-	if (IS_I830(dev_priv)) {
-		/* Workaround: Don't disable the overlay fully, since otherwise
-		 * it dies on the next OVERLAY_ON cmd. */
-		intel_ring_emit(ring, MI_NOOP);
-		intel_ring_emit(ring, MI_NOOP);
-		intel_ring_emit(ring, MI_NOOP);
-	} else {
-		intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
-		intel_ring_emit(ring, flip_addr);
-		intel_ring_emit(ring,
-				MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
-	}
+	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
+	intel_ring_emit(ring, flip_addr);
+	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+
 	intel_ring_advance(ring);
 
 	intel_overlay_flip_prepare(overlay, NULL);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:05PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We need an uninterruptible wait for the overlay off request during
> modeset. Restore the operation of dev_priv->mm.interruptible
> sufficiently for that.
> 
> Toss in a WARN_ON() to make sure the request succeeds.
> 
> Fixes: 7da844c5c6fc ("drm/i915: Move the special case wait-request handling to its one caller")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 36 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_request.h | 35 ++------------------------------
>  drivers/gpu/drm/i915/intel_display.c    |  2 +-
>  3 files changed, 39 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index fcf22b0e2967..1a7b88166c51 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1199,3 +1199,39 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
>  	for_each_engine(engine, dev_priv, id)
>  		engine_retire_requests(engine);
>  }
> +
> +/**
> + * i915_gem_active_retire - waits until the request is retired
> + * @active - the active request on which to wait
> + *
> + * i915_gem_active_retire() waits until the request is completed,
> + * and then ensures that at least the retirement handler for this
> + * @active tracker is called before returning. If the @active
> + * tracker is idle, the function returns immediately.
> + */
> +int __must_check
> +i915_gem_active_retire(struct i915_gem_active *active,
> +		       struct mutex *mutex)
> +{
> +	struct drm_i915_gem_request *request;
> +	long ret;
> +
> +	request = i915_gem_active_raw(active, mutex);
> +	if (!request)
> +		return 0;
> +
> +	ret = i915_wait_request(request,
> +				(request->i915->mm.interruptible ?
> +				 I915_WAIT_INTERRUPTIBLE : 0) |
> +				I915_WAIT_LOCKED,
> +				MAX_SCHEDULE_TIMEOUT);

At least pass in the flags.

> +	if (ret < 0)
> +		return ret;
> +
> +	list_del_init(&active->link);
> +	RCU_INIT_POINTER(active->request, NULL);
> +
> +	active->retire(active, request);
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> index e2b077df2da0..09add6b9cfc7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -657,39 +657,8 @@ i915_gem_active_wait(const struct i915_gem_active *active, unsigned int flags)
>  	return ret < 0 ? ret : 0;
>  }
>  
> -/**
> - * i915_gem_active_retire - waits until the request is retired
> - * @active - the active request on which to wait
> - *
> - * i915_gem_active_retire() waits until the request is completed,
> - * and then ensures that at least the retirement handler for this
> - * @active tracker is called before returning. If the @active
> - * tracker is idle, the function returns immediately.
> - */
> -static inline int __must_check
> -i915_gem_active_retire(struct i915_gem_active *active,
> -		       struct mutex *mutex)
> -{
> -	struct drm_i915_gem_request *request;
> -	long ret;
> -
> -	request = i915_gem_active_raw(active, mutex);
> -	if (!request)
> -		return 0;
> -
> -	ret = i915_wait_request(request,
> -				I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
> -				MAX_SCHEDULE_TIMEOUT);
> -	if (ret < 0)
> -		return ret;
> -
> -	list_del_init(&active->link);
> -	RCU_INIT_POINTER(active->request, NULL);
> -
> -	active->retire(active, request);
> -
> -	return 0;
> -}
> +int __must_check i915_gem_active_retire(struct i915_gem_active *active,
> +					struct mutex *mutex);
>  
>  #define for_each_active(mask, idx) \
>  	for (; mask ? idx = ffs(mask) - 1, 1 : 0; mask &= ~BIT(idx))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a41082e2750e..5a74991854e3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4923,7 +4923,7 @@ static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc)
>  
>  		mutex_lock(&dev->struct_mutex);
>  		dev_priv->mm.interruptible = false;
> -		(void) intel_overlay_switch_off(intel_crtc->overlay);
> +		WARN_ON(intel_overlay_switch_off(intel_crtc->overlay) != 0);
>  		dev_priv->mm.interruptible = true;
>  		mutex_unlock(&dev->struct_mutex);

Or we can push this wait to where it can interrupt, such as prepare_planes_fb.
(For intel_atomic_commit_tail, intel_crtc_disable_noatomic() should always
be a no-op.) And then we are down to just the shrinker running
uninterruptibly, just one last place to fix.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/11] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:04PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> the overlay code can set it to NULL. That obviously ends up oopsing.
> Fix it by setting the ->retire hook using init_request_active()
> so that it'll do the NULL->i915_gem_retire_noop conversion for us.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_overlay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 1cc963814224..786389dd5175 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -216,7 +216,7 @@ static void intel_overlay_submit_request(struct intel_overlay *overlay,
>  {
>  	GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
>  					&overlay->i915->drm.struct_mutex));
> -	overlay->last_flip.retire = retire;
> +	init_request_active(&overlay->last_flip, retire);

Hmm, init, not reinit. The alternative is to use i915_gem_retire_noop
instead of NULL. (And sorry, it did used to allow NULL.)

overlay->last_flip.retire = retire ?= i915_gem_retire_noop;

Or i915_gem_active_set_retire_fn(&overlay->last_flip, retire);
That's seems like it should make everyone a bit happier.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/11] drm/i915: Fix oops in overlay due to frontbuffer trakcing

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:03PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The vma will be NULL if the overlay was previously off, so
> dereferencing it will oops. Check for NULL before doing that.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Fixes: 9b3b7841b86d ("drm/i915/overlay: Use VMA as the primary tracker for images")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Oops,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/11] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff

[Ville Syrjälä]

On Wed, Dec 07, 2016 at 05:44:15PM +0000, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 07:28:04PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> > the overlay code can set it to NULL. That obviously ends up oopsing.
> > Fix it by setting the ->retire hook using init_request_active()
> > so that it'll do the NULL->i915_gem_retire_noop conversion for us.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_overlay.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> > index 1cc963814224..786389dd5175 100644
> > --- a/drivers/gpu/drm/i915/intel_overlay.c
> > +++ b/drivers/gpu/drm/i915/intel_overlay.c
> > @@ -216,7 +216,7 @@ static void intel_overlay_submit_request(struct intel_overlay *overlay,
> >  {
> >  	GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
> >  					&overlay->i915->drm.struct_mutex));
> > -	overlay->last_flip.retire = retire;
> > +	init_request_active(&overlay->last_flip, retire);
> 
> Hmm, init, not reinit.

Does it matter? The list head is the other thing in there I guess, but
at this point it shouldn't be on any list anymore AFAICS.

> The alternative is to use i915_gem_retire_noop
> instead of NULL. (And sorry, it did used to allow NULL.)
> 
> overlay->last_flip.retire = retire ?= i915_gem_retire_noop;
> 
> Or i915_gem_active_set_retire_fn(&overlay->last_flip, retire);
> That's seems like it should make everyone a bit happier.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay

[Ville Syrjälä]

On Wed, Dec 07, 2016 at 05:41:39PM +0000, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 07:28:05PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We need an uninterruptible wait for the overlay off request during
> > modeset. Restore the operation of dev_priv->mm.interruptible
> > sufficiently for that.
> > 
> > Toss in a WARN_ON() to make sure the request succeeds.
> > 
> > Fixes: 7da844c5c6fc ("drm/i915: Move the special case wait-request handling to its one caller")
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_request.c | 36 +++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_gem_request.h | 35 ++------------------------------
> >  drivers/gpu/drm/i915/intel_display.c    |  2 +-
> >  3 files changed, 39 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> > index fcf22b0e2967..1a7b88166c51 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_request.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> > @@ -1199,3 +1199,39 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
> >  	for_each_engine(engine, dev_priv, id)
> >  		engine_retire_requests(engine);
> >  }
> > +
> > +/**
> > + * i915_gem_active_retire - waits until the request is retired
> > + * @active - the active request on which to wait
> > + *
> > + * i915_gem_active_retire() waits until the request is completed,
> > + * and then ensures that at least the retirement handler for this
> > + * @active tracker is called before returning. If the @active
> > + * tracker is idle, the function returns immediately.
> > + */
> > +int __must_check
> > +i915_gem_active_retire(struct i915_gem_active *active,
> > +		       struct mutex *mutex)
> > +{
> > +	struct drm_i915_gem_request *request;
> > +	long ret;
> > +
> > +	request = i915_gem_active_raw(active, mutex);
> > +	if (!request)
> > +		return 0;
> > +
> > +	ret = i915_wait_request(request,
> > +				(request->i915->mm.interruptible ?
> > +				 I915_WAIT_INTERRUPTIBLE : 0) |
> > +				I915_WAIT_LOCKED,
> > +				MAX_SCHEDULE_TIMEOUT);
> 
> At least pass in the flags.
> 
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	list_del_init(&active->link);
> > +	RCU_INIT_POINTER(active->request, NULL);
> > +
> > +	active->retire(active, request);
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> > index e2b077df2da0..09add6b9cfc7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_request.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> > @@ -657,39 +657,8 @@ i915_gem_active_wait(const struct i915_gem_active *active, unsigned int flags)
> >  	return ret < 0 ? ret : 0;
> >  }
> >  
> > -/**
> > - * i915_gem_active_retire - waits until the request is retired
> > - * @active - the active request on which to wait
> > - *
> > - * i915_gem_active_retire() waits until the request is completed,
> > - * and then ensures that at least the retirement handler for this
> > - * @active tracker is called before returning. If the @active
> > - * tracker is idle, the function returns immediately.
> > - */
> > -static inline int __must_check
> > -i915_gem_active_retire(struct i915_gem_active *active,
> > -		       struct mutex *mutex)
> > -{
> > -	struct drm_i915_gem_request *request;
> > -	long ret;
> > -
> > -	request = i915_gem_active_raw(active, mutex);
> > -	if (!request)
> > -		return 0;
> > -
> > -	ret = i915_wait_request(request,
> > -				I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
> > -				MAX_SCHEDULE_TIMEOUT);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	list_del_init(&active->link);
> > -	RCU_INIT_POINTER(active->request, NULL);
> > -
> > -	active->retire(active, request);
> > -
> > -	return 0;
> > -}
> > +int __must_check i915_gem_active_retire(struct i915_gem_active *active,
> > +					struct mutex *mutex);
> >  
> >  #define for_each_active(mask, idx) \
> >  	for (; mask ? idx = ffs(mask) - 1, 1 : 0; mask &= ~BIT(idx))
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a41082e2750e..5a74991854e3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4923,7 +4923,7 @@ static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc)
> >  
> >  		mutex_lock(&dev->struct_mutex);
> >  		dev_priv->mm.interruptible = false;
> > -		(void) intel_overlay_switch_off(intel_crtc->overlay);
> > +		WARN_ON(intel_overlay_switch_off(intel_crtc->overlay) != 0);
> >  		dev_priv->mm.interruptible = true;
> >  		mutex_unlock(&dev->struct_mutex);
> 
> Or we can push this wait to where it can interrupt, such as prepare_planes_fb.
> (For intel_atomic_commit_tail, intel_crtc_disable_noatomic() should always
> be a no-op.) And then we are down to just the shrinker running
> uninterruptibly, just one last place to fix.

Hmm. Yeah I guess we could try to do this in a different place.

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff

[Chris Wilson]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The i915_gem_active stuff doesn't like a NULL ->retire hook, but
the overlay code can set it to NULL. That obviously ends up oopsing.
Fix it by introducing a new helper to assign the retirement callback
that will switch out the NULL function pointer with
i915_gem_retire_noop.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_request.h | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_overlay.c    |  3 ++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index 2fc6b8b3f580..b0d50aa81acb 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -440,6 +440,25 @@ i915_gem_active_set(struct i915_gem_active *active,
 	rcu_assign_pointer(active->request, request);
 }
 
+/**
+ * i915_gem_active_set_retire_fn - updates the retirement callback
+ * @active - the active tracker
+ * @fn - the routine called when the request is retired
+ * @mutex - struct_mutex used to guard retirements
+ *
+ * i915_gem_active_set_retire_fn() updates the function pointer that
+ * is called when the final request associated with the @active tracker
+ * is retired.
+ */
+static inline void
+i915_gem_active_set_retire_fn(struct i915_gem_active *active,
+			      i915_gem_retire_fn fn,
+			      struct mutex *mutex)
+{
+	lockdep_assert_held(mutex);
+	active->retire = fn ?: i915_gem_retire_noop;
+}
+
 static inline struct drm_i915_gem_request *
 __i915_gem_active_peek(const struct i915_gem_active *active)
 {
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 354f8cec96bb..29509f3055c8 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -216,7 +216,8 @@ static void intel_overlay_submit_request(struct intel_overlay *overlay,
 {
 	GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
 					&overlay->i915->drm.struct_mutex));
-	overlay->last_flip.retire = retire;
+	i915_gem_active_set_retire_fn(&overlay->last_flip, retire,
+				      &overlay->i915->drm.struct_mutex);
 	i915_gem_active_set(&overlay->last_flip, req);
 	i915_add_request(req);
 }
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff

[Ville Syrjälä]

On Wed, Dec 07, 2016 at 05:56:47PM +0000, Chris Wilson wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> the overlay code can set it to NULL. That obviously ends up oopsing.
> Fix it by introducing a new helper to assign the retirement callback
> that will switch out the NULL function pointer with
> i915_gem_retire_noop.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

lgtm, feel free to put yourself as the author is you want.

BTW I don't think we ever call the init_foo() thing on last_flip.
Should be do that in overlay_init() or some such place?

> ---
>  drivers/gpu/drm/i915/i915_gem_request.h | 19 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_overlay.c    |  3 ++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> index 2fc6b8b3f580..b0d50aa81acb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -440,6 +440,25 @@ i915_gem_active_set(struct i915_gem_active *active,
>  	rcu_assign_pointer(active->request, request);
>  }
>  
> +/**
> + * i915_gem_active_set_retire_fn - updates the retirement callback
> + * @active - the active tracker
> + * @fn - the routine called when the request is retired
> + * @mutex - struct_mutex used to guard retirements
> + *
> + * i915_gem_active_set_retire_fn() updates the function pointer that
> + * is called when the final request associated with the @active tracker
> + * is retired.
> + */
> +static inline void
> +i915_gem_active_set_retire_fn(struct i915_gem_active *active,
> +			      i915_gem_retire_fn fn,
> +			      struct mutex *mutex)
> +{
> +	lockdep_assert_held(mutex);
> +	active->retire = fn ?: i915_gem_retire_noop;
> +}
> +
>  static inline struct drm_i915_gem_request *
>  __i915_gem_active_peek(const struct i915_gem_active *active)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 354f8cec96bb..29509f3055c8 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -216,7 +216,8 @@ static void intel_overlay_submit_request(struct intel_overlay *overlay,
>  {
>  	GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
>  					&overlay->i915->drm.struct_mutex));
> -	overlay->last_flip.retire = retire;
> +	i915_gem_active_set_retire_fn(&overlay->last_flip, retire,
> +				      &overlay->i915->drm.struct_mutex);
>  	i915_gem_active_set(&overlay->last_flip, req);
>  	i915_add_request(req);
>  }
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff

[Chris Wilson]

On Wed, Dec 07, 2016 at 08:11:33PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 07, 2016 at 05:56:47PM +0000, Chris Wilson wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> > the overlay code can set it to NULL. That obviously ends up oopsing.
> > Fix it by introducing a new helper to assign the retirement callback
> > that will switch out the NULL function pointer with
> > i915_gem_retire_noop.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> lgtm, feel free to put yourself as the author is you want.
> 
> BTW I don't think we ever call the init_foo() thing on last_flip.
> Should be do that in overlay_init() or some such place?

Eeek. yes, we are missing the init_request_active() as well. A trivial
patch to add it to overlay_init, gratefully r-b'ed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:51:16PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 07, 2016 at 05:41:39PM +0000, Chris Wilson wrote:
> > Or we can push this wait to where it can interrupt, such as prepare_planes_fb.
> > (For intel_atomic_commit_tail, intel_crtc_disable_noatomic() should always
> > be a no-op.) And then we are down to just the shrinker running
> > uninterruptibly, just one last place to fix.
> 
> Hmm. Yeah I guess we could try to do this in a different place.

If we do the intel_overlay_off() via mmio (rather than CS) that makes it
simpler, as we just have to wait for the prior operation.

I'm not imagining that we can just use mmio, right?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Asynchronously wait for the overlay to finish

[Chris Wilson]

Treat the overlay as another wait-source, just like other rendering to
the fb, and asynchronously wait upon it before doing an atomic modeset.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
This is half the solution, it should prevent the wait before disabling
the overlay, but we still need to remove the wait from inside
intel_overlay_off() itself, preferably by switching to mmio instead of
CS here.
---
 drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_overlay.c | 19 +++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bfcd368ebf6f..6c51b0f2a96e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14183,9 +14183,18 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	int ret;
 
 	if (plane->state->fb && old_plane_needs_modeset(plane, new_state)) {
+		struct intel_crtc *crtc = to_intel_crtc(plane->state->crtc);
 		struct drm_i915_gem_object *old_obj =
 			intel_fb_obj(plane->state->fb);
 
+		/* Queue this update after a previous old-school overlay flip */
+		if (crtc->overlay) {
+			ret = intel_overlay_await(crtc->overlay,
+						  &intel_state->commit_ready);
+			if (ret)
+				return ret;
+		}
+
 		/* Big Hammer, we also need to ensure that any pending
 		 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
 		 * current scanout is retired before unpinning the old
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5c521065dc3b..87fd544339d9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1566,6 +1566,8 @@ void intel_attach_aspect_ratio_property(struct drm_connector *connector);
 /* intel_overlay.c */
 void intel_setup_overlay(struct drm_i915_private *dev_priv);
 void intel_cleanup_overlay(struct drm_i915_private *dev_priv);
+int intel_overlay_await(struct intel_overlay *overlay,
+			struct i915_sw_fence *fence);
 int intel_overlay_switch_off(struct intel_overlay *overlay);
 int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 79f93e7d699d..476a25961f34 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -855,6 +855,25 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 	return ret;
 }
 
+int intel_overlay_await(struct intel_overlay *overlay,
+			struct i915_sw_fence *fence)
+{
+	struct drm_i915_gem_request *request;
+	int err;
+
+	request = i915_gem_active_peek(&overlay->last_flip,
+				       &overlay->i915->drm.struct_mutex);
+	if (!request)
+		return 0;
+
+	err = i915_sw_fence_await_dma_fence(fence,
+					    &request->fence, 0, GFP_KERNEL);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 int intel_overlay_switch_off(struct intel_overlay *overlay)
 {
 	struct drm_i915_private *dev_priv = overlay->i915;
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay

[Ville Syrjälä]

On Wed, Dec 07, 2016 at 06:25:50PM +0000, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 07:51:16PM +0200, Ville Syrjälä wrote:
> > On Wed, Dec 07, 2016 at 05:41:39PM +0000, Chris Wilson wrote:
> > > Or we can push this wait to where it can interrupt, such as prepare_planes_fb.
> > > (For intel_atomic_commit_tail, intel_crtc_disable_noatomic() should always
> > > be a no-op.) And then we are down to just the shrinker running
> > > uninterruptibly, just one last place to fix.
> > 
> > Hmm. Yeah I guess we could try to do this in a different place.
> 
> If we do the intel_overlay_off() via mmio (rather than CS) that makes it
> simpler, as we just have to wait for the prior operation.
> 
> I'm not imagining that we can just use mmio, right?

Yeah, I have a branch that always uses mmio for the overlay
enable/disable/flips. I finished rebasing it the other night,
though it seems I have a bit of a buglet in there on 830 on
account of the DOVSTA "overlay update finished" bit not being 
truthful once the overlay has turned off. So still needs a bit
more work I think.

And we still need to use the ring for the texture cache vs.
overlay line buffers reconfiguration thing though. So far what
I have doesn't really split the steps apart that much. Ie. I
always emit the MI_OVERLAY_OFF as soon as the plane has turned
off. We could postpone it a bit just in case the plane is about
to be re-enabled almost immediately (eg. could be it's just
getting moved to the other pipe).

Though I'm not really sure if the cache reconfiguration alone
depends on the pipe somehow, or if we could just shut down the
pipe before the cache recofiguration has finished. I'd have to
play around with it a bit more to find out I guess.

So there are still a few unknowns with the mmio apporach.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Overlay fixes (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Overlay fixes (rev3)
URL   : https://patchwork.freedesktop.org/series/16495/
State : success

== Summary ==

Series 16495v3 drm/i915: Overlay fixes
https://patchwork.freedesktop.org/api/1.0/series/16495/revisions/3/mbox/


fi-bdw-5557u     total:247  pass:219  dwarn:0   dfail:0   fail:0   skip:28 
fi-bsw-n3050     total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-bxt-t5700     total:247  pass:206  dwarn:0   dfail:0   fail:0   skip:41 
fi-byt-j1900     total:247  pass:206  dwarn:0   dfail:0   fail:0   skip:41 
fi-byt-n2820     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45 
fi-hsw-4770      total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-hsw-4770r     total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-ilk-650       total:247  pass:181  dwarn:0   dfail:0   fail:0   skip:66 
fi-ivb-3520m     total:247  pass:213  dwarn:0   dfail:0   fail:0   skip:34 
fi-ivb-3770      total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35 
fi-kbl-7500u     total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35 
fi-skl-6260u     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-skl-6700hq    total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-skl-6700k     total:247  pass:210  dwarn:3   dfail:0   fail:0   skip:34 
fi-skl-6770hq    total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-snb-2520m     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45 
fi-snb-2600      total:247  pass:201  dwarn:0   dfail:0   fail:0   skip:46 

e46c97f4cfb63328f5bbbc0fb85c3f53703792f2 drm-tip: 2016y-12m-07d-17h-47m-18s UTC integration manifest
4166a37 drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
e2124bf drm/i915: Fix oops in overlay due to frontbuffer trakcing

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3224/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]

Hi Ville,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20161207]
[cannot apply to v4.9-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/ville-syrjala-linux-intel-com/drm-i915-Overlay-fixes/20161208-060045
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-s0-12080653 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   cc1: warnings being treated as errors
   drivers/gpu/drm/i915/intel_overlay.c: In function 'intel_overlay_off':
>> drivers/gpu/drm/i915/intel_overlay.c:409: error: unused variable 'dev_priv'

vim +/dev_priv +409 drivers/gpu/drm/i915/intel_overlay.c

18cb592b Ville Syrjälä 2016-12-07  403  		i830_overlay_clock_gating(dev_priv, true);
02e792fb Daniel Vetter 2009-09-15  404  }
02e792fb Daniel Vetter 2009-09-15  405  
02e792fb Daniel Vetter 2009-09-15  406  /* overlay needs to be disabled in OCMD reg */
ce453d81 Chris Wilson  2011-02-21  407  static int intel_overlay_off(struct intel_overlay *overlay)
02e792fb Daniel Vetter 2009-09-15  408  {
1ee8da6d Chris Wilson  2016-05-12 @409  	struct drm_i915_private *dev_priv = overlay->i915;
dad540ce John Harrison 2015-05-29  410  	struct drm_i915_gem_request *req;
7e37f889 Chris Wilson  2016-08-02  411  	struct intel_ring *ring;
8dc5d147 Chris Wilson  2010-08-12  412  	u32 flip_addr = overlay->flip_addr;

:::::: The code at line 409 was first introduced by commit
:::::: 1ee8da6df17dc481948aa2d04005e87bc7d387a4 drm/i915: Convert intel_overlay.c to use native drm_i915_private pointers

:::::: TO: Chris Wilson <chris@chris-wilson.co.uk>
:::::: CC: Chris Wilson <chris@chris-wilson.co.uk>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25659 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/11] drm/i915: Kill intel_panel_fitter_pipe()

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:08PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check pipe config gmch_pfit.control instead of using intel_panel_fitter_pipe()
> to figure out if the pipe for the overlay is using the panel fitter.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/11] drm/i915: Use pipe_src_w in overlay code

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the use of crtc->mode.h/vdisplay with the more appropriate
> config->pipe_src_w/h.

I'll ask the dumb question: wouldn't pipe_src be the fb dimensions? It
doesn't seem obvious to be they are the CRTC dimensions. Yes, they make
sense from the pov this is the source rectangle that the CRTC is pulling
pixels from. Hmm, hopefully I've answered my own question.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/11] drm/i915: Simplify SWIDTHSW calculation

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:09PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The formula in Bspec for computing the overlay SWIDTHSW is overly
> obfuscated. Simplify the formula to something that's easily parsed by
> humans.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, I've stared at it long enough to agree that they are indeed the
same.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/11] drm/i915: Reorganize overlay filter coeffs into a nicer form

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:10PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use two-dimensional arrays and named initializers to make the
> overlay filter coefficient tables easier to parse for humans.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Spot checking didn't reveal any typos. I presume that since they are u16
array of arrays, gcc is not adding any padding between rows?

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/11] drm/i915: Use primary plane->state for overlay ckey setup

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:11PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Extract the primary plane pixel format via plane state when setting up
> the overlay colorkey.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Minor stylistic differences aside (I would rearrange the code a bit more
to try and avoid having to initialise the locals, with the claim that it
keeps the code deciding upon the value together),
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/11] drm/i915: Disable L2 cache clock gating on 830 when using the overlay

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:12PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> BSpec says:
> "Overlay Clock Gating Must be Disabled: Overlay & L2 Cache clock gating
> must be disabled in order to prevent device hangs when turning off overlay.SW
> must turn off Ovrunit clock gating (6200h) and L2 Cache clock gating (C8h)."
> 
> We only turned off the overlay clock gating (due to lack of docs I
> presume). After a bit of experimentation it looks like the the magic
> C8h register lives in the PCI config space of device 0, and the magic
> bit appears to be bit 2. Or at the very least this eliminates the GPU
> death after MI_OVERLAY_OFF.
> 
> L2 clock gating seems to save ~80mW, so let's keep it on unless we need
> to actually use the overlay.
> 
> Also let's move the OVRUNIT clock gating to the same place since we can,
> and 845 supposedly doesn't need it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/11] drm/i915: Fix the overlay frontbuffer tracking

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:06PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Do the overlay frontbuffer tracking properly so that it matches
> the state of the overlay on/off/continue requests.
> 
> One slight problem is that intel_frontbuffer_flip_complete()
> may get delayed by an arbitrarily liong time due to the fact that
> the overlay code likes to bail out when a signal occurs. So the
> flip may not get completed until the ioctl is restarted. But fixing
> that would require bigger surgery, so I decided to ignore it for now.

Bigger surgery than conversion to spite planes? ;-)
 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Nothing to complain about, traced the new ref and found it freed.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2981 bytes --]

Hi Ville,

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20161208]
[cannot apply to v4.9-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/ville-syrjala-linux-intel-com/drm-i915-Overlay-fixes/20161208-060045
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a0-12081851 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_overlay.c: In function 'intel_overlay_off':
>> drivers/gpu/drm/i915/intel_overlay.c:409: warning: unused variable 'dev_priv'

vim +/dev_priv +409 drivers/gpu/drm/i915/intel_overlay.c

71f85de3 Ville Syrjälä 2016-12-07  393  		container_of(active, typeof(*overlay), last_flip);
18cb592b Ville Syrjälä 2016-12-07  394  	struct drm_i915_private *dev_priv = overlay->i915;
71f85de3 Ville Syrjälä 2016-12-07  395  
71f85de3 Ville Syrjälä 2016-12-07  396  	intel_overlay_release_old_vma(overlay);
03f77ea5 Daniel Vetter 2009-09-15  397  
b303cf95 Chris Wilson  2010-08-12  398  	overlay->crtc->overlay = NULL;
b303cf95 Chris Wilson  2010-08-12  399  	overlay->crtc = NULL;
209c2a5e Ville Syrjälä 2015-03-31  400  	overlay->active = false;
18cb592b Ville Syrjälä 2016-12-07  401  
18cb592b Ville Syrjälä 2016-12-07  402  	if (IS_I830(dev_priv))
18cb592b Ville Syrjälä 2016-12-07  403  		i830_overlay_clock_gating(dev_priv, true);
02e792fb Daniel Vetter 2009-09-15  404  }
02e792fb Daniel Vetter 2009-09-15  405  
02e792fb Daniel Vetter 2009-09-15  406  /* overlay needs to be disabled in OCMD reg */
ce453d81 Chris Wilson  2011-02-21  407  static int intel_overlay_off(struct intel_overlay *overlay)
02e792fb Daniel Vetter 2009-09-15  408  {
1ee8da6d Chris Wilson  2016-05-12 @409  	struct drm_i915_private *dev_priv = overlay->i915;
dad540ce John Harrison 2015-05-29  410  	struct drm_i915_gem_request *req;
7e37f889 Chris Wilson  2016-08-02  411  	struct intel_ring *ring;
8dc5d147 Chris Wilson  2010-08-12  412  	u32 flip_addr = overlay->flip_addr;
e1f99ce6 Chris Wilson  2010-10-27  413  	int ret;
02e792fb Daniel Vetter 2009-09-15  414  
77589f56 Ville Syrjälä 2015-03-31  415  	WARN_ON(!overlay->active);
02e792fb Daniel Vetter 2009-09-15  416  
02e792fb Daniel Vetter 2009-09-15  417  	/* According to intel docs the overlay hw may hang (when switching

:::::: The code at line 409 was first introduced by commit
:::::: 1ee8da6df17dc481948aa2d04005e87bc7d387a4 drm/i915: Convert intel_overlay.c to use native drm_i915_private pointers

:::::: TO: Chris Wilson <chris@chris-wilson.co.uk>
:::::: CC: Chris Wilson <chris@chris-wilson.co.uk>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30900 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/11] drm/i915: Use pipe_src_w in overlay code

[Ville Syrjälä]

On Thu, Dec 08, 2016 at 08:35:48AM +0000, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 07:28:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Replace the use of crtc->mode.h/vdisplay with the more appropriate
> > config->pipe_src_w/h.
> 
> I'll ask the dumb question: wouldn't pipe_src be the fb dimensions? It
> doesn't seem obvious to be they are the CRTC dimensions. Yes, they make
> sense from the pov this is the source rectangle that the CRTC is pulling
> pixels from. Hmm, hopefully I've answered my own question.

Indeed you have. The PIPESRC register name is a little confusing.
"Canvas size" or something along those lines might be less confusing.

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/11] drm/i915: Reorganize overlay filter coeffs into a nicer form

[Ville Syrjälä]

On Thu, Dec 08, 2016 at 08:45:31AM +0000, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 07:28:10PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use two-dimensional arrays and named initializers to make the
> > overlay filter coefficient tables easier to parse for humans.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Spot checking didn't reveal any typos. I presume that since they are u16
> array of arrays, gcc is not adding any padding between rows?

Didn't see any. I suppose we could

BUILD_BUG_ON(sizeof(y_static_hcoeffs) != N_PHASES * N_HORIZ_Y_TAPS * 2);
BUILD_BUG_ON(sizeof(uv_static_hcoeffs) != N_PHASES * N_HORIZ_UV_TAPS * 2);

for extra paranoia?

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/11] drm/i915: Reorganize overlay filter coeffs into a nicer form

[Chris Wilson]

On Thu, Dec 08, 2016 at 06:17:28PM +0200, Ville Syrjälä wrote:
> On Thu, Dec 08, 2016 at 08:45:31AM +0000, Chris Wilson wrote:
> > On Wed, Dec 07, 2016 at 07:28:10PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Use two-dimensional arrays and named initializers to make the
> > > overlay filter coefficient tables easier to parse for humans.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Spot checking didn't reveal any typos. I presume that since they are u16
> > array of arrays, gcc is not adding any padding between rows?
> 
> Didn't see any. I suppose we could
> 
> BUILD_BUG_ON(sizeof(y_static_hcoeffs) != N_PHASES * N_HORIZ_Y_TAPS * 2);
> BUILD_BUG_ON(sizeof(uv_static_hcoeffs) != N_PHASES * N_HORIZ_UV_TAPS * 2);
> 
> for extra paranoia?

I don't think it matters. Maybe a __packed and just leave it to gcc's
whims?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Initialize overlay->last_flip properly

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Initialize overlay->last_flip properly instead of leaving it zeroed.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 489c14ff2cb9..86e21d7468c4 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1427,6 +1427,8 @@ void intel_setup_overlay(struct drm_i915_private *dev_priv)
 	overlay->contrast = 75;
 	overlay->saturation = 146;
 
+	init_request_active(&overlay->last_flip, NULL);
+
 	regs = intel_overlay_map_regs(overlay);
 	if (!regs)
 		goto out_unpin_bo;
-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Overlay fixes (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Overlay fixes (rev4)
URL   : https://patchwork.freedesktop.org/series/16495/
State : success

== Summary ==

Series 16495v4 drm/i915: Overlay fixes
https://patchwork.freedesktop.org/api/1.0/series/16495/revisions/4/mbox/

Test kms_force_connector_basic:
        Subgroup force-connector-state:
                dmesg-warn -> PASS       (fi-ivb-3770)

fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:225  dwarn:1   dfail:0   fail:0   skip:21 
fi-bxt-t5700     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

f45701ec502e5ee9682561be7578bd39741ad4bb drm-tip: 2016y-12m-21d-11h-52m-22s UTC integration manifest
394316a drm/i915: Initialize overlay->last_flip properly
7835c04 drm/i915: Fix oops in overlay due to frontbuffer trakcing

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3356/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that we're disabling L2 clock gating MI_OVERLAY_OFF actually works
on 830, so let's use it.

v2: Nuke the unused dev_priv variable

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 70d2ef7ecc29..4473a611c664 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -407,7 +407,6 @@ static void intel_overlay_off_tail(struct i915_gem_active *active,
 /* overlay needs to be disabled in OCMD reg */
 static int intel_overlay_off(struct intel_overlay *overlay)
 {
-	struct drm_i915_private *dev_priv = overlay->i915;
 	struct drm_i915_gem_request *req;
 	struct intel_ring *ring;
 	u32 flip_addr = overlay->flip_addr;
@@ -432,23 +431,17 @@ static int intel_overlay_off(struct intel_overlay *overlay)
 	}
 
 	ring = req->ring;
+
 	/* wait for overlay to go idle */
 	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_CONTINUE);
 	intel_ring_emit(ring, flip_addr);
 	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+
 	/* turn overlay off */
-	if (IS_I830(dev_priv)) {
-		/* Workaround: Don't disable the overlay fully, since otherwise
-		 * it dies on the next OVERLAY_ON cmd. */
-		intel_ring_emit(ring, MI_NOOP);
-		intel_ring_emit(ring, MI_NOOP);
-		intel_ring_emit(ring, MI_NOOP);
-	} else {
-		intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
-		intel_ring_emit(ring, flip_addr);
-		intel_ring_emit(ring,
-				MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
-	}
+	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
+	intel_ring_emit(ring, flip_addr);
+	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+
 	intel_ring_advance(ring);
 
 	intel_overlay_flip_prepare(overlay, NULL);
-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/11] drm/i915: Reorganize overlay filter coeffs into a nicer form

[Ville Syrjälä]

On Thu, Dec 08, 2016 at 04:26:41PM +0000, Chris Wilson wrote:
> On Thu, Dec 08, 2016 at 06:17:28PM +0200, Ville Syrjälä wrote:
> > On Thu, Dec 08, 2016 at 08:45:31AM +0000, Chris Wilson wrote:
> > > On Wed, Dec 07, 2016 at 07:28:10PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Use two-dimensional arrays and named initializers to make the
> > > > overlay filter coefficient tables easier to parse for humans.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Spot checking didn't reveal any typos. I presume that since they are u16
> > > array of arrays, gcc is not adding any padding between rows?
> > 
> > Didn't see any. I suppose we could
> > 
> > BUILD_BUG_ON(sizeof(y_static_hcoeffs) != N_PHASES * N_HORIZ_Y_TAPS * 2);
> > BUILD_BUG_ON(sizeof(uv_static_hcoeffs) != N_PHASES * N_HORIZ_UV_TAPS * 2);
> > 
> > for extra paranoia?
> 
> I don't think it matters. Maybe a __packed and just leave it to gcc's
> whims?

Apparently __packed isn't a thing for arrays. So I went ahead and pushed
the patch as is.

I pushed all the other ones that had been reviewed as well. That leaves
us with the mm.interruptible thing which probably needs actual thought,
and the "nuke 830 MI_OVERLAY_OFF w/a" patch.

I tries to slap on some cc:stables to the oops fixes since the offenders
have by now reached v4.9.

Thanks for the reviews.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround

[Chris Wilson]

On Thu, Dec 22, 2016 at 09:52:22PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that we're disabling L2 clock gating MI_OVERLAY_OFF actually works
> on 830, so let's use it.
> 
> v2: Nuke the unused dev_priv variable
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Does what it says on the tin, and it does look consistent with the
earlier patch for L2 clock gating, so

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround

[Ville Syrjälä]

On Thu, Dec 22, 2016 at 09:40:41PM +0000, Chris Wilson wrote:
> On Thu, Dec 22, 2016 at 09:52:22PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Now that we're disabling L2 clock gating MI_OVERLAY_OFF actually works
> > on 830, so let's use it.
> > 
> > v2: Nuke the unused dev_priv variable
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Does what it says on the tin, and it does look consistent with the
> earlier patch for L2 clock gating, so
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks. Pushed as well.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx