| Message ID | 1649938766-6768-3-git-send-email-quic_sbillaka@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for the eDP panel over aux_bus | expand |
Hi, On Thu, Apr 14, 2022 at 5:20 AM Sankeerth Billakanti <quic_sbillaka@quicinc.com> wrote: > > The panel-edp enables the eDP panel power during probe, get_modes > and enable. Technically the panel-edp powers on the panel in pre_enable() > The eDP connect and disconnect interrupts for the eDP/DP > controller are directly dependent on panel power. As eDP display can be > assumed as always connected, the controller driver can skip the eDP > connect and disconnect interrupts. Any disruption in the link status > will be indicated via the IRQ_HPD interrupts. > > So, the eDP controller driver can just enable the IRQ_HPD and replug > interrupts. The DP controller driver still needs to enable all the > interrupts. > > Signed-off-by: Sankeerth Billakanti <quic_sbillaka@quicinc.com> > --- > > Changes in v7: > - reordered the patch in the series > - modified the return statement for isr > - connector check modified to just check for eDP > > drivers/gpu/drm/msm/dp/dp_catalog.c | 9 +++------ > drivers/gpu/drm/msm/dp/dp_display.c | 22 +++++++++++++++++++++- > 2 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c > index fac815f..07f2389 100644 > --- a/drivers/gpu/drm/msm/dp/dp_catalog.c > +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c > @@ -569,10 +569,6 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog *dp_catalog) > > u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER); > > - /* enable HPD plug and unplug interrupts */ > - dp_catalog_hpd_config_intr(dp_catalog, > - DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK, true); > - > /* Configure REFTIMER and enable it */ > reftimer |= DP_DP_HPD_REFTIMER_ENABLE; > dp_write_aux(catalog, REG_DP_DP_HPD_REFTIMER, reftimer); > @@ -599,13 +595,14 @@ u32 dp_catalog_hpd_get_intr_status(struct dp_catalog *dp_catalog) > { > struct dp_catalog_private *catalog = container_of(dp_catalog, > struct dp_catalog_private, dp_catalog); > - int isr = 0; > + int isr, mask; > > isr = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS); > dp_write_aux(catalog, REG_DP_DP_HPD_INT_ACK, > (isr & DP_DP_HPD_INT_MASK)); > + mask = dp_read_aux(catalog, REG_DP_DP_HPD_INT_MASK); > > - return isr; > + return isr & (mask | ~DP_DP_HPD_INT_MASK); Please add a comment above this explaining what the goal of the above statement is. I guess it's something like this, though you might want to modify it to remove snark and insert the real reason unless you like being snarky: /* * Report the raw status of all interrupts (AKA we still report the * interrupt as asserted even if it's masked) _except_ for HPD-related. * interrupts. We only report HPD-related interrupts if they're * unmasked. We do it this way because we thought it would be extra * confusing for readers of this code and we were bribed by Mordac to * confuse you. OK, maybe that's not true. We actually do it this way * because of <insert your compelling reason here>. */ Along the same lines as my comments in patch #1, I don't have a great feel for exactly when the various HPD bits are enabled / disabled and it feels like it need to be made super obvious / well documented. That being said, I'd be OK w/ that happening in the proposed cleanup. -Doug
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c index fac815f..07f2389 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.c +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c @@ -569,10 +569,6 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog *dp_catalog) u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER); - /* enable HPD plug and unplug interrupts */ - dp_catalog_hpd_config_intr(dp_catalog, - DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK, true); - /* Configure REFTIMER and enable it */ reftimer |= DP_DP_HPD_REFTIMER_ENABLE; dp_write_aux(catalog, REG_DP_DP_HPD_REFTIMER, reftimer); @@ -599,13 +595,14 @@ u32 dp_catalog_hpd_get_intr_status(struct dp_catalog *dp_catalog) { struct dp_catalog_private *catalog = container_of(dp_catalog, struct dp_catalog_private, dp_catalog); - int isr = 0; + int isr, mask; isr = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS); dp_write_aux(catalog, REG_DP_DP_HPD_INT_ACK, (isr & DP_DP_HPD_INT_MASK)); + mask = dp_read_aux(catalog, REG_DP_DP_HPD_INT_MASK); - return isr; + return isr & (mask | ~DP_DP_HPD_INT_MASK); } int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 43c59cb..1339c45 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -683,7 +683,8 @@ 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 */ - dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true); + if (!dp->dp_display.is_edp) + dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true); DRM_DEBUG_DP("After, type=%d hpd_state=%d\n", dp->dp_display.connector_type, state); @@ -1096,6 +1097,13 @@ 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) + dp_catalog_hpd_config_intr(dp->catalog, + DP_DP_HPD_PLUG_INT_MASK | + DP_DP_HPD_UNPLUG_INT_MASK, + true); + /* Enable interrupt first time * we are leaving dp clocks on during disconnect * and never disable interrupt @@ -1381,6 +1389,12 @@ static int dp_pm_resume(struct device *dev) dp_catalog_ctrl_hpd_config(dp->catalog); + if (!dp->dp_display.is_edp) + dp_catalog_hpd_config_intr(dp->catalog, + DP_DP_HPD_PLUG_INT_MASK | + DP_DP_HPD_UNPLUG_INT_MASK, + true); + if (dp_catalog_link_is_connected(dp->catalog)) { /* * set sink to normal operation mode -- D0 @@ -1654,6 +1668,9 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge) return; } + if (dp->is_edp) + dp_hpd_plug_handle(dp_display, 0); + mutex_lock(&dp_display->event_mutex); /* stop sentinel checking */ @@ -1718,6 +1735,9 @@ void dp_bridge_post_disable(struct drm_bridge *drm_bridge) dp_display = container_of(dp, struct dp_display_private, dp_display); + if (dp->is_edp) + dp_hpd_unplug_handle(dp_display, 0); + mutex_lock(&dp_display->event_mutex); /* stop sentinel checking */
The panel-edp enables the eDP panel power during probe, get_modes and enable. The eDP connect and disconnect interrupts for the eDP/DP controller are directly dependent on panel power. As eDP display can be assumed as always connected, the controller driver can skip the eDP connect and disconnect interrupts. Any disruption in the link status will be indicated via the IRQ_HPD interrupts. So, the eDP controller driver can just enable the IRQ_HPD and replug interrupts. The DP controller driver still needs to enable all the interrupts. Signed-off-by: Sankeerth Billakanti <quic_sbillaka@quicinc.com> --- Changes in v7: - reordered the patch in the series - modified the return statement for isr - connector check modified to just check for eDP drivers/gpu/drm/msm/dp/dp_catalog.c | 9 +++------ drivers/gpu/drm/msm/dp/dp_display.c | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 7 deletions(-)