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: <1610051425-20632-3-git-send-email-khsieh@codeaurora.org>
References: <y> <1610051425-20632-1-git-send-email-khsieh@codeaurora.org> <1610051425-20632-3-git-send-email-khsieh@codeaurora.org>
Subject: Re: [PATCH 2/2] drm/msm/dp: unplug interrupt missed after irq_hpd handler
From:   Stephen Boyd <swboyd@chromium.org>
Cc:     Kuogee Hsieh <khsieh@codeaurora.org>, tanmay@codeaurora.org,
        abhinavk@codeaurora.org, aravindh@codeaurora.org, airlied@linux.ie,
        daniel@ffwll.ch, linux-arm-msm@vger.kernel.org,
        freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org
To:     Kuogee Hsieh <khsieh@codeaurora.org>,
        dri-devel@lists.freedesktop.org, robdclark@gmail.com,
        sean@poorly.run
Date:   Mon, 11 Jan 2021 11:54:01 -0800
Message-ID: <161039484176.3661239.14240346276437866761@swboyd.mtv.corp.google.com>
User-Agent: alot/0.9.1
Precedence: bulk

Quoting Kuogee Hsieh (2021-01-07 12:30:25)
> There is HPD unplug interrupts missed at scenario of an irq_hpd
> followed by unplug interrupts with around 10 ms in between.
> Since both AUX_SW_RESET and DP_SW_RESET clear pending HPD interrupts,
> irq_hpd handler should not issues either aux or sw reset to avoid
> following unplug interrupt be cleared accidentally.

So the problem is that we're resetting the DP aux phy in the middle of
the HPD state machine transitioning states?

>=20
> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
> ---
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp=
/dp_catalog.c
> index 44f0c57..9c0ce98 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -190,6 +190,18 @@ int dp_catalog_aux_clear_hw_interrupts(struct dp_cat=
alog *dp_catalog)
>         return 0;
>  }
> =20
> +/**
> + * dp_catalog_aux_reset() - reset AUX controller
> + *
> + * @aux: DP catalog structure
> + *
> + * return: void
> + *
> + * This function reset AUX controller
> + *
> + * NOTE: reset AUX controller will also clear any pending HPD related in=
terrupts
> + *=20
> + */
>  void dp_catalog_aux_reset(struct dp_catalog *dp_catalog)
>  {
>         u32 aux_ctrl;
> @@ -483,6 +495,18 @@ int dp_catalog_ctrl_set_pattern(struct dp_catalog *d=
p_catalog,
>         return 0;
>  }
> =20
> +/**
> + * dp_catalog_ctrl_reset() - reset DP controller
> + *
> + * @aux: DP catalog structure

It's called dp_catalog though.

> + *
> + * return: void
> + *
> + * This function reset DP controller

resets the

> + *
> + * NOTE: reset DP controller will also clear any pending HPD related int=
errupts
> + *=20
> + */
>  void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog)
>  {
>         u32 sw_reset;
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp=
_ctrl.c
> index e3462f5..f96c415 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1296,7 +1296,8 @@ static int dp_ctrl_setup_main_link(struct dp_ctrl_p=
rivate *ctrl,
>          * transitioned to PUSH_IDLE. In order to start transmitting
>          * a link training pattern, we have to first do soft reset.
>          */
> -       dp_catalog_ctrl_reset(ctrl->catalog);
> +       if (*training_step !=3D DP_TRAINING_NONE)

Can we check for the positive value instead? i.e.
DP_TRAINING_1/DP_TRAINING_2

> +               dp_catalog_ctrl_reset(ctrl->catalog);
> =20
>         ret =3D dp_ctrl_link_train(ctrl, cr, training_step);
>