[v2,09/21] drm/i915/mtl: C20 HW readout

Commit Message

Mika Kahola Jan. 5, 2023, 12:54 p.m. UTC

Create a table for C20 DP1.4, DP2.0 and HDMI2.1 rates.
The PLL settings are based on table, not for algorithmic alternative.
For DP 1.4 only MPLLB is in use.

Once register settings are done, we read back C20 HW state.

BSpec: 64568

v2: Update rbr, hbr1, hbr2, and hbr3 pll configurations 4 and 5
    based on changes in BSpec consolidated table

Signed-off-by: Mika Kahola <[e-mail]>
Link: https://patchwork.freedesktop.org/patch/msgid/[e-mail]
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 495 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_cx0_phy.h |  10 +-
 drivers/gpu/drm/i915/display/intel_ddi.c     |   9 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c    |   6 +-
 drivers/gpu/drm/i915/display/intel_hdmi.h    |   1 +
 5 files changed, 513 insertions(+), 8 deletions(-)

Comments

Gustavo Sousa Feb. 7, 2023, 4:54 p.m. UTC | #1

On Thu, Jan 05, 2023 at 02:54:34PM +0200, Mika Kahola wrote:
> Create a table for C20 DP1.4, DP2.0 and HDMI2.1 rates.
> The PLL settings are based on table, not for algorithmic alternative.
> For DP 1.4 only MPLLB is in use.
> 
> Once register settings are done, we read back C20 HW state.
> 
> BSpec: 64568
> 
> v2: Update rbr, hbr1, hbr2, and hbr3 pll configurations 4 and 5
>     based on changes in BSpec consolidated table
> 
> Signed-off-by: Mika Kahola <[e-mail]>
> Link: https://patchwork.freedesktop.org/patch/msgid/[e-mail]
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 495 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_cx0_phy.h |  10 +-
>  drivers/gpu/drm/i915/display/intel_ddi.c     |   9 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c    |   6 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.h    |   1 +
>  5 files changed, 513 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 022888050a68..285e4cdd23eb 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -11,6 +11,7 @@
>  #include "intel_de.h"
>  #include "intel_display_types.h"
>  #include "intel_dp.h"
> +#include "intel_hdmi.h"
>  #include "intel_panel.h"
>  #include "intel_psr.h"
>  #include "intel_uncore.h"
> @@ -247,6 +248,23 @@ static void intel_c20_write(struct drm_i915_private *i915, enum port port,
>  	intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff, 1);
>  }
>  
> +static u16 intel_c20_read(struct drm_i915_private *i915, enum port port,
> +                          int lane, u16 addr)

Similar to my comment for intel_c20_write(), I think it would be better to name
this function intel_c20_sram_read().

Also, this function is indented with spaces instead of tabs.

> +{
> +       u16 val;
> +
> +       assert_dc_off(i915);
> +
> +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_L, addr & 0xff, 0);
> +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_H, (addr >> 8) & 0xff, 1);

Looks like the 0xff maks is unnecessary here.

Also, does the byte order matter here? The diagram for the read operation in the
BSpec suggests writing the higher part of the address first and then the lower
part.

