| Message ID | 20230125113529.13952-6-a-bhatia1@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DSS support for AM625 SoC | expand |
On 25/01/2023 13:35, Aradhya Bhatia wrote: > The ctrl mmr module of the AM625 is different from the AM65X SoC. Thus > the ctrl mmr registers that supported the OLDI TX power have become > different in AM625 SoC. > > The common mode voltage of the LVDS buffers becomes random when the > bandgap reference is turned off. This causes uncertainity in the LVDS > Data and Clock signal outputs, making it behave differently under > different conditions and panel setups. The bandgap reference must be > powered on before using the OLDI IOs, to keep the common voltage trimmed > down to desired levels. > > Add support to enable/disable OLDI IO signals as well as the bandgap > reference circuit for the LVDS signals. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > > Note: > - Dropped Tomi Valkeinen's reviewed-by tag in this patch because I did > not implement one of his comments which suggested to remove the > 'oldi_supported' variable. While the oldi support is indeed based on > SoC variations, keeping that variable helps take into account the case > where an OLDI supporting SoC by-passes OLDI TXes and gives out DPI > video signals straight from DSS. Hmm why is that relevent for this patch? It doesn't use oldi_supported or the new has_oldi. > drivers/gpu/drm/tidss/tidss_dispc.c | 57 +++++++++++++++++++----- > drivers/gpu/drm/tidss/tidss_dispc_regs.h | 40 ++++++++++++----- > 2 files changed, 76 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c > index 37a73e309330..0e03557bc142 100644 > --- a/drivers/gpu/drm/tidss/tidss_dispc.c > +++ b/drivers/gpu/drm/tidss/tidss_dispc.c > @@ -934,21 +934,56 @@ int dispc_vp_bus_check(struct dispc_device *dispc, u32 hw_videoport, > > static void dispc_oldi_tx_power(struct dispc_device *dispc, bool power) > { > - u32 val = power ? 0 : OLDI_PWRDN_TX; > + u32 val; > > if (WARN_ON(!dispc->oldi_io_ctrl)) > return; > > - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT0_IO_CTRL, > - OLDI_PWRDN_TX, val); > - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT1_IO_CTRL, > - OLDI_PWRDN_TX, val); > - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT2_IO_CTRL, > - OLDI_PWRDN_TX, val); > - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT3_IO_CTRL, > - OLDI_PWRDN_TX, val); > - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_CLK_IO_CTRL, > - OLDI_PWRDN_TX, val); > + if (dispc->feat->subrev == DISPC_AM65X) { Slight nitpick, but I think switch-case makes sense for the subrev. Even if there are just two options here, using switch makes the structure clearer. > + val = power ? 0 : AM65X_OLDI_PWRDN_TX; > + > + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT0_IO_CTRL, > + AM65X_OLDI_PWRDN_TX, val); > + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT1_IO_CTRL, > + AM65X_OLDI_PWRDN_TX, val); > + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT2_IO_CTRL, > + AM65X_OLDI_PWRDN_TX, val); > + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT3_IO_CTRL, > + AM65X_OLDI_PWRDN_TX, val); > + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_CLK_IO_CTRL, > + AM65X_OLDI_PWRDN_TX, val); > + > + } else if (dispc->feat->subrev == DISPC_AM625) { > + if (power) { > + switch (dispc->oldi_mode) { > + case OLDI_MODE_SINGLE_LINK: > + /* Power down OLDI TX 1 */ > + val = AM625_OLDI1_PWRDN_TX; > + break; > + > + case OLDI_MODE_CLONE_SINGLE_LINK: > + case OLDI_MODE_DUAL_LINK: > + /* No Power down */ > + val = 0; > + break; > + > + default: > + /* Power down both OLDI TXes and LVDS Bandgap */ > + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | > + AM625_OLDI_PWRDN_BG; > + break; > + } > + > + } else { > + /* Power down both OLDI TXes and LVDS Bandgap */ > + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | > + AM625_OLDI_PWRDN_BG; > + } > + > + regmap_update_bits(dispc->oldi_io_ctrl, AM625_OLDI_PD_CTRL, > + AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | > + AM625_OLDI_PWRDN_BG, val); > + } > } > > static void dispc_set_num_datalines(struct dispc_device *dispc, > diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/drm/tidss/tidss_dispc_regs.h > index 13feedfe5d6d..b2a148e96022 100644 > --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h > +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h > @@ -227,17 +227,37 @@ enum dispc_common_regs { > #define DISPC_VP_DSS_DMA_THREADSIZE_STATUS 0x174 /* J721E */ > > /* > - * OLDI IO_CTRL register offsets. On AM654 the registers are found > - * from CTRL_MMR0, there the syscon regmap should map 0x14 bytes from > - * CTRLMMR0P1_OLDI_DAT0_IO_CTRL to CTRLMMR0P1_OLDI_CLK_IO_CTRL > - * register range. > + * OLDI IO and PD CTRL register offsets. > + * These registers are found in the CTRL_MMR0, where the syscon regmap should map > + * > + * 1. 0x14 bytes from CTRLMMR0P1_OLDI_DAT0_IO_CTRL to CTRLMMR0P1_OLDI_CLK_IO_CTRL > + * register range for the AM65X DSS, and > + * > + * 2. 0x200 bytes from OLDI0_DAT0_IO_CTRL to OLDI_LB_CTRL register range for the > + * AM625 DSS. > */ > -#define OLDI_DAT0_IO_CTRL 0x00 > -#define OLDI_DAT1_IO_CTRL 0x04 > -#define OLDI_DAT2_IO_CTRL 0x08 > -#define OLDI_DAT3_IO_CTRL 0x0C > -#define OLDI_CLK_IO_CTRL 0x10 > > -#define OLDI_PWRDN_TX BIT(8) > +/* -- For AM65X OLDI TX -- */ > +/* Register offsets */ > +#define AM65X_OLDI_DAT0_IO_CTRL 0x00 > +#define AM65X_OLDI_DAT1_IO_CTRL 0x04 > +#define AM65X_OLDI_DAT2_IO_CTRL 0x08 > +#define AM65X_OLDI_DAT3_IO_CTRL 0x0C > +#define AM65X_OLDI_CLK_IO_CTRL 0x10 > + > +/* Power control bits */ > +#define AM65X_OLDI_PWRDN_TX BIT(8) > + > +/* -- For AM625 OLDI TX -- */ > +/* Register offsets */ > +#define AM625_OLDI_PD_CTRL 0x100 > +#define AM625_OLDI_LB_CTRL 0x104 > + > +/* Power control bits */ > +#define AM625_OLDI0_PWRDN_TX BIT(0) > +#define AM625_OLDI1_PWRDN_TX BIT(1) > + > +/* LVDS Bandgap reference Enable/Disable */ > +#define AM625_OLDI_PWRDN_BG BIT(8) > > #endif /* __TIDSS_DISPC_REGS_H */ Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Tomi
On 03-Feb-23 20:49, Tomi Valkeinen wrote: > On 25/01/2023 13:35, Aradhya Bhatia wrote: >> The ctrl mmr module of the AM625 is different from the AM65X SoC. Thus >> the ctrl mmr registers that supported the OLDI TX power have become >> different in AM625 SoC. >> >> The common mode voltage of the LVDS buffers becomes random when the >> bandgap reference is turned off. This causes uncertainity in the LVDS >> Data and Clock signal outputs, making it behave differently under >> different conditions and panel setups. The bandgap reference must be >> powered on before using the OLDI IOs, to keep the common voltage trimmed >> down to desired levels. >> >> Add support to enable/disable OLDI IO signals as well as the bandgap >> reference circuit for the LVDS signals. >> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >> --- >> >> Note: >> - Dropped Tomi Valkeinen's reviewed-by tag in this patch because I did >> not implement one of his comments which suggested to remove the >> 'oldi_supported' variable. While the oldi support is indeed based on >> SoC variations, keeping that variable helps take into account the case >> where an OLDI supporting SoC by-passes OLDI TXes and gives out DPI >> video signals straight from DSS. > > Hmm why is that relevent for this patch? It doesn't use oldi_supported > or the new has_oldi. It doesn't. Not directly atleast. In the previous version of this patch, there was a mention of 'oldi_supported' and your tag was conditional to that variable getting removed. Instead, I renamed the variable. > >> drivers/gpu/drm/tidss/tidss_dispc.c | 57 +++++++++++++++++++----- >> drivers/gpu/drm/tidss/tidss_dispc_regs.h | 40 ++++++++++++----- >> 2 files changed, 76 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c >> index 37a73e309330..0e03557bc142 100644 >> --- a/drivers/gpu/drm/tidss/tidss_dispc.c >> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c >> @@ -934,21 +934,56 @@ int dispc_vp_bus_check(struct dispc_device *dispc, u32 hw_videoport, >> static void dispc_oldi_tx_power(struct dispc_device *dispc, bool power) >> { >> - u32 val = power ? 0 : OLDI_PWRDN_TX; >> + u32 val; >> if (WARN_ON(!dispc->oldi_io_ctrl)) >> return; >> - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT0_IO_CTRL, >> - OLDI_PWRDN_TX, val); >> - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT1_IO_CTRL, >> - OLDI_PWRDN_TX, val); >> - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT2_IO_CTRL, >> - OLDI_PWRDN_TX, val); >> - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT3_IO_CTRL, >> - OLDI_PWRDN_TX, val); >> - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_CLK_IO_CTRL, >> - OLDI_PWRDN_TX, val); >> + if (dispc->feat->subrev == DISPC_AM65X) { > > Slight nitpick, but I think switch-case makes sense for the subrev. Even > if there are just two options here, using switch makes the structure clearer. Alright. I will make the edit. > >> + val = power ? 0 : AM65X_OLDI_PWRDN_TX; >> + >> + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT0_IO_CTRL, >> + AM65X_OLDI_PWRDN_TX, val); >> + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT1_IO_CTRL, >> + AM65X_OLDI_PWRDN_TX, val); >> + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT2_IO_CTRL, >> + AM65X_OLDI_PWRDN_TX, val); >> + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT3_IO_CTRL, >> + AM65X_OLDI_PWRDN_TX, val); >> + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_CLK_IO_CTRL, >> + AM65X_OLDI_PWRDN_TX, val); >> + >> + } else if (dispc->feat->subrev == DISPC_AM625) { >> + if (power) { >> + switch (dispc->oldi_mode) { >> + case OLDI_MODE_SINGLE_LINK: >> + /* Power down OLDI TX 1 */ >> + val = AM625_OLDI1_PWRDN_TX; >> + break; >> + >> + case OLDI_MODE_CLONE_SINGLE_LINK: >> + case OLDI_MODE_DUAL_LINK: >> + /* No Power down */ >> + val = 0; >> + break; >> + >> + default: >> + /* Power down both OLDI TXes and LVDS Bandgap */ >> + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | >> + AM625_OLDI_PWRDN_BG; >> + break; >> + } >> + >> + } else { >> + /* Power down both OLDI TXes and LVDS Bandgap */ >> + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | >> + AM625_OLDI_PWRDN_BG; >> + } >> + >> + regmap_update_bits(dispc->oldi_io_ctrl, AM625_OLDI_PD_CTRL, >> + AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | >> + AM625_OLDI_PWRDN_BG, val); >> + } >> } >> static void dispc_set_num_datalines(struct dispc_device *dispc, >> diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/drm/tidss/tidss_dispc_regs.h >> index 13feedfe5d6d..b2a148e96022 100644 >> --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h >> +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h >> @@ -227,17 +227,37 @@ enum dispc_common_regs { >> #define DISPC_VP_DSS_DMA_THREADSIZE_STATUS 0x174 /* J721E */ >> /* >> - * OLDI IO_CTRL register offsets. On AM654 the registers are found >> - * from CTRL_MMR0, there the syscon regmap should map 0x14 bytes from >> - * CTRLMMR0P1_OLDI_DAT0_IO_CTRL to CTRLMMR0P1_OLDI_CLK_IO_CTRL >> - * register range. >> + * OLDI IO and PD CTRL register offsets. >> + * These registers are found in the CTRL_MMR0, where the syscon regmap should map >> + * >> + * 1. 0x14 bytes from CTRLMMR0P1_OLDI_DAT0_IO_CTRL to CTRLMMR0P1_OLDI_CLK_IO_CTRL >> + * register range for the AM65X DSS, and >> + * >> + * 2. 0x200 bytes from OLDI0_DAT0_IO_CTRL to OLDI_LB_CTRL register range for the >> + * AM625 DSS. >> */ >> -#define OLDI_DAT0_IO_CTRL 0x00 >> -#define OLDI_DAT1_IO_CTRL 0x04 >> -#define OLDI_DAT2_IO_CTRL 0x08 >> -#define OLDI_DAT3_IO_CTRL 0x0C >> -#define OLDI_CLK_IO_CTRL 0x10 >> -#define OLDI_PWRDN_TX BIT(8) >> +/* -- For AM65X OLDI TX -- */ >> +/* Register offsets */ >> +#define AM65X_OLDI_DAT0_IO_CTRL 0x00 >> +#define AM65X_OLDI_DAT1_IO_CTRL 0x04 >> +#define AM65X_OLDI_DAT2_IO_CTRL 0x08 >> +#define AM65X_OLDI_DAT3_IO_CTRL 0x0C >> +#define AM65X_OLDI_CLK_IO_CTRL 0x10 >> + >> +/* Power control bits */ >> +#define AM65X_OLDI_PWRDN_TX BIT(8) >> + >> +/* -- For AM625 OLDI TX -- */ >> +/* Register offsets */ >> +#define AM625_OLDI_PD_CTRL 0x100 >> +#define AM625_OLDI_LB_CTRL 0x104 >> + >> +/* Power control bits */ >> +#define AM625_OLDI0_PWRDN_TX BIT(0) >> +#define AM625_OLDI1_PWRDN_TX BIT(1) >> + >> +/* LVDS Bandgap reference Enable/Disable */ >> +#define AM625_OLDI_PWRDN_BG BIT(8) >> #endif /* __TIDSS_DISPC_REGS_H */ > > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > Tomi > Regards Aradhya
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c index 37a73e309330..0e03557bc142 100644 --- a/drivers/gpu/drm/tidss/tidss_dispc.c +++ b/drivers/gpu/drm/tidss/tidss_dispc.c @@ -934,21 +934,56 @@ int dispc_vp_bus_check(struct dispc_device *dispc, u32 hw_videoport, static void dispc_oldi_tx_power(struct dispc_device *dispc, bool power) { - u32 val = power ? 0 : OLDI_PWRDN_TX; + u32 val; if (WARN_ON(!dispc->oldi_io_ctrl)) return; - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT0_IO_CTRL, - OLDI_PWRDN_TX, val); - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT1_IO_CTRL, - OLDI_PWRDN_TX, val); - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT2_IO_CTRL, - OLDI_PWRDN_TX, val); - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT3_IO_CTRL, - OLDI_PWRDN_TX, val); - regmap_update_bits(dispc->oldi_io_ctrl, OLDI_CLK_IO_CTRL, - OLDI_PWRDN_TX, val); + if (dispc->feat->subrev == DISPC_AM65X) { + val = power ? 0 : AM65X_OLDI_PWRDN_TX; + + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT0_IO_CTRL, + AM65X_OLDI_PWRDN_TX, val); + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT1_IO_CTRL, + AM65X_OLDI_PWRDN_TX, val); + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT2_IO_CTRL, + AM65X_OLDI_PWRDN_TX, val); + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_DAT3_IO_CTRL, + AM65X_OLDI_PWRDN_TX, val); + regmap_update_bits(dispc->oldi_io_ctrl, AM65X_OLDI_CLK_IO_CTRL, + AM65X_OLDI_PWRDN_TX, val); + + } else if (dispc->feat->subrev == DISPC_AM625) { + if (power) { + switch (dispc->oldi_mode) { + case OLDI_MODE_SINGLE_LINK: + /* Power down OLDI TX 1 */ + val = AM625_OLDI1_PWRDN_TX; + break; + + case OLDI_MODE_CLONE_SINGLE_LINK: + case OLDI_MODE_DUAL_LINK: + /* No Power down */ + val = 0; + break; + + default: + /* Power down both OLDI TXes and LVDS Bandgap */ + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | + AM625_OLDI_PWRDN_BG; + break; + } + + } else { + /* Power down both OLDI TXes and LVDS Bandgap */ + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | + AM625_OLDI_PWRDN_BG; + } + + regmap_update_bits(dispc->oldi_io_ctrl, AM625_OLDI_PD_CTRL, + AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX | + AM625_OLDI_PWRDN_BG, val); + } } static void dispc_set_num_datalines(struct dispc_device *dispc, diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/drm/tidss/tidss_dispc_regs.h index 13feedfe5d6d..b2a148e96022 100644 --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h @@ -227,17 +227,37 @@ enum dispc_common_regs { #define DISPC_VP_DSS_DMA_THREADSIZE_STATUS 0x174 /* J721E */ /* - * OLDI IO_CTRL register offsets. On AM654 the registers are found - * from CTRL_MMR0, there the syscon regmap should map 0x14 bytes from - * CTRLMMR0P1_OLDI_DAT0_IO_CTRL to CTRLMMR0P1_OLDI_CLK_IO_CTRL - * register range. + * OLDI IO and PD CTRL register offsets. + * These registers are found in the CTRL_MMR0, where the syscon regmap should map + * + * 1. 0x14 bytes from CTRLMMR0P1_OLDI_DAT0_IO_CTRL to CTRLMMR0P1_OLDI_CLK_IO_CTRL + * register range for the AM65X DSS, and + * + * 2. 0x200 bytes from OLDI0_DAT0_IO_CTRL to OLDI_LB_CTRL register range for the + * AM625 DSS. */ -#define OLDI_DAT0_IO_CTRL 0x00 -#define OLDI_DAT1_IO_CTRL 0x04 -#define OLDI_DAT2_IO_CTRL 0x08 -#define OLDI_DAT3_IO_CTRL 0x0C -#define OLDI_CLK_IO_CTRL 0x10 -#define OLDI_PWRDN_TX BIT(8) +/* -- For AM65X OLDI TX -- */ +/* Register offsets */ +#define AM65X_OLDI_DAT0_IO_CTRL 0x00 +#define AM65X_OLDI_DAT1_IO_CTRL 0x04 +#define AM65X_OLDI_DAT2_IO_CTRL 0x08 +#define AM65X_OLDI_DAT3_IO_CTRL 0x0C +#define AM65X_OLDI_CLK_IO_CTRL 0x10 + +/* Power control bits */ +#define AM65X_OLDI_PWRDN_TX BIT(8) + +/* -- For AM625 OLDI TX -- */ +/* Register offsets */ +#define AM625_OLDI_PD_CTRL 0x100 +#define AM625_OLDI_LB_CTRL 0x104 + +/* Power control bits */ +#define AM625_OLDI0_PWRDN_TX BIT(0) +#define AM625_OLDI1_PWRDN_TX BIT(1) + +/* LVDS Bandgap reference Enable/Disable */ +#define AM625_OLDI_PWRDN_BG BIT(8) #endif /* __TIDSS_DISPC_REGS_H */
The ctrl mmr module of the AM625 is different from the AM65X SoC. Thus the ctrl mmr registers that supported the OLDI TX power have become different in AM625 SoC. The common mode voltage of the LVDS buffers becomes random when the bandgap reference is turned off. This causes uncertainity in the LVDS Data and Clock signal outputs, making it behave differently under different conditions and panel setups. The bandgap reference must be powered on before using the OLDI IOs, to keep the common voltage trimmed down to desired levels. Add support to enable/disable OLDI IO signals as well as the bandgap reference circuit for the LVDS signals. Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> --- Note: - Dropped Tomi Valkeinen's reviewed-by tag in this patch because I did not implement one of his comments which suggested to remove the 'oldi_supported' variable. While the oldi support is indeed based on SoC variations, keeping that variable helps take into account the case where an OLDI supporting SoC by-passes OLDI TXes and gives out DPI video signals straight from DSS. drivers/gpu/drm/tidss/tidss_dispc.c | 57 +++++++++++++++++++----- drivers/gpu/drm/tidss/tidss_dispc_regs.h | 40 ++++++++++++----- 2 files changed, 76 insertions(+), 21 deletions(-)