[PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes

[PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes

[Paulo Zanoni]

We were previously adding all the planes owned by the CRTC even when
the ddb partitioning didn't change for them. As a consequence, a lot
of functions were being called when we were just moving the cursor
around the screen, such as skylake_update_primary_plane().

This was causing flickering on the primary plane when moving the
cursor. I'm not 100% sure which operation caused the flickering, but
we were writing to a lot of registers, so it could be any of these
writes. With this patch, just moving the mouse won't add the primary
plane to the commit since it won't trigger a change in DDB
partitioning.

Fixes: 05a76d3d6ad1 ("drm/i915/skl: Ensure pipes with changed wms get
added to the state")

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
Cc: Lyude <cpaul@redhat.com>
Cc: Mike Lothian <mike@fireburn.co.uk>
Cc: stable@vger.kernel.org
Reported-and-bisected-by: Mike Lothian <mike@fireburn.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)


I can confirm this fixes the flickering I was seeing.


diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5d39ad2..1cf34a3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3966,6 +3966,45 @@ pipes_modified(struct drm_atomic_state *state)
 	return ret;
 }
 
+int
+skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
+{
+	struct drm_atomic_state *state = cstate->base.state;
+	struct drm_device *dev = state->dev;
+	struct drm_crtc *crtc = cstate->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+	struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
+	struct drm_plane_state *plane_state;
+	struct drm_plane *plane;
+	enum pipe pipe = intel_crtc->pipe;
+	int id;
+
+	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
+
+	drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) {
+		id = skl_wm_plane_id(to_intel_plane(plane));
+
+		if (cur_ddb->plane[pipe][id].start ==
+		    new_ddb->plane[pipe][id].start &&
+		    cur_ddb->plane[pipe][id].end ==
+		    new_ddb->plane[pipe][id].end &&
+		    cur_ddb->y_plane[pipe][id].start ==
+		    new_ddb->y_plane[pipe][id].start &&
+		    cur_ddb->y_plane[pipe][id].end ==
+		    new_ddb->y_plane[pipe][id].end)
+			continue;
+
+		plane_state = drm_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state))
+			return PTR_ERR(plane_state);
+	}
+
+	return 0;
+}
+
 static int
 skl_compute_ddb(struct drm_atomic_state *state)
 {
@@ -4030,7 +4069,7 @@ skl_compute_ddb(struct drm_atomic_state *state)
 		if (ret)
 			return ret;
 
-		ret = drm_atomic_add_affected_planes(state, &intel_crtc->base);
+		ret = skl_ddb_add_affected_planes(cstate);
 		if (ret)
 			return ret;
 	}
-- 
2.7.4

