diff mbox series

[RFC,V2,2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider

Message ID 20240829021256.787615-2-aford173@gmail.com
State Superseded
Headers show
Series [RFC,V2,1/2] phy: freescale: fsl-samsung-hdmi: Replace register defines with macro | expand

Commit Message

Adam Ford Aug. 29, 2024, 2:12 a.m. UTC
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>

Comments

Vinod Koul Aug. 29, 2024, 5:55 p.m. UTC | #1
On 28-08-24, 21:12, Adam Ford wrote:
> 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.

Any reason why this is RFC?

> 
> 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..76e0899c6006 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,11 +33,17 @@
>  
>  #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];
>  };
>  
> +static struct phy_config custom_phy_pll_cfg;
> +
>  static const struct phy_config phy_pll_cfg[] = {
>  	{
>  		.pixclk = 22250000,
> @@ -440,10 +448,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) {

starts with 1 to 11.. why?

> +		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 +534,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));
> @@ -484,8 +592,17 @@ static unsigned long phy_clk_recalc_rate(struct clk_hw *hw,
>  static long phy_clk_round_rate(struct clk_hw *hw,
>  			       unsigned long rate, unsigned long *parent_rate)
>  {
> +	u32 int_div_clk;
>  	int i;
> +	u16 m;
> +	u8 p, s;
> +
> +	/* If the integer divider works, just use it */
> +	int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate * 5, &p, &m, &s) / 5;
> +	if (int_div_clk == rate)
> +		return int_div_clk;
>  
> +	/* Otherwise, fall back to the LUT */
>  	for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
>  		if (phy_pll_cfg[i].pixclk <= rate)
>  			return phy_pll_cfg[i].pixclk;
> @@ -497,16 +614,28 @@ static int phy_clk_set_rate(struct clk_hw *hw,
>  			    unsigned long rate, unsigned long parent_rate)
>  {
>  	struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
> +	u32 int_div_clk;
>  	int i;
> -
> -	for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
> -		if (phy_pll_cfg[i].pixclk <= rate)
> -			break;
> -
> -	if (i < 0)
> -		return -EINVAL;
> -
> -	phy->cur_cfg = &phy_pll_cfg[i];
> +	u16 m;
> +	u8 p, s;
> +
> +	/* If the integer divider works, just use it */
> +	int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate * 5, &p, &m, &s) / 5;
> +	if (int_div_clk == rate) {
> +		/* Just set the pixclk rate, the rest will be calculated */
> +		custom_phy_pll_cfg.pixclk = int_div_clk;
> +		phy->cur_cfg  = &custom_phy_pll_cfg;
> +	} else {
> +		/* Otherwise, search the LUT */
> +		for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
> +			if (phy_pll_cfg[i].pixclk <= rate)
> +				break;
> +
> +		if (i < 0)
> +			return -EINVAL;
> +
> +		phy->cur_cfg = &phy_pll_cfg[i];
> +	}
>  
>  	return fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);
>  }
> -- 
> 2.43.0
Adam Ford Aug. 29, 2024, 6:30 p.m. UTC | #2
On Thu, Aug 29, 2024 at 12:56 PM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 28-08-24, 21:12, Adam Ford wrote:
> > 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.
>
> Any reason why this is RFC?

Someone was asking for functionality, but I'm not 100% sure this is
the right approach or it would even work.  I am waiting for feedback
from Dominique to determine if this helps solve the display for that
particular display.

