Re: [PATCH v2 2/6] drm/i915: Only pin the fence for primary planes (and gen2/3)

Re: [PATCH v2 2/6] drm/i915: Only pin the fence for primary planes (and gen2/3)

[Guang (George) Bai]

On Wed, 21 Feb 2018 20:48:07 +0200
Ville Syrjala <ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we pin a fence on every plane doing tiled scanout. The
> number of planes we have available is fast apporaching the number
> of fences so we really should stop wasting them. Only FBC needs
> the fence on gen4+, so let's use fences only for the primary planes
> on those platforms.
> 
> v2: drop the tiling check from plane_uses_fence() as the obj is
>     NULL during initial_plane_config() and we don't rally need the
>     check since i915_vma_pin_fence() does the check anyway
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fbdev.c   |  2 +-
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index
> 66b269bc24b9..f2c1bb715e7b 100644 ---
> a/drivers/gpu/drm/i915/intel_display.c +++
> b/drivers/gpu/drm/i915/intel_display.c @@ -2067,9 +2067,18 @@ static
> unsigned int intel_surf_alignment(const struct drm_framebuffer *fb, }
>  }
>  
> +static bool intel_plane_uses_fence(const struct intel_plane_state
> *plane_state) +{
> +	struct intel_plane *plane =
> to_intel_plane(plane_state->base.plane);
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +
> +	return INTEL_GEN(dev_priv) < 4 || plane->id == PLANE_PRIMARY;
> +}
> +
>  struct i915_vma *
>  intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>  			   unsigned int rotation,
> +			   bool uses_fence,
>  			   unsigned long *out_flags)
>  {
>  	struct drm_device *dev = fb->dev;
> @@ -2122,7 +2131,7 @@ intel_pin_and_fence_fb_obj(struct
> drm_framebuffer *fb, if (IS_ERR(vma))
>  		goto err;
>  
> -	if (i915_vma_is_map_and_fenceable(vma)) {
> +	if (uses_fence && i915_vma_is_map_and_fenceable(vma)) {
>  		int ret;
>  
>  		/* Install a fence for tiled scan-out. Pre-i965
> always needs a @@ -2836,6 +2845,7 @@
> intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> intel_state->vma = intel_pin_and_fence_fb_obj(fb,
>  					   primary->state->rotation,
> +
> intel_plane_uses_fence(intel_state), &intel_state->flags);
>  	mutex_unlock(&dev->struct_mutex);
>  	if (IS_ERR(intel_state->vma)) {
> @@ -12744,6 +12754,7 @@ intel_prepare_plane_fb(struct drm_plane
> *plane, 
>  		vma = intel_pin_and_fence_fb_obj(fb,
>  						 new_state->rotation,
> +
> intel_plane_uses_fence(to_intel_plane_state(new_state)),
> &to_intel_plane_state(new_state)->flags); if (!IS_ERR(vma))
>  			to_intel_plane_state(new_state)->vma = vma;
> @@ -13162,6 +13173,7 @@ intel_legacy_cursor_update(struct drm_plane
> *plane, } else {
>  		vma = intel_pin_and_fence_fb_obj(fb,
>  						 new_plane_state->rotation,
> +						 false,
>  						 &to_intel_plane_state(new_plane_state)->flags);
>  		if (IS_ERR(vma)) {
>  			DRM_DEBUG_KMS("failed to pin object\n");
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h index 50874f4035cf..e3f78fdae859
> 100644 --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1508,6 +1508,7 @@ void intel_release_load_detect_pipe(struct
> drm_connector *connector, struct i915_vma *
>  intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>  			   unsigned int rotation,
> +			   bool uses_fence,
>  			   unsigned long *out_flags);
>  void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags);
>  struct drm_framebuffer *
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c index 055f409f8b75..6f12adc06365
> 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -215,7 +215,7 @@ static int intelfb_create(struct drm_fb_helper
> *helper, */
>  	vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base,
>  					 DRM_MODE_ROTATE_0,
> -					 &flags);
> +					 false, &flags);
>  	if (IS_ERR(vma)) {
>  		ret = PTR_ERR(vma);
>  		goto out_unlock;

All these fence and fbc related changes will fix the gen9lp fence
starvation problems from virtualization use cases.
Thanks,
Guang
 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 0/6] drm/i915: Scanout fence fixes/cleanups

[Ville Syrjala]

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

Rebase of my earlier attempts at fixing the scanout fence mess on top
of Chris's PLANE_HAS_FENCE stuff.

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

Ville Syrjälä (6):
  drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout
  drm/i915: Only pin the fence for primary planes (and gen2/3)
  drm/i915: Clean up fbc vs. plane checks
  drm/i915: Require fence only for FBC capable planes
  drm/i915: Extract intel_plane_{pin,unpin}_fb()
  drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation

 drivers/gpu/drm/i915/intel_display.c | 157 +++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_fbc.c     |  34 +++-----
 drivers/gpu/drm/i915/intel_fbdev.c   |   2 +-
 drivers/gpu/drm/i915/intel_pm.c      |   2 -
 5 files changed, 122 insertions(+), 75 deletions(-)

-- 
2.13.6

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

[PATCH 1/6] drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

[Ville Syrjala]

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

Gen2/3 display engine depends on the fence for tiled scanout. So if we
fail to get a fence fail the entire operation.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5d46771d58f6..66b269bc24b9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2123,6 +2123,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 		goto err;
 
 	if (i915_vma_is_map_and_fenceable(vma)) {
+		int ret;
+
 		/* Install a fence for tiled scan-out. Pre-i965 always needs a
 		 * fence, whereas 965+ only requires a fence if using
 		 * framebuffer compression.  For simplicity, we always, when
@@ -2139,7 +2141,13 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 		 * something and try to run the system in a "less than optimal"
 		 * mode that matches the user configuration.
 		 */
-		if (i915_vma_pin_fence(vma) == 0 && vma->fence)
+		ret = i915_vma_pin_fence(vma);
+		if (ret != 0 && INTEL_GEN(dev_priv) < 4) {
+			vma = ERR_PTR(ret);
+			goto err;
+		}
+
+		if (ret == 0 && vma->fence)
 			*out_flags |= PLANE_HAS_FENCE;
 	}
 
-- 
2.13.6

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

[PATCH 2/6] drm/i915: Only pin the fence for primary planes (and gen2/3)

[Ville Syrjala]

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

Currently we pin a fence on every plane doing tiled scanout. The
number of planes we have available is fast apporaching the number
of fences so we really should stop wasting them. Only FBC needs
the fence on gen4+, so let's use fences only for the primary planes
on those platforms.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 18 +++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_fbdev.c   |  2 +-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 66b269bc24b9..d2a66704e6f5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2067,9 +2067,22 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
 	}
 }
 
+static bool intel_plane_uses_fence(const struct intel_plane_state *plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	struct drm_i915_gem_object *obj = intel_fb_obj(plane_state->base.fb);
+
+	if (i915_gem_object_get_tiling(obj) == I915_TILING_NONE)
+		return false;
+
+	return INTEL_GEN(dev_priv) < 4 || plane->id == PLANE_PRIMARY;
+}
+
 struct i915_vma *
 intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 			   unsigned int rotation,
