On Fri, 2019-12-13 at 21:52 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Move all of haswell_crtc_disable() into the encoder
> .post_disable() hooks. Now we're left with just
> calling the .disable() and .post_disable() hooks
> back to back.
>
> I chose to move the code into the .post_disable() hook instead
> of the .enable() hook as most of the sequence is currently
s/.enable()/.disable()
> implemented in the .post_enable() hook.
>
s/post_enable()/.post_disable()
> We should collapse it all down to just one hook and then the
> encoders can drive the modeset sequence fully. But that may
> need some further refactoring as we currently call the
> ddi .post_disable() hook from mst code and we can't just
> replace that with a call to the ddi .disable() hook.
>
> Should also follow up with similar treatment for the enable
> sequence but let's start here where it's easier.
Pretty good change, removed the feature checks from
haswell_crtc_disable() and moved to the right place.
With this changes I can do what you asked and move the code in
intel_dp_mst_post_trans_disabled() to intel_mst_post_disable_dp().
With the typos above fixed:
Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
Thanks
>
> Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 12 +++++
> drivers/gpu/drm/i915/display/intel_crt.c | 8 +++
> drivers/gpu/drm/i915/display/intel_ddi.c | 35 ++++++++++++
> drivers/gpu/drm/i915/display/intel_display.c | 57 +++---------------
> --
> drivers/gpu/drm/i915/display/intel_display.h | 4 ++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 11 ++++
> drivers/gpu/drm/i915/display/vlv_dsi.c | 10 +++-
> 7 files changed, 86 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 03aa92d317a2..006b1a297e6f 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1251,6 +1251,17 @@ static void gen11_dsi_disable(struct
> intel_encoder *encoder,
> gen11_dsi_disable_io_power(encoder);
> }
>
> +static void gen11_dsi_post_disable(struct intel_encoder *encoder,
> + const struct intel_crtc_state
> *old_crtc_state,
> + const struct drm_connector_state
> *old_conn_state)
> +{
> + intel_crtc_vblank_off(old_crtc_state);
> +
> + intel_dsc_disable(old_crtc_state);
> +
> + skylake_scaler_disable(old_crtc_state);
> +}
> +
> static enum drm_mode_status gen11_dsi_mode_valid(struct
> drm_connector *connector,
> struct
> drm_display_mode *mode)
> {
> @@ -1697,6 +1708,7 @@ void icl_dsi_init(struct drm_i915_private
> *dev_priv)
> encoder->pre_pll_enable = gen11_dsi_pre_pll_enable;
> encoder->pre_enable = gen11_dsi_pre_enable;
> encoder->disable = gen11_dsi_disable;
> + encoder->post_disable = gen11_dsi_post_disable;
> encoder->port = port;
> encoder->get_config = gen11_dsi_get_config;
> encoder->update_pipe = intel_panel_update_backlight;
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c
> b/drivers/gpu/drm/i915/display/intel_crt.c
> index 50624b8f064d..b2b1336ecdb6 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -241,6 +241,14 @@ static void hsw_post_disable_crt(struct
> intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>
> + intel_crtc_vblank_off(old_crtc_state);
> +
> + intel_disable_pipe(old_crtc_state);
> +
> + intel_ddi_disable_transcoder_func(old_crtc_state);
> +
> + ironlake_pfit_disable(old_crtc_state);
> +
> intel_ddi_disable_pipe_clock(old_crtc_state);
>
> pch_post_disable_crt(encoder, old_crtc_state, old_conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index cac0be47e500..fa40ba7cbcad 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3851,6 +3851,25 @@ static void intel_ddi_post_disable_hdmi(struct
> intel_encoder *encoder,
> intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
> }
>
> +static void icl_disable_transcoder_port_sync(const struct
> intel_crtc_state *old_crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> >uapi.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + i915_reg_t reg;
> + u32 trans_ddi_func_ctl2_val;
> +
> + if (old_crtc_state->master_transcoder == INVALID_TRANSCODER)
> + return;
> +
> + DRM_DEBUG_KMS("Disabling Transcoder Port Sync on Slave
> Transcoder %s\n",
> + transcoder_name(old_crtc_state->cpu_transcoder));
> +
> + reg = TRANS_DDI_FUNC_CTL2(old_crtc_state->cpu_transcoder);
> + trans_ddi_func_ctl2_val = ~(PORT_SYNC_MODE_ENABLE |
> + PORT_SYNC_MODE_MASTER_SELECT_MASK);
> + I915_WRITE(reg, trans_ddi_func_ctl2_val);
> +}
> +
> static void intel_ddi_post_disable(struct intel_encoder *encoder,
> const struct intel_crtc_state
> *old_crtc_state,
> const struct drm_connector_state
> *old_conn_state)
> @@ -3860,6 +3879,22 @@ static void intel_ddi_post_disable(struct
> intel_encoder *encoder,
> enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
>
> + intel_crtc_vblank_off(old_crtc_state);
> +
> + intel_disable_pipe(old_crtc_state);
> +
> + if (INTEL_GEN(dev_priv) >= 11)
> + icl_disable_transcoder_port_sync(old_crtc_state);
> +
> + intel_ddi_disable_transcoder_func(old_crtc_state);
> +
> + intel_dsc_disable(old_crtc_state);
> +
> + if (INTEL_GEN(dev_priv) >= 9)
> + skylake_scaler_disable(old_crtc_state);
> + else
> + ironlake_pfit_disable(old_crtc_state);
> +
> /*
> * When called from DP MST code:
> * - old_conn_state will be NULL
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index df69e4cd4707..05502f1df3ad 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -167,7 +167,6 @@ static void chv_prepare_pll(struct intel_crtc
> *crtc,
> static void intel_crtc_init_scalers(struct intel_crtc *crtc,
> struct intel_crtc_state
> *crtc_state);
> static void skylake_pfit_enable(const struct intel_crtc_state
> *crtc_state);
> -static void ironlake_pfit_disable(const struct intel_crtc_state
> *old_crtc_state);
> static void ironlake_pfit_enable(const struct intel_crtc_state
> *crtc_state);
> static void intel_modeset_setup_hw_state(struct drm_device *dev,
> struct drm_modeset_acquire_ctx
> *ctx);
> @@ -1825,7 +1824,7 @@ static void intel_crtc_vblank_on(const struct
> intel_crtc_state *crtc_state)
> drm_crtc_vblank_on(&crtc->base);
> }
>
> -static void intel_crtc_vblank_off(const struct intel_crtc_state
> *crtc_state)
> +void intel_crtc_vblank_off(const struct intel_crtc_state
> *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>
> @@ -1891,7 +1890,7 @@ static void intel_enable_pipe(const struct
> intel_crtc_state *new_crtc_state)
> intel_wait_for_pipe_scanline_moving(crtc);
> }
>
> -static void intel_disable_pipe(const struct intel_crtc_state
> *old_crtc_state)
> +void intel_disable_pipe(const struct intel_crtc_state
> *old_crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> >uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -4582,25 +4581,6 @@ static void icl_enable_trans_port_sync(const
> struct intel_crtc_state *crtc_state
> trans_ddi_func_ctl2_val);
> }
>
> -static void icl_disable_transcoder_port_sync(const struct
> intel_crtc_state *old_crtc_state)
> -{
> - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> >uapi.crtc);
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - i915_reg_t reg;
> - u32 trans_ddi_func_ctl2_val;
> -
> - if (old_crtc_state->master_transcoder == INVALID_TRANSCODER)
> - return;
> -
> - DRM_DEBUG_KMS("Disabling Transcoder Port Sync on Slave
> Transcoder %s\n",
> - transcoder_name(old_crtc_state->cpu_transcoder));
> -
> - reg = TRANS_DDI_FUNC_CTL2(old_crtc_state->cpu_transcoder);
> - trans_ddi_func_ctl2_val = ~(PORT_SYNC_MODE_ENABLE |
> - PORT_SYNC_MODE_MASTER_SELECT_MASK);
> - I915_WRITE(reg, trans_ddi_func_ctl2_val);
> -}
> -
> static void intel_fdi_normal_train(struct intel_crtc *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> @@ -5771,7 +5751,7 @@ static int skl_update_scaler_plane(struct
> intel_crtc_state *crtc_state,
> return 0;
> }
>
> -static void skylake_scaler_disable(const struct intel_crtc_state
> *old_crtc_state)
> +void skylake_scaler_disable(const struct intel_crtc_state
> *old_crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> >uapi.crtc);
> int i;
> @@ -6668,7 +6648,7 @@ static void haswell_crtc_enable(struct
> intel_atomic_state *state,
> }
> }
>
> -static void ironlake_pfit_disable(const struct intel_crtc_state
> *old_crtc_state)
> +void ironlake_pfit_disable(const struct intel_crtc_state
> *old_crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> >uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -6743,32 +6723,11 @@ static void ironlake_crtc_disable(struct
> intel_atomic_state *state,
> static void haswell_crtc_disable(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> - const struct intel_crtc_state *old_crtc_state =
> - intel_atomic_get_old_crtc_state(state, crtc);
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - enum transcoder cpu_transcoder = old_crtc_state-
> >cpu_transcoder;
> -
> + /*
> + * FIXME collapse everything to one hook.
> + * Need care with mst->ddi interactions.
> + */
> intel_encoders_disable(state, crtc);
> -
> - intel_crtc_vblank_off(old_crtc_state);
> -
> - /* XXX: Do the pipe assertions at the right place for BXT DSI.
> */
> - if (!transcoder_is_dsi(cpu_transcoder))
> - intel_disable_pipe(old_crtc_state);
> -
> - if (INTEL_GEN(dev_priv) >= 11)
> - icl_disable_transcoder_port_sync(old_crtc_state);
> -
> - if (!transcoder_is_dsi(cpu_transcoder))
> - intel_ddi_disable_transcoder_func(old_crtc_state);
> -
> - intel_dsc_disable(old_crtc_state);
> -
> - if (INTEL_GEN(dev_priv) >= 9)
> - skylake_scaler_disable(old_crtc_state);
> - else
> - ironlake_pfit_disable(old_crtc_state);
> -
> intel_encoders_post_disable(state, crtc);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index 327376810f66..ff496cfbd4ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -485,6 +485,7 @@ enum phy intel_port_to_phy(struct
> drm_i915_private *i915, enum port port);
> bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
>
> void intel_plane_destroy(struct drm_plane *plane);
> +void intel_disable_pipe(const struct intel_crtc_state
> *old_crtc_state);
> void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe
> pipe);
> void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe
> pipe);
> enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> @@ -518,6 +519,7 @@ enum tc_port intel_port_to_tc(struct
> drm_i915_private *dev_priv,
> int intel_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void
> *data,
> struct drm_file *file_priv);
> u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
> +void intel_crtc_vblank_off(const struct intel_crtc_state
> *crtc_state);
>
> int ironlake_get_lanes_required(int target_clock, int link_bw, int
> bpp);
> void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
> @@ -576,6 +578,8 @@ void intel_crtc_arm_fifo_underrun(struct
> intel_crtc *crtc,
>
> u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
> int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
> +void skylake_scaler_disable(const struct intel_crtc_state
> *old_crtc_state);
> +void ironlake_pfit_disable(const struct intel_crtc_state
> *old_crtc_state);
> u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state);
> u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state
> *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 8bdbb15799ee..7aa0975c33b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -241,6 +241,17 @@ static void intel_mst_post_disable_dp(struct
> intel_encoder *encoder,
> intel_dp->active_mst_links--;
> last_mst_stream = intel_dp->active_mst_links == 0;
>
> + intel_crtc_vblank_off(old_crtc_state);
> +
> + intel_disable_pipe(old_crtc_state);
> +
> + intel_ddi_disable_transcoder_func(old_crtc_state);
> +
> + if (INTEL_GEN(dev_priv) >= 9)
> + skylake_scaler_disable(old_crtc_state);
> + else
> + ironlake_pfit_disable(old_crtc_state);
> +
> /*
> * From TGL spec: "If multi-stream slave transcoder: Configure
> * Transcoder Clock Select to direct no clock to the
> transcoder"
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c
> b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 8398a265b6a3..21e820299107 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -882,8 +882,8 @@ static void intel_dsi_clear_device_ready(struct
> intel_encoder *encoder)
> }
>
> static void intel_dsi_post_disable(struct intel_encoder *encoder,
> - const struct intel_crtc_state
> *pipe_config,
> - const struct drm_connector_state
> *conn_state)
> + const struct intel_crtc_state
> *old_crtc_state,
> + const struct drm_connector_state
> *old_conn_state)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> @@ -892,6 +892,12 @@ static void intel_dsi_post_disable(struct
> intel_encoder *encoder,
>
> DRM_DEBUG_KMS("\n");
>
> + if (IS_GEN9_LP(dev_priv)) {
> + intel_crtc_vblank_off(old_crtc_state);
> +
> + skylake_scaler_disable(old_crtc_state);
> + }
> +
> if (is_vid_mode(intel_dsi)) {
> for_each_dsi_port(port, intel_dsi->ports)
> vlv_dsi_wait_for_fifo_empty(intel_dsi, port);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
