Messages in this thread | ![/](/images/icornerl.gif) | | From | "Sankeerth Billakanti (QUIC)" <> | Subject | RE: [PATCH v6 3/8] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP | Date | Mon, 4 Apr 2022 12:56:32 +0000 |
| |
Hi Doug,
> On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti > <quic_sbillaka@quicinc.com> wrote: > > > > @@ -1374,6 +1382,12 @@ static int dp_pm_resume(struct device *dev) > > dp_catalog_ctrl_hpd_config(dp->catalog); > > > > > > + if (dp->dp_display.connector_type == > DRM_MODE_CONNECTOR_DisplayPort) > > + dp_catalog_hpd_config_intr(dp->catalog, > > + DP_DP_HPD_PLUG_INT_MASK | > > + DP_DP_HPD_UNPLUG_INT_MASK, > > + true); > > + > > nit: why are there two blank lines above? > >
Will remove a blank line.
> > @@ -1639,6 +1653,9 @@ void dp_bridge_enable(struct drm_bridge > *drm_bridge) > > return; > > } > > > > + if (dp->connector_type == DRM_MODE_CONNECTOR_eDP) > > + dp_hpd_plug_handle(dp_display, 0); > > + > > Should you add a "pre_enable" and do it there? That would make it more > symmetric with the fact that you have the countertpart in "post_disable". >
We want to handle the screen off or eDP disable like a cable unplug so that it can be integrated into the dp code. So, we can move plug_handle into pre_enable and the unplug_handle to pre_disable.
If we do unplug in post_disable, we need to handle the state changes also. It will complicate the driver code.
> > Overall: I'm probably not familiar enough with this code to give it a full > review. I'm hoping that Dmitry knows it well enough... ;-) > > > -Doug
Thank you, Sankeerth
| ![\](/images/icornerr.gif) |