On Mon, Jul 27, 2015 at 02:35:30PM +0200, Maarten Lankhorst wrote:
> Set active_changed to force a modeset if the panel fitter's force
> enabled.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Hm, shouldn't our fancy fastset logic be able to detect that we've changed
pfit change here and force a full modeset? Or am I blind again?
Abusing active_changed for this feels a bit tricksy tbh, can't we use
mode_changed for this? mode_changed is kinda for "crtc configuration that
needs a full modeset changed", not just for modes. active_changed is
"enable/disable it, but strictly speaking no need to recompute stuff".
At least that's how the atomic helpers treat it.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 82 +++++++++++-------------------------
> drivers/gpu/drm/i915/intel_display.c | 3 ++
> 2 files changed, 27 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 23a69307e12e..22467adee958 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3645,74 +3645,40 @@ static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
> return 0;
> }
>
> -static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
> +static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev, bool enable)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *crtc =
> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]);
> struct intel_crtc_state *pipe_config;
> + struct drm_atomic_state *state;
> + int ret = 0;
>
> drm_modeset_lock_all(dev);
> - pipe_config = to_intel_crtc_state(crtc->base.state);
> -
> - /*
> - * If we use the eDP transcoder we need to make sure that we don't
> - * bypass the pfit, since otherwise the pipe CRC source won't work. Only
> - * relevant on hsw with pipe A when using the always-on power well
> - * routing.
> - */
> - if (pipe_config->cpu_transcoder == TRANSCODER_EDP &&
> - !pipe_config->pch_pfit.enabled) {
> - bool active = pipe_config->base.active;
> -
> - if (active) {
> - intel_crtc_control(&crtc->base, false);
> - pipe_config = to_intel_crtc_state(crtc->base.state);
> - }
> -
> - pipe_config->pch_pfit.force_thru = true;
> -
> - intel_display_power_get(dev_priv,
> - POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
> -
> - if (active)
> - intel_crtc_control(&crtc->base, true);
> + state = drm_atomic_state_alloc(dev);
> + if (!state) {
> + ret = -ENOMEM;
> + goto out;
> }
> - drm_modeset_unlock_all(dev);
> -}
>
> -static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *crtc =
> - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]);
> - struct intel_crtc_state *pipe_config;
> -
> - drm_modeset_lock_all(dev);
> - /*
> - * If we use the eDP transcoder we need to make sure that we don't
> - * bypass the pfit, since otherwise the pipe CRC source won't work. Only
> - * relevant on hsw with pipe A when using the always-on power well
> - * routing.
> - */
> - pipe_config = to_intel_crtc_state(crtc->base.state);
> - if (pipe_config->pch_pfit.force_thru) {
> - bool active = pipe_config->base.active;
> -
> - if (active) {
> - intel_crtc_control(&crtc->base, false);
> - pipe_config = to_intel_crtc_state(crtc->base.state);
> - }
> -
> - pipe_config->pch_pfit.force_thru = false;
> + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(&crtc->base);
> + pipe_config = intel_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(pipe_config)) {
> + ret = PTR_ERR(pipe_config);
> + goto out;
> + }
>
> - intel_display_power_put(dev_priv,
> - POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
> + pipe_config->pch_pfit.force_thru = enable;
> + if (pipe_config->cpu_transcoder == TRANSCODER_EDP &&
> + pipe_config->pch_pfit.enabled != enable)
> + pipe_config->base.active_changed = true;
>
> - if (active)
> - intel_crtc_control(&crtc->base, true);
> - }
> + ret = drm_atomic_commit(state);
> +out:
> drm_modeset_unlock_all(dev);
> + WARN(ret, "Toggling workaround to %i returns %i\n", enable, ret);
> + if (ret)
> + drm_atomic_state_free(state);
> }
>
> static int ivb_pipe_crc_ctl_reg(struct drm_device *dev,
> @@ -3732,7 +3698,7 @@ static int ivb_pipe_crc_ctl_reg(struct drm_device *dev,
> break;
> case INTEL_PIPE_CRC_SOURCE_PF:
> if (IS_HASWELL(dev) && pipe == PIPE_A)
> - hsw_trans_edp_pipe_A_crc_wa(dev);
> + hsw_trans_edp_pipe_A_crc_wa(dev, true);
>
> *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB;
> break;
> @@ -3844,7 +3810,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
> else if (IS_VALLEYVIEW(dev))
> vlv_undo_pipe_scramble_reset(dev, pipe);
> else if (IS_HASWELL(dev) && pipe == PIPE_A)
> - hsw_undo_trans_edp_pipe_A_crc_wa(dev);
> + hsw_trans_edp_pipe_A_crc_wa(dev, false);
>
> hsw_enable_ips(crtc);
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 43b0f17ad1fa..7747520bf9f6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12160,6 +12160,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> struct intel_dpll_hw_state dpll_hw_state;
> enum intel_dpll_id shared_dpll;
> uint32_t ddi_pll_sel;
> + bool force_thru;
>
> /* FIXME: before the switch to atomic started, a new pipe_config was
> * kzalloc'd. Code that depends on any field being zero should be
> @@ -12171,6 +12172,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> shared_dpll = crtc_state->shared_dpll;
> dpll_hw_state = crtc_state->dpll_hw_state;
> ddi_pll_sel = crtc_state->ddi_pll_sel;
> + force_thru = crtc_state->pch_pfit.force_thru;
>
> memset(crtc_state, 0, sizeof *crtc_state);
>
> @@ -12179,6 +12181,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> crtc_state->shared_dpll = shared_dpll;
> crtc_state->dpll_hw_state = dpll_hw_state;
> crtc_state->ddi_pll_sel = ddi_pll_sel;
> + crtc_state->pch_pfit.force_thru = force_thru;
> }
>
> static int
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
