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@xxxxxxxxx>
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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