+			   bool uses_fence,
 			   unsigned long *out_flags)
 {
 	struct drm_device *dev = fb->dev;
@@ -2122,7 +2135,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 	if (IS_ERR(vma))
 		goto err;
 
-	if (i915_vma_is_map_and_fenceable(vma)) {
+	if (uses_fence && i915_vma_is_map_and_fenceable(vma)) {
 		int ret;
 
 		/* Install a fence for tiled scan-out. Pre-i965 always needs a
@@ -2836,6 +2849,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	intel_state->vma =
 		intel_pin_and_fence_fb_obj(fb,
 					   primary->state->rotation,
+					   intel_plane_uses_fence(intel_state),
 					   &intel_state->flags);
 	mutex_unlock(&dev->struct_mutex);
 	if (IS_ERR(intel_state->vma)) {
@@ -12744,6 +12758,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 
 		vma = intel_pin_and_fence_fb_obj(fb,
 						 new_state->rotation,
+						 intel_plane_uses_fence(to_intel_plane_state(new_state)),
 						 &to_intel_plane_state(new_state)->flags);
 		if (!IS_ERR(vma))
 			to_intel_plane_state(new_state)->vma = vma;
@@ -13162,6 +13177,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	} else {
 		vma = intel_pin_and_fence_fb_obj(fb,
 						 new_plane_state->rotation,
+						 false,
 						 &to_intel_plane_state(new_plane_state)->flags);
 		if (IS_ERR(vma)) {
 			DRM_DEBUG_KMS("failed to pin object\n");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 50874f4035cf..e3f78fdae859 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1508,6 +1508,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 struct i915_vma *
 intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 			   unsigned int rotation,
+			   bool uses_fence,
 			   unsigned long *out_flags);
 void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags);
 struct drm_framebuffer *
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 055f409f8b75..6f12adc06365 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -215,7 +215,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	 */
 	vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base,
 					 DRM_MODE_ROTATE_0,
-					 &flags);
+					 false, &flags);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		goto out_unlock;
-- 
2.13.6

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

[PATCH 3/6] drm/i915: Clean up fbc vs. plane checks

[Ville Syrjala]

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

Let's record the information whether a plane can do fbc or not under
struct inte_plane.

v2: Rebase due to i9xx_plane_id
    Handle BDW/HSW correctly

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_fbc.c     | 28 +++++++--------------------
 drivers/gpu/drm/i915/intel_pm.c      |  2 --
 4 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d2a66704e6f5..8aeb6b686bac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13238,6 +13238,32 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
 	.format_mod_supported = intel_cursor_plane_format_mod_supported,
 };
 
+static bool i9xx_plane_has_fbc(struct drm_i915_private *dev_priv,
+			       enum i9xx_plane_id i9xx_plane)
+{
+	if (!HAS_FBC(dev_priv))
+		return false;
+
+	if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+		return i9xx_plane == PLANE_A;
+	else if (IS_IVYBRIDGE(dev_priv))
+		return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B ||
+			i9xx_plane == PLANE_C;
+	else if (INTEL_GEN(dev_priv) >= 4)
+		return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B;
+	else
+		return i9xx_plane == PLANE_A;
+}
+
+static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
+			      enum pipe pipe, enum plane_id plane_id)
+{
+	if (!HAS_FBC(dev_priv))
+		return false;
+
+	return pipe == PIPE_A && plane_id == PLANE_PRIMARY;
+}
+
 static struct intel_plane *
 intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
@@ -13280,6 +13306,15 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
 	primary->id = PLANE_PRIMARY;
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, primary->id);
+
+	if (INTEL_GEN(dev_priv) >= 9)
+		primary->has_fbc = skl_plane_has_fbc(dev_priv,
+						     primary->pipe,
+						     primary->id);
+	else
+		primary->has_fbc = i9xx_plane_has_fbc(dev_priv,
+						      primary->i9xx_plane);
+
 	primary->check_plane = intel_check_primary_plane;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
@@ -14683,6 +14718,8 @@ int intel_modeset_init(struct drm_device *dev)
 		}
 	}
 
+	intel_fbc_init(dev_priv);
+
 	intel_shared_dpll_init(dev);
 	intel_update_fdi_pll_freq(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e3f78fdae859..af82feb4a3cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -935,6 +935,7 @@ struct intel_plane {
 	enum plane_id id;
 	enum pipe pipe;
 	bool can_scale;
+	bool has_fbc;
 	int max_downscale;
 	uint32_t frontbuffer_bit;
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f66f6fb5743d..97d088058d1d 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -46,16 +46,6 @@ static inline bool fbc_supported(struct drm_i915_private *dev_priv)
 	return HAS_FBC(dev_priv);
 }
 
-static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
-{
-	return IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8;
-}
-
-static inline bool fbc_on_plane_a_only(struct drm_i915_private *dev_priv)
-{
-	return INTEL_GEN(dev_priv) < 4;
-}
-
 static inline bool no_fbc_on_multiple_pipes(struct drm_i915_private *dev_priv)
 {
 	return INTEL_GEN(dev_priv) <= 3;
@@ -1094,13 +1084,10 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 		struct intel_crtc_state *crtc_state;
 		struct intel_crtc *crtc = to_intel_crtc(plane_state->base.crtc);
 
-		if (!plane_state->base.visible)
-			continue;
-
-		if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A)
+		if (!plane->has_fbc)
 			continue;
 
-		if (fbc_on_plane_a_only(dev_priv) && plane->i9xx_plane != PLANE_A)
+		if (!plane_state->base.visible)
 			continue;
 
 		crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
@@ -1357,7 +1344,7 @@ static bool need_fbc_vtd_wa(struct drm_i915_private *dev_priv)
 void intel_fbc_init(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
-	enum pipe pipe;
+	struct intel_plane *plane;
 
 	INIT_WORK(&fbc->work.work, intel_fbc_work_fn);
 	INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn);
@@ -1378,12 +1365,11 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
 		return;
 	}
 
-	for_each_pipe(dev_priv, pipe) {
-		fbc->possible_framebuffer_bits |=
-			INTEL_FRONTBUFFER(pipe, PLANE_PRIMARY);
 
-		if (fbc_on_pipe_a_only(dev_priv))
-			break;
+	for_each_intel_plane(&dev_priv->drm, plane) {
+		if (plane->has_fbc)
+			fbc->possible_framebuffer_bits |=
+				INTEL_FRONTBUFFER(plane->pipe, plane->id);
 	}
 
 	/* This value was pulled out of someone's hat */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index abf80e462833..e8180ff66f43 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -9049,8 +9049,6 @@ void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
 /* Set up chip specific power management-related functions */
 void intel_init_pm(struct drm_i915_private *dev_priv)
 {
-	intel_fbc_init(dev_priv);
-
 	/* For cxsr */
 	if (IS_PINEVIEW(dev_priv))
 		i915_pineview_get_mem_freq(dev_priv);
-- 
2.13.6

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

[PATCH 4/6] drm/i915: Require fence only for FBC capable planes

[Ville Syrjala]

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

As only a subset of primary planes are FBC capable there's no need
to waste fences on all of them. So let's skip the fence if the plane
isn't even fbc capable.

In the future we might extend this to skip the fence even for FBC
capable planes if the crtc and/or plane state isn't suitable
for FBC.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8aeb6b686bac..b4048b425ffd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2076,7 +2076,7 @@ static bool intel_plane_uses_fence(const struct intel_plane_state *plane_state)
 	if (i915_gem_object_get_tiling(obj) == I915_TILING_NONE)
 		return false;
 
-	return INTEL_GEN(dev_priv) < 4 || plane->id == PLANE_PRIMARY;
+	return INTEL_GEN(dev_priv) < 4 || plane->has_fbc;
 }
 
 struct i915_vma *
-- 
2.13.6

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

[PATCH 5/6] drm/i915: Extract intel_plane_{pin, unpin}_fb()

[Ville Syrjala]

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

We've replicated the fb pin/unpin code in a few places. Pull it into
convenint helpers.

Slight change in locking behaviour as intel_cleanup_plane_fb() now
grab struct_mutex unconditionally.

v2: Change the locking to be symmetric between pin and unpin

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b4048b425ffd..c79441756225 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12674,6 +12674,42 @@ static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
 	add_wait_queue(drm_crtc_vblank_waitqueue(crtc), &wait->wait);
 }
 
