On Mon, 2022-02-28 at 07:57 +0100, Marek Vasut wrote: > On 2/28/22 07:37, Liu Ying wrote: > > Hi Marek, > > Hi, > > > On Mon, 2022-02-28 at 01:45 +0100, Marek Vasut wrote: > > > Add compatible string for i.MX8MP LCDIF variant. This is called LCDIFv3 > > > and is completely different from the LCDIFv3 found in i.MX23 in that it > > > > In i.MX23 reference manual, there is no LCDIFv3 found, but only LCDIF. > > See i.MX23 HW_LCDIF_VERSION MAJOR=0x3 , that's LCDIF V3 . MX28 has LCDIF > V4 . Ok, got it now. AFAIK, the SoC design team calls i.MX8MP display controller as 'LCDIFv3'. Those in other SoCs are called 'LCDIF'. There is not even a register in i.MX8MP display controller to decribe the version. > > > > has a completely scrambled register layout compared to all previous LCDIF > > > > It looks like no single register of i.MX8MP LCDIFv3 overlaps with > > registers in other i.MX2x/6x/7x/8x LCDIFs. The LCDIFv3 block diagram is > > totally different from the LCDIF block diagram, according to the SoC > > reference manuals. LCDIFv3 supports SHADOW_EN bit to update horizontal > > and vertical size of graphic, position of graphic on the panel, address > > of graphic in memory and color formats or color palettes, which is not > > supported by LCDIF and impacts display driver control mechanism > > considerably. LCDIF supports DOTCLK interface, MPU interface and VSYNC > > interface, while LCDIFv3 only supports parallel output as a counterpart > > of the DOTCLK interface. > > > > Generally speaking, LCDIFv3 is just a new display IP which happens to > > have the word 'LCDIF' in its name. Although both of LCDIFv3 and LCDIF > > are display controllers for scanning out frames onto display devices, I > > don't think they are in one family. > > > > So, LCDIFv3 deserves a new separate dt-binding, IMO. > > It seems to me a lot of those bits just map to their previous > equivalents in older LCDIF, others were dropped, so this is some sort of > new LCDIF mutation, is it not ? I say 'LCDIFv3' and 'LCDIF' are totally two IPs, if I compare the names of registers and the names of register bits . > > I am aware NXP has a separate driver in its downstream, but I'm not > convinced the duplication of boilerplate code by introducing a separate > driver for what looks like another LCDIF variant is the right approach. Hmmm, given the two IPs, I think there should be separate drivers. With one single driver, there would be too many 'if/else' checks to separate the logics for the IPs, just like Patch 9/9 does. The boilerplate code to do things like registering a drm device is acceptable, IMO. Aside from that, with separate drivers, we don't have to test too many SoCs if we only want to touch either 'LCDIFv3' or 'LCDIF'. > > > > variants. The new LCDIFv3 also supports 36bit address space. However, > > > except for the complete bit reshuffling, this is still LCDIF and it still > > > works like one. > > [...]