* [PATCH v2 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2)
2016-06-17 20:42 [PATCH v2 0/3] SKL watermark fixes for !fbcon Matt Roper
@ 2016-06-17 20:42 ` Matt Roper
2016-07-13 15:10 ` Tvrtko Ursulin
2016-06-17 20:42 ` [PATCH v2 2/3] drm/i915/gen9: Compute data rates for all planes on first commit Matt Roper
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Matt Roper @ 2016-06-17 20:42 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2)
2016-06-17 20:42 ` [PATCH v2 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2) Matt Roper
@ 2016-07-13 15:10 ` Tvrtko Ursulin
0 siblings, 0 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:10 UTC (permalink / raw)
To: Matt Roper, intel-gfx
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] drm/i915/gen9: Compute data rates for all planes on first commit
2016-06-17 20:42 [PATCH v2 0/3] SKL watermark fixes for !fbcon Matt Roper
2016-06-17 20:42 ` [PATCH v2 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2) Matt Roper
@ 2016-06-17 20:42 ` Matt Roper
2016-06-17 20:42 ` [PATCH v2 3/3] drm/i915/gen9: Drop invalid WARN() during data rate calculation Matt Roper
2016-06-18 6:09 ` ✓ Ro.CI.BAT: success for SKL watermark fixes for !fbcon (rev2) Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Matt Roper @ 2016-06-17 20:42 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] drm/i915/gen9: Drop invalid WARN() during data rate calculation
2016-06-17 20:42 [PATCH v2 0/3] SKL watermark fixes for !fbcon Matt Roper
2016-06-17 20:42 ` [PATCH v2 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization (v2) Matt Roper
2016-06-17 20:42 ` [PATCH v2 2/3] drm/i915/gen9: Compute data rates for all planes on first commit Matt Roper
@ 2016-06-17 20:42 ` Matt Roper
2016-06-18 6:09 ` ✓ Ro.CI.BAT: success for SKL watermark fixes for !fbcon (rev2) Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Matt Roper @ 2016-06-17 20:42 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Ro.CI.BAT: success for SKL watermark fixes for !fbcon (rev2)
2016-06-17 20:42 [PATCH v2 0/3] SKL watermark fixes for !fbcon Matt Roper
` (2 preceding siblings ...)
2016-06-17 20:42 ` [PATCH v2 3/3] drm/i915/gen9: Drop invalid WARN() during data rate calculation Matt Roper
@ 2016-06-18 6:09 ` Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-06-18 6:09 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 6+ messages in thread