Hi Devarsh, Thanks for the patches. On 31/12/24 14:34, Devarsh Thakkar wrote: > Enable display for AM62L DSS [1] which supports only a single display > pipeline using a single overlay manager, single video port and a single > video lite pipeline which does not support scaling. > > The output of video port is routed to SoC boundary via DPI interface and > the DPI signals from the video port are also routed to DSI Tx controller > present within the SoC. > > [1]: Section 11.7 (Display Subsystem and Peripherals) > Link : https://www.ti.com/lit/pdf/sprujb4 > > Signed-off-by: Devarsh Thakkar <devarsht@xxxxxx> > --- > drivers/gpu/drm/tidss/tidss_dispc.c | 34 +++++++++++++++++++++++++++++ > drivers/gpu/drm/tidss/tidss_dispc.h | 2 ++ > drivers/gpu/drm/tidss/tidss_drv.c | 1 + > 3 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c > index cacb5f3d8085..cd322d60b825 100644 > --- a/drivers/gpu/drm/tidss/tidss_dispc.c > +++ b/drivers/gpu/drm/tidss/tidss_dispc.c > @@ -376,6 +376,35 @@ const struct dispc_features dispc_am62a7_feats = { > .vid_order = { 1, 0 }, > }; > > +const struct dispc_features dispc_am62l_feats = { > + .max_pclk_khz = { > + [DISPC_VP_DPI] = 165000, The TRM mentions that the max the VP PLL can go is 150MHz, so maybe you might need to update this. That said, as far as I understand, the IP itself can support 165 MHz, and I am wondering, what would we do if there comes out a new SoC that uses AM62L DSS as is, but just bumps up the PLL capacity to 165MHz. It would be odd to have a ditto feats structure with just the frequency updated. > + }, > + > + .subrev = DISPC_AM62L, > + > + .common = "common", > + .common_regs = tidss_am65x_common_regs, Also, I don't think we should utilize this as is. The AM62L common regions is different, and is, at best, a subset of the AM65x/AM62x common register space. For example, registers VID_IRQ{STATUS, ENABLE}_0 have been dropped, along with VP_IRQ{STATUS, ENABLE}_1. - Which brings to my next concern... > + > + .num_vps = 1, > + .vp_name = { "vp1" }, > + .ovr_name = { "ovr1" }, > + .vpclk_name = { "vp1" }, > + .vp_bus_type = { DISPC_VP_DPI }, > + > + .vp_feat = { .color = { > + .has_ctm = true, > + .gamma_size = 256, > + .gamma_type = TIDSS_GAMMA_8BIT, > + }, > + }, > + > + .num_planes = 1, > + .vid_name = { "vidl1" }, > + .vid_lite = { true }, > + .vid_order = { 0 }, ... Usually, VID1 is the first video pipeline, while VIDL1 remains the second. Which is why vid1 occupies the index 0, and vidl1 occupies 1 - even from the hardware perspective. While AM62L has only one video (lite) pipeline - vidl1, and there is no vid1, the hardware still treats vidl1 at index 1. For example, the TRM has defined DISPC_VID_IRQ{STATUS, ENABLE}_1 (and not _0) in the common region. So, the vid_order here should be 1, not 0. Regards Aradhya
