On Wed, Dec 11, 2024 at 02:54:31PM +0200, Jani Nikula wrote:
> It does not look like anything in intel_dp_compute_config() after the
> g4x_dp_set_clock() call depends on the changes it makes, namely setting
> dpll and clock_set in crtc_state.
Hmm, yeah I was a bit worried about DRRS and such, but as long
as don't mangle the clock itself/etc. then this looks fine.
Series is
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Move the call one level higher to
> g4x_dp_compute_config() to reduce the clutter in
> intel_dp_compute_config().
>
> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/g4x_dp.c | 13 ++++++++++---
> drivers/gpu/drm/i915/display/g4x_dp.h | 6 ------
> drivers/gpu/drm/i915/display/intel_dp.c | 3 ---
> 3 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
> index 9ac894a7411f..e06405a3b82d 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> @@ -55,8 +55,8 @@ const struct dpll *vlv_get_dpll(struct drm_i915_private *i915)
> return IS_CHERRYVIEW(i915) ? &chv_dpll[0] : &vlv_dpll[0];
> }
>
> -void g4x_dp_set_clock(struct intel_encoder *encoder,
> - struct intel_crtc_state *pipe_config)
> +static void g4x_dp_set_clock(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> const struct dpll *divisor = NULL;
> @@ -1228,11 +1228,18 @@ static int g4x_dp_compute_config(struct intel_encoder *encoder,
> struct drm_connector_state *conn_state)
> {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + int ret;
>
> if (HAS_PCH_SPLIT(i915) && encoder->port != PORT_A)
> crtc_state->has_pch_encoder = true;
>
> - return intel_dp_compute_config(encoder, crtc_state, conn_state);
> + ret = intel_dp_compute_config(encoder, crtc_state, conn_state);
> + if (ret)
> + return ret;
> +
> + g4x_dp_set_clock(encoder, crtc_state);
> +
> + return 0;
> }
>
> static void g4x_dp_suspend_complete(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.h b/drivers/gpu/drm/i915/display/g4x_dp.h
> index c75e64ae79b7..839a251dc069 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.h
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.h
> @@ -19,8 +19,6 @@ struct intel_encoder;
>
> #ifdef I915
> const struct dpll *vlv_get_dpll(struct drm_i915_private *i915);
> -void g4x_dp_set_clock(struct intel_encoder *encoder,
> - struct intel_crtc_state *pipe_config);
> bool g4x_dp_port_enabled(struct drm_i915_private *dev_priv,
> i915_reg_t dp_reg, enum port port,
> enum pipe *pipe);
> @@ -31,10 +29,6 @@ static inline const struct dpll *vlv_get_dpll(struct drm_i915_private *i915)
> {
> return NULL;
> }
> -static inline void g4x_dp_set_clock(struct intel_encoder *encoder,
> - struct intel_crtc_state *pipe_config)
> -{
> -}
> static inline bool g4x_dp_port_enabled(struct drm_i915_private *dev_priv,
> i915_reg_t dp_reg, int port,
> enum pipe *pipe)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index adc51567ec17..f8100c4f4d20 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3172,9 +3172,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> if (pipe_config->splitter.enable)
> pipe_config->dp_m_n.data_m *= pipe_config->splitter.link_count;
>
> - if (!HAS_DDI(dev_priv))
> - g4x_dp_set_clock(encoder, pipe_config);
> -
> intel_vrr_compute_config(pipe_config, conn_state);
> intel_dp_compute_as_sdp(intel_dp, pipe_config);
> intel_psr_compute_config(intel_dp, pipe_config, conn_state);
> --
> 2.39.5
--
Ville Syrjälä
Intel
