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