Re: [PATCH v2 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 1 potential issue(s) to consider:
- [High] mmsys: concurrent MOUT outputs broken by full bitmasks
--

commit 8ae801c4ff167fb410aace262b9f0eef4667f2c9
Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>

soc: mediatek: mt8167-mmsys: add routes for all display paths

This commit adds support for additional routes according to the SoC data
sheet and Android sources. It redefines existing routing table entries to
use full masks and adds new concurrent display output configurations.

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

[ ... ]

> @@ -34,16 +78,162 @@
>  
>  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),

[Severity: High]
Does changing this mask to the full MT8167_OVL0_MOUT_EN_MASK inadvertently
introduce a regression for concurrent multi-output pipelines?

Looking at mtk_mmsys_ddp_disconnect(), it performs a read-modify-write
operation on the register using (tmp & ~mask) | (val & mask). When
disconnecting a route, val is 0, so the register is masked with ~mask.

By using the full bitfield mask instead of the individual bit mask, does
disconnecting one route clear the enable bits for all other active
concurrent paths originating from the same block?

> +	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),

[Severity: High]
For example, if the OVL0-to-WDMA0 route is active alongside OVL0-to-COLOR0,
does disconnecting WDMA0 here clear the active OVL0-to-COLOR0 routing
bits as well because of the shared MT8167_OVL0_MOUT_EN_MASK mask?

[ ... ]

> +	/* DITHER<->UFOE */
> +	MMSYS_ROUTE(DITHER, 0, UFOE, 0,
> +		    MT8167_DISP_REG_CONFIG_DISP_DITHER_MOUT_EN, MT8167_DITHER_MOUT_EN_MASK,
> +		    MT8167_DITHER_MOUT_EN_UFOE),

[Severity: High]
Similarly, does using the full MT8167_DITHER_MOUT_EN_MASK here cause the
disconnect of the DITHER-to-UFOE route to accidentally break concurrent
routing to RDMA0 and WDMA0?

[ ... ]

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