Re: [PATCH 3/4] soc: mediatek: mt8167-mmsys: add routes for all display paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mmsys: incorrect register for OVL0 to WDMA0 routing
- [High] mmsys: missing SEL_IN routes for DITHER to UFOE and WDMA0
- [High] mmsys: incorrect tuple for RDMA1 to DSI0 SEL_IN routing
--

commit 2f4bc4d4c245498df33e07567c376707ba880cbe
Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>
Subject: soc: mediatek: mt8167-mmsys: add routes for all display paths

The commit adds support for additional routes in the mt8167 display output
pipeline, aligning with the SoC data sheet and Android sources.

> diff --git a/drivers/soc/mediatek/mt8167-mmsys.h b/drivers/soc/mediatek/mt8167-mmsys.h
> index 001379373507a..aab1774152c57 100644
> --- a/drivers/soc/mediatek/mt8167-mmsys.h
> +++ b/drivers/soc/mediatek/mt8167-mmsys.h

[ ... ]

>  static const struct mtk_mmsys_routes mt8167_mmsys_routing_table[] = {
> +	/* OVL0<->COLOR0 */
>  	MMSYS_ROUTE(OVL, 0, COLOR, 0,
> -		    MT8167_DISP_REG_CONFIG_DISP_OVL0_MOUT_EN, OVL0_MOUT_EN_COLOR0,
> -		    OVL0_MOUT_EN_COLOR0),
> +		    MT8167_DISP_REG_CONFIG_DISP_OVL0_MOUT_EN, MT8167_OVL0_MOUT_EN_MASK,
> +		    MT8167_OVL0_MOUT_EN_COLOR0),
> +	MMSYS_ROUTE(OVL, 0, COLOR, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_COLOR0_SEL_IN, MT8167_COLOR0_SEL_IN_MASK,
> +		    MT8167_COLOR0_SEL_IN_OVL0),
> +
> +	/* OVL0<->WDMA0 */
> +	MMSYS_ROUTE(OVL, 0, WDMA, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_OVL0_MOUT_EN, MT8167_OVL0_MOUT_EN_MASK,
> +		    MT8167_OVL0_MOUT_EN_WDMA0),
> +	MMSYS_ROUTE(OVL, 0, WDMA, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_COLOR0_SEL_IN, MT8167_COLOR0_SEL_IN_MASK,
> +		    MT8167_WDMA0_SEL_IN_OVL0),

[Severity: High]
Does this route mistakenly use MT8167_DISP_REG_CONFIG_DISP_COLOR0_SEL_IN
instead of MT8167_DISP_REG_CONFIG_DISP_WDMA0_SEL_IN?

By targeting the COLOR0 input selection register, this configuration might
corrupt COLOR0's input selection and fail to properly configure WDMA0's input
selection when establishing a route between OVL0 and WDMA0.

> +
> +	/* DITHER0 outputs */
>  	MMSYS_ROUTE(DITHER, 0, RDMA, 0,
>  		    MT8167_DISP_REG_CONFIG_DISP_DITHER_MOUT_EN, MT8167_DITHER_MOUT_EN_MASK,
>  		    MT8167_DITHER_MOUT_EN_RDMA0),
> -	MMSYS_ROUTE(OVL, 0, COLOR, 0,
> -		    MT8167_DISP_REG_CONFIG_DISP_COLOR0_SEL_IN, COLOR0_SEL_IN_OVL0,
> -		    COLOR0_SEL_IN_OVL0),
> -	MMSYS_ROUTE(RDMA, 0, DSI, 0,
> +	MMSYS_ROUTE(DITHER, 0, UFOE, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_DITHER_MOUT_EN, MT8167_DITHER_MOUT_EN_MASK,
> +		    MT8167_DITHER_MOUT_EN_UFOE),
> +	MMSYS_ROUTE(DITHER, 0, WDMA, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_DITHER_MOUT_EN, MT8167_DITHER_MOUT_EN_MASK,
> +		    MT8167_DITHER_MOUT_EN_WDMA0),

[Severity: High]
Are there missing SEL_IN routes for DITHER to UFOE and WDMA0 here?

The patch defines MT8167_UFOE_SEL_IN_DITHER and MT8167_WDMA0_SEL_IN_DITHER
constants and sets up the source output (MOUT_EN) routes above, but appears to
omit the MMSYS_ROUTE entries to configure the destination input multiplexers.
Without these, UFOE and WDMA0 might not switch their inputs to accept DITHER0.

[ ... ]

> +	/* RDMA1<->DSI0 */
> +	MMSYS_ROUTE(RDMA, 1, DSI, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_RDMA1_SOUT_SEL_IN, MT8167_RDMA1_SOUT_MASK,
> +		    MT8167_RDMA1_SOUT_DSI0),
> +	MMSYS_ROUTE(RDMA, 1, DPI, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_DSI0_SEL_IN, MT8167_DSI0_SEL_IN_MASK,
> +		    MT8167_DPI0_SEL_IN_RDMA1),

[Severity: High]
Should this tuple be (RDMA, 1, DSI, 0) instead of (RDMA, 1, DPI, 0)?

Because the destination is specified as DPI instead of DSI, this route
appears to be applied at the wrong time (when routing to DPI0) and missed
when routing to DSI0. This could leave the RDMA1 to DSI0 route unconfigured
and mistakenly corrupt the DSI0 input selection register when routing to DPI0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918175736.355305-1-l.scorcia@gmail.com?part=3