Hi Douglas,
void drm_bridge_chain_pre_enable(struct drm_bridge *bridge)
If you, or someone else, could r-b or ack the pending patches to remove this function, this part of the patch would no longer be needed.
OK. I will likely be able to take a look next week. Given that I'm helping Philip bringup a board with ps8640 it looks like your patch series will be quite relevant! I guess it would be good to figure out what would be the best order to land them. In my case we need this fix to be easy to pick back to fix the behavior on the Chrome OS 5.4 tree. My fix is easy to pick back, but perhaps yours is as well. Of course we could also just make a local divergent change in our tree if need be, too.
I do not mind the order - so whatever works for you guys. The only concern here is that we should not gain new users.
{ struct drm_encoder *encoder;
struct drm_bridge *iter; if (!bridge) return; encoder = bridge->encoder;list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {if (iter->funcs->pre_enable)iter->funcs->pre_enable(iter);if (iter == bridge)break;
list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {if (bridge->funcs->pre_enable)bridge->funcs->pre_enable(bridge); }} EXPORT_SYMBOL(drm_bridge_chain_pre_enable); @@ -684,26 +680,30 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, struct drm_atomic_state *old_state) { struct drm_encoder *encoder;
struct drm_bridge *iter;s/iter/bridge/ would make the patch simpler And then the bridge argument could be last_bridge or something. This would IMO increase readability of the code and make the patch smaller.
Yeah, I debated this too. I was trying to match drm_bridge_chain_disable() and in my mind keeping the two functions matching is more important than keeping this patch small.
Well, drm_bridge_chain_disable() is about to be deleted. So that the wrong one to look at.
Certainly I could add another patch in the series to rename "bridge" to "last_bridge" and "iter" to "bridge" in that function, but that defeats the goal of reducing churn... ...and clearly whoever wrote drm_bridge_chain_disable() liked "iter" better. :-P
In any case, I'll change it as you say if everyone likes it better, but otherwise I'll leave it as I have it.
if (!bridge) return; encoder = bridge->encoder;
list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {if (bridge->funcs->atomic_post_disable) {
list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {if (iter->funcs->atomic_post_disable) { struct drm_bridge_state *old_bridge_state; old_bridge_state = drm_atomic_get_old_bridge_state(old_state,
bridge);
iter); if (WARN_ON(!old_bridge_state)) return;
bridge->funcs->atomic_post_disable(bridge,old_bridge_state);} else if (bridge->funcs->post_disable) {bridge->funcs->post_disable(bridge);
iter->funcs->atomic_post_disable(iter,old_bridge_state);} else if (iter->funcs->post_disable) {iter->funcs->post_disable(iter); }if (iter == bridge)break;I cannot see why this is needed, we are at the end of the list here anyway.
I see, please include this change in your changelog and add it to the documentation in drm_bridge_h.
Sam