+static int intel_plane_pin_fb(struct intel_plane_state *plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	struct i915_vma *vma;
+
+	if (plane->id == PLANE_CURSOR &&
+	    INTEL_INFO(dev_priv)->cursor_needs_physical) {
+		struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+		const int align = intel_cursor_alignment(dev_priv);
+
+		return i915_gem_object_attach_phys(obj, align);
+	}
+
+	vma = intel_pin_and_fence_fb_obj(fb,
+					 plane_state->base.rotation,
+					 intel_plane_uses_fence(plane_state),
+					 &plane_state->flags);
+	if (IS_ERR(vma))
+		return PTR_ERR(vma);
+
+	plane_state->vma = vma;
+
+	return 0;
+}
+
+static void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
+{
+	struct i915_vma *vma;
+
+	vma = fetch_and_zero(&old_plane_state->vma);
+	if (vma)
+		intel_unpin_fb_vma(vma, old_plane_state->flags);
+}
+
 /**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
@@ -12748,23 +12784,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 		return ret;
 	}
 
-	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
-	    INTEL_INFO(dev_priv)->cursor_needs_physical) {
-		const int align = intel_cursor_alignment(dev_priv);
-
-		ret = i915_gem_object_attach_phys(obj, align);
-	} else {
-		struct i915_vma *vma;
-
-		vma = intel_pin_and_fence_fb_obj(fb,
-						 new_state->rotation,
-						 intel_plane_uses_fence(to_intel_plane_state(new_state)),
-						 &to_intel_plane_state(new_state)->flags);
-		if (!IS_ERR(vma))
-			to_intel_plane_state(new_state)->vma = vma;
-		else
-			ret =  PTR_ERR(vma);
-	}
+	ret = intel_plane_pin_fb(to_intel_plane_state(new_state));
 
 	i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
 
@@ -12808,15 +12828,12 @@ void
 intel_cleanup_plane_fb(struct drm_plane *plane,
 		       struct drm_plane_state *old_state)
 {
-	struct i915_vma *vma;
+	struct drm_i915_private *dev_priv = to_i915(plane->dev);
 
 	/* Should only be called after a successful intel_prepare_plane_fb()! */
-	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
-	if (vma) {
-		mutex_lock(&plane->dev->struct_mutex);
-		intel_unpin_fb_vma(vma, to_intel_plane_state(old_state)->flags);
-		mutex_unlock(&plane->dev->struct_mutex);
-	}
+	mutex_lock(&dev_priv->drm.struct_mutex);
+	intel_plane_unpin_fb(to_intel_plane_state(old_state));
+	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
 
 int
@@ -13107,7 +13124,6 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *old_fb;
 	struct drm_crtc_state *crtc_state = crtc->state;
-	struct i915_vma *old_vma, *vma;
 
 	/*
 	 * When crtc is inactive or there is a modeset pending,
@@ -13166,28 +13182,9 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	if (ret)
 		goto out_free;
 
-	if (INTEL_INFO(dev_priv)->cursor_needs_physical) {
-		int align = intel_cursor_alignment(dev_priv);
-
-		ret = i915_gem_object_attach_phys(intel_fb_obj(fb), align);
-		if (ret) {
-			DRM_DEBUG_KMS("failed to attach phys object\n");
-			goto out_unlock;
-		}
-	} else {
-		vma = intel_pin_and_fence_fb_obj(fb,
-						 new_plane_state->rotation,
-						 false,
-						 &to_intel_plane_state(new_plane_state)->flags);
-		if (IS_ERR(vma)) {
-			DRM_DEBUG_KMS("failed to pin object\n");
-
-			ret = PTR_ERR(vma);
-			goto out_unlock;
-		}
-
-		to_intel_plane_state(new_plane_state)->vma = vma;
-	}
+	ret = intel_plane_pin_fb(to_intel_plane_state(new_plane_state));
+	if (ret)
+		goto out_unlock;
 
 	old_fb = old_plane_state->fb;
 
@@ -13207,10 +13204,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 		intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
 	}
 
-	old_vma = fetch_and_zero(&to_intel_plane_state(old_plane_state)->vma);
-	if (old_vma)
-		intel_unpin_fb_vma(old_vma,
-				   to_intel_plane_state(old_plane_state)->flags);
+	intel_plane_unpin_fb(to_intel_plane_state(old_plane_state));
 
 out_unlock:
 	mutex_unlock(&dev_priv->drm.struct_mutex);
-- 
2.13.6

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

[PATCH 6/6] drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation

[Ville Syrjala]

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

Currently the FBC code doesn't handle the 90/270 degree rotated case
correctly. We would need the GTT tracking to monitor the fence on the
normal GTT view (the rotated view doesn't even have a fence). Not quite
sure how we should program the fence Y offset etc. in that case. For now
we'll end up disabling FBC with 90/270 degree rotation. Add a FIXME
to remind people about this fact.

v2: Reword the text (Chris)
    Move the FIXME to the fbc code

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

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 97d088058d1d..ee0ca429a681 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -809,6 +809,12 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 	 * Note that is possible for a tiled surface to be unmappable (and
 	 * so have no fence associated with it) due to aperture constaints
 	 * at the time of pinning.
+	 *
+	 * FIXME with 90/270 degree rotation we should use the fence on
+	 * the normal GTT view (the rotated view doesn't even have a
+	 * fence). Would need changes to the FBC fence Y offset as well.
+	 * For now this will effecively disable FBC with 90/270 degree
+	 * rotation.
 	 */
 	if (!(cache->flags & PLANE_HAS_FENCE)) {
 		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
-- 
2.13.6

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Scanout fence fixes/cleanups

[Patchwork]

== Series Details ==

Series: drm/i915: Scanout fence fixes/cleanups
URL   : https://patchwork.freedesktop.org/series/38714/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ebcdf69836d6 drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout
d18a59c871b1 drm/i915: Only pin the fence for primary planes (and gen2/3)
-:66: WARNING: line over 80 characters
#66: FILE: drivers/gpu/drm/i915/intel_display.c:12761:
+						 intel_plane_uses_fence(to_intel_plane_state(new_state)),

total: 0 errors, 1 warnings, 0 checks, 66 lines checked
45fefc858fba drm/i915: Clean up fbc vs. plane checks
7ff5c8f22136 drm/i915: Require fence only for FBC capable planes
7ca32351fe8b drm/i915: Extract intel_plane_{pin, unpin}_fb()
94778d0f601b drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation

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

✗ Fi.CI.BAT: failure for drm/i915: Scanout fence fixes/cleanups

[Patchwork]

== Series Details ==

Series: drm/i915: Scanout fence fixes/cleanups
URL   : https://patchwork.freedesktop.org/series/38714/
State : failure

== Summary ==

Series 38714v1 drm/i915: Scanout fence fixes/cleanups
https://patchwork.freedesktop.org/api/1.0/series/38714/revisions/1/mbox/

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-ivb-3770)
                pass       -> INCOMPLETE (fi-byt-j1900)
                pass       -> INCOMPLETE (fi-byt-n2820)
                pass       -> INCOMPLETE (fi-hsw-4770)
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> INCOMPLETE (fi-elk-e7500) fdo#103989
Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-gdg-551)
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-bwr-2160)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-bdw-gvtdvm)
                pass       -> DMESG-WARN (fi-bsw-n3050)
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6600u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-skl-6700k2)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-guc)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
                pass       -> DMESG-WARN (fi-bxt-dsi)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-kbl-7560u)
                pass       -> DMESG-WARN (fi-kbl-7567u) fdo#105078
                pass       -> DMESG-WARN (fi-kbl-r)
                pass       -> DMESG-WARN (fi-glk-1)
                pass       -> DMESG-WARN (fi-cfl-s2)

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#105078 https://bugs.freedesktop.org/show_bug.cgi?id=105078

