-----Original Message----- From: Marek Vasut marex@denx.de Sent: 2022年3月2日 10:50 To: Robby Cai robby.cai@nxp.com; Lucas Stach l.stach@pengutronix.de; Adam Ford aford173@gmail.com Cc: Ying Liu (OSS) victor.liu@oss.nxp.com; dri-devel dri-devel@lists.freedesktop.org; devicetree devicetree@vger.kernel.org; Peng Fan peng.fan@nxp.com; Alexander Stein alexander.stein@ew.tq-group.com; Rob Herring robh+dt@kernel.org; Laurent Pinchart laurent.pinchart@ideasonboard.com; Sam Ravnborg sam@ravnborg.org Subject: Re: [EXT] Re: [PATCH 1/9] dt-bindings: mxsfb: Add compatible for i.MX8MP
Caution: EXT Email
On 3/1/22 14:37, Robby Cai wrote:
Hi,
[...]
I tend to agree with Marek on this one. We have an instance where the blk-ctrl and the GPC driver between 8m, mini, nano, plus are close, but different enough where each SoC has it's own set of tables and some checks. Lucas created the framework, and others adapted it for various SoC's. If there really is nearly 50% common code for the LCDIF, why not either leave the driver as one or split the common code into its own driver like lcdif-common and then have smaller drivers that handle their specific variations.
I don't know exactly how the standalone driver looks like, but I guess the overlap is not really in any real HW specific parts, but the common DRM boilerplate, so there isn't much point in creating a
common lcdif driver.
As you brought up the blk-ctrl as an example: I'm all for supporting slightly different hardware in the same driver, as long as the HW interface is close enough. But then I also opted for a separate 8MP blk-ctrl driver for those blk-ctrls that differ significantly from the others, as I think it would make the common driver unmaintainable trying to support all the different variants in one driver.
Regards, Lucas
LCDIF on i.MX8MP is a different IP which is borrowed from non-iMX series,
although it's also called 'LCDIF'.
We prefer not mix these two series of IPs in one driver for ease of
maintenance and extension.
Where does the MX8MP LCDIF come from then, SGTL maybe ?
AFAIK, it's RT1170. You may have a check on RM [1]. Interestingly, this SoC has both eLCDIF and LCDIFv2, two IPs we are talking about.
[1] https://www.nxp.com/webapp/Download?colCode=IMXRT1170RM
Regards, Robby