| Message ID | 20210722062246.2512666-4-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: Drop deprecated functions | expand |
Hi, On Thu, Jul 22, 2021 at 08:22:42AM +0200, Sam Ravnborg wrote: > drm_bridge_new_crtc_state() will be used by bridge drivers to provide > easy access to the mode from the drm_bridge_funcs operations. > > The helper will be useful in the atomic operations of > struct drm_bridge_funcs. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Andrzej Hajda <a.hajda@samsung.com> > Cc: Neil Armstrong <narmstrong@baylibre.com> > Cc: Robert Foss <robert.foss@linaro.org> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/drm_atomic.c | 34 ++++++++++++++++++++++++++++++++++ > include/drm/drm_atomic.h | 3 +++ > 2 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index a8bbb021684b..93d513078e9a 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1133,6 +1133,40 @@ drm_atomic_get_new_bridge_state(struct drm_atomic_state *state, > } > EXPORT_SYMBOL(drm_atomic_get_new_bridge_state); > > +/** > + * drm_bridge_new_crtc_state - get crtc_state for the bridge > + * @bridge: bridge object > + * @old_bridge_state: state of the bridge > + * > + * This function returns the &struct drm_crtc_state for the given bridge/state, > + * or NULL if no crtc_state could be looked up. In case no crtc_state then this is > + * logged using WARN as the crtc_state is always expected to be present. > + * This function is often used in the &struct drm_bridge_funcs operations. > + */ > +const struct drm_crtc_state * > +drm_bridge_new_crtc_state(struct drm_bridge *bridge, Shouldn't we call this drm_atomic_bridge_get_new_crtc_state for consistency? > + struct drm_bridge_state *old_bridge_state) It seems odd to me to name it old_bridge_state, I guess it would make more sense to pass in the new bridge state? > +{ > + struct drm_atomic_state *state = old_bridge_state->base.state; > + const struct drm_connector_state *conn_state; > + const struct drm_crtc_state *crtc_state; > + struct drm_connector *connector; > + > + connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder); > + if (WARN_ON(!connector)) > + return NULL; > + > + conn_state = drm_atomic_get_new_connector_state(state, connector); > + if (WARN_ON(!conn_state || !conn_state->crtc)) > + return NULL; > + > + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > + if (WARN_ON(!crtc_state)) > + return NULL; > + > + return crtc_state; You don't even seem to use the bridge state itself, so maybe we just need to pass the drm_atomic_state? And thus we end up with something like drm_atomic_get_new_connector_for_encoder, so maybe we should just call it drm_atomic_get_new_crtc_for_bridge? Also, can we end up with a commit that affects the bridge state but not the crtc state (like a connector property change)? In such a case drm_atomic_get_new_crtc_state would return NULL. I'm not sure if it's a big deal or not, but we should make it clear in the documentation and remove the WARN_ON. Maxime
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index a8bbb021684b..93d513078e9a 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1133,6 +1133,40 @@ drm_atomic_get_new_bridge_state(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_atomic_get_new_bridge_state); +/** + * drm_bridge_new_crtc_state - get crtc_state for the bridge + * @bridge: bridge object + * @old_bridge_state: state of the bridge + * + * This function returns the &struct drm_crtc_state for the given bridge/state, + * or NULL if no crtc_state could be looked up. In case no crtc_state then this is + * logged using WARN as the crtc_state is always expected to be present. + * This function is often used in the &struct drm_bridge_funcs operations. + */ +const struct drm_crtc_state * +drm_bridge_new_crtc_state(struct drm_bridge *bridge, + struct drm_bridge_state *old_bridge_state) +{ + struct drm_atomic_state *state = old_bridge_state->base.state; + const struct drm_connector_state *conn_state; + const struct drm_crtc_state *crtc_state; + struct drm_connector *connector; + + connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder); + if (WARN_ON(!connector)) + return NULL; + + conn_state = drm_atomic_get_new_connector_state(state, connector); + if (WARN_ON(!conn_state || !conn_state->crtc)) + return NULL; + + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); + if (WARN_ON(!crtc_state)) + return NULL; + + return crtc_state; +} + /** * drm_atomic_add_encoder_bridges - add bridges attached to an encoder * @state: atomic state diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 1701c2128a5c..a3001eef98bf 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -1119,5 +1119,8 @@ drm_atomic_get_old_bridge_state(struct drm_atomic_state *state, struct drm_bridge_state * drm_atomic_get_new_bridge_state(struct drm_atomic_state *state, struct drm_bridge *bridge); +const struct drm_crtc_state * +drm_bridge_new_crtc_state(struct drm_bridge *bridge, + struct drm_bridge_state *old_bridge_state); #endif /* DRM_ATOMIC_H_ */
drm_bridge_new_crtc_state() will be used by bridge drivers to provide easy access to the mode from the drm_bridge_funcs operations. The helper will be useful in the atomic operations of struct drm_bridge_funcs. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Robert Foss <robert.foss@linaro.org> Cc: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/drm_atomic.c | 34 ++++++++++++++++++++++++++++++++++ include/drm/drm_atomic.h | 3 +++ 2 files changed, 37 insertions(+)