Re: [PATCH v5 1/4] drm/bridge: Add a drm_bridge_state object
On Mon, Jan 06, 2020 at 11:03:54AM +0100, Boris Brezillon wrote: > On Thu, 19 Dec 2019 11:11:48 +0100 Neil Armstrong wrote: > > > +/** > > + * drm_atomic_helper_duplicate_bridge_state() - Default duplicate state > > helper > > + * @bridge: bridge containing the state to duplicate > > + * > > + * Default implementation of _bridge_funcs.atomic_duplicate(). > > + * > > + * RETURNS: > > + * a valid state object or NULL if the allocation fails. > > + */ > > +struct drm_bridge_state * > > +drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge) > > +{ > > + struct drm_bridge_state *new; > > + > > + if (WARN_ON(!bridge->base.state)) > > + return NULL; > > + > > + new = kzalloc(sizeof(*new), GFP_KERNEL); > > + if (new) > > + __drm_atomic_helper_bridge_duplicate_state(bridge, new); > > + > > + return new; > > +} > > +EXPORT_SYMBOL(drm_atomic_helper_bridge_duplicate_state); > > IIRC, Laurent suggested to make those functions private. I'd also > recommend changing the names to > drm_atomic_*default*_bridge__state() and dropping the kernel doc > header since making them static means they're no longer helper > functions. Please note that static functions may still benefit from documentation. In this specific case the documentation can probably be dropped, but if other functions have useful comments, please keep them. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/4] drm/bridge: Add a drm_bridge_state object
On 06/01/2020 11:03, Boris Brezillon wrote: > Hi Neil, > > On Thu, 19 Dec 2019 11:11:48 +0100 > Neil Armstrong wrote: > >> +/** >> + * drm_atomic_helper_duplicate_bridge_state() - Default duplicate state >> helper >> + * @bridge: bridge containing the state to duplicate >> + * >> + * Default implementation of _bridge_funcs.atomic_duplicate(). >> + * >> + * RETURNS: >> + * a valid state object or NULL if the allocation fails. >> + */ >> +struct drm_bridge_state * >> +drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge) >> +{ >> +struct drm_bridge_state *new; >> + >> +if (WARN_ON(!bridge->base.state)) >> +return NULL; >> + >> +new = kzalloc(sizeof(*new), GFP_KERNEL); >> +if (new) >> +__drm_atomic_helper_bridge_duplicate_state(bridge, new); >> + >> +return new; >> +} >> +EXPORT_SYMBOL(drm_atomic_helper_bridge_duplicate_state); > > IIRC, Laurent suggested to make those functions private. I'd also > recommend changing the names to > drm_atomic_*default*_bridge__state() and dropping the kernel doc > header since making them static means they're no longer helper > functions. > > Regards, > > Boris > Ack, time for a v6 ! Neil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/4] drm/bridge: Add a drm_bridge_state object
Hi Neil, On Thu, 19 Dec 2019 11:11:48 +0100 Neil Armstrong wrote: > +/** > + * drm_atomic_helper_duplicate_bridge_state() - Default duplicate state > helper > + * @bridge: bridge containing the state to duplicate > + * > + * Default implementation of _bridge_funcs.atomic_duplicate(). > + * > + * RETURNS: > + * a valid state object or NULL if the allocation fails. > + */ > +struct drm_bridge_state * > +drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge) > +{ > + struct drm_bridge_state *new; > + > + if (WARN_ON(!bridge->base.state)) > + return NULL; > + > + new = kzalloc(sizeof(*new), GFP_KERNEL); > + if (new) > + __drm_atomic_helper_bridge_duplicate_state(bridge, new); > + > + return new; > +} > +EXPORT_SYMBOL(drm_atomic_helper_bridge_duplicate_state); IIRC, Laurent suggested to make those functions private. I'd also recommend changing the names to drm_atomic_*default*_bridge__state() and dropping the kernel doc header since making them static means they're no longer helper functions. Regards, Boris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/4] drm/bridge: Add a drm_bridge_state object
Hi! Dne četrtek, 19. december 2019 ob 11:11:48 CET je Neil Armstrong napisal(a): > From: Boris Brezillon > > One of the last remaining objects to not have its atomic state. > > This is being motivated by our attempt to support runtime bus-format > negotiation between elements of the bridge chain. > This patch just paves the road for such a feature by adding a new > drm_bridge_state object inheriting from drm_private_obj so we can > re-use some of the existing state initialization/tracking logic. > > Signed-off-by: Boris Brezillon > Reviewed-by: Neil Armstrong > Reviewed-by: Laurent Pinchart > Signed-off-by: Neil Armstrong > --- Reviewed by: Jernej Skrabec Best regards, Jernej > Changes in v5: > * Re-introduced the helpers from v4 > > Changes in v4: > * Fix the doc > * Kill default helpers (inlined) > * Fix drm_atomic_get_bridge_state() to check for an ERR_PTR() > * Add Neil's R-b > > Changes in v3: > * None > > Changes in v2: > * Use drm_for_each_bridge_in_chain() > * Rename helpers to be more consistent with the rest of the DRM API > * Improve/fix the doc > --- > drivers/gpu/drm/drm_atomic.c| 39 +++ > drivers/gpu/drm/drm_atomic_helper.c | 20 > drivers/gpu/drm/drm_bridge.c| 169 +++- > include/drm/drm_atomic.h| 3 + > include/drm/drm_bridge.h| 120 > 5 files changed, 345 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index ab4508f25986..0b7db3d9c610 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -30,6 +30,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -1017,6 +1018,44 @@ static void drm_atomic_connector_print_state(struct > drm_printer *p, connector->funcs->atomic_print_state(p, state); > } > > +/** > + * drm_atomic_add_encoder_bridges - add bridges attached to an encoder > + * @state: atomic state > + * @encoder: DRM encoder > + * > + * This function adds all bridges attached to @encoder. This is needed to > add + * bridge states to @state and make them available when > + * _funcs.atomic_{check,pre_enable,enable,disable_post_disable}() > are + * called > + * > + * Returns: > + * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is > EDEADLK + * then the w/w mutex code has detected a deadlock and the entire > atomic + * sequence must be restarted. All other errors are fatal. > + */ > +int > +drm_atomic_add_encoder_bridges(struct drm_atomic_state *state, > +struct drm_encoder *encoder) > +{ > + struct drm_bridge_state *bridge_state; > + struct drm_bridge *bridge; > + > + if (!encoder) > + return 0; > + > + DRM_DEBUG_ATOMIC("Adding all bridges for [encoder:%d:%s] to %p\n", > + encoder->base.id, encoder->name, state); > + > + drm_for_each_bridge_in_chain(encoder, bridge) { > + bridge_state = drm_atomic_get_bridge_state(state, bridge); > + if (IS_ERR(bridge_state)) > + return PTR_ERR(bridge_state); > + } > + > + return 0; > +} > +EXPORT_SYMBOL(drm_atomic_add_encoder_bridges); > + > /** > * drm_atomic_add_affected_connectors - add connectors for crtc > * @state: atomic state > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index 57a84555a6bd..b3e1aedd9d7a > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -730,6 +730,26 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > return ret; > } > > + /* > + * Iterate over all connectors again, and add all affected bridges to > + * the state. > + */ > + for_each_oldnew_connector_in_state(state, connector, > +old_connector_state, > +new_connector_state, i) { > + struct drm_encoder *encoder; > + > + encoder = old_connector_state->best_encoder; > + ret = drm_atomic_add_encoder_bridges(state, encoder); > + if (ret) > + return ret; > + > + encoder = new_connector_state->best_encoder; > + ret = drm_atomic_add_encoder_bridges(state, encoder); > + if (ret) > + return ret; > + } > + > ret = mode_valid(state); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index c2cf0c90fa26..a3921b45f044 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -25,6 +25,7 @@ > #include > #include > > +#include > #include > #include > > @@ -89,6 +90,38 @@ void drm_bridge_remove(struct drm_bridge *bridge) > } > EXPORT_SYMBOL(drm_bridge_remove); > > +static struct drm_private_state * >
[PATCH v5 1/4] drm/bridge: Add a drm_bridge_state object
From: Boris Brezillon One of the last remaining objects to not have its atomic state. This is being motivated by our attempt to support runtime bus-format negotiation between elements of the bridge chain. This patch just paves the road for such a feature by adding a new drm_bridge_state object inheriting from drm_private_obj so we can re-use some of the existing state initialization/tracking logic. Signed-off-by: Boris Brezillon Reviewed-by: Neil Armstrong Reviewed-by: Laurent Pinchart Signed-off-by: Neil Armstrong --- Changes in v5: * Re-introduced the helpers from v4 Changes in v4: * Fix the doc * Kill default helpers (inlined) * Fix drm_atomic_get_bridge_state() to check for an ERR_PTR() * Add Neil's R-b Changes in v3: * None Changes in v2: * Use drm_for_each_bridge_in_chain() * Rename helpers to be more consistent with the rest of the DRM API * Improve/fix the doc --- drivers/gpu/drm/drm_atomic.c| 39 +++ drivers/gpu/drm/drm_atomic_helper.c | 20 drivers/gpu/drm/drm_bridge.c| 169 +++- include/drm/drm_atomic.h| 3 + include/drm/drm_bridge.h| 120 5 files changed, 345 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index ab4508f25986..0b7db3d9c610 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -1017,6 +1018,44 @@ static void drm_atomic_connector_print_state(struct drm_printer *p, connector->funcs->atomic_print_state(p, state); } +/** + * drm_atomic_add_encoder_bridges - add bridges attached to an encoder + * @state: atomic state + * @encoder: DRM encoder + * + * This function adds all bridges attached to @encoder. This is needed to add + * bridge states to @state and make them available when + * _funcs.atomic_{check,pre_enable,enable,disable_post_disable}() are + * called + * + * Returns: + * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK + * then the w/w mutex code has detected a deadlock and the entire atomic + * sequence must be restarted. All other errors are fatal. + */ +int +drm_atomic_add_encoder_bridges(struct drm_atomic_state *state, + struct drm_encoder *encoder) +{ + struct drm_bridge_state *bridge_state; + struct drm_bridge *bridge; + + if (!encoder) + return 0; + + DRM_DEBUG_ATOMIC("Adding all bridges for [encoder:%d:%s] to %p\n", +encoder->base.id, encoder->name, state); + + drm_for_each_bridge_in_chain(encoder, bridge) { + bridge_state = drm_atomic_get_bridge_state(state, bridge); + if (IS_ERR(bridge_state)) + return PTR_ERR(bridge_state); + } + + return 0; +} +EXPORT_SYMBOL(drm_atomic_add_encoder_bridges); + /** * drm_atomic_add_affected_connectors - add connectors for crtc * @state: atomic state diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 57a84555a6bd..b3e1aedd9d7a 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -730,6 +730,26 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, return ret; } + /* +* Iterate over all connectors again, and add all affected bridges to +* the state. +*/ + for_each_oldnew_connector_in_state(state, connector, + old_connector_state, + new_connector_state, i) { + struct drm_encoder *encoder; + + encoder = old_connector_state->best_encoder; + ret = drm_atomic_add_encoder_bridges(state, encoder); + if (ret) + return ret; + + encoder = new_connector_state->best_encoder; + ret = drm_atomic_add_encoder_bridges(state, encoder); + if (ret) + return ret; + } + ret = mode_valid(state); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c2cf0c90fa26..a3921b45f044 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -89,6 +90,38 @@ void drm_bridge_remove(struct drm_bridge *bridge) } EXPORT_SYMBOL(drm_bridge_remove); +static struct drm_private_state * +drm_bridge_atomic_duplicate_priv_state(struct drm_private_obj *obj) +{ + struct drm_bridge *bridge = drm_priv_to_bridge(obj); + struct drm_bridge_state *state; + + if (bridge->funcs->atomic_duplicate_state) + state = bridge->funcs->atomic_duplicate_state(bridge); + else + state =