| Message ID | 20220829184031.1863663-8-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: bridge: Add Samsung MIPI DSIM bridge | expand |
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 >
On Tue, Aug 30, 2022 at 1:12 AM Adam Ford <aford173@gmail.com> wrote: > > 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) PMS_P value set in sec_mipi_dsim_check_pll_out using PLLCTRL_SET_P() with offset 13 and then an additional offset of one bit added in sec_mipi_dsim_config_pll via PLLCTRL_SET_PMS(). Please check this for additional information https://github.com/fschrempf/linux/commit/bbb3549a99e162ef6ad4c83d5fd4d39a6daa6d56 I'll update the additional information in commit messages in v5. Thanks, Jagan.
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; };