Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <78036f23979206070bd9c9df180e2866@codeaurora.org>
References: <1618355504-5401-1-git-send-email-khsieh@codeaurora.org> <161843459482.46595.11409016331159748598@swboyd.mtv.corp.google.com> <60bceecc3d4dcc71c66a4b093d0e6c0f@codeaurora.org> <161851718969.46595.12896385877607476879@swboyd.mtv.corp.google.com> <78036f23979206070bd9c9df180e2866@codeaurora.org>
Subject: Re: [PATCH v2 3/3] drm/msm/dp: check main link status before start aux read
From:   Stephen Boyd <swboyd@chromium.org>
Cc:     robdclark@gmail.com, sean@poorly.run, abhinavk@codeaurora.org,
        aravindh@codeaurora.org, airlied@linux.ie, daniel@ffwll.ch,
        linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
        freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org
To:     khsieh@codeaurora.org
Date:   Thu, 15 Apr 2021 15:46:59 -0700
Message-ID: <161852681935.46595.9941294298184495830@swboyd.mtv.corp.google.com>
User-Agent: alot/0.9.1
Precedence: bulk

Quoting khsieh@codeaurora.org (2021-04-15 15:02:40)
> On 2021-04-15 13:06, Stephen Boyd wrote:
> >=20
> > Is it really necessary to have this patch at all? I think there are
> > bigger problems with suspend/resume of the DP driver in relation to the
> > kthread stopping. I hope that the aux channel would start NAKing
> > transfers once the cable is disconnected too, so that we don't need to
> > do an extra check for each aux transfer.
>=20
> I am working on duplicate this problem, but it is not happen on me yet=20
> so far.
>  From kernel dump, i can see it crash at dp_irq_hdp_handle() after=20
> suspended.
> dp_irq_hpd_handle and dp_pm_suspend() are serialized by event_mutex.
>=20
> After suspend, ahb clock is disabled.
> Hence next dp_catalog_link_is_connected() crash at acess dp ctrl=20
> registers.
>=20
>=20
> aux channel does not do NAKing immediately if unplugged. Therefore=20
> aux_transfer will wait until timeout (HZ/4).
> worst, drm_dp_dpcd_access() will retry 32 times before return dpcd=20
> read/write failed.
> This patch try to eliminate the time spinning on waiting for timeout 32=20
> times.
>=20

Would be useful to have that level of detail in the commit text.

Maybe when the cable is disconnected the DP phy should be shutdown and
some bit in the phy could effectively "cut off" the aux channel and then
NAKs would start coming through here in the DP controller I/O register
space?