Re: [PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes

[Lyude Paul]

On Thu, 2016-09-29 at 15:39 -0300, Paulo Zanoni wrote:
> We were previously adding all the planes owned by the CRTC even when
> the ddb partitioning didn't change for them. As a consequence, a lot
> of functions were being called when we were just moving the cursor
> around the screen, such as skylake_update_primary_plane().
> 
> This was causing flickering on the primary plane when moving the
> cursor. I'm not 100% sure which operation caused the flickering, but
> we were writing to a lot of registers, so it could be any of these
> writes. With this patch, just moving the mouse won't add the primary
> plane to the commit since it won't trigger a change in DDB
> partitioning.
> 
> Fixes: 05a76d3d6ad1 ("drm/i915/skl: Ensure pipes with changed wms get
> added to the state")
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
> Cc: Lyude <cpaul@redhat.com>
> Cc: Mike Lothian <mike@fireburn.co.uk>
> Cc: stable@vger.kernel.org
> Reported-and-bisected-by: Mike Lothian <mike@fireburn.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 41
> ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> 
> I can confirm this fixes the flickering I was seeing.
> 
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 5d39ad2..1cf34a3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3966,6 +3966,45 @@ pipes_modified(struct drm_atomic_state *state)
>  	return ret;
>  }
>  
> +int
> +skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
> +{
> +	struct drm_atomic_state *state = cstate->base.state;
> +	struct drm_device *dev = state->dev;
> +	struct drm_crtc *crtc = cstate->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> +	struct skl_ddb_allocation *new_ddb = &intel_state-
> >wm_results.ddb;
> +	struct skl_ddb_allocation *cur_ddb = &dev_priv-
> >wm.skl_hw.ddb;
> +	struct drm_plane_state *plane_state;
> +	struct drm_plane *plane;
> +	enum pipe pipe = intel_crtc->pipe;
> +	int id;
> +
> +	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
> +
> +	drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) 
> {
> +		id = skl_wm_plane_id(to_intel_plane(plane));
> +
> +		if (cur_ddb->plane[pipe][id].start ==
> +		    new_ddb->plane[pipe][id].start &&
> +		    cur_ddb->plane[pipe][id].end ==
> +		    new_ddb->plane[pipe][id].end &&
> +		    cur_ddb->y_plane[pipe][id].start ==
> +		    new_ddb->y_plane[pipe][id].start &&
> +		    cur_ddb->y_plane[pipe][id].end ==
> +		    new_ddb->y_plane[pipe][id].end)
> +			continue;

Just use skl_ddb_entry_equals() here.
With that fixed:

Reviewed-by: Lyude <cpaul@redhat.com>

> +
> +		plane_state = drm_atomic_get_plane_state(state,
> plane);
> +		if (IS_ERR(plane_state))
> +			return PTR_ERR(plane_state);
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  skl_compute_ddb(struct drm_atomic_state *state)
>  {
> @@ -4030,7 +4069,7 @@ skl_compute_ddb(struct drm_atomic_state *state)
>  		if (ret)
>  			return ret;
>  
> -		ret = drm_atomic_add_affected_planes(state,
> &intel_crtc->base);
> +		ret = skl_ddb_add_affected_planes(cstate);
>  		if (ret)
>  			return ret;
>  	}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes

[Lyude Paul]

On Thu, 2016-09-29 at 15:39 -0300, Paulo Zanoni wrote:
> We were previously adding all the planes owned by the CRTC even when
> the ddb partitioning didn't change for them. As a consequence, a lot
> of functions were being called when we were just moving the cursor
> around the screen, such as skylake_update_primary_plane().
> 
> This was causing flickering on the primary plane when moving the
> cursor. I'm not 100% sure which operation caused the flickering, but
> we were writing to a lot of registers, so it could be any of these
> writes. With this patch, just moving the mouse won't add the primary
> plane to the commit since it won't trigger a change in DDB
> partitioning.
> 
> Fixes: 05a76d3d6ad1 ("drm/i915/skl: Ensure pipes with changed wms get
> added to the state")
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
> Cc: Lyude <cpaul@redhat.com>
> Cc: Mike Lothian <mike@fireburn.co.uk>
> Cc: stable@vger.kernel.org
> Reported-and-bisected-by: Mike Lothian <mike@fireburn.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 41
> ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> 
> I can confirm this fixes the flickering I was seeing.
> 
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 5d39ad2..1cf34a3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3966,6 +3966,45 @@ pipes_modified(struct drm_atomic_state *state)
>  	return ret;
>  }
>  
> +int
> +skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
> +{
> +	struct drm_atomic_state *state = cstate->base.state;
> +	struct drm_device *dev = state->dev;
> +	struct drm_crtc *crtc = cstate->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> +	struct skl_ddb_allocation *new_ddb = &intel_state-
> >wm_results.ddb;
> +	struct skl_ddb_allocation *cur_ddb = &dev_priv-
> >wm.skl_hw.ddb;
> +	struct drm_plane_state *plane_state;
> +	struct drm_plane *plane;
> +	enum pipe pipe = intel_crtc->pipe;
> +	int id;
> +
> +	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
> +
> +	drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) 
> {
> +		id = skl_wm_plane_id(to_intel_plane(plane));
> +
> +		if (cur_ddb->plane[pipe][id].start ==
> +		    new_ddb->plane[pipe][id].start &&
> +		    cur_ddb->plane[pipe][id].end ==
> +		    new_ddb->plane[pipe][id].end &&
> +		    cur_ddb->y_plane[pipe][id].start ==
> +		    new_ddb->y_plane[pipe][id].start &&
> +		    cur_ddb->y_plane[pipe][id].end ==
> +		    new_ddb->y_plane[pipe][id].end)
> +			continue;

Just use skl_ddb_entry_equals() here.
With that fixed:

Reviewed-by: Lyude <cpaul@redhat.com>

> +
> +		plane_state = drm_atomic_get_plane_state(state,
> plane);
> +		if (IS_ERR(plane_state))
> +			return PTR_ERR(plane_state);
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  skl_compute_ddb(struct drm_atomic_state *state)
>  {
> @@ -4030,7 +4069,7 @@ skl_compute_ddb(struct drm_atomic_state *state)
>  		if (ret)
>  			return ret;
>  
> -		ret = drm_atomic_add_affected_planes(state,
> &intel_crtc->base);
> +		ret = skl_ddb_add_affected_planes(cstate);
>  		if (ret)
>  			return ret;
>  	}

✗ Fi.CI.BAT: warning for drm/i915/gen9: only add the planes actually affected by ddb changes

[Patchwork]

== Series Details ==

Series: drm/i915/gen9: only add the planes actually affected by ddb changes
URL   : https://patchwork.freedesktop.org/series/13095/
State : warning

== Summary ==

Series 13095v1 drm/i915/gen9: only add the planes actually affected by ddb changes
https://patchwork.freedesktop.org/api/1.0/series/13095/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-byt-j1900)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:244  pass:214  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:244  pass:211  dwarn:1   dfail:0   fail:1   skip:31 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:244  pass:181  dwarn:1   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
fi-skl-6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2596/

2fbc239494fa3883e164cc89d35f54d22f0c9e1f drm-intel-nightly: 2016y-09m-29d-13h-31m-39s UTC integration manifest
58c2acb drm/i915/gen9: only add the planes actually affected by ddb changes

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

[PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes

[Paulo Zanoni]

We were previously adding all the planes owned by the CRTC even when
the ddb partitioning didn't change for them. As a consequence, a lot
of functions were being called when we were just moving the cursor
around the screen, such as skylake_update_primary_plane().

This was causing flickering on the primary plane when moving the
cursor. I'm not 100% sure which operation caused the flickering, but
we were writing to a lot of registers, so it could be any of these
writes. With this patch, just moving the mouse won't add the primary
plane to the commit since it won't trigger a change in DDB
partitioning.

v2: Use skl_ddb_entry_equal() (Lyude).

Fixes: 05a76d3d6ad1 ("drm/i915/skl: Ensure pipes with changed wms get added to the state")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
Cc: Lyude <cpaul@redhat.com>
Cc: Mike Lothian <mike@fireburn.co.uk>
Cc: stable@vger.kernel.org
Reported-and-bisected-by: Mike Lothian <mike@fireburn.co.uk>
Reviewed-by: Lyude <cpaul@redhat.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5d39ad2..425544b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3966,6 +3966,41 @@ pipes_modified(struct drm_atomic_state *state)
 	return ret;
 }
 
+int
+skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
+{
+	struct drm_atomic_state *state = cstate->base.state;
+	struct drm_device *dev = state->dev;
+	struct drm_crtc *crtc = cstate->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+	struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
+	struct drm_plane_state *plane_state;
+	struct drm_plane *plane;
+	enum pipe pipe = intel_crtc->pipe;
+	int id;
+
+	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
+
+	drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) {
+		id = skl_wm_plane_id(to_intel_plane(plane));
+
+		if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][id],
+					&new_ddb->plane[pipe][id]) &&
+		    skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][id],
+					&new_ddb->y_plane[pipe][id]))
+			continue;
+
+		plane_state = drm_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state))
+			return PTR_ERR(plane_state);
+	}
+
+	return 0;
+}
+
 static int
 skl_compute_ddb(struct drm_atomic_state *state)
 {
@@ -4030,7 +4065,7 @@ skl_compute_ddb(struct drm_atomic_state *state)
 		if (ret)
 			return ret;
 
-		ret = drm_atomic_add_affected_planes(state, &intel_crtc->base);
+		ret = skl_ddb_add_affected_planes(cstate);
 		if (ret)
 			return ret;
 	}
