Received-SPF: pass (google.com: domain of linux-kernel+bounces-177852-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240513080243.3952292-1-victor.liu@nxp.com> <4b6e49ee-d2fd-4e54-88d5-ab06d8ebf644@bosc.ac.cn>
In-Reply-To: <4b6e49ee-d2fd-4e54-88d5-ab06d8ebf644@bosc.ac.cn>
From: Robert Foss <rfoss@kernel.org>
Date: Mon, 13 May 2024 18:49:59 +0200
Message-ID: <CAN6tsi5uZf57G26PmSiUZMCCPAV1dhWGDWT35HPmpVDevVJoxA@mail.gmail.com>
Subject: Re: drm/bridge: adv7511: Attach next bridge without creating connector
To: Sui Jingfeng <suijingfeng@bosc.ac.cn>
Cc: Liu Ying <victor.liu@nxp.com>, dri-devel@lists.freedesktop.org, 
	linux-kernel@vger.kernel.org, andrzej.hajda@intel.com, 
	neil.armstrong@linaro.org, Laurent.pinchart@ideasonboard.com, jonas@kwiboo.se, 
	jernej.skrabec@gmail.com, maarten.lankhorst@linux.intel.com, 
	mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch, 
	dmitry.baryshkov@linaro.org, biju.das.jz@bp.renesas.com, aford173@gmail.com, 
	bli@bang-olufsen.dk, robh@kernel.org, jani.nikula@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, May 13, 2024 at 6:30=E2=80=AFPM Sui Jingfeng <suijingfeng@bosc.ac.c=
n> wrote:
>
> Hi,
>
>
> On 5/13/24 16:02, Liu Ying wrote:
> > The connector is created by either this ADV7511 bridge driver or
> > any DRM device driver/previous bridge driver, so this ADV7511
> > bridge driver should not let the next bridge driver create connector.
> >
> > If the next bridge is a HDMI connector, the next bridge driver
> > would fail to attach bridge from display_connector_attach() without
> > the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
> >
> > Add that flag to drm_bridge_attach() function call in
> > adv7511_bridge_attach() to fix the issue.
> >
> > This fixes the issue where the HDMI connector bridge fails to attach
> > to the previous ADV7535 bridge on i.MX8MP EVK platform:
> >
> > [    2.216442] [drm:drm_bridge_attach] *ERROR* failed to attach bridge =
/hdmi-connector to encoder None-37: -22
> > [    2.220675] mmc1: SDHCI controller on 30b50000.mmc [30b50000.mmc] us=
ing ADMA
> > [    2.226262] [drm:drm_bridge_attach] *ERROR* failed to attach bridge =
/soc@0/bus@30800000/i2c@30a30000/hdmi@3d to encoder None-37: -22
> > [    2.245204] [drm:drm_bridge_attach] *ERROR* failed to attach bridge =
/soc@0/bus@32c00000/dsi@32e60000 to encoder None-37: -22
> > [    2.256445] imx-lcdif 32e80000.display-controller: error -EINVAL: Fa=
iled to attach bridge for endpoint0
> > [    2.265850] imx-lcdif 32e80000.display-controller: error -EINVAL: Ca=
nnot connect bridge
> > [    2.274009] imx-lcdif 32e80000.display-controller: probe with driver=
 imx-lcdif failed with error -22
> >
> > Fixes: 14b3cdbd0e5b ("drm/bridge: adv7511: make it honour next bridge i=
n DT")
> > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > ---
> >   drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu=
/drm/bridge/adv7511/adv7511_drv.c
> > index dd21b81bd28f..66ccb61e2a66 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > @@ -953,7 +953,8 @@ static int adv7511_bridge_attach(struct drm_bridge =
*bridge,
> >       int ret =3D 0;
> >
> >       if (adv->next_bridge) {
> > -             ret =3D drm_bridge_attach(bridge->encoder, adv->next_brid=
ge, bridge, flags);
> > +             ret =3D drm_bridge_attach(bridge->encoder, adv->next_brid=
ge, bridge,
> > +                                     flags | DRM_BRIDGE_ATTACH_NO_CONN=
ECTOR);
>
> As a side note, I think, maybe you could do better in the future.
>
> If we know that the KMS display driver side has the HDMI connector
> already created for us, we should pass DRM_BRIDGE_ATTACH_NO_CONNECTOR
> from the root KMS driver side. Which is to forbidden all potential
> drm bridge drivers to create a connector in the middle.
>
> The KMS display driver side could parse the DT to know if there is
> a hdmi connector, or merely just hdmi connector device node, or
> something else.
>
> However, other maintainer and/or reviewer's opinion are of cause
> more valuable. I send a A-b because I thought the bug is urgency
> and it's probably more important to solve this bug first. And
> maybe you can Cc: <stable@vger.kernel.org> if you like.
>

Reviewed-by: Robert Foss <rfoss@kernel.org>