Re: [PATCH v2 28/37] drm/mediatek: dpi: Switch to atomic bridge callbacks

[sashiko-bot@kernel.org]

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 <mripard@kernel.org>
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/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=28