[PATCH v2 0/3] SKL watermark fixes for !fbcon

[PATCH v2 0/3] SKL watermark fixes for !fbcon

[Matt Roper]

There are a handful of watermark bugs that are only triggered (or more easily
triggered) when running without fbcon.  Thanks Tvrtko for pointing these out.

Mate Roper (3):
  drm/i915/gen9: Initialize intel_state->active_crtcs during WM
    sanitization (v2)
  drm/i915/gen9: Compute data rates for all planes on first commit
  drm/i915/gen9: Drop invalid WARN() during data rate calculation

 drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

-- 
2.1.4

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

[PATCH v2 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2)

[Matt Roper]

intel_state->active_crtcs is usually only initialized when doing a
modeset.  During our first atomic commit after boot, we're effectively
faking a modeset to sanitize the DDB/wm setup, so ensure that this field
gets initialized before use.

v2:
 - Don't clobber active_crtcs if our first commit really is a modeset
   (Maarten)
 - Grab connection_mutex when faking a modeset during sanitization
   (Maarten)

Reported-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 658a756..c06a8a3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3896,9 +3896,24 @@ skl_compute_ddb(struct drm_atomic_state *state)
 	 * pretend that all pipes switched active status so that we'll
 	 * ensure a full DDB recompute.
 	 */
-	if (dev_priv->wm.distrust_bios_wm)
+	if (dev_priv->wm.distrust_bios_wm) {
+		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
+				       state->acquire_ctx);
+		if (ret)
+			return ret;
+
 		intel_state->active_pipe_changes = ~0;
 
+		/*
+		 * We usually only initialize intel_state->active_crtcs if we
+		 * we're doing a modeset; make sure this field is always
+		 * initialized during the sanitization process that happens
+		 * on the first commit too.
+		 */
+		if (!intel_state->modeset)
+			intel_state->active_crtcs = dev_priv->active_crtcs;
+	}
+
 	/*
 	 * If the modeset changes which CRTC's are active, we need to
 	 * recompute the DDB allocation for *all* active pipes, even
-- 
2.1.4

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

[PATCH v2 2/3] drm/i915/gen9: Compute data rates for all planes on first commit

[Matt Roper]

When we sanitize our DDB and watermark info during the first atomic
commit, we need to calculate the total data rate.  Since we haven't
explicitly added the planes for each CRTC to our atomic state, the total
data rate calculation will try to use the cached values from a previous
commit (which are 0 since there was no previous commit); this result is
incorrect if we inherited any active planes from the BIOS.

During our very first atomic commit, we need to explicitly add all
active planes to the atomic state to ensure that valid data rate values
are calculated for them.  Subsequent commits will then have valid cached
values to fall back on.

Reported-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c06a8a3..4c425f6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3939,6 +3939,18 @@ skl_compute_ddb(struct drm_atomic_state *state)
 		if (IS_ERR(cstate))
 			return PTR_ERR(cstate);
 
+		/*
+		 * If this is our first commit after hw readout, we don't have
+		 * valid data rate values cached.  Add all planes to ensure we
+		 * calculate a valid data rate.
+		 */
+		if (dev_priv->wm.distrust_bios_wm) {
+			ret = drm_atomic_add_affected_planes(state,
+							     &intel_crtc->base);
+			if (ret)
+				return ret;
+		}
+
 		ret = skl_allocate_pipe_ddb(cstate, ddb);
 		if (ret)
 			return ret;
-- 
2.1.4

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

[PATCH v2 3/3] drm/i915/gen9: Drop invalid WARN() during data rate calculation

[Matt Roper]

It's possible to have a non-zero plane mask and still wind up with a
total data rate of zero.  There are two cases where this can happen:

 * planes are active (from the KMS point of view), but are
   all fully clipped (positioned offscreen)
 * the only active plane on a CRTC is the cursor (which is handled
   independently and not counted into the general data rate computations

These are both valid display setups (although unusual), so we need to
drop the WARN().

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Testcase: kms_universal_planes.cursor-only-pipe-*
---
 drivers/gpu/drm/i915/intel_pm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c425f6..1dc2e1d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3107,8 +3107,6 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate)
 		total_data_rate += intel_cstate->wm.skl.plane_y_data_rate[id];
 	}
 
