From: Doug Anderson <dianders@chromium.org> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: Sankeerth Billakanti <quic_sbillaka@quicinc.com>, dri-devel <dri-devel@lists.freedesktop.org>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, freedreno <freedreno@lists.freedesktop.org>, LKML <linux-kernel@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, Rob Clark <robdclark@gmail.com>, Sean Paul <seanpaul@chromium.org>, Stephen Boyd <swboyd@chromium.org>, quic_kalyant <quic_kalyant@quicinc.com>, "Abhinav Kumar (QUIC)" <quic_abhinavk@quicinc.com>, "Kuogee Hsieh (QUIC)" <quic_khsieh@quicinc.com>, Bjorn Andersson <bjorn.andersson@linaro.org>, Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, quic_vproddut <quic_vproddut@quicinc.com>, quic_aravindh@quicinc.com Subject: Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus Date: Sat, 2 Apr 2022 10:06:24 -0700 [thread overview] Message-ID: <CAD=FV=W4PYK-t607yjRbfjDjjEZX0KdgHDRukw_vSH8E8EDH6w@mail.gmail.com> (raw) In-Reply-To: <392b933f-760c-3c81-1040-c514045df3da@linaro.org> Hi, On Sat, Apr 2, 2022 at 3:37 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On 01/04/2022 02:22, Doug Anderson wrote: > > Hi, > > > > On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti > > <quic_sbillaka@quicinc.com> wrote: > >> > >> @@ -1547,6 +1593,10 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, > >> > >> dp_display->encoder = encoder; > >> > >> + ret = dp_display_get_next_bridge(dp_display); > >> + if (ret) > >> + return ret; > > > > It feels weird to me that this is in a function called "modeset_init", > > though I certainly don't know the structure of the MSM display code > > well enough to fully comment. > > It's called modeset_init() as it initializes KMS objects used by DP > driver. We have similar functions for dsi and hdmi Sorry, I wasn't meaning to imply that modeset_init() was a bad name or anything. Mostly saying that I wasn't sure that modeset init was the proper time to populate the aux bus. ...but then again, perhaps it is given the current structure of this driver? > > My expectation would have been that > > devm_of_dp_aux_populate_ep_devices() would have been called from your > > probe routine and then you would have returned -EPROBE_DEFER from your > > probe if you were unable to find the panel afterwards. > > I don't think it's possible to call it from probe() since > drm_dp_aux_register() is called only from dp_display_bind(). > The PHY also isn't initialized at that moment, so we can not probe AUX > devices. > > The overall semantics of the AUX bus is not clear to me. > Typically the bus is populated (and probed) when devices are accessible. > But for the display related buses this might not be the case. In general the AUX bus is modeled much like the i2c bus. You probe the sub-device when you're able to transfer. Then you can confirm that the device is actually there and init the device. > For example for the DSI bus we clearly define that DSI transfer are not > possible before the corresponding bridge's (or panel's) enable call. > > Maybe the same approach should be adopted for the AUX bus. This would > allow us to populate the AUX bus before hardware access is actually > possible, thus creating all the DRM bridges before the hardware is > actually up and running. So I guess what you're proposing is that you could probe the devices under the AUX bus and they could acquire resources (and possibly return EPROBE_DEFER) at a point in time _before_ it's actually possible to transfer. Then I guess you'd later do the transfer? I guess conceivably one could re-design the DRM subsystem like that, but I don't think it's trivial. Why? I believe that we need to know things about the panel at probe time. For instance, we need to be able to populate the panel's modes. To get this information we need the EDID which means we need to be able to do a transfer. If we're using an AUX backlight we also need to add info about the backlight at probe time and that also needs the transfer to work. So I guess the net result is maybe we should just keep it where it is. Long term I'd be interested in knowing if there's a reason why we can't structure the driver so that AUX transfers can happen with less intertwining with the rest of the code, but that can happen later. I would expect that you'd basically just need clocks and regulators on and maybe your PHY on. Ideally with some pm_runtime fun we should be able to do that independently with anything else the driver needs to do? -Doug
WARNING: multiple messages have this Message-ID (diff)
From: Doug Anderson <dianders@chromium.org> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: quic_kalyant <quic_kalyant@quicinc.com>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, Sankeerth Billakanti <quic_sbillaka@quicinc.com>, "Abhinav Kumar \(QUIC\)" <quic_abhinavk@quicinc.com>, quic_vproddut <quic_vproddut@quicinc.com>, David Airlie <airlied@linux.ie>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, LKML <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Stephen Boyd <swboyd@chromium.org>, Sean Paul <sean@poorly.run>, Sean Paul <seanpaul@chromium.org>, quic_aravindh@quicinc.com, "Kuogee Hsieh \(QUIC\)" <quic_khsieh@quicinc.com>, freedreno <freedreno@lists.freedesktop.org> Subject: Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus Date: Sat, 2 Apr 2022 10:06:24 -0700 [thread overview] Message-ID: <CAD=FV=W4PYK-t607yjRbfjDjjEZX0KdgHDRukw_vSH8E8EDH6w@mail.gmail.com> (raw) In-Reply-To: <392b933f-760c-3c81-1040-c514045df3da@linaro.org> Hi, On Sat, Apr 2, 2022 at 3:37 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On 01/04/2022 02:22, Doug Anderson wrote: > > Hi, > > > > On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti > > <quic_sbillaka@quicinc.com> wrote: > >> > >> @@ -1547,6 +1593,10 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, > >> > >> dp_display->encoder = encoder; > >> > >> + ret = dp_display_get_next_bridge(dp_display); > >> + if (ret) > >> + return ret; > > > > It feels weird to me that this is in a function called "modeset_init", > > though I certainly don't know the structure of the MSM display code > > well enough to fully comment. > > It's called modeset_init() as it initializes KMS objects used by DP > driver. We have similar functions for dsi and hdmi Sorry, I wasn't meaning to imply that modeset_init() was a bad name or anything. Mostly saying that I wasn't sure that modeset init was the proper time to populate the aux bus. ...but then again, perhaps it is given the current structure of this driver? > > My expectation would have been that > > devm_of_dp_aux_populate_ep_devices() would have been called from your > > probe routine and then you would have returned -EPROBE_DEFER from your > > probe if you were unable to find the panel afterwards. > > I don't think it's possible to call it from probe() since > drm_dp_aux_register() is called only from dp_display_bind(). > The PHY also isn't initialized at that moment, so we can not probe AUX > devices. > > The overall semantics of the AUX bus is not clear to me. > Typically the bus is populated (and probed) when devices are accessible. > But for the display related buses this might not be the case. In general the AUX bus is modeled much like the i2c bus. You probe the sub-device when you're able to transfer. Then you can confirm that the device is actually there and init the device. > For example for the DSI bus we clearly define that DSI transfer are not > possible before the corresponding bridge's (or panel's) enable call. > > Maybe the same approach should be adopted for the AUX bus. This would > allow us to populate the AUX bus before hardware access is actually > possible, thus creating all the DRM bridges before the hardware is > actually up and running. So I guess what you're proposing is that you could probe the devices under the AUX bus and they could acquire resources (and possibly return EPROBE_DEFER) at a point in time _before_ it's actually possible to transfer. Then I guess you'd later do the transfer? I guess conceivably one could re-design the DRM subsystem like that, but I don't think it's trivial. Why? I believe that we need to know things about the panel at probe time. For instance, we need to be able to populate the panel's modes. To get this information we need the EDID which means we need to be able to do a transfer. If we're using an AUX backlight we also need to add info about the backlight at probe time and that also needs the transfer to work. So I guess the net result is maybe we should just keep it where it is. Long term I'd be interested in knowing if there's a reason why we can't structure the driver so that AUX transfers can happen with less intertwining with the rest of the code, but that can happen later. I would expect that you'd basically just need clocks and regulators on and maybe your PHY on. Ideally with some pm_runtime fun we should be able to do that independently with anything else the driver needs to do? -Doug
next prev parent reply other threads:[~2022-04-02 17:06 UTC|newest] Thread overview: 140+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-30 16:02 [PATCH v6 0/8] Add support for the eDP panel over aux_bus Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-30 16:02 ` [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-30 23:19 ` Dmitry Baryshkov 2022-03-30 23:19 ` Dmitry Baryshkov 2022-03-31 0:33 ` Doug Anderson 2022-03-31 0:33 ` Doug Anderson 2022-03-31 23:22 ` Doug Anderson 2022-03-31 23:22 ` Doug Anderson 2022-04-02 10:37 ` Dmitry Baryshkov 2022-04-02 10:37 ` Dmitry Baryshkov 2022-04-02 17:06 ` Doug Anderson [this message] 2022-04-02 17:06 ` Doug Anderson 2022-04-02 20:26 ` Dmitry Baryshkov 2022-04-02 20:26 ` Dmitry Baryshkov 2022-04-04 20:53 ` Doug Anderson 2022-04-04 20:53 ` Doug Anderson 2022-04-05 12:53 ` Dmitry Baryshkov 2022-04-05 12:53 ` Dmitry Baryshkov 2022-04-05 17:02 ` Doug Anderson 2022-04-05 17:02 ` Doug Anderson 2022-04-05 17:36 ` Dmitry Baryshkov 2022-04-05 17:36 ` Dmitry Baryshkov 2022-04-05 18:11 ` Doug Anderson 2022-04-05 18:11 ` Doug Anderson 2022-04-07 14:19 ` Sankeerth Billakanti (QUIC) 2022-04-07 14:19 ` Sankeerth Billakanti (QUIC) 2022-04-07 17:07 ` Doug Anderson 2022-04-07 17:07 ` Doug Anderson 2022-04-07 20:11 ` Abhinav Kumar 2022-04-07 20:11 ` Abhinav Kumar 2022-04-07 20:47 ` Doug Anderson 2022-04-07 20:47 ` Doug Anderson 2022-04-07 22:03 ` Abhinav Kumar 2022-04-07 22:03 ` Abhinav Kumar 2022-04-07 23:34 ` Doug Anderson 2022-04-07 23:34 ` Doug Anderson 2022-04-07 23:46 ` Dmitry Baryshkov 2022-04-07 23:46 ` Dmitry Baryshkov 2022-04-08 0:21 ` Doug Anderson 2022-04-08 0:21 ` Doug Anderson 2022-04-08 12:19 ` Dmitry Baryshkov 2022-04-08 12:19 ` Dmitry Baryshkov 2022-04-08 13:43 ` Doug Anderson 2022-04-08 13:43 ` Doug Anderson 2022-04-08 14:58 ` Dmitry Baryshkov 2022-04-08 14:58 ` Dmitry Baryshkov 2022-04-08 17:23 ` Abhinav Kumar 2022-04-08 17:23 ` Abhinav Kumar 2022-04-07 23:35 ` Dmitry Baryshkov 2022-04-07 23:35 ` Dmitry Baryshkov 2022-04-08 0:20 ` Doug Anderson 2022-04-08 0:20 ` Doug Anderson 2022-04-08 12:13 ` Dmitry Baryshkov 2022-04-08 12:13 ` Dmitry Baryshkov 2022-04-08 13:56 ` Doug Anderson 2022-04-08 13:56 ` Doug Anderson 2022-04-08 14:17 ` Dmitry Baryshkov 2022-04-08 14:17 ` Dmitry Baryshkov 2022-03-30 16:02 ` [PATCH v6 2/8] drm/msm/dp: wait for hpd high before aux transaction Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-31 23:22 ` Doug Anderson 2022-03-31 23:22 ` Doug Anderson 2022-04-04 12:43 ` Sankeerth Billakanti (QUIC) 2022-04-04 12:43 ` Sankeerth Billakanti (QUIC) 2022-03-30 16:02 ` [PATCH v6 3/8] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-31 23:22 ` Doug Anderson 2022-03-31 23:22 ` Doug Anderson 2022-04-04 12:56 ` Sankeerth Billakanti (QUIC) 2022-04-04 12:56 ` Sankeerth Billakanti (QUIC) 2022-03-30 16:02 ` [PATCH v6 4/8] drm/msm/dp: avoid handling masked interrupts Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-30 22:16 ` Dmitry Baryshkov 2022-03-30 22:16 ` Dmitry Baryshkov 2022-03-31 5:53 ` Sankeerth Billakanti (QUIC) 2022-03-31 5:53 ` Sankeerth Billakanti (QUIC) 2022-03-31 10:10 ` Dmitry Baryshkov 2022-03-31 10:10 ` Dmitry Baryshkov 2022-03-31 11:04 ` Sankeerth Billakanti 2022-03-31 11:04 ` Sankeerth Billakanti 2022-03-31 11:06 ` Dmitry Baryshkov 2022-03-31 11:06 ` Dmitry Baryshkov 2022-04-04 17:56 ` Sankeerth Billakanti (QUIC) 2022-04-04 17:56 ` Sankeerth Billakanti (QUIC) 2022-03-30 16:02 ` [PATCH v6 5/8] drm/msm/dp: prevent multiple votes for dp resources Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-31 23:23 ` Doug Anderson 2022-03-31 23:23 ` Doug Anderson 2022-04-08 16:14 ` Dmitry Baryshkov 2022-04-08 16:14 ` Dmitry Baryshkov 2022-04-08 17:12 ` Sankeerth Billakanti 2022-04-08 17:12 ` Sankeerth Billakanti 2022-04-08 18:02 ` Dmitry Baryshkov 2022-04-08 18:02 ` Dmitry Baryshkov 2022-03-30 16:02 ` [PATCH v6 6/8] drm/msm/dp: remove unnecessary delay during boot Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-31 23:23 ` Doug Anderson 2022-03-31 23:23 ` Doug Anderson 2022-04-04 13:52 ` Sankeerth Billakanti (QUIC) 2022-04-04 13:52 ` Sankeerth Billakanti (QUIC) 2022-04-04 21:13 ` Dmitry Baryshkov 2022-04-04 21:13 ` Dmitry Baryshkov 2022-04-07 12:40 ` Sankeerth Billakanti (QUIC) 2022-04-07 12:40 ` Sankeerth Billakanti (QUIC) 2022-03-30 16:02 ` [PATCH v6 7/8] drm/msm/dp: Support edp/dp without hpd Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-31 23:23 ` Doug Anderson 2022-03-31 23:23 ` Doug Anderson 2022-04-04 18:32 ` Sankeerth Billakanti (QUIC) 2022-04-04 18:32 ` Sankeerth Billakanti (QUIC) 2022-04-04 21:15 ` Dmitry Baryshkov 2022-04-04 21:15 ` Dmitry Baryshkov 2022-04-07 12:41 ` Sankeerth Billakanti (QUIC) 2022-04-07 12:41 ` Sankeerth Billakanti (QUIC) 2022-03-30 16:02 ` [PATCH v6 8/8] drm/msm/dp: Handle eDP mode_valid differently from dp Sankeerth Billakanti 2022-03-30 16:02 ` Sankeerth Billakanti 2022-03-30 22:08 ` Dmitry Baryshkov 2022-03-30 22:08 ` Dmitry Baryshkov 2022-03-31 6:02 ` Sankeerth Billakanti (QUIC) 2022-03-31 6:02 ` Sankeerth Billakanti (QUIC) 2022-03-31 23:24 ` Doug Anderson 2022-03-31 23:24 ` Doug Anderson 2022-04-04 18:20 ` Sankeerth Billakanti (QUIC) 2022-04-04 18:20 ` Sankeerth Billakanti (QUIC) 2022-04-04 21:29 ` Dmitry Baryshkov 2022-04-04 21:29 ` Dmitry Baryshkov 2022-04-07 14:05 ` Sankeerth Billakanti (QUIC) 2022-04-07 14:05 ` Sankeerth Billakanti (QUIC) 2022-04-08 12:08 ` Dmitry Baryshkov 2022-04-08 12:08 ` Dmitry Baryshkov 2022-04-08 15:50 ` Sankeerth Billakanti 2022-04-08 15:50 ` Sankeerth Billakanti 2022-04-08 16:47 ` Dmitry Baryshkov 2022-04-08 16:47 ` Dmitry Baryshkov 2022-04-08 17:38 ` Sankeerth Billakanti 2022-04-08 17:38 ` Sankeerth Billakanti 2022-04-08 18:06 ` Dmitry Baryshkov 2022-04-08 18:06 ` Dmitry Baryshkov
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=W4PYK-t607yjRbfjDjjEZX0KdgHDRukw_vSH8E8EDH6w@mail.gmail.com' \ --to=dianders@chromium.org \ --cc=airlied@linux.ie \ --cc=bjorn.andersson@linaro.org \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=dmitry.baryshkov@linaro.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=freedreno@lists.freedesktop.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=quic_abhinavk@quicinc.com \ --cc=quic_aravindh@quicinc.com \ --cc=quic_kalyant@quicinc.com \ --cc=quic_khsieh@quicinc.com \ --cc=quic_sbillaka@quicinc.com \ --cc=quic_vproddut@quicinc.com \ --cc=robdclark@gmail.com \ --cc=sean@poorly.run \ --cc=seanpaul@chromium.org \ --cc=swboyd@chromium.org \ /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.