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: <20201030232324.11170-1-khsieh@codeaurora.org>
References: <20201030232324.11170-1-khsieh@codeaurora.org>
Subject: Re: [PATCH] drm/msm/dp: promote irq_hpd handle to handle link trainign correctly
From:   Stephen Boyd <swboyd@chromium.org>
Cc:     tanmay@codeaurora.org, abhinavk@codeaurora.org,
        aravindh@codeaurora.org, khsieh@codeaurora.org,
        rnayak@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:     Kuogee Hsieh <khsieh@codeaurora.org>, robdclark@gmail.com,
        sean@poorly.run
Date:   Mon, 02 Nov 2020 11:29:28 -0800
Message-ID: <160434536802.884498.16389146296525781476@swboyd.mtv.corp.google.com>
User-Agent: alot/0.9.1
Precedence: bulk

Subject has a typo in "training".

Quoting Kuogee Hsieh (2020-10-30 16:23:24)
> Some dongles, such as Apple, required link training done at irq_hpd

s/required/require/

> request instead of plugin request. This patch promote irq_hpd hanlder

s/hanlder/handler/

> to handle link training and setup hpd_state correctly.
>=20
> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
> ---

Any Fixes tag?

>  drivers/gpu/drm/msm/dp/dp_display.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp=
/dp_display.c
> index 13b66266cd69..55627530957c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -483,10 +485,24 @@ static int dp_display_usbpd_attention_cb(struct dev=
ice *dev)
>                 return -ENODEV;
>         }
> =20
> +       hpd =3D dp->usbpd;
> +
>         /* check for any test request issued by sink */
>         rc =3D dp_link_process_request(dp->link);
> -       if (!rc)
> -               dp_display_handle_irq_hpd(dp);
> +       if (!rc) {
> +               sink_request =3D dp->link->sink_request;
> +               if (sink_request & DS_PORT_STATUS_CHANGED) {
> +                       dp->hpd_state =3D ST_CONNECT_PENDING;
> +                       hpd->hpd_high =3D 1;
> +               }
> +
> +               rc =3D dp_display_handle_irq_hpd(dp);
> +
> +               if (rc && sink_request & DS_PORT_STATUS_CHANGED) {

Can you add parenthesis around this?

		if (rc && (sink_request & DS_PORT_STATUS_CHANGED)) {


I honestly don't know what's going on in this patch. It talks about
making link training happen during irq hpd handler but this is the
attention handler and we're checking port status changed? This is
related? The code is really not clear.

> +                       hpd->hpd_high =3D 0;
> +                       dp->hpd_state =3D ST_DISCONNECTED;
> +               }
> +       }
> =20
>         return rc;
>  }
>=20
> base-commit: 0e162b10644605428cd2596c12f8ed410cf9d2d9

What commit is this?