-	WARN_ON(cstate->plane_mask && total_data_rate == 0);
-
 	return total_data_rate;
 }
 
-- 
2.1.4

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

✓ Ro.CI.BAT: success for SKL watermark fixes for !fbcon (rev2)

[Patchwork]

== Series Details ==

Series: SKL watermark fixes for !fbcon (rev2)
URL   : https://patchwork.freedesktop.org/series/8513/
State : success

== Summary ==

Series 8513v2 SKL watermark fixes for !fbcon
http://patchwork.freedesktop.org/api/1.0/series/8513/revisions/2/mbox

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-skl-i7-6700k  total:213  pass:185  dwarn:1   dfail:0   fail:2   skip:25 
ro-bdw-i5-5250u  total:213  pass:194  dwarn:2   dfail:0   fail:2   skip:15 
ro-bdw-i7-5557U  total:213  pass:195  dwarn:1   dfail:0   fail:2   skip:15 
ro-bdw-i7-5600u  total:213  pass:182  dwarn:1   dfail:0   fail:2   skip:28 
ro-bsw-n3050     total:213  pass:169  dwarn:1   dfail:0   fail:4   skip:39 
ro-byt-n2820     total:213  pass:170  dwarn:1   dfail:0   fail:5   skip:37 
ro-hsw-i3-4010u  total:213  pass:187  dwarn:1   dfail:0   fail:2   skip:23 
ro-hsw-i7-4770r  total:213  pass:187  dwarn:1   dfail:0   fail:2   skip:23 
ro-ilk-i7-620lm  total:213  pass:147  dwarn:1   dfail:0   fail:3   skip:62 
ro-ilk1-i5-650   total:208  pass:147  dwarn:1   dfail:0   fail:3   skip:57 
ro-ivb-i7-3770   total:213  pass:178  dwarn:1   dfail:0   fail:2   skip:32 
ro-ivb2-i7-3770  total:213  pass:182  dwarn:1   dfail:0   fail:2   skip:28 
ro-skl3-i5-6260u total:213  pass:199  dwarn:1   dfail:0   fail:2   skip:11 
ro-snb-i7-2620M  total:213  pass:171  dwarn:1   dfail:0   fail:3   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
fi-skl-i5-6260u failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1223/

8bf2b76 drm-intel-nightly: 2016y-06m-17d-19h-51m-02s UTC integration manifest
3948927 drm/i915/gen9: Drop invalid WARN() during data rate calculation
3e766e8 drm/i915/gen9: Compute data rates for all planes on first commit
36f6e9b drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2)

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

Re: [PATCH v2 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2)

[Tvrtko Ursulin]

On 17/06/16 21:42, Matt Roper wrote:
> intel_state->active_crtcs is usually only initialized when doing a
> modeset.  During our first atomic commit after boot, we're effectively
> faking a modeset to sanitize the DDB/wm setup, so ensure that this field
> gets initialized before use.
>
> v2:
>   - Don't clobber active_crtcs if our first commit really is a modeset
>     (Maarten)
>   - Grab connection_mutex when faking a modeset during sanitization
>     (Maarten)
>
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I am still applying this series locally in order to run nightly on SKL 
RVP with DP and no fbcon. :)

Regards,

Tvrtko

> ---
>   drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 658a756..c06a8a3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3896,9 +3896,24 @@ skl_compute_ddb(struct drm_atomic_state *state)
>   	 * pretend that all pipes switched active status so that we'll
>   	 * ensure a full DDB recompute.
>   	 */
> -	if (dev_priv->wm.distrust_bios_wm)
> +	if (dev_priv->wm.distrust_bios_wm) {
> +		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
> +				       state->acquire_ctx);
> +		if (ret)
> +			return ret;
> +
>   		intel_state->active_pipe_changes = ~0;
>
> +		/*
> +		 * We usually only initialize intel_state->active_crtcs if we
> +		 * we're doing a modeset; make sure this field is always
> +		 * initialized during the sanitization process that happens
> +		 * on the first commit too.
> +		 */
> +		if (!intel_state->modeset)
> +			intel_state->active_crtcs = dev_priv->active_crtcs;
> +	}
> +
>   	/*
>   	 * If the modeset changes which CRTC's are active, we need to
>   	 * recompute the DDB allocation for *all* active pipes, even
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx