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 <[email protected]> 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/[email protected]?part=3
