Re: [PATCH v2 19/78] drm/bridge: imx8qxp-pxl2dpi: Switch to atomic_create_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Global device state corruption in atomic_check.
--

commit e8d715f17c3f8417137a6f4c52d8a5d876a47307
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: imx8qxp-pxl2dpi: Switch to atomic_create_state

This commit replaces the deprecated atomic_reset callback and its helper
with the atomic_create_state callback and its counterpart.

> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> index 87305d3e0c398..2fc9831f7e611 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> @@ -210,7 +210,7 @@ imx8qxp_pxl2dpi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>  static const struct drm_bridge_funcs imx8qxp_pxl2dpi_bridge_funcs = {
>  	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
>  	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
> -	.atomic_reset		= drm_atomic_helper_bridge_reset,
> +	.atomic_create_state		= drm_atomic_helper_bridge_create_state,
>  	.attach			= imx8qxp_pxl2dpi_bridge_attach,
>  	.destroy		= imx8qxp_pxl2dpi_bridge_destroy,
>  	.atomic_check		= imx8qxp_pxl2dpi_bridge_atomic_check,

[Severity: High]
This isn't a bug introduced by this patch, but does
imx8qxp_pxl2dpi_bridge_atomic_check() mutate global device state during the
atomic check phase?

In the DRM atomic framework, atomic_check can be executed for test-only
commits (DRM_MODE_ATOMIC_TEST_ONLY), which never proceed to the hardware
commit phase.

Looking at imx8qxp_pxl2dpi_bridge_atomic_check() in
drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c:

    p2d->in_bus_format = bridge_state->input_bus_cfg.format;
    p2d->out_bus_format = bridge_state->output_bus_cfg.format;

For non-blocking commits, the commit tail executes in a background worker
without holding modeset locks. It then calls mode_set where this state is read:

drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c:imx8qxp_pxl2dpi_bridge_mode_set() {
    ...
    switch (p2d->out_bus_format) {
    ...
}

Can a concurrent test-only commit overwrite this global state while an
ongoing non-blocking commit is reading it, leading to the display hardware
being misconfigured with test-only bus formats?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-drm-no-more-bridge-reset-v2-0-0a91018bf886@kernel.org?part=19