Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 1/2] phy: freescale: fsl-samsung-hdmi: Replace register defines with macro
@ 2024-08-28  2:48 Adam Ford
  2024-08-28  2:48 ` [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider Adam Ford
  0 siblings, 1 reply; 5+ messages in thread
From: Adam Ford @ 2024-08-28  2:48 UTC (permalink / raw)
  To: linux-phy
  Cc: dominique.martinet, linux-imx, festevam, frieder.schrempf, aford,
	Adam Ford, Vinod Koul, Kishon Vijay Abraham I,
	Uwe Kleine-König, Marco Felsch, Lucas Stach, linux-kernel

There are 47 registers defined as PHY_REG_xx were xx goes from 00 to
47.  Simplify this by replacing them all with a macro which is passed
the register number to return the proper register offset.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index 9048cdc760c2..bc5d3625ece6 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -14,76 +14,20 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
-#define PHY_REG_00		0x00
-#define PHY_REG_01		0x04
-#define PHY_REG_02		0x08
-#define PHY_REG_08		0x20
-#define PHY_REG_09		0x24
-#define PHY_REG_10		0x28
-#define PHY_REG_11		0x2c
-
-#define PHY_REG_12		0x30
-#define  REG12_CK_DIV_MASK	GENMASK(5, 4)
-
-#define PHY_REG_13		0x34
-#define  REG13_TG_CODE_LOW_MASK	GENMASK(7, 0)
-
-#define PHY_REG_14		0x38
-#define  REG14_TOL_MASK		GENMASK(7, 4)
-#define  REG14_RP_CODE_MASK	GENMASK(3, 1)
-#define  REG14_TG_CODE_HIGH_MASK	GENMASK(0, 0)
-
-#define PHY_REG_15		0x3c
-#define PHY_REG_16		0x40
-#define PHY_REG_17		0x44
-#define PHY_REG_18		0x48
-#define PHY_REG_19		0x4c
-#define PHY_REG_20		0x50
-
-#define PHY_REG_21		0x54
-#define  REG21_SEL_TX_CK_INV	BIT(7)
-#define  REG21_PMS_S_MASK	GENMASK(3, 0)
-
-#define PHY_REG_22		0x58
-#define PHY_REG_23		0x5c
-#define PHY_REG_24		0x60
-#define PHY_REG_25		0x64
-#define PHY_REG_26		0x68
-#define PHY_REG_27		0x6c
-#define PHY_REG_28		0x70
-#define PHY_REG_29		0x74
-#define PHY_REG_30		0x78
-#define PHY_REG_31		0x7c
-#define PHY_REG_32		0x80
-
-/*
- * REG33 does not match the ref manual. According to Sandor Yu from NXP,
- * "There is a doc issue on the i.MX8MP latest RM"
- * REG33 is being used per guidance from Sandor
- */
-
-#define PHY_REG_33		0x84
-#define  REG33_MODE_SET_DONE	BIT(7)
-#define  REG33_FIX_DA		BIT(1)
-
-#define PHY_REG_34		0x88
-#define  REG34_PHY_READY	BIT(7)
-#define  REG34_PLL_LOCK		BIT(6)
-#define  REG34_PHY_CLK_READY	BIT(5)
-
-#define PHY_REG_35		0x8c
-#define PHY_REG_36		0x90
-#define PHY_REG_37		0x94
-#define PHY_REG_38		0x98
-#define PHY_REG_39		0x9c
-#define PHY_REG_40		0xa0
-#define PHY_REG_41		0xa4
-#define PHY_REG_42		0xa8
-#define PHY_REG_43		0xac
-#define PHY_REG_44		0xb0
-#define PHY_REG_45		0xb4
-#define PHY_REG_46		0xb8
-#define PHY_REG_47		0xbc
+#define PHY_REG(reg)		(reg * 4)
+
+#define REG12_CK_DIV_MASK	GENMASK(5, 4)
+#define REG13_TG_CODE_LOW_MASK	GENMASK(7, 0)
+#define REG14_TOL_MASK		GENMASK(7, 4)
+#define REG14_RP_CODE_MASK	GENMASK(3, 1)
+#define REG14_TG_CODE_HIGH_MASK	GENMASK(0, 0)
+#define REG21_SEL_TX_CK_INV	BIT(7)
+#define REG21_PMS_S_MASK	GENMASK(3, 0)
+#define REG33_MODE_SET_DONE	BIT(7)
+#define REG33_FIX_DA		BIT(1)
+#define REG34_PHY_READY	BIT(7)
+#define REG34_PLL_LOCK		BIT(6)
+#define REG34_PHY_CLK_READY	BIT(5)
 
 #define PHY_PLL_DIV_REGS_NUM 6
 
@@ -369,29 +313,29 @@ struct reg_settings {
 };
 
 static const struct reg_settings common_phy_cfg[] = {
-	{ PHY_REG_00, 0x00 }, { PHY_REG_01, 0xd1 },
-	{ PHY_REG_08, 0x4f }, { PHY_REG_09, 0x30 },
-	{ PHY_REG_10, 0x33 }, { PHY_REG_11, 0x65 },
+	{ PHY_REG(0), 0x00 }, { PHY_REG(1), 0xd1 },
+	{ PHY_REG(8), 0x4f }, { PHY_REG(9), 0x30 },
+	{ PHY_REG(10), 0x33 }, { PHY_REG(11), 0x65 },
 	/* REG12 pixclk specific */
 	/* REG13 pixclk specific */
 	/* REG14 pixclk specific */
-	{ PHY_REG_15, 0x80 }, { PHY_REG_16, 0x6c },
-	{ PHY_REG_17, 0xf2 }, { PHY_REG_18, 0x67 },
-	{ PHY_REG_19, 0x00 }, { PHY_REG_20, 0x10 },
+	{ PHY_REG(15), 0x80 }, { PHY_REG(16), 0x6c },
+	{ PHY_REG(17), 0xf2 }, { PHY_REG(18), 0x67 },
+	{ PHY_REG(19), 0x00 }, { PHY_REG(20), 0x10 },
 	/* REG21 pixclk specific */
-	{ PHY_REG_22, 0x30 }, { PHY_REG_23, 0x32 },
-	{ PHY_REG_24, 0x60 }, { PHY_REG_25, 0x8f },
-	{ PHY_REG_26, 0x00 }, { PHY_REG_27, 0x00 },
-	{ PHY_REG_28, 0x08 }, { PHY_REG_29, 0x00 },
-	{ PHY_REG_30, 0x00 }, { PHY_REG_31, 0x00 },
-	{ PHY_REG_32, 0x00 }, { PHY_REG_33, 0x80 },
-	{ PHY_REG_34, 0x00 }, { PHY_REG_35, 0x00 },
-	{ PHY_REG_36, 0x00 }, { PHY_REG_37, 0x00 },
-	{ PHY_REG_38, 0x00 }, { PHY_REG_39, 0x00 },
-	{ PHY_REG_40, 0x00 }, { PHY_REG_41, 0xe0 },
-	{ PHY_REG_42, 0x83 }, { PHY_REG_43, 0x0f },
-	{ PHY_REG_44, 0x3E }, { PHY_REG_45, 0xf8 },
-	{ PHY_REG_46, 0x00 }, { PHY_REG_47, 0x00 }
+	{ PHY_REG(22), 0x30 }, { PHY_REG(23), 0x32 },
+	{ PHY_REG(24), 0x60 }, { PHY_REG(25), 0x8f },
+	{ PHY_REG(26), 0x00 }, { PHY_REG(27), 0x00 },
+	{ PHY_REG(28), 0x08 }, { PHY_REG(29), 0x00 },
+	{ PHY_REG(30), 0x00 }, { PHY_REG(31), 0x00 },
+	{ PHY_REG(32), 0x00 }, { PHY_REG(33), 0x80 },
+	{ PHY_REG(34), 0x00 }, { PHY_REG(35), 0x00 },
+	{ PHY_REG(36), 0x00 }, { PHY_REG(37), 0x00 },
+	{ PHY_REG(38), 0x00 }, { PHY_REG(39), 0x00 },
+	{ PHY_REG(40), 0x00 }, { PHY_REG(41), 0xe0 },
+	{ PHY_REG(42), 0x83 }, { PHY_REG(43), 0x0f },
+	{ PHY_REG(44), 0x3E }, { PHY_REG(45), 0xf8 },
+	{ PHY_REG(46), 0x00 }, { PHY_REG(47), 0x00 }
 };
 
 struct fsl_samsung_hdmi_phy {
@@ -442,7 +386,7 @@ fsl_samsung_hdmi_phy_configure_pixclk(struct fsl_samsung_hdmi_phy *phy,
 	}
 
 	writeb(REG21_SEL_TX_CK_INV | FIELD_PREP(REG21_PMS_S_MASK, div),
-	       phy->regs + PHY_REG_21);
+	       phy->regs + PHY_REG(21));
 }
 
 static void
@@ -469,7 +413,7 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
 		break;
 	}
 
-	writeb(FIELD_PREP(REG12_CK_DIV_MASK, ilog2(div)), phy->regs + PHY_REG_12);
+	writeb(FIELD_PREP(REG12_CK_DIV_MASK, ilog2(div)), phy->regs + PHY_REG(12));
 
 	/*
 	 * Calculation for the frequency lock detector target code (fld_tg_code)
@@ -489,11 +433,11 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
 
 	/* FLD_TOL and FLD_RP_CODE taken from downstream driver */
 	writeb(FIELD_PREP(REG13_TG_CODE_LOW_MASK, fld_tg_code),
-	       phy->regs + PHY_REG_13);
+	       phy->regs + PHY_REG(13));
 	writeb(FIELD_PREP(REG14_TOL_MASK, 2) |
 	       FIELD_PREP(REG14_RP_CODE_MASK, 2) |
 	       FIELD_PREP(REG14_TG_CODE_HIGH_MASK, fld_tg_code >> 8),
-	       phy->regs + PHY_REG_14);
+	       phy->regs + PHY_REG(14));
 }
 
 static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
@@ -503,7 +447,7 @@ static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
 	u8 val;
 
 	/* HDMI PHY init */
-	writeb(REG33_FIX_DA, phy->regs + PHY_REG_33);
+	writeb(REG33_FIX_DA, phy->regs + PHY_REG(33));
 
 	/* common PHY registers */
 	for (i = 0; i < ARRAY_SIZE(common_phy_cfg); i++)
@@ -511,14 +455,14 @@ static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
 
 	/* set individual PLL registers PHY_REG2 ... PHY_REG7 */
 	for (i = 0; i < PHY_PLL_DIV_REGS_NUM; i++)
-		writeb(cfg->pll_div_regs[i], phy->regs + PHY_REG_02 + i * 4);
+		writeb(cfg->pll_div_regs[i], phy->regs + PHY_REG(2) + i * 4);
 
 	fsl_samsung_hdmi_phy_configure_pixclk(phy, cfg);
 	fsl_samsung_hdmi_phy_configure_pll_lock_det(phy, cfg);
 
-	writeb(REG33_FIX_DA | REG33_MODE_SET_DONE, phy->regs + PHY_REG_33);
+	writeb(REG33_FIX_DA | REG33_MODE_SET_DONE, phy->regs + PHY_REG(33));
 
-	ret = readb_poll_timeout(phy->regs + PHY_REG_34, val,
+	ret = readb_poll_timeout(phy->regs + PHY_REG(34), val,
 				 val & REG34_PLL_LOCK, 50, 20000);
 	if (ret)
 		dev_err(phy->dev, "PLL failed to lock\n");
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider
  2024-08-28  2:48 [RFC 1/2] phy: freescale: fsl-samsung-hdmi: Replace register defines with macro Adam Ford
@ 2024-08-28  2:48 ` Adam Ford
  2024-08-28  8:47   ` Dominique Martinet
  0 siblings, 1 reply; 5+ messages in thread
From: Adam Ford @ 2024-08-28  2:48 UTC (permalink / raw)
  To: linux-phy
  Cc: dominique.martinet, linux-imx, festevam, frieder.schrempf, aford,
	Adam Ford, Vinod Koul, Kishon Vijay Abraham I,
	Uwe Kleine-König, Marco Felsch, Lucas Stach, linux-kernel

There is currently a look-up table for a variety of resolutions.
Since the phy has the ability to dynamically calculate the values
necessary to use the intger divider which should allow more
resolutions without having to update the look-up-table.  If the
integer calculator cannot get an exact frequency, it falls back
to the look-up-table.  Because the LUT algorithm does some
rounding, I did not remove integer entries from the LUT.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index bc5d3625ece6..03b598f734e7 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -16,6 +16,8 @@
 
 #define PHY_REG(reg)		(reg * 4)
 
+#define REG01_PMS_P_MASK	GENMASK(3, 0)
+#define REG03_PMS_S_MASK	GENMASK(7, 4)
 #define REG12_CK_DIV_MASK	GENMASK(5, 4)
 #define REG13_TG_CODE_LOW_MASK	GENMASK(7, 0)
 #define REG14_TOL_MASK		GENMASK(7, 4)
@@ -31,6 +33,10 @@
 
 #define PHY_PLL_DIV_REGS_NUM 6
 
+#ifndef MHZ
+#define MHZ	(1000UL * 1000UL)
+#endif
+
 struct phy_config {
 	u32	pixclk;
 	u8	pll_div_regs[PHY_PLL_DIV_REGS_NUM];
@@ -440,10 +446,83 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
 	       phy->regs + PHY_REG(14));
 }
 
+static unsigned long fsl_samsung_hdmi_phy_find_pms(unsigned long fout, u8 *p, u16 *m, u8 *s)
+{
+	unsigned long best_freq = 0;
+	u32 min_delta = 0xffffffff;
+	u8 _p, best_p;
+	u16 _m, best_m;
+	u8 _s, best_s;
+
+	for (_p = 1; _p <= 11; ++_p) {
+		for (_s = 1; _s <= 16; ++_s) {
+			u64 tmp;
+			u32 delta;
+
+			/* s must be even */
+			if (_s > 1 && (_s & 0x01) == 1)
+				_s++;
+
+			/* _s cannot be 14 per the TRM */
+			if (_s == 14)
+				continue;
+
+			/*
+			 * TODO: Ref Manual doesn't state the range of _m
+			 * so this should be further refined if possible.
+			 * This range was set based on the original values
+			 * in the look-up table
+			 */
+			tmp = (u64)fout * (_p * _s);
+			do_div(tmp, 24 * MHZ);
+			_m = tmp;
+			if (_m < 0x30 || _m > 0x7b)
+				continue;
+
+			/*
+			 * Rev 2 of the Ref Manual states the
+			 * VCO can range between 750MHz and
+			 * 3GHz.  The VCO is assumed to be _m x
+			 * the reference frequency of 24MHz divided
+			 * by the prescaler, _p
+			 */
+			tmp = (u64)_m * 24 * MHZ;
+			do_div(tmp, _p);
+			if (tmp < 750 * MHZ ||
+			    tmp > 3000 * MHZ)
+				continue;
+
+			tmp = (u64)_m * 24 * MHZ;
+			do_div(tmp, _p * _s);
+
+			delta = abs(fout - tmp);
+			if (delta < min_delta) {
+				best_p = _p;
+				best_s = _s;
+				best_m = _m;
+				min_delta = delta;
+				best_freq = tmp;
+			}
+		}
+	}
+
+	if (best_freq) {
+		*p = best_p;
+		*m = best_m;
+		*s = best_s;
+	}
+
+	return best_freq;
+}
+
 static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
 					  const struct phy_config *cfg)
 {
+	u32 desired_clock = cfg->pixclk * 5;
+	u32 close_freq;
 	int i, ret;
+	u16 m;
+	u8 p, s;
 	u8 val;
 
 	/* HDMI PHY init */
@@ -453,11 +532,38 @@ static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
 	for (i = 0; i < ARRAY_SIZE(common_phy_cfg); i++)
 		writeb(common_phy_cfg[i].val, phy->regs + common_phy_cfg[i].reg);
 
-	/* set individual PLL registers PHY_REG2 ... PHY_REG7 */
-	for (i = 0; i < PHY_PLL_DIV_REGS_NUM; i++)
-		writeb(cfg->pll_div_regs[i], phy->regs + PHY_REG(2) + i * 4);
+	/* Using the PMS calculator alone, determine if can use integer divider */
+	close_freq = fsl_samsung_hdmi_phy_find_pms(desired_clock, &p, &m, &s);
+
+	/* If the clock cannot be configured with integer divder, use the fractional divider */
+	if (close_freq != desired_clock) {
+		dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: use fractional divider\n");
+		/* set individual PLL registers PHY_REG2 ... PHY_REG7 */
+		for (i = 0; i < PHY_PLL_DIV_REGS_NUM; i++)
+			writeb(cfg->pll_div_regs[i], phy->regs + PHY_REG(2) + i * 4);
+		fsl_samsung_hdmi_phy_configure_pixclk(phy, cfg);
+	} else {
+		dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: use integer divider\n");
+		dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: P = %d\n", p);
+		dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: M = %d\n", m);
+		dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: S = %d\n", s);
+		dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: frequency = %u\n", close_freq);
+
+		/* Write integer divder values for PMS */
+		writeb(FIELD_PREP(REG01_PMS_P_MASK, p), phy->regs + PHY_REG(1));
+		writeb(m, phy->regs + PHY_REG(2));
+		writeb(FIELD_PREP(REG03_PMS_S_MASK, s-1), phy->regs + PHY_REG(3));
+
+		/* Configure PHY to disable fractional divider */
+		writeb(0x00, phy->regs + PHY_REG(4));
+		writeb(0x00, phy->regs + PHY_REG(5));
+		writeb(0x80, phy->regs + PHY_REG(6));
+		writeb(0x00, phy->regs + PHY_REG(7));
+
+		writeb(REG21_SEL_TX_CK_INV | FIELD_PREP(REG21_PMS_S_MASK, s-1),
+		       phy->regs + PHY_REG(21));
+	}
 
-	fsl_samsung_hdmi_phy_configure_pixclk(phy, cfg);
 	fsl_samsung_hdmi_phy_configure_pll_lock_det(phy, cfg);
 
 	writeb(REG33_FIX_DA | REG33_MODE_SET_DONE, phy->regs + PHY_REG(33));
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider
  2024-08-28  2:48 ` [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider Adam Ford
@ 2024-08-28  8:47   ` Dominique Martinet
  2024-08-28 14:11     ` Adam Ford
  0 siblings, 1 reply; 5+ messages in thread
From: Dominique Martinet @ 2024-08-28  8:47 UTC (permalink / raw)
  To: Adam Ford
  Cc: linux-phy, linux-imx, festevam, frieder.schrempf, aford,
	Vinod Koul, Kishon Vijay Abraham I, Uwe Kleine-König,
	Marco Felsch, Lucas Stach, linux-kernel, Makoto Sato

Adam Ford wrote on Tue, Aug 27, 2024 at 09:48:02PM -0500:
> There is currently a look-up table for a variety of resolutions.
> Since the phy has the ability to dynamically calculate the values
> necessary to use the intger divider which should allow more
> resolutions without having to update the look-up-table.  If the
> integer calculator cannot get an exact frequency, it falls back
> to the look-up-table.  Because the LUT algorithm does some
> rounding, I did not remove integer entries from the LUT.

Thank you!

We're still running 5.10 so I backported the driver as of it's current
state first (that part works), unfortunately our 51.2MHz display does
not work with this.

After phy_clk_round_rate() not round the pixel clock to the table values
(otherwise we'd only get rounded values), and making phy_clk_set_rate()
pass the requested `rate` instead of using the next smaller cfg->pixclk,
the display no longer comes up.

It comes up with the values obtained for 50.4MHz (next closest value),
which also has an exact match so uses the integer divider this patch
computes instead of the table values, but not with the 51.2MHz it
requests...
I'm afraid at this point I don't know how to debug that further without
getting a scope out (I don't know if the soc isn't generating something
correct or if the display actually doesn't like the frequency it
requests?! the later could be checked by plugging it in to another PC
that might support that frequency...), and that is going to take quite a
while...

Hopefully Frieder will have more success with his displays?
It could also be very well due to some of the differences with our 5.10
tree, sorry about that.

>  static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
>  					  const struct phy_config *cfg)
>  {
> +	u32 desired_clock = cfg->pixclk * 5;

(I don't really understand where that `* 5` comes from, but I guess it's
expected? works for other display and neighbor frequency anyway...)

Thanks,
-- 
Dominique



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider
  2024-08-28  8:47   ` Dominique Martinet
@ 2024-08-28 14:11     ` Adam Ford
  2024-08-28 19:28       ` Adam Ford
  0 siblings, 1 reply; 5+ messages in thread
From: Adam Ford @ 2024-08-28 14:11 UTC (permalink / raw)
  To: Dominique Martinet
  Cc: linux-phy, linux-imx, festevam, frieder.schrempf, aford,
	Vinod Koul, Kishon Vijay Abraham I, Uwe Kleine-König,
	Marco Felsch, Lucas Stach, linux-kernel, Makoto Sato

On Wed, Aug 28, 2024 at 3:47 AM Dominique Martinet
<dominique.martinet@atmark-techno.com> wrote:
>
> Adam Ford wrote on Tue, Aug 27, 2024 at 09:48:02PM -0500:
> > There is currently a look-up table for a variety of resolutions.
> > Since the phy has the ability to dynamically calculate the values
> > necessary to use the intger divider which should allow more
> > resolutions without having to update the look-up-table.  If the
> > integer calculator cannot get an exact frequency, it falls back
> > to the look-up-table.  Because the LUT algorithm does some
> > rounding, I did not remove integer entries from the LUT.
>
> Thank you!
>
> We're still running 5.10 so I backported the driver as of it's current
> state first (that part works), unfortunately our 51.2MHz display does
> not work with this.
>
> After phy_clk_round_rate() not round the pixel clock to the table values
> (otherwise we'd only get rounded values), and making phy_clk_set_rate()
> pass the requested `rate` instead of using the next smaller cfg->pixclk,
> the display no longer comes up.

Do you want me to create a table entry for 51.2MHz?  I am pretty sure
the integer engine can handle it.  I'll investigate what's calling
phy_clk_round_rate to see if we can use the PMS calcualtor to avoid
the look-up table.


>
> It comes up with the values obtained for 50.4MHz (next closest value),
> which also has an exact match so uses the integer divider this patch
> computes instead of the table values, but not with the 51.2MHz it
> requests...
> I'm afraid at this point I don't know how to debug that further without
> getting a scope out (I don't know if the soc isn't generating something
> correct or if the display actually doesn't like the frequency it
> requests?! the later could be checked by plugging it in to another PC
> that might support that frequency...), and that is going to take quite a
> while...
>
> Hopefully Frieder will have more success with his displays?
> It could also be very well due to some of the differences with our 5.10
> tree, sorry about that.
>
> >  static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
> >                                         const struct phy_config *cfg)
> >  {
> > +     u32 desired_clock = cfg->pixclk * 5;
>
> (I don't really understand where that `* 5` comes from, but I guess it's
> expected? works for other display and neighbor frequency anyway...)

When I looked at the LUT entries for the clock rates that don't need
the fractional divider, I noted the clock rate was 5x the pix clock.
This was consistent for each entry.  Even when the fractional divider
was used, it appeared that the clock rate was 5x the pix clk.  Because
the PMS calculator I wrote is expecting a clock rate, I just
multiplied by 5 here instead of inside the PMS calculator.

adam
>
> Thanks,
> --
> Dominique
>
>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider
  2024-08-28 14:11     ` Adam Ford
@ 2024-08-28 19:28       ` Adam Ford
  0 siblings, 0 replies; 5+ messages in thread
From: Adam Ford @ 2024-08-28 19:28 UTC (permalink / raw)
  To: Dominique Martinet
  Cc: linux-phy, linux-imx, festevam, frieder.schrempf, aford,
	Vinod Koul, Kishon Vijay Abraham I, Uwe Kleine-König,
	Marco Felsch, Lucas Stach, linux-kernel, Makoto Sato

On Wed, Aug 28, 2024 at 9:11 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Aug 28, 2024 at 3:47 AM Dominique Martinet
> <dominique.martinet@atmark-techno.com> wrote:
> >
> > Adam Ford wrote on Tue, Aug 27, 2024 at 09:48:02PM -0500:
> > > There is currently a look-up table for a variety of resolutions.
> > > Since the phy has the ability to dynamically calculate the values
> > > necessary to use the intger divider which should allow more
> > > resolutions without having to update the look-up-table.  If the
> > > integer calculator cannot get an exact frequency, it falls back
> > > to the look-up-table.  Because the LUT algorithm does some
> > > rounding, I did not remove integer entries from the LUT.
> >
> > Thank you!
> >
> > We're still running 5.10 so I backported the driver as of it's current
> > state first (that part works), unfortunately our 51.2MHz display does
> > not work with this.
> >
> > After phy_clk_round_rate() not round the pixel clock to the table values
> > (otherwise we'd only get rounded values), and making phy_clk_set_rate()
> > pass the requested `rate` instead of using the next smaller cfg->pixclk,
> > the display no longer comes up.

It looks like I need to update phy_clk_round_rate to calculate the
integer clock value there as well and if it's not possible to reach
the desired value, fall back to the LUT.

I likely won't get that done today, but I'll try.

adam
>
> Do you want me to create a table entry for 51.2MHz?  I am pretty sure
> the integer engine can handle it.  I'll investigate what's calling
> phy_clk_round_rate to see if we can use the PMS calcualtor to avoid
> the look-up table.
>
>
> >
> > It comes up with the values obtained for 50.4MHz (next closest value),
> > which also has an exact match so uses the integer divider this patch
> > computes instead of the table values, but not with the 51.2MHz it
> > requests...
> > I'm afraid at this point I don't know how to debug that further without
> > getting a scope out (I don't know if the soc isn't generating something
> > correct or if the display actually doesn't like the frequency it
> > requests?! the later could be checked by plugging it in to another PC
> > that might support that frequency...), and that is going to take quite a
> > while...
> >
> > Hopefully Frieder will have more success with his displays?
> > It could also be very well due to some of the differences with our 5.10
> > tree, sorry about that.
> >
> > >  static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
> > >                                         const struct phy_config *cfg)
> > >  {
> > > +     u32 desired_clock = cfg->pixclk * 5;
> >
> > (I don't really understand where that `* 5` comes from, but I guess it's
> > expected? works for other display and neighbor frequency anyway...)
>
> When I looked at the LUT entries for the clock rates that don't need
> the fractional divider, I noted the clock rate was 5x the pix clock.
> This was consistent for each entry.  Even when the fractional divider
> was used, it appeared that the clock rate was 5x the pix clk.  Because
> the PMS calculator I wrote is expecting a clock rate, I just
> multiplied by 5 here instead of inside the PMS calculator.
>
> adam
> >
> > Thanks,
> > --
> > Dominique
> >
> >

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-08-28 19:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-28  2:48 [RFC 1/2] phy: freescale: fsl-samsung-hdmi: Replace register defines with macro Adam Ford
2024-08-28  2:48 ` [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider Adam Ford
2024-08-28  8:47   ` Dominique Martinet
2024-08-28 14:11     ` Adam Ford
2024-08-28 19:28       ` Adam Ford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox