On Tue, 29 Oct 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Pull the DSI min cdclk calculation into a helper and hide
> it inside vlv_dsi.c in order to keep most DSI related
> details in one place.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++------------------
> drivers/gpu/drm/i915/display/vlv_dsi.c | 25 ++++++++++++++++++++++
> drivers/gpu/drm/i915/display/vlv_dsi.h | 8 +++++++
> 3 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 89d12c521411..e10378744607 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -46,6 +46,7 @@
> #include "intel_vdsc.h"
> #include "skl_watermark.h"
> #include "skl_watermark_regs.h"
> +#include "vlv_dsi.h"
> #include "vlv_sideband.h"
>
> /**
> @@ -2849,8 +2850,6 @@ static int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
>
> int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> {
> - struct intel_display *display = to_intel_display(crtc_state);
> - struct drm_i915_private *dev_priv = to_i915(display->drm);
> int min_cdclk;
>
> if (!crtc_state->hw.enable)
> @@ -2859,25 +2858,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
> min_cdclk = max(hsw_ips_min_cdclk(crtc_state), min_cdclk);
> min_cdclk = max(intel_audio_min_cdclk(crtc_state), min_cdclk);
> -
> - /*
> - * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
> - * than 320000KHz.
> - */
> - if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> - IS_VALLEYVIEW(dev_priv))
> - min_cdclk = max(320000, min_cdclk);
> -
> - /*
> - * On Geminilake once the CDCLK gets as low as 79200
> - * picture gets unstable, despite that values are
> - * correct for DSI PLL and DE PLL.
> - */
> - if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> - IS_GEMINILAKE(dev_priv))
> - min_cdclk = max(158400, min_cdclk);
> -
> - /* Account for additional needs from the planes */
> + min_cdclk = max(vlv_dsi_min_cdclk(crtc_state), min_cdclk);
> min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
>
> if (crtc_state->dsc.compression_enable)
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 9383eedee2d4..49a895589150 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1760,6 +1760,31 @@ static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
> intel_dsi_log_params(intel_dsi);
> }
>
> +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> + int min_cdclk = 0;
> +
> + /*
> + * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
> + * than 320000KHz.
> + */
> + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> + IS_VALLEYVIEW(dev_priv))
> + min_cdclk = max(320000, min_cdclk);
> +
> + /*
> + * On Geminilake once the CDCLK gets as low as 79200
> + * picture gets unstable, despite that values are
> + * correct for DSI PLL and DE PLL.
> + */
> + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> + IS_GEMINILAKE(dev_priv))
> + min_cdclk = max(158400, min_cdclk);
> +
> + return min_cdclk;
> +}
> +
> typedef void (*vlv_dsi_dmi_quirk_func)(struct intel_dsi *intel_dsi);
>
> /*
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.h b/drivers/gpu/drm/i915/display/vlv_dsi.h
> index cf9d7b82f288..5f99059b4c48 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.h
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.h
> @@ -8,13 +8,17 @@
>
> #include <linux/types.h>
>
> +#include <drm/drm_mipi_dsi.h>
> +
Huh, why is this required? At least it's unrelated to the patch.
Other than that,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> enum port;
> struct drm_i915_private;
> +struct intel_crtc_state;
> struct intel_dsi;
>
> #ifdef I915
> void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
> enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
> +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state);
> void vlv_dsi_init(struct drm_i915_private *dev_priv);
> #else
> static inline void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
> @@ -24,6 +28,10 @@ static inline enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt
> {
> return 0;
> }
> +static inline int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
> +{
> + return 0;
> +}
> static inline void vlv_dsi_init(struct drm_i915_private *dev_priv)
> {
> }
--
Jani Nikula, Intel