-- 
2.7.4

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

[PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes

[Paulo Zanoni]

We were previously adding all the planes owned by the CRTC even when
the ddb partitioning didn't change for them. As a consequence, a lot
of functions were being called when we were just moving the cursor
around the screen, such as skylake_update_primary_plane().

This was causing flickering on the primary plane when moving the
cursor. I'm not 100% sure which operation caused the flickering, but
we were writing to a lot of registers, so it could be any of these
writes. With this patch, just moving the mouse won't add the primary
plane to the commit since it won't trigger a change in DDB
partitioning.

v2: Use skl_ddb_entry_equal() (Lyude).

Fixes: 05a76d3d6ad1 ("drm/i915/skl: Ensure pipes with changed wms get added to the state")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
Cc: Lyude <cpaul@redhat.com>
Cc: Mike Lothian <mike@fireburn.co.uk>
Cc: stable@vger.kernel.org
Reported-and-bisected-by: Mike Lothian <mike@fireburn.co.uk>
Reviewed-by: Lyude <cpaul@redhat.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5d39ad2..425544b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3966,6 +3966,41 @@ pipes_modified(struct drm_atomic_state *state)
 	return ret;
 }
 
+int
+skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
+{
+	struct drm_atomic_state *state = cstate->base.state;
+	struct drm_device *dev = state->dev;
+	struct drm_crtc *crtc = cstate->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+	struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
+	struct drm_plane_state *plane_state;
+	struct drm_plane *plane;
+	enum pipe pipe = intel_crtc->pipe;
+	int id;
+
+	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
+
+	drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) {
+		id = skl_wm_plane_id(to_intel_plane(plane));
+
+		if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][id],
+					&new_ddb->plane[pipe][id]) &&
+		    skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][id],
+					&new_ddb->y_plane[pipe][id]))
+			continue;
+
+		plane_state = drm_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state))
+			return PTR_ERR(plane_state);
+	}
+
+	return 0;
+}
+
 static int
 skl_compute_ddb(struct drm_atomic_state *state)
 {
@@ -4030,7 +4065,7 @@ skl_compute_ddb(struct drm_atomic_state *state)
 		if (ret)
 			return ret;
 
-		ret = drm_atomic_add_affected_planes(state, &intel_crtc->base);
+		ret = skl_ddb_add_affected_planes(cstate);
 		if (ret)
 			return ret;
 	}
