From: Philippe CORNU <philippe.cornu@st.com> To: Archit Taneja <architt@codeaurora.org>, Benjamin Gaignard <benjamin.gaignard@linaro.org>, Eric Anholt <eric@anholt.net> Cc: "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, Andrzej Hajda <a.hajda@samsung.com>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, Thierry Reding <thierry.reding@gmail.com>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Boris Brezillon <boris.brezillon@free-electrons.com> Subject: Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge. Date: Thu, 22 Jun 2017 09:31:18 +0000 [thread overview] Message-ID: <479892b4-909b-e2c0-025d-844f75d72899@st.com> (raw) In-Reply-To: <dd33044e-46d6-6f33-1011-b0bc343ddc96@codeaurora.org> On 06/22/2017 10:17 AM, Archit Taneja wrote: > > > On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: >> 2017-06-20 19:31 GMT+02:00 Eric Anholt <eric@anholt.net>: >>> Archit Taneja <architt@codeaurora.org> writes: >>> >>>> On 06/16/2017 08:13 PM, Eric Anholt wrote: >>>>> Archit Taneja <architt@codeaurora.org> writes: >>>>> >>>>>> On 06/16/2017 02:11 AM, Eric Anholt wrote: >>>>>>> If the panel-bridge is being set up after the drm_mode_config_reset(), >>>>>>> then the connector's state would never get initialized, and we'd >>>>>>> dereference the NULL in the hotplug path. We also need to register >>>>>>> the connector, so that userspace can get at it. >>>>>>> >>>>>> >>>>>> Shouldn't the KMS driver make sure the panel-bridge is set up before >>>>>> drm_mode_config_reset? Is it the case when we're inserting the >>>>>> panel-bridge driver as a module? >>>>>> >>>>>> >>>>>> All the connectors that have been added are registered automatically >>>>>> when drm_dev_register() is called by the KMS driver. Registering a >>>>>> connector in the middle of setting up our driver is prone to race >>>>>> conditions if the userspace decides to use them immediately. >>>>> >>>>> Yeah, this is fixing initializing panel_bridge at DSI host_attach time, >>>>> which in the case of a panel module that creates the DSI device >>>>> (adv7533-style, like you said I should use as a reference) will be after >>>>> drm_mode_config_reset() and drm_dev_register(). >>>> >>>> Okay. In the case of the msm kms driver, we defer probe until the >>>> adv7533 module is inserted, only then we proceed to drm_mode_config_reset() >>>> and drm_dev_register(). I assumed this was the general practice followed by >>>> most kms drivers. I.,e the kms driver defers probe until all connector >>>> related modules are inserted, and only then proceed to create a drm device. >>> >>> The problem, though, is the panel driver needs the MIPI DSI host to >>> exist to call mipi_dsi_device_register_full() during the probe process. >>> The adv7533 driver gets around this by registering the DSI device in the >>> bridge attach step, but drm_panel doesn't have an attach step. > > I'm not sure how we can get around this. We had discussion about this on irc > recently, but couldn't come up with a good conclusion. We could come up with a > panel_attach() callback to make it similar to bridges, but that's just us avoiding > the real issue. > >>> >>> Another alternative is my original version of the panel driver that was >>> a mipi_dsi_device driver that registered the panel during the DSI device >>> probe. That's why vc4's panel lookup is during the MIPI DSI attach >>> phase, currently. >> > > This would require you to have a DSI device node in DT, rather than an i2c > node, right? I don't know if we should do that because of a limitation in > our drm_mipi_dsi and drm_panel frameworks. > > Does anyone have better ideas? > > Thanks, > Archit > >> + Philippe in copy because we have the same probing issue when adding >> panel-bridge >> with the dsi bridge >> When adding panel-bridge support to the Synopsys DesignWare DSI bridge, I came to the conclusion that the only solution to make it works properly (without patching drm) was to "add the DSI bridge" at the end of the dw_mipi_dsi_host_attach() (see https://lists.freedesktop.org/archives/dri-devel/2017-June/144717.html) ie. defers crtc & encoder probing until the panel-bridge connector is created. Before that, I spent a lot of time trying different solutions like patching panel-bridge as Eric did (drm_connector_register...), adding bind/unbind() everywhere, debugging around __drm_mode_object_add... Surely DSI Host & device mechanism + panels add complexity to the related connector creation... After reading this thread, I have no good solution to suggest... and "deferring probing until connector registration" works fine now on my side and seems to be the way others drivers work... Philippe >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >>> >
WARNING: multiple messages have this Message-ID (diff)
From: Philippe CORNU <philippe.cornu@st.com> To: Archit Taneja <architt@codeaurora.org>, Benjamin Gaignard <benjamin.gaignard@linaro.org>, Eric Anholt <eric@anholt.net> Cc: Mark Rutland <mark.rutland@arm.com>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, Boris Brezillon <boris.brezillon@free-electrons.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, Rob Herring <robh+dt@kernel.org>, Thierry Reding <thierry.reding@gmail.com>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Subject: Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge. Date: Thu, 22 Jun 2017 09:31:18 +0000 [thread overview] Message-ID: <479892b4-909b-e2c0-025d-844f75d72899@st.com> (raw) In-Reply-To: <dd33044e-46d6-6f33-1011-b0bc343ddc96@codeaurora.org> On 06/22/2017 10:17 AM, Archit Taneja wrote: > > > On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: >> 2017-06-20 19:31 GMT+02:00 Eric Anholt <eric@anholt.net>: >>> Archit Taneja <architt@codeaurora.org> writes: >>> >>>> On 06/16/2017 08:13 PM, Eric Anholt wrote: >>>>> Archit Taneja <architt@codeaurora.org> writes: >>>>> >>>>>> On 06/16/2017 02:11 AM, Eric Anholt wrote: >>>>>>> If the panel-bridge is being set up after the drm_mode_config_reset(), >>>>>>> then the connector's state would never get initialized, and we'd >>>>>>> dereference the NULL in the hotplug path. We also need to register >>>>>>> the connector, so that userspace can get at it. >>>>>>> >>>>>> >>>>>> Shouldn't the KMS driver make sure the panel-bridge is set up before >>>>>> drm_mode_config_reset? Is it the case when we're inserting the >>>>>> panel-bridge driver as a module? >>>>>> >>>>>> >>>>>> All the connectors that have been added are registered automatically >>>>>> when drm_dev_register() is called by the KMS driver. Registering a >>>>>> connector in the middle of setting up our driver is prone to race >>>>>> conditions if the userspace decides to use them immediately. >>>>> >>>>> Yeah, this is fixing initializing panel_bridge at DSI host_attach time, >>>>> which in the case of a panel module that creates the DSI device >>>>> (adv7533-style, like you said I should use as a reference) will be after >>>>> drm_mode_config_reset() and drm_dev_register(). >>>> >>>> Okay. In the case of the msm kms driver, we defer probe until the >>>> adv7533 module is inserted, only then we proceed to drm_mode_config_reset() >>>> and drm_dev_register(). I assumed this was the general practice followed by >>>> most kms drivers. I.,e the kms driver defers probe until all connector >>>> related modules are inserted, and only then proceed to create a drm device. >>> >>> The problem, though, is the panel driver needs the MIPI DSI host to >>> exist to call mipi_dsi_device_register_full() during the probe process. >>> The adv7533 driver gets around this by registering the DSI device in the >>> bridge attach step, but drm_panel doesn't have an attach step. > > I'm not sure how we can get around this. We had discussion about this on irc > recently, but couldn't come up with a good conclusion. We could come up with a > panel_attach() callback to make it similar to bridges, but that's just us avoiding > the real issue. > >>> >>> Another alternative is my original version of the panel driver that was >>> a mipi_dsi_device driver that registered the panel during the DSI device >>> probe. That's why vc4's panel lookup is during the MIPI DSI attach >>> phase, currently. >> > > This would require you to have a DSI device node in DT, rather than an i2c > node, right? I don't know if we should do that because of a limitation in > our drm_mipi_dsi and drm_panel frameworks. > > Does anyone have better ideas? > > Thanks, > Archit > >> + Philippe in copy because we have the same probing issue when adding >> panel-bridge >> with the dsi bridge >> When adding panel-bridge support to the Synopsys DesignWare DSI bridge, I came to the conclusion that the only solution to make it works properly (without patching drm) was to "add the DSI bridge" at the end of the dw_mipi_dsi_host_attach() (see https://lists.freedesktop.org/archives/dri-devel/2017-June/144717.html) ie. defers crtc & encoder probing until the panel-bridge connector is created. Before that, I spent a lot of time trying different solutions like patching panel-bridge as Eric did (drm_connector_register...), adding bind/unbind() everywhere, debugging around __drm_mode_object_add... Surely DSI Host & device mechanism + panels add complexity to the related connector creation... After reading this thread, I have no good solution to suggest... and "deferring probing until connector registration" works fine now on my side and seems to be the way others drivers work... Philippe >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >>> > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-06-22 9:31 UTC|newest] Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-15 20:41 [PATCH 0/7] RPi touchscreen as a panel driver again Eric Anholt 2017-06-15 20:41 ` Eric Anholt 2017-06-15 20:41 ` [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge Eric Anholt 2017-06-15 20:41 ` Eric Anholt 2017-06-16 5:43 ` Archit Taneja 2017-06-16 5:43 ` Archit Taneja 2017-06-16 14:43 ` Eric Anholt 2017-06-16 14:43 ` Eric Anholt 2017-06-20 3:48 ` Archit Taneja 2017-06-20 3:48 ` Archit Taneja 2017-06-20 7:31 ` Laurent Pinchart 2017-06-20 7:31 ` Laurent Pinchart 2017-06-20 17:31 ` Eric Anholt 2017-06-20 17:31 ` Eric Anholt 2017-06-22 7:50 ` Benjamin Gaignard 2017-06-22 7:50 ` Benjamin Gaignard 2017-06-22 8:17 ` Archit Taneja 2017-06-22 8:17 ` Archit Taneja 2017-06-22 9:23 ` Boris Brezillon 2017-06-22 9:23 ` Boris Brezillon 2017-06-22 12:29 ` Andrzej Hajda 2017-06-22 12:29 ` Andrzej Hajda 2017-06-22 12:41 ` Boris Brezillon 2017-06-22 12:41 ` Boris Brezillon 2017-06-22 13:16 ` Andrzej Hajda 2017-06-22 13:16 ` Andrzej Hajda 2017-06-22 13:34 ` Boris Brezillon 2017-06-22 13:34 ` Boris Brezillon 2017-06-23 7:22 ` Andrzej Hajda 2017-06-23 7:22 ` Andrzej Hajda 2017-06-23 7:32 ` Boris Brezillon 2017-06-23 7:32 ` Boris Brezillon 2017-06-23 13:54 ` Archit Taneja 2017-06-23 21:50 ` Eric Anholt 2017-06-23 21:50 ` Eric Anholt 2017-06-27 6:53 ` Archit Taneja 2017-06-27 6:53 ` Archit Taneja 2017-06-22 9:31 ` Philippe CORNU [this message] 2017-06-22 9:31 ` Philippe CORNU 2017-06-23 21:36 ` Eric Anholt 2017-06-23 21:36 ` Eric Anholt 2017-06-23 9:04 ` Daniel Vetter 2017-06-23 9:04 ` Daniel Vetter 2017-06-15 20:41 ` [PATCH 2/7] drm/vc4: Fix DSI T_INIT timing Eric Anholt 2017-06-15 20:41 ` Eric Anholt 2017-06-15 20:41 ` [PATCH 3/7] drm/vc4: Fix misleading name of the continuous flag Eric Anholt 2017-06-15 20:41 ` Eric Anholt 2017-06-15 20:41 ` [PATCH 4/7] drm/vc4: Use drm_mode_vrefresh() in DSI fixup, in case vrefresh is 0 Eric Anholt 2017-06-15 20:41 ` Eric Anholt 2017-06-15 20:41 ` [PATCH 5/7] dt-bindings: Document the Raspberry Pi Touchscreen nodes Eric Anholt 2017-06-23 18:44 ` Rob Herring 2017-06-23 18:44 ` Rob Herring 2017-06-15 20:41 ` [PATCH 6/7] drm/panel: Add support for the Raspberry Pi 7" Touchscreen Eric Anholt 2017-06-15 20:41 ` Eric Anholt 2017-06-15 20:41 ` [PATCH 7/7] ARM: dts: bcm2835: Enable the Raspberry Pi touchscreen panel Eric Anholt 2017-06-15 20:41 ` Eric Anholt
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=479892b4-909b-e2c0-025d-844f75d72899@st.com \ --to=philippe.cornu@st.com \ --cc=Laurent.pinchart@ideasonboard.com \ --cc=a.hajda@samsung.com \ --cc=architt@codeaurora.org \ --cc=benjamin.gaignard@linaro.org \ --cc=boris.brezillon@free-electrons.com \ --cc=devicetree@vger.kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=eric@anholt.net \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=robh+dt@kernel.org \ --cc=thierry.reding@gmail.com \ /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.