fi-bdw-5557u     total:288  pass:266  dwarn:1   dfail:0   fail:0   skip:21  time:415s
fi-bdw-gvtdvm    total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:421s
fi-blb-e6850     total:288  pass:222  dwarn:2   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:241  dwarn:1   dfail:0   fail:0   skip:46  time:478s
fi-bwr-2160      total:288  pass:182  dwarn:1   dfail:0   fail:0   skip:105 time:285s
fi-bxt-dsi       total:288  pass:257  dwarn:1   dfail:0   fail:0   skip:30  time:480s
fi-bxt-j4205     total:288  pass:258  dwarn:1   dfail:0   fail:0   skip:29  time:488s
fi-byt-j1900     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-byt-n2820     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-cfl-s2        total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:562s
fi-elk-e7500     total:218  pass:172  dwarn:0   dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:283s
fi-glk-1         total:288  pass:259  dwarn:1   dfail:0   fail:0   skip:28  time:504s
fi-hsw-4770      total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-ilk-650       total:288  pass:227  dwarn:1   dfail:0   fail:0   skip:60  time:408s
fi-ivb-3520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-ivb-3770      total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-7500u     total:288  pass:262  dwarn:2   dfail:0   fail:0   skip:24  time:448s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:492s
fi-kbl-7567u     total:288  pass:267  dwarn:1   dfail:0   fail:0   skip:20  time:448s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:492s
fi-pnv-d510      total:288  pass:221  dwarn:2   dfail:0   fail:0   skip:65  time:582s
fi-skl-6260u     total:288  pass:267  dwarn:1   dfail:0   fail:0   skip:20  time:425s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:502s
fi-skl-6700hq    total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:516s
fi-skl-6700k2    total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:482s
fi-skl-6770hq    total:288  pass:267  dwarn:1   dfail:0   fail:0   skip:20  time:467s
fi-skl-guc       total:288  pass:259  dwarn:1   dfail:0   fail:0   skip:28  time:409s
fi-skl-gvtdvm    total:288  pass:264  dwarn:1   dfail:0   fail:0   skip:23  time:426s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:247  dwarn:1   dfail:0   fail:0   skip:40  time:392s

42d073db2a8593a8d1832037ffe0988403031568 drm-tip: 2018y-02m-21d-14h-03m-58s UTC integration manifest
94778d0f601b drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation
7ca32351fe8b drm/i915: Extract intel_plane_{pin, unpin}_fb()
7ff5c8f22136 drm/i915: Require fence only for FBC capable planes
45fefc858fba drm/i915: Clean up fbc vs. plane checks
d18a59c871b1 drm/i915: Only pin the fence for primary planes (and gen2/3)
ebcdf69836d6 drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8109/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 3/6] drm/i915: Clean up fbc vs. plane checks

[Ville Syrjala]

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

Let's record the information whether a plane can do fbc or not under
struct inte_plane.

v2: Rebase due to i9xx_plane_id
    Handle BDW/HSW correctly
v3: Move inte_fbc_init() back since we depend on it happening
    even with i915.disable_display, and populate
    fbc->possible_framebuffer_bits directly from the
    plane init code instead

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_fbc.c     | 26 ++---------------------
 3 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d2a66704e6f5..c60d2215b377 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13238,6 +13238,32 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
 	.format_mod_supported = intel_cursor_plane_format_mod_supported,
 };
 
+static bool i9xx_plane_has_fbc(struct drm_i915_private *dev_priv,
+			       enum i9xx_plane_id i9xx_plane)
+{
+	if (!HAS_FBC(dev_priv))
+		return false;
+
+	if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+		return i9xx_plane == PLANE_A;
+	else if (IS_IVYBRIDGE(dev_priv))
+		return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B ||
+			i9xx_plane == PLANE_C;
+	else if (INTEL_GEN(dev_priv) >= 4)
+		return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B;
+	else
+		return i9xx_plane == PLANE_A;
+}
+
+static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
+			      enum pipe pipe, enum plane_id plane_id)
+{
+	if (!HAS_FBC(dev_priv))
+		return false;
+
+	return pipe == PIPE_A && plane_id == PLANE_PRIMARY;
+}
+
 static struct intel_plane *
 intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
@@ -13280,6 +13306,21 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
 	primary->id = PLANE_PRIMARY;
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, primary->id);
+
+	if (INTEL_GEN(dev_priv) >= 9)
+		primary->has_fbc = skl_plane_has_fbc(dev_priv,
+						     primary->pipe,
+						     primary->id);
+	else
+		primary->has_fbc = i9xx_plane_has_fbc(dev_priv,
+						      primary->i9xx_plane);
+
+	if (primary->has_fbc) {
+		struct intel_fbc *fbc = &dev_priv->fbc;
+
+		fbc->possible_framebuffer_bits |= primary->frontbuffer_bit;
+	}
+
 	primary->check_plane = intel_check_primary_plane;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e3f78fdae859..af82feb4a3cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -935,6 +935,7 @@ struct intel_plane {
 	enum plane_id id;
 	enum pipe pipe;
 	bool can_scale;
+	bool has_fbc;
 	int max_downscale;
 	uint32_t frontbuffer_bit;
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f66f6fb5743d..7fcb22a12e39 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -46,16 +46,6 @@ static inline bool fbc_supported(struct drm_i915_private *dev_priv)
 	return HAS_FBC(dev_priv);
 }
 
-static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
-{
-	return IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8;
-}
-
-static inline bool fbc_on_plane_a_only(struct drm_i915_private *dev_priv)
-{
-	return INTEL_GEN(dev_priv) < 4;
-}
-
 static inline bool no_fbc_on_multiple_pipes(struct drm_i915_private *dev_priv)
 {
 	return INTEL_GEN(dev_priv) <= 3;
@@ -1094,13 +1084,10 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 		struct intel_crtc_state *crtc_state;
 		struct intel_crtc *crtc = to_intel_crtc(plane_state->base.crtc);
 
-		if (!plane_state->base.visible)
+		if (!plane->has_fbc)
 			continue;
 
-		if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A)
-			continue;
-
-		if (fbc_on_plane_a_only(dev_priv) && plane->i9xx_plane != PLANE_A)
+		if (!plane_state->base.visible)
 			continue;
 
 		crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
