| Message ID | 5564307E.60205@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 26, 2015 at 10:36:14AM +0200, Maarten Lankhorst wrote: > This needs to be a global check because at the time of crtc checking > not all modesets have to be calculated yet. Use intel_crtc->atomic > because there's no reason to keep it in state. The note about intel_crtc->atomic is out of date as of v3. Otherwise, I think this looks okay. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Matt > > Changes since v1: > - Use intel_crtc->atomic as a place to put hsw_workaround_pipe. > - Make sure quirk only applies to haswell. > - Use first loop to iterate over newly enabled crtc's only. > This increases readability. > Changes since v2: > - Move hsw_workaround_pipe back to crtc_state. > Changes since v3: > - Return errors from haswell_mode_set_planes_workaround. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++---------- > drivers/gpu/drm/i915/intel_drv.h | 3 + > 2 files changed, 80 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index de13c1c14c93..f6733a777590 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4867,42 +4867,15 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) > return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A; > } > > -/* > - * This implements the workaround described in the "notes" section of the mode > - * set sequence documentation. When going from no pipes or single pipe to > - * multiple pipes, and planes are enabled after the pipe, we need to wait at > - * least 2 vblanks on the first pipe before enabling planes on the second pipe. > - */ > -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc) > -{ > - struct drm_device *dev = crtc->base.dev; > - struct intel_crtc *crtc_it, *other_active_crtc = NULL; > - > - /* We want to get the other_active_crtc only if there's only 1 other > - * active crtc. */ > - for_each_intel_crtc(dev, crtc_it) { > - if (!crtc_it->active || crtc_it == crtc) > - continue; > - > - if (other_active_crtc) > - return; > - > - other_active_crtc = crtc_it; > - } > - if (!other_active_crtc) > - return; > - > - intel_wait_for_vblank(dev, other_active_crtc->pipe); > - intel_wait_for_vblank(dev, other_active_crtc->pipe); > -} > - > static void haswell_crtc_enable(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_encoder *encoder; > - int pipe = intel_crtc->pipe; > + int pipe = intel_crtc->pipe, hsw_workaround_pipe; > + struct intel_crtc_state *pipe_config = > + to_intel_crtc_state(crtc->state); > > if (WARN_ON(intel_crtc->active)) > return; > @@ -4979,7 +4952,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) > > /* If we change the relative order between pipe/planes enabling, we need > * to change the workaround. */ > - haswell_mode_set_planes_workaround(intel_crtc); > + hsw_workaround_pipe = pipe_config->hsw_workaround_pipe; > + if (IS_HASWELL(dev) && hsw_workaround_pipe != INVALID_PIPE) { > + intel_wait_for_vblank(dev, hsw_workaround_pipe); > + intel_wait_for_vblank(dev, hsw_workaround_pipe); > + } > } > > static void ironlake_pfit_disable(struct intel_crtc *crtc) > @@ -12169,6 +12146,71 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) > return ret; > } > > +/* > + * This implements the workaround described in the "notes" section of the mode > + * set sequence documentation. When going from no pipes or single pipe to > + * multiple pipes, and planes are enabled after the pipe, we need to wait at > + * least 2 vblanks on the first pipe before enabling planes on the second pipe. > + */ > +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) > +{ > + struct drm_crtc_state *crtc_state; > + struct intel_crtc *intel_crtc; > + struct drm_crtc *crtc; > + struct intel_crtc_state *first_crtc_state = NULL; > + struct intel_crtc_state *other_crtc_state = NULL; > + enum pipe first_pipe = INVALID_PIPE, enabled_pipe = INVALID_PIPE; > + int i; > + > + /* look at all crtc's that are going to be enabled in during modeset */ > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + intel_crtc = to_intel_crtc(crtc); > + > + if (!crtc_state->active || !needs_modeset(crtc_state)) > + continue; > + > + if (first_crtc_state) { > + other_crtc_state = to_intel_crtc_state(crtc_state); > + break; > + } else { > + first_crtc_state = to_intel_crtc_state(crtc_state); > + first_pipe = intel_crtc->pipe; > + } > + } > + > + /* No workaround needed? */ > + if (!first_crtc_state) > + return 0; > + > + /* w/a possibly needed, check how many crtc's are already enabled. */ > + for_each_intel_crtc(state->dev, intel_crtc) { > + struct intel_crtc_state *pipe_config; > + > + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc); > + if (IS_ERR(pipe_config)) > + return PTR_ERR(pipe_config); > + > + pipe_config->hsw_workaround_pipe = INVALID_PIPE; > + > + if (!pipe_config->base.active || > + needs_modeset(&pipe_config->base)) > + continue; > + > + /* 2 or more enabled crtcs means no need for w/a */ > + if (enabled_pipe != INVALID_PIPE) > + return 0; > + > + enabled_pipe = intel_crtc->pipe; > + } > + > + if (enabled_pipe != INVALID_PIPE) > + first_crtc_state->hsw_workaround_pipe = enabled_pipe; > + else if (other_crtc_state) > + other_crtc_state->hsw_workaround_pipe = first_pipe; > + > + return 0; > +} > + > /* Code that should eventually be part of atomic_check() */ > static int __intel_set_mode_checks(struct drm_atomic_state *state) > { > @@ -12192,7 +12234,10 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state) > if (ret) > return ret; > > - return 0; > + if (IS_HASWELL(dev)) > + ret = haswell_mode_set_planes_workaround(state); > + > + return ret; > } > > static int > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 665e249ae8bf..d5f38e3d732b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -443,6 +443,9 @@ struct intel_crtc_state { > int pbn; > > struct intel_crtc_scaler_state scaler_state; > + > + /* w/a for waiting 2 vblanks during crtc enable */ > + enum pipe hsw_workaround_pipe; > }; > > struct intel_pipe_wm { > -- > 2.1.0 > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index de13c1c14c93..f6733a777590 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4867,42 +4867,15 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A; } -/* - * This implements the workaround described in the "notes" section of the mode - * set sequence documentation. When going from no pipes or single pipe to - * multiple pipes, and planes are enabled after the pipe, we need to wait at - * least 2 vblanks on the first pipe before enabling planes on the second pipe. - */ -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc) -{ - struct drm_device *dev = crtc->base.dev; - struct intel_crtc *crtc_it, *other_active_crtc = NULL; - - /* We want to get the other_active_crtc only if there's only 1 other - * active crtc. */ - for_each_intel_crtc(dev, crtc_it) { - if (!crtc_it->active || crtc_it == crtc) - continue; - - if (other_active_crtc) - return; - - other_active_crtc = crtc_it; - } - if (!other_active_crtc) - return; - - intel_wait_for_vblank(dev, other_active_crtc->pipe); - intel_wait_for_vblank(dev, other_active_crtc->pipe); -} - static void haswell_crtc_enable(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_encoder *encoder; - int pipe = intel_crtc->pipe; + int pipe = intel_crtc->pipe, hsw_workaround_pipe; + struct intel_crtc_state *pipe_config = + to_intel_crtc_state(crtc->state); if (WARN_ON(intel_crtc->active)) return; @@ -4979,7 +4952,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) /* If we change the relative order between pipe/planes enabling, we need * to change the workaround. */ - haswell_mode_set_planes_workaround(intel_crtc); + hsw_workaround_pipe = pipe_config->hsw_workaround_pipe; + if (IS_HASWELL(dev) && hsw_workaround_pipe != INVALID_PIPE) { + intel_wait_for_vblank(dev, hsw_workaround_pipe); + intel_wait_for_vblank(dev, hsw_workaround_pipe); + } } static void ironlake_pfit_disable(struct intel_crtc *crtc) @@ -12169,6 +12146,71 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) return ret; } +/* + * This implements the workaround described in the "notes" section of the mode + * set sequence documentation. When going from no pipes or single pipe to + * multiple pipes, and planes are enabled after the pipe, we need to wait at + * least 2 vblanks on the first pipe before enabling planes on the second pipe. + */ +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct intel_crtc *intel_crtc; + struct drm_crtc *crtc; + struct intel_crtc_state *first_crtc_state = NULL; + struct intel_crtc_state *other_crtc_state = NULL; + enum pipe first_pipe = INVALID_PIPE, enabled_pipe = INVALID_PIPE; + int i; + + /* look at all crtc's that are going to be enabled in during modeset */ + for_each_crtc_in_state(state, crtc, crtc_state, i) { + intel_crtc = to_intel_crtc(crtc); + + if (!crtc_state->active || !needs_modeset(crtc_state)) + continue; + + if (first_crtc_state) { + other_crtc_state = to_intel_crtc_state(crtc_state); + break; + } else { + first_crtc_state = to_intel_crtc_state(crtc_state); + first_pipe = intel_crtc->pipe; + } + } + + /* No workaround needed? */ + if (!first_crtc_state) + return 0; + + /* w/a possibly needed, check how many crtc's are already enabled. */ + for_each_intel_crtc(state->dev, intel_crtc) { + struct intel_crtc_state *pipe_config; + + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc); + if (IS_ERR(pipe_config)) + return PTR_ERR(pipe_config); + + pipe_config->hsw_workaround_pipe = INVALID_PIPE; + + if (!pipe_config->base.active || + needs_modeset(&pipe_config->base)) + continue; + + /* 2 or more enabled crtcs means no need for w/a */ + if (enabled_pipe != INVALID_PIPE) + return 0; + + enabled_pipe = intel_crtc->pipe; + } + + if (enabled_pipe != INVALID_PIPE) + first_crtc_state->hsw_workaround_pipe = enabled_pipe; + else if (other_crtc_state) + other_crtc_state->hsw_workaround_pipe = first_pipe; + + return 0; +} + /* Code that should eventually be part of atomic_check() */ static int __intel_set_mode_checks(struct drm_atomic_state *state) { @@ -12192,7 +12234,10 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state) if (ret) return ret; - return 0; + if (IS_HASWELL(dev)) + ret = haswell_mode_set_planes_workaround(state); + + return ret; } static int diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 665e249ae8bf..d5f38e3d732b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -443,6 +443,9 @@ struct intel_crtc_state { int pbn; struct intel_crtc_scaler_state scaler_state; + + /* w/a for waiting 2 vblanks during crtc enable */ + enum pipe hsw_workaround_pipe; }; struct intel_pipe_wm {
This needs to be a global check because at the time of crtc checking not all modesets have to be calculated yet. Use intel_crtc->atomic because there's no reason to keep it in state. Changes since v1: - Use intel_crtc->atomic as a place to put hsw_workaround_pipe. - Make sure quirk only applies to haswell. - Use first loop to iterate over newly enabled crtc's only. This increases readability. Changes since v2: - Move hsw_workaround_pipe back to crtc_state. Changes since v3: - Return errors from haswell_mode_set_planes_workaround. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++---------- drivers/gpu/drm/i915/intel_drv.h | 3 + 2 files changed, 80 insertions(+), 32 deletions(-)