> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, February 24, 2021 4:42 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: [PATCH 5/6] drm/i915: Add encoder->is_clock_enabled()
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Support reading out the current state of the DDI clock.
>
> Not sure we really want this. Seems a bit excessive just to restore the debug
> print to icl_sanitize_encoder_pll_mapping()?
> But maybe there's more use for it?
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
I guess there is no harm done if we have the state of the DDI clock in store.
Reviewed-by: Mika Kahola <mika.kahola@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 19 +++
> drivers/gpu/drm/i915/display/intel_crt.c | 1 +
> drivers/gpu/drm/i915/display/intel_ddi.c | 123 +++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_ddi.h | 1 +
> .../drm/i915/display/intel_display_types.h | 4 +
> 5 files changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 29fe4919392a..7f2abc088a66 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -655,6 +655,24 @@ static void gen11_dsi_ungate_clocks(struct
> intel_encoder *encoder)
> mutex_unlock(&dev_priv->dpll.lock);
> }
>
> +static bool gen11_dsi_is_clock_enabled(struct intel_encoder *encoder) {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> + bool clock_enabled = false;
> + enum phy phy;
> + u32 tmp;
> +
> + tmp = intel_de_read(dev_priv, ICL_DPCLKA_CFGCR0);
> +
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + if (!(tmp & ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy)))
> + clock_enabled = true;
> + }
> +
> + return clock_enabled;
> +}
> +
> static void gen11_dsi_map_pll(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state) { @@ -
> 1939,6 +1957,7 @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
> encoder->power_domain = POWER_DOMAIN_PORT_DSI;
> encoder->get_power_domains = gen11_dsi_get_power_domains;
> encoder->disable_clock = gen11_dsi_gate_clocks;
> + encoder->is_clock_enabled = gen11_dsi_is_clock_enabled;
>
> /* register DSI connector with DRM subsystem */
> drm_connector_init(dev, connector, &gen11_dsi_connector_funcs,
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c
> b/drivers/gpu/drm/i915/display/intel_crt.c
> index b03f74076f64..7f3d11c5ce3e 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -1078,6 +1078,7 @@ void intel_crt_init(struct drm_i915_private
> *dev_priv)
> crt->base.post_disable = hsw_post_disable_crt;
> crt->base.enable_clock = hsw_ddi_enable_clock;
> crt->base.disable_clock = hsw_ddi_disable_clock;
> + crt->base.is_clock_enabled = hsw_ddi_is_clock_enabled;
> } else {
> if (HAS_PCH_SPLIT(dev_priv)) {
> crt->base.compute_config =
> pch_crt_compute_config; diff --git
> a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 56f5f55a7c8f..7d477c4007c7 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1589,6 +1589,12 @@ static void _cnl_ddi_disable_clock(struct
> drm_i915_private *i915, i915_reg_t reg
> mutex_unlock(&i915->dpll.lock);
> }
>
> +static bool _cnl_ddi_is_clock_enabled(struct drm_i915_private *i915,
> i915_reg_t reg,
> + u32 clk_off)
> +{
> + return !(intel_de_read(i915, reg) & clk_off); }
> +
> static struct intel_shared_dpll *
> _cnl_ddi_get_pll(struct drm_i915_private *i915, i915_reg_t reg,
> u32 clk_sel_mask, u32 clk_sel_shift) @@ -1625,6 +1631,15
> @@ static void adls_ddi_disable_clock(struct intel_encoder *encoder)
> ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> }
>
> +static bool adls_ddi_is_clock_enabled(struct intel_encoder *encoder) {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum phy phy = intel_port_to_phy(i915, encoder->port);
> +
> + return _cnl_ddi_is_clock_enabled(i915, ADLS_DPCLKA_CFGCR(phy),
> +
> ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> +}
> +
> static struct intel_shared_dpll *adls_ddi_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 1660,6 +1675,15 @@ static void rkl_ddi_disable_clock(struct intel_encoder
> *encoder)
> RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> }
>
> +static bool rkl_ddi_is_clock_enabled(struct intel_encoder *encoder) {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum phy phy = intel_port_to_phy(i915, encoder->port);
> +
> + return _cnl_ddi_is_clock_enabled(i915, ICL_DPCLKA_CFGCR0,
> +
> RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> +}
> +
> static struct intel_shared_dpll *rkl_ddi_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 1704,6 +1728,15 @@ static void dg1_ddi_disable_clock(struct intel_encoder
> *encoder)
> DG1_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> }
>
> +static bool dg1_ddi_is_clock_enabled(struct intel_encoder *encoder) {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum phy phy = intel_port_to_phy(i915, encoder->port);
> +
> + return _cnl_ddi_is_clock_enabled(i915, DG1_DPCLKA_CFGCR0(phy),
> +
> DG1_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> +}
> +
> static struct intel_shared_dpll *dg1_ddi_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 1739,6 +1772,15 @@ static void icl_ddi_combo_disable_clock(struct
> intel_encoder *encoder)
> ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> }
>
> +static bool icl_ddi_combo_is_clock_enabled(struct intel_encoder
> +*encoder) {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum phy phy = intel_port_to_phy(i915, encoder->port);
> +
> + return _cnl_ddi_is_clock_enabled(i915, ICL_DPCLKA_CFGCR0,
> +
> ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> +}
> +
> struct intel_shared_dpll *icl_ddi_combo_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 1778,6 +1820,20 @@ static void jsl_ddi_tc_disable_clock(struct intel_encoder
> *encoder)
> intel_de_write(i915, DDI_CLK_SEL(port), DDI_CLK_SEL_NONE); }
>
> +static bool jsl_ddi_tc_is_clock_enabled(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum port port = encoder->port;
> + u32 tmp;
> +
> + tmp = intel_de_read(i915, DDI_CLK_SEL(port));
> +
> + if ((tmp & DDI_CLK_SEL_MASK) == DDI_CLK_SEL_NONE)
> + return false;
> +
> + return icl_ddi_combo_is_clock_enabled(encoder);
> +}
> +
> static void icl_ddi_tc_enable_clock(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state) {
> @@ -1816,6 +1872,23 @@ static void icl_ddi_tc_disable_clock(struct
> intel_encoder *encoder)
> intel_de_write(i915, DDI_CLK_SEL(port), DDI_CLK_SEL_NONE); }
>
> +static bool icl_ddi_tc_is_clock_enabled(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum tc_port tc_port = intel_port_to_tc(i915, encoder->port);
> + enum port port = encoder->port;
> + u32 tmp;
> +
> + tmp = intel_de_read(i915, DDI_CLK_SEL(port));
> +
> + if ((tmp & DDI_CLK_SEL_MASK) == DDI_CLK_SEL_NONE)
> + return false;
> +
> + tmp = intel_de_read(i915, ICL_DPCLKA_CFGCR0);
> +
> + return !(tmp & ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port));
> +}
> +
> static struct intel_shared_dpll *icl_ddi_tc_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 1871,6 +1944,15 @@ static void cnl_ddi_disable_clock(struct intel_encoder
> *encoder)
> DPCLKA_CFGCR0_DDI_CLK_OFF(port)); }
>
> +static bool cnl_ddi_is_clock_enabled(struct intel_encoder *encoder) {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum port port = encoder->port;
> +
> + return _cnl_ddi_is_clock_enabled(i915, DPCLKA_CFGCR0,
> +
> DPCLKA_CFGCR0_DDI_CLK_OFF(port)); }
> +
> static struct intel_shared_dpll *cnl_ddi_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 1938,6 +2020,18 @@ static void skl_ddi_disable_clock(struct intel_encoder
> *encoder)
> mutex_unlock(&i915->dpll.lock);
> }
>
> +static bool skl_ddi_is_clock_enabled(struct intel_encoder *encoder) {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum port port = encoder->port;
> +
> + /*
> + * FIXME Not sure if the override affects both
> + * the PLL selection and the CLK_OFF bit.
> + */
> + return !(intel_de_read(i915, DPLL_CTRL2) &
> +DPLL_CTRL2_DDI_CLK_OFF(port)); }
> +
> static struct intel_shared_dpll *skl_ddi_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 1981,6 +2075,14 @@ void hsw_ddi_disable_clock(struct intel_encoder
> *encoder)
> intel_de_write(i915, PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); }
>
> +bool hsw_ddi_is_clock_enabled(struct intel_encoder *encoder) {
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + enum port port = encoder->port;
> +
> + return intel_de_read(i915, PORT_CLK_SEL(port)) !=
> PORT_CLK_SEL_NONE; }
> +
> static struct intel_shared_dpll *hsw_ddi_get_pll(struct intel_encoder
> *encoder) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev); @@ -
> 2084,8 +2186,15 @@ void icl_sanitize_encoder_pll_mapping(struct
> intel_encoder *encoder)
> ddi_clk_needed = false;
> }
>
> - if (!ddi_clk_needed && encoder->disable_clock)
> - encoder->disable_clock(encoder);
> + if (ddi_clk_needed || !encoder->disable_clock ||
> + !encoder->is_clock_enabled(encoder))
> + return;
> +
> + drm_notice(&i915->drm,
> + "[ENCODER:%d:%s] is disabled/in DSI mode with an
> ungated DDI clock, gate it\n",
> + encoder->base.base.id, encoder->base.name);
> +
> + encoder->disable_clock(encoder);
> }
>
> static void
> @@ -4335,38 +4444,46 @@ void intel_ddi_init(struct drm_i915_private
> *dev_priv, enum port port)
> if (IS_ALDERLAKE_S(dev_priv)) {
> encoder->enable_clock = adls_ddi_enable_clock;
> encoder->disable_clock = adls_ddi_disable_clock;
> + encoder->is_clock_enabled = adls_ddi_is_clock_enabled;
> encoder->get_config = adls_ddi_get_config;
> } else if (IS_ROCKETLAKE(dev_priv)) {
> encoder->enable_clock = rkl_ddi_enable_clock;
> encoder->disable_clock = rkl_ddi_disable_clock;
> + encoder->is_clock_enabled = rkl_ddi_is_clock_enabled;
> encoder->get_config = rkl_ddi_get_config;
> } else if (IS_DG1(dev_priv)) {
> encoder->enable_clock = dg1_ddi_enable_clock;
> encoder->disable_clock = dg1_ddi_disable_clock;
> + encoder->is_clock_enabled = dg1_ddi_is_clock_enabled;
> encoder->get_config = dg1_ddi_get_config;
> } else if (IS_JSL_EHL(dev_priv)) {
> if (intel_ddi_is_tc(dev_priv, port)) {
> encoder->enable_clock = jsl_ddi_tc_enable_clock;
> encoder->disable_clock = jsl_ddi_tc_disable_clock;
> + encoder->is_clock_enabled =
> jsl_ddi_tc_is_clock_enabled;
> encoder->get_config = icl_ddi_combo_get_config;
> } else {
> encoder->enable_clock =
> icl_ddi_combo_enable_clock;
> encoder->disable_clock =
> icl_ddi_combo_disable_clock;
> + encoder->is_clock_enabled =
> icl_ddi_combo_is_clock_enabled;
> encoder->get_config = icl_ddi_combo_get_config;
> }
> } else if (INTEL_GEN(dev_priv) >= 11) {
> if (intel_ddi_is_tc(dev_priv, port)) {
> encoder->enable_clock = icl_ddi_tc_enable_clock;
> encoder->disable_clock = icl_ddi_tc_disable_clock;
> + encoder->is_clock_enabled =
> icl_ddi_tc_is_clock_enabled;
> encoder->get_config = icl_ddi_tc_get_config;
> } else {
> encoder->enable_clock =
> icl_ddi_combo_enable_clock;
> encoder->disable_clock =
> icl_ddi_combo_disable_clock;
> + encoder->is_clock_enabled =
> icl_ddi_combo_is_clock_enabled;
> encoder->get_config = icl_ddi_combo_get_config;
> }
> } else if (IS_CANNONLAKE(dev_priv)) {
> encoder->enable_clock = cnl_ddi_enable_clock;
> encoder->disable_clock = cnl_ddi_disable_clock;
> + encoder->is_clock_enabled = cnl_ddi_is_clock_enabled;
> encoder->get_config = cnl_ddi_get_config;
> } else if (IS_GEN9_LP(dev_priv)) {
> /* BXT/GLK have fixed PLL->port mapping */ @@ -4374,10
> +4491,12 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum
> port port)
> } else if (IS_GEN9_BC(dev_priv)) {
> encoder->enable_clock = skl_ddi_enable_clock;
> encoder->disable_clock = skl_ddi_disable_clock;
> + encoder->is_clock_enabled = skl_ddi_is_clock_enabled;
> encoder->get_config = skl_ddi_get_config;
> } else if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) {
> encoder->enable_clock = hsw_ddi_enable_clock;
> encoder->disable_clock = hsw_ddi_disable_clock;
> + encoder->is_clock_enabled = hsw_ddi_is_clock_enabled;
> encoder->get_config = hsw_ddi_get_config;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h
> b/drivers/gpu/drm/i915/display/intel_ddi.h
> index 0780c47efe0f..99cebbe6b586 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> @@ -36,6 +36,7 @@ void intel_ddi_get_clock(struct intel_encoder *encoder,
> void hsw_ddi_enable_clock(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state); void
> hsw_ddi_disable_clock(struct intel_encoder *encoder);
> +bool hsw_ddi_is_clock_enabled(struct intel_encoder *encoder);
> void hsw_ddi_get_config(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state); struct
> intel_shared_dpll *icl_ddi_combo_get_pll(struct intel_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 1a76e1d9de7a..5b2e81db0a20 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -226,6 +226,10 @@ struct intel_encoder {
> void (*enable_clock)(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state);
> void (*disable_clock)(struct intel_encoder *encoder);
> + /*
> + * Returns whether the port clock is enabled or not.
> + */
> + bool (*is_clock_enabled)(struct intel_encoder *encoder);
> enum hpd_pin hpd_pin;
> enum intel_display_power_domain power_domain;
> /* for communication with audio component; protected by av_mutex
> */
> --
> 2.26.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
