diff mbox

[RFC,8/8] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

Message ID 20170720131519.30104-9-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst July 20, 2017, 1:15 p.m. UTC
The watermarks it should calculate against are the old optimal watermarks.
The currently active crtc watermarks are pure fiction, and are invalid in
case of a nonblocking modeset, page flip enabling/disabling planes or any
other reason.

When the crtc is disabled or during a modeset the intermediate watermarks
don't need to be programmed separately, and could be directly assigned
to the optimal watermarks.

CXSR must always be disabled in the intermediate case for modesets, else
we get a WARN for vblank wait timeout.

Also rename crtc_state to new_crtc_state, to distinguish it from the old state.

Changes since v1:
- Use intel_atomic_get_old_crtc_state. (ville)
Changes since v2:
- Always unset cxsr during modeset.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101597
---
 drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

Maarten Lankhorst July 20, 2017, 4:26 p.m. UTC | #1
Op 20-07-17 om 15:15 schreef Maarten Lankhorst:
> The watermarks it should calculate against are the old optimal watermarks.
> The currently active crtc watermarks are pure fiction, and are invalid in
> case of a nonblocking modeset, page flip enabling/disabling planes or any
> other reason.
>
> When the crtc is disabled or during a modeset the intermediate watermarks
> don't need to be programmed separately, and could be directly assigned
> to the optimal watermarks.
>
> CXSR must always be disabled in the intermediate case for modesets, else
> we get a WARN for vblank wait timeout.
>
> Also rename crtc_state to new_crtc_state, to distinguish it from the old state.
>
> Changes since v1:
> - Use intel_atomic_get_old_crtc_state. (ville)
> Changes since v2:
> - Always unset cxsr during modeset.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101597
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0048bf043337..96de4f8e3b2a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2004,16 +2004,25 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  static int vlv_compute_intermediate_wm(struct drm_device *dev,
>  				       struct intel_crtc *crtc,
> -				       struct intel_crtc_state *crtc_state)
> +				       struct intel_crtc_state *new_crtc_state)
>  {
> -	struct vlv_wm_state *intermediate = &crtc_state->wm.vlv.intermediate;
> -	const struct vlv_wm_state *optimal = &crtc_state->wm.vlv.optimal;
> -	const struct vlv_wm_state *active = &crtc->wm.active.vlv;
> +	struct vlv_wm_state *intermediate = &new_crtc_state->wm.vlv.intermediate;
> +	const struct vlv_wm_state *optimal = &new_crtc_state->wm.vlv.optimal;
> +	const struct intel_crtc_state *old_crtc_state =
> +		intel_atomic_get_old_crtc_state(new_crtc_state->base.state, crtc);
> +	const struct vlv_wm_state *active = &old_crtc_state->wm.vlv.optimal;
>  	int level;
>  
> +	if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
> +		*intermediate = *optimal;
> +
> +		intermediate->cxsr = false;
> +		goto out;
> +	}
Derp, can't fix the above bug since it's PNV.. But still needs to be done here.

So I'll have to take out the bugzilla listed above..
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0048bf043337..96de4f8e3b2a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2004,16 +2004,25 @@  static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 static int vlv_compute_intermediate_wm(struct drm_device *dev,
 				       struct intel_crtc *crtc,
-				       struct intel_crtc_state *crtc_state)
+				       struct intel_crtc_state *new_crtc_state)
 {
-	struct vlv_wm_state *intermediate = &crtc_state->wm.vlv.intermediate;
-	const struct vlv_wm_state *optimal = &crtc_state->wm.vlv.optimal;
-	const struct vlv_wm_state *active = &crtc->wm.active.vlv;
+	struct vlv_wm_state *intermediate = &new_crtc_state->wm.vlv.intermediate;
+	const struct vlv_wm_state *optimal = &new_crtc_state->wm.vlv.optimal;
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(new_crtc_state->base.state, crtc);
+	const struct vlv_wm_state *active = &old_crtc_state->wm.vlv.optimal;
 	int level;
 
+	if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
+		*intermediate = *optimal;
+
+		intermediate->cxsr = false;
+		goto out;
+	}
+
 	intermediate->num_levels = min(optimal->num_levels, active->num_levels);
 	intermediate->cxsr = optimal->cxsr && active->cxsr &&
-		!crtc_state->disable_cxsr;
+		!new_crtc_state->disable_cxsr;
 
 	for (level = 0; level < intermediate->num_levels; level++) {
 		enum plane_id plane_id;
@@ -2032,12 +2041,13 @@  static int vlv_compute_intermediate_wm(struct drm_device *dev,
 
 	vlv_invalidate_wms(crtc, intermediate, level);
 
+out:
 	/*
 	 * If our intermediate WM are identical to the final WM, then we can
 	 * omit the post-vblank programming; only update if it's different.
 	 */
 	if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
-		crtc_state->wm.need_postvbl_update = true;
+		new_crtc_state->wm.need_postvbl_update = true;
 
 	return 0;
 }