lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHCN7xKW=zxips+J73913eEfS+p_e3dN9BWU08=poj599JbUxA@mail.gmail.com>
Date: Thu, 29 Aug 2024 13:30:29 -0500
From: Adam Ford <aford173@...il.com>
To: Vinod Koul <vkoul@...nel.org>
Cc: linux-phy@...ts.infradead.org, dominique.martinet@...ark-techno.com, 
	linux-imx@....com, festevam@...il.com, frieder.schrempf@...tron.de, 
	aford@...conembedded.com, Kishon Vijay Abraham I <kishon@...nel.org>, 
	Lucas Stach <l.stach@...gutronix.de>, Marco Felsch <m.felsch@...gutronix.de>, 
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC V2 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic
 integer divider

On Thu, Aug 29, 2024 at 12:56 PM Vinod Koul <vkoul@...nel.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@...il.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