@@ -1357,7 +1344,6 @@ static bool need_fbc_vtd_wa(struct drm_i915_private *dev_priv)
 void intel_fbc_init(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
-	enum pipe pipe;
 
 	INIT_WORK(&fbc->work.work, intel_fbc_work_fn);
 	INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn);
@@ -1378,14 +1364,6 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
 		return;
 	}
 
-	for_each_pipe(dev_priv, pipe) {
-		fbc->possible_framebuffer_bits |=
-			INTEL_FRONTBUFFER(pipe, PLANE_PRIMARY);
-
-		if (fbc_on_pipe_a_only(dev_priv))
-			break;
-	}
-
 	/* This value was pulled out of someone's hat */
 	if (INTEL_GEN(dev_priv) <= 4 && !IS_GM45(dev_priv))
 		I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
-- 
2.13.6

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Scanout fence fixes/cleanups (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Scanout fence fixes/cleanups (rev2)
URL   : https://patchwork.freedesktop.org/series/38714/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c4c108df218b drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout
b5042b8f97b3 drm/i915: Only pin the fence for primary planes (and gen2/3)
-:66: WARNING: line over 80 characters
#66: FILE: drivers/gpu/drm/i915/intel_display.c:12761:
+						 intel_plane_uses_fence(to_intel_plane_state(new_state)),

total: 0 errors, 1 warnings, 0 checks, 66 lines checked
8219c0edc31e drm/i915: Clean up fbc vs. plane checks
097680894b64 drm/i915: Require fence only for FBC capable planes
39cccc994518 drm/i915: Extract intel_plane_{pin, unpin}_fb()
0225359291c9 drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation

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

✗ Fi.CI.BAT: failure for drm/i915: Scanout fence fixes/cleanups (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Scanout fence fixes/cleanups (rev2)
URL   : https://patchwork.freedesktop.org/series/38714/
State : failure

== Summary ==

Series 38714v2 drm/i915: Scanout fence fixes/cleanups
https://patchwork.freedesktop.org/api/1.0/series/38714/revisions/2/mbox/

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-ivb-3770)
                pass       -> INCOMPLETE (fi-byt-j1900)
                pass       -> INCOMPLETE (fi-byt-n2820)
                pass       -> INCOMPLETE (fi-hsw-4770)
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> FAIL       (fi-ilk-650) fdo#104008

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:416s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:421s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:481s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:286s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:475s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:480s
fi-byt-j1900     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-byt-n2820     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:414s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:281s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:508s
fi-hsw-4770      total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-ilk-650       total:288  pass:227  dwarn:0   dfail:0   fail:1   skip:60  time:407s
fi-ivb-3520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-ivb-3770      total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:448s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:492s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:490s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:583s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:424s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:501s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:519s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:485s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:465s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:407s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:435s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:516s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:396s

42d073db2a8593a8d1832037ffe0988403031568 drm-tip: 2018y-02m-21d-14h-03m-58s UTC integration manifest
0225359291c9 drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation
39cccc994518 drm/i915: Extract intel_plane_{pin, unpin}_fb()
097680894b64 drm/i915: Require fence only for FBC capable planes
8219c0edc31e drm/i915: Clean up fbc vs. plane checks
b5042b8f97b3 drm/i915: Only pin the fence for primary planes (and gen2/3)
c4c108df218b drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8111/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/6] drm/i915: Only pin the fence for primary planes (and gen2/3)

[Ville Syrjala]

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

Currently we pin a fence on every plane doing tiled scanout. The
number of planes we have available is fast apporaching the number
of fences so we really should stop wasting them. Only FBC needs
the fence on gen4+, so let's use fences only for the primary planes
on those platforms.

v2: drop the tiling check from plane_uses_fence() as the obj is
    NULL during initial_plane_config() and we don't rally need the
    check since i915_vma_pin_fence() does the check anyway

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_fbdev.c   |  2 +-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 66b269bc24b9..f2c1bb715e7b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2067,9 +2067,18 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
 	}
 }
 
+static bool intel_plane_uses_fence(const struct intel_plane_state *plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+
+	return INTEL_GEN(dev_priv) < 4 || plane->id == PLANE_PRIMARY;
+}
+
 struct i915_vma *
 intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 			   unsigned int rotation,
+			   bool uses_fence,
 			   unsigned long *out_flags)
 {
 	struct drm_device *dev = fb->dev;
@@ -2122,7 +2131,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 	if (IS_ERR(vma))
 		goto err;
 
-	if (i915_vma_is_map_and_fenceable(vma)) {
+	if (uses_fence && i915_vma_is_map_and_fenceable(vma)) {
 		int ret;
 
 		/* Install a fence for tiled scan-out. Pre-i965 always needs a
@@ -2836,6 +2845,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	intel_state->vma =
 		intel_pin_and_fence_fb_obj(fb,
 					   primary->state->rotation,
+					   intel_plane_uses_fence(intel_state),
 					   &intel_state->flags);
 	mutex_unlock(&dev->struct_mutex);
 	if (IS_ERR(intel_state->vma)) {
@@ -12744,6 +12754,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 
 		vma = intel_pin_and_fence_fb_obj(fb,
 						 new_state->rotation,
+						 intel_plane_uses_fence(to_intel_plane_state(new_state)),
 						 &to_intel_plane_state(new_state)->flags);
 		if (!IS_ERR(vma))
 			to_intel_plane_state(new_state)->vma = vma;
@@ -13162,6 +13173,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	} else {
 		vma = intel_pin_and_fence_fb_obj(fb,
 						 new_plane_state->rotation,
+						 false,
 						 &to_intel_plane_state(new_plane_state)->flags);
 		if (IS_ERR(vma)) {
 			DRM_DEBUG_KMS("failed to pin object\n");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 50874f4035cf..e3f78fdae859 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1508,6 +1508,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 struct i915_vma *
 intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 			   unsigned int rotation,
+			   bool uses_fence,
 			   unsigned long *out_flags);
 void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags);
 struct drm_framebuffer *
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 055f409f8b75..6f12adc06365 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -215,7 +215,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	 */
 	vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base,
 					 DRM_MODE_ROTATE_0,
-					 &flags);
+					 false, &flags);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		goto out_unlock;
-- 
2.13.6

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Scanout fence fixes/cleanups (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Scanout fence fixes/cleanups (rev3)
URL   : https://patchwork.freedesktop.org/series/38714/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ec43511e598e drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout
a686f373992f drm/i915: Only pin the fence for primary planes (and gen2/3)
-:66: WARNING: line over 80 characters
#66: FILE: drivers/gpu/drm/i915/intel_display.c:12757:
+						 intel_plane_uses_fence(to_intel_plane_state(new_state)),

total: 0 errors, 1 warnings, 0 checks, 62 lines checked
5be9e603df0c drm/i915: Clean up fbc vs. plane checks
1b9e5a1fcd03 drm/i915: Require fence only for FBC capable planes
1a5caafb2917 drm/i915: Extract intel_plane_{pin, unpin}_fb()
a88746960907 drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation

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

✓ Fi.CI.BAT: success for drm/i915: Scanout fence fixes/cleanups (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Scanout fence fixes/cleanups (rev3)
URL   : https://patchwork.freedesktop.org/series/38714/
State : success

== Summary ==

Series 38714v3 drm/i915: Scanout fence fixes/cleanups
https://patchwork.freedesktop.org/api/1.0/series/38714/revisions/3/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:416s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:423s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:485s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:479s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:481s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:464s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:451s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:558s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:412s
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:284s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:506s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:386s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:406s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:446s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:417s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:449s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:487s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:492s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:585s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:436s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:499s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:517s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:478s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:405s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:426s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:400s

562a3886d1cd6e2763a022037e6090d7ceb00ee3 drm-tip: 2018y-02m-21d-18h-35m-44s UTC integration manifest
a88746960907 drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation
1a5caafb2917 drm/i915: Extract intel_plane_{pin, unpin}_fb()
1b9e5a1fcd03 drm/i915: Require fence only for FBC capable planes
5be9e603df0c drm/i915: Clean up fbc vs. plane checks
a686f373992f drm/i915: Only pin the fence for primary planes (and gen2/3)
ec43511e598e drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8113/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

[Chris Wilson]

Quoting Ville Syrjala (2018-02-21 16:02:30)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Gen2/3 display engine depends on the fence for tiled scanout. So if we
> fail to get a fence fail the entire operation.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5d46771d58f6..66b269bc24b9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2123,6 +2123,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>                 goto err;
>  
>         if (i915_vma_is_map_and_fenceable(vma)) {
> +               int ret;
> +
>                 /* Install a fence for tiled scan-out. Pre-i965 always needs a
>                  * fence, whereas 965+ only requires a fence if using
>                  * framebuffer compression.  For simplicity, we always, when
> @@ -2139,7 +2141,13 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>                  * something and try to run the system in a "less than optimal"
>                  * mode that matches the user configuration.
>                  */
> -               if (i915_vma_pin_fence(vma) == 0 && vma->fence)
> +               ret = i915_vma_pin_fence(vma);
> +               if (ret != 0 && INTEL_GEN(dev_priv) < 4) {
> +                       vma = ERR_PTR(ret);
> +                       goto err;
> +               }
> +
> +               if (ret == 0 && vma->fence)
>                         *out_flags |= PLANE_HAS_FENCE;
>         }

Ok, I'd like to see INTEL_GEN(dev_priv) < 4 be replaced with say
needs_fence (and may be passed in from the caller like wants_fence?).
Then I'm wondering if a 
	if (WARN_ON(needs_fence && !(*flags & PLANE_HAS_FENCE))
makes sense.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/6] drm/i915: Only pin the fence for primary planes (and gen2/3)

[Chris Wilson]

Quoting Ville Syrjala (2018-02-21 18:48:07)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we pin a fence on every plane doing tiled scanout. The
> number of planes we have available is fast apporaching the number
> of fences so we really should stop wasting them. Only FBC needs
> the fence on gen4+, so let's use fences only for the primary planes
> on those platforms.
> 
> v2: drop the tiling check from plane_uses_fence() as the obj is
>     NULL during initial_plane_config() and we don't rally need the
>     check since i915_vma_pin_fence() does the check anyway
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fbdev.c   |  2 +-
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 66b269bc24b9..f2c1bb715e7b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2067,9 +2067,18 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
>         }
>  }
>  
> +static bool intel_plane_uses_fence(const struct intel_plane_state *plane_state)
> +{
> +       struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> +       struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +
> +       return INTEL_GEN(dev_priv) < 4 || plane->id == PLANE_PRIMARY;
> +}
> +
>  struct i915_vma *
>  intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>                            unsigned int rotation,
> +                          bool uses_fence,

(Before long you'll have more than one bool :)

>                            unsigned long *out_flags)
>  {
>         struct drm_device *dev = fb->dev;
> @@ -2122,7 +2131,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>         if (IS_ERR(vma))
>                 goto err;
>  
> -       if (i915_vma_is_map_and_fenceable(vma)) {
> +       if (uses_fence && i915_vma_is_map_and_fenceable(vma)) {

Ok.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 3/6] drm/i915: Clean up fbc vs. plane checks

[Chris Wilson]

Quoting Ville Syrjala (2018-02-21 17:31:01)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's record the information whether a plane can do fbc or not under
> struct inte_plane.
> 
> v2: Rebase due to i9xx_plane_id
>     Handle BDW/HSW correctly
> v3: Move inte_fbc_init() back since we depend on it happening
>     even with i915.disable_display, and populate
>     fbc->possible_framebuffer_bits directly from the
>     plane init code instead
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c     | 26 ++---------------------
>  3 files changed, 44 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d2a66704e6f5..c60d2215b377 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13238,6 +13238,32 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
>         .format_mod_supported = intel_cursor_plane_format_mod_supported,
>  };
>  
> +static bool i9xx_plane_has_fbc(struct drm_i915_private *dev_priv,
> +                              enum i9xx_plane_id i9xx_plane)
> +{
> +       if (!HAS_FBC(dev_priv))
> +               return false;

> -static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
> -{
> -       return IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8;
> -}
> -
> -static inline bool fbc_on_plane_a_only(struct drm_i915_private *dev_priv)
> -{
> -       return INTEL_GEN(dev_priv) < 4;
> -}

> +
> +       if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> +               return i9xx_plane == PLANE_A;

Where PLANE_A is tied to PIPE_A by construction. Is that worth a quick
comment?

> +       else if (IS_IVYBRIDGE(dev_priv))
> +               return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B ||
> +                       i9xx_plane == PLANE_C;
> +       else if (INTEL_GEN(dev_priv) >= 4)
> +               return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B;
> +       else
> +               return i9xx_plane == PLANE_A;

Ok, looks a bit more complete than before.

> +}
> +
> +static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
> +                             enum pipe pipe, enum plane_id plane_id)
> +{
> +       if (!HAS_FBC(dev_priv))
> +               return false;
> +
> +       return pipe == PIPE_A && plane_id == PLANE_PRIMARY;
> +}
> +
>  static struct intel_plane *
>  intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> @@ -13280,6 +13306,21 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>                 primary->i9xx_plane = (enum i9xx_plane_id) pipe;
>         primary->id = PLANE_PRIMARY;
>         primary->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, primary->id);
> +
> +       if (INTEL_GEN(dev_priv) >= 9)
> +               primary->has_fbc = skl_plane_has_fbc(dev_priv,
> +                                                    primary->pipe,
> +                                                    primary->id);
> +       else
> +               primary->has_fbc = i9xx_plane_has_fbc(dev_priv,
> +                                                     primary->i9xx_plane);
> +
> +       if (primary->has_fbc) {
> +               struct intel_fbc *fbc = &dev_priv->fbc;
> +
> +               fbc->possible_framebuffer_bits |= primary->frontbuffer_bit;
> +       }

So an equivalent init loop to

> -       for_each_pipe(dev_priv, pipe) {
> -               fbc->possible_framebuffer_bits |=
> -                       INTEL_FRONTBUFFER(pipe, PLANE_PRIMARY);
> -
> -               if (fbc_on_pipe_a_only(dev_priv))
> -                       break;
> -       }

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Require fence only for FBC capable planes

