On Wed, Jun 10, 2026 at 06:31:02PM +0200, Marek Vasut wrote:
> On 6/9/26 10:26 AM, Liu Ying wrote:
> 
> Hello Liu,

Hello Marek,

> 
> > > > > I brought this series up on the i.MX95 15x15 FRDM (IT6263 
> > > > > LVDS-to-HDMI on
> > > > > LVDS ch1). It mostly works, but I ran into a few issues around DI 
> > > > > routing,
> > > > > LVDS format handling, and DC enable sequencing which needed rework 
> > > > > before
> > > > > HDMI would come up reliably on the board.
> > > > > 
> > > > > I don't see a v2 of the series and things seem to have been quiet 
> > > > > since
> > > > > November. Are you planning to post an updated version?
> > > > 
> > > > My plan was to enable prefetch engine support[1] for i.MX8QXP display
> > > > controller and add device tree for a whole i.MX8QXP LVDS display 
> > > > pipeline,
> > > > before adding i.MX95 display controller support.
> > > > 
> > > > Unfortunately, it seems that Marek is not a big fan of [1]
> > > 
> > > I am fine with [1] as long as it can be isolated and does not affect every
> > > SoC that might reuse this driver, which I think it can be done.
> > 
> > How can it be isolated?
> 
> if (compatible("mx8q"))
>   something->prefetch_op = somefunction;

I certainly cannot add this to [1], because currently the driver only supports
i.MX8QXP display controller.

> 
> And then wherever is prefetch used, do
> 
> if (something->prefetch_op)
>   something->prefetch_op()

These 'if' checks can be avoided, if i.MX95 display controller is supported
by a separate driver.  The checks are just telling us that the i.MX8QXP and
i.MX95 display controllers are different from H/W point of view - i.MX8QXP
SoC has prefetch engines, while i.MX95 SoC has not.  Of course, there are a
lot more H/W differences, hence it's worth separate drivers.  Without
prefetch engines to do tile to linear resolving for framebuffers, i.MX95
display controller's DRM planes cannot advertise tile specific modifiers.
The separate driver's IOCTL designs for blit-engine could also be different,
because of the prefetch engine and no Linear Tile Store(LTS) [2] attached to
i.MX95 display controller Store unit's output.  Userspace could easily check
the DRM driver names to get the IOCTLS that a particular driver supports.

[2] 
https://elixir.bootlin.com/linux/v7.0/source/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-store.yaml#L73-L76

> 
> Or something along those lines ?
> 
> > > > and I'm busy
> > > > with downstream development so the plan doesn't move forward well.  I 
> > > > still
> > > > think [1] makes sense(maybe I need to rebase it on latest 
> > > > drm-misc-next),
> > > > so I'd like to see review comments on [1] and hopefully people think 
> > > > that
> > > > the overall idea of [1] is ok.
> > > 
> > > My only concern is, to keep it isolated to MX8Q, so this driver can be
> > > reused by MX95.
> > > 
> > > > > I've accumulated a fair amount of rework while getting this running 
> > > > > on the
> > > > > FRDM. If you're not planning a v2, I can clean things up and send one 
> > > > > based
> > > > > on the current series.
> > > > 
> > > > I still think that i.MX95 display controller driver should be in a 
> > > > separate
> > > > driver, rather than sharing the same driver with i.MX8QXP display 
> > > > controller
> > > > like this patch series does, because the two display controllers are 
> > > > quite
> > > > different as I mentioned in comments on this patch series and in 
> > > > discussion
> > > > in [1].  Also, the common part between the two display controllers 
> > > > should
> > > > be extracted to a common helper library as I mentioned there too.
> > > Are they really? It seems this series adds support for the MX95 DC without
> > > that many changes, so are the DCs really that different ? It seems the 
> > > MX95
> > > DC is simply a reuse/evolution of the MX8Q DC blocks, so duplicating the
> > > code seems like the wrong direction, it will only lead to disparate sets 
> > > of
> > > bugs in two drivers, which isn't desired.
> > 
> > I pointed out a lot of H/W differences between the two display controllers
> > during the discussions for this patch series and my i.MX8QXP prefetch engine
> > patch series[1].  Please take a look at [1], which clearly shows that the
> > prefetch engine would considerably impact CRTC/plane atomic callback
> > implementations.
> 
> Is the prefetch engine actually grown into the CRTC/DE or not ? I suspect it
> is separate and instead part of the built-in DMA, right ?

It is separate and not grown into CRTC/DE.
It sits between DRAM and pixel engine's or blit engine's fetchunit.

> 
> > Display controller internal blocks would also impact
> > the implementations, e.g., DomainBlend block in i.MX95 display controller
> > doesn't present in i.MX8QXP display controller.  It makes sense to use
> > separate drivers for the two display controllers instead of adding 'if/else'
> > checks to a single driver's atomic callbacks or introducing two pairs of
> > atomic callbacks to that single driver.  I mentioned before, the code to
> > simply add a DRM driver(struct drm_driver) is fairly limited.
> 
> Can't we simply have two sets of ops (one for mx8q and one for mx95) for
> those ops which are too complicated to implement as a single op with if/else
> statements ?

Technically, we can, but using separate drivers is a better design:
- no 'if/else' checks to advertise those tile specific modifiers or not.
- better IOCTL designs for blit-engine with different DRM driver names.
- again, the code to add a DRM driver(struct drm_driver) is fairly limited.

> 
> > I also mentioned before that separate drivers make them easier to maintain:
> > we don't have to test both i.MX8QXP and i.MX95 if only one display 
> > controller
> > specific code is changed.
> 
> The downside is lack of code reuse, which leads to disparate sets of bugs in
> these two drivers and code duplication. And it seems to me, that large parts
> of the MX8Q and MX95 DC are effectively identical.

A common helper library achieves code reuse, so that's not a downside.

I'd say a lot of display controller internal units are identical, but H/W
differences would significantly impact driver implementations:
- i.MX95-only display controller DomainBlend unit supports 4 different modes,
  which impacts plane/CRTC drivers.
- i.MX8QXP-only prefetch engine impacts plane/CRTC drivers and blit-engine
  driver/IOCTLs.
- i.MX8QXP-only LTS impacts blit-engine driver/IOCTLs.

> 
> > > (I might not fully understand what you have in mind with the helper 
> > > library
> > > though?)
> > 
> > I said this could be something like imx-ldb-helper.c and plus perhaps some
> > callbacks like fg->dc_fg_cfg_videomode().
> Do you perceive that the DC driver cannot be parametrized easily enough that
> it has to be turned into a library like that ? When I look at this patchset,
> esp. the first half which updates the various blocks, it does not seem to me
> that way.

Yes, I do perceive that.  Your patchset touches mostly the identical display
controller units(DomainBlend is an exception and could impact plane/CRTC
drivers a lot) and enables kind of minimal features.  That's probaly why you
feel that way.  If think more about the different part, probably it will make
you feel the other way around.

-- 
Regards,
Liu Ying

Reply via email to