Re: [Intel-gfx] [PATCH v2 09/13] drm/i915/icl: Unify disable and enable phy clock gating functions

2019-09-20 Thread Lucas De Marchi
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

2019-09-18 Thread José Roberto de Souza
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);
+