Hi,
On Thu, Jul 22, 2021 at 08:22:43AM +0200, Sam Ravnborg wrote:
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 sam@ravnborg.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Neil Armstrong narmstrong@baylibre.com Cc: Robert Foss robert.foss@linaro.org 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_he...
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