On Mon, May 22, 2023 at 03:35:13PM -0700, Kuogee Hsieh wrote:
>
>
> > > > -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 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,
> > > > - true);
> > > > -
> > > > - /* Enable interrupt first time
> > > > - * we are leaving dp clocks on during disconnect
> > > > - * and never disable interrupt
> > > > - */
> > > > - enable_irq(dp->irq);
> > >
> > > ...we need dp->irq enabled for handling the other interrupts, otherwise
> > > e.g. AUX transfers will time out.
> > >
> > > I added enable_irq(dp_priv->irq) to the EV_HPD_INIT_SETUP case below,
> > > just for testing, and with that the patch seems to be working fine.
> > >
> > >
> > > Is there any reason why we need to delay its enablement to after we
> > > unmask the HPD interrupts?
>
> my though is aux transaction should happen after plugin interrupt detected.
>
I have a next_bridge which implements DRM_BRIDGE_OP_HPD, in which case
dp_bridge_hpd_enable() will never be called (the next_bridge's
hpd_enable will be called instead).
> can you please let me know what did you do to trigger aux timeout?
>
In this setup I just connect the cable, wait a few seconds and the
transfers fails - as there's no interrupts signalling them being
completed.
> It does not happen on me at my test.
>
Given that you have the HPD signal on a GPIO, you should be able to
rewrite your DTS in line with:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sa8295p-adp.dts#n28
And pinmux the HPD signal as GPIO instead.
I believe this would allow you to test both code paths - without the
actual TCPM known to Linux.
> >
> > As I wrote, I'd probably prefer to see enable_irq() and disable_irq()
> > calls gone.
>
> ok, i will move enable_irq() out of here.
>
Thanks,
Bjorn
>
> > >
> > > Regards,
> > > Bjorn
> > >
> >
