Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <1694813901-26952-1-git-send-email-quic_khsieh@quicinc.com> <1694813901-26952-3-git-send-email-quic_khsieh@quicinc.com>
In-Reply-To: <1694813901-26952-3-git-send-email-quic_khsieh@quicinc.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Sat, 16 Sep 2023 04:51:36 +0300
Message-ID: <CAA8EJprRrTW3pqmjVbfKG5t8t_QNGvPboETNgaAUxg3PvkRjcA@mail.gmail.com>
Subject: Re: [PATCH v3 2/7] drm/msm/dp: replace is_connected with link_ready
To:     Kuogee Hsieh <quic_khsieh@quicinc.com>
Cc:     dri-devel@lists.freedesktop.org, robdclark@gmail.com,
        sean@poorly.run, swboyd@chromium.org, dianders@chromium.org,
        vkoul@kernel.org, daniel@ffwll.ch, airlied@gmail.com,
        agross@kernel.org, andersson@kernel.org, quic_abhinavk@quicinc.com,
        quic_jesszhan@quicinc.com, quic_sbillaka@quicinc.com,
        marijn.suijten@somainline.org, freedreno@lists.freedesktop.org,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, 16 Sept 2023 at 00:38, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
> The is_connected flag is set to true after DP mainlink successfully
> finish link training. Replace the is_connected flag with link_ready

finishes.
Also this is not a replace, this patch renames the flag.

> flag to avoid confusing.

confusing what with what?

>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 19 +++++++++----------
>  drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>  drivers/gpu/drm/msm/dp/dp_drm.c     | 14 +++++++-------
>  3 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index c217430..18d16c7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -367,12 +367,11 @@ static void dp_display_send_hpd_event(struct msm_dp *dp_display)
>         drm_helper_hpd_irq_event(connector->dev);
>  }
>
> -
>  static int dp_display_send_hpd_notification(struct dp_display_private *dp,
>                                             bool hpd)
>  {
> -       if ((hpd && dp->dp_display.is_connected) ||
> -                       (!hpd && !dp->dp_display.is_connected)) {
> +       if ((hpd && dp->dp_display.link_ready) ||
> +                       (!hpd && !dp->dp_display.link_ready)) {
>                 drm_dbg_dp(dp->drm_dev, "HPD already %s\n",
>                                 (hpd ? "on" : "off"));
>                 return 0;
> @@ -382,7 +381,7 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp,
>         if (!hpd)
>                 dp->panel->video_test = false;
>
> -       dp->dp_display.is_connected = hpd;
> +       dp->dp_display.link_ready = hpd;
>
>         drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n",
>                         dp->dp_display.connector_type, hpd);
> @@ -922,7 +921,7 @@ int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>
>         dp_display->plugged_cb = fn;
>         dp_display->codec_dev = codec_dev;
> -       plugged = dp_display->is_connected;
> +       plugged = dp_display->link_ready;
>         dp_display_handle_plugged_change(dp_display, plugged);
>
>         return 0;
> @@ -1352,16 +1351,16 @@ static int dp_pm_resume(struct device *dev)
>          * also only signal audio when disconnected
>          */
>         if (dp->link->sink_count) {
> -               dp->dp_display.is_connected = true;
> +               dp->dp_display.link_ready = true;
>         } else {
> -               dp->dp_display.is_connected = false;
> +               dp->dp_display.link_ready = false;
>                 dp_display_handle_plugged_change(dp_display, false);
>         }
>
>         drm_dbg_dp(dp->drm_dev,
>                 "After, type=%d sink=%d conn=%d core_init=%d phy_init=%d power=%d\n",
>                 dp->dp_display.connector_type, dp->link->sink_count,
> -               dp->dp_display.is_connected, dp->core_initialized,
> +               dp->dp_display.link_ready, dp->core_initialized,
>                 dp->phy_initialized, dp_display->power_on);
>
>         mutex_unlock(&dp->event_mutex);
> @@ -1754,8 +1753,8 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
>                 return;
>         }
>
> -       if (!dp_display->is_connected && status == connector_status_connected)
> +       if (!dp_display->link_ready && status == connector_status_connected)
>                 dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> -       else if (dp_display->is_connected && status == connector_status_disconnected)
> +       else if (dp_display->link_ready && status == connector_status_disconnected)
>                 dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
>  }
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
> index b3c08de..d65693e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -16,7 +16,7 @@ struct msm_dp {
>         struct drm_bridge *bridge;
>         struct drm_connector *connector;
>         struct drm_bridge *next_bridge;
> -       bool is_connected;
> +       bool link_ready;
>         bool audio_enabled;
>         bool power_on;
>         unsigned int connector_type;
> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> index 785d766..ee945ca 100644
> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> @@ -24,10 +24,10 @@ static enum drm_connector_status dp_bridge_detect(struct drm_bridge *bridge)
>
>         dp = to_dp_bridge(bridge)->dp_display;
>
> -       drm_dbg_dp(dp->drm_dev, "is_connected = %s\n",
> -               (dp->is_connected) ? "true" : "false");
> +       drm_dbg_dp(dp->drm_dev, "link_ready = %s\n",
> +               (dp->link_ready) ? "true" : "false");
>
> -       return (dp->is_connected) ? connector_status_connected :
> +       return (dp->link_ready) ? connector_status_connected :
>                                         connector_status_disconnected;
>  }
>
> @@ -40,8 +40,8 @@ static int dp_bridge_atomic_check(struct drm_bridge *bridge,
>
>         dp = to_dp_bridge(bridge)->dp_display;
>
> -       drm_dbg_dp(dp->drm_dev, "is_connected = %s\n",
> -               (dp->is_connected) ? "true" : "false");
> +       drm_dbg_dp(dp->drm_dev, "link_ready = %s\n",
> +               (dp->link_ready) ? "true" : "false");
>
>         /*
>          * There is no protection in the DRM framework to check if the display
> @@ -55,7 +55,7 @@ static int dp_bridge_atomic_check(struct drm_bridge *bridge,
>          * After that this piece of code can be removed.
>          */
>         if (bridge->ops & DRM_BRIDGE_OP_HPD)
> -               return (dp->is_connected) ? 0 : -ENOTCONN;
> +               return (dp->link_ready) ? 0 : -ENOTCONN;
>
>         return 0;
>  }
> @@ -78,7 +78,7 @@ static int dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *
>         dp = to_dp_bridge(bridge)->dp_display;
>
>         /* pluggable case assumes EDID is read when HPD */
> -       if (dp->is_connected) {
> +       if (dp->link_ready) {
>                 rc = dp_display_get_modes(dp);
>                 if (rc <= 0) {
>                         DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
> --
> 2.7.4
>


-- 
With best wishes
Dmitry