On 11/4/26 10:46, CK Hu (胡俊光) wrote:
On Tue, 2026-07-14 at 13:43 +0200, AngeloGioacchino Del Regno wrote:
In preparation for adding support for newer, more complex, display
controllers like the ones found in the MediaTek Kompanio Ultra and
Genio Pro 5100 (MT8196/MT8894), add support for multi-stage layers
as in newer hardware, there is a special strategy using multiple
hardware components of one (or multiple) display controller(s) to
form one single layer.
Multi-stage layers are one more step towards extra flexibility of
the Display Subsystem, making it possible to change capabilities
of Display Controller Pipelines depending on the final application
of a board: for example, one board may choose to use fewer layers
per pipeline to achieve more pipelines (more concurrent outputs),
or more layers to achieve less, but more capable (as in offloading
and in achievable bandwidth) concurrent outputs.
This currently adds boilerplate to support this hardware, but does
not yet implement the IP-specific support for MT8196/8894, which
will be done after all preparation is done.
Could you add mt8196 support based on original layer architecture first then
introduce this multi-stage architecture?
Unfortunately, no.
MT8196 cannot work with the previous code made for the previous display
controller
as it is fundamentally different from the previous ones.
This is effectively enabling support for the new MediaTek display controllers.
Without this, MT8196 would not have layers.
This series is so large and I would like to break it into multiple series.
I can break it in multiple series, but merging one series and not the other
would leave you with just dead code (as everything in here is needed to actually
get the rest to work) in mediatek-drm that would:
1. Make no sense; and
2. Maybe break before you merge the other series.
This really needs to be one big series and really needs to be merged all
together:
it is restructuring lots of stuff in mediatek-drm, and you can't really merge
in a
partial restructuring, leaving the rest floating, as that would simply create a
huge mess in the whole driver for literally no reason.
For other upstreamed SoC, the hardware also has some flexibility to route
display pipeline,
This series is already huge, if I start implementing other components for the
older
SoCs, this gets to 100+ commits, and becomes unmanageable.
Honestly, we're carrying this series in our integration kernel, and it's become
a
huge burden for us to rebase it everytime, because it's ... well, again, huge.
but we still implement the fixed pipeline first.
The spirit of this restructuring is continuing the previous restructuring work
that
I already upstreamed (where I added the OF Graph support): I remind you that the
target of that one was to remove the fixed pipeline arrays because having board
specific pipelines as arrays in code is plain wrong.
Unless "we still implement the fixed pipeline first" means "we still have it
here
for the old socs": in this case, well, ABI can't be broken, and if we remove
that
the old SoCs running on old devicetrees will get regressions (as in, display
will
not work anymore).
I'd really like to remove those big senseless arrays but there's no way: past
ABI
related mistakes will always have their ghosts haunting us here,
unfortunately...
...and this is why I added a compat layer, trying to separate those from the
main
mediatek-drm code as much as possible, because then otherwise the alternative
would
have been to clone mediatek-drm to another driver, and freeze this one, which
means
that with the technical debt that this driver has, it'd break in 5 upstream
kernel
versions and old SoCs wouldn't work anymore.
This suggestion just want to simplify the review process.
I know, and I appreciate your suggestions to allow me to make this as smooth as
possible - being a maintainer myself I can completely understand you on that,
but
this series is really that big, and really that complicated.
I can try and retry and re-retry again to make things easier for you to review
and
apply, but I don't think there's much more I can do for that.
The only thing that I can tell you here is that we've been carrying this series
since kernel 7.0.0 (included), rebased on 7.1, 7.2, 7.3, and always checked for
any regression on Genio boards and Chromebooks (8173, 8183, 8188, 8192, 8195).
All of those are working as expected and good, and have been working great for
all this time.
Well, including MT8196, of course, and Genio Pro 5100.
Ah, one last thing - this series is part 2: the first one was in soc/mediatek
which
I was able to merge, and then there are another two series that I did *not* send
yet, adding *full* support for MT8196... so this was already split!! :-)
Signed-off-by: AngeloGioacchino Del Regno
<[email protected]>
---
[snip]
+static void mtk_crtc_config_layer(struct mtk_crtc *mtk_crtc,
+ struct mtk_plane_state *plane_state,
+ int layer_num, struct cmdq_pkt *cmdq_pkt)
+{
+ u8 *layer_stages = mtk_crtc->hwlayers[layer_num].layer_stages;
+ u8 i = 0;
In mtk_ddp_comp_for_plane(), it consider mt8183 main path which two component
has OVL layers: OVL0 and OVL_2L0.
That crtc has 6 planes.
When calling mtk_ddp_comp_layer_config() with OVL_2L0, the layer_num would be 0
or 1 not 4 or 5.
So add this adjustment like mtk_ddp_comp_for_plane().
You didn't understand this code: mt8183 is effectively untouched.
The OVL_2L0 is not evaluated as a layerstage, but as a layer, exactly like the
old
code was doing.
Seriously, the code that is specific to multi-stage layers is practically doing
*nothing* for the old display controllers (other than shuffling things around in
code) and not changing their behavior *at all*.
It's just adding stages for the new controllers, found in MT6991/6993 (where
8196==6991).
I would not review rest part because you may base on a wrong assumption to
implement.
As I explained, I didn't use any wrong assumptions here.
Cheers,
Angelo
Regards,
CK
+
+ do {
+ mtk_ddp_comp_layer_config(mtk_crtc->ddp_comp[layer_stages[i]],
+ layer_num, plane_state, cmdq_pkt);
+ } while (++i < mtk_crtc->hwlayers[layer_num].layer_stages_nr);
+
+ return;
+}
+
