* [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: 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
* 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
* [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: [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
* 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
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.