Re: [RFC] drm/imx: upstream direction for i.MX95 display support

[Liu Ying <victor.liu@nxp.com>]

On Mon, Jul 06, 2026 at 08:31:42PM +0530, Piyush Patle wrote:
> On Mon, Jul 6, 2026 at 4:00 PM Liu Ying <victor.liu@nxp.com> wrote:
> >
> > On Mon, Jul 06, 2026 at 03:15:24PM +0530, Piyush Patle wrote:
> > > On Mon, Jul 6, 2026 at 8:07 AM Liu Ying <victor.liu@nxp.com> wrote:
> > > >
> > > > On Sun, Jul 05, 2026 at 03:33:51AM +0530, Piyush Patle wrote:
> > > > > On Fri, Jun 26, 2026 at 3:22 PM Liu Ying <victor.liu@nxp.com> wrote:
> > > > > >
> > > > > > On Wed, Jun 24, 2026 at 03:33:18PM +0530, Piyush Patle wrote:
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > The current dc/ implementation is a multi-device component driver
> > > with one
> > > > > > > platform_driver per block bound via the component framework. The
> > > downstream
> > > > > > > i.MX95 driver is a single monolithic platform_driver mapping all
> > > blocks from
> > > > > > > one register base. Unifying appears to require reconciling two bind
> > > models,
> > > > > > > rather than only adding match_data.
> > > > > >
> > > > > > I think that upstream i.MX95 display controller driver would also be
> > > based
> > > > > > on the component helper.  That's something for sure.
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > There is also anticipated divergence which is not yet upstream
> > > (i.MX8QXP
> > > > > > > prefetch/PRG, LTS and tiling modifiers, and the downstream i.MX95
> > > blit
> > > > > > > engine), although mainline dc/ is KMS-only today.
> > > > > >
> > > > > > Just want to point out that I sent out v5 patch set[2] to add i.MX8QXP
> > > > > > prefetch engine(DPRC + PRG) support for KMS.  That changes the
> > > driver's
> > > > > > mode setting code a lot.
> > > > > >
> > > > > > [2]
> > > https://lore.kernel.org/all/20251027-imx8-dc-prefetch-v5-0-4ecb6c6d4941@nxp.com/
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > One question for Liu Ying is whether the separate-driver plus shared
> > > > > > > helper-library approach is still the preferred direction, and where
> > > the
> > > > > > > helper boundary would be drawn (which blocks/ops are shared versus
> > > > > > > implemented per driver).
> > > > > >
> > > > > > Yes, separate DRM drivers + a helper library approach is still the
> > > direction
> > > > > > I want.  I think that the drivers and library would sit in the same
> > > > > > directory drivers/gpu/drm/imx/dc/.
> > > > > >
> > > > > > The purpose to add a library is to share code to reduce overall code
> > > lines.
> > > > > > I'd assume that shared blocks or common part of slightly different
> > > blocks
> > > > > > should be covered by the library.
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > how the component and monolithic driver models
> > > > > > > would be reconciled given the differences described above.
> > > > > >
> > > > > > Like I said above, I don't think upstream driver would be monolithic.
> > > > > >
> > > > > > --
> > > > > > Regards,
> > > > > > Liu Ying
> > > > >
> > > > > Hi Liu,
> > > >
> > > > Hi Piyush,
> > > >
> > > > >
> > > > > A quick follow-up with some progress.
> > > > > I went ahead with the refactoring we discussed and now have it working
> > > on the
> > > > > i.MX95 15x15 FRDM, following the component-based model.
> > > > > The common implementations for ConstFrame, ExtDst, LayerBlend, FrameGen
> > > and
> > > > > the FetchUnit base have been moved into shared dc-lib-* helpers, with
> > > >
> > > > Can you please share the code in a public place, like github?
> > > >
> > >
> > > Sure. I've pushed the current work-in-progress branch here:
> > > https://github.com/PiyushPatle26/linux-mecha-im95-wip/tree/imx95-dc-preview
> >
> > I gave this branch a quick look.  The major problem is that you are using
> > the same 'struct drm_driver dc_drm_driver' for i.MX95 and i.MX8QXP display
> > controllers instead of using separate DRM drivers.  From userspace's POV,
> > they should see different driver names(drm_driver.name).  From drivers'
> > POVs, they should implement different mode setting callbacks.
> 
> I've reworked it based on your feedback and updated version to the same branch
> I've split them into separate DRM drivers.
> The shared dc_drm_driver and dc-drv-common.c are gone. i.MX8QXP and i.MX95
> now each have their own struct drm_driver, component bind path, probe logic and
> mode-setting callbacks. Userspace now also sees different driver names
> for the two SoCs.

