| Message ID | 20211020181901.2114645-4-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/7] drm/bridge: ps8640: Use atomic variants of drm_bridge_funcs | expand |
Hi, On Wed, Oct 20, 2021 at 08:18:57PM +0200, Sam Ravnborg wrote: > drm_atomic_get_new_crtc_for_bridge() 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 conversion to the atomic > operations of struct drm_bridge_funcs. > > v2: > - Renamed to drm_atomic_get_new_crtc_for_bridge (Maxime) > - Use atomic_state, not bridge_State (Maxime) > - Drop WARN on crtc_state as it is a valid case (Maxime) > - Added small code snip to help readers > - Updated description, fixed kernel-doc and exported the symbol > > 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 | 42 ++++++++++++++++++++++++++++++++++++ > include/drm/drm_atomic.h | 3 +++ > 2 files changed, 45 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index ff1416cd609a..8b107194b157 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1134,6 +1134,48 @@ drm_atomic_get_new_bridge_state(struct drm_atomic_state *state, > } > EXPORT_SYMBOL(drm_atomic_get_new_bridge_state); > > +/** > + * drm_atomic_get_new_crtc_for_bridge - get new crtc_state for the bridge > + * @state: state of the bridge > + * @bridge: bridge object I appreciate that the function name is fairly long already, but given its name I'd expect it to return a drm_crtc, not drm_crtc_state. All the other state related functions are named using the pattern drm_atomic_get_(old|new)_$object_state. Moreover, we already have a drm_atomic_get_new_connector_for_encoder function that does return a drm_connector, so I think we should make it consistent there and call it drm_atomic_get_new_crtc_state_for_bridge Maxime
Hi Maxime, On Fri, Oct 22, 2021 at 01:03:55PM +0200, Maxime Ripard wrote: > Hi, > > On Wed, Oct 20, 2021 at 08:18:57PM +0200, Sam Ravnborg wrote: > > drm_atomic_get_new_crtc_for_bridge() 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 conversion to the atomic > > operations of struct drm_bridge_funcs. > > > > v2: > > - Renamed to drm_atomic_get_new_crtc_for_bridge (Maxime) > > - Use atomic_state, not bridge_State (Maxime) > > - Drop WARN on crtc_state as it is a valid case (Maxime) > > - Added small code snip to help readers > > - Updated description, fixed kernel-doc and exported the symbol > > > > 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 | 42 ++++++++++++++++++++++++++++++++++++ > > include/drm/drm_atomic.h | 3 +++ > > 2 files changed, 45 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index ff1416cd609a..8b107194b157 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -1134,6 +1134,48 @@ drm_atomic_get_new_bridge_state(struct drm_atomic_state *state, > > } > > EXPORT_SYMBOL(drm_atomic_get_new_bridge_state); > > > > +/** > > + * drm_atomic_get_new_crtc_for_bridge - get new crtc_state for the bridge > > + * @state: state of the bridge > > + * @bridge: bridge object > > I appreciate that the function name is fairly long already, but given > its name I'd expect it to return a drm_crtc, not drm_crtc_state. > > All the other state related functions are named using the pattern > drm_atomic_get_(old|new)_$object_state. > > Moreover, we already have a drm_atomic_get_new_connector_for_encoder > function that does return a drm_connector, so I think we should make it > consistent there and call it drm_atomic_get_new_crtc_state_for_bridge That's a better name - thanks! I will fix it in v3. Sam
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index ff1416cd609a..8b107194b157 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1134,6 +1134,48 @@ drm_atomic_get_new_bridge_state(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_atomic_get_new_bridge_state); +/** + * drm_atomic_get_new_crtc_for_bridge - get new crtc_state for the bridge + * @state: state of the bridge + * @bridge: bridge object + * + * This function is often used in the &struct drm_bridge_funcs operations + * to provide easy access to the mode like this: + * + * .. code-block:: c + * + * crtc_state = drm_atomic_get_new_crtc_for_bridge(old_bridge_state->base.state, + * bridge); + * if (crtc_state) { + * mode = &crtc_state->mode; + * ... + * + * If no connector can be looked up or if no connector state is available + * then this will be logged using WARN(). + * + * Returns: + * The &struct drm_crtc_state for the given bridge/state, or NULL + * if no crtc_state could be looked up. + */ +const struct drm_crtc_state * +drm_atomic_get_new_crtc_for_bridge(struct drm_atomic_state *state, + struct drm_bridge *bridge) +{ + const struct drm_connector_state *conn_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; + + return drm_atomic_get_new_crtc_state(state, conn_state->crtc); +} +EXPORT_SYMBOL(drm_atomic_get_new_crtc_for_bridge); + /** * 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..f861d73296cc 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_atomic_get_new_crtc_for_bridge(struct drm_atomic_state *state, + struct drm_bridge *bridge); #endif /* DRM_ATOMIC_H_ */
drm_atomic_get_new_crtc_for_bridge() 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 conversion to the atomic operations of struct drm_bridge_funcs. v2: - Renamed to drm_atomic_get_new_crtc_for_bridge (Maxime) - Use atomic_state, not bridge_State (Maxime) - Drop WARN on crtc_state as it is a valid case (Maxime) - Added small code snip to help readers - Updated description, fixed kernel-doc and exported the symbol 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 | 42 ++++++++++++++++++++++++++++++++++++ include/drm/drm_atomic.h | 3 +++ 2 files changed, 45 insertions(+)