Date:   Tue, 1 Aug 2023 07:49:14 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Benjamin Tissoires <bentiss@...nel.org>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Sam Ravnborg <sam@...nborg.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        linux-arm-msm@...r.kernel.org,
        yangcong5@...qin.corp-partner.google.com,
        devicetree@...r.kernel.org, Daniel Vetter <daniel@...ll.ch>,
        hsinyi@...gle.com, Chris Morgan <macroalpha82@...il.com>,
        linux-input@...r.kernel.org, cros-qcom-dts-watchers@...omium.org,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v4 00/11] drm/panel and i2c-hid: Allow panels and
 touchscreens to power sequence together

Hi,

On Fri, Jul 28, 2023 at 10:24 AM Doug Anderson <dianders@...omium.org> wrote:
>
> Hi,
>
> On Fri, Jul 28, 2023 at 8:31 AM Benjamin Tissoires <bentiss@...nel.org> wrote:
> >
> > On Jul 27 2023, Douglas Anderson wrote:
> > >
> > > The big motivation for this patch series is mostly described in the patch
> > > ("drm/panel: Add a way for other devices to follow panel state"), but to
> > > quickly summarize here: for touchscreens that are connected to a panel we
> > > need the ability to power sequence the two device together. This is not a
> > > new need, but so far we've managed to get by through a combination of
> > > inefficiency, added costs, or perhaps just a little bit of brokenness.
> > > It's time to do better. This patch series allows us to do better.
> > >
> > > Assuming that people think this patch series looks OK, we'll have to
> > > figure out the right way to land it. The panel patches and i2c-hid
> > > patches will go through very different trees and so either we'll need
> > > an Ack from one side or the other or someone to create a tag for the
> > > other tree to pull in. This will _probably_ require the true drm-misc
> > > maintainers to get involved, not a lowly committer. ;-)
> > >
> > > Version 4 of this series adds a new patch that suspends i2c-hid
> > > devices at remove time even for non panel-followers to make things
> > > consistent. It also attempts to isolate the panel follower code a bit
> > > more as per Benjamin's feedback on v3 and adds an item to the DRM todo
> > > list as per Maxime's request. As per Maxime's response to my v3 cover
> > > letter, I added his Reviewed-by tag to all 10 patches that were part
> > > of v3 (but left it off of the new i2c-hid patch in v4).
> > >
> > > Version 3 of this series was a long time coming after v2. Maxime and I
> > > had a very long discussion trying to figure out if there was a beter
> > > way and in the end we didn't find one so he was OK with the series in
> > > general [1]. After that got resolved, I tried to resolve Benjamin's
> > > feedback but got stuck [2]. Eventually I made my best guess. The end
> > > result was a v3 that wasn't that different from v2 but that had a tiny
> > > bit more code split out.
> > >
> > > Version 2 of this patch series didn't change too much. At a high level:
> > > * I added all the forgotten "static" to functions.
> > > * I've hopefully made the bindings better.
> > > * I've integrated into fw_devlink.
> > > * I cleaned up a few descriptions / comments.
> > >
> > > As far as I can tell, as of v4 everyone is on the same page that this
> > > patch series looks like a reasonable solution to the problem and we
> > > just need to get all the nits fixed and figure out how to land it.
> >
> > Thanks a lot for the new version. I like it much more on the HID side:
> >
> > for the HID part:
> > Reviewed-by: Benjamin Tissoires <bentiss@...nel.org>
> >
> > I wouldn't mind having this series taken from the drm tree if that is
> > easier. i2c-hid is a low patch rate driver, so having it updated through
> > DRM should not be an issue.
> >
> > In that case:
> > Acked-by: Benjamin Tissoires <bentiss@...nel.org>
>
> Thanks for your reviews and your help getting this whipped into shape.
>
> Lading through drm makes sense to me. I'm a drm committer, so with
> your Ack I believe it should be fine for me to land the series (minus
> the dts) in drm-misc-next. This series has been around for a while,
> has been reviewed by relevant folks, and the last few changes haven't
> fundamentally changed anything about the design, so I'm not going to
> twiddle my thumbs too long. That being said, I'll still plan to wait
> until early next week (Tuesday?) before landing to allow for any last
> minute shouts.
>
> Given how drm-misc works [1] and the fact that mainline is currently
> at v6.5-rc3 (it will be -rc4 when I land it), I'd expect that these
> commits will find their way into v6.6.
>
> [1] https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html

Pushed the first 10 patches to drm-misc-next. Bjorn: whenever it's
convenient you could land patch #11 (the device tree change) into the
Qualcomm tree.

76edfcf430cc HID: i2c-hid: Do panel follower work on the system_wq
96a37bfd232a HID: i2c-hid: Support being a panel follower
5f8838e9405d HID: i2c-hid: Suspend i2c-hid devices in remove
d93d28477222 HID: i2c-hid: Make suspend and resume into helper functions
675cd877c952 HID: i2c-hid: Rearrange probe() to power things up later
a889ee12d53d HID: i2c-hid: Switch to SYSTEM_SLEEP_PM_OPS()
fbf0ea2da3c7 of: property: fw_devlink: Add a devlink for panel followers
de0874165b83 drm/panel: Add a way for other devices to follow panel state
d2aacaf07395 drm/panel: Check for already prepared/enabled in drm_panel
2ca376ef18f6 dt-bindings: HID: i2c-hid: Add "panel" property to
i2c-hid backed touchscreens

-Doug

Powered by blists - more mailing lists