Re: [PATCH 1/9] dt-bindings: mxsfb: Add compatible for i.MX8MP

[Marek Vasut <marex@denx.de>]

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 ?