Re: [PATCH v2 2/7] drm/tegra: rgb: Parameterize configuration based on bus flags

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/tegra: lockdep splat in tegra_rgb_encoder_enable() due to dropped locks
- [High] drm/tegra: fallback bus flags invert default pixel clock polarity
--

commit 9e423aa598f153ae0ba6a206e3338f678a83bd15
Author: Svyatoslav Ryhel <clamor95@gmail.com>

drm/tegra: rgb: Parameterize configuration based on bus flags

This commit parameterizes the display configuration based on bus flags passed
from the bridge or panel. The list of supported flags now includes pixel
clock polarity, display enable polarity, and data alignment.

> diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
> index b6d09a8d8e5ea..86bff5f7f7964 100644
> --- a/drivers/gpu/drm/tegra/rgb.c
> +++ b/drivers/gpu/drm/tegra/rgb.c

[ ... ]

> @@ -104,14 +105,24 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder)
>  	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
>  	struct tegra_output *output = encoder_to_output(encoder);
>  	struct tegra_rgb *rgb = to_rgb(output);
> -	u32 value;
> +	struct drm_bridge_state *bridge_state;
> +	u32 bus_flags, value;
> +
> +	/* Get but flags from the bridge state. */
> +	bridge_state = drm_bridge_get_current_state(output->bridge);

[Severity: High]
Does calling drm_bridge_get_current_state() here trigger a lockdep warning?

The helper function explicitly requires the modeset lock to be held and
asserts it via drm_modeset_lock_assert_held(). If a userspace application
issues a non-blocking atomic commit, the commit_tail worker executes this
tegra_rgb_encoder_enable() hook asynchronously after the modeset locks have
been dropped, which will trigger a WARN.

> +	if (bridge_state)
> +		bus_flags = bridge_state->input_bus_cfg.flags;
> +	else
> +		bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE |
> +			    DRM_BUS_FLAG_DE_HIGH |
> +			    DRM_BUS_FLAG_DATA_MSB_TO_LSB;

[ ... ]

> @@ -124,11 +135,31 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder)
>  	else
>  		value &= ~LVS_OUTPUT_POLARITY_LOW;
>  
> +	if (bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE)
> +		value &= ~LSC0_OUTPUT_POLARITY_LOW;
> +	else
> +		value |= LSC0_OUTPUT_POLARITY_LOW;

[Severity: High]
Does this fallback configuration silently invert the default pixel clock
polarity for older panels?

The legacy hardware initialization array rgb_enable historically set
LSC0_OUTPUT_POLARITY_LOW to 1 by default. By hardcoding
DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE in the fallback bus_flags for
missing bridge states, the code now clears LSC0_OUTPUT_POLARITY_LOW.
Will this corrupt the display output or cause blank screens on legacy
configurations without explicitly configured bus flags?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004163114.181800-1-clamor95@gmail.com?part=2