Message-ID: <CAD=FV=V=a1CnT8fqTJR40WoS3BaDQ3xZ=HnHVHqZh=MEmVUZBA@mail.gmail.com>
Date:   Tue, 5 Apr 2022 10:02:42 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     Sankeerth Billakanti <quic_sbillaka@...cinc.com>,
        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 <bjorn.andersson@...aro.org>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        quic_vproddut <quic_vproddut@...cinc.com>,
        "Aravind Venkateswaran (QUIC)" <quic_aravindh@...cinc.com>
Subject: Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus

Hi,

On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov
<dmitry.baryshkov@...aro.org> wrote:
> > 3. For DP and eDP HPD means something a little different. Essentially
> > there are two concepts: a) is a display physically connected and b) is
> > the display powered up and ready. For DP, the two are really tied
> > together. From the kernel's point of view you never "power down" a DP
> > display and you can't detect that it's physically connected until it's
> > ready. Said another way, on you tie "is a display there" to the HPD
> > line and the moment a display is there it's ready for you to do AUX
> > transfers. For eDP, in the lowest power state of a display it _won't_
> > assert its "HPD" signal. However, it's still physically present. For
> > eDP you simply have to _assume_ it's present without any actual proof
> > since you can't get proof until you power it up. Thus for eDP, you
> > report that the display is there as soon as we're asked. We can't
> > _talk_ to the display yet, though. So in get_modes() we need to be
> > able to power the display on enough to talk over the AUX channel to
> > it. As part of this, we wait for the signal named "HPD" which really
> > means "panel finished powering on" in this context.
> >
> > NOTE: for aux transfer, we don't have the _display_ pipe and clocks
> > running. We only have enough stuff running to do the AUX transfer.
> > We're not clocking out pixels. We haven't fully powered on the
> > display. The AUX transfer is designed to be something that can be done
> > early _before_ you turn on the display.
> >
> >
> > OK, so basically that was a longwinded way of saying: yes, we could
> > avoid the AUX transfer in probe, but we can't wait all the way to
> > enable. We have to be able to transfer in get_modes(). If you think
> > that's helpful I think it'd be a pretty easy patch to write even if it
> > would look a tad bit awkward IMO. Let me know if you want me to post
> > it up.
>
> I think it would be a good idea. At least it will allow us to judge,
> which is the more correct way.

I'm still happy to prototype this, but the more I think about it the
more it feels like a workaround for the Qualcomm driver. The eDP panel
driver is actually given a pointer to the AUX bus at probe time. It's
really weird to say that we can't do a transfer on it yet... As you
said, this is a little sideband bus. It should be able to be used
without all the full blown infra of the rest of the driver.


> And I also think it might help the ti,sn65dsi86 driver, as it won't
> have to ensure that gpio is available during the AUX bus probe.

The ti,sn65dsi86 GPIO issue has been solved for a while, though so not
sure why we need to do something there? I'm also unclear how it would
have helped. In this discussion, we've agreed that the panel driver
would still acquire resources during its probe time and the only thing
that would be delayed would be the first AUX transfer. The GPIO is a
resource here and it's ideal to acquire it at probe time so we could
EPROBE_DEFER if needed.


> BTW, another random idea, before you start coding.
>
> We have the bridge's hpd_notify call. Currently it is called only by
> the means of drm_bridge_connector's HPD mechanism, tied to the bridge
> registering as DRM_BRIDGE_OP_HPD.
> It looks to me like it might be a perfect fit for the first aux-bus
> related reads.
>
> We'd need to trigger it manually once and tie it to the new
> drm_panel_funcs callback, which in turn would probe the aux bus,
> create backlight, etc.
>
> Regarding the Sankeerth's patch. I have been comparing it with the
> hpd_event_thread()'s calls.
> It looks to me like we should reuse dp_display_config_hpd()
> /EV_HPD_INIT_SETUP and maybe others.
>
> What I'm trying to say is that if we split AUX probing and first AUX
> transfers, it would be possible to reuse a significant part of MSM DP
> HPD machine rather than hacking around it and replicating it manually.

I'm not sure I completely understand, but I'm pretty wary here. It's
my assertion that all of the current "HPD" infrastructure in DRM all
relates to the physical presence of the panel. If you start
implementing these functions for eDP I think you're going to confuse
the heck out of everything. The kernel will think that this is a
display that's sometimes not there. Whenever the display is powered
off then HPD will be low and it will look like there's no display.
Nothing will ever try to power it on because it looks like there's no
display.

I think your idea is to "trigger once" at bootup and then it all
magically works, right? ...but what about after bootup? If you turn
the display off for whatever reason (modeset or you simply close the
lid of your laptop because you're using an external display) and then
you want to use the eDP display again, how do you kickstart the
process another time? You can't reboot, and when the display is off
the HPD line is low.

I can't say it enough times, HPD on eDP _does not mean hot plug
detect_. The panel is always there. HPD is really a "panel ready /
panel notify" signal for eDP. That's fully what its function is.

-Doug

Powered by blists - more mailing lists