> +
> +       val = intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_H);
> +       val <<= 8;
> +       val |= intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_L);
> +
> +        return val;
> +}
> +
>  static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
>  			    int lane, u16 addr, u8 clear, u8 set, bool committed)
>  {
> @@ -588,6 +606,192 @@ static const struct intel_c10mpllb_state * const mtl_c10_edp_tables[] = {
>  	NULL,
>  };
>  
> +/* C20 basic DP 1.4 tables */
> +static const struct intel_c20pll_state mtl_c20_dp_rbr = {
> +	.clock = 162000,
> +	.tx = {	0xbe88, /* tx cfg0 */
> +		0x5800, /* tx cfg1 */
> +		0x0000, /* tx cfg2 */
> +		},
> +	.cmn = {0x0500, /* cmn cfg0*/
> +		0x0005, /* cmn cfg1 */
> +		0x0000, /* cmn cfg2 */
> +		0x0000, /* cmn cfg3 */
> +		},
> +	.mpllb = { 0x50a8,	/* mpllb cfg0 */
> +		0x2120,		/* mpllb cfg1 */
> +		0xcd9a,		/* mpllb cfg2 */
> +		0xbfc1,		/* mpllb cfg3 */
> +		0x5ab8,         /* mpllb cfg4 */
> +		0x4c34,         /* mpllb cfg5 */
> +		0x2000,		/* mpllb cfg6 */
> +		0x0001,		/* mpllb cfg7 */
> +		0x6000,		/* mpllb cfg8 */
> +		0x0000,		/* mpllb cfg9 */
> +		0x0000,		/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_dp_hbr1 = {
> +	.clock = 270000,
> +	.tx = {	0xbe88, /* tx cfg0 */
> +		0x4800, /* tx cfg1 */
> +		0x0000, /* tx cfg2 */
> +		},
> +	.cmn = {0x0500, /* cmn cfg0*/
> +		0x0005, /* cmn cfg1 */
> +		0x0000, /* cmn cfg2 */
> +		0x0000, /* cmn cfg3 */
> +		},
> +	.mpllb = { 0x308c,	/* mpllb cfg0 */
> +		0x2110,		/* mpllb cfg1 */
> +		0xcc9c,		/* mpllb cfg2 */
> +		0xbfc1,		/* mpllb cfg3 */
> +		0x489a,         /* mpllb cfg4 */
> +		0x3f81,         /* mpllb cfg5 */
> +		0x2000,		/* mpllb cfg6 */
> +		0x0001,		/* mpllb cfg7 */
> +		0x5000,		/* mpllb cfg8 */
> +		0x0000,		/* mpllb cfg9 */
> +		0x0000,		/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_dp_hbr2 = {
> +	.clock = 540000,
> +	.tx = {	0xbe88, /* tx cfg0 */
> +		0x4800, /* tx cfg1 */
> +		0x0000, /* tx cfg2 */
> +		},
> +	.cmn = {0x0500, /* cmn cfg0*/
> +		0x0005, /* cmn cfg1 */
> +		0x0000, /* cmn cfg2 */
> +		0x0000, /* cmn cfg3 */
> +		},
> +	.mpllb = { 0x108c,	/* mpllb cfg0 */
> +		0x2108,		/* mpllb cfg1 */
> +		0xcc9c,		/* mpllb cfg2 */
> +		0xbfc1,		/* mpllb cfg3 */
> +		0x489a,         /* mpllb cfg4 */
> +		0x3f81,         /* mpllb cfg5 */
> +		0x2000,		/* mpllb cfg6 */
> +		0x0001,		/* mpllb cfg7 */
> +		0x5000,		/* mpllb cfg8 */
> +		0x0000,		/* mpllb cfg9 */
> +		0x0000,		/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_dp_hbr3 = {
> +	.clock = 810000,
> +	.tx = {	0xbe88, /* tx cfg0 */
> +		0x4800, /* tx cfg1 */
> +		0x0000, /* tx cfg2 */
> +		},
> +	.cmn = {0x0500, /* cmn cfg0*/
> +		0x0005, /* cmn cfg1 */
> +		0x0000, /* cmn cfg2 */
> +		0x0000, /* cmn cfg3 */
> +		},
> +	.mpllb = { 0x10d2,	/* mpllb cfg0 */
> +		0x2108,		/* mpllb cfg1 */
> +		0x8d98,		/* mpllb cfg2 */
> +		0xbfc1,		/* mpllb cfg3 */
> +		0x7166,         /* mpllb cfg4 */
> +		0x5f42,         /* mpllb cfg5 */
> +		0x2000,		/* mpllb cfg6 */
> +		0x0001,		/* mpllb cfg7 */
> +		0x7800,		/* mpllb cfg8 */
> +		0x0000,		/* mpllb cfg9 */
> +		0x0000,		/* mpllb cfg10 */
> +		},
> +};
> +
> +/* C20 basic DP 2.0 tables */
> +static const struct intel_c20pll_state mtl_c20_dp_uhbr10 = {
> +	.clock = 312500,
> +	.tx = {	0xbe21, /* tx cfg0 */
> +		0x4800, /* tx cfg1 */
> +		0x0000, /* tx cfg2 */
> +		},
> +	.cmn = {0x0500, /* cmn cfg0*/
> +		0x0005, /* cmn cfg1 */
> +		0x0000, /* cmn cfg2 */
> +		0x0000, /* cmn cfg3 */
> +		},
> +	.mplla = { 0x3104,	/* mplla cfg0 */
> +		0xd105,		/* mplla cfg1 */
> +		0xc025,		/* mplla cfg2 */
> +		0xc025,		/* mplla cfg3 */
> +		0xa6ab,		/* mplla cfg4 */
> +		0x8c00,		/* mplla cfg5 */
> +		0x4000,		/* mplla cfg6 */
> +		0x0003,		/* mplla cfg7 */
> +		0x3555,		/* mplla cfg8 */
> +		0x0001,		/* mplla cfg9 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_dp_uhbr13_5 = {
> +	.clock = 421875,
> +	.tx = {	0xbea0, /* tx cfg0 */
> +		0x4800, /* tx cfg1 */
> +		0x0000, /* tx cfg2 */
> +		},
> +	.cmn = {0x0500, /* cmn cfg0*/
> +		0x0005, /* cmn cfg1 */
> +		0x0000, /* cmn cfg2 */
> +		0x0000, /* cmn cfg3 */
> +		},
> +	.mpllb = { 0x015f,	/* mpllb cfg0 */
> +		0x2205,		/* mpllb cfg1 */
> +		0x1b17,		/* mpllb cfg2 */
> +		0xffc1,		/* mpllb cfg3 */
> +		0xe100,		/* mpllb cfg4 */
> +		0xbd00,		/* mpllb cfg5 */
> +		0x2000,		/* mpllb cfg6 */
> +		0x0001,		/* mpllb cfg7 */
> +		0x4800,		/* mpllb cfg8 */
> +		0x0000,		/* mpllb cfg9 */
> +		0x0000,		/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_dp_uhbr20 = {
> +	.clock = 625000,
> +	.tx = {	0xbe20, /* tx cfg0 */
> +		0x4800, /* tx cfg1 */
> +		0x0000, /* tx cfg2 */
> +		},
> +	.cmn = {0x0500, /* cmn cfg0*/
> +		0x0005, /* cmn cfg1 */
> +		0x0000, /* cmn cfg2 */
> +		0x0000, /* cmn cfg3 */
> +		},
> +	.mplla = { 0x3104,	/* mplla cfg0 */
> +		0xd105,		/* mplla cfg1 */
> +		0xc025,		/* mplla cfg2 */
> +		0xc025,		/* mplla cfg3 */
> +		0xa6ab,		/* mplla cfg4 */
> +		0x8c00,		/* mplla cfg5 */
> +		0x4000,		/* mplla cfg6 */
> +		0x0003,		/* mplla cfg7 */
> +		0x3555,		/* mplla cfg8 */
> +		0x0001,		/* mplla cfg9 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state * const mtl_c20_dp_tables[] = {
> +        &mtl_c20_dp_rbr,
> +        &mtl_c20_dp_hbr1,
> +        &mtl_c20_dp_hbr2,
> +        &mtl_c20_dp_hbr3,
> +	&mtl_c20_dp_uhbr10,
> +	&mtl_c20_dp_uhbr13_5,
> +	&mtl_c20_dp_uhbr20,
> +        NULL,
> +};
> +
>  /*
>   * HDMI link rates with 38.4 MHz reference clock.
>   */
> @@ -1115,7 +1319,181 @@ static const struct intel_c10mpllb_state * const mtl_c10_hdmi_tables[] = {
>  	NULL,
>  };
>  
> -int intel_c10_phy_check_hdmi_link_rate(int clock)
> +static const struct intel_c20pll_state mtl_c20_hdmi_25_175 = {
> +	.clock = 25175,
> +	.mpllb = { 0xa0d2,	/* mpllb cfg0 */
> +		   0x7d80,	/* mpllb cfg1 */
> +		   0x0906,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x0200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x0000,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0001,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_27_0 = {
> +	.clock = 27000,
> +	.mpllb = { 0xa0e0,	/* mpllb cfg0 */
> +		   0x7d80,	/* mpllb cfg1 */
> +		   0x0906,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x8000,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0001,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_74_25 = {
> +	.clock = 74250,
> +	.mpllb = { 0x609a,	/* mpllb cfg0 */
> +		   0x7d40,	/* mpllb cfg1 */
> +		   0xca06,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x5800,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0001,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_148_5 = {
> +	.clock = 148500,
> +	.mpllb = { 0x409a,	/* mpllb cfg0 */
> +		   0x7d20,	/* mpllb cfg1 */
> +		   0xca06,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x5800,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0001,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_594 = {
> +	.clock = 594000,
> +	.mpllb = { 0x009a,	/* mpllb cfg0 */
> +		   0x7d08,	/* mpllb cfg1 */
> +		   0xca06,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x5800,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0001,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_300 = {
> +	.clock = 166670,
> +	.mpllb = { 0x209c,	/* mpllb cfg0 */
> +		   0x7d10,	/* mpllb cfg1 */
> +		   0xca06,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x2000,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0004,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_600 = {
> +	.clock = 333330,
> +	.mpllb = { 0x009c,	/* mpllb cfg0 */
> +		   0x7d08,	/* mpllb cfg1 */
> +		   0xca06,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x2000,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0004,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_800 = {
> +	.clock = 444440,
> +	.mpllb = { 0x00d0,	/* mpllb cfg0 */
> +		   0x7d08,	/* mpllb cfg1 */
> +		   0x4a06,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0003,	/* mpllb cfg7 */
> +		   0x2aaa,	/* mpllb cfg8 */
> +		   0x0002,	/* mpllb cfg9 */
> +		   0x0004,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_1000 = {
> +	.clock = 555560,
> +	.mpllb = { 0x1104,	/* mpllb cfg0 */
> +		   0x7d08,	/* mpllb cfg1 */
> +		   0x0a06,	/* mpllb cfg2 */
> +		   0xbe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0003,	/* mpllb cfg7 */
> +		   0x3555,	/* mpllb cfg8 */
> +		   0x0001,	/* mpllb cfg9 */
> +		   0x0004,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state mtl_c20_hdmi_1200 = {
> +	.clock = 666670,
> +	.mpllb = { 0x0138,	/* mpllb cfg0 */
> +		   0x7d08,	/* mpllb cfg1 */
> +		   0x5486,	/* mpllb cfg2 */
> +		   0xfe40,	/* mpllb cfg3 */
> +		   0x0000,	/* mpllb cfg4 */
> +		   0x0000,	/* mpllb cfg5 */
> +		   0x2200,	/* mpllb cfg6 */
> +		   0x0001,	/* mpllb cfg7 */
> +		   0x4000,	/* mpllb cfg8 */
> +		   0x0000,	/* mpllb cfg9 */
> +		   0x0004,	/* mpllb cfg10 */
> +		},
> +};
> +
> +static const struct intel_c20pll_state * const mtl_c20_hdmi_tables[] = {
> +	&mtl_c20_hdmi_25_175,
> +	&mtl_c20_hdmi_27_0,
> +	&mtl_c20_hdmi_74_25,
> +	&mtl_c20_hdmi_148_5,
> +	&mtl_c20_hdmi_594,
> +	&mtl_c20_hdmi_300,
> +	&mtl_c20_hdmi_600,
> +	&mtl_c20_hdmi_800,
> +	&mtl_c20_hdmi_1000,
> +	&mtl_c20_hdmi_1200,
> +	NULL,
> +};
> +
> +static int intel_c10_phy_check_hdmi_link_rate(int clock)
>  {
>  	const struct intel_c10mpllb_state * const *tables = mtl_c10_hdmi_tables;
>  	int i;
> @@ -1128,6 +1506,17 @@ int intel_c10_phy_check_hdmi_link_rate(int clock)
>  	return MODE_CLOCK_RANGE;
>  }
>  
> +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int clock)
> +{
> +	struct intel_digital_port *dig_port = hdmi_to_dig_port(hdmi);
> +	struct drm_i915_private *i915 = intel_hdmi_to_i915(hdmi);
> +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> +
> +	if (intel_is_c10phy(i915, phy))
> +		return intel_c10_phy_check_hdmi_link_rate(clock);
> +	return intel_c20_phy_check_hdmi_link_rate(clock);
> +}
> +
>  static const struct intel_c10mpllb_state * const *
>  intel_c10_mpllb_tables_get(struct intel_crtc_state *crtc_state,
>  			   struct intel_encoder *encoder)
> @@ -1312,6 +1701,53 @@ 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]);
>  }
>  
> +int intel_c20_phy_check_hdmi_link_rate(int clock)
> +{
> +        const struct intel_c20pll_state * const *tables = mtl_c20_hdmi_tables;
> +        int i;
> +
> +        for (i = 0; tables[i]; i++) {
> +                if (clock == tables[i]->clock)
> +                        return MODE_OK;
> +        }
> +
> +        return MODE_CLOCK_RANGE;
> +}
> +
> +static const struct intel_c20pll_state * const *
> +intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
> +			 struct intel_encoder *encoder)
> +{
> +	if (intel_crtc_has_dp_encoder(crtc_state)) {
> +		return mtl_c20_dp_tables;
> +	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> +		return mtl_c20_hdmi_tables;
> +	}
> +
> +	MISSING_CASE(encoder->type);
> +	return NULL;
> +}
> +
> +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> +	                    struct intel_encoder *encoder)

Looks like this function should be static: the only user is intel_cx0_phy.c.

> +{
> +	const struct intel_c20pll_state * const *tables;
> +	int i;
> +
> +	tables = intel_c20_pll_tables_get(crtc_state, encoder);
> +	if (!tables)
> +		return -EINVAL;
> +
> +	for (i = 0; tables[i]; i++) {
> +		if (crtc_state->port_clock <= tables[i]->clock) {
> +			crtc_state->cx0pll_state.c20pll_state = *tables[i];
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static bool intel_c20_use_mplla(u32 clock)
>  {
>  	/* 10G and 20G rates use MPLLA */
> @@ -1321,6 +1757,63 @@ static bool intel_c20_use_mplla(u32 clock)
>  	return false;
>  }
>  
> +void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> +				   struct intel_c20pll_state *pll_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	bool cntx, use_mplla;
> +	u32 val;
> +	int i;
> +
> +        /* 1. Read current context selection */
> +        cntx = intel_cx0_read(i915, encoder->port, 0, PHY_C20_VDR_CUSTOM_SERDES_RATE) & PHY_C20_CONTEXT_TOGGLE;
> +
> +	/* Read Tx configuration */
> +	for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) {
> +		if (cntx)
> +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> +							  PHY_C20_B_TX_CNTX_CFG(i));
> +		else
> +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> +							  PHY_C20_A_TX_CNTX_CFG(i));
> +	}
> +
> +	/* Read common configuration */
> +	for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) {
> +		if (cntx)
> +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port, 0,
> +							   PHY_C20_B_CMN_CNTX_CFG(i));
> +		else
> +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port, 0,
> +							   PHY_C20_A_CMN_CNTX_CFG(i));
> +	}
> +
> +	val = intel_c20_read(i915, encoder->port, 0, PHY_C20_A_MPLLA_CNTX_CFG(6));
> +	use_mplla = val & C20_MPLLB_FRACEN;

Some questions about this:

  1. This is hardcoded to read from context A. Shouldn't we read from the
     current context?

  2. s/C20_MPLLB_FRACEN/C20_MPLLA_FRACEN/ ?

  3. When we are programming PLL for MPLLB, we are not clearing
     PHY_C20_A_MPLLA_CNTX_CFG(6). Wouldn't this be problematic if MPLLB is the
     current one in use MPLLB but MPLLA was already used in a previous
     configuration?

> +
> +	if (use_mplla) {
> +		/* MPLLA configuration */
> +		for (i = 0; i < ARRAY_SIZE(pll_state->mplla); i++) {
> +			if (cntx)
> +				pll_state->mplla[i] = intel_c20_read(i915, encoder->port, 0,
> +								     PHY_C20_B_MPLLA_CNTX_CFG(i));
> +			else
> +				pll_state->mplla[i] = intel_c20_read(i915, encoder->port, 0,
> +								     PHY_C20_A_MPLLA_CNTX_CFG(i));
> +		}
> +	} else {
> +		/* MPLLB configuration */
> +		for (i = 0; i < ARRAY_SIZE(pll_state->mpllb); i++) {
> +			if (cntx)
> +				pll_state->mpllb[i] = intel_c20_read(i915, encoder->port, 0,
> +								     PHY_C20_B_MPLLB_CNTX_CFG(i));
> +			else
> +				pll_state->mpllb[i] = intel_c20_read(i915, encoder->port, 0,
> +								     PHY_C20_A_MPLLB_CNTX_CFG(i));
> +		}
> +	}
> +}
> +
>  static u8 intel_c20_get_dp_rate(u32 clock)
>  {
>  	switch (clock) {
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.h b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> index 952c7deeffaa..86edbc4b1718 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> @@ -33,14 +33,22 @@ void intel_c10mpllb_readout_hw_state(struct intel_encoder *encoder,
>  				     struct intel_c10mpllb_state *pll_state);
>  int intel_cx0mpllb_calc_state(struct intel_crtc_state *crtc_state,
>  			      struct intel_encoder *encoder);
> +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> +			    struct intel_encoder *encoder);
> +void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> +                                   struct intel_c20pll_state *pll_state);
>  void intel_c10mpllb_dump_hw_state(struct drm_i915_private *dev_priv,
>  				  const struct intel_c10mpllb_state *hw_state);
>  int intel_c10mpllb_calc_port_clock(struct intel_encoder *encoder,
>  				   const struct intel_c10mpllb_state *pll_state);
>  void intel_c10mpllb_state_verify(struct intel_atomic_state *state,
>  				 struct intel_crtc_state *new_crtc_state);
> -int intel_c10_phy_check_hdmi_link_rate(int clock);
> +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int clock);
>  void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
>  				     const struct intel_crtc_state *crtc_state);
>  
> +int intel_c20_phy_check_hdmi_link_rate(int clock);
> +void intel_cx0_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> +				       const struct intel_crtc_state *crtc_state,
> +				       u32 level);
>  #endif /* __INTEL_CX0_PHY_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a5fbc4fed28d..c37f7f7d84cf 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3532,10 +3532,13 @@ static void mtl_ddi_get_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	enum phy phy = intel_port_to_phy(i915, encoder->port);
>  
> -	drm_WARN_ON(&i915->drm, !intel_is_c10phy(i915, phy));
> +	if (intel_is_c10phy(i915, phy)) {
> +		intel_c10mpllb_readout_hw_state(encoder, &crtc_state->cx0pll_state.c10mpllb_state);
> +		intel_c10mpllb_dump_hw_state(i915, &crtc_state->cx0pll_state.c10mpllb_state);
> +	} else {
> +		intel_c20pll_readout_hw_state(encoder, &crtc_state->cx0pll_state.c20pll_state);
> +	}
>  
> -	intel_c10mpllb_readout_hw_state(encoder, &crtc_state->cx0pll_state.c10mpllb_state);
> -	intel_c10mpllb_dump_hw_state(i915, &crtc_state->cx0pll_state.c10mpllb_state);
>  	crtc_state->port_clock = intel_c10mpllb_calc_port_clock(encoder, &crtc_state->cx0pll_state.c10mpllb_state);
>  
>  	intel_ddi_get_config(encoder, crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 7b81781a64b6..e60fcd477091 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -58,7 +58,7 @@
>  #include "intel_panel.h"
>  #include "intel_snps_phy.h"
>  
> -static struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi *intel_hdmi)
> +inline struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi *intel_hdmi)
>  {
>  	return to_i915(hdmi_to_dig_port(intel_hdmi)->base.base.dev);
>  }
> @@ -1883,8 +1883,8 @@ hdmi_port_clock_valid(struct intel_hdmi *hdmi,
>  	 * FIXME: We will hopefully get an algorithmic way of programming
>  	 * the MPLLB for HDMI in the future.
>  	 */
> -	if (IS_METEORLAKE(dev_priv))
> -		return intel_c10_phy_check_hdmi_link_rate(clock);
> +	if (DISPLAY_VER(dev_priv) >= 14)
> +		return intel_cx0_phy_check_hdmi_link_rate(hdmi, clock);
>  	else if (IS_DG2(dev_priv))
>  		return intel_snps_phy_check_hdmi_link_rate(clock);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
> index 774dda2376ed..492bd3921385 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> @@ -54,5 +54,6 @@ int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
>  				  int src_max_slices, int src_max_slice_width,
>  				  int hdmi_max_slices, int hdmi_throughput);
>  int intel_hdmi_dsc_get_slice_height(int vactive);
> +struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi *intel_hdmi);
>  
>  #endif /* __INTEL_HDMI_H__ */
> -- 
> 2.34.1
>

Mika Kahola Feb. 23, 2023, 9:47 a.m. UTC | #2

> -----Original Message-----
> From: Sousa, Gustavo <[e-mail]>
> Sent: Tuesday, February 7, 2023 6:54 PM
> To: Kahola, Mika <[e-mail]>; [e-mail]
> Cc: Nikula, Jani <[e-mail]>
> Subject: Re: [Intel-gfx] [PATCH v2 09/21] drm/i915/mtl: C20 HW readout
> 
> On Thu, Jan 05, 2023 at 02:54:34PM +0200, Mika Kahola wrote:
> > Create a table for C20 DP1.4, DP2.0 and HDMI2.1 rates.
> > The PLL settings are based on table, not for algorithmic alternative.
> > For DP 1.4 only MPLLB is in use.
> >
> > Once register settings are done, we read back C20 HW state.
> >
> > BSpec: 64568
> >
> > v2: Update rbr, hbr1, hbr2, and hbr3 pll configurations 4 and 5
> >     based on changes in BSpec consolidated table
> >
> > Signed-off-by: Mika Kahola <[e-mail]>
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-10
> > [e-mail]
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 495
> > ++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_cx0_phy.h |  10 +-
> >  drivers/gpu/drm/i915/display/intel_ddi.c     |   9 +-
> >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   6 +-
> >  drivers/gpu/drm/i915/display/intel_hdmi.h    |   1 +
> >  5 files changed, 513 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index 022888050a68..285e4cdd23eb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -11,6 +11,7 @@
> >  #include "intel_de.h"
> >  #include "intel_display_types.h"
> >  #include "intel_dp.h"
> > +#include "intel_hdmi.h"
> >  #include "intel_panel.h"
> >  #include "intel_psr.h"
> >  #include "intel_uncore.h"
> > @@ -247,6 +248,23 @@ static void intel_c20_write(struct drm_i915_private
> *i915, enum port port,
> >  	intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff,
> > 1);  }
> >
> > +static u16 intel_c20_read(struct drm_i915_private *i915, enum port port,
> > +                          int lane, u16 addr)
> 
> Similar to my comment for intel_c20_write(), I think it would be better to name
> this function intel_c20_sram_read().

We can rename these read/writes differently.
> 
> Also, this function is indented with spaces instead of tabs.
This comes from copy-paste error. Thanks for spotting!

> 
> > +{
> > +       u16 val;
> > +
> > +       assert_dc_off(i915);
> > +
> > +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_L, addr & 0xff,
> 0);
> > +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_H, (addr
> > + >> 8) & 0xff, 1);
> 
> Looks like the 0xff maks is unnecessary here.
Ok.

> 
> Also, does the byte order matter here? The diagram for the read operation in
> the BSpec suggests writing the higher part of the address first and then the
> lower part.
I don't think it matters here but to be aligned with the spec we could swap these writes.

> 
> > +
> > +       val = intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_H);
> > +       val <<= 8;
> > +       val |= intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_L);
> > +
> > +        return val;
> > +}
> > +
> >  static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
> >  			    int lane, u16 addr, u8 clear, u8 set, bool committed)  {
> @@
> > -588,6 +606,192 @@ static const struct intel_c10mpllb_state * const
> mtl_c10_edp_tables[] = {
> >  	NULL,
> >  };
> >
> > +/* C20 basic DP 1.4 tables */
> > +static const struct intel_c20pll_state mtl_c20_dp_rbr = {
> > +	.clock = 162000,
> > +	.tx = {	0xbe88, /* tx cfg0 */
> > +		0x5800, /* tx cfg1 */
> > +		0x0000, /* tx cfg2 */
> > +		},
> > +	.cmn = {0x0500, /* cmn cfg0*/
> > +		0x0005, /* cmn cfg1 */
> > +		0x0000, /* cmn cfg2 */
> > +		0x0000, /* cmn cfg3 */
> > +		},
> > +	.mpllb = { 0x50a8,	/* mpllb cfg0 */
> > +		0x2120,		/* mpllb cfg1 */
> > +		0xcd9a,		/* mpllb cfg2 */
> > +		0xbfc1,		/* mpllb cfg3 */
> > +		0x5ab8,         /* mpllb cfg4 */
> > +		0x4c34,         /* mpllb cfg5 */
> > +		0x2000,		/* mpllb cfg6 */
> > +		0x0001,		/* mpllb cfg7 */
> > +		0x6000,		/* mpllb cfg8 */
> > +		0x0000,		/* mpllb cfg9 */
> > +		0x0000,		/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_dp_hbr1 = {
> > +	.clock = 270000,
> > +	.tx = {	0xbe88, /* tx cfg0 */
> > +		0x4800, /* tx cfg1 */
> > +		0x0000, /* tx cfg2 */
> > +		},
> > +	.cmn = {0x0500, /* cmn cfg0*/
> > +		0x0005, /* cmn cfg1 */
> > +		0x0000, /* cmn cfg2 */
> > +		0x0000, /* cmn cfg3 */
> > +		},
> > +	.mpllb = { 0x308c,	/* mpllb cfg0 */
> > +		0x2110,		/* mpllb cfg1 */
> > +		0xcc9c,		/* mpllb cfg2 */
> > +		0xbfc1,		/* mpllb cfg3 */
> > +		0x489a,         /* mpllb cfg4 */
> > +		0x3f81,         /* mpllb cfg5 */
> > +		0x2000,		/* mpllb cfg6 */
> > +		0x0001,		/* mpllb cfg7 */
> > +		0x5000,		/* mpllb cfg8 */
> > +		0x0000,		/* mpllb cfg9 */
> > +		0x0000,		/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_dp_hbr2 = {
> > +	.clock = 540000,
> > +	.tx = {	0xbe88, /* tx cfg0 */
> > +		0x4800, /* tx cfg1 */
> > +		0x0000, /* tx cfg2 */
> > +		},
> > +	.cmn = {0x0500, /* cmn cfg0*/
> > +		0x0005, /* cmn cfg1 */
> > +		0x0000, /* cmn cfg2 */
> > +		0x0000, /* cmn cfg3 */
> > +		},
> > +	.mpllb = { 0x108c,	/* mpllb cfg0 */
> > +		0x2108,		/* mpllb cfg1 */
> > +		0xcc9c,		/* mpllb cfg2 */
> > +		0xbfc1,		/* mpllb cfg3 */
> > +		0x489a,         /* mpllb cfg4 */
> > +		0x3f81,         /* mpllb cfg5 */
> > +		0x2000,		/* mpllb cfg6 */
> > +		0x0001,		/* mpllb cfg7 */
> > +		0x5000,		/* mpllb cfg8 */
> > +		0x0000,		/* mpllb cfg9 */
> > +		0x0000,		/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_dp_hbr3 = {
> > +	.clock = 810000,
> > +	.tx = {	0xbe88, /* tx cfg0 */
> > +		0x4800, /* tx cfg1 */
> > +		0x0000, /* tx cfg2 */
> > +		},
> > +	.cmn = {0x0500, /* cmn cfg0*/
> > +		0x0005, /* cmn cfg1 */
> > +		0x0000, /* cmn cfg2 */
> > +		0x0000, /* cmn cfg3 */
> > +		},
> > +	.mpllb = { 0x10d2,	/* mpllb cfg0 */
> > +		0x2108,		/* mpllb cfg1 */
> > +		0x8d98,		/* mpllb cfg2 */
> > +		0xbfc1,		/* mpllb cfg3 */
> > +		0x7166,         /* mpllb cfg4 */
> > +		0x5f42,         /* mpllb cfg5 */
> > +		0x2000,		/* mpllb cfg6 */
> > +		0x0001,		/* mpllb cfg7 */
> > +		0x7800,		/* mpllb cfg8 */
> > +		0x0000,		/* mpllb cfg9 */
> > +		0x0000,		/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +/* C20 basic DP 2.0 tables */
> > +static const struct intel_c20pll_state mtl_c20_dp_uhbr10 = {
> > +	.clock = 312500,
> > +	.tx = {	0xbe21, /* tx cfg0 */
> > +		0x4800, /* tx cfg1 */
> > +		0x0000, /* tx cfg2 */
> > +		},
> > +	.cmn = {0x0500, /* cmn cfg0*/
> > +		0x0005, /* cmn cfg1 */
> > +		0x0000, /* cmn cfg2 */
> > +		0x0000, /* cmn cfg3 */
> > +		},
> > +	.mplla = { 0x3104,	/* mplla cfg0 */
> > +		0xd105,		/* mplla cfg1 */
> > +		0xc025,		/* mplla cfg2 */
> > +		0xc025,		/* mplla cfg3 */
> > +		0xa6ab,		/* mplla cfg4 */
> > +		0x8c00,		/* mplla cfg5 */
> > +		0x4000,		/* mplla cfg6 */
> > +		0x0003,		/* mplla cfg7 */
> > +		0x3555,		/* mplla cfg8 */
> > +		0x0001,		/* mplla cfg9 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_dp_uhbr13_5 = {
> > +	.clock = 421875,
> > +	.tx = {	0xbea0, /* tx cfg0 */
> > +		0x4800, /* tx cfg1 */
> > +		0x0000, /* tx cfg2 */
> > +		},
> > +	.cmn = {0x0500, /* cmn cfg0*/
> > +		0x0005, /* cmn cfg1 */
> > +		0x0000, /* cmn cfg2 */
> > +		0x0000, /* cmn cfg3 */
> > +		},
> > +	.mpllb = { 0x015f,	/* mpllb cfg0 */
> > +		0x2205,		/* mpllb cfg1 */
> > +		0x1b17,		/* mpllb cfg2 */
> > +		0xffc1,		/* mpllb cfg3 */
> > +		0xe100,		/* mpllb cfg4 */
> > +		0xbd00,		/* mpllb cfg5 */
> > +		0x2000,		/* mpllb cfg6 */
> > +		0x0001,		/* mpllb cfg7 */
> > +		0x4800,		/* mpllb cfg8 */
> > +		0x0000,		/* mpllb cfg9 */
> > +		0x0000,		/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_dp_uhbr20 = {
> > +	.clock = 625000,
> > +	.tx = {	0xbe20, /* tx cfg0 */
> > +		0x4800, /* tx cfg1 */
> > +		0x0000, /* tx cfg2 */
> > +		},
> > +	.cmn = {0x0500, /* cmn cfg0*/
> > +		0x0005, /* cmn cfg1 */
> > +		0x0000, /* cmn cfg2 */
> > +		0x0000, /* cmn cfg3 */
> > +		},
> > +	.mplla = { 0x3104,	/* mplla cfg0 */
> > +		0xd105,		/* mplla cfg1 */
> > +		0xc025,		/* mplla cfg2 */
> > +		0xc025,		/* mplla cfg3 */
> > +		0xa6ab,		/* mplla cfg4 */
> > +		0x8c00,		/* mplla cfg5 */
> > +		0x4000,		/* mplla cfg6 */
> > +		0x0003,		/* mplla cfg7 */
> > +		0x3555,		/* mplla cfg8 */
> > +		0x0001,		/* mplla cfg9 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state * const mtl_c20_dp_tables[] = {
> > +        &mtl_c20_dp_rbr,
> > +        &mtl_c20_dp_hbr1,
> > +        &mtl_c20_dp_hbr2,
> > +        &mtl_c20_dp_hbr3,
> > +	&mtl_c20_dp_uhbr10,
> > +	&mtl_c20_dp_uhbr13_5,
> > +	&mtl_c20_dp_uhbr20,
> > +        NULL,
> > +};
> > +
> >  /*
> >   * HDMI link rates with 38.4 MHz reference clock.
> >   */
> > @@ -1115,7 +1319,181 @@ static const struct intel_c10mpllb_state * const
> mtl_c10_hdmi_tables[] = {
> >  	NULL,
> >  };
> >
> > -int intel_c10_phy_check_hdmi_link_rate(int clock)
> > +static const struct intel_c20pll_state mtl_c20_hdmi_25_175 = {
> > +	.clock = 25175,
> > +	.mpllb = { 0xa0d2,	/* mpllb cfg0 */
> > +		   0x7d80,	/* mpllb cfg1 */
> > +		   0x0906,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x0200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x0000,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0001,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_27_0 = {
> > +	.clock = 27000,
> > +	.mpllb = { 0xa0e0,	/* mpllb cfg0 */
> > +		   0x7d80,	/* mpllb cfg1 */
> > +		   0x0906,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x8000,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0001,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_74_25 = {
> > +	.clock = 74250,
> > +	.mpllb = { 0x609a,	/* mpllb cfg0 */
> > +		   0x7d40,	/* mpllb cfg1 */
> > +		   0xca06,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x5800,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0001,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_148_5 = {
> > +	.clock = 148500,
> > +	.mpllb = { 0x409a,	/* mpllb cfg0 */
> > +		   0x7d20,	/* mpllb cfg1 */
> > +		   0xca06,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x5800,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0001,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_594 = {
> > +	.clock = 594000,
> > +	.mpllb = { 0x009a,	/* mpllb cfg0 */
> > +		   0x7d08,	/* mpllb cfg1 */
> > +		   0xca06,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x5800,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0001,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_300 = {
> > +	.clock = 166670,
> > +	.mpllb = { 0x209c,	/* mpllb cfg0 */
> > +		   0x7d10,	/* mpllb cfg1 */
> > +		   0xca06,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x2000,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0004,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_600 = {
> > +	.clock = 333330,
> > +	.mpllb = { 0x009c,	/* mpllb cfg0 */
> > +		   0x7d08,	/* mpllb cfg1 */
> > +		   0xca06,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x2000,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0004,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_800 = {
> > +	.clock = 444440,
> > +	.mpllb = { 0x00d0,	/* mpllb cfg0 */
> > +		   0x7d08,	/* mpllb cfg1 */
> > +		   0x4a06,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0003,	/* mpllb cfg7 */
> > +		   0x2aaa,	/* mpllb cfg8 */
> > +		   0x0002,	/* mpllb cfg9 */
> > +		   0x0004,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_1000 = {
> > +	.clock = 555560,
> > +	.mpllb = { 0x1104,	/* mpllb cfg0 */
> > +		   0x7d08,	/* mpllb cfg1 */
> > +		   0x0a06,	/* mpllb cfg2 */
> > +		   0xbe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0003,	/* mpllb cfg7 */
> > +		   0x3555,	/* mpllb cfg8 */
> > +		   0x0001,	/* mpllb cfg9 */
> > +		   0x0004,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state mtl_c20_hdmi_1200 = {
> > +	.clock = 666670,
> > +	.mpllb = { 0x0138,	/* mpllb cfg0 */
> > +		   0x7d08,	/* mpllb cfg1 */
> > +		   0x5486,	/* mpllb cfg2 */
> > +		   0xfe40,	/* mpllb cfg3 */
> > +		   0x0000,	/* mpllb cfg4 */
> > +		   0x0000,	/* mpllb cfg5 */
> > +		   0x2200,	/* mpllb cfg6 */
> > +		   0x0001,	/* mpllb cfg7 */
> > +		   0x4000,	/* mpllb cfg8 */
> > +		   0x0000,	/* mpllb cfg9 */
> > +		   0x0004,	/* mpllb cfg10 */
> > +		},
> > +};
> > +
> > +static const struct intel_c20pll_state * const mtl_c20_hdmi_tables[] = {
> > +	&mtl_c20_hdmi_25_175,
> > +	&mtl_c20_hdmi_27_0,
> > +	&mtl_c20_hdmi_74_25,
> > +	&mtl_c20_hdmi_148_5,
> > +	&mtl_c20_hdmi_594,
> > +	&mtl_c20_hdmi_300,
> > +	&mtl_c20_hdmi_600,
> > +	&mtl_c20_hdmi_800,
> > +	&mtl_c20_hdmi_1000,
> > +	&mtl_c20_hdmi_1200,
> > +	NULL,
> > +};
> > +
> > +static int intel_c10_phy_check_hdmi_link_rate(int clock)
> >  {
> >  	const struct intel_c10mpllb_state * const *tables =
> mtl_c10_hdmi_tables;
> >  	int i;
> > @@ -1128,6 +1506,17 @@ int intel_c10_phy_check_hdmi_link_rate(int clock)
> >  	return MODE_CLOCK_RANGE;
> >  }
> >
> > +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int
> > +clock) {
> > +	struct intel_digital_port *dig_port = hdmi_to_dig_port(hdmi);
> > +	struct drm_i915_private *i915 = intel_hdmi_to_i915(hdmi);
> > +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> > +
> > +	if (intel_is_c10phy(i915, phy))
> > +		return intel_c10_phy_check_hdmi_link_rate(clock);
> > +	return intel_c20_phy_check_hdmi_link_rate(clock);
> > +}
> > +
> >  static const struct intel_c10mpllb_state * const *
> > intel_c10_mpllb_tables_get(struct intel_crtc_state *crtc_state,
> >  			   struct intel_encoder *encoder)
> > @@ -1312,6 +1701,53 @@ 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]);
> }
> >
> > +int intel_c20_phy_check_hdmi_link_rate(int clock) {
> > +        const struct intel_c20pll_state * const *tables = mtl_c20_hdmi_tables;
> > +        int i;
> > +
> > +        for (i = 0; tables[i]; i++) {
> > +                if (clock == tables[i]->clock)
> > +                        return MODE_OK;
> > +        }
> > +
> > +        return MODE_CLOCK_RANGE;
> > +}
> > +
> > +static const struct intel_c20pll_state * const *
> > +intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
> > +			 struct intel_encoder *encoder)
> > +{
> > +	if (intel_crtc_has_dp_encoder(crtc_state)) {
> > +		return mtl_c20_dp_tables;
> > +	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> > +		return mtl_c20_hdmi_tables;
> > +	}
> > +
> > +	MISSING_CASE(encoder->type);
> > +	return NULL;
> > +}
> > +
> > +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> > +	                    struct intel_encoder *encoder)
> 
> Looks like this function should be static: the only user is intel_cx0_phy.c.
> 
> > +{
> > +	const struct intel_c20pll_state * const *tables;
> > +	int i;
> > +
> > +	tables = intel_c20_pll_tables_get(crtc_state, encoder);
> > +	if (!tables)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; tables[i]; i++) {
> > +		if (crtc_state->port_clock <= tables[i]->clock) {
> > +			crtc_state->cx0pll_state.c20pll_state = *tables[i];
> > +			return 0;
> > +		}
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> >  static bool intel_c20_use_mplla(u32 clock)  {
> >  	/* 10G and 20G rates use MPLLA */
> > @@ -1321,6 +1757,63 @@ static bool intel_c20_use_mplla(u32 clock)
> >  	return false;
> >  }
> >
> > +void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> > +				   struct intel_c20pll_state *pll_state) {
> > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +	bool cntx, use_mplla;
> > +	u32 val;
> > +	int i;
> > +
> > +        /* 1. Read current context selection */
> > +        cntx = intel_cx0_read(i915, encoder->port, 0,
> > + PHY_C20_VDR_CUSTOM_SERDES_RATE) & PHY_C20_CONTEXT_TOGGLE;
> > +
> > +	/* Read Tx configuration */
> > +	for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) {
> > +		if (cntx)
> > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> > +
> PHY_C20_B_TX_CNTX_CFG(i));
> > +		else
> > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> > +
> PHY_C20_A_TX_CNTX_CFG(i));
> > +	}
> > +
> > +	/* Read common configuration */
> > +	for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) {
> > +		if (cntx)
> > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port,
> 0,
> > +
> PHY_C20_B_CMN_CNTX_CFG(i));
> > +		else
> > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port,
> 0,
> > +
> PHY_C20_A_CMN_CNTX_CFG(i));
> > +	}
> > +
> > +	val = intel_c20_read(i915, encoder->port, 0,
> PHY_C20_A_MPLLA_CNTX_CFG(6));
> > +	use_mplla = val & C20_MPLLB_FRACEN;
> 
> Some questions about this:
> 
>   1. This is hardcoded to read from context A. Shouldn't we read from the
>      current context?
> 
>   2. s/C20_MPLLB_FRACEN/C20_MPLLA_FRACEN/ ?
The both uses the same bit 13 for frac_en. Maybe just renaming differently C10_MPLLx_FRACEN?

> 
>   3. When we are programming PLL for MPLLB, we are not clearing
>      PHY_C20_A_MPLLA_CNTX_CFG(6). Wouldn't this be problematic if MPLLB is
> the
>      current one in use MPLLB but MPLLA was already used in a previous
>      configuration?
Do you mean this when we are switching the context? In this case, as far as I have understood the spec, we wouldn't need to clear previous configuration.

> 
> > +
> > +	if (use_mplla) {
> > +		/* MPLLA configuration */
> > +		for (i = 0; i < ARRAY_SIZE(pll_state->mplla); i++) {
> > +			if (cntx)
> > +				pll_state->mplla[i] = intel_c20_read(i915,
> encoder->port, 0,
> > +
> PHY_C20_B_MPLLA_CNTX_CFG(i));
> > +			else
> > +				pll_state->mplla[i] = intel_c20_read(i915,
> encoder->port, 0,
> > +
> PHY_C20_A_MPLLA_CNTX_CFG(i));
> > +		}
> > +	} else {
> > +		/* MPLLB configuration */
> > +		for (i = 0; i < ARRAY_SIZE(pll_state->mpllb); i++) {
> > +			if (cntx)
> > +				pll_state->mpllb[i] = intel_c20_read(i915,
> encoder->port, 0,
> > +
> PHY_C20_B_MPLLB_CNTX_CFG(i));
> > +			else
> > +				pll_state->mpllb[i] = intel_c20_read(i915,
> encoder->port, 0,
> > +
> PHY_C20_A_MPLLB_CNTX_CFG(i));
> > +		}
> > +	}
> > +}
> > +
> >  static u8 intel_c20_get_dp_rate(u32 clock)  {
> >  	switch (clock) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > index 952c7deeffaa..86edbc4b1718 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > @@ -33,14 +33,22 @@ void intel_c10mpllb_readout_hw_state(struct
> intel_encoder *encoder,
> >  				     struct intel_c10mpllb_state *pll_state);  int
> > intel_cx0mpllb_calc_state(struct intel_crtc_state *crtc_state,
> >  			      struct intel_encoder *encoder);
> > +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> > +			    struct intel_encoder *encoder); void
> > +intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> > +                                   struct intel_c20pll_state
> > +*pll_state);
> >  void intel_c10mpllb_dump_hw_state(struct drm_i915_private *dev_priv,
> >  				  const struct intel_c10mpllb_state *hw_state);
> int
> > intel_c10mpllb_calc_port_clock(struct intel_encoder *encoder,
> >  				   const struct intel_c10mpllb_state *pll_state);
> void
> > intel_c10mpllb_state_verify(struct intel_atomic_state *state,
> >  				 struct intel_crtc_state *new_crtc_state); -int
> > intel_c10_phy_check_hdmi_link_rate(int clock);
> > +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int
> > +clock);
> >  void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
> >  				     const struct intel_crtc_state *crtc_state);
> >
> > +int intel_c20_phy_check_hdmi_link_rate(int clock); void
> > +intel_cx0_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> > +				       const struct intel_crtc_state *crtc_state,
> > +				       u32 level);
> >  #endif /* __INTEL_CX0_PHY_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index a5fbc4fed28d..c37f7f7d84cf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3532,10 +3532,13 @@ static void mtl_ddi_get_config(struct
> intel_encoder *encoder,
> >  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	enum phy phy = intel_port_to_phy(i915, encoder->port);
> >
> > -	drm_WARN_ON(&i915->drm, !intel_is_c10phy(i915, phy));
> > +	if (intel_is_c10phy(i915, phy)) {
> > +		intel_c10mpllb_readout_hw_state(encoder, &crtc_state-
> >cx0pll_state.c10mpllb_state);
> > +		intel_c10mpllb_dump_hw_state(i915, &crtc_state-
> >cx0pll_state.c10mpllb_state);
> > +	} else {
> > +		intel_c20pll_readout_hw_state(encoder, &crtc_state-
> >cx0pll_state.c20pll_state);
> > +	}
> >
> > -	intel_c10mpllb_readout_hw_state(encoder, &crtc_state-
> >cx0pll_state.c10mpllb_state);
> > -	intel_c10mpllb_dump_hw_state(i915, &crtc_state-
> >cx0pll_state.c10mpllb_state);
> >  	crtc_state->port_clock = intel_c10mpllb_calc_port_clock(encoder,
> > &crtc_state->cx0pll_state.c10mpllb_state);
> >
> >  	intel_ddi_get_config(encoder, crtc_state); diff --git
> > a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 7b81781a64b6..e60fcd477091 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -58,7 +58,7 @@
> >  #include "intel_panel.h"
> >  #include "intel_snps_phy.h"
> >
> > -static struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi
> > *intel_hdmi)
> > +inline struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi
> > +*intel_hdmi)
> >  {
> >  	return to_i915(hdmi_to_dig_port(intel_hdmi)->base.base.dev);
> >  }
> > @@ -1883,8 +1883,8 @@ hdmi_port_clock_valid(struct intel_hdmi *hdmi,
> >  	 * FIXME: We will hopefully get an algorithmic way of programming
> >  	 * the MPLLB for HDMI in the future.
> >  	 */
> > -	if (IS_METEORLAKE(dev_priv))
> > -		return intel_c10_phy_check_hdmi_link_rate(clock);
> > +	if (DISPLAY_VER(dev_priv) >= 14)
> > +		return intel_cx0_phy_check_hdmi_link_rate(hdmi, clock);
> >  	else if (IS_DG2(dev_priv))
> >  		return intel_snps_phy_check_hdmi_link_rate(clock);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h
> > b/drivers/gpu/drm/i915/display/intel_hdmi.h
> > index 774dda2376ed..492bd3921385 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> > @@ -54,5 +54,6 @@ int intel_hdmi_dsc_get_num_slices(const struct
> intel_crtc_state *crtc_state,
> >  				  int src_max_slices, int src_max_slice_width,
> >  				  int hdmi_max_slices, int hdmi_throughput);
> int
> > intel_hdmi_dsc_get_slice_height(int vactive);
> > +struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi
> > +*intel_hdmi);
> >
> >  #endif /* __INTEL_HDMI_H__ */
> > --
> > 2.34.1
> >

Mika Kahola Feb. 23, 2023, 10:36 a.m. UTC | #3

> -----Original Message-----
> From: Intel-gfx <[e-mail]> On Behalf Of Kahola,
> Mika
> Sent: Thursday, February 23, 2023 11:47 AM
> To: Sousa, Gustavo <[e-mail]>; intel-
> [e-mail]
> Cc: Nikula, Jani <[e-mail]>
> Subject: Re: [Intel-gfx] [PATCH v2 09/21] drm/i915/mtl: C20 HW readout
> 
> > -----Original Message-----
> > From: Sousa, Gustavo <[e-mail]>
> > Sent: Tuesday, February 7, 2023 6:54 PM
> > To: Kahola, Mika <[e-mail]>;
> > [e-mail]
> > Cc: Nikula, Jani <[e-mail]>
> > Subject: Re: [Intel-gfx] [PATCH v2 09/21] drm/i915/mtl: C20 HW readout
> >
> > On Thu, Jan 05, 2023 at 02:54:34PM +0200, Mika Kahola wrote:
> > > Create a table for C20 DP1.4, DP2.0 and HDMI2.1 rates.
> > > The PLL settings are based on table, not for algorithmic alternative.
> > > For DP 1.4 only MPLLB is in use.
> > >
> > > Once register settings are done, we read back C20 HW state.
> > >
> > > BSpec: 64568
> > >
> > > v2: Update rbr, hbr1, hbr2, and hbr3 pll configurations 4 and 5
> > >     based on changes in BSpec consolidated table
> > >
> > > Signed-off-by: Mika Kahola <[e-mail]>
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-
> > > 10
> > > [e-mail]
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 495
> > > ++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_cx0_phy.h |
> > > ++++++++++++++++++10 +-
> > >  drivers/gpu/drm/i915/display/intel_ddi.c     |   9 +-
> > >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   6 +-
> > >  drivers/gpu/drm/i915/display/intel_hdmi.h    |   1 +
> > >  5 files changed, 513 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > index 022888050a68..285e4cdd23eb 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > @@ -11,6 +11,7 @@
> > >  #include "intel_de.h"
> > >  #include "intel_display_types.h"
> > >  #include "intel_dp.h"
> > > +#include "intel_hdmi.h"
> > >  #include "intel_panel.h"
> > >  #include "intel_psr.h"
> > >  #include "intel_uncore.h"
> > > @@ -247,6 +248,23 @@ static void intel_c20_write(struct
> > > drm_i915_private
> > *i915, enum port port,
> > >  	intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff,
> > > 1);  }
> > >
> > > +static u16 intel_c20_read(struct drm_i915_private *i915, enum port port,
> > > +                          int lane, u16 addr)
> >
> > Similar to my comment for intel_c20_write(), I think it would be
> > better to name this function intel_c20_sram_read().
> 
> We can rename these read/writes differently.
> >
> > Also, this function is indented with spaces instead of tabs.
> This comes from copy-paste error. Thanks for spotting!
> 
> >
> > > +{
> > > +       u16 val;
> > > +
> > > +       assert_dc_off(i915);
> > > +
> > > +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_L, addr
> > > + & 0xff,
> > 0);
> > > +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_H,
> > > + (addr
> > > + >> 8) & 0xff, 1);
> >
> > Looks like the 0xff maks is unnecessary here.
> Ok.
> 
> >
> > Also, does the byte order matter here? The diagram for the read
> > operation in the BSpec suggests writing the higher part of the address
> > first and then the lower part.
> I don't think it matters here but to be aligned with the spec we could swap these
> writes.
> 
> >
> > > +
> > > +       val = intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_H);
> > > +       val <<= 8;
> > > +       val |= intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_L);
> > > +
> > > +        return val;
> > > +}
> > > +
> > >  static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
> > >  			    int lane, u16 addr, u8 clear, u8 set, bool committed)  {
> > @@
> > > -588,6 +606,192 @@ static const struct intel_c10mpllb_state * const
> > mtl_c10_edp_tables[] = {
> > >  	NULL,
> > >  };
> > >
> > > +/* C20 basic DP 1.4 tables */
> > > +static const struct intel_c20pll_state mtl_c20_dp_rbr = {
> > > +	.clock = 162000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x5800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x50a8,	/* mpllb cfg0 */
> > > +		0x2120,		/* mpllb cfg1 */
> > > +		0xcd9a,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x5ab8,         /* mpllb cfg4 */
> > > +		0x4c34,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x6000,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_hbr1 = {
> > > +	.clock = 270000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x308c,	/* mpllb cfg0 */
> > > +		0x2110,		/* mpllb cfg1 */
> > > +		0xcc9c,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x489a,         /* mpllb cfg4 */
> > > +		0x3f81,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x5000,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_hbr2 = {
> > > +	.clock = 540000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x108c,	/* mpllb cfg0 */
> > > +		0x2108,		/* mpllb cfg1 */
> > > +		0xcc9c,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x489a,         /* mpllb cfg4 */
> > > +		0x3f81,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x5000,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_hbr3 = {
> > > +	.clock = 810000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x10d2,	/* mpllb cfg0 */
> > > +		0x2108,		/* mpllb cfg1 */
> > > +		0x8d98,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x7166,         /* mpllb cfg4 */
> > > +		0x5f42,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x7800,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +/* C20 basic DP 2.0 tables */
> > > +static const struct intel_c20pll_state mtl_c20_dp_uhbr10 = {
> > > +	.clock = 312500,
> > > +	.tx = {	0xbe21, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mplla = { 0x3104,	/* mplla cfg0 */
> > > +		0xd105,		/* mplla cfg1 */
> > > +		0xc025,		/* mplla cfg2 */
> > > +		0xc025,		/* mplla cfg3 */
> > > +		0xa6ab,		/* mplla cfg4 */
> > > +		0x8c00,		/* mplla cfg5 */
> > > +		0x4000,		/* mplla cfg6 */
> > > +		0x0003,		/* mplla cfg7 */
> > > +		0x3555,		/* mplla cfg8 */
> > > +		0x0001,		/* mplla cfg9 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_uhbr13_5 = {
> > > +	.clock = 421875,
> > > +	.tx = {	0xbea0, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x015f,	/* mpllb cfg0 */
> > > +		0x2205,		/* mpllb cfg1 */
> > > +		0x1b17,		/* mpllb cfg2 */
> > > +		0xffc1,		/* mpllb cfg3 */
> > > +		0xe100,		/* mpllb cfg4 */
> > > +		0xbd00,		/* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x4800,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_uhbr20 = {
> > > +	.clock = 625000,
> > > +	.tx = {	0xbe20, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mplla = { 0x3104,	/* mplla cfg0 */
> > > +		0xd105,		/* mplla cfg1 */
> > > +		0xc025,		/* mplla cfg2 */
> > > +		0xc025,		/* mplla cfg3 */
> > > +		0xa6ab,		/* mplla cfg4 */
> > > +		0x8c00,		/* mplla cfg5 */
> > > +		0x4000,		/* mplla cfg6 */
> > > +		0x0003,		/* mplla cfg7 */
> > > +		0x3555,		/* mplla cfg8 */
> > > +		0x0001,		/* mplla cfg9 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state * const mtl_c20_dp_tables[] = {
> > > +        &mtl_c20_dp_rbr,
> > > +        &mtl_c20_dp_hbr1,
> > > +        &mtl_c20_dp_hbr2,
> > > +        &mtl_c20_dp_hbr3,
> > > +	&mtl_c20_dp_uhbr10,
> > > +	&mtl_c20_dp_uhbr13_5,
> > > +	&mtl_c20_dp_uhbr20,
> > > +        NULL,
> > > +};
> > > +
> > >  /*
> > >   * HDMI link rates with 38.4 MHz reference clock.
> > >   */
> > > @@ -1115,7 +1319,181 @@ static const struct intel_c10mpllb_state *
> > > const
> > mtl_c10_hdmi_tables[] = {
> > >  	NULL,
> > >  };
> > >
> > > -int intel_c10_phy_check_hdmi_link_rate(int clock)
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_25_175 = {
> > > +	.clock = 25175,
> > > +	.mpllb = { 0xa0d2,	/* mpllb cfg0 */
> > > +		   0x7d80,	/* mpllb cfg1 */
> > > +		   0x0906,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x0200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x0000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_27_0 = {
> > > +	.clock = 27000,
> > > +	.mpllb = { 0xa0e0,	/* mpllb cfg0 */
> > > +		   0x7d80,	/* mpllb cfg1 */
> > > +		   0x0906,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x8000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_74_25 = {
> > > +	.clock = 74250,
> > > +	.mpllb = { 0x609a,	/* mpllb cfg0 */
> > > +		   0x7d40,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x5800,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_148_5 = {
> > > +	.clock = 148500,
> > > +	.mpllb = { 0x409a,	/* mpllb cfg0 */
> > > +		   0x7d20,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x5800,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_594 = {
> > > +	.clock = 594000,
> > > +	.mpllb = { 0x009a,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x5800,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_300 = {
> > > +	.clock = 166670,
> > > +	.mpllb = { 0x209c,	/* mpllb cfg0 */
> > > +		   0x7d10,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x2000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_600 = {
> > > +	.clock = 333330,
> > > +	.mpllb = { 0x009c,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x2000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_800 = {
> > > +	.clock = 444440,
> > > +	.mpllb = { 0x00d0,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0x4a06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0003,	/* mpllb cfg7 */
> > > +		   0x2aaa,	/* mpllb cfg8 */
> > > +		   0x0002,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_1000 = {
> > > +	.clock = 555560,
> > > +	.mpllb = { 0x1104,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0x0a06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0003,	/* mpllb cfg7 */
> > > +		   0x3555,	/* mpllb cfg8 */
> > > +		   0x0001,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_1200 = {
> > > +	.clock = 666670,
> > > +	.mpllb = { 0x0138,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0x5486,	/* mpllb cfg2 */
> > > +		   0xfe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x4000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state * const mtl_c20_hdmi_tables[] = {
> > > +	&mtl_c20_hdmi_25_175,
> > > +	&mtl_c20_hdmi_27_0,
> > > +	&mtl_c20_hdmi_74_25,
> > > +	&mtl_c20_hdmi_148_5,
> > > +	&mtl_c20_hdmi_594,
> > > +	&mtl_c20_hdmi_300,
> > > +	&mtl_c20_hdmi_600,
> > > +	&mtl_c20_hdmi_800,
> > > +	&mtl_c20_hdmi_1000,
> > > +	&mtl_c20_hdmi_1200,
> > > +	NULL,
> > > +};
> > > +
> > > +static int intel_c10_phy_check_hdmi_link_rate(int clock)
> > >  {
> > >  	const struct intel_c10mpllb_state * const *tables =
> > mtl_c10_hdmi_tables;
> > >  	int i;
> > > @@ -1128,6 +1506,17 @@ int intel_c10_phy_check_hdmi_link_rate(int
> clock)
> > >  	return MODE_CLOCK_RANGE;
> > >  }
> > >
> > > +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int
> > > +clock) {
> > > +	struct intel_digital_port *dig_port = hdmi_to_dig_port(hdmi);
> > > +	struct drm_i915_private *i915 = intel_hdmi_to_i915(hdmi);
> > > +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> > > +
> > > +	if (intel_is_c10phy(i915, phy))
> > > +		return intel_c10_phy_check_hdmi_link_rate(clock);
> > > +	return intel_c20_phy_check_hdmi_link_rate(clock);
> > > +}
> > > +
> > >  static const struct intel_c10mpllb_state * const *
> > > intel_c10_mpllb_tables_get(struct intel_crtc_state *crtc_state,
> > >  			   struct intel_encoder *encoder) @@ -1312,6 +1701,53
> @@ 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]);
> > }
> > >
> > > +int intel_c20_phy_check_hdmi_link_rate(int clock) {
> > > +        const struct intel_c20pll_state * const *tables = mtl_c20_hdmi_tables;
> > > +        int i;
> > > +
> > > +        for (i = 0; tables[i]; i++) {
> > > +                if (clock == tables[i]->clock)
> > > +                        return MODE_OK;
> > > +        }
> > > +
> > > +        return MODE_CLOCK_RANGE;
> > > +}
> > > +
> > > +static const struct intel_c20pll_state * const *
> > > +intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
> > > +			 struct intel_encoder *encoder)
> > > +{
> > > +	if (intel_crtc_has_dp_encoder(crtc_state)) {
> > > +		return mtl_c20_dp_tables;
> > > +	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> > > +		return mtl_c20_hdmi_tables;
> > > +	}
> > > +
> > > +	MISSING_CASE(encoder->type);
> > > +	return NULL;
> > > +}
> > > +
> > > +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> > > +	                    struct intel_encoder *encoder)
> >
> > Looks like this function should be static: the only user is intel_cx0_phy.c.
Yes, this can be a static function

> >
> > > +{
> > > +	const struct intel_c20pll_state * const *tables;
> > > +	int i;
> > > +
> > > +	tables = intel_c20_pll_tables_get(crtc_state, encoder);
> > > +	if (!tables)
> > > +		return -EINVAL;
> > > +
> > > +	for (i = 0; tables[i]; i++) {
> > > +		if (crtc_state->port_clock <= tables[i]->clock) {
> > > +			crtc_state->cx0pll_state.c20pll_state = *tables[i];
> > > +			return 0;
> > > +		}
> > > +	}
> > > +
> > > +	return -EINVAL;
> > > +}
> > > +
> > >  static bool intel_c20_use_mplla(u32 clock)  {
> > >  	/* 10G and 20G rates use MPLLA */
> > > @@ -1321,6 +1757,63 @@ static bool intel_c20_use_mplla(u32 clock)
> > >  	return false;
> > >  }
> > >
> > > +void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> > > +				   struct intel_c20pll_state *pll_state) {
> > > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > +	bool cntx, use_mplla;
> > > +	u32 val;
> > > +	int i;
> > > +
> > > +        /* 1. Read current context selection */
> > > +        cntx = intel_cx0_read(i915, encoder->port, 0,
> > > + PHY_C20_VDR_CUSTOM_SERDES_RATE) & PHY_C20_CONTEXT_TOGGLE;
> > > +
> > > +	/* Read Tx configuration */
> > > +	for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) {
> > > +		if (cntx)
> > > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> > > +
> > PHY_C20_B_TX_CNTX_CFG(i));
> > > +		else
> > > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> > > +
> > PHY_C20_A_TX_CNTX_CFG(i));
> > > +	}
> > > +
> > > +	/* Read common configuration */
> > > +	for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) {
> > > +		if (cntx)
> > > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port,
> > 0,
> > > +
> > PHY_C20_B_CMN_CNTX_CFG(i));
> > > +		else
> > > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port,
> > 0,
> > > +
> > PHY_C20_A_CMN_CNTX_CFG(i));
> > > +	}
> > > +
> > > +	val = intel_c20_read(i915, encoder->port, 0,
> > PHY_C20_A_MPLLA_CNTX_CFG(6));
> > > +	use_mplla = val & C20_MPLLB_FRACEN;
> >
> > Some questions about this:
> >
> >   1. This is hardcoded to read from context A. Shouldn't we read from the
> >      current context?
Right, we should read from current context.
> >
> >   2. s/C20_MPLLB_FRACEN/C20_MPLLA_FRACEN/ ?
> The both uses the same bit 13 for frac_en. Maybe just renaming differently
> C10_MPLLx_FRACEN?
Sorry, I was too hesitant to reply. Bit 13 is for C20_MPLLB_FRACEN and bit 14 C20_MPLLA_FRACEN. So C20_MPLL_FRACEN should be used.

> 
> >
> >   3. When we are programming PLL for MPLLB, we are not clearing
> >      PHY_C20_A_MPLLA_CNTX_CFG(6). Wouldn't this be problematic if
> > MPLLB is the
> >      current one in use MPLLB but MPLLA was already used in a previous
> >      configuration?
> Do you mean this when we are switching the context? In this case, as far as I
> have understood the spec, we wouldn't need to clear previous configuration.
> 
> >
> > > +
> > > +	if (use_mplla) {
> > > +		/* MPLLA configuration */
> > > +		for (i = 0; i < ARRAY_SIZE(pll_state->mplla); i++) {
> > > +			if (cntx)
> > > +				pll_state->mplla[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_B_MPLLA_CNTX_CFG(i));
> > > +			else
> > > +				pll_state->mplla[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_A_MPLLA_CNTX_CFG(i));
> > > +		}
> > > +	} else {
> > > +		/* MPLLB configuration */
> > > +		for (i = 0; i < ARRAY_SIZE(pll_state->mpllb); i++) {
> > > +			if (cntx)
> > > +				pll_state->mpllb[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_B_MPLLB_CNTX_CFG(i));
> > > +			else
> > > +				pll_state->mpllb[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_A_MPLLB_CNTX_CFG(i));
> > > +		}
> > > +	}
> > > +}
> > > +
> > >  static u8 intel_c20_get_dp_rate(u32 clock)  {
> > >  	switch (clock) {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > index 952c7deeffaa..86edbc4b1718 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > @@ -33,14 +33,22 @@ void intel_c10mpllb_readout_hw_state(struct
> > intel_encoder *encoder,
> > >  				     struct intel_c10mpllb_state *pll_state);  int
> > > intel_cx0mpllb_calc_state(struct intel_crtc_state *crtc_state,
> > >  			      struct intel_encoder *encoder);
> > > +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> > > +			    struct intel_encoder *encoder); void
> > > +intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> > > +                                   struct intel_c20pll_state
> > > +*pll_state);
> > >  void intel_c10mpllb_dump_hw_state(struct drm_i915_private *dev_priv,
> > >  				  const struct intel_c10mpllb_state *hw_state);
> > int
> > > intel_c10mpllb_calc_port_clock(struct intel_encoder *encoder,
> > >  				   const struct intel_c10mpllb_state *pll_state);
> > void
> > > intel_c10mpllb_state_verify(struct intel_atomic_state *state,
> > >  				 struct intel_crtc_state *new_crtc_state); -int
> > > intel_c10_phy_check_hdmi_link_rate(int clock);
> > > +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int
> > > +clock);
> > >  void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
> > >  				     const struct intel_crtc_state *crtc_state);
> > >
> > > +int intel_c20_phy_check_hdmi_link_rate(int clock); void
> > > +intel_cx0_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > +				       const struct intel_crtc_state *crtc_state,
> > > +				       u32 level);
> > >  #endif /* __INTEL_CX0_PHY_H__ */
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index a5fbc4fed28d..c37f7f7d84cf 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -3532,10 +3532,13 @@ static void mtl_ddi_get_config(struct
> > intel_encoder *encoder,
> > >  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > >  	enum phy phy = intel_port_to_phy(i915, encoder->port);
> > >
> > > -	drm_WARN_ON(&i915->drm, !intel_is_c10phy(i915, phy));
> > > +	if (intel_is_c10phy(i915, phy)) {
> > > +		intel_c10mpllb_readout_hw_state(encoder, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > > +		intel_c10mpllb_dump_hw_state(i915, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > > +	} else {
> > > +		intel_c20pll_readout_hw_state(encoder, &crtc_state-
> > >cx0pll_state.c20pll_state);
> > > +	}
> > >
> > > -	intel_c10mpllb_readout_hw_state(encoder, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > > -	intel_c10mpllb_dump_hw_state(i915, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > >  	crtc_state->port_clock = intel_c10mpllb_calc_port_clock(encoder,
> > > &crtc_state->cx0pll_state.c10mpllb_state);
> > >
> > >  	intel_ddi_get_config(encoder, crtc_state); diff --git
> > > a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > index 7b81781a64b6..e60fcd477091 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > @@ -58,7 +58,7 @@
> > >  #include "intel_panel.h"
> > >  #include "intel_snps_phy.h"
> > >
> > > -static struct drm_i915_private *intel_hdmi_to_i915(struct
> > > intel_hdmi
> > > *intel_hdmi)
> > > +inline struct drm_i915_private *intel_hdmi_to_i915(struct
> > > +intel_hdmi
> > > +*intel_hdmi)
> > >  {
> > >  	return to_i915(hdmi_to_dig_port(intel_hdmi)->base.base.dev);
> > >  }
> > > @@ -1883,8 +1883,8 @@ hdmi_port_clock_valid(struct intel_hdmi *hdmi,
> > >  	 * FIXME: We will hopefully get an algorithmic way of programming
> > >  	 * the MPLLB for HDMI in the future.
> > >  	 */
> > > -	if (IS_METEORLAKE(dev_priv))
> > > -		return intel_c10_phy_check_hdmi_link_rate(clock);
> > > +	if (DISPLAY_VER(dev_priv) >= 14)
> > > +		return intel_cx0_phy_check_hdmi_link_rate(hdmi, clock);
> > >  	else if (IS_DG2(dev_priv))
> > >  		return intel_snps_phy_check_hdmi_link_rate(clock);
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > b/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > index 774dda2376ed..492bd3921385 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > @@ -54,5 +54,6 @@ int intel_hdmi_dsc_get_num_slices(const struct
> > intel_crtc_state *crtc_state,
> > >  				  int src_max_slices, int src_max_slice_width,
> > >  				  int hdmi_max_slices, int hdmi_throughput);
> > int
> > > intel_hdmi_dsc_get_slice_height(int vactive);
> > > +struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi
> > > +*intel_hdmi);
> > >
> > >  #endif /* __INTEL_HDMI_H__ */
> > > --
> > > 2.34.1
> > >

Gustavo Sousa March 24, 2023, 8:51 p.m. UTC | #4

On Thu, Feb 23, 2023 at 06:47:27AM -0300, Kahola, Mika wrote:
> > -----Original Message-----
> > From: Sousa, Gustavo <[e-mail]>
> > Sent: Tuesday, February 7, 2023 6:54 PM
> > To: Kahola, Mika <[e-mail]>; [e-mail]
> > Cc: Nikula, Jani <[e-mail]>
> > Subject: Re: [Intel-gfx] [PATCH v2 09/21] drm/i915/mtl: C20 HW readout
> > 
> > On Thu, Jan 05, 2023 at 02:54:34PM +0200, Mika Kahola wrote:
> > > Create a table for C20 DP1.4, DP2.0 and HDMI2.1 rates.
> > > The PLL settings are based on table, not for algorithmic alternative.
> > > For DP 1.4 only MPLLB is in use.
> > >
> > > Once register settings are done, we read back C20 HW state.
> > >
> > > BSpec: 64568
> > >
> > > v2: Update rbr, hbr1, hbr2, and hbr3 pll configurations 4 and 5
> > >     based on changes in BSpec consolidated table
> > >
> > > Signed-off-by: Mika Kahola <[e-mail]>
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-10
> > > [e-mail]
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 495
> > > ++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_cx0_phy.h |  10 +-
> > >  drivers/gpu/drm/i915/display/intel_ddi.c     |   9 +-
> > >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   6 +-
> > >  drivers/gpu/drm/i915/display/intel_hdmi.h    |   1 +
> > >  5 files changed, 513 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > index 022888050a68..285e4cdd23eb 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > @@ -11,6 +11,7 @@
> > >  #include "intel_de.h"
> > >  #include "intel_display_types.h"
> > >  #include "intel_dp.h"
> > > +#include "intel_hdmi.h"
> > >  #include "intel_panel.h"
> > >  #include "intel_psr.h"
> > >  #include "intel_uncore.h"
> > > @@ -247,6 +248,23 @@ static void intel_c20_write(struct drm_i915_private
> > *i915, enum port port,
> > >  	intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff,
> > > 1);  }
> > >
> > > +static u16 intel_c20_read(struct drm_i915_private *i915, enum port port,
> > > +                          int lane, u16 addr)
> > 
> > Similar to my comment for intel_c20_write(), I think it would be better to name
> > this function intel_c20_sram_read().
> 
> We can rename these read/writes differently.
> > 
> > Also, this function is indented with spaces instead of tabs.
> This comes from copy-paste error. Thanks for spotting!
> 
> > 
> > > +{
> > > +       u16 val;
> > > +
> > > +       assert_dc_off(i915);
> > > +
> > > +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_L, addr & 0xff,
> > 0);
> > > +       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_H, (addr
> > > + >> 8) & 0xff, 1);
> > 
> > Looks like the 0xff maks is unnecessary here.
> Ok.
> 
> > 
> > Also, does the byte order matter here? The diagram for the read operation in
> > the BSpec suggests writing the higher part of the address first and then the
> > lower part.
> I don't think it matters here but to be aligned with the spec we could swap these writes.
> 
> > 
> > > +
> > > +       val = intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_H);
> > > +       val <<= 8;
> > > +       val |= intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_L);
> > > +
> > > +        return val;
> > > +}
> > > +
> > >  static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
> > >  			    int lane, u16 addr, u8 clear, u8 set, bool committed)  {
> > @@
> > > -588,6 +606,192 @@ static const struct intel_c10mpllb_state * const
> > mtl_c10_edp_tables[] = {
> > >  	NULL,
> > >  };
> > >
> > > +/* C20 basic DP 1.4 tables */
> > > +static const struct intel_c20pll_state mtl_c20_dp_rbr = {
> > > +	.clock = 162000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x5800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x50a8,	/* mpllb cfg0 */
> > > +		0x2120,		/* mpllb cfg1 */
> > > +		0xcd9a,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x5ab8,         /* mpllb cfg4 */
> > > +		0x4c34,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x6000,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_hbr1 = {
> > > +	.clock = 270000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x308c,	/* mpllb cfg0 */
> > > +		0x2110,		/* mpllb cfg1 */
> > > +		0xcc9c,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x489a,         /* mpllb cfg4 */
> > > +		0x3f81,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x5000,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_hbr2 = {
> > > +	.clock = 540000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x108c,	/* mpllb cfg0 */
> > > +		0x2108,		/* mpllb cfg1 */
> > > +		0xcc9c,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x489a,         /* mpllb cfg4 */
> > > +		0x3f81,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x5000,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_hbr3 = {
> > > +	.clock = 810000,
> > > +	.tx = {	0xbe88, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x10d2,	/* mpllb cfg0 */
> > > +		0x2108,		/* mpllb cfg1 */
> > > +		0x8d98,		/* mpllb cfg2 */
> > > +		0xbfc1,		/* mpllb cfg3 */
> > > +		0x7166,         /* mpllb cfg4 */
> > > +		0x5f42,         /* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x7800,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +/* C20 basic DP 2.0 tables */
> > > +static const struct intel_c20pll_state mtl_c20_dp_uhbr10 = {
> > > +	.clock = 312500,
> > > +	.tx = {	0xbe21, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mplla = { 0x3104,	/* mplla cfg0 */
> > > +		0xd105,		/* mplla cfg1 */
> > > +		0xc025,		/* mplla cfg2 */
> > > +		0xc025,		/* mplla cfg3 */
> > > +		0xa6ab,		/* mplla cfg4 */
> > > +		0x8c00,		/* mplla cfg5 */
> > > +		0x4000,		/* mplla cfg6 */
> > > +		0x0003,		/* mplla cfg7 */
> > > +		0x3555,		/* mplla cfg8 */
> > > +		0x0001,		/* mplla cfg9 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_uhbr13_5 = {
> > > +	.clock = 421875,
> > > +	.tx = {	0xbea0, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mpllb = { 0x015f,	/* mpllb cfg0 */
> > > +		0x2205,		/* mpllb cfg1 */
> > > +		0x1b17,		/* mpllb cfg2 */
> > > +		0xffc1,		/* mpllb cfg3 */
> > > +		0xe100,		/* mpllb cfg4 */
> > > +		0xbd00,		/* mpllb cfg5 */
> > > +		0x2000,		/* mpllb cfg6 */
> > > +		0x0001,		/* mpllb cfg7 */
> > > +		0x4800,		/* mpllb cfg8 */
> > > +		0x0000,		/* mpllb cfg9 */
> > > +		0x0000,		/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_dp_uhbr20 = {
> > > +	.clock = 625000,
> > > +	.tx = {	0xbe20, /* tx cfg0 */
> > > +		0x4800, /* tx cfg1 */
> > > +		0x0000, /* tx cfg2 */
> > > +		},
> > > +	.cmn = {0x0500, /* cmn cfg0*/
> > > +		0x0005, /* cmn cfg1 */
> > > +		0x0000, /* cmn cfg2 */
> > > +		0x0000, /* cmn cfg3 */
> > > +		},
> > > +	.mplla = { 0x3104,	/* mplla cfg0 */
> > > +		0xd105,		/* mplla cfg1 */
> > > +		0xc025,		/* mplla cfg2 */
> > > +		0xc025,		/* mplla cfg3 */
> > > +		0xa6ab,		/* mplla cfg4 */
> > > +		0x8c00,		/* mplla cfg5 */
> > > +		0x4000,		/* mplla cfg6 */
> > > +		0x0003,		/* mplla cfg7 */
> > > +		0x3555,		/* mplla cfg8 */
> > > +		0x0001,		/* mplla cfg9 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state * const mtl_c20_dp_tables[] = {
> > > +        &mtl_c20_dp_rbr,
> > > +        &mtl_c20_dp_hbr1,
> > > +        &mtl_c20_dp_hbr2,
> > > +        &mtl_c20_dp_hbr3,
> > > +	&mtl_c20_dp_uhbr10,
> > > +	&mtl_c20_dp_uhbr13_5,
> > > +	&mtl_c20_dp_uhbr20,
> > > +        NULL,
> > > +};
> > > +
> > >  /*
> > >   * HDMI link rates with 38.4 MHz reference clock.
> > >   */
> > > @@ -1115,7 +1319,181 @@ static const struct intel_c10mpllb_state * const
> > mtl_c10_hdmi_tables[] = {
> > >  	NULL,
> > >  };
> > >
> > > -int intel_c10_phy_check_hdmi_link_rate(int clock)
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_25_175 = {
> > > +	.clock = 25175,
> > > +	.mpllb = { 0xa0d2,	/* mpllb cfg0 */
> > > +		   0x7d80,	/* mpllb cfg1 */
> > > +		   0x0906,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x0200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x0000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_27_0 = {
> > > +	.clock = 27000,
> > > +	.mpllb = { 0xa0e0,	/* mpllb cfg0 */
> > > +		   0x7d80,	/* mpllb cfg1 */
> > > +		   0x0906,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x8000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_74_25 = {
> > > +	.clock = 74250,
> > > +	.mpllb = { 0x609a,	/* mpllb cfg0 */
> > > +		   0x7d40,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x5800,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_148_5 = {
> > > +	.clock = 148500,
> > > +	.mpllb = { 0x409a,	/* mpllb cfg0 */
> > > +		   0x7d20,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x5800,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_594 = {
> > > +	.clock = 594000,
> > > +	.mpllb = { 0x009a,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x5800,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0001,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_300 = {
> > > +	.clock = 166670,
> > > +	.mpllb = { 0x209c,	/* mpllb cfg0 */
> > > +		   0x7d10,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x2000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_600 = {
> > > +	.clock = 333330,
> > > +	.mpllb = { 0x009c,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0xca06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x2000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_800 = {
> > > +	.clock = 444440,
> > > +	.mpllb = { 0x00d0,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0x4a06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0003,	/* mpllb cfg7 */
> > > +		   0x2aaa,	/* mpllb cfg8 */
> > > +		   0x0002,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_1000 = {
> > > +	.clock = 555560,
> > > +	.mpllb = { 0x1104,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0x0a06,	/* mpllb cfg2 */
> > > +		   0xbe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0003,	/* mpllb cfg7 */
> > > +		   0x3555,	/* mpllb cfg8 */
> > > +		   0x0001,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state mtl_c20_hdmi_1200 = {
> > > +	.clock = 666670,
> > > +	.mpllb = { 0x0138,	/* mpllb cfg0 */
> > > +		   0x7d08,	/* mpllb cfg1 */
> > > +		   0x5486,	/* mpllb cfg2 */
> > > +		   0xfe40,	/* mpllb cfg3 */
> > > +		   0x0000,	/* mpllb cfg4 */
> > > +		   0x0000,	/* mpllb cfg5 */
> > > +		   0x2200,	/* mpllb cfg6 */
> > > +		   0x0001,	/* mpllb cfg7 */
> > > +		   0x4000,	/* mpllb cfg8 */
> > > +		   0x0000,	/* mpllb cfg9 */
> > > +		   0x0004,	/* mpllb cfg10 */
> > > +		},
> > > +};
> > > +
> > > +static const struct intel_c20pll_state * const mtl_c20_hdmi_tables[] = {
> > > +	&mtl_c20_hdmi_25_175,
> > > +	&mtl_c20_hdmi_27_0,
> > > +	&mtl_c20_hdmi_74_25,
> > > +	&mtl_c20_hdmi_148_5,
> > > +	&mtl_c20_hdmi_594,
> > > +	&mtl_c20_hdmi_300,
> > > +	&mtl_c20_hdmi_600,
> > > +	&mtl_c20_hdmi_800,
> > > +	&mtl_c20_hdmi_1000,
> > > +	&mtl_c20_hdmi_1200,
> > > +	NULL,
> > > +};
> > > +
> > > +static int intel_c10_phy_check_hdmi_link_rate(int clock)
> > >  {
> > >  	const struct intel_c10mpllb_state * const *tables =
> > mtl_c10_hdmi_tables;
> > >  	int i;
> > > @@ -1128,6 +1506,17 @@ int intel_c10_phy_check_hdmi_link_rate(int clock)
> > >  	return MODE_CLOCK_RANGE;
> > >  }
> > >
> > > +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int
> > > +clock) {
> > > +	struct intel_digital_port *dig_port = hdmi_to_dig_port(hdmi);
> > > +	struct drm_i915_private *i915 = intel_hdmi_to_i915(hdmi);
> > > +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> > > +
> > > +	if (intel_is_c10phy(i915, phy))
> > > +		return intel_c10_phy_check_hdmi_link_rate(clock);
> > > +	return intel_c20_phy_check_hdmi_link_rate(clock);
> > > +}
> > > +
> > >  static const struct intel_c10mpllb_state * const *
> > > intel_c10_mpllb_tables_get(struct intel_crtc_state *crtc_state,
> > >  			   struct intel_encoder *encoder)
> > > @@ -1312,6 +1701,53 @@ 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]);
> > }
> > >
> > > +int intel_c20_phy_check_hdmi_link_rate(int clock) {
> > > +        const struct intel_c20pll_state * const *tables = mtl_c20_hdmi_tables;
> > > +        int i;
> > > +
> > > +        for (i = 0; tables[i]; i++) {
> > > +                if (clock == tables[i]->clock)
> > > +                        return MODE_OK;
> > > +        }
> > > +
> > > +        return MODE_CLOCK_RANGE;
> > > +}
> > > +
> > > +static const struct intel_c20pll_state * const *
> > > +intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
> > > +			 struct intel_encoder *encoder)
> > > +{
> > > +	if (intel_crtc_has_dp_encoder(crtc_state)) {
> > > +		return mtl_c20_dp_tables;
> > > +	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> > > +		return mtl_c20_hdmi_tables;
> > > +	}
> > > +
> > > +	MISSING_CASE(encoder->type);
> > > +	return NULL;
> > > +}
> > > +
> > > +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> > > +	                    struct intel_encoder *encoder)
> > 
> > Looks like this function should be static: the only user is intel_cx0_phy.c.
> > 
> > > +{
> > > +	const struct intel_c20pll_state * const *tables;
> > > +	int i;
> > > +
> > > +	tables = intel_c20_pll_tables_get(crtc_state, encoder);
> > > +	if (!tables)
> > > +		return -EINVAL;
> > > +
> > > +	for (i = 0; tables[i]; i++) {
> > > +		if (crtc_state->port_clock <= tables[i]->clock) {
> > > +			crtc_state->cx0pll_state.c20pll_state = *tables[i];
> > > +			return 0;
> > > +		}
> > > +	}
> > > +
> > > +	return -EINVAL;
> > > +}
> > > +
> > >  static bool intel_c20_use_mplla(u32 clock)  {
> > >  	/* 10G and 20G rates use MPLLA */
> > > @@ -1321,6 +1757,63 @@ static bool intel_c20_use_mplla(u32 clock)
> > >  	return false;
> > >  }
> > >
> > > +void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> > > +				   struct intel_c20pll_state *pll_state) {
> > > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > +	bool cntx, use_mplla;
> > > +	u32 val;
> > > +	int i;
> > > +
> > > +        /* 1. Read current context selection */
> > > +        cntx = intel_cx0_read(i915, encoder->port, 0,
> > > + PHY_C20_VDR_CUSTOM_SERDES_RATE) & PHY_C20_CONTEXT_TOGGLE;
> > > +
> > > +	/* Read Tx configuration */
> > > +	for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) {
> > > +		if (cntx)
> > > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> > > +
> > PHY_C20_B_TX_CNTX_CFG(i));
> > > +		else
> > > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
> > > +
> > PHY_C20_A_TX_CNTX_CFG(i));
> > > +	}
> > > +
> > > +	/* Read common configuration */
> > > +	for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) {
> > > +		if (cntx)
> > > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port,
> > 0,
> > > +
> > PHY_C20_B_CMN_CNTX_CFG(i));
> > > +		else
> > > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port,
> > 0,
> > > +
> > PHY_C20_A_CMN_CNTX_CFG(i));
> > > +	}
> > > +
> > > +	val = intel_c20_read(i915, encoder->port, 0,
> > PHY_C20_A_MPLLA_CNTX_CFG(6));
> > > +	use_mplla = val & C20_MPLLB_FRACEN;
> > 
> > Some questions about this:
> > 
> >   1. This is hardcoded to read from context A. Shouldn't we read from the
> >      current context?
> > 
> >   2. s/C20_MPLLB_FRACEN/C20_MPLLA_FRACEN/ ?
> The both uses the same bit 13 for frac_en. Maybe just renaming differently C10_MPLLx_FRACEN?
> 
> > 
> >   3. When we are programming PLL for MPLLB, we are not clearing
> >      PHY_C20_A_MPLLA_CNTX_CFG(6). Wouldn't this be problematic if MPLLB is
> > the
> >      current one in use MPLLB but MPLLA was already used in a previous
> >      configuration?
> Do you mean this when we are switching the context? In this case, as far as I have understood the spec, we wouldn't need to clear previous configuration.

Hi, Mika. Sorry to taking so long to reply!

What I mean is as follows. Consider the sequence of events below for an example:

  1. For a certain PLL programming, context A was used and MPLLA was selected.
  2. For a new PLL programming, context B is used and we are not clearing
     PHY_C20_A_MPLLA_CNTX_CFG(6).
  3. Context A is used again for the next PLL programming, but this time MPLLB
     is selected.

My concern is the following: *If* PHY_C20_A_MPLLA_CNTX_CFG(6) is not
automatically cleared by the hardware in event (2), then after (3) this function
would still think the current configuration is using MPLLA while it is in fact
using MPLLB.

Now, if we have guarantee that PHY_C20_A_MPLLA_CNTX_CFG(6) is reset
automatically when we switch to context B, then we wouldn't have to worry here.

--
Gustavo Sousa

> 
> > 
> > > +
> > > +	if (use_mplla) {
> > > +		/* MPLLA configuration */
> > > +		for (i = 0; i < ARRAY_SIZE(pll_state->mplla); i++) {
> > > +			if (cntx)
> > > +				pll_state->mplla[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_B_MPLLA_CNTX_CFG(i));
> > > +			else
> > > +				pll_state->mplla[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_A_MPLLA_CNTX_CFG(i));
> > > +		}
> > > +	} else {
> > > +		/* MPLLB configuration */
> > > +		for (i = 0; i < ARRAY_SIZE(pll_state->mpllb); i++) {
> > > +			if (cntx)
> > > +				pll_state->mpllb[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_B_MPLLB_CNTX_CFG(i));
> > > +			else
> > > +				pll_state->mpllb[i] = intel_c20_read(i915,
> > encoder->port, 0,
> > > +
> > PHY_C20_A_MPLLB_CNTX_CFG(i));
> > > +		}
> > > +	}
> > > +}
> > > +
> > >  static u8 intel_c20_get_dp_rate(u32 clock)  {
> > >  	switch (clock) {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > index 952c7deeffaa..86edbc4b1718 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> > > @@ -33,14 +33,22 @@ void intel_c10mpllb_readout_hw_state(struct
> > intel_encoder *encoder,
> > >  				     struct intel_c10mpllb_state *pll_state);  int
> > > intel_cx0mpllb_calc_state(struct intel_crtc_state *crtc_state,
> > >  			      struct intel_encoder *encoder);
> > > +int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> > > +			    struct intel_encoder *encoder); void
> > > +intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> > > +                                   struct intel_c20pll_state
> > > +*pll_state);
> > >  void intel_c10mpllb_dump_hw_state(struct drm_i915_private *dev_priv,
> > >  				  const struct intel_c10mpllb_state *hw_state);
> > int
> > > intel_c10mpllb_calc_port_clock(struct intel_encoder *encoder,
> > >  				   const struct intel_c10mpllb_state *pll_state);
> > void
> > > intel_c10mpllb_state_verify(struct intel_atomic_state *state,
> > >  				 struct intel_crtc_state *new_crtc_state); -int
> > > intel_c10_phy_check_hdmi_link_rate(int clock);
> > > +int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int
> > > +clock);
> > >  void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
> > >  				     const struct intel_crtc_state *crtc_state);
> > >
> > > +int intel_c20_phy_check_hdmi_link_rate(int clock); void
> > > +intel_cx0_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > +				       const struct intel_crtc_state *crtc_state,
> > > +				       u32 level);
> > >  #endif /* __INTEL_CX0_PHY_H__ */
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index a5fbc4fed28d..c37f7f7d84cf 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -3532,10 +3532,13 @@ static void mtl_ddi_get_config(struct
> > intel_encoder *encoder,
> > >  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > >  	enum phy phy = intel_port_to_phy(i915, encoder->port);
> > >
> > > -	drm_WARN_ON(&i915->drm, !intel_is_c10phy(i915, phy));
> > > +	if (intel_is_c10phy(i915, phy)) {
> > > +		intel_c10mpllb_readout_hw_state(encoder, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > > +		intel_c10mpllb_dump_hw_state(i915, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > > +	} else {
> > > +		intel_c20pll_readout_hw_state(encoder, &crtc_state-
> > >cx0pll_state.c20pll_state);
> > > +	}
> > >
> > > -	intel_c10mpllb_readout_hw_state(encoder, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > > -	intel_c10mpllb_dump_hw_state(i915, &crtc_state-
> > >cx0pll_state.c10mpllb_state);
> > >  	crtc_state->port_clock = intel_c10mpllb_calc_port_clock(encoder,
> > > &crtc_state->cx0pll_state.c10mpllb_state);
> > >
> > >  	intel_ddi_get_config(encoder, crtc_state); diff --git
> > > a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > index 7b81781a64b6..e60fcd477091 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > @@ -58,7 +58,7 @@
> > >  #include "intel_panel.h"
> > >  #include "intel_snps_phy.h"
> > >
> > > -static struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi
> > > *intel_hdmi)
> > > +inline struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi
> > > +*intel_hdmi)
> > >  {
> > >  	return to_i915(hdmi_to_dig_port(intel_hdmi)->base.base.dev);
> > >  }
> > > @@ -1883,8 +1883,8 @@ hdmi_port_clock_valid(struct intel_hdmi *hdmi,
> > >  	 * FIXME: We will hopefully get an algorithmic way of programming
> > >  	 * the MPLLB for HDMI in the future.
> > >  	 */
> > > -	if (IS_METEORLAKE(dev_priv))
> > > -		return intel_c10_phy_check_hdmi_link_rate(clock);
> > > +	if (DISPLAY_VER(dev_priv) >= 14)
> > > +		return intel_cx0_phy_check_hdmi_link_rate(hdmi, clock);
> > >  	else if (IS_DG2(dev_priv))
> > >  		return intel_snps_phy_check_hdmi_link_rate(clock);
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > b/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > index 774dda2376ed..492bd3921385 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> > > @@ -54,5 +54,6 @@ int intel_hdmi_dsc_get_num_slices(const struct
> > intel_crtc_state *crtc_state,
> > >  				  int src_max_slices, int src_max_slice_width,
> > >  				  int hdmi_max_slices, int hdmi_throughput);
> > int
> > > intel_hdmi_dsc_get_slice_height(int vactive);
> > > +struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi
> > > +*intel_hdmi);
> > >
> > >  #endif /* __INTEL_HDMI_H__ */
> > > --
> > > 2.34.1
> > >

Imre Deak March 27, 2023, 9:07 a.m. UTC | #5

On Fri, Mar 24, 2023 at 05:51:11PM -0300, Gustavo Sousa wrote:
> On Thu, Feb 23, 2023 at 06:47:27AM -0300, Kahola, Mika wrote:
> > > > [...]
> > > > +void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
> > > > +				   struct intel_c20pll_state *pll_state) {
> > > > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > > +	bool cntx, use_mplla;
> > > > +	u32 val;
> > > > +	int i;
> > > > +
> > > > +        /* 1. Read current context selection */
> > > > +        cntx = intel_cx0_read(i915, encoder->port, 0, PHY_C20_VDR_CUSTOM_SERDES_RATE) & PHY_C20_CONTEXT_TOGGLE;
> > > > +
> > > > +	/* Read Tx configuration */
> > > > +	for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) {
> > > > +		if (cntx)
> > > > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0, PHY_C20_B_TX_CNTX_CFG(i));
> > > > +		else
> > > > +			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0, PHY_C20_A_TX_CNTX_CFG(i));
> > > > +	}
> > > > +
> > > > +	/* Read common configuration */
> > > > +	for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) {
> > > > +		if (cntx)
> > > > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port, 0, PHY_C20_B_CMN_CNTX_CFG(i));
> > > > +		else
> > > > +			pll_state->cmn[i] = intel_c20_read(i915, encoder->port, 0, PHY_C20_A_CMN_CNTX_CFG(i));
> > > > +	}
> > > > +
> > > > +	val = intel_c20_read(i915, encoder->port, 0, PHY_C20_A_MPLLA_CNTX_CFG(6));
> > > > +	use_mplla = val & C20_MPLLB_FRACEN;
> > > 
> > >
> > > Some questions about this:
> > > 
> > >   1. This is hardcoded to read from context A. Shouldn't we read from the
> > >      current context?
> > > 
> > >   2. s/C20_MPLLB_FRACEN/C20_MPLLA_FRACEN/ ?
> >
> > The both uses the same bit 13 for frac_en. Maybe just renaming
> > differently C10_MPLLx_FRACEN?
> > > 
> > >   3. When we are programming PLL for MPLLB, we are not clearing
> > >      PHY_C20_A_MPLLA_CNTX_CFG(6). Wouldn't this be problematic if MPLLB is the
> > >      current one in use MPLLB but MPLLA was already used in a previous
> > >      configuration?
> >
> > Do you mean this when we are switching the context? In this case, as
> > far as I have understood the spec, we wouldn't need to clear
> > previous configuration.
> 
> Hi, Mika. Sorry to taking so long to reply!
> 
> What I mean is as follows. Consider the sequence of events below for an example:
> 
>   1. For a certain PLL programming, context A was used and MPLLA was selected.
>   2. For a new PLL programming, context B is used and we are not clearing
>      PHY_C20_A_MPLLA_CNTX_CFG(6).
>   3. Context A is used again for the next PLL programming, but this time MPLLB
>      is selected.
> 
> My concern is the following: *If* PHY_C20_A_MPLLA_CNTX_CFG(6) is not
> automatically cleared by the hardware in event (2), then after (3) this function
> would still think the current configuration is using MPLLA while it is in fact
> using MPLLB.
> 
> Now, if we have guarantee that PHY_C20_A_MPLLA_CNTX_CFG(6) is reset
> automatically when we switch to context B, then we wouldn't have to
> worry here.

Why the PLL's FRACEN flag used here? It's a detail in how the PLL is
programmed and may be either 0 or 1 (even if it's now happens to be 1).
I think PHY_C20_<cnxt>_TX_CNTX_CFG_0[7] should be used which is txX_mpllb_sel
(see Bspec 68862, C20 TX Context programming).

--Imre

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 022888050a68..285e4cdd23eb 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -11,6 +11,7 @@ 
 #include "intel_de.h"
 #include "intel_display_types.h"
 #include "intel_dp.h"
+#include "intel_hdmi.h"
 #include "intel_panel.h"
 #include "intel_psr.h"
 #include "intel_uncore.h"
@@ -247,6 +248,23 @@  static void intel_c20_write(struct drm_i915_private *i915, enum port port,
 	intel_cx0_write(i915, port, lane, PHY_C20_WR_DATA_L, data & 0xff, 1);
 }
 
+static u16 intel_c20_read(struct drm_i915_private *i915, enum port port,
+                          int lane, u16 addr)
+{
+       u16 val;
+
+       assert_dc_off(i915);
+
+       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_L, addr & 0xff, 0);
+       intel_cx0_write(i915, port, lane, PHY_C20_RD_ADDRESS_H, (addr >> 8) & 0xff, 1);
+
+       val = intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_H);
+       val <<= 8;
+       val |= intel_cx0_read(i915, port, lane, PHY_C20_RD_DATA_L);
+
+        return val;
+}
+
 static void __intel_cx0_rmw(struct drm_i915_private *i915, enum port port,
 			    int lane, u16 addr, u8 clear, u8 set, bool committed)
 {
@@ -588,6 +606,192 @@  static const struct intel_c10mpllb_state * const mtl_c10_edp_tables[] = {
 	NULL,
 };
 
+/* C20 basic DP 1.4 tables */
+static const struct intel_c20pll_state mtl_c20_dp_rbr = {
+	.clock = 162000,
+	.tx = {	0xbe88, /* tx cfg0 */
+		0x5800, /* tx cfg1 */
+		0x0000, /* tx cfg2 */
+		},
+	.cmn = {0x0500, /* cmn cfg0*/
+		0x0005, /* cmn cfg1 */
+		0x0000, /* cmn cfg2 */
+		0x0000, /* cmn cfg3 */
+		},
+	.mpllb = { 0x50a8,	/* mpllb cfg0 */
+		0x2120,		/* mpllb cfg1 */
+		0xcd9a,		/* mpllb cfg2 */
+		0xbfc1,		/* mpllb cfg3 */
+		0x5ab8,         /* mpllb cfg4 */
+		0x4c34,         /* mpllb cfg5 */
+		0x2000,		/* mpllb cfg6 */
+		0x0001,		/* mpllb cfg7 */
+		0x6000,		/* mpllb cfg8 */
+		0x0000,		/* mpllb cfg9 */
+		0x0000,		/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_dp_hbr1 = {
+	.clock = 270000,
+	.tx = {	0xbe88, /* tx cfg0 */
+		0x4800, /* tx cfg1 */
+		0x0000, /* tx cfg2 */
+		},
+	.cmn = {0x0500, /* cmn cfg0*/
+		0x0005, /* cmn cfg1 */
+		0x0000, /* cmn cfg2 */
+		0x0000, /* cmn cfg3 */
+		},
+	.mpllb = { 0x308c,	/* mpllb cfg0 */
+		0x2110,		/* mpllb cfg1 */
+		0xcc9c,		/* mpllb cfg2 */
+		0xbfc1,		/* mpllb cfg3 */
+		0x489a,         /* mpllb cfg4 */
+		0x3f81,         /* mpllb cfg5 */
+		0x2000,		/* mpllb cfg6 */
+		0x0001,		/* mpllb cfg7 */
+		0x5000,		/* mpllb cfg8 */
+		0x0000,		/* mpllb cfg9 */
+		0x0000,		/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_dp_hbr2 = {
+	.clock = 540000,
+	.tx = {	0xbe88, /* tx cfg0 */
+		0x4800, /* tx cfg1 */
+		0x0000, /* tx cfg2 */
+		},
+	.cmn = {0x0500, /* cmn cfg0*/
+		0x0005, /* cmn cfg1 */
+		0x0000, /* cmn cfg2 */
+		0x0000, /* cmn cfg3 */
+		},
+	.mpllb = { 0x108c,	/* mpllb cfg0 */
+		0x2108,		/* mpllb cfg1 */
+		0xcc9c,		/* mpllb cfg2 */
+		0xbfc1,		/* mpllb cfg3 */
+		0x489a,         /* mpllb cfg4 */
+		0x3f81,         /* mpllb cfg5 */
+		0x2000,		/* mpllb cfg6 */
+		0x0001,		/* mpllb cfg7 */
+		0x5000,		/* mpllb cfg8 */
+		0x0000,		/* mpllb cfg9 */
+		0x0000,		/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_dp_hbr3 = {
+	.clock = 810000,
+	.tx = {	0xbe88, /* tx cfg0 */
+		0x4800, /* tx cfg1 */
+		0x0000, /* tx cfg2 */
+		},
+	.cmn = {0x0500, /* cmn cfg0*/
+		0x0005, /* cmn cfg1 */
+		0x0000, /* cmn cfg2 */
+		0x0000, /* cmn cfg3 */
+		},
+	.mpllb = { 0x10d2,	/* mpllb cfg0 */
+		0x2108,		/* mpllb cfg1 */
+		0x8d98,		/* mpllb cfg2 */
+		0xbfc1,		/* mpllb cfg3 */
+		0x7166,         /* mpllb cfg4 */
+		0x5f42,         /* mpllb cfg5 */
+		0x2000,		/* mpllb cfg6 */
+		0x0001,		/* mpllb cfg7 */
+		0x7800,		/* mpllb cfg8 */
+		0x0000,		/* mpllb cfg9 */
+		0x0000,		/* mpllb cfg10 */
+		},
+};
+
+/* C20 basic DP 2.0 tables */
+static const struct intel_c20pll_state mtl_c20_dp_uhbr10 = {
+	.clock = 312500,
+	.tx = {	0xbe21, /* tx cfg0 */
+		0x4800, /* tx cfg1 */
+		0x0000, /* tx cfg2 */
+		},
+	.cmn = {0x0500, /* cmn cfg0*/
+		0x0005, /* cmn cfg1 */
+		0x0000, /* cmn cfg2 */
+		0x0000, /* cmn cfg3 */
+		},
+	.mplla = { 0x3104,	/* mplla cfg0 */
+		0xd105,		/* mplla cfg1 */
+		0xc025,		/* mplla cfg2 */
+		0xc025,		/* mplla cfg3 */
+		0xa6ab,		/* mplla cfg4 */
+		0x8c00,		/* mplla cfg5 */
+		0x4000,		/* mplla cfg6 */
+		0x0003,		/* mplla cfg7 */
+		0x3555,		/* mplla cfg8 */
+		0x0001,		/* mplla cfg9 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_dp_uhbr13_5 = {
+	.clock = 421875,
+	.tx = {	0xbea0, /* tx cfg0 */
+		0x4800, /* tx cfg1 */
+		0x0000, /* tx cfg2 */
+		},
+	.cmn = {0x0500, /* cmn cfg0*/
+		0x0005, /* cmn cfg1 */
+		0x0000, /* cmn cfg2 */
+		0x0000, /* cmn cfg3 */
+		},
+	.mpllb = { 0x015f,	/* mpllb cfg0 */
+		0x2205,		/* mpllb cfg1 */
+		0x1b17,		/* mpllb cfg2 */
+		0xffc1,		/* mpllb cfg3 */
+		0xe100,		/* mpllb cfg4 */
+		0xbd00,		/* mpllb cfg5 */
+		0x2000,		/* mpllb cfg6 */
+		0x0001,		/* mpllb cfg7 */
+		0x4800,		/* mpllb cfg8 */
+		0x0000,		/* mpllb cfg9 */
+		0x0000,		/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_dp_uhbr20 = {
+	.clock = 625000,
+	.tx = {	0xbe20, /* tx cfg0 */
+		0x4800, /* tx cfg1 */
+		0x0000, /* tx cfg2 */
+		},
+	.cmn = {0x0500, /* cmn cfg0*/
+		0x0005, /* cmn cfg1 */
+		0x0000, /* cmn cfg2 */
+		0x0000, /* cmn cfg3 */
+		},
+	.mplla = { 0x3104,	/* mplla cfg0 */
+		0xd105,		/* mplla cfg1 */
+		0xc025,		/* mplla cfg2 */
+		0xc025,		/* mplla cfg3 */
+		0xa6ab,		/* mplla cfg4 */
+		0x8c00,		/* mplla cfg5 */
+		0x4000,		/* mplla cfg6 */
+		0x0003,		/* mplla cfg7 */
+		0x3555,		/* mplla cfg8 */
+		0x0001,		/* mplla cfg9 */
+		},
+};
+
+static const struct intel_c20pll_state * const mtl_c20_dp_tables[] = {
+        &mtl_c20_dp_rbr,
+        &mtl_c20_dp_hbr1,
+        &mtl_c20_dp_hbr2,
+        &mtl_c20_dp_hbr3,
+	&mtl_c20_dp_uhbr10,
+	&mtl_c20_dp_uhbr13_5,
+	&mtl_c20_dp_uhbr20,
+        NULL,
+};
+
 /*
  * HDMI link rates with 38.4 MHz reference clock.
  */
@@ -1115,7 +1319,181 @@  static const struct intel_c10mpllb_state * const mtl_c10_hdmi_tables[] = {
 	NULL,
 };
 
-int intel_c10_phy_check_hdmi_link_rate(int clock)
+static const struct intel_c20pll_state mtl_c20_hdmi_25_175 = {
+	.clock = 25175,
+	.mpllb = { 0xa0d2,	/* mpllb cfg0 */
+		   0x7d80,	/* mpllb cfg1 */
+		   0x0906,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x0200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x0000,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0001,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_27_0 = {
+	.clock = 27000,
+	.mpllb = { 0xa0e0,	/* mpllb cfg0 */
+		   0x7d80,	/* mpllb cfg1 */
+		   0x0906,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x8000,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0001,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_74_25 = {
+	.clock = 74250,
+	.mpllb = { 0x609a,	/* mpllb cfg0 */
+		   0x7d40,	/* mpllb cfg1 */
+		   0xca06,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x5800,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0001,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_148_5 = {
+	.clock = 148500,
+	.mpllb = { 0x409a,	/* mpllb cfg0 */
+		   0x7d20,	/* mpllb cfg1 */
+		   0xca06,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x5800,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0001,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_594 = {
+	.clock = 594000,
+	.mpllb = { 0x009a,	/* mpllb cfg0 */
+		   0x7d08,	/* mpllb cfg1 */
+		   0xca06,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x5800,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0001,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_300 = {
+	.clock = 166670,
+	.mpllb = { 0x209c,	/* mpllb cfg0 */
+		   0x7d10,	/* mpllb cfg1 */
+		   0xca06,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x2000,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0004,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_600 = {
+	.clock = 333330,
+	.mpllb = { 0x009c,	/* mpllb cfg0 */
+		   0x7d08,	/* mpllb cfg1 */
+		   0xca06,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x2000,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0004,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_800 = {
+	.clock = 444440,
+	.mpllb = { 0x00d0,	/* mpllb cfg0 */
+		   0x7d08,	/* mpllb cfg1 */
+		   0x4a06,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0003,	/* mpllb cfg7 */
+		   0x2aaa,	/* mpllb cfg8 */
+		   0x0002,	/* mpllb cfg9 */
+		   0x0004,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_1000 = {
+	.clock = 555560,
+	.mpllb = { 0x1104,	/* mpllb cfg0 */
+		   0x7d08,	/* mpllb cfg1 */
+		   0x0a06,	/* mpllb cfg2 */
+		   0xbe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0003,	/* mpllb cfg7 */
+		   0x3555,	/* mpllb cfg8 */
+		   0x0001,	/* mpllb cfg9 */
+		   0x0004,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state mtl_c20_hdmi_1200 = {
+	.clock = 666670,
+	.mpllb = { 0x0138,	/* mpllb cfg0 */
+		   0x7d08,	/* mpllb cfg1 */
+		   0x5486,	/* mpllb cfg2 */
+		   0xfe40,	/* mpllb cfg3 */
+		   0x0000,	/* mpllb cfg4 */
+		   0x0000,	/* mpllb cfg5 */
+		   0x2200,	/* mpllb cfg6 */
+		   0x0001,	/* mpllb cfg7 */
+		   0x4000,	/* mpllb cfg8 */
+		   0x0000,	/* mpllb cfg9 */
+		   0x0004,	/* mpllb cfg10 */
+		},
+};
+
+static const struct intel_c20pll_state * const mtl_c20_hdmi_tables[] = {
+	&mtl_c20_hdmi_25_175,
+	&mtl_c20_hdmi_27_0,
+	&mtl_c20_hdmi_74_25,
+	&mtl_c20_hdmi_148_5,
+	&mtl_c20_hdmi_594,
+	&mtl_c20_hdmi_300,
+	&mtl_c20_hdmi_600,
+	&mtl_c20_hdmi_800,
+	&mtl_c20_hdmi_1000,
+	&mtl_c20_hdmi_1200,
+	NULL,
+};
+
+static int intel_c10_phy_check_hdmi_link_rate(int clock)
 {
 	const struct intel_c10mpllb_state * const *tables = mtl_c10_hdmi_tables;
 	int i;
@@ -1128,6 +1506,17 @@  int intel_c10_phy_check_hdmi_link_rate(int clock)
 	return MODE_CLOCK_RANGE;
 }
 
+int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int clock)
+{
+	struct intel_digital_port *dig_port = hdmi_to_dig_port(hdmi);
+	struct drm_i915_private *i915 = intel_hdmi_to_i915(hdmi);
+	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
+
+	if (intel_is_c10phy(i915, phy))
+		return intel_c10_phy_check_hdmi_link_rate(clock);
+	return intel_c20_phy_check_hdmi_link_rate(clock);
+}
+
 static const struct intel_c10mpllb_state * const *
 intel_c10_mpllb_tables_get(struct intel_crtc_state *crtc_state,
 			   struct intel_encoder *encoder)
@@ -1312,6 +1701,53 @@  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]);
 }
 
+int intel_c20_phy_check_hdmi_link_rate(int clock)
+{
+        const struct intel_c20pll_state * const *tables = mtl_c20_hdmi_tables;
+        int i;
+
+        for (i = 0; tables[i]; i++) {
+                if (clock == tables[i]->clock)
+                        return MODE_OK;
+        }
+
+        return MODE_CLOCK_RANGE;
+}
+
+static const struct intel_c20pll_state * const *
+intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
+			 struct intel_encoder *encoder)
+{
+	if (intel_crtc_has_dp_encoder(crtc_state)) {
+		return mtl_c20_dp_tables;
+	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
+		return mtl_c20_hdmi_tables;
+	}
+
+	MISSING_CASE(encoder->type);
+	return NULL;
+}
+
+int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
+	                    struct intel_encoder *encoder)
+{
+	const struct intel_c20pll_state * const *tables;
+	int i;
+
+	tables = intel_c20_pll_tables_get(crtc_state, encoder);
+	if (!tables)
+		return -EINVAL;
+
+	for (i = 0; tables[i]; i++) {
+		if (crtc_state->port_clock <= tables[i]->clock) {
+			crtc_state->cx0pll_state.c20pll_state = *tables[i];
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static bool intel_c20_use_mplla(u32 clock)
 {
 	/* 10G and 20G rates use MPLLA */
@@ -1321,6 +1757,63 @@  static bool intel_c20_use_mplla(u32 clock)
 	return false;
 }
 
+void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
+				   struct intel_c20pll_state *pll_state)
+{
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	bool cntx, use_mplla;
+	u32 val;
+	int i;
+
+        /* 1. Read current context selection */
+        cntx = intel_cx0_read(i915, encoder->port, 0, PHY_C20_VDR_CUSTOM_SERDES_RATE) & PHY_C20_CONTEXT_TOGGLE;
+
+	/* Read Tx configuration */
+	for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) {
+		if (cntx)
+			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
+							  PHY_C20_B_TX_CNTX_CFG(i));
+		else
+			pll_state->tx[i] = intel_c20_read(i915, encoder->port, 0,
+							  PHY_C20_A_TX_CNTX_CFG(i));
+	}
+
+	/* Read common configuration */
+	for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) {
+		if (cntx)
+			pll_state->cmn[i] = intel_c20_read(i915, encoder->port, 0,
+							   PHY_C20_B_CMN_CNTX_CFG(i));
+		else
+			pll_state->cmn[i] = intel_c20_read(i915, encoder->port, 0,
+							   PHY_C20_A_CMN_CNTX_CFG(i));
+	}
+
+	val = intel_c20_read(i915, encoder->port, 0, PHY_C20_A_MPLLA_CNTX_CFG(6));
+	use_mplla = val & C20_MPLLB_FRACEN;
+
+	if (use_mplla) {
+		/* MPLLA configuration */
+		for (i = 0; i < ARRAY_SIZE(pll_state->mplla); i++) {
+			if (cntx)
+				pll_state->mplla[i] = intel_c20_read(i915, encoder->port, 0,
+								     PHY_C20_B_MPLLA_CNTX_CFG(i));
+			else
+				pll_state->mplla[i] = intel_c20_read(i915, encoder->port, 0,
+								     PHY_C20_A_MPLLA_CNTX_CFG(i));
+		}
+	} else {
+		/* MPLLB configuration */
+		for (i = 0; i < ARRAY_SIZE(pll_state->mpllb); i++) {
+			if (cntx)
+				pll_state->mpllb[i] = intel_c20_read(i915, encoder->port, 0,
+								     PHY_C20_B_MPLLB_CNTX_CFG(i));
+			else
+				pll_state->mpllb[i] = intel_c20_read(i915, encoder->port, 0,
+								     PHY_C20_A_MPLLB_CNTX_CFG(i));
+		}
+	}
+}
+
 static u8 intel_c20_get_dp_rate(u32 clock)
 {
 	switch (clock) {
diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.h b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
index 952c7deeffaa..86edbc4b1718 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
@@ -33,14 +33,22 @@  void intel_c10mpllb_readout_hw_state(struct intel_encoder *encoder,
 				     struct intel_c10mpllb_state *pll_state);
 int intel_cx0mpllb_calc_state(struct intel_crtc_state *crtc_state,
 			      struct intel_encoder *encoder);
+int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
+			    struct intel_encoder *encoder);
+void intel_c20pll_readout_hw_state(struct intel_encoder *encoder,
+                                   struct intel_c20pll_state *pll_state);
 void intel_c10mpllb_dump_hw_state(struct drm_i915_private *dev_priv,
 				  const struct intel_c10mpllb_state *hw_state);
 int intel_c10mpllb_calc_port_clock(struct intel_encoder *encoder,
 				   const struct intel_c10mpllb_state *pll_state);
 void intel_c10mpllb_state_verify(struct intel_atomic_state *state,
 				 struct intel_crtc_state *new_crtc_state);
-int intel_c10_phy_check_hdmi_link_rate(int clock);
+int intel_cx0_phy_check_hdmi_link_rate(struct intel_hdmi *hdmi, int clock);
 void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
 				     const struct intel_crtc_state *crtc_state);
 
+int intel_c20_phy_check_hdmi_link_rate(int clock);
+void intel_cx0_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
+				       const struct intel_crtc_state *crtc_state,
+				       u32 level);
 #endif /* __INTEL_CX0_PHY_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a5fbc4fed28d..c37f7f7d84cf 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3532,10 +3532,13 @@  static void mtl_ddi_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	enum phy phy = intel_port_to_phy(i915, encoder->port);
 
-	drm_WARN_ON(&i915->drm, !intel_is_c10phy(i915, phy));
+	if (intel_is_c10phy(i915, phy)) {
+		intel_c10mpllb_readout_hw_state(encoder, &crtc_state->cx0pll_state.c10mpllb_state);
+		intel_c10mpllb_dump_hw_state(i915, &crtc_state->cx0pll_state.c10mpllb_state);
+	} else {
+		intel_c20pll_readout_hw_state(encoder, &crtc_state->cx0pll_state.c20pll_state);
+	}
 
-	intel_c10mpllb_readout_hw_state(encoder, &crtc_state->cx0pll_state.c10mpllb_state);
-	intel_c10mpllb_dump_hw_state(i915, &crtc_state->cx0pll_state.c10mpllb_state);
 	crtc_state->port_clock = intel_c10mpllb_calc_port_clock(encoder, &crtc_state->cx0pll_state.c10mpllb_state);
 
 	intel_ddi_get_config(encoder, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 7b81781a64b6..e60fcd477091 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -58,7 +58,7 @@ 
 #include "intel_panel.h"
 #include "intel_snps_phy.h"
 
-static struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi *intel_hdmi)
+inline struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi *intel_hdmi)
 {
 	return to_i915(hdmi_to_dig_port(intel_hdmi)->base.base.dev);
 }
@@ -1883,8 +1883,8 @@  hdmi_port_clock_valid(struct intel_hdmi *hdmi,
 	 * FIXME: We will hopefully get an algorithmic way of programming
 	 * the MPLLB for HDMI in the future.
 	 */
-	if (IS_METEORLAKE(dev_priv))
-		return intel_c10_phy_check_hdmi_link_rate(clock);
+	if (DISPLAY_VER(dev_priv) >= 14)
+		return intel_cx0_phy_check_hdmi_link_rate(hdmi, clock);
 	else if (IS_DG2(dev_priv))
 		return intel_snps_phy_check_hdmi_link_rate(clock);
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
index 774dda2376ed..492bd3921385 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.h
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
@@ -54,5 +54,6 @@  int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
 				  int src_max_slices, int src_max_slice_width,
 				  int hdmi_max_slices, int hdmi_throughput);
 int intel_hdmi_dsc_get_slice_height(int vactive);
+struct drm_i915_private *intel_hdmi_to_i915(struct intel_hdmi *intel_hdmi);
 
 #endif /* __INTEL_HDMI_H__ */