-- 
2.7.4

✗ Fi.CI.BAT: warning for drm/i915/gen9: only add the planes actually affected by ddb changes (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gen9: only add the planes actually affected by ddb changes (rev2)
URL   : https://patchwork.freedesktop.org/series/13095/
State : warning

== Summary ==

Series 13095v2 drm/i915/gen9: only add the planes actually affected by ddb changes
https://patchwork.freedesktop.org/api/1.0/series/13095/revisions/2/mbox/

Test drv_module_reload_basic:
                pass       -> SKIP       (fi-ivb-3520m)
Test kms_busy:
        Subgroup basic-flip-default-a:
                pass       -> DMESG-WARN (fi-ilk-650)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> SKIP       (fi-ivb-3770)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:244  pass:214  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:244  pass:211  dwarn:1   dfail:0   fail:1   skip:31 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:244  pass:181  dwarn:1   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:244  pass:218  dwarn:0   dfail:0   fail:0   skip:26 
fi-ivb-3770      total:244  pass:206  dwarn:0   dfail:0   fail:0   skip:38 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
fi-skl-6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2597/

2fbc239494fa3883e164cc89d35f54d22f0c9e1f drm-intel-nightly: 2016y-09m-29d-13h-31m-39s UTC integration manifest
8bbcff6 drm/i915/gen9: only add the planes actually affected by ddb changes

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

Re: [PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes

[Lyude]

Pushed, thanks!

On Thu, 2016-09-29 at 16:36 -0300, Paulo Zanoni wrote:
> We were previously adding all the planes owned by the CRTC even when
> the ddb partitioning didn't change for them. As a consequence, a lot
> of functions were being called when we were just moving the cursor
> around the screen, such as skylake_update_primary_plane().
> 
> This was causing flickering on the primary plane when moving the
> cursor. I'm not 100% sure which operation caused the flickering, but
> we were writing to a lot of registers, so it could be any of these
> writes. With this patch, just moving the mouse won't add the primary
> plane to the commit since it won't trigger a change in DDB
> partitioning.
> 
> v2: Use skl_ddb_entry_equal() (Lyude).
> 
> Fixes: 05a76d3d6ad1 ("drm/i915/skl: Ensure pipes with changed wms get
> added to the state")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
> Cc: Lyude <cpaul@redhat.com>
> Cc: Mike Lothian <mike@fireburn.co.uk>
> Cc: stable@vger.kernel.org
> Reported-and-bisected-by: Mike Lothian <mike@fireburn.co.uk>
> Reviewed-by: Lyude <cpaul@redhat.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 37
> ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 5d39ad2..425544b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3966,6 +3966,41 @@ pipes_modified(struct drm_atomic_state *state)
>  	return ret;
>  }
>  
> +int
> +skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
> +{
> +	struct drm_atomic_state *state = cstate->base.state;
> +	struct drm_device *dev = state->dev;
> +	struct drm_crtc *crtc = cstate->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> +	struct skl_ddb_allocation *new_ddb = &intel_state-
> >wm_results.ddb;
> +	struct skl_ddb_allocation *cur_ddb = &dev_priv-
> >wm.skl_hw.ddb;
> +	struct drm_plane_state *plane_state;
> +	struct drm_plane *plane;
> +	enum pipe pipe = intel_crtc->pipe;
> +	int id;
> +
> +	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
> +
> +	drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) 
> {
> +		id = skl_wm_plane_id(to_intel_plane(plane));
> +
> +		if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][id],
> +					&new_ddb->plane[pipe][id])
> &&
> +		    skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][id],
> +					&new_ddb-
> >y_plane[pipe][id]))
> +			continue;
> +
> +		plane_state = drm_atomic_get_plane_state(state,
> plane);
> +		if (IS_ERR(plane_state))
> +			return PTR_ERR(plane_state);
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  skl_compute_ddb(struct drm_atomic_state *state)
>  {
> @@ -4030,7 +4065,7 @@ skl_compute_ddb(struct drm_atomic_state *state)
>  		if (ret)
>  			return ret;
>  
> -		ret = drm_atomic_add_affected_planes(state,
> &intel_crtc->base);
> +		ret = skl_ddb_add_affected_planes(cstate);
>  		if (ret)
>  			return ret;
>  	}
-- 
Cheers,
	Lyude