| Message ID | 20241211125431.680227-2-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/i915/dp: add g4x_dp_compute_config() | expand |
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@linux.intel.com> > 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@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > 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
On Wed, 11 Dec 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > 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@linux.intel.com> Thanks, pushed to din. BR, Jani. > >> 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@linux.intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> 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
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);
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. 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@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- 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(-)