Received-SPF: pass (google.com: domain of linux-kernel+bounces-33738-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240118015916.2296741-1-hsinyi@chromium.org> <CAD=FV=WYm0=uyQMf8yZpqaCWN2dympTt_bUVPOe+nafBLdS_DQ@mail.gmail.com>
In-Reply-To: <CAD=FV=WYm0=uyQMf8yZpqaCWN2dympTt_bUVPOe+nafBLdS_DQ@mail.gmail.com>
From: Doug Anderson <dianders@chromium.org>
Date: Mon, 22 Jan 2024 09:01:19 -0800
Message-ID: <CAD=FV=UXqsULhVL0Mpk=V0VoR1yQaeBQA_6zYriCdQxNfEHktA@mail.gmail.com>
Subject: Re: [PATCH] drm/bridge: anx7625: Ensure bridge is suspended in disable()
To: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: dri-devel@lists.freedesktop.org, Andrzej Hajda <andrzej.hajda@intel.com>, 
	Neil Armstrong <neil.armstrong@linaro.org>, Robert Foss <rfoss@kernel.org>, 
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, Jonas Karlman <jonas@kwiboo.se>, 
	Jernej Skrabec <jernej.skrabec@gmail.com>, 
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, 
	Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, 
	Xin Ji <xji@analogixsemi.com>, Pin-yen Lin <treapking@chromium.org>, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, Jan 18, 2024 at 9:30=E2=80=AFAM Doug Anderson <dianders@chromium.or=
g> wrote:
>
> Hi,
>
> On Wed, Jan 17, 2024 at 5:59=E2=80=AFPM Hsin-Yi Wang <hsinyi@chromium.org=
> wrote:
> >
> > Similar to commit 26db46bc9c67 ("drm/bridge: parade-ps8640: Ensure brid=
ge
> > is suspended in .post_disable()"). Add a mutex to ensure that aux trans=
fer
> > won't race with atomic_disable by holding the PM reference and prevent
> > the bridge from suspend.
> >
> > Also we need to use pm_runtime_put_sync_suspend() to suspend the bridge
> > instead of idle with pm_runtime_put_sync().
> >
> > Fixes: 3203e497eb76 ("drm/bridge: anx7625: Synchronously run runtime su=
spend.")
> > Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through=
 aux channel")
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Tested-by: Xuxin Xiong <xuxinxiong@huaqin.corp-partner.google.com>
> > ---
> >  drivers/gpu/drm/bridge/analogix/anx7625.c | 7 ++++++-
> >  drivers/gpu/drm/bridge/analogix/anx7625.h | 2 ++
> >  2 files changed, 8 insertions(+), 1 deletion(-)
>
> This looks good to me.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I guess this started showing up more because of commit 49ddab089611
> ("drm/panel-edp: use put_sync in unprepare"), right? Since that's a
> recent commit then that means there's probably a little more urgency
> in getting this landed. That being said, it feels like it would be
> most legit to allow this to hang out on the list for a few days before
> landing. I'll aim for early next week unless someone else has any
> comments.

Pushed to drm-misc-fixes.

4d5b7daa3c61 drm/bridge: anx7625: Ensure bridge is suspended in disable()


> I guess we should see if we need to do something similar for
> ti-sn65dsi86 too since I could imagine it having similar problems.

I tried this myself and I couldn't see any problems there. I also
spent some time thinking about it and it seemed fine. There should be
no fundamental reason why we'd have to necessarily power cycle the
bridge chip in this case. Presumably ps8640 needs it because the
embedded firmware on ps8640 is easy to confuse. I'm less familiar with
the anx bridge chip, but I could believe something similar is
happening there.

-Doug