linux-kernel - Re: [PATCH v2] drm/msm/dp: return correct connection status after suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <160133831826.310579.7427870479947629849@swboyd.mtv.corp.google.com>
Date:   Mon, 28 Sep 2020 17:11:58 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Kuogee Hsieh <khsieh@...eaurora.org>, robdclark@...il.com,
        sean@...rly.run
Cc:     tanmay@...eaurora.org, abhinavk@...eaurora.org,
        aravindh@...eaurora.org, khsieh@...eaurora.org, airlied@...ux.ie,
        daniel@...ll.ch, linux-arm-msm@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm/dp: return correct connection status after suspend

Quoting Kuogee Hsieh (2020-09-26 13:34:54)
> At dp_pm_resume, reinitialize both dp host controller and hpd block

dp_pm_resume()

> so that hpd connection can be detected at realtime by reading hpd state
> status register. Also hpd plug interrupt can be generated accordingly.

Can you describe more here? The subject says "return correct connection
status after suspend" so it seems that suspend connection status is
broken. How is it broken? What can be done to see if it is broken? I
think you can suspend, disconnect the DP cable, and then resume and see
that the device is connected still?

What does "hpd plug interrupt can be generated accordingly" mean? Is the
interrupt not being generated?

> 
> Changes in v2:
> -- use container_of to cast correct dp_display_private pointer
>    at both dp_pm_suspend and dp_pm_resume.
> 
> Signed-off-by: Kuogee Hsieh <khsieh@...eaurora.org>

Any Fixes tag?

> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index b15b4ce4ba35..63c5ada34c21 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -572,6 +572,19 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog *dp_catalog)
>         dp_write_aux(catalog, REG_DP_DP_HPD_CTRL, DP_DP_HPD_CTRL_HPD_EN);
>  }
>  
> +u32 dp_catalog_hpd_get_state_status(struct dp_catalog *dp_catalog)
> +{
> +       struct dp_catalog_private *catalog = container_of(dp_catalog,
> +                               struct dp_catalog_private, dp_catalog);
> +       u32 status = 0;

We don't need to assign to 0 to reassign immediately after.

> +
> +       status = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS);
> +       status >>= DP_DP_HPD_STATE_STATUS_BITS_SHIFT;
> +       status &= DP_DP_HPD_STATE_STATUS_BITS_MASK;
> +
> +       return status;
> +}
> +
>  u32 dp_catalog_hpd_get_intr_status(struct dp_catalog *dp_catalog)
>  {
>         struct dp_catalog_private *catalog = container_of(dp_catalog,