| Message ID | 20220304002508.75676-7-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: bridge: icn6211: Fix hard-coded panel settings and add I2C support | expand |
On Fri, Mar 4, 2022 at 5:55 AM Marek Vasut <marex@denx.de> wrote: > > The chip contains fractional PLL, however the driver currently hard-codes > one specific PLL setting. Implement generic PLL parameter calculation code, > so any DPI panel with arbitrary pixel clock can be attached to this bridge. > > The datasheet for this bridge is not available, the PLL behavior has been > inferred from [1] and [2] and by analyzing the DPI pixel clock with scope. > The PLL limits might be wrong, but at least the calculated values match all > the example code available. This is better than one hard-coded pixel clock > value anyway. > > [1] https://github.com/rockchip-linux/kernel/blob/develop-4.19/drivers/gpu/drm/bridge/icn6211.c > [2] https://github.com/tdjastrzebski/ICN6211-Configurator > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Jagan Teki <jagan@amarulasolutions.com> > Cc: Maxime Ripard <maxime@cerno.tech> > Cc: Robert Foss <robert.foss@linaro.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > To: dri-devel@lists.freedesktop.org > --- > V2: Rebase on next-20220214 > V3: Add AB from Maxime > --- > drivers/gpu/drm/bridge/chipone-icn6211.c | 87 +++++++++++++++++++++++- > 1 file changed, 84 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c > index df8e75a068ad0..71c83a18984fa 100644 > --- a/drivers/gpu/drm/bridge/chipone-icn6211.c > +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c > @@ -163,6 +163,87 @@ static inline int chipone_dsi_write(struct chipone *icn, const void *seq, > chipone_dsi_write(icn, d, ARRAY_SIZE(d)); \ > } > > +static void chipone_configure_pll(struct chipone *icn, > + const struct drm_display_mode *mode) > +{ > + unsigned int best_p = 0, best_m = 0, best_s = 0; > + unsigned int delta, min_delta = 0xffffffff; > + unsigned int freq_p, freq_s, freq_out; > + unsigned int p_min, p_max; > + unsigned int p, m, s; > + unsigned int fin; > + > + /* > + * DSI clock lane frequency (input into PLL) is calculated as: > + * DSI_CLK = mode clock * bpp / dsi_data_lanes / 2 > + * the 2 is there because the bus is DDR. > + * > + * DPI pixel clock frequency (output from PLL) is mode clock. > + * > + * The chip contains fractional PLL which works as follows: > + * DPI_CLK = ((DSI_CLK / P) * M) / S > + * P is pre-divider, register PLL_REF_DIV[3:0] is 2^(n+1) divider > + * register PLL_REF_DIV[4] is extra 1:2 divider > + * M is integer multiplier, register PLL_INT(0) is multiplier > + * S is post-divider, register PLL_REF_DIV[7:5] is 2^(n+1) divider > + * > + * It seems the PLL input clock after applying P pre-divider have > + * to be lower than 20 MHz. > + */ > + fin = mode->clock * mipi_dsi_pixel_format_to_bpp(icn->dsi->format) / no dsi in chipone structure. please preserve format during the probe. drivers/gpu/drm/bridge/chipone-icn6211.c:193:61: error: ‘struct chipone’ has no member named ‘dsi’ 193 | fin = mode->clock * mipi_dsi_pixel_format_to_bpp(icn->dsi->format) / Thanks, Jagan.
On 3/8/22 09:07, Jagan Teki wrote: > On Fri, Mar 4, 2022 at 5:55 AM Marek Vasut <marex@denx.de> wrote: >> >> The chip contains fractional PLL, however the driver currently hard-codes >> one specific PLL setting. Implement generic PLL parameter calculation code, >> so any DPI panel with arbitrary pixel clock can be attached to this bridge. >> >> The datasheet for this bridge is not available, the PLL behavior has been >> inferred from [1] and [2] and by analyzing the DPI pixel clock with scope. >> The PLL limits might be wrong, but at least the calculated values match all >> the example code available. This is better than one hard-coded pixel clock >> value anyway. >> >> [1] https://github.com/rockchip-linux/kernel/blob/develop-4.19/drivers/gpu/drm/bridge/icn6211.c >> [2] https://github.com/tdjastrzebski/ICN6211-Configurator >> >> Acked-by: Maxime Ripard <maxime@cerno.tech> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Jagan Teki <jagan@amarulasolutions.com> >> Cc: Maxime Ripard <maxime@cerno.tech> >> Cc: Robert Foss <robert.foss@linaro.org> >> Cc: Sam Ravnborg <sam@ravnborg.org> >> Cc: Thomas Zimmermann <tzimmermann@suse.de> >> To: dri-devel@lists.freedesktop.org >> --- >> V2: Rebase on next-20220214 >> V3: Add AB from Maxime >> --- >> drivers/gpu/drm/bridge/chipone-icn6211.c | 87 +++++++++++++++++++++++- >> 1 file changed, 84 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c >> index df8e75a068ad0..71c83a18984fa 100644 >> --- a/drivers/gpu/drm/bridge/chipone-icn6211.c >> +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c >> @@ -163,6 +163,87 @@ static inline int chipone_dsi_write(struct chipone *icn, const void *seq, >> chipone_dsi_write(icn, d, ARRAY_SIZE(d)); \ >> } >> >> +static void chipone_configure_pll(struct chipone *icn, >> + const struct drm_display_mode *mode) >> +{ >> + unsigned int best_p = 0, best_m = 0, best_s = 0; >> + unsigned int delta, min_delta = 0xffffffff; >> + unsigned int freq_p, freq_s, freq_out; >> + unsigned int p_min, p_max; >> + unsigned int p, m, s; >> + unsigned int fin; >> + >> + /* >> + * DSI clock lane frequency (input into PLL) is calculated as: >> + * DSI_CLK = mode clock * bpp / dsi_data_lanes / 2 >> + * the 2 is there because the bus is DDR. >> + * >> + * DPI pixel clock frequency (output from PLL) is mode clock. >> + * >> + * The chip contains fractional PLL which works as follows: >> + * DPI_CLK = ((DSI_CLK / P) * M) / S >> + * P is pre-divider, register PLL_REF_DIV[3:0] is 2^(n+1) divider >> + * register PLL_REF_DIV[4] is extra 1:2 divider >> + * M is integer multiplier, register PLL_INT(0) is multiplier >> + * S is post-divider, register PLL_REF_DIV[7:5] is 2^(n+1) divider >> + * >> + * It seems the PLL input clock after applying P pre-divider have >> + * to be lower than 20 MHz. >> + */ >> + fin = mode->clock * mipi_dsi_pixel_format_to_bpp(icn->dsi->format) / > > no dsi in chipone structure. please preserve format during the probe. > > drivers/gpu/drm/bridge/chipone-icn6211.c:193:61: error: ‘struct > chipone’ has no member named ‘dsi’ > 193 | fin = mode->clock * > mipi_dsi_pixel_format_to_bpp(icn->dsi->format) / Ah , yes, this was added in [PATCH V3 11/13] drm: bridge: icn6211: Add I2C configuration support and needs to be moved here.
diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c index df8e75a068ad0..71c83a18984fa 100644 --- a/drivers/gpu/drm/bridge/chipone-icn6211.c +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c @@ -163,6 +163,87 @@ static inline int chipone_dsi_write(struct chipone *icn, const void *seq, chipone_dsi_write(icn, d, ARRAY_SIZE(d)); \ } +static void chipone_configure_pll(struct chipone *icn, + const struct drm_display_mode *mode) +{ + unsigned int best_p = 0, best_m = 0, best_s = 0; + unsigned int delta, min_delta = 0xffffffff; + unsigned int freq_p, freq_s, freq_out; + unsigned int p_min, p_max; + unsigned int p, m, s; + unsigned int fin; + + /* + * DSI clock lane frequency (input into PLL) is calculated as: + * DSI_CLK = mode clock * bpp / dsi_data_lanes / 2 + * the 2 is there because the bus is DDR. + * + * DPI pixel clock frequency (output from PLL) is mode clock. + * + * The chip contains fractional PLL which works as follows: + * DPI_CLK = ((DSI_CLK / P) * M) / S + * P is pre-divider, register PLL_REF_DIV[3:0] is 2^(n+1) divider + * register PLL_REF_DIV[4] is extra 1:2 divider + * M is integer multiplier, register PLL_INT(0) is multiplier + * S is post-divider, register PLL_REF_DIV[7:5] is 2^(n+1) divider + * + * It seems the PLL input clock after applying P pre-divider have + * to be lower than 20 MHz. + */ + fin = mode->clock * mipi_dsi_pixel_format_to_bpp(icn->dsi->format) / + icn->dsi_lanes / 2; /* in kHz */ + + /* Minimum value of P predivider for PLL input in 5..20 MHz */ + p_min = ffs(fin / 20000); + p_max = (fls(fin / 5000) - 1) & 0x1f; + + for (p = p_min; p < p_max; p++) { /* PLL_REF_DIV[4,3:0] */ + freq_p = fin / BIT(p + 1); + if (freq_p == 0) /* Divider too high */ + break; + + for (s = 0; s < 0x7; s++) { /* PLL_REF_DIV[7:5] */ + freq_s = freq_p / BIT(s + 1); + if (freq_s == 0) /* Divider too high */ + break; + + m = mode->clock / freq_s; + + /* Multiplier is 8 bit */ + if (m > 0xff) + continue; + + /* Limit PLL VCO frequency to 1 GHz */ + freq_out = (fin * m) / BIT(p + 1); + if (freq_out > 1000000) + continue; + + /* Apply post-divider */ + freq_out /= BIT(s + 1); + + delta = abs(mode->clock - freq_out); + if (delta < min_delta) { + best_p = p; + best_m = m; + best_s = s; + min_delta = delta; + } + } + } + + dev_dbg(icn->dev, + "PLL: P[3:0]=2^%d P[4]=2*%d M=%d S[7:5]=2^%d delta=%d => DSI f_in=%d kHz ; DPI f_out=%ld kHz\n", + best_p, !!best_p, best_m, best_s + 1, min_delta, fin, + (fin * best_m) / BIT(best_p + best_s + 2)); + + /* Clock source selection fixed to MIPI DSI clock lane */ + ICN6211_DSI(icn, PLL_CTRL(6), PLL_CTRL_6_MIPI_CLK); + ICN6211_DSI(icn, PLL_REF_DIV, + (best_p ? PLL_REF_DIV_Pe : 0) | /* Prefer /2 pre-divider */ + PLL_REF_DIV_P(best_p) | PLL_REF_DIV_S(best_s)); + ICN6211_DSI(icn, PLL_INT(0), best_m); +} + static void chipone_atomic_enable(struct drm_bridge *bridge, struct drm_bridge_state *old_bridge_state) { @@ -228,9 +309,9 @@ static void chipone_atomic_enable(struct drm_bridge *bridge, ((bus_flags & DRM_BUS_FLAG_DE_HIGH) ? BIST_POL_DE_POL : 0); ICN6211_DSI(icn, BIST_POL, pol); - ICN6211_DSI(icn, PLL_CTRL(6), PLL_CTRL_6_MIPI_CLK); - ICN6211_DSI(icn, PLL_REF_DIV, 0x71); - ICN6211_DSI(icn, PLL_INT(0), 0x2b); + /* Configure PLL settings */ + chipone_configure_pll(icn, mode); + ICN6211_DSI(icn, SYS_CTRL(0), 0x40); ICN6211_DSI(icn, SYS_CTRL(1), 0x98);