It seems that there are common stuff that can be shared between dc-drv-imx95.c
and dc-drv-imx8qxp.c.

Please don't rename driver name from "imx8-dc" to "imx8qxp-dc".  Diver name
is supposed to be stable ABI.  "imx8-dc" implies that the driver would support
both i.MX8QXP display controller and i.MX8QM display controller which have
some H/W differences but still fit into a same driver.

I'm not sure why you mention 'DC8000' in your code.  We don't have this term
internally to describe the display controllers.

Those i.MX95 clocks used by dc-drv-imx95.c are not correct.  I'd assume only
cfg/axi/ocram clocks are needed.

> 
> The mode_valid, atomic_check, atomic_begin and atomic_flush callbacks
> were already
> identical in the previous version, so I've left them as a small shared
> helper to avoid
> duplicating the same code. If they diverge later, I'll split them into
> separate implementations.

It seems that mode-setting callbacks in dc-plane.c are still shared, which
seems to be worth splitting.

> 
> >
> > It looks like it's not worth extracting common code into those dc-lib-* files.
> > Each dc-[block].c is a library itself, though I could be wrong - after all,
> > I haven't find a time slice to add i.MX95 blocks to them and see how they
> > will be.
> >
> 
> Agreed. I removed the dc-lib-* split. The shared register code is now placed
> directly inside each dc-[block].c file, so every block stays
> self-contained while
> still being reused by both drivers where needed.

Didn't look into dc-[block].c files in detail.  Maybe, things need to be
improved there too.

