Re: [PATCH v1 4/7] drm/bridge: lontium-lt9611: Use atomic variants of drm_bridge_funcs
From: Maxime Ripard <hidden>
Date: 2021-07-22 07:44:11
Also in:
dri-devel, linux-mediatek
Hi, On Thu, Jul 22, 2021 at 08:22:43AM +0200, Sam Ravnborg wrote:
quoted hunk ↗ jump to hunk
The atomic variants of enable/disable in drm_bridge_funcs are the preferred operations - introduce these. Use of mode_set is deprecated - merge the functionality with atomic_enable() Signed-off-by: Sam Ravnborg <redacted> Cc: Andrzej Hajda <redacted> Cc: Neil Armstrong <redacted> Cc: Robert Foss <redacted> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@gmail.com> --- drivers/gpu/drm/bridge/lontium-lt9611.c | 69 ++++++++++--------------- 1 file changed, 27 insertions(+), 42 deletions(-)diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c index 29b1ce2140ab..dfa7baefe2ab 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c@@ -700,9 +700,17 @@ lt9611_connector_mode_valid(struct drm_connector *connector, } /* bridge funcs */ -static void lt9611_bridge_enable(struct drm_bridge *bridge) +static void lt9611_bridge_atomic_enable(struct drm_bridge *bridge, + struct drm_bridge_state *old_bridge_state) { struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + const struct drm_display_mode *mode; + const struct drm_crtc_state *crtc_state; + struct hdmi_avi_infoframe avi_frame; + int ret; + + crtc_state = drm_bridge_new_crtc_state(bridge, old_bridge_state); + mode = &crtc_state->mode;
So, yeah, it looks like you can't make the assumption that crtc_state is going to be valid here. I'm not entirely clear on how bridge states are allocated, but it looks to me that they are through drm_atomic_add_encoder_bridges, which is called for all the affected connectors in a commit here: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_atomic_helper.c#L744 Then, the atomic_enable call is made through drm_atomic_bridge_chain_enable(), which is called in drm_atomic_helper_commit_modeset_enables only if the CRTC is active and needs a modeset. I guess this means that we won't have a null pointer for crtc_state there, but wouldn't that cause some issues? I can imagine a property like the bpc count or output format where it wouldn't imply a modeset but would definitely affect the bridges in the chain? Maxime _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel