On 6/9/26 10:26 AM, Liu Ying wrote:
Hello Liu,
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;
And then wherever is prefetch used, do
if (something->prefetch_op)
something->prefetch_op()
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 ?
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 ?
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.
(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.
