On 3/2/22 10:23, Lucas Stach wrote:
[...]
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.
The mxsfb currently has 1280 LoC as of patch 8/9 of this series. Of
that, there is some 400 LoC which are specific to old LCDIF and this
patch adds 380 LoC for the new LCDIF. So that's 800 LoC or ~60% of
shared boilerplate that would be duplicated .
That is probably ignoring the fact that the 8MP LCDIF does not support
any overlays, so it could use the drm_simple_display_pipe
infrastructure to reduce the needed boilerplate.
It seems the IMXRT1070 LCDIF v2 (heh ...) does support overlays, so no,
the mxsfb and hypothetical lcdif drivers would look really very similar.
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.
But then you also need to maintain two sets of boilerplate, they
diverge, and that is not good.
I don't think that there is much chance for bugs going unfixed due to
divergence in the boilerplate, especially if you use the simple pipe
framework to handle most of that stuff for you, which gives you a lot
of code sharing with other simple DRM drivers.
But I can not use the simple pipe because overlays, see imxrt1070 .
[...]
We can always split the drivers later if this becomes unmaintainable
too, no ?
