linux-kernel - RE: [PATCH v6 3/8] 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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 4 Apr 2022 12:56:32 +0000
From:   "Sankeerth Billakanti (QUIC)" <quic_sbillaka@...cinc.com>
To:     Doug Anderson <dianders@...omium.org>,
        "Sankeerth Billakanti (QUIC)" <quic_sbillaka@...cinc.com>
CC:     dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        freedreno <freedreno@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Rob Clark <robdclark@...il.com>,
        Sean Paul <seanpaul@...omium.org>,
        Stephen Boyd <swboyd@...omium.org>,
        quic_kalyant <quic_kalyant@...cinc.com>,
        "Abhinav Kumar (QUIC)" <quic_abhinavk@...cinc.com>,
        "Kuogee Hsieh (QUIC)" <quic_khsieh@...cinc.com>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        "dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>,
        quic_vproddut <quic_vproddut@...cinc.com>,
        "Aravind Venkateswaran (QUIC)" <quic_aravindh@...cinc.com>
Subject: RE: [PATCH v6 3/8] drm/msm/dp: Support only IRQ_HPD and REPLUG
 interrupts for eDP

Hi Doug,

> On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti
> <quic_sbillaka@...cinc.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