Messages in this thread | | | Date | Tue, 20 Dec 2022 15:02:08 +0200 | Subject | Re: [PATCH v6 4/5] drm/tidss: Add IO CTRL and Power support for OLDI TX in am625 | From | Tomi Valkeinen <> |
| |
Hi,
On 19/11/2022 19:30, 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. > > Add IO CTRL support and control the OLDI TX power for AM625. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > drivers/gpu/drm/tidss/tidss_dispc.c | 55 ++++++++++++++++++------ > drivers/gpu/drm/tidss/tidss_dispc_regs.h | 37 +++++++++++----- > 2 files changed, 70 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c > index 472226a83251..f26129fb1d8f 100644 > --- a/drivers/gpu/drm/tidss/tidss_dispc.c > +++ b/drivers/gpu/drm/tidss/tidss_dispc.c > @@ -930,21 +930,52 @@ 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_SINGLE_LINK_SINGLE_MODE: > + /* Power down OLDI TX 1 */ > + val = AM625_OLDI1_PWRDN_TX; > + break; > + > + case OLDI_SINGLE_LINK_CLONE_MODE: > + case OLDI_DUAL_LINK_MODE: > + /* No Power down */ > + val = 0; > + break; > + > + default: > + /* Power down both the OLDI TXes */ > + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX; > + break; > + } > + } else { > + /* Power down both the OLDI TXes */ > + val = AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX; > + } > + > + regmap_update_bits(dispc->oldi_io_ctrl, AM625_OLDI_PD_CTRL, > + AM625_OLDI0_PWRDN_TX | AM625_OLDI1_PWRDN_TX, val); > + } > } > > static void dispc_set_num_datalines(struct dispc_device *dispc, > @@ -2841,7 +2872,7 @@ int dispc_init(struct tidss_device *tidss) > dispc->vp_data[i].gamma_table = gamma_table; > } > > - if (feat->subrev == DISPC_AM65X) { > + if (feat->oldi_supported) { > r = dispc_init_am65x_oldi_io_ctrl(dev, dispc); > if (r) > return r;
I think it makes more sense to test the SoC version here, rather than the generic "oldi_supported". And if the same function is used for am625, maybe rename the func to "_am6xx_".
Other than that:
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
| |