Re: [Intel-gfx] [PATCH v2 08/21] drm/i915/mtl: C20 PLL programming

2023-02-21 Thread Kahola, Mika 
> -Original Message-
> From: Sousa, Gustavo 
> Sent: Tuesday, February 7, 2023 6:53 PM
> To: Kahola, Mika ; intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani 
> Subject: Re: [Intel-gfx] [PATCH v2 08/21] drm/i915/mtl: C20 PLL programming
> 
> On Thu, Jan 05, 2023 at 02:54:33PM +0200, Mika Kahola wrote:
> > C20 phy PLL programming sequence for DP, DP2.0, HDMI2.x non-FRL and
> > HDMI2.x FRL. This enables C20 MPLLA and MPLLB programming sequence.
> > add
> > 4 lane support for c20.
> >
> > Signed-off-by: José Roberto de Souza 
> > Signed-off-by: Mika Kahola 
> > Signed-off-by: Bhanuprakash Modem 
> > Signed-off-by: Imre Deak 
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-9-
> > mika.kah...@intel.com
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 279
> > +++---  .../gpu/drm/i915/display/intel_cx0_phy_regs.h |  30 ++
> >  drivers/gpu/drm/i915/display/intel_ddi.c  |  11 +-
> >  .../drm/i915/display/intel_display_types.h|  19 +-
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  12 +-
> >  5 files changed, 311 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index 3d86b0f1c36d..022888050a68 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -14,6 +14,7 @@
> >  #include "intel_panel.h"
> >  #include "intel_psr.h"
> >  #include "intel_uncore.h"
> > +#include "intel_tc.h"
> >
> >  bool intel_is_c10phy(struct drm_i915_private *dev_priv, enum phy phy)
> > { @@ -234,6 +235,18 @@ static void intel_cx0_write(struct
> > drm_i915_private *i915, enum port port,
> > }
> >  }
> >
> > +static void intel_c20_write(struct drm_i915_private *i915, enum port port,
> > +   int lane, u16 addr, u16 data)
> 
> I think it would be better to name this function intel_c20_sram_write(), so 
> there
> is no confusion about the semantics of intel_cx0_write() vs intel_c20_write().

Yes, this is a bit confusing as what we really are doing here is to reuse c10 
read/writes for C20.

> 
> Technically, this is for both SRAM and CREG registers, but I think just using
> "sram" in the function name is enough to make the distinction. We could add a
> comment one deems necessary.

What about just intel_c20_reg_write() to make it memory type agnostic?

> 
> > +{
> > +   assert_dc_off(i915);
> > +
> > +   intel_cx0_write(i915, port, lane, PHY_C20_WR_ADDRESS_H, (addr >> 8)
> > +& 0xff, 0);
> 
> I think the 0xff mask is unnecessary here.

Yes, it seems unnecessary
> 
> > +   intel_cx0_write(i915, port, lane, PHY_C20_WR_ADDRESS_L, addr & 0xff,
> > +0);
> > +
> > +   intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_H, (data >> 8) &
> > +0xff, 0);
> 
> Also here I think the 0xff mask is unnecessary.


Yes.