>
> >
> > 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..76e0899c6006 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,11 +33,17 @@
> >
> >  #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];
> >  };
> >
> > +static struct phy_config custom_phy_pll_cfg;
> > +
> >  static const struct phy_config phy_pll_cfg[] = {
> >       {
> >               .pixclk = 22250000,
> > @@ -440,10 +448,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) {
>
> starts with 1 to 11.. why?

According to Rev 2 of the 8MP Reference Manual, the Previder range is
between 1 and 11.

>
> > +             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 +534,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));
> > @@ -484,8 +592,17 @@ static unsigned long phy_clk_recalc_rate(struct clk_hw *hw,
> >  static long phy_clk_round_rate(struct clk_hw *hw,
> >                              unsigned long rate, unsigned long *parent_rate)
> >  {
> > +     u32 int_div_clk;
> >       int i;
> > +     u16 m;
> > +     u8 p, s;
> > +
> > +     /* If the integer divider works, just use it */
> > +     int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate * 5, &p, &m, &s) / 5;
> > +     if (int_div_clk == rate)
> > +             return int_div_clk;
> >
> > +     /* Otherwise, fall back to the LUT */
> >       for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
> >               if (phy_pll_cfg[i].pixclk <= rate)
> >                       return phy_pll_cfg[i].pixclk;
> > @@ -497,16 +614,28 @@ static int phy_clk_set_rate(struct clk_hw *hw,
> >                           unsigned long rate, unsigned long parent_rate)
> >  {
> >       struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
> > +     u32 int_div_clk;
> >       int i;
> > -
> > -     for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
> > -             if (phy_pll_cfg[i].pixclk <= rate)
> > -                     break;
> > -
> > -     if (i < 0)
> > -             return -EINVAL;
> > -
> > -     phy->cur_cfg = &phy_pll_cfg[i];
> > +     u16 m;
> > +     u8 p, s;
> > +
> > +     /* If the integer divider works, just use it */
> > +     int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate * 5, &p, &m, &s) / 5;
> > +     if (int_div_clk == rate) {
> > +             /* Just set the pixclk rate, the rest will be calculated */
> > +             custom_phy_pll_cfg.pixclk = int_div_clk;
> > +             phy->cur_cfg  = &custom_phy_pll_cfg;
> > +     } else {
> > +             /* Otherwise, search the LUT */
> > +             for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
> > +                     if (phy_pll_cfg[i].pixclk <= rate)
> > +                             break;
> > +
> > +             if (i < 0)
> > +                     return -EINVAL;
> > +
> > +             phy->cur_cfg = &phy_pll_cfg[i];
> > +     }
> >
> >       return fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);
> >  }
> > --
> > 2.43.0
>
> --
> ~Vinod
Vinod Koul Aug. 30, 2024, 7:55 a.m. UTC | #3
On 29-08-24, 13:30, Adam Ford wrote:
> On Thu, Aug 29, 2024 at 12:56 PM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 28-08-24, 21:12, Adam Ford wrote:
> > > 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.
> >
> > Any reason why this is RFC?
> 
> Someone was asking for functionality, but I'm not 100% sure this is
> the right approach or it would even work.  I am waiting for feedback
> from Dominique to determine if this helps solve the display for that
> particular display.
> 
> >
> > >
> > > 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..76e0899c6006 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,11 +33,17 @@
> > >
> > >  #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];
> > >  };
> > >
> > > +static struct phy_config custom_phy_pll_cfg;
> > > +
> > >  static const struct phy_config phy_pll_cfg[] = {
> > >       {
> > >               .pixclk = 22250000,
> > > @@ -440,10 +448,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) {
> >
> > starts with 1 to 11.. why?
> 
> According to Rev 2 of the 8MP Reference Manual, the Previder range is
> between 1 and 11.

Would be better to document these assumptions, am sure if someone asks
you this next year, it would be hard to recall :-)
Adam Ford Aug. 30, 2024, 1:05 p.m. UTC | #4
On Fri, Aug 30, 2024 at 2:55 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 29-08-24, 13:30, Adam Ford wrote:
> > On Thu, Aug 29, 2024 at 12:56 PM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > On 28-08-24, 21:12, Adam Ford wrote:
> > > > 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.
> > >
> > > Any reason why this is RFC?
> >
> > Someone was asking for functionality, but I'm not 100% sure this is
> > the right approach or it would even work.  I am waiting for feedback
> > from Dominique to determine if this helps solve the display for that
> > particular display.
> >
> > >
> > > >
> > > > 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..76e0899c6006 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,11 +33,17 @@
> > > >
> > > >  #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];
> > > >  };
> > > >
> > > > +static struct phy_config custom_phy_pll_cfg;
> > > > +
> > > >  static const struct phy_config phy_pll_cfg[] = {
> > > >       {
> > > >               .pixclk = 22250000,
> > > > @@ -440,10 +448,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) {
> > >
> > > starts with 1 to 11.. why?
> >
> > According to Rev 2 of the 8MP Reference Manual, the Previder range is
> > between 1 and 11.
>
> Would be better to document these assumptions, am sure if someone asks
> you this next year, it would be hard to recall :-)

I updated the note n V3.

Dominique confirmed V3 appears to be working, so I'll investigate his
suggestions, and submit a patch based on my V3 without the RFC.

adam
>
> --
> ~Vinod
diff mbox series

Patch

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index bc5d3625ece6..76e0899c6006 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,11 +33,17 @@ 
 
 #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];
 };
 
+static struct phy_config custom_phy_pll_cfg;
+
 static const struct phy_config phy_pll_cfg[] = {
 	{
 		.pixclk = 22250000,
@@ -440,10 +448,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 +534,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));
@@ -484,8 +592,17 @@  static unsigned long phy_clk_recalc_rate(struct clk_hw *hw,
 static long phy_clk_round_rate(struct clk_hw *hw,
 			       unsigned long rate, unsigned long *parent_rate)
 {
+	u32 int_div_clk;
 	int i;
+	u16 m;
+	u8 p, s;
+
+	/* If the integer divider works, just use it */
+	int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate * 5, &p, &m, &s) / 5;
+	if (int_div_clk == rate)
+		return int_div_clk;
 
+	/* Otherwise, fall back to the LUT */
 	for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
 		if (phy_pll_cfg[i].pixclk <= rate)
 			return phy_pll_cfg[i].pixclk;
@@ -497,16 +614,28 @@  static int phy_clk_set_rate(struct clk_hw *hw,
 			    unsigned long rate, unsigned long parent_rate)
 {
 	struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
+	u32 int_div_clk;
 	int i;
-
-	for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
-		if (phy_pll_cfg[i].pixclk <= rate)
-			break;
-
-	if (i < 0)
-		return -EINVAL;
-
-	phy->cur_cfg = &phy_pll_cfg[i];
+	u16 m;
+	u8 p, s;
+
+	/* If the integer divider works, just use it */
+	int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate * 5, &p, &m, &s) / 5;
+	if (int_div_clk == rate) {
+		/* Just set the pixclk rate, the rest will be calculated */
+		custom_phy_pll_cfg.pixclk = int_div_clk;
+		phy->cur_cfg  = &custom_phy_pll_cfg;
+	} else {
+		/* Otherwise, search the LUT */
+		for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
+			if (phy_pll_cfg[i].pixclk <= rate)
+				break;
+
+		if (i < 0)
+			return -EINVAL;
+
+		phy->cur_cfg = &phy_pll_cfg[i];
+	}
 
 	return fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);
 }