From: Doug Anderson <dianders@chromium.org> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: dri-devel <dri-devel@lists.freedesktop.org>, Robert Foss <robert.foss@linaro.org>, Hsin-Yi Wang <hsinyi@chromium.org>, Abhinav Kumar <quic_abhinavk@quicinc.com>, Sankeerth Billakanti <quic_sbillaka@quicinc.com>, Philip Chen <philipchen@chromium.org>, Stephen Boyd <swboyd@chromium.org>, Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>, Linus Walleij <linus.walleij@linaro.org>, Lyude Paul <lyude@redhat.com>, Thomas Zimmermann <tzimmermann@suse.de>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly Date: Fri, 15 Apr 2022 14:13:12 -0700 [thread overview] Message-ID: <CAD=FV=WKwErpD7iCu+2jFvMutmmmgLUEhAnw8s=27wUxcpF-aQ@mail.gmail.com> (raw) In-Reply-To: <a9a5dfb7-819b-d3a2-2c47-d5b239d21ad3@linaro.org> Hi, On Thu, Apr 14, 2022 at 5:47 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On 09/04/2022 05:36, Douglas Anderson wrote: > > As talked about in the kerneldoc for "struct dp_aux_ep_client" in this > > patch and also in the past in commit a1e3667a9835 ("drm/bridge: > > ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), to use the > > DP AUX bus properly we really need two "struct device"s. One "struct > > device" is in charge of providing the DP AUX bus and the other is > > where we'll try to get a reference to the newly probed endpoint > > devices. > > > > In ti-sn65dsi86 this wasn't too difficult to accomplish. That driver > > is already broken up into several "struct devices" anyway because it > > also provides a PWM and some GPIOs. Adding one more wasn't that > > difficult / ugly. > > > > When I tried to do the same solution in parade-ps8640, it felt like I > > was copying too much boilerplate code. I made the realization that I > > didn't _really_ need a separate "driver" for each person that wanted > > to do the same thing. By putting all the "driver" related code in a > > common place then we could save a bit of hassle. This change > > effectively adds a new "ep_client" driver that can be used by > > anyone. The devices instantiated by this driver will just call through > > to the probe/remove/shutdown calls provided. > > > > At the moment, the "ep_client" driver is backed by the Linux auxiliary > > bus (unfortunate naming--this has nothing to do with DP AUX). I didn't > > want to expose this to clients, though, so as far as clients are > > concerned they get a vanilla "struct device". > > I have been thinking about your approach for quite some time. I think > that enforcing a use of auxilliary device is an overkill. What do we > really need is the the set callbacks in the bus struct or a notifier. We > have to notify the aux_bus controller side that the client has been > probed successfully or that the client is going to be removed. It seems like these new callbacks would be nearly the same as the probe/remove callbacks in my proposal except: * They rely on there being exactly 1 AUX device, or we make it a rule that we wait for all AUX devices to probe (?) * We need to come up with a system for detecting when everything probes or is going to be removed, though that's probably not too hard. I guess the DP AUX bus could just replace the panel's probe function with its own and essentially "tail patch" it. I guess it could "head patch" the remove call? ...or is there some better way you were thinking of knowing when all our children probed? * The callback on the aux bus controller side would not be able to DEFER. In other words trying to acquire a reference to the panel can always be the last thing we do so we know there can be no reasons to defer after. This should be doable, but at least in the ps8640 case it will require changing the code a bit. I notice that today it actually tries to get the panel side _before_ it gets the MIPI side and it potentially can return -EPROBE_DEFER if it can't find the MIPI side. I guess I have a niggling feeling that we'll find some reason in the future that we can't be last, but we can probably ignore that. ;-) I can switch this all to normal callbacks if that's what everyone wants, but it doesn't feel significantly cleaner to me and does seem to have some (small) downsides. > And this > approach would make driver's life easier, since e.g. the bus code can > pm_get the EP device before calling callbacks/notifiers and > pm_put_autosuspend it afterwards. Not sure about doing the pm calls on behalf of the EP device. What's the goal there?
WARNING: multiple messages have this Message-ID (diff)
From: Doug Anderson <dianders@chromium.org> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: Sankeerth Billakanti <quic_sbillaka@quicinc.com>, Philip Chen <philipchen@chromium.org>, Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@linux.ie>, LKML <linux-kernel@vger.kernel.org>, Abhinav Kumar <quic_abhinavk@quicinc.com>, Robert Foss <robert.foss@linaro.org>, Stephen Boyd <swboyd@chromium.org>, dri-devel <dri-devel@lists.freedesktop.org>, Hsin-Yi Wang <hsinyi@chromium.org> Subject: Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly Date: Fri, 15 Apr 2022 14:13:12 -0700 [thread overview] Message-ID: <CAD=FV=WKwErpD7iCu+2jFvMutmmmgLUEhAnw8s=27wUxcpF-aQ@mail.gmail.com> (raw) In-Reply-To: <a9a5dfb7-819b-d3a2-2c47-d5b239d21ad3@linaro.org> Hi, On Thu, Apr 14, 2022 at 5:47 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On 09/04/2022 05:36, Douglas Anderson wrote: > > As talked about in the kerneldoc for "struct dp_aux_ep_client" in this > > patch and also in the past in commit a1e3667a9835 ("drm/bridge: > > ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), to use the > > DP AUX bus properly we really need two "struct device"s. One "struct > > device" is in charge of providing the DP AUX bus and the other is > > where we'll try to get a reference to the newly probed endpoint > > devices. > > > > In ti-sn65dsi86 this wasn't too difficult to accomplish. That driver > > is already broken up into several "struct devices" anyway because it > > also provides a PWM and some GPIOs. Adding one more wasn't that > > difficult / ugly. > > > > When I tried to do the same solution in parade-ps8640, it felt like I > > was copying too much boilerplate code. I made the realization that I > > didn't _really_ need a separate "driver" for each person that wanted > > to do the same thing. By putting all the "driver" related code in a > > common place then we could save a bit of hassle. This change > > effectively adds a new "ep_client" driver that can be used by > > anyone. The devices instantiated by this driver will just call through > > to the probe/remove/shutdown calls provided. > > > > At the moment, the "ep_client" driver is backed by the Linux auxiliary > > bus (unfortunate naming--this has nothing to do with DP AUX). I didn't > > want to expose this to clients, though, so as far as clients are > > concerned they get a vanilla "struct device". > > I have been thinking about your approach for quite some time. I think > that enforcing a use of auxilliary device is an overkill. What do we > really need is the the set callbacks in the bus struct or a notifier. We > have to notify the aux_bus controller side that the client has been > probed successfully or that the client is going to be removed. It seems like these new callbacks would be nearly the same as the probe/remove callbacks in my proposal except: * They rely on there being exactly 1 AUX device, or we make it a rule that we wait for all AUX devices to probe (?) * We need to come up with a system for detecting when everything probes or is going to be removed, though that's probably not too hard. I guess the DP AUX bus could just replace the panel's probe function with its own and essentially "tail patch" it. I guess it could "head patch" the remove call? ...or is there some better way you were thinking of knowing when all our children probed? * The callback on the aux bus controller side would not be able to DEFER. In other words trying to acquire a reference to the panel can always be the last thing we do so we know there can be no reasons to defer after. This should be doable, but at least in the ps8640 case it will require changing the code a bit. I notice that today it actually tries to get the panel side _before_ it gets the MIPI side and it potentially can return -EPROBE_DEFER if it can't find the MIPI side. I guess I have a niggling feeling that we'll find some reason in the future that we can't be last, but we can probably ignore that. ;-) I can switch this all to normal callbacks if that's what everyone wants, but it doesn't feel significantly cleaner to me and does seem to have some (small) downsides. > And this > approach would make driver's life easier, since e.g. the bus code can > pm_get the EP device before calling callbacks/notifiers and > pm_put_autosuspend it afterwards. Not sure about doing the pm calls on behalf of the EP device. What's the goal there?
next prev parent reply other threads:[~2022-04-15 21:13 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-09 2:36 [RFC PATCH 0/6] drm/dp: Improvements for DP AUX channel Douglas Anderson 2022-04-09 2:36 ` Douglas Anderson 2022-04-09 2:36 ` [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly Douglas Anderson 2022-04-09 2:36 ` Douglas Anderson 2022-04-09 5:11 ` kernel test robot 2022-04-11 8:34 ` Jani Nikula 2022-04-11 8:34 ` Jani Nikula 2022-04-11 13:37 ` Doug Anderson 2022-04-11 13:37 ` Doug Anderson 2022-04-14 23:51 ` Stephen Boyd 2022-04-14 23:51 ` Stephen Boyd 2022-04-15 21:13 ` Doug Anderson 2022-04-15 21:13 ` Doug Anderson 2022-04-15 0:46 ` Dmitry Baryshkov 2022-04-15 0:46 ` Dmitry Baryshkov 2022-04-15 21:13 ` Doug Anderson [this message] 2022-04-15 21:13 ` Doug Anderson 2022-04-15 22:44 ` Dmitry Baryshkov 2022-04-15 22:44 ` Dmitry Baryshkov 2022-04-16 0:09 ` Doug Anderson 2022-04-16 0:09 ` Doug Anderson 2022-04-16 0:54 ` Dmitry Baryshkov 2022-04-16 0:54 ` Dmitry Baryshkov 2022-04-18 23:10 ` Doug Anderson 2022-04-18 23:10 ` Doug Anderson 2022-05-03 22:45 ` Doug Anderson 2022-05-03 22:45 ` Doug Anderson 2022-05-03 23:23 ` Doug Anderson 2022-05-03 23:23 ` Doug Anderson 2022-04-09 2:36 ` [RFC PATCH 2/6] drm/bridge: parade-ps8640: Break probe in two to handle DP AUX better Douglas Anderson 2022-04-09 2:36 ` Douglas Anderson 2022-04-09 10:27 ` kernel test robot 2022-04-09 2:36 ` [RFC PATCH 3/6] drm/dp: Add is_hpd_asserted() callback to struct drm_dp_aux Douglas Anderson 2022-04-09 2:36 ` Douglas Anderson 2022-04-15 0:48 ` Dmitry Baryshkov 2022-04-15 0:48 ` Dmitry Baryshkov 2022-04-09 2:36 ` [RFC PATCH 4/6] drm/panel-edp: Take advantage of is_hpd_asserted() in " Douglas Anderson 2022-04-09 2:36 ` Douglas Anderson 2022-04-15 0:51 ` Dmitry Baryshkov 2022-04-15 0:51 ` Dmitry Baryshkov 2022-04-15 21:17 ` Doug Anderson 2022-04-15 21:17 ` Doug Anderson 2022-04-15 22:11 ` Dmitry Baryshkov 2022-04-15 22:11 ` Dmitry Baryshkov 2022-04-16 0:12 ` Doug Anderson 2022-04-16 0:12 ` Doug Anderson 2022-04-16 0:14 ` Dmitry Baryshkov 2022-04-16 0:14 ` Dmitry Baryshkov 2022-04-18 17:18 ` Doug Anderson 2022-04-18 17:18 ` Doug Anderson 2022-04-09 2:36 ` [RFC PATCH 5/6] drm/panel: atna33xc20: " Douglas Anderson 2022-04-09 2:36 ` Douglas Anderson 2022-04-09 2:36 ` [RFC PATCH 6/6] drm/bridge: parade-ps8640: Provide " Douglas Anderson 2022-04-09 2:36 ` Douglas Anderson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAD=FV=WKwErpD7iCu+2jFvMutmmmgLUEhAnw8s=27wUxcpF-aQ@mail.gmail.com' \ --to=dianders@chromium.org \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dmitry.baryshkov@linaro.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=hsinyi@chromium.org \ --cc=linus.walleij@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lyude@redhat.com \ --cc=philipchen@chromium.org \ --cc=quic_abhinavk@quicinc.com \ --cc=quic_sbillaka@quicinc.com \ --cc=robert.foss@linaro.org \ --cc=swboyd@chromium.org \ --cc=tzimmermann@suse.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.