Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Modifying persistent driver state in `atomic_check` instead of atomic bridge state. - [High] The bridge enable callback ignores the return value of mtk_dpi_power_on(), proceeding to access hardware registers even if clock enablement fails. -- --- Patch [28]: [PATCH v2 28/37] drm/mediatek: dpi: Switch to atomic bridge callbacks --- Note: The format of this review has been altered to use a free-form summary instead of inline diff quoting due to text recitation restrictions. commit 396f1ff8f70eedabf798b2a8fbc7bac8f10dfaad Author: Maxime Ripard <[email protected]> drm/mediatek: dpi: Switch to atomic bridge callbacks This commit switches the dpi bridge to use atomic bridge callbacks instead of the legacy non-atomic ones, updating the function signatures and adding atomic state handlers via a Coccinelle script. [Severity: High] This is a pre-existing issue, but I noticed something while reviewing the atomic conversions in mtk_dpi_bridge_atomic_check(). The check callback appears to mutate the global dpi structure directly: mtk_dpi_bridge_atomic_check() dpi->output_fmt = out_bus_format; dpi->bit_num = mtk_dpi_bus_fmt_bit_num(out_bus_format); ... Rather than modifying the local bridge_state, the persistent driver state is altered. Could this cause state corruption if a userspace application performs a test-only atomic commit (DRM_MODE_ATOMIC_TEST_ONLY)? It looks like a test-only commit might permanently alter the active configuration used by mtk_dpi_set_display_mode(), potentially breaking subsequent display operations. [Severity: High] This is also a pre-existing issue, but looking at the newly renamed mtk_dpi_bridge_enable() callback, it calls mtk_dpi_power_on(dpi) without checking its return value: mtk_dpi_bridge_enable() ... mtk_dpi_power_on(dpi); mtk_dpi_set_display_mode(dpi, &dpi->mode); mtk_dpi_enable(dpi); If any of the required clocks (like engine_clk, tvd_clk, or pixel_clk) fail to enable, mtk_dpi_power_on() will return an error, but the caller proceeds to access hardware registers anyway. Will accessing this unclocked register space via mtk_dpi_set_display_mode() and mtk_dpi_enable() trigger a bus fault and result in an immediate system panic? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=28