> 
> > The idea is to share some code between the two DRM drivers *naturally*.
> >
> > >
> > > The files you asked about are:
> > > drivers/gpu/drm/imx/dc/dc-drv-common.c
> > > drivers/gpu/drm/imx/dc/dc-lib-*.c
> > > One thing I'd like to point out is that the pixel interleaver, display pixel
> > > link, LDB and LVDS PHY drivers are ports of the existing NXP downstream
> > > drivers. The new work in this branch is mainly the DC refactoring into the
> > > component model and the shared helper library.
> > > > > SoC-specific register layouts and tables supplied as data. The per-SoC
> > > glue is
> > > > > split into dc-drv-common.c, dc-drv-imx8qxp.c and dc-drv-imx95.c, while
> > > >
> > > > Same here, just want to see how dc-drv-common.c and dc-lib-* would look
> > > like.
> > > >
> > > The files you mentioned are available under:
> > > drivers/gpu/drm/imx/dc/dc-drv-common.c
> > > drivers/gpu/drm/imx/dc/dc-lib-*.c
> > > The helper library currently contains the common implementations for
> > > ConstFrame, ExtDst, LayerBlend, FrameGen and the FetchUnit base.
> > >
> > > > > i.MX95-specific blocks such as DomainBlend, Dither, FetchEco, FetchYUV,
> > > > > HScaler and VScaler remain separate component drivers.
> > > >
> > > > i.MX8QXP display controller also has Dither, FetchEco, HScaler and
> > > Vscaler,
> > > > so they are not i.MX95-specific blocks.  To add minimal feautures at
> > > first,
> > > > we don't need to support them as of now.  i.MX95-specific FetchYUV is
> > > similar
> > > > to i.MX8QXP-specific FetchDecode, so maybe FetchYUV won't be a separate
> > > > component driver at the end of the day.  To start up small, I think the
> > > > must-have DomainBlend and Dither(for Data-Enable, HSync and VSync polarity
> > > > controls) and two FetchLayers can be enabled to support two display
> > > pipelines.
> > > >
> > >
> > > Okay, I've reduced the implementation accordingly. The current branch
> > > only keeps DomainBlend, Dither and two FetchLayers. FetchEco, FetchYUV,
> > > HScaler and VScaler have been dropped for now.
> > > > BTW, do you have the i.MX95 TRM?  If no, then it would be difficult for
> > > you
> > > > to add DT bindings for *all* blocks.  Note that the DT bindings are
> > > supposed
> > > > to be complete at the first place.
> > > >
> > >
> > > Yes, I do.
> > >
> > > > >
> > > > > The implementation is functional on the FRDM over the pipeline
> > > > > DPU -> pixel-interleaver -> pixel-link -> LDB -> LVDS PHY -> IT6263 ->
> > > HDMI
> > > >
> > > > As I said before in separate mail thread, pixel link is not used in the
> > > LVDS
> > > > display pipeline, but instead it's only used in the MIPI DSI display
> > > pipeline.
> > > >
> > >
> > > Thanks for pointing that out. After checking the TRM, I updated the DT graph
> > > so the LVDS pipeline is now:
> > >
> > > DC -> Pixel-interleaver -> LDB -> LVDS PHY -> IT6263 -> HDMI
> >
> > Yes, this pipeline is correct.
> >
> > >
> > > Removing display_pixel_link from the LVDS path also required moving the
> > > bridge
> > > format translation into the pixel interleaver driver.
> > > > >
> > > > > EDID is read successfully, the initial modeset works for all tested
> > > modes
> > > > > (1920x1080@60, 1280x720@60, 720x480 and 640x480), and Weston and sway
> > > both run.
> > > > >
> > > > > There are still two known limitations. The initial modeset works, but
> > > > > subsequent mode changes currently wedge the ExtDst content shadow load,
> > > which
> > > > > appears related to the lack of a reset path for the i.MX95 FrameGen.
> > > >
> > > > Not sure what's the cause of the issue, but FrameGen needs to be disabled
> > > and
> > > > then re-enabled if you want to do a full modeset.  I'd say that's not a
> > > reset
> > > > path.  This critical issue needs to be fixed before posting formal
> > > patches.
> > > >
> > >
> > > Understood. I'll continue investigating the modeset issue before posting the
> > > formal patch series.
> > >
> > > > > In
> > > > > addition, the DSI path is implemented on the DC side but remains
> > > unvalidated.
> > > >
> > > > It's fine to support the LVDS display pipeline first as long as it doesn't
> > > > do harm to the MIPI DSI display pipeline support.  So, maybe drop the
> > > latter
> > > > first.
> > >
> > > I've done that in the current branch. It now focuses only on the validated
> > > LVDS pipeline.
> > >
> > > >
> > > > >
> > > > > The implementation is currently based on v7.2-rc1 together with your
> > > prefetch
> > > > > v5 series.
> > > > >
> > > > > The work is now ready to be posted as patch series: Adding LVDS/DSI
> > > > > i.MX95 support.
> > > > >
> > > > >
> > > > > Regards,
> > > > > Piyush Patle
> > > >
> > > > --
> > > > Regards,
> > > > Liu Ying
> > >
> > > Thanks again for taking the time to review this.
> > >
> > > Regards,
> > > Piyush Patle
> >
> > --
> > Regards,
> > Liu Ying
> 
> framegen re-tune issue is still unresolved.
> I'll fix that before posting the formal patch series.

I need to point out that i.MX95 display controller also has 2 display engines
and 1 pixel engine, which is something missing in your code.  This makes me
think that you have no enough i.MX95 display controller documents.

Generally speaking, I'd say there is a lot to be improved before posting the
formal patch series.  Gut feeling is that maybe it's a bit too difficult
for you to get the drivers right.  As I said earlier, my original plan is to
add i.MX8QXP prefetch engine support first with the posted patch series and
then to add i.MX95 display controller support, but maybe I can try to add the
latter first - just need to find a time slot.  However, adding prefetch engine
support first would directly show a reason to people why two DRM drivers are
warranted when adding i.MX95 display controller support.

> 
> Regards,
> Piyush Patle

-- 
Regards,
Liu Ying