linux-kernel - Re: [PATCH v9 2/4] drm/msm/dp: Support only IRQ

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Apr 2022 17:29:33 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Sankeerth Billakanti <quic_sbillaka@...cinc.com>,
        devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     robdclark@...il.com, seanpaul@...omium.org,
        quic_kalyant@...cinc.com, quic_abhinavk@...cinc.com,
        dianders@...omium.org, quic_khsieh@...cinc.com,
        bjorn.andersson@...aro.org, sean@...rly.run, airlied@...ux.ie,
        daniel@...ll.ch, dmitry.baryshkov@...aro.org,
        quic_vproddut@...cinc.com, quic_aravindh@...cinc.com,
        steev@...i.org
Subject: Re: [PATCH v9 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG
 interrupts for eDP

Quoting Sankeerth Billakanti (2022-04-22 02:11:04)
> The panel-edp enables the eDP panel power during probe, get_modes
> and 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@...cinc.com>

The sprinkling of if conditions and manual driving of the DP plug/unplug
state machine is pretty convoluted. To make it better the driver needs
an overhaul. Anyway, it looks mostly fine to me except for this replug
interrupt question below. Otherwise

Reviewed-by: Stephen Boyd <swboyd@...omium.org>

>  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 055681a..dea4de9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -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);
> +

It seems like only the plug and unplug is enabled for DP here. Is replug
enabled for eDP when it shouldn't be?

>         /* Enable interrupt first time
>          * we are leaving dp clocks on during disconnect
>          * and never disable interrupt