> 
> > +   intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff,
> > +1); }
> > +
> >  static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
> > int lane, u16 addr, u8 clear, u8 set, bool 
> > committed)  {
> @@
> > -1155,7 +1168,7 @@ static int intel_c10mpllb_calc_state(struct
> > intel_crtc_state *crtc_state,
> >
> > for (i = 0; tables[i]; i++) {
> > if (crtc_state->port_clock <= tables[i]->clock) {
> > -   crtc_state->c10mpllb_state = *tables[i];
> > +   crtc_state->cx0pll_state.c10mpllb_state = *tables[i];
> > return 0;
> > }
> > }
> > @@ -1215,7 +1228,7 @@ static void intel_c10_pll_program(struct
> drm_i915_private *i915,
> >   const struct intel_crtc_state *crtc_state,
> >   struct intel_encoder *encoder)
> >  {
> > -   const struct intel_c10mpllb_state *pll_state = _state-
> >c10mpllb_state;
> > +   const struct intel_c10mpllb_state *pll_state =
> > +_state->cx0pll_state.c10mpllb_state;
> > struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > bool lane_reversal = dig_port->saved_port_bits &
> DDI_BUF_PORT_REVERSAL;
> > u8 master_lane = lane_reversal ? INTEL_CX0_LANE1 :
> > @@ -1299,6 +1312,218 @@ void intel_c10mpllb_dump_hw_state(struct
> drm_i915_private *dev_priv,
> > i + 2, hw_state-

Re: [Intel-gfx] [PATCH v2 08/21] drm/i915/mtl: C20 PLL programming

2023-02-07 Thread Gustavo Sousa 
On Thu, Jan 05, 2023 at 02:54:33PM +0200, Mika Kahola wrote:
> C20 phy PLL programming sequence for DP, DP2.0, HDMI2.x non-FRL and
> HDMI2.x FRL. This enables C20 MPLLA and MPLLB programming sequence. add
> 4 lane support for c20.
> 
> Signed-off-by: José Roberto de Souza 
> Signed-off-by: Mika Kahola 
> Signed-off-by: Bhanuprakash Modem 
> Signed-off-by: Imre Deak 
> Link: 
> https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-9-mika.kah...@intel.com
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 279 +++---
>  .../gpu/drm/i915/display/intel_cx0_phy_regs.h |  30 ++
>  drivers/gpu/drm/i915/display/intel_ddi.c  |  11 +-
>  .../drm/i915/display/intel_display_types.h|  19 +-
>  drivers/gpu/drm/i915/display/intel_dp.c   |  12 +-
>  5 files changed, 311 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c 
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 3d86b0f1c36d..022888050a68 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -14,6 +14,7 @@
>  #include "intel_panel.h"
>  #include "intel_psr.h"
>  #include "intel_uncore.h"
> +#include "intel_tc.h"
>  
>  bool intel_is_c10phy(struct drm_i915_private *dev_priv, enum phy phy)
>  {
> @@ -234,6 +235,18 @@ static void intel_cx0_write(struct drm_i915_private 
> *i915, enum port port,
>   }
>  }
>  
> +static void intel_c20_write(struct drm_i915_private *i915, enum port port,
> + int lane, u16 addr, u16 data)

I think it would be better to name this function intel_c20_sram_write(), so
there is no confusion about the semantics of intel_cx0_write() vs
intel_c20_write().

Technically, this is for both SRAM and CREG registers, but I think just using
"sram" in the function name is enough to make the distinction. We could add a
comment one deems necessary.

> +{
> + assert_dc_off(i915);
> +
> + intel_cx0_write(i915, port, lane, PHY_C20_WR_ADDRESS_H, (addr >> 8) & 
> 0xff, 0);

I think the 0xff mask is unnecessary here.

> + intel_cx0_write(i915, port, lane, PHY_C20_WR_ADDRESS_L, addr & 0xff, 0);
> +
> + intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_H, (data >> 8) & 
> 0xff, 0);

Also here I think the 0xff mask is unnecessary.

> + intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff, 1);
> +}
> +
>  static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
>   int lane, u16 addr, u8 clear, u8 set, bool 
> committed)
>  {
> @@ -1155,7 +1168,7 @@ static int intel_c10mpllb_calc_state(struct 
> intel_crtc_state *crtc_state,
>  
>   for (i = 0; tables[i]; i++) {
>   if (crtc_state->port_clock <= tables[i]->clock) {
> - crtc_state->c10mpllb_state = *tables[i];
> + crtc_state->cx0pll_state.c10mpllb_state = *tables[i];
>   return 0;
>   }
>   }
> @@ -1215,7 +1228,7 @@ static void intel_c10_pll_program(struct 
> drm_i915_private *i915,
> const struct intel_crtc_state *crtc_state,
> struct intel_encoder *encoder)
>  {
> - const struct intel_c10mpllb_state *pll_state = 
> _state->c10mpllb_state;
> + const struct intel_c10mpllb_state *pll_state = 
> _state->cx0pll_state.c10mpllb_state;
>   struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>   bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
>   u8 master_lane = lane_reversal ? INTEL_CX0_LANE1 :
> @@ -1299,6 +1312,218 @@ void intel_c10mpllb_dump_hw_state(struct 
> drm_i915_private *dev_priv,
>   i + 2, hw_state->pll[i + 2], i + 3, hw_state->pll[i 
> + 3]);
>  }
>  
> +static bool intel_c20_use_mplla(u32 clock)
> +{
> + /* 10G and 20G rates use MPLLA */
> + if (clock == 312500 || clock == 625000)
> + return true;
> +
> + return false;
> +}
> +
> +static u8 intel_c20_get_dp_rate(u32 clock)
> +{
> + switch (clock) {
> + case 162000: /* 1.62 Gbps DP1.4 */
> + return 0;
> + case 27: /* 2.7 Gbps DP1.4 */
> + return 1;
> + case 54: /* 5.4 Gbps DP 1.4 */
> + return 2;
> + case 81: /* 8.1 Gbps DP1.4 */
> + return 3;
> + case 216000: /* 2.16 Gbps eDP */
> + return 4;
> + case 243000: /* 2.43 Gbps eDP */
> + return 5;
> + case 324000: /* 3.24 Gbps eDP */
> + return 6;
> + case 432000: /* 4.32 Gbps eDP */
> + return 7;
> + case 312500: /* 10 Gbps DP2.0 */
> + return 8;
> + case 421875: /* 13.5 Gbps DP2.0 */
> + return 9;
> + case 625000: /* 20 Gbps DP2.0*/
> + return 10;
> + default:
> + MISSING_CASE(clock);
> + return 0;
> + }
> +}
> +
> +static u8 intel_c20_get_hdmi_rate(u32 clock)
> +{
> + switch

[Intel-gfx] [PATCH v2 08/21] drm/i915/mtl: C20 PLL programming

2023-01-05 Thread Mika Kahola 
C20 phy PLL programming sequence for DP, DP2.0, HDMI2.x non-FRL and
HDMI2.x FRL. This enables C20 MPLLA and MPLLB programming sequence. add
4 lane support for c20.

Signed-off-by: José Roberto de Souza 
Signed-off-by: Mika Kahola 
Signed-off-by: Bhanuprakash Modem 
Signed-off-by: Imre Deak 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-9-mika.kah...@intel.com
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 279 +++---
 .../gpu/drm/i915/display/intel_cx0_phy_regs.h |  30 ++
 drivers/gpu/drm/i915/display/intel_ddi.c  |  11 +-
 .../drm/i915/display/intel_display_types.h|  19 +-
 drivers/gpu/drm/i915/display/intel_dp.c   |  12 +-
 5 files changed, 311 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c 
b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 3d86b0f1c36d..022888050a68 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -14,6 +14,7 @@
 #include "intel_panel.h"
 #include "intel_psr.h"
 #include "intel_uncore.h"
+#include "intel_tc.h"
 
 bool intel_is_c10phy(struct drm_i915_private *dev_priv, enum phy phy)
 {
@@ -234,6 +235,18 @@ static void intel_cx0_write(struct drm_i915_private *i915, 
enum port port,
}
 }
 
+static void intel_c20_write(struct drm_i915_private *i915, enum port port,
+   int lane, u16 addr, u16 data)
+{
+   assert_dc_off(i915);
+
+   intel_cx0_write(i915, port, lane, PHY_C20_WR_ADDRESS_H, (addr >> 8) & 
0xff, 0);
+   intel_cx0_write(i915, port, lane, PHY_C20_WR_ADDRESS_L, addr & 0xff, 0);
+
+   intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_H, (data >> 8) & 
0xff, 0);
+   intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff, 1);
+}
+
 static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
int lane, u16 addr, u8 clear, u8 set, bool 
committed)
 {
@@ -1155,7 +1168,7 @@ static int intel_c10mpllb_calc_state(struct 
intel_crtc_state *crtc_state,
 
for (i = 0; tables[i]; i++) {
if (crtc_state->port_clock <= tables[i]->clock) {
-   crtc_state->c10mpllb_state = *tables[i];
+   crtc_state->cx0pll_state.c10mpllb_state = *tables[i];
return 0;
}
}
@@ -1215,7 +1228,7 @@ static void intel_c10_pll_program(struct drm_i915_private 
*i915,
  const struct intel_crtc_state *crtc_state,
  struct intel_encoder *encoder)
 {
-   const struct intel_c10mpllb_state *pll_state = 
_state->c10mpllb_state;
+   const struct intel_c10mpllb_state *pll_state = 
_state->cx0pll_state.c10mpllb_state;
struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
u8 master_lane = lane_reversal ? INTEL_CX0_LANE1 :
@@ -1299,6 +1312,218 @@ void intel_c10mpllb_dump_hw_state(struct 
drm_i915_private *dev_priv,
i + 2, hw_state->pll[i + 2], i + 3, hw_state->pll[i 
+ 3]);
 }
 
+static bool intel_c20_use_mplla(u32 clock)
+{
+   /* 10G and 20G rates use MPLLA */
+   if (clock == 312500 || clock == 625000)
+   return true;
+
+   return false;
+}
+
+static u8 intel_c20_get_dp_rate(u32 clock)
+{
+   switch (clock) {
+   case 162000: /* 1.62 Gbps DP1.4 */
+   return 0;
+   case 27: /* 2.7 Gbps DP1.4 */
+   return 1;
+   case 54: /* 5.4 Gbps DP 1.4 */
+   return 2;
+   case 81: /* 8.1 Gbps DP1.4 */
+   return 3;
+   case 216000: /* 2.16 Gbps eDP */
+   return 4;
+   case 243000: /* 2.43 Gbps eDP */
+   return 5;
+   case 324000: /* 3.24 Gbps eDP */
+   return 6;
+   case 432000: /* 4.32 Gbps eDP */
+   return 7;
+   case 312500: /* 10 Gbps DP2.0 */
+   return 8;
+   case 421875: /* 13.5 Gbps DP2.0 */
+   return 9;
+   case 625000: /* 20 Gbps DP2.0*/
+   return 10;
+   default:
+   MISSING_CASE(clock);
+   return 0;
+   }
+}
+
+static u8 intel_c20_get_hdmi_rate(u32 clock)
+{
+   switch (clock) {
+   case 25175:
+   case 27000:
+   case 74250:
+   case 148500:
+   case 594000:
+   return 0;
+   case 166670: /* 3 Gbps */
+   case 30: /* 6 Gbps */
+   case 70: /* 12 Gbps */
+   return 1;
+   case 40: /* 8 Gbps */
+   return 2;
+   case 60: /* 10 Gbps */
+   return 3;
+   default:
+   MISSING_CASE(clock);
+   return 0;
+   }
+}
+
+static bool is_dp2(u32 clock)
+{
+   /* DP2.0 clock rates */
+   if (clock == 312500 || clock == 421875 || clock  == 625000)
+   return true;