On Wed, Jan 15, 2025 at 10:05:32PM +0100, Maxime Ripard wrote: > Now that connectors are no longer necessarily created by the bridges > drivers themselves but might be created by drm_bridge_connector, it's > pretty hard for bridge drivers to retrieve pointers to the connector and > CRTC they are attached to. > > Indeed, the only way to retrieve the CRTC is to follow the drm_bridge > encoder field, and then the drm_encoder crtc field, both of them being > deprecated. > > And for the connector, since we can have multiple connectors attached to > a CRTC, we don't really have a reliable way to get it. > > Let's provide both pointers in the drm_bridge_state structure so we > don't have to follow deprecated, non-atomic, pointers, and be more > consistent with the other KMS entities. > > Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx> > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 5 +++++ > drivers/gpu/drm/drm_bridge.c | 21 +++++++++++++-------- > include/drm/drm_atomic.h | 14 ++++++++++++++ > 3 files changed, 32 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c > index 519228eb109533d2596e899a57b571fa0995824f..66661dca077215b78dffca7bc1712f56d35e3918 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > @@ -777,10 +777,15 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_duplicate_state); > * that don't subclass the bridge state. > */ > void drm_atomic_helper_bridge_destroy_state(struct drm_bridge *bridge, > struct drm_bridge_state *state) > { > + if (state->connector) { > + drm_connector_put(state->connector); > + state->connector = NULL; > + } > + > kfree(state); > } > EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state); > > /** > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index c937980d6591fd98e33e37d799ebf84e7e6c5529..069c105aa59636c64caffbefcf482133b0db97d9 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -829,19 +829,24 @@ 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) > { > + struct drm_bridge_state *bridge_state; > + int ret; > + > + bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state, > + bridge); It felt like an error to me to call this function for a non-atomic bridges, until I fully followed the code path to find that it will return NULL if the bridge isn't registered as a private object. BTW: if my grep-foo isn't deceiving me, we currently have 34 non-atomic bridges out of 90. Should we start forcebly updating them to use atomic interface in attempt to drop the mode_fixup() and other non-atomic callbacks? > + if (WARN_ON(!bridge_state)) > + return -EINVAL; > + > + bridge_state->crtc = crtc_state->crtc; > + > + drm_connector_get(conn_state->connector); > + bridge_state->connector = conn_state->connector; > + > if (bridge->funcs->atomic_check) { > - 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; > } else if (bridge->funcs->mode_fixup) { -- With best wishes Dmitry