Re: [Intel-gfx] [PATCH v2 09/13] drm/i915/icl: Unify disable and enable phy clock gating functions
On Wed, Sep 18, 2019 at 5:07 PM José Roberto de Souza wrote: > > Adding a enable parameters allow us to share most of the code between > enable and disable functions. > > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 71 > 1 file changed, 22 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index fd271118d1f5..78c6974a52d4 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -3145,67 +3145,40 @@ tgl_phy_clock_gating(struct intel_digital_port > *dig_port, bool enable) > } > } > > -static void icl_enable_phy_clock_gating(struct intel_digital_port *dig_port) > +static void > +icl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable) name is confusing: is it a getter or setter? icl_phy_set_clock_gating()? > { > struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); > enum port port = dig_port->base.port; > enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > - u32 val; > + u32 val, regs; s/regs/bits/ With some additional effort we could even migrate this to intel_tc.c. Not for this patch though. With the changes above: Reviewed-by: Lucas De Marchi Lucas De Marchi > int ln; > > if (tc_port == PORT_TC_NONE) > return; > > - for (ln = 0; ln < 2; ln++) { > - val = I915_READ(MG_DP_MODE(ln, port)); > - val |= MG_DP_MODE_CFG_TR2PWR_GATING | > - MG_DP_MODE_CFG_TRPWR_GATING | > - MG_DP_MODE_CFG_CLNPWR_GATING | > - MG_DP_MODE_CFG_DIGPWR_GATING | > - MG_DP_MODE_CFG_GAONPWR_GATING; > - I915_WRITE(MG_DP_MODE(ln, port), val); > - } > - > - val = I915_READ(MG_MISC_SUS0(tc_port)); > - val |= MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3) | > - MG_MISC_SUS0_CFG_TR2PWR_GATING | > - MG_MISC_SUS0_CFG_CL2PWR_GATING | > - MG_MISC_SUS0_CFG_GAONPWR_GATING | > - MG_MISC_SUS0_CFG_TRPWR_GATING | > - MG_MISC_SUS0_CFG_CL1PWR_GATING | > - MG_MISC_SUS0_CFG_DGPWR_GATING; > - I915_WRITE(MG_MISC_SUS0(tc_port), val); > -} > - > -static void icl_disable_phy_clock_gating(struct intel_digital_port *dig_port) > -{ > - struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); > - enum port port = dig_port->base.port; > - enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > - u32 val; > - int ln; > - > - if (tc_port == PORT_TC_NONE) > - return; > + regs = MG_DP_MODE_CFG_TR2PWR_GATING | MG_DP_MODE_CFG_TRPWR_GATING | > + MG_DP_MODE_CFG_CLNPWR_GATING | MG_DP_MODE_CFG_DIGPWR_GATING | > + MG_DP_MODE_CFG_GAONPWR_GATING; > > for (ln = 0; ln < 2; ln++) { > val = I915_READ(MG_DP_MODE(ln, port)); > - val &= ~(MG_DP_MODE_CFG_TR2PWR_GATING | > -MG_DP_MODE_CFG_TRPWR_GATING | > -MG_DP_MODE_CFG_CLNPWR_GATING | > -MG_DP_MODE_CFG_DIGPWR_GATING | > -MG_DP_MODE_CFG_GAONPWR_GATING); > + if (enable) > + val |= regs; > + else > + val &= ~regs; > I915_WRITE(MG_DP_MODE(ln, port), val); > } > > + regs = MG_MISC_SUS0_CFG_TR2PWR_GATING | > MG_MISC_SUS0_CFG_CL2PWR_GATING | > + MG_MISC_SUS0_CFG_GAONPWR_GATING | > MG_MISC_SUS0_CFG_TRPWR_GATING | > + MG_MISC_SUS0_CFG_CL1PWR_GATING | MG_MISC_SUS0_CFG_DGPWR_GATING; > + > val = I915_READ(MG_MISC_SUS0(tc_port)); > - val &= ~(MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK | > -MG_MISC_SUS0_CFG_TR2PWR_GATING | > -MG_MISC_SUS0_CFG_CL2PWR_GATING | > -MG_MISC_SUS0_CFG_GAONPWR_GATING | > -MG_MISC_SUS0_CFG_TRPWR_GATING | > -MG_MISC_SUS0_CFG_CL1PWR_GATING | > -MG_MISC_SUS0_CFG_DGPWR_GATING); > + if (enable) > + val |= (regs | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3)); > + else > + val &= ~(regs | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK); > I915_WRITE(MG_MISC_SUS0(tc_port), val); > } > > @@ -3538,7 +3511,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder > *encoder, > dig_port->ddi_io_power_domain); > > icl_program_mg_dp_mode(dig_port); > - icl_disable_phy_clock_gating(dig_port); > + icl_phy_clock_gating(dig_port, false); > > if (INTEL_GEN(dev_priv) >= 11) > icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > @@ -3571,7 +3544,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder
[Intel-gfx] [PATCH v2 09/13] drm/i915/icl: Unify disable and enable phy clock gating functions
Adding a enable parameters allow us to share most of the code between enable and disable functions. Signed-off-by: José Roberto de Souza --- drivers/gpu/drm/i915/display/intel_ddi.c | 71 1 file changed, 22 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index fd271118d1f5..78c6974a52d4 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3145,67 +3145,40 @@ tgl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable) } } -static void icl_enable_phy_clock_gating(struct intel_digital_port *dig_port) +static void +icl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable) { struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); enum port port = dig_port->base.port; enum tc_port tc_port = intel_port_to_tc(dev_priv, port); - u32 val; + u32 val, regs; int ln; if (tc_port == PORT_TC_NONE) return; - for (ln = 0; ln < 2; ln++) { - val = I915_READ(MG_DP_MODE(ln, port)); - val |= MG_DP_MODE_CFG_TR2PWR_GATING | - MG_DP_MODE_CFG_TRPWR_GATING | - MG_DP_MODE_CFG_CLNPWR_GATING | - MG_DP_MODE_CFG_DIGPWR_GATING | - MG_DP_MODE_CFG_GAONPWR_GATING; - I915_WRITE(MG_DP_MODE(ln, port), val); - } - - val = I915_READ(MG_MISC_SUS0(tc_port)); - val |= MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3) | - MG_MISC_SUS0_CFG_TR2PWR_GATING | - MG_MISC_SUS0_CFG_CL2PWR_GATING | - MG_MISC_SUS0_CFG_GAONPWR_GATING | - MG_MISC_SUS0_CFG_TRPWR_GATING | - MG_MISC_SUS0_CFG_CL1PWR_GATING | - MG_MISC_SUS0_CFG_DGPWR_GATING; - I915_WRITE(MG_MISC_SUS0(tc_port), val); -} - -static void icl_disable_phy_clock_gating(struct intel_digital_port *dig_port) -{ - struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); - enum port port = dig_port->base.port; - enum tc_port tc_port = intel_port_to_tc(dev_priv, port); - u32 val; - int ln; - - if (tc_port == PORT_TC_NONE) - return; + regs = MG_DP_MODE_CFG_TR2PWR_GATING | MG_DP_MODE_CFG_TRPWR_GATING | + MG_DP_MODE_CFG_CLNPWR_GATING | MG_DP_MODE_CFG_DIGPWR_GATING | + MG_DP_MODE_CFG_GAONPWR_GATING; for (ln = 0; ln < 2; ln++) { val = I915_READ(MG_DP_MODE(ln, port)); - val &= ~(MG_DP_MODE_CFG_TR2PWR_GATING | -MG_DP_MODE_CFG_TRPWR_GATING | -MG_DP_MODE_CFG_CLNPWR_GATING | -MG_DP_MODE_CFG_DIGPWR_GATING | -MG_DP_MODE_CFG_GAONPWR_GATING); + if (enable) + val |= regs; + else + val &= ~regs; I915_WRITE(MG_DP_MODE(ln, port), val); } + regs = MG_MISC_SUS0_CFG_TR2PWR_GATING | MG_MISC_SUS0_CFG_CL2PWR_GATING | + MG_MISC_SUS0_CFG_GAONPWR_GATING | MG_MISC_SUS0_CFG_TRPWR_GATING | + MG_MISC_SUS0_CFG_CL1PWR_GATING | MG_MISC_SUS0_CFG_DGPWR_GATING; + val = I915_READ(MG_MISC_SUS0(tc_port)); - val &= ~(MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK | -MG_MISC_SUS0_CFG_TR2PWR_GATING | -MG_MISC_SUS0_CFG_CL2PWR_GATING | -MG_MISC_SUS0_CFG_GAONPWR_GATING | -MG_MISC_SUS0_CFG_TRPWR_GATING | -MG_MISC_SUS0_CFG_CL1PWR_GATING | -MG_MISC_SUS0_CFG_DGPWR_GATING); + if (enable) + val |= (regs | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3)); + else + val &= ~(regs | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK); I915_WRITE(MG_MISC_SUS0(tc_port), val); } @@ -3538,7 +3511,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder *encoder, dig_port->ddi_io_power_domain); icl_program_mg_dp_mode(dig_port); - icl_disable_phy_clock_gating(dig_port); + icl_phy_clock_gating(dig_port, false); if (INTEL_GEN(dev_priv) >= 11) icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, @@ -3571,7 +3544,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_ddi_enable_fec(encoder, crtc_state); - icl_enable_phy_clock_gating(dig_port); + icl_phy_clock_gating(dig_port, true); if (!is_mst) intel_ddi_enable_pipe_clock(crtc_state); @@ -3611,7 +3584,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, if (INTEL_GEN(dev_priv) >= 12) tgl_phy_clock_gating(dig_port, false); else - icl_disable_phy_clock_gating(dig_port); +