From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5F277C433E0 for ; Mon, 22 Feb 2021 05:56:37 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0697464E57 for ; Mon, 22 Feb 2021 05:56:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0697464E57 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=55fyWjkW8U24NyvltMWEaNY/AvoQ5PAl1jsGi5O1Qek=; b=XVzOORoNtz4hTty0ufrqUtRbO 4AuN0ds4AYLVaR4hTPWJFhPh3+2pG2rS8u7ljmBoAWCMDA8hD+V+dP3Cl7tcUS8DKN/uqxOofehBB a8bRN6fXfoiDPQs9mNHSvWW6kpoDsJtGQL3g8YCH5OoeE61+1MS0yupEEVEsn/ubrrjUrrc1YbpOT fZZFU1Nbn7ubMd8nPXT4/lGcIfGYWixxY+64SqaOUoaUxDuDzd74Gw8sak3/41UEwDoF2q8Sf4sHM ue1/X1PFYWQq8mUM0LncqqOxgUY9Sokh9LJS2TQw/DfGAdlnv21EEdHLOK53WQb5i0hkc0oXLxHst npuzlEwxw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lE4Ba-0000rC-E2; Mon, 22 Feb 2021 05:55:10 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1lE4BY-0000qh-1Y for linux-arm-kernel@lists.infradead.org; Mon, 22 Feb 2021 05:55:09 +0000 Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0C9C1344; Mon, 22 Feb 2021 06:55:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1613973305; bh=s5Sf/8itHX3j/tX54sgaRdM+CLR9kswsZj0KP+smwyo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=t21IH+Ye39Y0Mp3BGrGaxYYuREWC1aK8Gts+hbw69RcrQ254uOgUD2o7bKYMKNk3x bmfgW+gG8PhMvzQULviLdxHoiJ1PZhTydNWmN8w8AK/4cOH1YVcQZ7W93CAulBkpIj 3Nm7mkMblnhijbqIkj+LTM+4Qtl/6ky+oCa8LpUQ= Date: Mon, 22 Feb 2021 07:54:38 +0200 From: Laurent Pinchart To: Jagan Teki Subject: Re: [PATCH v3 5/7] drm: bridge: Queue the bridge chain instead of stacking Message-ID: References: <20210214194102.126146-1-jagan@amarulasolutions.com> <20210214194102.126146-6-jagan@amarulasolutions.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210214194102.126146-6-jagan@amarulasolutions.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210222_005508_323710_011892D9 X-CRM114-Status: GOOD ( 19.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jernej Skrabec , Maarten Lankhorst , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Chen-Yu Tsai , Maxime Ripard , Thomas Zimmermann , linux-amarula@amarulasolutions.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Jagan, Thank you for the patch. On Mon, Feb 15, 2021 at 01:11:00AM +0530, Jagan Teki wrote: > drm_bridge_attach has stacked the bridge chain, so the bridge > that gets pushed last can trigger its bridge function pre_enable > first from drm_atomic_bridge_chain_pre_enable. > > This indeed gives a chance to trigger slave bridge pre_enable > first without triggering its host bridge pre_enable for the > usual host to slave device model like DSI host with panel slave. > > For fully enabled bridge drivers, host bridge pre_enable has all > host related clock, reset, PHY configuration code that needs to > initialized before sending commands or configuration from a slave > to communicate its host. > > Queue the bridge chain instead of stacking it so-that the bridges > that got enqueued first can have a chance to trigger first. First of all, won't thus break all the drivers that currently rely on the existing behaviour ? > Cc: Maarten Lankhorst > Cc: Thomas Zimmermann > Signed-off-by: Jagan Teki > --- > Changes for v3: > - new patch > > drivers/gpu/drm/drm_bridge.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index 64f0effb52ac..e75d1a080c55 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -191,9 +191,9 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, > bridge->encoder = encoder; > > if (previous) > - list_add(&bridge->chain_node, &previous->chain_node); > + list_add_tail(&bridge->chain_node, &previous->chain_node); > else > - list_add(&bridge->chain_node, &encoder->bridge_chain); > + list_add_tail(&bridge->chain_node, &encoder->bridge_chain); Then, this will create a really weird order, as the list will contain bridges in the reverse order. Assuming three bridges, A, B and C, which are connected at the hardware level as follows: Encoder -> A -> B -> C the list would contain Encoder -> C -> B -> A This isn't intuitive, and if you want to reverse the order in which bridge operations are called, it would be better to do so in the operations themselves, for instance replacing list_for_each_entry_reverse() with list_for_each_entry() in drm_atomic_bridge_chain_pre_enable(). Still, this will likely break drivers that depend on the existing order, so I don't think that's an acceptable solution as-is. > > if (bridge->funcs->attach) { > ret = bridge->funcs->attach(bridge, flags); -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel