| Message ID | 20250213-bridge-connector-v3-30-e71598f49c8f@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/bridge: Various quality of life improvements | expand |
On Thu, Feb 13, 2025 at 03:43:49PM +0100, Maxime Ripard wrote: > The drm_atomic_bridge_check() runs the atomic_check callback if needed, > or falls back to mode_fixup if it's not there. > > Going forward, we'll need to setup the bridge atomic state even though > drivers might not be implementing atomic_check at all. > > We can thus switch to using drm_bridge_is_atomic() to take the atomic > path, and do additional things there in the future, or go the mode_fixup > route. This will break bridges like tc358768, rcar_lvds and mtk_hdmi: they implement atomic_reset() and mode_fixup(). > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > --- > drivers/gpu/drm/drm_bridge.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index d2525d0b1324cc3a63e32f5bf6ca6c4f9034eb4e..b6d24092674c8fa33d9b6ebab9ece0f91fb8f8ea 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -803,23 +803,25 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_enable); > > static int drm_atomic_bridge_check(struct drm_bridge *bridge, > struct drm_crtc_state *crtc_state, > struct drm_connector_state *conn_state) > { > - if (bridge->funcs->atomic_check) { > + if (drm_bridge_is_atomic(bridge)) { > struct drm_bridge_state *bridge_state; > int ret; > > bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state, > bridge); > if (WARN_ON(!bridge_state)) > return -EINVAL; > > - ret = bridge->funcs->atomic_check(bridge, bridge_state, > - crtc_state, conn_state); > - if (ret) > - return ret; > + if (bridge->funcs->atomic_check) { > + ret = bridge->funcs->atomic_check(bridge, bridge_state, > + crtc_state, conn_state); > + if (ret) > + return ret; > + } > } else if (bridge->funcs->mode_fixup) { > if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode, > &crtc_state->adjusted_mode)) > return -EINVAL; > } > > -- > 2.48.0 >
On Thu, Feb 13, 2025 at 06:29:46PM +0200, Dmitry Baryshkov wrote: > On Thu, Feb 13, 2025 at 03:43:49PM +0100, Maxime Ripard wrote: > > The drm_atomic_bridge_check() runs the atomic_check callback if needed, > > or falls back to mode_fixup if it's not there. > > > > Going forward, we'll need to setup the bridge atomic state even though > > drivers might not be implementing atomic_check at all. > > > > We can thus switch to using drm_bridge_is_atomic() to take the atomic > > path, and do additional things there in the future, or go the mode_fixup > > route. > > This will break bridges like tc358768, rcar_lvds and mtk_hdmi: they > implement atomic_reset() and mode_fixup(). What is your suggestion then? I kind of did what you were suggesting on patch 1, but it wasn't working. Then you want me to roll back to that, or something else? Maxime
On Fri, Feb 14, 2025 at 01:59:01PM +0100, Maxime Ripard wrote: > On Thu, Feb 13, 2025 at 06:29:46PM +0200, Dmitry Baryshkov wrote: > > On Thu, Feb 13, 2025 at 03:43:49PM +0100, Maxime Ripard wrote: > > > The drm_atomic_bridge_check() runs the atomic_check callback if needed, > > > or falls back to mode_fixup if it's not there. > > > > > > Going forward, we'll need to setup the bridge atomic state even though > > > drivers might not be implementing atomic_check at all. > > > > > > We can thus switch to using drm_bridge_is_atomic() to take the atomic > > > path, and do additional things there in the future, or go the mode_fixup > > > route. > > > > This will break bridges like tc358768, rcar_lvds and mtk_hdmi: they > > implement atomic_reset() and mode_fixup(). > > What is your suggestion then? I kind of did what you were suggesting on > patch 1, but it wasn't working. Then you want me to roll back to that, > or something else? Well, we just need to call mode_fixup. So something like: if (drm_bridge_is_atomic(bridge)) { bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state, bridge); if (WARN_ON(!bridge_state)) return -EINVAL; } if (bridge->funcs->atomic_check) { /* drm_bridge_add() returns void, so there is no way to * reject non-atomic bridges with atomic_check() * callback. */ if (!bridge_state) return -EINVAL; ret = bridge->funcs->atomic_check(bridge, bridge_state, crtc_state, conn_state); if (ret) return ret; } else if (bridge->funcs->mode_fixup) { if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode, &crtc_state->adjusted_mode)) return -EINVAL; }
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index d2525d0b1324cc3a63e32f5bf6ca6c4f9034eb4e..b6d24092674c8fa33d9b6ebab9ece0f91fb8f8ea 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -803,23 +803,25 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_enable); static int drm_atomic_bridge_check(struct drm_bridge *bridge, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { - if (bridge->funcs->atomic_check) { + if (drm_bridge_is_atomic(bridge)) { struct drm_bridge_state *bridge_state; int ret; bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state, bridge); if (WARN_ON(!bridge_state)) return -EINVAL; - ret = bridge->funcs->atomic_check(bridge, bridge_state, - crtc_state, conn_state); - if (ret) - return ret; + if (bridge->funcs->atomic_check) { + ret = bridge->funcs->atomic_check(bridge, bridge_state, + crtc_state, conn_state); + if (ret) + return ret; + } } else if (bridge->funcs->mode_fixup) { if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode, &crtc_state->adjusted_mode)) return -EINVAL; }
The drm_atomic_bridge_check() runs the atomic_check callback if needed, or falls back to mode_fixup if it's not there. Going forward, we'll need to setup the bridge atomic state even though drivers might not be implementing atomic_check at all. We can thus switch to using drm_bridge_is_atomic() to take the atomic path, and do additional things there in the future, or go the mode_fixup route. Signed-off-by: Maxime Ripard <mripard@kernel.org> --- drivers/gpu/drm/drm_bridge.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)