subject:"\[PATCH v5 08\/12\] drm\/msm\/dp\: Rely on hpd

Re: [PATCH v5 08/12] drm/msm/dp: Rely on hpd_enable/disable callbacks

2022-12-08 Thread Kuogee Hsieh




On 12/7/2022 2:00 PM, Bjorn Andersson wrote:

From: Bjorn Andersson 

The DisplayPort controller's internal HPD interrupt handling is used for
cases where the HPD signal is connected to a GPIO which is pinmuxed into
the DisplayPort controller. In other configurations the HPD notification
might be delivered by the DRM framework from an associated bridge.

This difference is not appropriately represented by the "is_edp"
boolean, but is properly represented by the frameworks invocation of the
hpd_enable() and hpd_disable() callbacks. Switch the current condition
to rely on these callbacks instead.

This ensures appropriate handling of the three cases; no bridge
connected, a bridge without DRM_BRIDGE_OP_HPD and a bridge with
DRM_BRIDGE_OP_HPD.

Signed-off-by: Bjorn Andersson 
Signed-off-by: Bjorn Andersson 
Reviewed-by: Dmitry Baryshkov 

Reviewed-by: Kuogee Hsieh 

---

Changes since v4:
- Reordered the hpd_enable/disable patch earlier
- Squashed in internal_hpd conditional changes into the same patch

  drivers/gpu/drm/msm/dp/dp_display.c | 40 +
  drivers/gpu/drm/msm/dp/dp_display.h |  1 +
  drivers/gpu/drm/msm/dp/dp_drm.c |  2 ++
  drivers/gpu/drm/msm/dp/dp_drm.h |  2 ++
  4 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 666b45c8ab80..095adf528e43 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -610,8 +610,10 @@ static int dp_hpd_plug_handle(struct dp_display_private 
*dp, u32 data)
}
  
  	/* enable HDP irq_hpd/replug interrupt */

-   dp_catalog_hpd_config_intr(dp->catalog,
-   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, true);
+   if (dp->dp_display.internal_hpd)
+   dp_catalog_hpd_config_intr(dp->catalog,
+  DP_DP_IRQ_HPD_INT_MASK | 
DP_DP_HPD_REPLUG_INT_MASK,
+  true);
  
  	drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",

dp->dp_display.connector_type, state);
@@ -651,8 +653,10 @@ static int dp_hpd_unplug_handle(struct dp_display_private 
*dp, u32 data)
dp->dp_display.connector_type, state);
  
  	/* disable irq_hpd/replug interrupts */

-   dp_catalog_hpd_config_intr(dp->catalog,
-   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, false);
+   if (dp->dp_display.internal_hpd)
+   dp_catalog_hpd_config_intr(dp->catalog,
+  DP_DP_IRQ_HPD_INT_MASK | 
DP_DP_HPD_REPLUG_INT_MASK,
+  false);
  
  	/* unplugged, no more irq_hpd handle */

dp_del_event(dp, EV_IRQ_HPD_INT);
@@ -678,7 +682,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private 
*dp, u32 data)
}
  
  	/* disable HPD plug interrupts */

-   dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
+   if (dp->dp_display.internal_hpd)
+   dp_catalog_hpd_config_intr(dp->catalog, 
DP_DP_HPD_PLUG_INT_MASK, false);
  
  	/*

 * We don't need separate work for disconnect as
@@ -696,7 +701,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private 
*dp, u32 data)
dp_display_handle_plugged_change(>dp_display, false);
  
  	/* enable HDP plug interrupt to prepare for next plugin */

-   if (!dp->dp_display.is_edp)
+   if (dp->dp_display.internal_hpd)
dp_catalog_hpd_config_intr(dp->catalog, 
DP_DP_HPD_PLUG_INT_MASK, true);
  
  	drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",

@@ -1081,8 +1086,8 @@ static void dp_display_config_hpd(struct 
dp_display_private *dp)
dp_display_host_init(dp);
dp_catalog_ctrl_hpd_config(dp->catalog);
  
-	/* Enable plug and unplug interrupts only for external DisplayPort */

-   if (!dp->dp_display.is_edp)
+   /* Enable plug and unplug interrupts only if requested */
+   if (dp->dp_display.internal_hpd)
dp_catalog_hpd_config_intr(dp->catalog,
DP_DP_HPD_PLUG_INT_MASK |
DP_DP_HPD_UNPLUG_INT_MASK,
@@ -1374,8 +1379,7 @@ static int dp_pm_resume(struct device *dev)
  
  	dp_catalog_ctrl_hpd_config(dp->catalog);
  
-

-   if (!dp->dp_display.is_edp)
+   if (dp->dp_display.internal_hpd)
dp_catalog_hpd_config_intr(dp->catalog,
DP_DP_HPD_PLUG_INT_MASK |
DP_DP_HPD_UNPLUG_INT_MASK,
@@ -1772,3 +1776,19 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
dp_display->dp_mode.h_active_low =
!!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC);
  }
+
+void dp_bridge_hpd_enable(struct drm_bridge *bridge)
+{
+   struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge);
+   struct msm_dp *dp_display = dp_bridge->dp_display;
+
+

[PATCH v5 08/12] drm/msm/dp: Rely on hpd_enable/disable callbacks

2022-12-07 Thread Bjorn Andersson

From: Bjorn Andersson 

The DisplayPort controller's internal HPD interrupt handling is used for
cases where the HPD signal is connected to a GPIO which is pinmuxed into
the DisplayPort controller. In other configurations the HPD notification
might be delivered by the DRM framework from an associated bridge.

This difference is not appropriately represented by the "is_edp"
boolean, but is properly represented by the frameworks invocation of the
hpd_enable() and hpd_disable() callbacks. Switch the current condition
to rely on these callbacks instead.

This ensures appropriate handling of the three cases; no bridge
connected, a bridge without DRM_BRIDGE_OP_HPD and a bridge with
DRM_BRIDGE_OP_HPD.

Signed-off-by: Bjorn Andersson 
Signed-off-by: Bjorn Andersson 
Reviewed-by: Dmitry Baryshkov 
---

Changes since v4:
- Reordered the hpd_enable/disable patch earlier
- Squashed in internal_hpd conditional changes into the same patch

 drivers/gpu/drm/msm/dp/dp_display.c | 40 +
 drivers/gpu/drm/msm/dp/dp_display.h |  1 +
 drivers/gpu/drm/msm/dp/dp_drm.c |  2 ++
 drivers/gpu/drm/msm/dp/dp_drm.h |  2 ++
 4 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 666b45c8ab80..095adf528e43 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -610,8 +610,10 @@ static int dp_hpd_plug_handle(struct dp_display_private 
*dp, u32 data)
}
 
/* enable HDP irq_hpd/replug interrupt */
-   dp_catalog_hpd_config_intr(dp->catalog,
-   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, true);
+   if (dp->dp_display.internal_hpd)
+   dp_catalog_hpd_config_intr(dp->catalog,
+  DP_DP_IRQ_HPD_INT_MASK | 
DP_DP_HPD_REPLUG_INT_MASK,
+  true);
 
drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
dp->dp_display.connector_type, state);
@@ -651,8 +653,10 @@ static int dp_hpd_unplug_handle(struct dp_display_private 
*dp, u32 data)
dp->dp_display.connector_type, state);
 
/* disable irq_hpd/replug interrupts */
-   dp_catalog_hpd_config_intr(dp->catalog,
-   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, false);
+   if (dp->dp_display.internal_hpd)
+   dp_catalog_hpd_config_intr(dp->catalog,
+  DP_DP_IRQ_HPD_INT_MASK | 
DP_DP_HPD_REPLUG_INT_MASK,
+  false);
 
/* unplugged, no more irq_hpd handle */
dp_del_event(dp, EV_IRQ_HPD_INT);
@@ -678,7 +682,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private 
*dp, u32 data)
}
 
/* disable HPD plug interrupts */
-   dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
+   if (dp->dp_display.internal_hpd)
+   dp_catalog_hpd_config_intr(dp->catalog, 
DP_DP_HPD_PLUG_INT_MASK, false);
 
/*
 * We don't need separate work for disconnect as
@@ -696,7 +701,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private 
*dp, u32 data)
dp_display_handle_plugged_change(>dp_display, false);
 
/* enable HDP plug interrupt to prepare for next plugin */
-   if (!dp->dp_display.is_edp)
+   if (dp->dp_display.internal_hpd)
dp_catalog_hpd_config_intr(dp->catalog, 
DP_DP_HPD_PLUG_INT_MASK, true);
 
drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
@@ -1081,8 +1086,8 @@ static void dp_display_config_hpd(struct 
dp_display_private *dp)
dp_display_host_init(dp);
dp_catalog_ctrl_hpd_config(dp->catalog);
 
-   /* Enable plug and unplug interrupts only for external DisplayPort */
-   if (!dp->dp_display.is_edp)
+   /* Enable plug and unplug interrupts only if requested */
+   if (dp->dp_display.internal_hpd)
dp_catalog_hpd_config_intr(dp->catalog,
DP_DP_HPD_PLUG_INT_MASK |
DP_DP_HPD_UNPLUG_INT_MASK,
@@ -1374,8 +1379,7 @@ static int dp_pm_resume(struct device *dev)
 
dp_catalog_ctrl_hpd_config(dp->catalog);
 
-
-   if (!dp->dp_display.is_edp)
+   if (dp->dp_display.internal_hpd)
dp_catalog_hpd_config_intr(dp->catalog,
DP_DP_HPD_PLUG_INT_MASK |
DP_DP_HPD_UNPLUG_INT_MASK,
@@ -1772,3 +1776,19 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
dp_display->dp_mode.h_active_low =
!!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC);
 }
+
+void dp_bridge_hpd_enable(struct drm_bridge *bridge)
+{
+   struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge);
+   struct msm_dp *dp_display = dp_bridge->dp_display;
+
+   dp_display->internal_hpd = true;
+}
+
+void

Re: [PATCH v5 08/12] drm/msm/dp: Rely on hpd_enable/disable callbacks

[PATCH v5 08/12] drm/msm/dp: Rely on hpd_enable/disable callbacks

2 matches

Site Navigation

Mail list logo

Footer information