| Message ID | 20250115-bridge-connector-v1-23-9a2fecd886a6@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/bridge: Various quality of life improvements | expand |
On Wed, Jan 15, 2025 at 10:05:30PM +0100, Maxime Ripard wrote: > The current bridge state is accessible from the drm_bridge structure, > but since it's fairly indirect it's not easy to figure out. > > Provide a helper to retrieve it. > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > --- > include/drm/drm_bridge.h | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 8e18130be8bb85fc2463917dde9bf1d281934184..95c5037a6335e4c1be511e6c31308202015c7754 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -955,10 +955,27 @@ static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np) > { > return NULL; > } > #endif > > +/** > + * @drm_bridge_get_current_state() - Get the current bridge state > + * @bridge: bridge object > + * > + * RETURNS: > + * > + * The current bridge state, or NULL if there is none. Are there any kind of limitations on when and how this function can be used? I don't think we can be accessing the state randomly, as the framework can change it at some points. E.g. what if the driver uses this state from audio or cec callbacks, while the DRM framework performs atomic commit and changes / frees the state right concurrently? > + */ > +static inline struct drm_bridge_state * > +drm_bridge_get_current_state(struct drm_bridge *bridge) > +{ > + if (!bridge) > + return NULL; > + > + return drm_priv_to_bridge_state(bridge->base.state); > +} > + > /** > * drm_bridge_get_next_bridge() - Get the next bridge in the chain > * @bridge: bridge object > * > * RETURNS: > > -- > 2.47.1 >
Hi, On Thu, Jan 16, 2025 at 02:43:37AM +0200, Dmitry Baryshkov wrote: > On Wed, Jan 15, 2025 at 10:05:30PM +0100, Maxime Ripard wrote: > > The current bridge state is accessible from the drm_bridge structure, > > but since it's fairly indirect it's not easy to figure out. > > > > Provide a helper to retrieve it. > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > > --- > > include/drm/drm_bridge.h | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > > index 8e18130be8bb85fc2463917dde9bf1d281934184..95c5037a6335e4c1be511e6c31308202015c7754 100644 > > --- a/include/drm/drm_bridge.h > > +++ b/include/drm/drm_bridge.h > > @@ -955,10 +955,27 @@ static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np) > > { > > return NULL; > > } > > #endif > > > > +/** > > + * @drm_bridge_get_current_state() - Get the current bridge state > > + * @bridge: bridge object > > + * > > + * RETURNS: > > + * > > + * The current bridge state, or NULL if there is none. > > Are there any kind of limitations on when and how this function can be > used? I don't think we can be accessing the state randomly, as the > framework can change it at some points. E.g. what if the driver uses > this state from audio or cec callbacks, while the DRM framework performs > atomic commit and changes / frees the state right concurrently? The semantics are equivalent to drm_connector->state, drm_crtc->state, etc, but I'm not sure we ever defined it clearly. Also, it looks like I can never remember what the locking rules are :) It looks like a good occasion to ask Sima and write some more documentation. Thanks! Maxime
On Thu, Jan 16, 2025 at 02:43:37AM +0200, Dmitry Baryshkov wrote: > On Wed, Jan 15, 2025 at 10:05:30PM +0100, Maxime Ripard wrote: > > The current bridge state is accessible from the drm_bridge structure, > > but since it's fairly indirect it's not easy to figure out. > > > > Provide a helper to retrieve it. > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > > --- > > include/drm/drm_bridge.h | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > > index 8e18130be8bb85fc2463917dde9bf1d281934184..95c5037a6335e4c1be511e6c31308202015c7754 100644 > > --- a/include/drm/drm_bridge.h > > +++ b/include/drm/drm_bridge.h > > @@ -955,10 +955,27 @@ static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np) > > { > > return NULL; > > } > > #endif > > > > +/** > > + * @drm_bridge_get_current_state() - Get the current bridge state > > + * @bridge: bridge object > > + * > > + * RETURNS: > > + * > > + * The current bridge state, or NULL if there is none. > > Are there any kind of limitations on when and how this function can be > used? I don't think we can be accessing the state randomly, as the > framework can change it at some points. E.g. what if the driver uses > this state from audio or cec callbacks, while the DRM framework performs > atomic commit and changes / frees the state right concurrently? Yeah you can only look at this when either holding the corresponding modesetlock, or in atomic commit. But in the latter it's a much cleaner design pattern to instead look up the state from the drm_atomic_state, since that pointer is the magic thing which guarantees lifetim/ownership for lockless access in atomic commit code. -Sima > > > + */ > > +static inline struct drm_bridge_state * > > +drm_bridge_get_current_state(struct drm_bridge *bridge) > > +{ > > + if (!bridge) > > + return NULL; > > + > > + return drm_priv_to_bridge_state(bridge->base.state); > > +} > > + > > /** > > * drm_bridge_get_next_bridge() - Get the next bridge in the chain > > * @bridge: bridge object > > * > > * RETURNS: > > > > -- > > 2.47.1 > > > > -- > With best wishes > Dmitry
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 8e18130be8bb85fc2463917dde9bf1d281934184..95c5037a6335e4c1be511e6c31308202015c7754 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -955,10 +955,27 @@ static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np) { return NULL; } #endif +/** + * @drm_bridge_get_current_state() - Get the current bridge state + * @bridge: bridge object + * + * RETURNS: + * + * The current bridge state, or NULL if there is none. + */ +static inline struct drm_bridge_state * +drm_bridge_get_current_state(struct drm_bridge *bridge) +{ + if (!bridge) + return NULL; + + return drm_priv_to_bridge_state(bridge->base.state); +} + /** * drm_bridge_get_next_bridge() - Get the next bridge in the chain * @bridge: bridge object * * RETURNS:
The current bridge state is accessible from the drm_bridge structure, but since it's fairly indirect it's not easy to figure out. Provide a helper to retrieve it. Signed-off-by: Maxime Ripard <mripard@kernel.org> --- include/drm/drm_bridge.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)