* [PATCH] drm/i915: Postpone plane readout until after encoder readout
@ 2015-07-31 13:04 Patrik Jakobsson
2015-08-03 6:31 ` Maarten Lankhorst
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Patrik Jakobsson @ 2015-07-31 13:04 UTC (permalink / raw)
To: intel-gfx
When reading out hw state for planes we disable inactive planes which in
turn triggers an update of the watermarks. The update depends on the
crtc_clock being set which is done when reading out encoders. Thus
postpone the plane readout until after encoder readout.
This prevents a warning in skl_compute_linetime_wm() where pixel_rate
becomes 0 when crtc_clock is 0.
Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 13a6608..73b2d4a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15205,27 +15205,25 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
}
-static void readout_plane_state(struct intel_crtc *crtc,
- struct intel_crtc_state *crtc_state)
+static void intel_sanitize_plane(struct intel_plane *plane)
{
- struct intel_plane *p;
struct intel_plane_state *plane_state;
- bool active = crtc_state->base.active;
+ struct intel_crtc *crtc;
- for_each_intel_plane(crtc->base.dev, p) {
- if (crtc->pipe != p->pipe)
- continue;
+ plane_state = to_intel_plane_state(plane->base.state);
- plane_state = to_intel_plane_state(p->base.state);
+ if (!plane_state->base.crtc)
+ return;
- if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
- plane_state->visible = primary_get_hw_state(crtc);
- else {
- if (active)
- p->disable_plane(&p->base, &crtc->base);
+ crtc = to_intel_crtc(plane_state->base.crtc);
- plane_state->visible = false;
- }
+ if (plane->base.type == DRM_PLANE_TYPE_PRIMARY) {
+ plane_state->visible = primary_get_hw_state(crtc);
+ } else {
+ if (crtc->base.state->active)
+ plane->disable_plane(&plane->base, &crtc->base);
+
+ plane_state->visible = false;
}
}
@@ -15276,7 +15274,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
}
crtc->base.hwmode = crtc->config->base.adjusted_mode;
- readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
crtc->base.base.id,
@@ -15349,6 +15346,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
enum pipe pipe;
struct intel_crtc *crtc;
struct intel_encoder *encoder;
+ struct intel_plane *plane;
int i;
intel_modeset_readout_hw_state(dev);
@@ -15360,6 +15358,14 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
for_each_pipe(dev_priv, pipe) {
crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+
+ for_each_intel_plane(crtc->base.dev, plane) {
+ if (crtc->pipe != plane->pipe)
+ continue;
+
+ intel_sanitize_plane(plane);
+ }
+
intel_sanitize_crtc(crtc);
intel_dump_pipe_config(crtc, crtc->config,
"[setup_hw_state]");
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout
2015-07-31 13:04 [PATCH] drm/i915: Postpone plane readout until after encoder readout Patrik Jakobsson
@ 2015-08-03 6:31 ` Maarten Lankhorst
2015-08-03 14:36 ` Maarten Lankhorst
2015-08-10 17:56 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout shuang.he
2 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-08-03 6:31 UTC (permalink / raw)
To: Patrik Jakobsson, intel-gfx
Op 31-07-15 om 15:04 schreef Patrik Jakobsson:
> When reading out hw state for planes we disable inactive planes which in
> turn triggers an update of the watermarks. The update depends on the
> crtc_clock being set which is done when reading out encoders. Thus
> postpone the plane readout until after encoder readout.
>
> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
> becomes 0 when crtc_clock is 0.
>
The plane loop doesn't have to be nested.
But that's just a minor nitpick, feel free to ignore..
Reviewed-By: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout
2015-07-31 13:04 [PATCH] drm/i915: Postpone plane readout until after encoder readout Patrik Jakobsson
2015-08-03 6:31 ` Maarten Lankhorst
@ 2015-08-03 14:36 ` Maarten Lankhorst
2015-08-04 15:37 ` Patrik Jakobsson
2015-08-10 17:56 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout shuang.he
2 siblings, 1 reply; 14+ messages in thread
From: Maarten Lankhorst @ 2015-08-03 14:36 UTC (permalink / raw)
To: Patrik Jakobsson, intel-gfx
Hey,
Op 31-07-15 om 15:04 schreef Patrik Jakobsson:
> When reading out hw state for planes we disable inactive planes which in
> turn triggers an update of the watermarks. The update depends on the
> crtc_clock being set which is done when reading out encoders. Thus
> postpone the plane readout until after encoder readout.
>
> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
> becomes 0 when crtc_clock is 0.
>
Running this on skylake it looks like like it's a partial fix.
You would need to move hwmode like below too?
Also:
References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index add7f807a82d..49907f4e939a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14888,8 +14888,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
}
- crtc->base.hwmode = crtc->config->base.adjusted_mode;
-
DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
crtc->base.base.id,
crtc->active ? "enabled" : "disabled");
@@ -14971,6 +14969,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
for_each_pipe(dev_priv, pipe) {
crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+ crtc->base.hwmode = crtc->config->base.adjusted_mode;
for_each_intel_plane(crtc->base.dev, plane) {
if (crtc->pipe != plane->pipe)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout
2015-08-03 14:36 ` Maarten Lankhorst
@ 2015-08-04 15:37 ` Patrik Jakobsson
2015-08-05 10:45 ` drm/i915: Postpone plane readout until after encoder readout, v2 Maarten Lankhorst
0 siblings, 1 reply; 14+ messages in thread
From: Patrik Jakobsson @ 2015-08-04 15:37 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Intel Graphics Development
On Mon, Aug 3, 2015 at 4:36 PM, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Hey,
>
> Op 31-07-15 om 15:04 schreef Patrik Jakobsson:
>> When reading out hw state for planes we disable inactive planes which in
>> turn triggers an update of the watermarks. The update depends on the
>> crtc_clock being set which is done when reading out encoders. Thus
>> postpone the plane readout until after encoder readout.
>>
>> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
>> becomes 0 when crtc_clock is 0.
>>
> Running this on skylake it looks like like it's a partial fix.
> You would need to move hwmode like below too?
>
> Also:
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
Ah yes, I'll send a v2. Thanks.
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index add7f807a82d..49907f4e939a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14888,8 +14888,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> }
>
> - crtc->base.hwmode = crtc->config->base.adjusted_mode;
> -
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> crtc->active ? "enabled" : "disabled");
> @@ -14971,6 +14969,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>
> for_each_pipe(dev_priv, pipe) {
> crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + crtc->base.hwmode = crtc->config->base.adjusted_mode;
>
> for_each_intel_plane(crtc->base.dev, plane) {
> if (crtc->pipe != plane->pipe)
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* drm/i915: Postpone plane readout until after encoder readout, v2.
2015-08-04 15:37 ` Patrik Jakobsson
@ 2015-08-05 10:45 ` Maarten Lankhorst
2015-08-26 14:43 ` Daniel Vetter
0 siblings, 1 reply; 14+ messages in thread
From: Maarten Lankhorst @ 2015-08-05 10:45 UTC (permalink / raw)
To: Patrik Jakobsson; +Cc: Intel Graphics Development
From: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
When reading out hw state for planes we disable inactive planes which in
turn triggers an update of the watermarks. The update depends on the
crtc_clock being set which is done when reading out encoders. Thus
postpone the plane readout until after encoder readout.
This prevents a warning in skl_compute_linetime_wm() where pixel_rate
becomes 0 when crtc_clock is 0.
Changes since v1:
- Set all modes after all state is read out. (Maarten)
References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index df237ad4a780..85d52158d476 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15042,27 +15042,25 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
}
-static void readout_plane_state(struct intel_crtc *crtc,
- struct intel_crtc_state *crtc_state)
+static void intel_sanitize_plane(struct intel_plane *plane)
{
- struct intel_plane *p;
struct intel_plane_state *plane_state;
- bool active = crtc_state->base.active;
+ struct intel_crtc *crtc;
- for_each_intel_plane(crtc->base.dev, p) {
- if (crtc->pipe != p->pipe)
- continue;
+ plane_state = to_intel_plane_state(plane->base.state);
- plane_state = to_intel_plane_state(p->base.state);
+ if (!plane_state->base.crtc)
+ return;
- if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
- plane_state->visible = primary_get_hw_state(crtc);
- else {
- if (active)
- p->disable_plane(&p->base, &crtc->base);
+ crtc = to_intel_crtc(plane_state->base.crtc);
- plane_state->visible = false;
- }
+ if (plane->base.type == DRM_PLANE_TYPE_PRIMARY) {
+ plane_state->visible = primary_get_hw_state(crtc);
+ } else {
+ if (crtc->base.state->active)
+ plane->disable_plane(&plane->base, &crtc->base);
+
+ plane_state->visible = false;
}
}
@@ -15086,35 +15084,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
crtc->base.state->active = crtc->active;
crtc->base.enabled = crtc->active;
- memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
- if (crtc->base.state->active) {
- intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
- intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
- WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
-
- /*
- * The initial mode needs to be set in order to keep
- * the atomic core happy. It wants a valid mode if the
- * crtc's enabled, so we do the above call.
- *
- * At this point some state updated by the connectors
- * in their ->detect() callback has not run yet, so
- * no recalculation can be done yet.
- *
- * Even if we could do a recalculation and modeset
- * right now it would cause a double modeset if
- * fbdev or userspace chooses a different initial mode.
- *
- * If that happens, someone indicated they wanted a
- * mode change, which means it's safe to do a full
- * recalculation.
- */
- crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
- }
-
- crtc->base.hwmode = crtc->config->base.adjusted_mode;
- readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
-
DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
crtc->base.base.id,
crtc->active ? "enabled" : "disabled");
@@ -15184,6 +15153,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
enum pipe pipe;
struct intel_crtc *crtc;
struct intel_encoder *encoder;
+ struct intel_plane *plane;
int i;
intel_modeset_readout_hw_state(dev);
@@ -15195,6 +15165,40 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
for_each_pipe(dev_priv, pipe) {
crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+ crtc->base.hwmode = crtc->config->base.adjusted_mode;
+ memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
+ if (crtc->base.state->active) {
+ intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
+ intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
+ WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
+
+ /*
+ * The initial mode needs to be set in order to keep
+ * the atomic core happy. It wants a valid mode if the
+ * crtc's enabled, so we do the above call.
+ *
+ * At this point some state updated by the connectors
+ * in their ->detect() callback has not run yet, so
+ * no recalculation can be done yet.
+ *
+ * Even if we could do a recalculation and modeset
+ * right now it would cause a double modeset if
+ * fbdev or userspace chooses a different initial mode.
+ *
+ * If that happens, someone indicated they wanted a
+ * mode change, which means it's safe to do a full
+ * recalculation.
+ */
+ crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
+ }
+
+ for_each_intel_plane(crtc->base.dev, plane) {
+ if (crtc->pipe != plane->pipe)
+ continue;
+
+ intel_sanitize_plane(plane);
+ }
+
intel_sanitize_crtc(crtc);
intel_dump_pipe_config(crtc, crtc->config,
"[setup_hw_state]");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout
2015-07-31 13:04 [PATCH] drm/i915: Postpone plane readout until after encoder readout Patrik Jakobsson
2015-08-03 6:31 ` Maarten Lankhorst
2015-08-03 14:36 ` Maarten Lankhorst
@ 2015-08-10 17:56 ` shuang.he
2 siblings, 0 replies; 14+ messages in thread
From: shuang.he @ 2015-08-10 17:56 UTC (permalink / raw)
To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
patrik.jakobsson
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6911
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK -1 302/302 301/302
SNB 315/315 315/315
IVB 336/336 336/336
BYT -2 283/283 281/283
HSW 378/378 378/378
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(1)
*BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1)
*BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: drm/i915: Postpone plane readout until after encoder readout, v2.
2015-08-05 10:45 ` drm/i915: Postpone plane readout until after encoder readout, v2 Maarten Lankhorst
@ 2015-08-26 14:43 ` Daniel Vetter
2015-08-27 11:05 ` Maarten Lankhorst
2015-08-27 11:47 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3 Maarten Lankhorst
0 siblings, 2 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-08-26 14:43 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Intel Graphics Development
On Wed, Aug 05, 2015 at 12:45:48PM +0200, Maarten Lankhorst wrote:
> From: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>
> When reading out hw state for planes we disable inactive planes which in
> turn triggers an update of the watermarks. The update depends on the
> crtc_clock being set which is done when reading out encoders. Thus
> postpone the plane readout until after encoder readout.
>
> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
> becomes 0 when crtc_clock is 0.
>
> Changes since v1:
> - Set all modes after all state is read out. (Maarten)
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Hm I still think we have a bit a mess here - the plane readout code is
still spread out between modeset_readout_hw_state and the per-plane loop
in intel_modeset_init after setup_hw_state.
I thought that we only ever need to do the plane state readout on initial
load (they're all off on resume anyway and we force a full modeset to make
sure plane state is correct again). Can't we just have a setup_plane_state
which has that loop from the end of modeset_init with all the other plane
state unified there?
-Daniel
> ---
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index df237ad4a780..85d52158d476 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15042,27 +15042,25 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
> }
>
> -static void readout_plane_state(struct intel_crtc *crtc,
> - struct intel_crtc_state *crtc_state)
> +static void intel_sanitize_plane(struct intel_plane *plane)
> {
> - struct intel_plane *p;
> struct intel_plane_state *plane_state;
> - bool active = crtc_state->base.active;
> + struct intel_crtc *crtc;
>
> - for_each_intel_plane(crtc->base.dev, p) {
> - if (crtc->pipe != p->pipe)
> - continue;
> + plane_state = to_intel_plane_state(plane->base.state);
>
> - plane_state = to_intel_plane_state(p->base.state);
> + if (!plane_state->base.crtc)
> + return;
>
> - if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
> - plane_state->visible = primary_get_hw_state(crtc);
> - else {
> - if (active)
> - p->disable_plane(&p->base, &crtc->base);
> + crtc = to_intel_crtc(plane_state->base.crtc);
>
> - plane_state->visible = false;
> - }
> + if (plane->base.type == DRM_PLANE_TYPE_PRIMARY) {
> + plane_state->visible = primary_get_hw_state(crtc);
> + } else {
> + if (crtc->base.state->active)
> + plane->disable_plane(&plane->base, &crtc->base);
> +
> + plane_state->visible = false;
> }
> }
>
> @@ -15086,35 +15084,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
>
> - memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> - if (crtc->base.state->active) {
> - intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> - intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> - WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> -
> - /*
> - * The initial mode needs to be set in order to keep
> - * the atomic core happy. It wants a valid mode if the
> - * crtc's enabled, so we do the above call.
> - *
> - * At this point some state updated by the connectors
> - * in their ->detect() callback has not run yet, so
> - * no recalculation can be done yet.
> - *
> - * Even if we could do a recalculation and modeset
> - * right now it would cause a double modeset if
> - * fbdev or userspace chooses a different initial mode.
> - *
> - * If that happens, someone indicated they wanted a
> - * mode change, which means it's safe to do a full
> - * recalculation.
> - */
> - crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> - }
> -
> - crtc->base.hwmode = crtc->config->base.adjusted_mode;
> - readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
> -
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> crtc->active ? "enabled" : "disabled");
> @@ -15184,6 +15153,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
> enum pipe pipe;
> struct intel_crtc *crtc;
> struct intel_encoder *encoder;
> + struct intel_plane *plane;
> int i;
>
> intel_modeset_readout_hw_state(dev);
> @@ -15195,6 +15165,40 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>
> for_each_pipe(dev_priv, pipe) {
> crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + crtc->base.hwmode = crtc->config->base.adjusted_mode;
> + memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> + if (crtc->base.state->active) {
> + intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> + intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> + WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> +
> + /*
> + * The initial mode needs to be set in order to keep
> + * the atomic core happy. It wants a valid mode if the
> + * crtc's enabled, so we do the above call.
> + *
> + * At this point some state updated by the connectors
> + * in their ->detect() callback has not run yet, so
> + * no recalculation can be done yet.
> + *
> + * Even if we could do a recalculation and modeset
> + * right now it would cause a double modeset if
> + * fbdev or userspace chooses a different initial mode.
> + *
> + * If that happens, someone indicated they wanted a
> + * mode change, which means it's safe to do a full
> + * recalculation.
> + */
> + crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> + }
> +
> + for_each_intel_plane(crtc->base.dev, plane) {
> + if (crtc->pipe != plane->pipe)
> + continue;
> +
> + intel_sanitize_plane(plane);
> + }
> +
> intel_sanitize_crtc(crtc);
> intel_dump_pipe_config(crtc, crtc->config,
> "[setup_hw_state]");
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: drm/i915: Postpone plane readout until after encoder readout, v2.
2015-08-26 14:43 ` Daniel Vetter
@ 2015-08-27 11:05 ` Maarten Lankhorst
2015-09-01 9:52 ` Daniel Vetter
2015-08-27 11:47 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3 Maarten Lankhorst
1 sibling, 1 reply; 14+ messages in thread
From: Maarten Lankhorst @ 2015-08-27 11:05 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
Hey,
Op 26-08-15 om 16:43 schreef Daniel Vetter:
> On Wed, Aug 05, 2015 at 12:45:48PM +0200, Maarten Lankhorst wrote:
>> From: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>>
>> When reading out hw state for planes we disable inactive planes which in
>> turn triggers an update of the watermarks. The update depends on the
>> crtc_clock being set which is done when reading out encoders. Thus
>> postpone the plane readout until after encoder readout.
>>
>> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
>> becomes 0 when crtc_clock is 0.
>>
>> Changes since v1:
>> - Set all modes after all state is read out. (Maarten)
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
>> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Hm I still think we have a bit a mess here - the plane readout code is
> still spread out between modeset_readout_hw_state and the per-plane loop
> in intel_modeset_init after setup_hw_state.
>
> I thought that we only ever need to do the plane state readout on initial
> load (they're all off on resume anyway and we force a full modeset to make
> sure plane state is correct again). Can't we just have a setup_plane_state
> which has that loop from the end of modeset_init with all the other plane
> state unified there?
>
Perhaps, but intel_crtc_disable_noatomic cares about visibility state.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3.
2015-08-26 14:43 ` Daniel Vetter
2015-08-27 11:05 ` Maarten Lankhorst
@ 2015-08-27 11:47 ` Maarten Lankhorst
2015-08-27 12:05 ` Ville Syrjälä
` (2 more replies)
1 sibling, 3 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-08-27 11:47 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
When reading out hw state for planes we disable inactive planes which in
turn triggers an update of the watermarks. The update depends on the
crtc_clock being set which is done when reading out encoders. Thus
postpone the plane readout until after encoder readout.
This prevents a warning in skl_compute_linetime_wm() where pixel_rate
becomes 0 when crtc_clock is 0.
Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
---
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e3922d973db0..118b205e7bd5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15033,30 +15033,6 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
}
-static void readout_plane_state(struct intel_crtc *crtc,
- struct intel_crtc_state *crtc_state)
-{
- struct intel_plane *p;
- struct intel_plane_state *plane_state;
- bool active = crtc_state->base.active;
-
- for_each_intel_plane(crtc->base.dev, p) {
- if (crtc->pipe != p->pipe)
- continue;
-
- plane_state = to_intel_plane_state(p->base.state);
-
- if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
- plane_state->visible = primary_get_hw_state(crtc);
- else {
- if (active)
- p->disable_plane(&p->base, &crtc->base);
-
- plane_state->visible = false;
- }
- }
-}
-
static void intel_modeset_readout_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -15076,35 +15052,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
crtc->base.state->active = crtc->active;
crtc->base.enabled = crtc->active;
-
- memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
- if (crtc->base.state->active) {
- intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
- intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
- WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
-
- /*
- * The initial mode needs to be set in order to keep
- * the atomic core happy. It wants a valid mode if the
- * crtc's enabled, so we do the above call.
- *
- * At this point some state updated by the connectors
- * in their ->detect() callback has not run yet, so
- * no recalculation can be done yet.
- *
- * Even if we could do a recalculation and modeset
- * right now it would cause a double modeset if
- * fbdev or userspace chooses a different initial mode.
- *
- * If that happens, someone indicated they wanted a
- * mode change, which means it's safe to do a full
- * recalculation.
- */
- crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
- }
-
- crtc->base.hwmode = crtc->config->base.adjusted_mode;
- readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
+ to_intel_plane_state(crtc->base.primary->state)->visible =
+ primary_get_hw_state(crtc);
DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
crtc->base.base.id,
@@ -15186,6 +15135,33 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
for_each_pipe(dev_priv, pipe) {
crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+ crtc->base.hwmode = crtc->config->base.adjusted_mode;
+ memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
+ if (crtc->base.state->active) {
+ intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
+ intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
+ WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
+
+ /*
+ * The initial mode needs to be set in order to keep
+ * the atomic core happy. It wants a valid mode if the
+ * crtc's enabled, so we do the above call.
+ *
+ * At this point some state updated by the connectors
+ * in their ->detect() callback has not run yet, so
+ * no recalculation can be done yet.
+ *
+ * Even if we could do a recalculation and modeset
+ * right now it would cause a double modeset if
+ * fbdev or userspace chooses a different initial mode.
+ *
+ * If that happens, someone indicated they wanted a
+ * mode change, which means it's safe to do a full
+ * recalculation.
+ */
+ crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
+ }
+
intel_sanitize_crtc(crtc);
intel_dump_pipe_config(crtc, crtc->config,
"[setup_hw_state]");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3.
2015-08-27 11:47 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3 Maarten Lankhorst
@ 2015-08-27 12:05 ` Ville Syrjälä
2015-08-31 5:59 ` shuang.he
2015-10-13 12:40 ` Jani Nikula
2 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2015-08-27 12:05 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Intel Graphics Development
On Thu, Aug 27, 2015 at 01:47:26PM +0200, Maarten Lankhorst wrote:
> When reading out hw state for planes we disable inactive planes which in
> turn triggers an update of the watermarks. The update depends on the
> crtc_clock being set which is done when reading out encoders. Thus
> postpone the plane readout until after encoder readout.
>
> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
> becomes 0 when crtc_clock is 0.
>
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
> ---
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e3922d973db0..118b205e7bd5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15033,30 +15033,6 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
> }
>
> -static void readout_plane_state(struct intel_crtc *crtc,
> - struct intel_crtc_state *crtc_state)
> -{
> - struct intel_plane *p;
> - struct intel_plane_state *plane_state;
> - bool active = crtc_state->base.active;
> -
> - for_each_intel_plane(crtc->base.dev, p) {
> - if (crtc->pipe != p->pipe)
> - continue;
> -
> - plane_state = to_intel_plane_state(p->base.state);
> -
> - if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
> - plane_state->visible = primary_get_hw_state(crtc);
> - else {
> - if (active)
> - p->disable_plane(&p->base, &crtc->base);
> -
> - plane_state->visible = false;
> - }
> - }
> -}
> -
> static void intel_modeset_readout_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -15076,35 +15052,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
> -
> - memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> - if (crtc->base.state->active) {
> - intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> - intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> - WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> -
> - /*
> - * The initial mode needs to be set in order to keep
> - * the atomic core happy. It wants a valid mode if the
> - * crtc's enabled, so we do the above call.
> - *
> - * At this point some state updated by the connectors
> - * in their ->detect() callback has not run yet, so
> - * no recalculation can be done yet.
> - *
> - * Even if we could do a recalculation and modeset
> - * right now it would cause a double modeset if
> - * fbdev or userspace chooses a different initial mode.
> - *
> - * If that happens, someone indicated they wanted a
> - * mode change, which means it's safe to do a full
> - * recalculation.
> - */
> - crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> - }
> -
> - crtc->base.hwmode = crtc->config->base.adjusted_mode;
> - readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
> + to_intel_plane_state(crtc->base.primary->state)->visible =
> + primary_get_hw_state(crtc);
The commit message is for a totally different patch it seems.
Hmm. Looking at this, I should have just moved the entire mode setup
block in my hwmode fix patch to the end of the function, and things would
be fixed.
>
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> @@ -15186,6 +15135,33 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>
> for_each_pipe(dev_priv, pipe) {
> crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + crtc->base.hwmode = crtc->config->base.adjusted_mode;
> + memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> + if (crtc->base.state->active) {
> + intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> + intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> + WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> +
> + /*
> + * The initial mode needs to be set in order to keep
> + * the atomic core happy. It wants a valid mode if the
> + * crtc's enabled, so we do the above call.
> + *
> + * At this point some state updated by the connectors
> + * in their ->detect() callback has not run yet, so
> + * no recalculation can be done yet.
> + *
> + * Even if we could do a recalculation and modeset
> + * right now it would cause a double modeset if
> + * fbdev or userspace chooses a different initial mode.
> + *
> + * If that happens, someone indicated they wanted a
> + * mode change, which means it's safe to do a full
> + * recalculation.
> + */
> + crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> + }
> +
Why not keep it in intel_modeset_readout_hw_state()? That way all the
low level gunk stays in intel_modeset_readout_hw_state() and
intel_modeset_setup_hw_state() just has the higher level stuff.
> intel_sanitize_crtc(crtc);
> intel_dump_pipe_config(crtc, crtc->config,
> "[setup_hw_state]");
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3.
2015-08-27 11:47 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3 Maarten Lankhorst
2015-08-27 12:05 ` Ville Syrjälä
@ 2015-08-31 5:59 ` shuang.he
2015-10-13 12:40 ` Jani Nikula
2 siblings, 0 replies; 14+ messages in thread
From: shuang.he @ 2015-08-31 5:59 UTC (permalink / raw)
To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
maarten.lankhorst
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7272
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK -1 302/302 301/302
SNB 315/315 315/315
IVB 336/336 336/336
BYT -1 283/283 282/283
HSW 378/378 378/378
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@kms_flip@wf_vblank-vs-modeset-interruptible PASS(1) DMESG_WARN(1)
*BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: drm/i915: Postpone plane readout until after encoder readout, v2.
2015-08-27 11:05 ` Maarten Lankhorst
@ 2015-09-01 9:52 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-09-01 9:52 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Intel Graphics Development
On Thu, Aug 27, 2015 at 01:05:12PM +0200, Maarten Lankhorst wrote:
> Hey,
>
> Op 26-08-15 om 16:43 schreef Daniel Vetter:
> > On Wed, Aug 05, 2015 at 12:45:48PM +0200, Maarten Lankhorst wrote:
> >> From: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> >>
> >> When reading out hw state for planes we disable inactive planes which in
> >> turn triggers an update of the watermarks. The update depends on the
> >> crtc_clock being set which is done when reading out encoders. Thus
> >> postpone the plane readout until after encoder readout.
> >>
> >> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
> >> becomes 0 when crtc_clock is 0.
> >>
> >> Changes since v1:
> >> - Set all modes after all state is read out. (Maarten)
> >>
> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
> >> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Hm I still think we have a bit a mess here - the plane readout code is
> > still spread out between modeset_readout_hw_state and the per-plane loop
> > in intel_modeset_init after setup_hw_state.
> >
> > I thought that we only ever need to do the plane state readout on initial
> > load (they're all off on resume anyway and we force a full modeset to make
> > sure plane state is correct again). Can't we just have a setup_plane_state
> > which has that loop from the end of modeset_init with all the other plane
> > state unified there?
> >
> Perhaps, but intel_crtc_disable_noatomic cares about visibility state.
I don't see a need for that any more at all, since disable_noatomic is
only used in sanitize_crtc. And that's only called when the bios fucks
something up and i915.ko needs to patch it up. No way a pageflip or
anything else is pending there. Also we don't seem to set up plane_mask
early enough for that function to correctly kill the primary plane.
That might explain some of the display faults Chris is seeing ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3.
2015-08-27 11:47 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3 Maarten Lankhorst
2015-08-27 12:05 ` Ville Syrjälä
2015-08-31 5:59 ` shuang.he
@ 2015-10-13 12:40 ` Jani Nikula
2015-10-13 12:44 ` Maarten Lankhorst
2 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2015-10-13 12:40 UTC (permalink / raw)
To: Maarten Lankhorst, Daniel Vetter; +Cc: Intel Graphics Development
On Thu, 27 Aug 2015, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> When reading out hw state for planes we disable inactive planes which in
> turn triggers an update of the watermarks. The update depends on the
> crtc_clock being set which is done when reading out encoders. Thus
> postpone the plane readout until after encoder readout.
>
> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
> becomes 0 when crtc_clock is 0.
>
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
Maarten, I presume this patch is superseded by the commits referenced in
that bug. Is that correct?
BR,
Jani.
> ---
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e3922d973db0..118b205e7bd5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15033,30 +15033,6 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
> }
>
> -static void readout_plane_state(struct intel_crtc *crtc,
> - struct intel_crtc_state *crtc_state)
> -{
> - struct intel_plane *p;
> - struct intel_plane_state *plane_state;
> - bool active = crtc_state->base.active;
> -
> - for_each_intel_plane(crtc->base.dev, p) {
> - if (crtc->pipe != p->pipe)
> - continue;
> -
> - plane_state = to_intel_plane_state(p->base.state);
> -
> - if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
> - plane_state->visible = primary_get_hw_state(crtc);
> - else {
> - if (active)
> - p->disable_plane(&p->base, &crtc->base);
> -
> - plane_state->visible = false;
> - }
> - }
> -}
> -
> static void intel_modeset_readout_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -15076,35 +15052,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
> -
> - memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> - if (crtc->base.state->active) {
> - intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> - intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> - WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> -
> - /*
> - * The initial mode needs to be set in order to keep
> - * the atomic core happy. It wants a valid mode if the
> - * crtc's enabled, so we do the above call.
> - *
> - * At this point some state updated by the connectors
> - * in their ->detect() callback has not run yet, so
> - * no recalculation can be done yet.
> - *
> - * Even if we could do a recalculation and modeset
> - * right now it would cause a double modeset if
> - * fbdev or userspace chooses a different initial mode.
> - *
> - * If that happens, someone indicated they wanted a
> - * mode change, which means it's safe to do a full
> - * recalculation.
> - */
> - crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> - }
> -
> - crtc->base.hwmode = crtc->config->base.adjusted_mode;
> - readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
> + to_intel_plane_state(crtc->base.primary->state)->visible =
> + primary_get_hw_state(crtc);
>
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> @@ -15186,6 +15135,33 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>
> for_each_pipe(dev_priv, pipe) {
> crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + crtc->base.hwmode = crtc->config->base.adjusted_mode;
> + memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> + if (crtc->base.state->active) {
> + intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> + intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> + WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> +
> + /*
> + * The initial mode needs to be set in order to keep
> + * the atomic core happy. It wants a valid mode if the
> + * crtc's enabled, so we do the above call.
> + *
> + * At this point some state updated by the connectors
> + * in their ->detect() callback has not run yet, so
> + * no recalculation can be done yet.
> + *
> + * Even if we could do a recalculation and modeset
> + * right now it would cause a double modeset if
> + * fbdev or userspace chooses a different initial mode.
> + *
> + * If that happens, someone indicated they wanted a
> + * mode change, which means it's safe to do a full
> + * recalculation.
> + */
> + crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> + }
> +
> intel_sanitize_crtc(crtc);
> intel_dump_pipe_config(crtc, crtc->config,
> "[setup_hw_state]");
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3.
2015-10-13 12:40 ` Jani Nikula
@ 2015-10-13 12:44 ` Maarten Lankhorst
0 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-10-13 12:44 UTC (permalink / raw)
To: Jani Nikula, Daniel Vetter; +Cc: Intel Graphics Development
Op 13-10-15 om 14:40 schreef Jani Nikula:
> On Thu, 27 Aug 2015, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>> When reading out hw state for planes we disable inactive planes which in
>> turn triggers an update of the watermarks. The update depends on the
>> crtc_clock being set which is done when reading out encoders. Thus
>> postpone the plane readout until after encoder readout.
>>
>> This prevents a warning in skl_compute_linetime_wm() where pixel_rate
>> becomes 0 when crtc_clock is 0.
>>
>> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=91428
> Maarten, I presume this patch is superseded by the commits referenced in
> that bug. Is that correct?
>
Correct. :)
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-10-13 12:44 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-31 13:04 [PATCH] drm/i915: Postpone plane readout until after encoder readout Patrik Jakobsson
2015-08-03 6:31 ` Maarten Lankhorst
2015-08-03 14:36 ` Maarten Lankhorst
2015-08-04 15:37 ` Patrik Jakobsson
2015-08-05 10:45 ` drm/i915: Postpone plane readout until after encoder readout, v2 Maarten Lankhorst
2015-08-26 14:43 ` Daniel Vetter
2015-08-27 11:05 ` Maarten Lankhorst
2015-09-01 9:52 ` Daniel Vetter
2015-08-27 11:47 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout, v3 Maarten Lankhorst
2015-08-27 12:05 ` Ville Syrjälä
2015-08-31 5:59 ` shuang.he
2015-10-13 12:40 ` Jani Nikula
2015-10-13 12:44 ` Maarten Lankhorst
2015-08-10 17:56 ` [PATCH] drm/i915: Postpone plane readout until after encoder readout shuang.he
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.