linux-kernel - Re: [PATCH v3] drm/msm/dp: enable HDP plugin/unplugged interrupts at hpd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230522224751.5tf4oifp5z5k45pf@ripper>
Date:   Mon, 22 May 2023 15:47:51 -0700
From:   Bjorn Andersson <andersson@...nel.org>
To:     Kuogee Hsieh <quic_khsieh@...cinc.com>
Cc:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        robdclark@...il.com, sean@...rly.run, swboyd@...omium.org,
        dianders@...omium.org, vkoul@...nel.org, daniel@...ll.ch,
        airlied@...il.com, agross@...nel.org,
        marijn.suijten@...ainline.org, quic_abhinavk@...cinc.com,
        quic_jesszhan@...cinc.com, quic_sbillaka@...cinc.com,
        freedreno@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] drm/msm/dp: enable HDP plugin/unplugged interrupts at
 hpd_enable/disable

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
> > > 
> >