Hi,
On Wed, Apr 14, 2021 at 6:56 PM Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Doug,
On Wed, Apr 14, 2021 at 06:19:13PM -0700, Doug Anderson wrote:
On Sun, Apr 4, 2021 at 5:50 PM Laurent Pinchart wrote:
On Fri, Apr 02, 2021 at 03:28:35PM -0700, Douglas Anderson wrote:
The drm_bridge_chain_pre_enable() is not the proper opposite of drm_bridge_chain_post_disable(). It continues along the chain to _before_ the starting bridge. Let's fix that.
Fixes: 05193dc38197 ("drm/bridge: Make the bridge chain a double-linked list") Signed-off-by: Douglas Anderson dianders@chromium.org Reviewed-by: Andrzej Hajda a.hajda@samsung.com
(no changes since v1)
drivers/gpu/drm/drm_bridge.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..044acd07c153 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -522,6 +522,9 @@ void drm_bridge_chain_pre_enable(struct drm_bridge *bridge) 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;This looks good as it matches drm_atomic_bridge_chain_disable().
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Thanks for your review here and several of the other patches. Can you suggest any plan for getting them landed? It would at least be nice to get the non-controversial ones landed.
Do you have commit access to drm-misc ? If not, given your contributions, I think you qualify for it.
No, I don't have access. I searched for how to get it and read through the qualifications and, you're right, I think I do. I've hopefully followed the right flow and created an issue to give me ssh access:
https://gitlab.freedesktop.org/freedesktop/freedesktop/-/issues/348
Is that something you (or someone else on this CC list) approves?
I'm curious though, given that the bridge passed to the function should be the one closest to the encoder, does this make a difference ?
Yes, that's how I discovered it originally. Let's see. So if I don't have this patch but have the rest of the series then I get a splat at bootup. This shows that dsi_mgr_bridge_pre_enable() must be "earlier" in the chain than my bridge chip. Here's the splat:
Right, I think it's caused by a later patch in the series calling this function with a different bridge than the one closest to the encoder.
Yup! I still wanted this patch to be first in the series, though, since it's a bugfix that we'd want to land even if the later patches changed in some way.
-Doug