| Message ID | 20221205174433.16847-11-quic_bjorande@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm: Add SC8280XP support | expand |
On 5 December 2022 20:44:30 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >From: Bjorn Andersson <bjorn.andersson@linaro.org> > >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 <bjorn.andersson@linaro.org> >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >--- > >Worth mentioning, I did look into moving the HPD enablement/disablement >completely into these new callbacks, but that affect the entire power >management model of the driver, so I think it's worth to tackle that in >subsequent changes. It seems also reasonable to expect that we by such >modifications could leave the block unclocked until the external HPD >notification arrives... I see... I still suppose this is the way to go in the long term. For now: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >Changes since v3: >- Introduced reliance on hpd_enable/disable callbacks instead of next_bridge > > drivers/gpu/drm/msm/dp/dp_display.c | 35 ++++++++++++++++++++--------- > 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, 30 insertions(+), 10 deletions(-) > >diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >index bb92c33beff8..3e464c33ff11 100644 >--- a/drivers/gpu/drm/msm/dp/dp_display.c >+++ b/drivers/gpu/drm/msm/dp/dp_display.c >@@ -610,7 +610,7 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data) > } > > /* enable HDP irq_hpd/replug interrupt */ >- if (!dp->dp_display.is_edp) >+ 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); >@@ -653,7 +653,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > dp->dp_display.connector_type, state); > > /* disable irq_hpd/replug interrupts */ >- if (!dp->dp_display.is_edp) >+ 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); >@@ -682,7 +682,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > } > > /* disable HPD plug interrupts */ >- 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, false); > > /* >@@ -701,7 +701,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > dp_display_handle_plugged_change(&dp->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", >@@ -1086,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, >@@ -1379,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, >@@ -1778,6 +1777,22 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > !!(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 dp_bridge_hpd_disable(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 = false; >+} >+ > void dp_bridge_hpd_notify(struct drm_bridge *bridge, > enum drm_connector_status status) > { >@@ -1785,8 +1800,8 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge, > struct msm_dp *dp_display = dp_bridge->dp_display; > struct dp_display_private *dp = container_of(dp_display, struct dp_display_private, dp_display); > >- /* Without next_bridge interrupts are handled by the DP core directly */ >- if (!dp_display->next_bridge) >+ /* Plug events are generated by the dp_display_irq_handler() */ >+ if (dp_display->internal_hpd) > return; > > if (!dp->core_initialized) { >diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h >index dcedf021f7fe..371337d0fae2 100644 >--- a/drivers/gpu/drm/msm/dp/dp_display.h >+++ b/drivers/gpu/drm/msm/dp/dp_display.h >@@ -21,6 +21,7 @@ struct msm_dp { > bool power_on; > unsigned int connector_type; > bool is_edp; >+ bool internal_hpd; > > hdmi_codec_plugged_cb plugged_cb; > >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c >index 3898366ebd5e..275370f21115 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.c >+++ b/drivers/gpu/drm/msm/dp/dp_drm.c >@@ -102,6 +102,8 @@ static const struct drm_bridge_funcs dp_bridge_ops = { > .get_modes = dp_bridge_get_modes, > .detect = dp_bridge_detect, > .atomic_check = dp_bridge_atomic_check, >+ .hpd_enable = dp_bridge_hpd_enable, >+ .hpd_disable = dp_bridge_hpd_disable, > .hpd_notify = dp_bridge_hpd_notify, > }; > >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h >index 79e6b2cf2d25..250f7c66201f 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.h >+++ b/drivers/gpu/drm/msm/dp/dp_drm.h >@@ -32,6 +32,8 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, > void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > const struct drm_display_mode *mode, > const struct drm_display_mode *adjusted_mode); >+void dp_bridge_hpd_enable(struct drm_bridge *bridge); >+void dp_bridge_hpd_disable(struct drm_bridge *bridge); > void dp_bridge_hpd_notify(struct drm_bridge *bridge, > enum drm_connector_status status); >
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index bb92c33beff8..3e464c33ff11 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -610,7 +610,7 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data) } /* enable HDP irq_hpd/replug interrupt */ - if (!dp->dp_display.is_edp) + 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); @@ -653,7 +653,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) dp->dp_display.connector_type, state); /* disable irq_hpd/replug interrupts */ - if (!dp->dp_display.is_edp) + 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); @@ -682,7 +682,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) } /* disable HPD plug interrupts */ - 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, false); /* @@ -701,7 +701,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) dp_display_handle_plugged_change(&dp->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", @@ -1086,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, @@ -1379,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, @@ -1778,6 +1777,22 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge, !!(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 dp_bridge_hpd_disable(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 = false; +} + void dp_bridge_hpd_notify(struct drm_bridge *bridge, enum drm_connector_status status) { @@ -1785,8 +1800,8 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge, struct msm_dp *dp_display = dp_bridge->dp_display; struct dp_display_private *dp = container_of(dp_display, struct dp_display_private, dp_display); - /* Without next_bridge interrupts are handled by the DP core directly */ - if (!dp_display->next_bridge) + /* Plug events are generated by the dp_display_irq_handler() */ + if (dp_display->internal_hpd) return; if (!dp->core_initialized) { diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h index dcedf021f7fe..371337d0fae2 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -21,6 +21,7 @@ struct msm_dp { bool power_on; unsigned int connector_type; bool is_edp; + bool internal_hpd; hdmi_codec_plugged_cb plugged_cb; diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c index 3898366ebd5e..275370f21115 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -102,6 +102,8 @@ static const struct drm_bridge_funcs dp_bridge_ops = { .get_modes = dp_bridge_get_modes, .detect = dp_bridge_detect, .atomic_check = dp_bridge_atomic_check, + .hpd_enable = dp_bridge_hpd_enable, + .hpd_disable = dp_bridge_hpd_disable, .hpd_notify = dp_bridge_hpd_notify, }; diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h index 79e6b2cf2d25..250f7c66201f 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.h +++ b/drivers/gpu/drm/msm/dp/dp_drm.h @@ -32,6 +32,8 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, void dp_bridge_mode_set(struct drm_bridge *drm_bridge, const struct drm_display_mode *mode, const struct drm_display_mode *adjusted_mode); +void dp_bridge_hpd_enable(struct drm_bridge *bridge); +void dp_bridge_hpd_disable(struct drm_bridge *bridge); void dp_bridge_hpd_notify(struct drm_bridge *bridge, enum drm_connector_status status);