Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1683750665-8764-1-git-send-email-quic_khsieh@quicinc.com>
 <1683750665-8764-2-git-send-email-quic_khsieh@quicinc.com>
 <CAA8EJprtQF0x_LCOTrt5bvRnJ+xRz6QxLF6QAP-4Pff6V5TJ2g@mail.gmail.com>
 <20230511155331.2jmfe7xcs5tihdgb@ripper> <5ef83699-01de-d062-6239-9bb834c70458@linaro.org>
 <8ac0b5f5-27da-2b28-8f10-b2fca447511a@quicinc.com> <CAA8EJpokAoGni7vNwuijs7EvmjCweO3pgChij3Qx3OUkVTVpiQ@mail.gmail.com>
 <CAE-0n51SHQhUJiY=BJp8CQQ1aTAOxeMDr2+NX_vpmjN_cyJJrA@mail.gmail.com>
In-Reply-To: <CAE-0n51SHQhUJiY=BJp8CQQ1aTAOxeMDr2+NX_vpmjN_cyJJrA@mail.gmail.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Fri, 12 May 2023 21:30:13 +0300
Message-ID: <CAA8EJprynCeTdYk5PH2aKcjoZZa5kh_iJRpXUua2jqhtMD5B=Q@mail.gmail.com>
Subject: Re: [PATCH v1 1/2] drm/msm/dp: enable HDP plugin/unplugged interrupts
 to hpd_enable/disable
To:     Stephen Boyd <swboyd@chromium.org>
Cc:     Kuogee Hsieh <quic_khsieh@quicinc.com>,
        Bjorn Andersson <andersson@kernel.org>,
        dri-devel@lists.freedesktop.org, robdclark@gmail.com,
        sean@poorly.run, dianders@chromium.org, vkoul@kernel.org,
        daniel@ffwll.ch, airlied@gmail.com, agross@kernel.org,
        quic_abhinavk@quicinc.com, quic_jesszhan@quicinc.com,
        quic_sbillaka@quicinc.com, marijn.suijten@somainline.org,
        freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 12 May 2023 at 21:03, Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Dmitry Baryshkov (2023-05-11 17:54:19)
> > On Fri, 12 May 2023 at 03:16, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> > > 1) DP with GPIO: No downstream drm_bridge are connected, is_edp = false
> > > and internal HPD-logic is in used (internal_hpd = true). Power needs to
> > > be on at all times etc.
> > >
> > > 2) DP without GPIO: Downstream drm_bridge connected, is_edp = false and
> > > internal HPD-logic should not be used/enabled (internal_hpd = false).
> > > Power doesn't need to be on unless hpd_notify is invoked to tell us that
> > > there's something connected...
> > >
> > > - dp_bridge_hpd_notify(). What is the point of this check? <== i have
> > > below two questions,
> > >
> > > 1) can you explain when/what this dp_bridge_hpd_notify() will be called?
> >
> > The call chain is drm_bridge_hpd_notify() ->
> > drm_bridge_connector_hpd_notify() -> .hpd_notify() for all drm_bridge
> > in chain
> >
> > One should add a call to drm_bridge_hpd_notify() when the hotplug
> > event has been detected.
> >
> > Also please note the patch https://patchwork.freedesktop.org/patch/484432/
> >
> > >
> > > 2) is dp_bridge_hpd_notify() only will be called at above case #2? and
> > > it will not be used by case #1?
> >
> > Once the driver calls drm_bridge_hpd_notify() in the hpd path, the
> > hpd_notify callbacks will be called in case#1 too.
> >
> > BTW: I don't see drm_bridge_hpd_notify() or
> > drm_kms_{,connector_}_hotplug_event() HPD notifications in the DP
> > driver at all. This should be fixed.
> >
>
> Is dp_bridge_hpd_notify() being called by
> drm_helper_probe_single_connector_modes() when the connectors are
> detected?
>
> I see drm_helper_probe_detect() calls connector->funcs->detect() which I
> think calls
> drm_bridge_connector_funcs::drm_bridge_connector_hpd_notify() but I
> haven't confirmed yet. The 'detect' bridge is the DP bridge in msm
> driver
>
>          if (!dp_display->is_edp) {
>                 bridge->ops =
>                         DRM_BRIDGE_OP_DETECT |
>
> so if the bridge_connector is being used then I think when fill_modes()
> is called we'll run the detect cycle and call the hpd_notify callbacks
> on the bridge chain.

Yes. This call chain is correct.
drm_helper_probe_single_connector_modes() ->
drm_bridge_connector_detect() -> drm_bridge_connector_hpd_notify().

However on HPD events the DP driver doesn't call into the drm core
(which I believe should be fixed).

-- 
With best wishes
Dmitry