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 <s...@ravnborg.org>
> > Suggested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> > Cc: Maxime Ripard <mrip...@kernel.org>
> > Cc: Thomas Zimmermann <tzimmerm...@suse.de>
> > Cc: Andrzej Hajda <a.ha...@samsung.com>
> > Cc: Neil Armstrong <narmstr...@baylibre.com>
> > Cc: Robert Foss <robert.f...@linaro.org>
> > Cc: Daniel Vetter <dan...@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
