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

From: Stephen Boyd
Date: Mon Sep 28 2020 - 20:12:03 EST


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@xxxxxxxxxxxxxx>

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,