[PATCH 1/3] phy: freescale: fsl-samsung-hdmi: Expand Integer divider range

[PATCH 1/3] phy: freescale: fsl-samsung-hdmi: Expand Integer divider range

[Adam Ford]

The Integer divder uses values of P,M, and S to determine the PLL
rate.  Currently, the range of M was set based on a series of
table entries where the range was limited.  Since the ref manual
shows it is 8-bit wide, expand the range to be up to 255.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index 2c8038864357..3f9578f3f0ac 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -406,16 +406,15 @@ static unsigned long fsl_samsung_hdmi_phy_find_pms(unsigned long fout, u8 *p, u1
 				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 lookup table
+			 * The Ref manual doesn't explicitly state the range of M,
+			 * bit it does show it as an 8-bit value, so we'll reject
+			 * any value above 255.
 			 */
 			tmp = (u64)fout * (_p * _s);
 			do_div(tmp, 24 * MHZ);
-			_m = tmp;
-			if (_m < 0x30 || _m > 0x7b)
+			if (tmp > 255)
 				continue;
+			_m = tmp;
 
 			/*
 			 * Rev 2 of the Ref Manual states the
-- 
2.45.2

[PATCH 2/3] phy: freescale: fsl-samsung-hdmi: Stop searching when exact match is found

[Adam Ford]

There are a series of for-loops which check various values of P and S
for the integer divder PLL.  The for loops search all entries and use
the one closest to the nominal, but it continues to searches through
all for loops even after the nominal is achieved.  Ending when the
nominal value is found stops wasting time, since it will not find
a better value than a deviation of 0 Hz.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index 3f9578f3f0ac..719f8972cb5a 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -440,9 +440,13 @@ static unsigned long fsl_samsung_hdmi_phy_find_pms(unsigned long fout, u8 *p, u1
 				min_delta = delta;
 				best_freq = tmp;
 			}
+
+			/* If we have an exact match, stop looking for a better value */
+			if (!delta)
+				goto done;
 		}
 	}
-
+done:
 	if (best_freq) {
 		*p = best_p;
 		*m = best_m;
-- 
2.45.2

Re: [PATCH 2/3] phy: freescale: fsl-samsung-hdmi: Stop searching when exact match is found

[Frieder Schrempf]

On 20.10.24 6:50 PM, Adam Ford wrote:
> There are a series of for-loops which check various values of P and S
> for the integer divder PLL.  The for loops search all entries and use
> the one closest to the nominal, but it continues to searches through
> all for loops even after the nominal is achieved.  Ending when the
> nominal value is found stops wasting time, since it will not find
> a better value than a deviation of 0 Hz.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>  drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> index 3f9578f3f0ac..719f8972cb5a 100644
> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> @@ -440,9 +440,13 @@ static unsigned long fsl_samsung_hdmi_phy_find_pms(unsigned long fout, u8 *p, u1
>  				min_delta = delta;
>  				best_freq = tmp;
>  			}
> +
> +			/* If we have an exact match, stop looking for a better value */
> +			if (!delta)
> +				goto done;
>  		}
>  	}
> -
> +done:
>  	if (best_freq) {
>  		*p = best_p;
>  		*m = best_m;

[PATCH 3/3] phy: freescale: fsl-samsung-hdmi: Clean up fld_tg_code calculation

[Adam Ford]

Currently, the calcuation for fld_tg_code is based on a lookup table,
but there are gaps in the lookup table, and frequencies in these
gaps may not properly use the correct divider.  Based on the description
of FLD_CK_DIV, the internal PLL frequency should be less than 50 MHz,
so directly calcuate the value of FLD_CK_DIV from pixclk.
This allow for proper calcuation of any pixel clock and eliminates a
few gaps in the LUT.

Since the value of the int_pllclk is in Hz, do the fixed-point
math in Hz to achieve a more accurate value and reduces the complexity
of the caluation to 24MHz * (256 / int_pllclk).

Fixes: 6ad082bee902 ("phy: freescale: add Samsung HDMI PHY")
Signed-off-by: Adam Ford <aford173@gmail.com>
---
 drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 32 +++++++-------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index 719f8972cb5a..0bfe0c0907a0 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -331,25 +331,17 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
 {
 	u32 pclk = cfg->pixclk;
 	u32 fld_tg_code;
-	u32 pclk_khz;
-	u8 div = 1;
-
-	switch (cfg->pixclk) {
-	case  22250000 ...  47500000:
-		div = 1;
-		break;
-	case  50349650 ...  99000000:
-		div = 2;
-		break;
-	case 100699300 ... 198000000:
-		div = 4;
-		break;
-	case 205000000 ... 297000000:
-		div = 8;
-		break;
+	u32 int_pllclk;
+	u8 div;
+
+	/* Find int_pllclk speed */
+	for (div = 0; div < 4; div++) {
+		int_pllclk = pclk / (1 << div);
+		if (int_pllclk < (50 * MHZ))
+			break;
 	}
 
-	writeb(FIELD_PREP(REG12_CK_DIV_MASK, ilog2(div)), phy->regs + PHY_REG(12));
+	writeb(FIELD_PREP(REG12_CK_DIV_MASK, div), phy->regs + PHY_REG(12));
 
 	/*
 	 * Calculation for the frequency lock detector target code (fld_tg_code)
@@ -362,10 +354,8 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
 	 *        settings rounding up always too. TODO: Check if that is
 	 *        correct.
 	 */
-	pclk /= div;
-	pclk_khz = pclk / 1000;
-	fld_tg_code = 256 * 1000 * 1000 / pclk_khz * 24;
-	fld_tg_code = DIV_ROUND_UP(fld_tg_code, 1000);
+
+	fld_tg_code =  DIV_ROUND_UP(24 * MHZ * 256, int_pllclk);
 
 	/* FLD_TOL and FLD_RP_CODE taken from downstream driver */
 	writeb(FIELD_PREP(REG13_TG_CODE_LOW_MASK, fld_tg_code),
-- 
2.45.2

Re: [PATCH 3/3] phy: freescale: fsl-samsung-hdmi: Clean up fld_tg_code calculation

[Frieder Schrempf]

On 20.10.24 6:50 PM, Adam Ford wrote:
> Currently, the calcuation for fld_tg_code is based on a lookup table,
> but there are gaps in the lookup table, and frequencies in these
> gaps may not properly use the correct divider.  Based on the description
> of FLD_CK_DIV, the internal PLL frequency should be less than 50 MHz,
> so directly calcuate the value of FLD_CK_DIV from pixclk.
> This allow for proper calcuation of any pixel clock and eliminates a
> few gaps in the LUT.
> 
> Since the value of the int_pllclk is in Hz, do the fixed-point
> math in Hz to achieve a more accurate value and reduces the complexity
> of the caluation to 24MHz * (256 / int_pllclk).
> 
> Fixes: 6ad082bee902 ("phy: freescale: add Samsung HDMI PHY")
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>  drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 32 +++++++-------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> index 719f8972cb5a..0bfe0c0907a0 100644
> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> @@ -331,25 +331,17 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
>  {
>  	u32 pclk = cfg->pixclk;
>  	u32 fld_tg_code;
> -	u32 pclk_khz;
> -	u8 div = 1;
> -
> -	switch (cfg->pixclk) {
> -	case  22250000 ...  47500000:
> -		div = 1;
> -		break;
> -	case  50349650 ...  99000000:
> -		div = 2;
> -		break;
> -	case 100699300 ... 198000000:
> -		div = 4;
> -		break;
> -	case 205000000 ... 297000000:
> -		div = 8;
> -		break;
> +	u32 int_pllclk;
> +	u8 div;
> +
> +	/* Find int_pllclk speed */
> +	for (div = 0; div < 4; div++) {
> +		int_pllclk = pclk / (1 << div);
> +		if (int_pllclk < (50 * MHZ))
> +			break;
>  	}
>  
> -	writeb(FIELD_PREP(REG12_CK_DIV_MASK, ilog2(div)), phy->regs + PHY_REG(12));
> +	writeb(FIELD_PREP(REG12_CK_DIV_MASK, div), phy->regs + PHY_REG(12));
>  
>  	/*
>  	 * Calculation for the frequency lock detector target code (fld_tg_code)
> @@ -362,10 +354,8 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
>  	 *        settings rounding up always too. TODO: Check if that is
>  	 *        correct.
>  	 */
> -	pclk /= div;
> -	pclk_khz = pclk / 1000;
> -	fld_tg_code = 256 * 1000 * 1000 / pclk_khz * 24;
> -	fld_tg_code = DIV_ROUND_UP(fld_tg_code, 1000);
> +
> +	fld_tg_code =  DIV_ROUND_UP(24 * MHZ * 256, int_pllclk);
>  
>  	/* FLD_TOL and FLD_RP_CODE taken from downstream driver */
>  	writeb(FIELD_PREP(REG13_TG_CODE_LOW_MASK, fld_tg_code),

Re: [PATCH 1/3] phy: freescale: fsl-samsung-hdmi: Expand Integer divider range

[Frieder Schrempf]

On 20.10.24 6:50 PM, Adam Ford wrote:
> The Integer divder uses values of P,M, and S to determine the PLL
> rate.  Currently, the range of M was set based on a series of
> table entries where the range was limited.  Since the ref manual
> shows it is 8-bit wide, expand the range to be up to 255.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>

With the typo below fixed:

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>  drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> index 2c8038864357..3f9578f3f0ac 100644
> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> @@ -406,16 +406,15 @@ static unsigned long fsl_samsung_hdmi_phy_find_pms(unsigned long fout, u8 *p, u1
>  				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 lookup table
> +			 * The Ref manual doesn't explicitly state the range of M,
> +			 * bit it does show it as an 8-bit value, so we'll reject

                           ^ but

> +			 * any value above 255.
>  			 */
>  			tmp = (u64)fout * (_p * _s);
>  			do_div(tmp, 24 * MHZ);
> -			_m = tmp;
> -			if (_m < 0x30 || _m > 0x7b)
> +			if (tmp > 255)
>  				continue;
> +			_m = tmp;
>  
>  			/*
>  			 * Rev 2 of the Ref Manual states the

Re: [PATCH 1/3] phy: freescale: fsl-samsung-hdmi: Expand Integer divider range

[Adam Ford]

On Tue, Oct 22, 2024 at 2:04 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
>
> On 20.10.24 6:50 PM, Adam Ford wrote:
> > The Integer divder uses values of P,M, and S to determine the PLL
> > rate.  Currently, the range of M was set based on a series of
> > table entries where the range was limited.  Since the ref manual
> > shows it is 8-bit wide, expand the range to be up to 255.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> With the typo below fixed:
>
> Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Thanks for the review.  I'll post V2 in the next few days.
>
> > ---
> >  drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> > index 2c8038864357..3f9578f3f0ac 100644
> > --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> > +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> > @@ -406,16 +406,15 @@ static unsigned long fsl_samsung_hdmi_phy_find_pms(unsigned long fout, u8 *p, u1
> >                               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 lookup table
> > +                      * The Ref manual doesn't explicitly state the range of M,
> > +                      * bit it does show it as an 8-bit value, so we'll reject
>
>                            ^ but

whoops!  Good catch.

adam
>
> > +                      * any value above 255.
> >                        */
> >                       tmp = (u64)fout * (_p * _s);
> >                       do_div(tmp, 24 * MHZ);
> > -                     _m = tmp;
> > -                     if (_m < 0x30 || _m > 0x7b)
> > +                     if (tmp > 255)
> >                               continue;
> > +                     _m = tmp;
> >
> >                       /*
> >                        * Rev 2 of the Ref Manual states the
>