[Chris Wilson]

Quoting Ville Syrjala (2018-02-21 16:02:33)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> As only a subset of primary planes are FBC capable there's no need
> to waste fences on all of them. So let's skip the fence if the plane
> isn't even fbc capable.
> 
> In the future we might extend this to skip the fence even for FBC
> capable planes if the crtc and/or plane state isn't suitable
> for FBC.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8aeb6b686bac..b4048b425ffd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2076,7 +2076,7 @@ static bool intel_plane_uses_fence(const struct intel_plane_state *plane_state)
>         if (i915_gem_object_get_tiling(obj) == I915_TILING_NONE)
>                 return false;
>  
> -       return INTEL_GEN(dev_priv) < 4 || plane->id == PLANE_PRIMARY;
> +       return INTEL_GEN(dev_priv) < 4 || plane->has_fbc;

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Extract intel_plane_{pin, unpin}_fb()

[Chris Wilson]

Quoting Ville Syrjala (2018-02-21 16:02:34)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We've replicated the fb pin/unpin code in a few places. Pull it into
> convenint helpers.
> 
> Slight change in locking behaviour as intel_cleanup_plane_fb() now
> grab struct_mutex unconditionally.
> 
> v2: Change the locking to be symmetric between pin and unpin
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Add a FIXME about FBC vs. fence. 90/270 degree rotation

[Chris Wilson]

Quoting Ville Syrjala (2018-02-21 16:02:35)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently the FBC code doesn't handle the 90/270 degree rotated case
> correctly. We would need the GTT tracking to monitor the fence on the
> normal GTT view (the rotated view doesn't even have a fence). Not quite
> sure how we should program the fence Y offset etc. in that case. For now
> we'll end up disabling FBC with 90/270 degree rotation. Add a FIXME
> to remind people about this fact.

Oh my. plane_state would have to track both normal_vma and rotated_vma,
at least that is less complicated than having rotated_vma automagically
pin the normal_vma.

> v2: Reword the text (Chris)
>     Move the FIXME to the fbc code
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Scanout fence fixes/cleanups (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Scanout fence fixes/cleanups (rev3)
URL   : https://patchwork.freedesktop.org/series/38714/
State : success

== Summary ==

Test drv_suspend:
        Subgroup fence-restore-untiled:
                skip       -> PASS       (shard-snb)
Test kms_flip:
        Subgroup plain-flip-fb-recreate:
                fail       -> PASS       (shard-hsw) fdo#100368 +1
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (shard-hsw) fdo#103928
        Subgroup modeset-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-apl) fdo#103060
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> FAIL       (shard-hsw) fdo#104152
Test perf:
        Subgroup oa-exponents:
                incomplete -> FAIL       (shard-apl) fdo#102254
Test kms_cursor_crc:
        Subgroup cursor-128x128-suspend:
                pass       -> INCOMPLETE (shard-hsw) fdo#103540

fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#104152 https://bugs.freedesktop.org/show_bug.cgi?id=104152
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540

shard-apl        total:3369 pass:1764 dwarn:1   dfail:0   fail:14  skip:1589 time:12050s
shard-hsw        total:3455 pass:1758 dwarn:1   dfail:0   fail:4   skip:1690 time:11313s
shard-snb        total:3464 pass:1356 dwarn:1   dfail:0   fail:3   skip:2104 time:6647s
Blacklisted hosts:
shard-kbl        total:3464 pass:1959 dwarn:1   dfail:0   fail:16  skip:1488 time:9690s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8113/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 3/6] drm/i915: Clean up fbc vs. plane checks

[Ville Syrjälä]

On Wed, Feb 21, 2018 at 09:59:27PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-02-21 17:31:01)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Let's record the information whether a plane can do fbc or not under
> > struct inte_plane.
> > 
> > v2: Rebase due to i9xx_plane_id
> >     Handle BDW/HSW correctly
> > v3: Move inte_fbc_init() back since we depend on it happening
> >     even with i915.disable_display, and populate
> >     fbc->possible_framebuffer_bits directly from the
> >     plane init code instead
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_fbc.c     | 26 ++---------------------
> >  3 files changed, 44 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d2a66704e6f5..c60d2215b377 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13238,6 +13238,32 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> >         .format_mod_supported = intel_cursor_plane_format_mod_supported,
> >  };
> >  
> > +static bool i9xx_plane_has_fbc(struct drm_i915_private *dev_priv,
> > +                              enum i9xx_plane_id i9xx_plane)
> > +{
> > +       if (!HAS_FBC(dev_priv))
> > +               return false;
> 
> > -static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
> > -{
> > -       return IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8;
> > -}
> > -
> > -static inline bool fbc_on_plane_a_only(struct drm_i915_private *dev_priv)
> > -{
> > -       return INTEL_GEN(dev_priv) < 4;
> > -}
> 
> > +
> > +       if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> > +               return i9xx_plane == PLANE_A;
> 
> Where PLANE_A is tied to PIPE_A by construction. Is that worth a quick
> comment?

Wouldn't hurt to spell it out I suppose.

> 
> > +       else if (IS_IVYBRIDGE(dev_priv))
> > +               return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B ||
> > +                       i9xx_plane == PLANE_C;
> > +       else if (INTEL_GEN(dev_priv) >= 4)
> > +               return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B;
> > +       else
> > +               return i9xx_plane == PLANE_A;
> 
> Ok, looks a bit more complete than before.
> 
> > +}
> > +
> > +static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
> > +                             enum pipe pipe, enum plane_id plane_id)
> > +{
> > +       if (!HAS_FBC(dev_priv))
> > +               return false;
> > +
> > +       return pipe == PIPE_A && plane_id == PLANE_PRIMARY;
> > +}
> > +
> >  static struct intel_plane *
> >  intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  {
> > @@ -13280,6 +13306,21 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> >                 primary->i9xx_plane = (enum i9xx_plane_id) pipe;
> >         primary->id = PLANE_PRIMARY;
> >         primary->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, primary->id);
> > +
> > +       if (INTEL_GEN(dev_priv) >= 9)
> > +               primary->has_fbc = skl_plane_has_fbc(dev_priv,
> > +                                                    primary->pipe,
> > +                                                    primary->id);
> > +       else
> > +               primary->has_fbc = i9xx_plane_has_fbc(dev_priv,
> > +                                                     primary->i9xx_plane);
> > +
> > +       if (primary->has_fbc) {
> > +               struct intel_fbc *fbc = &dev_priv->fbc;
> > +
> > +               fbc->possible_framebuffer_bits |= primary->frontbuffer_bit;
> > +       }
> 
> So an equivalent init loop to
> 
> > -       for_each_pipe(dev_priv, pipe) {
> > -               fbc->possible_framebuffer_bits |=
> > -                       INTEL_FRONTBUFFER(pipe, PLANE_PRIMARY);
> > -
> > -               if (fbc_on_pipe_a_only(dev_priv))
> > -                       break;
> > -       }
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

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

Re: [PATCH 1/6] drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

[Ville Syrjälä]

On Wed, Feb 21, 2018 at 09:52:04PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-02-21 16:02:30)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Gen2/3 display engine depends on the fence for tiled scanout. So if we
> > fail to get a fence fail the entire operation.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 5d46771d58f6..66b269bc24b9 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2123,6 +2123,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> >                 goto err;
> >  
> >         if (i915_vma_is_map_and_fenceable(vma)) {
> > +               int ret;
> > +
> >                 /* Install a fence for tiled scan-out. Pre-i965 always needs a
> >                  * fence, whereas 965+ only requires a fence if using
> >                  * framebuffer compression.  For simplicity, we always, when
> > @@ -2139,7 +2141,13 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> >                  * something and try to run the system in a "less than optimal"
> >                  * mode that matches the user configuration.
> >                  */
> > -               if (i915_vma_pin_fence(vma) == 0 && vma->fence)
> > +               ret = i915_vma_pin_fence(vma);
> > +               if (ret != 0 && INTEL_GEN(dev_priv) < 4) {
> > +                       vma = ERR_PTR(ret);
> > +                       goto err;
> > +               }
> > +
> > +               if (ret == 0 && vma->fence)
> >                         *out_flags |= PLANE_HAS_FENCE;
> >         }
> 
> Ok, I'd like to see INTEL_GEN(dev_priv) < 4 be replaced with say
> needs_fence (and may be passed in from the caller like wants_fence?).

I had that earlier, but then I didn't have the uses_fence. Maybe I
cook up some kind of input flags thing here with PLANE_NEEDS_FENCE
and PLANE_WANTS_FENCE (maybe with a better naming scheme to
distinguish from the output flags, or should we just share the
same namespace?).

And should we then move the gmch check out and instead have something
like PLANE_NEEDS_MAPPABLE?

> Then I'm wondering if a 
> 	if (WARN_ON(needs_fence && !(*flags & PLANE_HAS_FENCE))
> makes sense.

Just to make sure i915_vma_pin_fence() did its job correctly?

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

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

Re: [PATCH 1/6] drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

[Chris Wilson]

Quoting Ville Syrjälä (2018-02-22 14:13:34)
> On Wed, Feb 21, 2018 at 09:52:04PM +0000, Chris Wilson wrote:
> > Ok, I'd like to see INTEL_GEN(dev_priv) < 4 be replaced with say
> > needs_fence (and may be passed in from the caller like wants_fence?).
> 
> I had that earlier, but then I didn't have the uses_fence. Maybe I
> cook up some kind of input flags thing here with PLANE_NEEDS_FENCE
> and PLANE_WANTS_FENCE (maybe with a better naming scheme to
> distinguish from the output flags, or should we just share the
> same namespace?).

It's probably not worth it unless we want some more flexibility in
future.

> And should we then move the gmch check out and instead have something
> like PLANE_NEEDS_MAPPABLE?
> 
> > Then I'm wondering if a 
> >       if (WARN_ON(needs_fence && !(*flags & PLANE_HAS_FENCE))
> > makes sense.
> 
> Just to make sure i915_vma_pin_fence() did its job correctly?

and i915_vma_pin() etc, yes.

At the end we would have something like:

	i915_vma_pin();
	if (uses_fence && i915_vma_pin_fence())
		flags |= HAS_FENCE;
	if (WARN_ON(needs_fence && !(flags & HAS_FENCE))
		...

(with the error checking along the way, it will be even less clear).
I expect the controlling logic to only get more complicated, so having a
few sanity checks between wants and gets seems useful.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 3/6] drm/i915: Clean up fbc vs. plane checks

[Ville Syrjälä]

On Thu, Feb 22, 2018 at 04:07:53PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 21, 2018 at 09:59:27PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-02-21 17:31:01)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Let's record the information whether a plane can do fbc or not under
> > > struct inte_plane.
> > > 
> > > v2: Rebase due to i9xx_plane_id
> > >     Handle BDW/HSW correctly
> > > v3: Move inte_fbc_init() back since we depend on it happening
> > >     even with i915.disable_display, and populate
> > >     fbc->possible_framebuffer_bits directly from the
> > >     plane init code instead
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> > >  drivers/gpu/drm/i915/intel_fbc.c     | 26 ++---------------------
> > >  3 files changed, 44 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index d2a66704e6f5..c60d2215b377 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13238,6 +13238,32 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> > >         .format_mod_supported = intel_cursor_plane_format_mod_supported,
> > >  };
> > >  
> > > +static bool i9xx_plane_has_fbc(struct drm_i915_private *dev_priv,
> > > +                              enum i9xx_plane_id i9xx_plane)
> > > +{
> > > +       if (!HAS_FBC(dev_priv))
> > > +               return false;
> > 
> > > -static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
> > > -{
> > > -       return IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8;
> > > -}
> > > -
> > > -static inline bool fbc_on_plane_a_only(struct drm_i915_private *dev_priv)
> > > -{
> > > -       return INTEL_GEN(dev_priv) < 4;
> > > -}
> > 
> > > +
> > > +       if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> > > +               return i9xx_plane == PLANE_A;
> > 
> > Where PLANE_A is tied to PIPE_A by construction. Is that worth a quick
> > comment?
> 
> Wouldn't hurt to spell it out I suppose.

Note added and series pushed to dinq. Thanks for the review.

I'll think a bit more about the need_fence thing and try to come up
a followup patch/series.

> 
> > 
> > > +       else if (IS_IVYBRIDGE(dev_priv))
> > > +               return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B ||
> > > +                       i9xx_plane == PLANE_C;
> > > +       else if (INTEL_GEN(dev_priv) >= 4)
> > > +               return i9xx_plane == PLANE_A || i9xx_plane == PLANE_B;
> > > +       else
> > > +               return i9xx_plane == PLANE_A;
> > 
> > Ok, looks a bit more complete than before.
> > 
> > > +}
> > > +
> > > +static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
> > > +                             enum pipe pipe, enum plane_id plane_id)
> > > +{
> > > +       if (!HAS_FBC(dev_priv))
> > > +               return false;
> > > +
> > > +       return pipe == PIPE_A && plane_id == PLANE_PRIMARY;
> > > +}
> > > +
> > >  static struct intel_plane *
> > >  intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> > >  {
> > > @@ -13280,6 +13306,21 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> > >                 primary->i9xx_plane = (enum i9xx_plane_id) pipe;
> > >         primary->id = PLANE_PRIMARY;
> > >         primary->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, primary->id);
> > > +
> > > +       if (INTEL_GEN(dev_priv) >= 9)
> > > +               primary->has_fbc = skl_plane_has_fbc(dev_priv,
> > > +                                                    primary->pipe,
> > > +                                                    primary->id);
> > > +       else
> > > +               primary->has_fbc = i9xx_plane_has_fbc(dev_priv,
> > > +                                                     primary->i9xx_plane);
> > > +
> > > +       if (primary->has_fbc) {
> > > +               struct intel_fbc *fbc = &dev_priv->fbc;
> > > +
> > > +               fbc->possible_framebuffer_bits |= primary->frontbuffer_bit;
> > > +       }
> > 
> > So an equivalent init loop to
> > 
> > > -       for_each_pipe(dev_priv, pipe) {
> > > -               fbc->possible_framebuffer_bits |=
> > > -                       INTEL_FRONTBUFFER(pipe, PLANE_PRIMARY);
> > > -
> > > -               if (fbc_on_pipe_a_only(dev_priv))
> > > -                       break;
> > > -       }
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > -Chris
> 
> -- 
> Ville Syrjälä
> Intel OTC

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