Re: [PATCH v3 10/21] drm/bridge: Clarify the atomic enable/disable hooks semantics
Hi Boris, Thank you for the patch. On Wed, Oct 23, 2019 at 05:45:01PM +0200, Boris Brezillon wrote: > The [pre_]enable/[post_]disable hooks are passed the old atomic state. > Update the doc and rename the arguments to make it clear. > > Signed-off-by: Boris Brezillon > --- > Changes in v3: > * New patch > --- > drivers/gpu/drm/drm_bridge.c | 24 > include/drm/drm_bridge.h | 16 > 2 files changed, 24 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index c53966767782..ca74bfe028c9 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -447,7 +447,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable); > /** > * drm_atomic_bridge_chain_disable - disables all bridges in the encoder > chain > * @bridge: bridge control structure > - * @state: atomic state being committed > + * @old_state: old atomic state > * > * Calls _bridge_funcs.atomic_disable (falls back on > * _bridge_funcs.disable) op for all the bridges in the encoder chain, > @@ -457,7 +457,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable); > * Note: the bridge passed should be the one closest to the encoder > */ > void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge, > - struct drm_atomic_state *state) > + struct drm_atomic_state *old_state) > { > struct drm_encoder *encoder; > struct drm_bridge *iter; > @@ -469,7 +469,7 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge > *bridge, > list_for_each_entry_reverse(iter, >bridge_chain, > chain_node) { > if (iter->funcs->atomic_disable) > - iter->funcs->atomic_disable(iter, state); > + iter->funcs->atomic_disable(iter, old_state); > else if (iter->funcs->disable) > iter->funcs->disable(iter); > > @@ -483,7 +483,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable); > * drm_atomic_bridge_chain_post_disable - cleans up after disabling all > bridges > * in the encoder chain > * @bridge: bridge control structure > - * @state: atomic state being committed > + * @old_state: old atomic state > * > * Calls _bridge_funcs.atomic_post_disable (falls back on > * _bridge_funcs.post_disable) op for all the bridges in the encoder > chain, > @@ -493,7 +493,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable); > * Note: the bridge passed should be the one closest to the encoder > */ > void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, > - struct drm_atomic_state *state) > + struct drm_atomic_state *old_state) > { > struct drm_encoder *encoder; > > @@ -504,7 +504,7 @@ void drm_atomic_bridge_chain_post_disable(struct > drm_bridge *bridge, > list_for_each_entry_from(bridge, >bridge_chain, >chain_node) { > if (bridge->funcs->atomic_post_disable) > - bridge->funcs->atomic_post_disable(bridge, state); > + bridge->funcs->atomic_post_disable(bridge, old_state); > else if (bridge->funcs->post_disable) > bridge->funcs->post_disable(bridge); > } > @@ -515,7 +515,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); > * drm_atomic_bridge_chain_pre_enable - prepares for enabling all bridges in > * the encoder chain > * @bridge: bridge control structure > - * @state: atomic state being committed > + * @old_state: old atomic state > * > * Calls _bridge_funcs.atomic_pre_enable (falls back on > * _bridge_funcs.pre_enable) op for all the bridges in the encoder chain, > @@ -525,7 +525,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); > * Note: the bridge passed should be the one closest to the encoder > */ > void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, > - struct drm_atomic_state *state) > + struct drm_atomic_state *old_state) > { > struct drm_encoder *encoder; > struct drm_bridge *iter; > @@ -537,7 +537,7 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge > *bridge, > list_for_each_entry_reverse(iter, >encoder->bridge_chain, > chain_node) { > if (iter->funcs->atomic_pre_enable) > - iter->funcs->atomic_pre_enable(iter, state); > + iter->funcs->atomic_pre_enable(iter, old_state); > else if (iter->funcs->pre_enable) > iter->funcs->pre_enable(iter); > > @@ -550,7 +550,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable); > /** > * drm_atomic_bridge_chain_enable - enables all
Re: [PATCH v3 10/21] drm/bridge: Clarify the atomic enable/disable hooks semantics
On 23/10/2019 17:45, Boris Brezillon wrote: > The [pre_]enable/[post_]disable hooks are passed the old atomic state. > Update the doc and rename the arguments to make it clear. > > Signed-off-by: Boris Brezillon > --- > Changes in v3: > * New patch > --- > drivers/gpu/drm/drm_bridge.c | 24 > include/drm/drm_bridge.h | 16 > 2 files changed, 24 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index c53966767782..ca74bfe028c9 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -447,7 +447,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable); > /** > * drm_atomic_bridge_chain_disable - disables all bridges in the encoder > chain > * @bridge: bridge control structure > - * @state: atomic state being committed > + * @old_state: old atomic state > * > * Calls _bridge_funcs.atomic_disable (falls back on > * _bridge_funcs.disable) op for all the bridges in the encoder chain, > @@ -457,7 +457,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable); > * Note: the bridge passed should be the one closest to the encoder > */ > void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge, > - struct drm_atomic_state *state) > + struct drm_atomic_state *old_state) > { > struct drm_encoder *encoder; > struct drm_bridge *iter; > @@ -469,7 +469,7 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge > *bridge, > list_for_each_entry_reverse(iter, >bridge_chain, > chain_node) { > if (iter->funcs->atomic_disable) > - iter->funcs->atomic_disable(iter, state); > + iter->funcs->atomic_disable(iter, old_state); > else if (iter->funcs->disable) > iter->funcs->disable(iter); > > @@ -483,7 +483,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable); > * drm_atomic_bridge_chain_post_disable - cleans up after disabling all > bridges > * in the encoder chain > * @bridge: bridge control structure > - * @state: atomic state being committed > + * @old_state: old atomic state > * > * Calls _bridge_funcs.atomic_post_disable (falls back on > * _bridge_funcs.post_disable) op for all the bridges in the encoder > chain, > @@ -493,7 +493,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable); > * Note: the bridge passed should be the one closest to the encoder > */ > void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, > - struct drm_atomic_state *state) > + struct drm_atomic_state *old_state) > { > struct drm_encoder *encoder; > > @@ -504,7 +504,7 @@ void drm_atomic_bridge_chain_post_disable(struct > drm_bridge *bridge, > list_for_each_entry_from(bridge, >bridge_chain, >chain_node) { > if (bridge->funcs->atomic_post_disable) > - bridge->funcs->atomic_post_disable(bridge, state); > + bridge->funcs->atomic_post_disable(bridge, old_state); > else if (bridge->funcs->post_disable) > bridge->funcs->post_disable(bridge); > } > @@ -515,7 +515,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); > * drm_atomic_bridge_chain_pre_enable - prepares for enabling all bridges in > * the encoder chain > * @bridge: bridge control structure > - * @state: atomic state being committed > + * @old_state: old atomic state > * > * Calls _bridge_funcs.atomic_pre_enable (falls back on > * _bridge_funcs.pre_enable) op for all the bridges in the encoder chain, > @@ -525,7 +525,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); > * Note: the bridge passed should be the one closest to the encoder > */ > void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, > - struct drm_atomic_state *state) > + struct drm_atomic_state *old_state) > { > struct drm_encoder *encoder; > struct drm_bridge *iter; > @@ -537,7 +537,7 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge > *bridge, > list_for_each_entry_reverse(iter, >encoder->bridge_chain, > chain_node) { > if (iter->funcs->atomic_pre_enable) > - iter->funcs->atomic_pre_enable(iter, state); > + iter->funcs->atomic_pre_enable(iter, old_state); > else if (iter->funcs->pre_enable) > iter->funcs->pre_enable(iter); > > @@ -550,7 +550,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable); > /** > * drm_atomic_bridge_chain_enable - enables all bridges in the encoder chain > * @bridge: bridge control
[PATCH v3 10/21] drm/bridge: Clarify the atomic enable/disable hooks semantics
The [pre_]enable/[post_]disable hooks are passed the old atomic state. Update the doc and rename the arguments to make it clear. Signed-off-by: Boris Brezillon --- Changes in v3: * New patch --- drivers/gpu/drm/drm_bridge.c | 24 include/drm/drm_bridge.h | 16 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c53966767782..ca74bfe028c9 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -447,7 +447,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable); /** * drm_atomic_bridge_chain_disable - disables all bridges in the encoder chain * @bridge: bridge control structure - * @state: atomic state being committed + * @old_state: old atomic state * * Calls _bridge_funcs.atomic_disable (falls back on * _bridge_funcs.disable) op for all the bridges in the encoder chain, @@ -457,7 +457,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable); * Note: the bridge passed should be the one closest to the encoder */ void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge, -struct drm_atomic_state *state) +struct drm_atomic_state *old_state) { struct drm_encoder *encoder; struct drm_bridge *iter; @@ -469,7 +469,7 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge, list_for_each_entry_reverse(iter, >bridge_chain, chain_node) { if (iter->funcs->atomic_disable) - iter->funcs->atomic_disable(iter, state); + iter->funcs->atomic_disable(iter, old_state); else if (iter->funcs->disable) iter->funcs->disable(iter); @@ -483,7 +483,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable); * drm_atomic_bridge_chain_post_disable - cleans up after disabling all bridges * in the encoder chain * @bridge: bridge control structure - * @state: atomic state being committed + * @old_state: old atomic state * * Calls _bridge_funcs.atomic_post_disable (falls back on * _bridge_funcs.post_disable) op for all the bridges in the encoder chain, @@ -493,7 +493,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable); * Note: the bridge passed should be the one closest to the encoder */ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, - struct drm_atomic_state *state) + struct drm_atomic_state *old_state) { struct drm_encoder *encoder; @@ -504,7 +504,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, list_for_each_entry_from(bridge, >bridge_chain, chain_node) { if (bridge->funcs->atomic_post_disable) - bridge->funcs->atomic_post_disable(bridge, state); + bridge->funcs->atomic_post_disable(bridge, old_state); else if (bridge->funcs->post_disable) bridge->funcs->post_disable(bridge); } @@ -515,7 +515,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); * drm_atomic_bridge_chain_pre_enable - prepares for enabling all bridges in * the encoder chain * @bridge: bridge control structure - * @state: atomic state being committed + * @old_state: old atomic state * * Calls _bridge_funcs.atomic_pre_enable (falls back on * _bridge_funcs.pre_enable) op for all the bridges in the encoder chain, @@ -525,7 +525,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); * Note: the bridge passed should be the one closest to the encoder */ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, - struct drm_atomic_state *state) + struct drm_atomic_state *old_state) { struct drm_encoder *encoder; struct drm_bridge *iter; @@ -537,7 +537,7 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, list_for_each_entry_reverse(iter, >encoder->bridge_chain, chain_node) { if (iter->funcs->atomic_pre_enable) - iter->funcs->atomic_pre_enable(iter, state); + iter->funcs->atomic_pre_enable(iter, old_state); else if (iter->funcs->pre_enable) iter->funcs->pre_enable(iter); @@ -550,7 +550,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable); /** * drm_atomic_bridge_chain_enable - enables all bridges in the encoder chain * @bridge: bridge control structure - * @state: atomic state being committed + * @old_state: old atomic state * * Calls _bridge_funcs.atomic_enable (falls back on * _bridge_funcs.enable) op for all the bridges in the