Hi Neil,
Sorry for the late reply.
On Tue, 5 Nov 2019 17:02:30 +0100 Neil Armstrong narmstrong@baylibre.com wrote:
Hi,
On 25/10/2019 15:29, Neil Armstrong wrote:
On 23/10/2019 17:44, Boris Brezillon wrote:
So that each element in the chain can easily access its predecessor. This will be needed to support bus format negotiation between elements of the bridge chain.
Signed-off-by: Boris Brezillon boris.brezillon@collabora.com
Changes in v3:
- None
Changes in v2:
- Adjust things to the "dummy encoder bridge" change (patch 2 in this series)
drivers/gpu/drm/drm_bridge.c | 171 ++++++++++++++++++++++------------ drivers/gpu/drm/drm_encoder.c | 16 +--- include/drm/drm_bridge.h | 12 ++- include/drm/drm_encoder.h | 9 +- 4 files changed, 135 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 54c874493c57..c5cf8a9c4237 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c
[...]
@@ -426,15 +471,23 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, struct drm_atomic_state *state) {
- struct drm_encoder *encoder;
- struct drm_bridge *iter;
- if (!bridge) return;
- drm_atomic_bridge_chain_pre_enable(bridge->next, state);
- encoder = bridge->encoder;
- list_for_each_entry_reverse(iter, &bridge->encoder->bridge_chain,
chain_node) {This should use the encoder local variable in list_for_each_entry_reverse()
if (iter->funcs->atomic_pre_enable)iter->funcs->atomic_pre_enable(iter, state);else if (iter->funcs->pre_enable)iter->funcs->pre_enable(iter);
- if (bridge->funcs->atomic_pre_enable)
bridge->funcs->atomic_pre_enable(bridge, state);- else if (bridge->funcs->pre_enable)
bridge->funcs->pre_enable(bridge);
if (iter == bridge)break;- }
} EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
@@ -453,15 +506,19 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable); void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge, struct drm_atomic_state *state) {
- struct drm_encoder *encoder;
- if (!bridge) return;
- if (bridge->funcs->atomic_enable)
bridge->funcs->atomic_enable(bridge, state);- else if (bridge->funcs->enable)
bridge->funcs->enable(bridge);- drm_atomic_bridge_chain_enable(bridge->next, state);
- encoder = bridge->encoder;
- list_for_each_entry_from(bridge, &bridge->encoder->bridge_chain,
chain_node) {This should use encoder instead of bridge->encoder otherwise bridge will change and bridge->encoder->bridge_chain won't be valid during the for_each and cause the following :
Oops, indeed. I fixed the very same problem in another helper but somehow missed those 2. Thanks for testing/reporting the bug.
Boris