* [PATCH v2 0/4] drm/i915: Calculate intermediate watermarks correctly.
@ 2017-05-01 13:34 Maarten Lankhorst
2017-05-01 13:34 ` [PATCH v2 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state Maarten Lankhorst
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-01 13:34 UTC (permalink / raw)
To: intel-gfx
Calculate intermediate watermarks with the correct previous optimal watermark values.
This is the same as v1, but with helper macro to get the old state.
Maarten Lankhorst (4):
drm/i915: Change use get_new_plane_state instead of existing plane
state
drm/i915: Change get_existing_crtc_state to old state
drm/i915: Calculate ironlake intermediate watermarks correctly, v2.
drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
drivers/gpu/drm/i915/intel_atomic.c | 4 ++--
drivers/gpu/drm/i915/intel_drv.h | 10 +++++-----
drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++++-----------
3 files changed, 29 insertions(+), 18 deletions(-)
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state
2017-05-01 13:34 [PATCH v2 0/4] drm/i915: Calculate intermediate watermarks correctly Maarten Lankhorst
@ 2017-05-01 13:34 ` Maarten Lankhorst
2017-05-03 13:45 ` Ville Syrjälä
2017-05-01 13:34 ` [PATCH v2 2/4] drm/i915: Change get_existing_crtc_state to old state Maarten Lankhorst
` (3 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-01 13:34 UTC (permalink / raw)
To: intel-gfx
The get_existing macros are deprecated and should be replaced by
get_old/new_state for clarity.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 4 ++--
drivers/gpu/drm/i915/intel_drv.h | 4 ++--
drivers/gpu/drm/i915/intel_pm.c | 6 ++----
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index d791b3ef89b5..87b1dd464eee 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -301,8 +301,8 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
continue;
}
- plane_state = intel_atomic_get_existing_plane_state(drm_state,
- intel_plane);
+ plane_state = intel_atomic_get_new_plane_state(drm_state,
+ intel_plane);
scaler_id = &plane_state->scaler_id;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 110d9088be35..054b454a8d07 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1929,12 +1929,12 @@ intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
}
static inline struct intel_plane_state *
-intel_atomic_get_existing_plane_state(struct drm_atomic_state *state,
+intel_atomic_get_new_plane_state(struct drm_atomic_state *state,
struct intel_plane *plane)
{
struct drm_plane_state *plane_state;
- plane_state = drm_atomic_get_existing_plane_state(state, &plane->base);
+ plane_state = drm_atomic_get_new_plane_state(state, &plane->base);
return to_intel_plane_state(plane_state);
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cacb65fa2dd5..246e0723e6e9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2561,8 +2561,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate)
for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
struct intel_plane_state *ps;
- ps = intel_atomic_get_existing_plane_state(state,
- intel_plane);
+ ps = intel_atomic_get_new_plane_state(state, intel_plane);
if (!ps)
continue;
@@ -3924,8 +3923,7 @@ skl_compute_wm_level(const struct drm_i915_private *dev_priv,
if (state)
intel_pstate =
- intel_atomic_get_existing_plane_state(state,
- intel_plane);
+ intel_atomic_get_new_plane_state(state, intel_plane);
/*
* Note: If we start supporting multiple pending atomic commits against
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/4] drm/i915: Change get_existing_crtc_state to old state
2017-05-01 13:34 [PATCH v2 0/4] drm/i915: Calculate intermediate watermarks correctly Maarten Lankhorst
2017-05-01 13:34 ` [PATCH v2 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state Maarten Lankhorst
@ 2017-05-01 13:34 ` Maarten Lankhorst
2017-05-03 13:46 ` Ville Syrjälä
2017-05-01 13:34 ` [PATCH v2 3/4] drm/i915: Calculate ironlake intermediate watermarks correctly, v2 Maarten Lankhorst
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-01 13:34 UTC (permalink / raw)
To: intel-gfx
get_existing_crtc_state is currently unused, but the next commit
needs to access the old intel state. Rename this and use this
as convenience wrapper.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 054b454a8d07..f69b56d8f7d7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1915,12 +1915,12 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
}
static inline struct intel_crtc_state *
-intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
- struct intel_crtc *crtc)
+intel_atomic_get_old_crtc_state(struct drm_atomic_state *state,
+ struct intel_crtc *crtc)
{
struct drm_crtc_state *crtc_state;
- crtc_state = drm_atomic_get_existing_crtc_state(state, &crtc->base);
+ crtc_state = drm_atomic_get_old_crtc_state(state, &crtc->base);
if (crtc_state)
return to_intel_crtc_state(crtc_state);
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/4] drm/i915: Calculate ironlake intermediate watermarks correctly, v2.
2017-05-01 13:34 [PATCH v2 0/4] drm/i915: Calculate intermediate watermarks correctly Maarten Lankhorst
2017-05-01 13:34 ` [PATCH v2 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state Maarten Lankhorst
2017-05-01 13:34 ` [PATCH v2 2/4] drm/i915: Change get_existing_crtc_state to old state Maarten Lankhorst
@ 2017-05-01 13:34 ` Maarten Lankhorst
2017-05-03 13:48 ` Ville Syrjälä
2017-05-01 13:34 ` [PATCH v2 4/4] drm/i915: Calculate vlv/chv " Maarten Lankhorst
2017-05-01 13:53 ` ✓ Fi.CI.BAT: success for drm/i915: Calculate intermediate watermarks correctly Patchwork
4 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-01 13:34 UTC (permalink / raw)
To: intel-gfx
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.
Changes since v1:
- Use intel_atomic_get_old_crtc_state. (ville)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 246e0723e6e9..0f344b1fff45 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2638,7 +2638,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
struct intel_crtc_state *newstate)
{
struct intel_pipe_wm *a = &newstate->wm.ilk.intermediate;
- struct intel_pipe_wm *b = &intel_crtc->wm.active.ilk;
+ const struct intel_crtc_state *oldstate =
+ intel_atomic_get_old_crtc_state(newstate->base.state, intel_crtc);
+ const struct intel_pipe_wm *b = &oldstate->wm.ilk.optimal;
int level, max_level = ilk_wm_max_level(to_i915(dev));
/*
@@ -2647,6 +2649,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
* and after the vblank.
*/
*a = newstate->wm.ilk.optimal;
+ if (!newstate->base.active || drm_atomic_crtc_needs_modeset(&newstate->base))
+ return 0;
+
a->pipe_enabled |= b->pipe_enabled;
a->sprites_enabled |= b->sprites_enabled;
a->sprites_scaled |= b->sprites_scaled;
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-01 13:34 [PATCH v2 0/4] drm/i915: Calculate intermediate watermarks correctly Maarten Lankhorst
` (2 preceding siblings ...)
2017-05-01 13:34 ` [PATCH v2 3/4] drm/i915: Calculate ironlake intermediate watermarks correctly, v2 Maarten Lankhorst
@ 2017-05-01 13:34 ` Maarten Lankhorst
2017-05-03 13:45 ` Ville Syrjälä
2017-05-01 13:53 ` ✓ Fi.CI.BAT: success for drm/i915: Calculate intermediate watermarks correctly Patchwork
4 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-01 13:34 UTC (permalink / raw)
To: intel-gfx
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.
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)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0f344b1fff45..a09396ee1f3d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1458,16 +1458,24 @@ 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;
+
+ return 0;
+ }
+
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;
@@ -1491,7 +1499,7 @@ static int vlv_compute_intermediate_wm(struct drm_device *dev,
* 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;
}
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Calculate intermediate watermarks correctly.
2017-05-01 13:34 [PATCH v2 0/4] drm/i915: Calculate intermediate watermarks correctly Maarten Lankhorst
` (3 preceding siblings ...)
2017-05-01 13:34 ` [PATCH v2 4/4] drm/i915: Calculate vlv/chv " Maarten Lankhorst
@ 2017-05-01 13:53 ` Patchwork
4 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2017-05-01 13:53 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Calculate intermediate watermarks correctly.
URL : https://patchwork.freedesktop.org/series/23769/
State : success
== Summary ==
Series 23769v1 drm/i915: Calculate intermediate watermarks correctly.
https://patchwork.freedesktop.org/api/1.0/series/23769/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:429s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:436s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:584s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:509s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:563s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:491s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:483s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:412s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:402s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:416s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:478s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:475s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:455s
fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:567s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:455s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:570s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:463s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:497s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:428s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:525s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:399s
1d490e4b6d5324cfbf8dc800cf4a99471252802c drm-tip: 2017y-04m-28d-14h-14m-47s UTC integration manifest
aefcd83 drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
e039db0 drm/i915: Calculate ironlake intermediate watermarks correctly, v2.
157989e drm/i915: Change get_existing_crtc_state to old state
60a0a96 drm/i915: Change use get_new_plane_state instead of existing plane state
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4586/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-01 13:34 ` [PATCH v2 4/4] drm/i915: Calculate vlv/chv " Maarten Lankhorst
@ 2017-05-03 13:45 ` Ville Syrjälä
2017-05-03 14:06 ` Maarten Lankhorst
0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-03 13:45 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
> 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.
>
> 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)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0f344b1fff45..a09396ee1f3d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1458,16 +1458,24 @@ 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;
> +
> + return 0;
> + }
> +
> 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;
We need to consider disable_cxsr even in the modeset case.
>
> for (level = 0; level < intermediate->num_levels; level++) {
> enum plane_id plane_id;
> @@ -1491,7 +1499,7 @@ static int vlv_compute_intermediate_wm(struct drm_device *dev,
> * 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;
> }
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state
2017-05-01 13:34 ` [PATCH v2 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state Maarten Lankhorst
@ 2017-05-03 13:45 ` Ville Syrjälä
0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-03 13:45 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, May 01, 2017 at 03:34:31PM +0200, Maarten Lankhorst wrote:
> The get_existing macros are deprecated and should be replaced by
> get_old/new_state for clarity.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 4 ++--
> drivers/gpu/drm/i915/intel_drv.h | 4 ++--
> drivers/gpu/drm/i915/intel_pm.c | 6 ++----
> 3 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index d791b3ef89b5..87b1dd464eee 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -301,8 +301,8 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
> continue;
> }
>
> - plane_state = intel_atomic_get_existing_plane_state(drm_state,
> - intel_plane);
> + plane_state = intel_atomic_get_new_plane_state(drm_state,
> + intel_plane);
> scaler_id = &plane_state->scaler_id;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 110d9088be35..054b454a8d07 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1929,12 +1929,12 @@ intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
> }
>
> static inline struct intel_plane_state *
> -intel_atomic_get_existing_plane_state(struct drm_atomic_state *state,
> +intel_atomic_get_new_plane_state(struct drm_atomic_state *state,
> struct intel_plane *plane)
> {
> struct drm_plane_state *plane_state;
>
> - plane_state = drm_atomic_get_existing_plane_state(state, &plane->base);
> + plane_state = drm_atomic_get_new_plane_state(state, &plane->base);
>
> return to_intel_plane_state(plane_state);
> }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index cacb65fa2dd5..246e0723e6e9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2561,8 +2561,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate)
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> struct intel_plane_state *ps;
>
> - ps = intel_atomic_get_existing_plane_state(state,
> - intel_plane);
> + ps = intel_atomic_get_new_plane_state(state, intel_plane);
> if (!ps)
> continue;
>
> @@ -3924,8 +3923,7 @@ skl_compute_wm_level(const struct drm_i915_private *dev_priv,
>
> if (state)
> intel_pstate =
> - intel_atomic_get_existing_plane_state(state,
> - intel_plane);
> + intel_atomic_get_new_plane_state(state, intel_plane);
>
> /*
> * Note: If we start supporting multiple pending atomic commits against
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] drm/i915: Change get_existing_crtc_state to old state
2017-05-01 13:34 ` [PATCH v2 2/4] drm/i915: Change get_existing_crtc_state to old state Maarten Lankhorst
@ 2017-05-03 13:46 ` Ville Syrjälä
0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-03 13:46 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, May 01, 2017 at 03:34:32PM +0200, Maarten Lankhorst wrote:
> get_existing_crtc_state is currently unused, but the next commit
> needs to access the old intel state. Rename this and use this
> as convenience wrapper.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 054b454a8d07..f69b56d8f7d7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1915,12 +1915,12 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
> }
>
> static inline struct intel_crtc_state *
> -intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
> - struct intel_crtc *crtc)
> +intel_atomic_get_old_crtc_state(struct drm_atomic_state *state,
> + struct intel_crtc *crtc)
> {
> struct drm_crtc_state *crtc_state;
>
> - crtc_state = drm_atomic_get_existing_crtc_state(state, &crtc->base);
> + crtc_state = drm_atomic_get_old_crtc_state(state, &crtc->base);
>
> if (crtc_state)
> return to_intel_crtc_state(crtc_state);
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] drm/i915: Calculate ironlake intermediate watermarks correctly, v2.
2017-05-01 13:34 ` [PATCH v2 3/4] drm/i915: Calculate ironlake intermediate watermarks correctly, v2 Maarten Lankhorst
@ 2017-05-03 13:48 ` Ville Syrjälä
0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-03 13:48 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, May 01, 2017 at 03:34:33PM +0200, Maarten Lankhorst wrote:
> 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.
>
> Changes since v1:
> - Use intel_atomic_get_old_crtc_state. (ville)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 246e0723e6e9..0f344b1fff45 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2638,7 +2638,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
> struct intel_crtc_state *newstate)
> {
> struct intel_pipe_wm *a = &newstate->wm.ilk.intermediate;
> - struct intel_pipe_wm *b = &intel_crtc->wm.active.ilk;
> + const struct intel_crtc_state *oldstate =
> + intel_atomic_get_old_crtc_state(newstate->base.state, intel_crtc);
> + const struct intel_pipe_wm *b = &oldstate->wm.ilk.optimal;
> int level, max_level = ilk_wm_max_level(to_i915(dev));
>
> /*
> @@ -2647,6 +2649,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
> * and after the vblank.
> */
> *a = newstate->wm.ilk.optimal;
> + if (!newstate->base.active || drm_atomic_crtc_needs_modeset(&newstate->base))
> + return 0;
> +
> a->pipe_enabled |= b->pipe_enabled;
> a->sprites_enabled |= b->sprites_enabled;
> a->sprites_scaled |= b->sprites_scaled;
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 13:45 ` Ville Syrjälä
@ 2017-05-03 14:06 ` Maarten Lankhorst
2017-05-03 14:11 ` Ville Syrjälä
0 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-03 14:06 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 03-05-17 om 15:45 schreef Ville Syrjälä:
> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
>> 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.
>>
>> 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)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 0f344b1fff45..a09396ee1f3d 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -1458,16 +1458,24 @@ 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;
>> +
>> + return 0;
>> + }
>> +
>> 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;
> We need to consider disable_cxsr even in the modeset case.
Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 14:06 ` Maarten Lankhorst
@ 2017-05-03 14:11 ` Ville Syrjälä
2017-05-03 15:53 ` Maarten Lankhorst
0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-03 14:11 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
> > On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
> >> 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.
> >>
> >> 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)
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
> >> 1 file changed, 14 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 0f344b1fff45..a09396ee1f3d 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -1458,16 +1458,24 @@ 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;
> >> +
> >> + return 0;
> >> + }
> >> +
> >> 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;
> > We need to consider disable_cxsr even in the modeset case.
> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
It's set if any plane is enabling/disabling, which should be quite
typical during a modeset.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 14:11 ` Ville Syrjälä
@ 2017-05-03 15:53 ` Maarten Lankhorst
2017-05-03 16:07 ` Ville Syrjälä
0 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-03 15:53 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 03-05-17 om 16:11 schreef Ville Syrjälä:
> On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
>> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
>>> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
>>>> 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.
>>>>
>>>> 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)
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
>>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>> index 0f344b1fff45..a09396ee1f3d 100644
>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>> @@ -1458,16 +1458,24 @@ 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;
>>>> +
>>>> + return 0;
>>>> + }
>>>> +
>>>> 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;
>>> We need to consider disable_cxsr even in the modeset case.
>> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
> It's set if any plane is enabling/disabling, which should be quite
> typical during a modeset.
Yeah but .initial_watermarks is called during crtc_enable, so cxsr will get enabled anyway.
Maybe I'm missing something, but why are the intermediate watermarks and optimize_watermarks needed here?
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 15:53 ` Maarten Lankhorst
@ 2017-05-03 16:07 ` Ville Syrjälä
2017-05-03 16:18 ` Maarten Lankhorst
0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-03 16:07 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, May 03, 2017 at 05:53:34PM +0200, Maarten Lankhorst wrote:
> Op 03-05-17 om 16:11 schreef Ville Syrjälä:
> > On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
> >> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
> >>> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
> >>>> 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.
> >>>>
> >>>> 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)
> >>>>
> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
> >>>> 1 file changed, 14 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>>> index 0f344b1fff45..a09396ee1f3d 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_pm.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >>>> @@ -1458,16 +1458,24 @@ 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;
> >>>> +
> >>>> + return 0;
> >>>> + }
> >>>> +
> >>>> 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;
> >>> We need to consider disable_cxsr even in the modeset case.
> >> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
> > It's set if any plane is enabling/disabling, which should be quite
> > typical during a modeset.
>
> Yeah but .initial_watermarks is called during crtc_enable, so cxsr will get enabled anyway.
Which is not what we want. CxSR must stay off until the planes have been
enabled.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 16:07 ` Ville Syrjälä
@ 2017-05-03 16:18 ` Maarten Lankhorst
2017-05-03 18:03 ` Ville Syrjälä
0 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-03 16:18 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 03-05-17 om 18:07 schreef Ville Syrjälä:
> On Wed, May 03, 2017 at 05:53:34PM +0200, Maarten Lankhorst wrote:
>> Op 03-05-17 om 16:11 schreef Ville Syrjälä:
>>> On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
>>>> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
>>>>> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
>>>>>> 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.
>>>>>>
>>>>>> 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)
>>>>>>
>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
>>>>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>>> index 0f344b1fff45..a09396ee1f3d 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>>> @@ -1458,16 +1458,24 @@ 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;
>>>>>> +
>>>>>> + return 0;
>>>>>> + }
>>>>>> +
>>>>>> 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;
>>>>> We need to consider disable_cxsr even in the modeset case.
>>>> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
>>> It's set if any plane is enabling/disabling, which should be quite
>>> typical during a modeset.
>> Yeah but .initial_watermarks is called during crtc_enable, so cxsr will get enabled anyway.
> Which is not what we want. CxSR must stay off until the planes have been
> enabled.
>
In that case why is it enabled in .initial_watermarks at all? It should be in optimize_watermarks then..
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 16:18 ` Maarten Lankhorst
@ 2017-05-03 18:03 ` Ville Syrjälä
2017-05-03 20:28 ` Maarten Lankhorst
2017-05-04 8:12 ` Maarten Lankhorst
0 siblings, 2 replies; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-03 18:03 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, May 03, 2017 at 06:18:46PM +0200, Maarten Lankhorst wrote:
> Op 03-05-17 om 18:07 schreef Ville Syrjälä:
> > On Wed, May 03, 2017 at 05:53:34PM +0200, Maarten Lankhorst wrote:
> >> Op 03-05-17 om 16:11 schreef Ville Syrjälä:
> >>> On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
> >>>> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
> >>>>> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
> >>>>>> 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.
> >>>>>>
> >>>>>> 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)
> >>>>>>
> >>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>>> ---
> >>>>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
> >>>>>> 1 file changed, 14 insertions(+), 6 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>>>>> index 0f344b1fff45..a09396ee1f3d 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >>>>>> @@ -1458,16 +1458,24 @@ 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;
> >>>>>> +
> >>>>>> + return 0;
> >>>>>> + }
> >>>>>> +
> >>>>>> 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;
> >>>>> We need to consider disable_cxsr even in the modeset case.
> >>>> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
> >>> It's set if any plane is enabling/disabling, which should be quite
> >>> typical during a modeset.
> >> Yeah but .initial_watermarks is called during crtc_enable, so cxsr will get enabled anyway.
> > Which is not what we want. CxSR must stay off until the planes have been
> > enabled.
> >
> In that case why is it enabled in .initial_watermarks at all? It should be in optimize_watermarks then..
Because we can keep it enabled across the update unless planes are
getting enabled or disabled.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 18:03 ` Ville Syrjälä
@ 2017-05-03 20:28 ` Maarten Lankhorst
2017-05-04 8:12 ` Maarten Lankhorst
1 sibling, 0 replies; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-03 20:28 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 03-05-17 om 20:03 schreef Ville Syrjälä:
> On Wed, May 03, 2017 at 06:18:46PM +0200, Maarten Lankhorst wrote:
>> Op 03-05-17 om 18:07 schreef Ville Syrjälä:
>>> On Wed, May 03, 2017 at 05:53:34PM +0200, Maarten Lankhorst wrote:
>>>> Op 03-05-17 om 16:11 schreef Ville Syrjälä:
>>>>> On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
>>>>>> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
>>>>>>> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> 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)
>>>>>>>>
>>>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>>>> ---
>>>>>>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
>>>>>>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>>>>> index 0f344b1fff45..a09396ee1f3d 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>>>>> @@ -1458,16 +1458,24 @@ 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;
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> 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;
>>>>>>> We need to consider disable_cxsr even in the modeset case.
>>>>>> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
>>>>> It's set if any plane is enabling/disabling, which should be quite
>>>>> typical during a modeset.
>>>> Yeah but .initial_watermarks is called during crtc_enable, so cxsr will get enabled anyway.
>>> Which is not what we want. CxSR must stay off until the planes have been
>>> enabled.
>>>
>> In that case why is it enabled in .initial_watermarks at all? It should be in optimize_watermarks then..
> Because we can keep it enabled across the update unless planes are
> getting enabled or disabled.
In that case the watermarks still need some fixing to handle this correctly, and optimize watermarks should be called on modeset too.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-03 18:03 ` Ville Syrjälä
2017-05-03 20:28 ` Maarten Lankhorst
@ 2017-05-04 8:12 ` Maarten Lankhorst
2017-05-04 10:56 ` Ville Syrjälä
1 sibling, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2017-05-04 8:12 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 03-05-17 om 20:03 schreef Ville Syrjälä:
> On Wed, May 03, 2017 at 06:18:46PM +0200, Maarten Lankhorst wrote:
>> Op 03-05-17 om 18:07 schreef Ville Syrjälä:
>>> On Wed, May 03, 2017 at 05:53:34PM +0200, Maarten Lankhorst wrote:
>>>> Op 03-05-17 om 16:11 schreef Ville Syrjälä:
>>>>> On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
>>>>>> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
>>>>>>> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> 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)
>>>>>>>>
>>>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>>>> ---
>>>>>>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
>>>>>>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>>>>> index 0f344b1fff45..a09396ee1f3d 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>>>>> @@ -1458,16 +1458,24 @@ 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;
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> 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;
>>>>>>> We need to consider disable_cxsr even in the modeset case.
>>>>>> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
>>>>> It's set if any plane is enabling/disabling, which should be quite
>>>>> typical during a modeset.
>>>> Yeah but .initial_watermarks is called during crtc_enable, so cxsr will get enabled anyway.
>>> Which is not what we want. CxSR must stay off until the planes have been
>>> enabled.
>>>
>> In that case why is it enabled in .initial_watermarks at all? It should be in optimize_watermarks then..
> Because we can keep it enabled across the update unless planes are
> getting enabled or disabled.
>
So for the modeset case, computing intermediate watermarks:
*intermediate = *optimal;
if (needs_modeset)
intermediate->cxsr = false;
if (optimal->cxsr && !intermediate->cxsr)
new_crtc_state->wm.need_postvbl_update = true;
?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v2.
2017-05-04 8:12 ` Maarten Lankhorst
@ 2017-05-04 10:56 ` Ville Syrjälä
0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2017-05-04 10:56 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, May 04, 2017 at 10:12:52AM +0200, Maarten Lankhorst wrote:
> Op 03-05-17 om 20:03 schreef Ville Syrjälä:
> > On Wed, May 03, 2017 at 06:18:46PM +0200, Maarten Lankhorst wrote:
> >> Op 03-05-17 om 18:07 schreef Ville Syrjälä:
> >>> On Wed, May 03, 2017 at 05:53:34PM +0200, Maarten Lankhorst wrote:
> >>>> Op 03-05-17 om 16:11 schreef Ville Syrjälä:
> >>>>> On Wed, May 03, 2017 at 04:06:37PM +0200, Maarten Lankhorst wrote:
> >>>>>> Op 03-05-17 om 15:45 schreef Ville Syrjälä:
> >>>>>>> On Mon, May 01, 2017 at 03:34:34PM +0200, Maarten Lankhorst wrote:
> >>>>>>>> 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.
> >>>>>>>>
> >>>>>>>> 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)
> >>>>>>>>
> >>>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>>>>> ---
> >>>>>>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++------
> >>>>>>>> 1 file changed, 14 insertions(+), 6 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>>>>>>> index 0f344b1fff45..a09396ee1f3d 100644
> >>>>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
> >>>>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >>>>>>>> @@ -1458,16 +1458,24 @@ 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;
> >>>>>>>> +
> >>>>>>>> + return 0;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> 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;
> >>>>>>> We need to consider disable_cxsr even in the modeset case.
> >>>>>> Why is this? crtc_state->disable_cxsr is set if any plane is part of the crtc during modeset, so it's disabled during modeset already.
> >>>>> It's set if any plane is enabling/disabling, which should be quite
> >>>>> typical during a modeset.
> >>>> Yeah but .initial_watermarks is called during crtc_enable, so cxsr will get enabled anyway.
> >>> Which is not what we want. CxSR must stay off until the planes have been
> >>> enabled.
> >>>
> >> In that case why is it enabled in .initial_watermarks at all? It should be in optimize_watermarks then..
> > Because we can keep it enabled across the update unless planes are
> > getting enabled or disabled.
> >
> So for the modeset case, computing intermediate watermarks:
>
> *intermediate = *optimal;
> if (needs_modeset)
> intermediate->cxsr = false;
>
> if (optimal->cxsr && !intermediate->cxsr)
> new_crtc_state->wm.need_postvbl_update = true;
>
> ?
Or maybe
if (blah) {
*intermediate = *optimal;
goto out;
}
// min/max stuff
out:
if (disable_cxsr)
intermediate->cxsr = false;
if (memcmp(...
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-05-04 10:56 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-01 13:34 [PATCH v2 0/4] drm/i915: Calculate intermediate watermarks correctly Maarten Lankhorst
2017-05-01 13:34 ` [PATCH v2 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state Maarten Lankhorst
2017-05-03 13:45 ` Ville Syrjälä
2017-05-01 13:34 ` [PATCH v2 2/4] drm/i915: Change get_existing_crtc_state to old state Maarten Lankhorst
2017-05-03 13:46 ` Ville Syrjälä
2017-05-01 13:34 ` [PATCH v2 3/4] drm/i915: Calculate ironlake intermediate watermarks correctly, v2 Maarten Lankhorst
2017-05-03 13:48 ` Ville Syrjälä
2017-05-01 13:34 ` [PATCH v2 4/4] drm/i915: Calculate vlv/chv " Maarten Lankhorst
2017-05-03 13:45 ` Ville Syrjälä
2017-05-03 14:06 ` Maarten Lankhorst
2017-05-03 14:11 ` Ville Syrjälä
2017-05-03 15:53 ` Maarten Lankhorst
2017-05-03 16:07 ` Ville Syrjälä
2017-05-03 16:18 ` Maarten Lankhorst
2017-05-03 18:03 ` Ville Syrjälä
2017-05-03 20:28 ` Maarten Lankhorst
2017-05-04 8:12 ` Maarten Lankhorst
2017-05-04 10:56 ` Ville Syrjälä
2017-05-01 13:53 ` ✓ Fi.CI.BAT: success for drm/i915: Calculate intermediate watermarks correctly Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.