Re: [PATCH v4 07/12] drm: bridge: samsung-dsim: Fix PLL_P (PMS_P) offset

[Adam Ford <aford173@gmail.com>]

On Mon, Aug 29, 2022 at 1:41 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> The i.MX 8M Mini Applications Processor Reference Manual, Rev. 3, 11/2020
> with 13.7.10.1 Master PLL PMS Value setting Register mentioned PMS_P offset
> range from BIT[18-13] and the upstream driver is using the same offset.
>
> However, offset 13 is not working on i.MX8M Mini platforms but downstream
> NXP driver is using 14 [1] and it is working with i.MX8M Mini SoC.

From the line you highlighted in the link, the downstream NXP ones
shows 13 if I'm reading it correctly.

#define PLLCTRL_SET_P(x) REG_PUT(x, 18, 13)

From what I can tell the PMS calculation here needs to be updated for
the Mini because the ranges of the FCO calculator are different.  Took
your series and tweaked it a bit [2] which changes a few settings, and
the PMS calculator appears to more closely match the values I get from
the NXP one.  I think it could be further tweaked because p min and
p_max also have changed.

>
> Not sure about whether it is reference manual documentation or something
> else but this patch trusts the downstream code and fixes the PLL_P offset.
>
> [1] https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n211
>

[2] -https://github.com/aford173/linux/commit/a5fa184160ec9ea45a7546eaa0d8b8fc760cf3d9

> v4, v3, v2:
> * none
>
> v1:
> * updated commit message
> * add downstream driver link
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 10 ++++++++--
>  include/drm/bridge/samsung-dsim.h     |  1 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index b6883a6d4681..b6d17c0c9e58 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -168,7 +168,7 @@
>  /* DSIM_PLLCTRL */
>  #define DSIM_FREQ_BAND(x)              ((x) << 24)
>  #define DSIM_PLL_EN                    (1 << 23)
> -#define DSIM_PLL_P(x)                  ((x) << 13)
> +#define DSIM_PLL_P(x, offset)          ((x) << (offset))
>  #define DSIM_PLL_M(x)                  ((x) << 4)
>  #define DSIM_PLL_S(x)                  ((x) << 1)
>
> @@ -368,6 +368,7 @@ static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
>         .max_freq = 1000,
>         .wait_for_reset = 1,
>         .num_bits_resol = 11,
> +       .pll_p_offset = 13,
>         .reg_values = reg_values,
>         .quirks = DSIM_QUIRK_PLAT_DATA,
>  };
> @@ -381,6 +382,7 @@ static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
>         .max_freq = 1000,
>         .wait_for_reset = 1,
>         .num_bits_resol = 11,
> +       .pll_p_offset = 13,
>         .reg_values = reg_values,
>         .quirks = DSIM_QUIRK_PLAT_DATA,
>  };
> @@ -392,6 +394,7 @@ static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
>         .max_freq = 1000,
>         .wait_for_reset = 1,
>         .num_bits_resol = 11,
> +       .pll_p_offset = 13,
>         .reg_values = reg_values,
>         .quirks = DSIM_QUIRK_PLAT_DATA,
>  };
> @@ -404,6 +407,7 @@ static const struct samsung_dsim_driver_data exynos5433_dsi_driver_data = {
>         .max_freq = 1500,
>         .wait_for_reset = 0,
>         .num_bits_resol = 12,
> +       .pll_p_offset = 13,
>         .reg_values = exynos5433_reg_values,
>         .quirks = DSIM_QUIRK_PLAT_DATA,
>  };
> @@ -416,6 +420,7 @@ static const struct samsung_dsim_driver_data exynos5422_dsi_driver_data = {
>         .max_freq = 1500,
>         .wait_for_reset = 1,
>         .num_bits_resol = 12,
> +       .pll_p_offset = 13,
>         .reg_values = exynos5422_reg_values,
>         .quirks = DSIM_QUIRK_PLAT_DATA,
>  };
> @@ -563,7 +568,8 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
>         writel(driver_data->reg_values[PLL_TIMER],
>                         dsi->reg_base + driver_data->plltmr_reg);
>
> -       reg = DSIM_PLL_EN | DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);
> +       reg = DSIM_PLL_EN | DSIM_PLL_P(p, driver_data->pll_p_offset) |
> +             DSIM_PLL_M(m) | DSIM_PLL_S(s);
>
>         if (driver_data->has_freqband) {
>                 static const unsigned long freq_bands[] = {
> diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
> index e15fbfd49efe..95d3f89aec4f 100644
> --- a/include/drm/bridge/samsung-dsim.h
> +++ b/include/drm/bridge/samsung-dsim.h
> @@ -47,6 +47,7 @@ struct samsung_dsim_driver_data {
>         unsigned int max_freq;
>         unsigned int wait_for_reset;
>         unsigned int num_bits_resol;
> +       unsigned int pll_p_offset;
>         const unsigned int *reg_values;
>         enum samsung_dsim_quirks quirks;
>  };
> --
> 2.25.1
>