On 11/02/2025, Marek Vasut wrote:
> On 10/20/25 4:15 AM, Ying Liu wrote:
>
> Hello Liu,
Hello Marek,
>
>>>>> I wanted to put this whole thing on the list first, before I start
>>>>> splitting it
>>> up.
>>>>>
>>>>> For starters, I think I can send these separately:
>>>>
>>>> Before discussing how to split, a bigger question is that is it fine to
>>>> support both i.MX8qxp DC and i.MX95 DC in the same imx8_dc_drm
>>> module?
>>>> Separate modules look more reasonable to me, considering the fact that
>>>> there are quite a lot difference between the two DCs.
>>>
>>> (maybe I do not quite understand your suggestion with "separate module",
>>> I assume this means entirely duplicate driver, is that correct? I
>>> operate with that assumption in the text below.)
>>
>> I'd expect separate modules: the existing imx8_dc_drm(which can be
>> modprobe'd) and something like imx95_dc_drm. I wouldn't call them
>> *entirely* duplicated drivers since I mentioned common part of the DCs
>> could be wrapped as helpers in an additional module(something like
>> imx_dc_drm_common).
>
> How would this look if everything is built into the kernel ?
Something like imx8qm_ldb, imx8qxp_ldb and imx_ldb_helper modules -
DRM_IMX8QM_LDB and DRM_IMX8QXP_LDB select DRM_IMX_LDB_HELPER.
Note you may make CONFIG_DRM_IMX8QM_LDB=y and CONFIG_DRM_IMX8QXP_LDB=m with
CONFIG_DRM_IMX_LDB_HELPER=y.
>
>>> This series indicates that the functional units in the DC are basically
>>> identical, with the majority of changes being register base addresses of
>>> the whole DC and an odd bit or register offset here and there. Most of
>>> the code can be reused, as can be seen in the first half of the series.
>>
>> The major differences between the i.MX95 and i.MX8qxp DCs are
>> different components in Display Engines(especially the additional
>> Domain Blend Unit in i.MX95 DC)
>
> This seems to be one component in the whole pipeline,
Yes, DB is i.MX95 DC component, but not in i.MX8qxp DC.
> with links going into it. Those links are already in place.
DB's inputs are not configurable. They are ED0/1/4/5, going into DB0/1's
primary and secondary inputs. So, DB is nothing to do with LINK_ID_xxx.
> Maybe I am not seeing the specialty of this particular DB component ?
I commented on patch 1 earlier:
-8<-
Add more description about the unit according to i.MX95 DC IP spec:
The unit operates in four modes:
- Primary mode: The primary input is used for output.
- Secondary mode: The secondary input is used for output.
- Blend mode: Primary and secondary inputs are blended, according to the
programmed blending functions.
- SidebySide mode: Primary and secondary streams are projected side by side,
i.e., primary video on the left side and secondary on the
right.
-8<-
The four modes make DB really special.
>
>> plus i.MX8qxp DC's capability
>> to connect with the prefetch engines(DPRC & PRG).
>
> Isn't the prefetch engine only a bit of register configuration,
Yes, but maybe more - See my prefetch engine patches, there are quite a few
logics implemented(especailly in CRTC driver) to make sure proper sequence
to enable or disable prefetch engine.
> that can be abstracted out ?
How to abstract that out?
>
>> Both would
>> have significant impact on how we implement the drivers. We'll
>> certainly end up having different implementations for callbacks to
>> enable/disable CRTCs or update/disable planes.
>
> It doesn't seem they have to be entirely different so far, do they ?
Well, if you add really limited features, say just primary planes
without prefetch engine support, then I'd say they are not entirely
different. But if you consider to add prefetch engine, overlay planes
and those HW differences(especially DB), they are.
>
>> The sort of minor difference is in Pixel Engine(including Blit Engine)
>> where FethUnit types and numbers are different plus different numbers
>> of Scaler Engine. I'd expect logics to allocate FetchUnits for planes can be
>> implemented in the imx_dc_drm_common module
>
> This part is fine, but ultimately, it would have to be possible to build
> both the MX8Q and MX95 drivers into the kernel too. And then there would
> have to be some logic to deal with that, likely based on driver data, as
> it is done here.
>
>>> The addition of iMX95 into the iMX8QXP DC also does not seem to be
>>> making the driver in any way more complicated.
>>
>> Disagree. The addition would introduce quite a few i.MX95 or i.MX8qxp
>> DC specific code branches due to the differences mentioned above.
>
> This does not need to be the case, see the first half of this series ?
The first half kind of handles difference for some shared i.MX95 & i.MX8QXP
DC components. I'd expect the common part can be put into imx_dc_drm_common
module. The CRTC and plane drivers in the second half would really be
quite different.
>
>> I'd say i.MX95 DC support could be in drivers/gpu/drm/imx/dc, but it
>> needs to be in a separate module like again imx95_dc_drm.
>>
>> This makes feel that the debate here becomes kind of similar to what
>> we did for single mxsfb module vs mxsfb + imx_lcdif separate modules...
>
> The mxsfb (the old iMX23 LCDIF) and lcdifv3 (the new iMX8MP/iMXRT LCDIF)
> are entirely different hardware IPs, the former is from SigmaTel , the
> later is from somewhere else, right ?
Well, I'd say they shared some HW IP logics. I bet quite a few registers
are shared. After all, you the one who tried to support both i.MX23 LCDIF
and i.MX8MP LCDIFv3 in one single mxsfb module in the first place :-P
But now it ends up with mxsfb + imx_lcdif separate modules.
>
>>> What would be the benefit of having duplicate driver for IP that is
>>> basically identical, for i.MX95 ?
>>
>> Cleaner driver implementation and easier to maintain. I don’t want
>> to test both i.MX95 and i.MX8qxp platforms when only either
>> i.MX95 DC specific or i.MX8qxp DC specific code is changed.
>>
>> But again, they won't be entirely duplicated drivers. Common
>> part could be shared between the drivers with software techniques,
>> like the imx_dc_drm_common module mentioned above.
> Please have a look at the first part of this patchset, there do not
> seem to be that many new or complex branches.
Like I said above, I'd expect the common part of the first halp can be
put into imx_dc_drm_common module.
--
Regards,
Liu Ying
