On 10/14/2025, Marek Vasut wrote:
> On 10/14/25 10:51 AM, Liu Ying wrote:
>> Hi Marek,
> 
> Hi,
> 
>> On 10/11/2025, Marek Vasut wrote:
>>> This large series adds support for the i.MX95 display pipeline, including
>>> DPU, DSI and LVDS support. Most of the components extend existin drivers,
>>> DPU is added into DC driver, DSI into iMX93 DSI driver, LVDS into iMX8MP
>>> LDB. Pixel link and pixel interleaver drivers are reworked to work as two
>>> independent channels, since there seems to be no dependency between their
>>> two channels. The i.MX95 DTSI changes are also included.
>>>
>>> Since the DPU chapter is missing from the i.MX95 RM, this is based on the
>>> NXP downstream kernel fork code and there might be issues.
>>>
>>> Majority of this series are DPU patches on top of the DC driver, I tried
>>> to keep them separate and easy to review. Later part adds LVDS and DSI
>>> support, this can be split into separate series.
>>
>> Like you said that this patch series is large, please split it.
>> Also, make sure proper maintainers are in TO or CC lists for each patch(b4
>> tool should do that automatically for you), e.g., patch 37 should be sent
>> to Thomas Gleixner <[email protected]> according to MAINTAINERS.
> 
> I had to trim down the CC list for this series, it was enormous.
> 
> 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.

> 
> - drm/imx: dc: Use bulk clock

I don't think this one is needed because reach relevant block has only one
clock.

> - drm/imx: dc: Rework dc_subdev_get_id() to drop ARRAY_SIZE() use

It doesn't seem that this one is needed either, if we use separate modules.

> - drm/imx: dc: Rename i.MX8QXP specific Link IDs

TBH, I'm not a big fan of adding LINK_ID_x_MXy to enum dc_link_id, since
the members may have the same value and it's kind of a mess considering
future SoCs.

> - drm/imx: Add more RGB swizzling options

This one seems ok.

> - dt-bindings: interrupt-controller: fsl,irqsteer: Add i.MX95 support

Ditto.

> 
> Then in second round, probably all these clean ups:
> 
> - drm/imx: dc: *: Pass struct dc_*_subdev_match_data via OF match data

Same, doesn't seem needed, if we use separate modules.

> 
> And then rest afterward.
> 
> What do you think ?

I kind of opt to separate modules.  Maybe, to save some code, an additional
module can be introduced to wrap common part as helpers, plus some callback
magics, like fg->dc_fg_cfg_videomode().

-- 
Regards,
Liu Ying

Reply via email to