| Message ID | 20240326-hotplug-drm-bridge-v1-0-4b51b5eb75d5@bootlin.com |
|---|---|
| Headers | show |
| Series | drm: add support for hot-pluggable bridges | expand |
On Tue, Mar 26, 2024 at 05:28:14PM +0100, Luca Ceresoli wrote: > This driver implements the point of a DRM pipeline where a connector allows > removal of all the following bridges up to the panel. > > The DRM subsystem currently allows hotplug of the monitor but not preceding > components. However there are embedded devices where the "tail" of the DRM > pipeline, including one or more bridges, can be physically removed: > > .------------------------. > | DISPLAY CONTROLLER | > | .---------. .------. | > | | ENCODER |<--| CRTC | | > | '---------' '------' | > '------|-----------------' > | > | HOTPLUG > V CONNECTOR > .---------. .--. .-. .---------. .-------. > | 0 to N | | _| _| | | 1 to N | | | > | BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | > | | | | | | | | | | > '---------' '--' '-' '---------' '-------' > > [--- fixed components --] [----------- removable add-on -----------] > > This driver supports such devices, where the final segment of a MIPI DSI > bus, including one or more bridges, can be physically disconnected and > reconnected at runtime, possibly with a different model. > > This implementation supports a MIPI DSI bus only, but it is designed to be > as far as possible generic and extendable to other busses that have no > native hotplug and model ID discovery. > > This driver does not provide facilities to add and remove the hot-pluggable > components from the kernel: this needs to be done by other means > (e.g. device tree overlay runtime insertion and removal). The > hotplug-bridge gets notified of hot-plugging by the DRM bridge notifier > callbacks after they get added or before they get removed. > > The hotplug-bridge role is to implement the "hot-pluggable connector" in > the bridge chain. In this position, what the hotplug-bridge should ideally > do is: > > * communicate with the previous component (bridge or encoder) so that it > believes it always has a connected bridge following it and the DRM card > is always present > * be notified of the addition and removal of the following bridge and > attach/detach to/from it > * communicate with the following bridge so that it will attach and detach > using the normal procedure (as if the entire pipeline were being created > or destroyed, not only the tail) > * expose the "add-on connected/disconnected" status via the DRM connector > connected/disconnected status, so that users of the DRM pipeline know > when they can render output on the display > > However some aspects make it a bit more complex than that. Most notably: > > * the next bridge can be probed and removed at any moment and all probing > sequences need to be handled > * the DSI host/device registration process, which adds to the DRM bridge > attach process, makes the initial card registration tricky > * the need to register and deregister the following bridges at runtime > without tearing down the whole DRM card prevents using the functions > that are normally recommended > * the automatic mechanism to call the appropriate .get_modes operation > (typically provided by the panel bridge) cannot work as the panel can > disappear and reappear as a different model, so an ad-hoc lookup is > needed There's several additional hurdles there: - You mentioned the connector in your ideal scenario. But as soon as you remove the last bridge, the connector will probably go away too. There's two scenarii here then: - The driver is ok, and it will stay there until the last user its to the main DRM device. Which means that if you create a new one, you'll have the old one and the new one together, but you can't tell which one you're supposed to use. - If the driver isn't ok, the connector will be freed immediately. There's plenty of lingering pointers in the framework, and especially the states though, leading to use-after-free errors. - So far, we told everyone that the graphics pipeline wasn't going to change. How do you expect applications to deal with a connector going away without any regression? I guess the natural thing here would be to emit a uevent just like we do when the connection status change, but the thing is: we're doing that for the connector, and the connector is gone. Between the userspace expectations and the memory-safety issue plaguing way too many drivers, I'm not sure this approach can work. I guess one way to somewhat achieve what you're trying to do would be to introduce the connection status at the bridge level, reflect the aggregate connection status of all bridges on the connector, and make each bridge driver probe its device in the connect hook through DCS or I2C. We wouldn't be able to change the bridge halfway through the system's life, but like I said, KMS cannot hotplug connectors at the moment and doing so requires far more work. Maxime
Hi Maxime, On Wed, 27 Mar 2024 13:42:40 +0100 Maxime Ripard <mripard@kernel.org> wrote: > On Tue, Mar 26, 2024 at 05:28:14PM +0100, Luca Ceresoli wrote: > > This driver implements the point of a DRM pipeline where a connector allows > > removal of all the following bridges up to the panel. > > > > The DRM subsystem currently allows hotplug of the monitor but not preceding > > components. However there are embedded devices where the "tail" of the DRM > > pipeline, including one or more bridges, can be physically removed: > > > > .------------------------. > > | DISPLAY CONTROLLER | > > | .---------. .------. | > > | | ENCODER |<--| CRTC | | > > | '---------' '------' | > > '------|-----------------' > > | > > | HOTPLUG > > V CONNECTOR > > .---------. .--. .-. .---------. .-------. > > | 0 to N | | _| _| | | 1 to N | | | > > | BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | > > | | | | | | | | | | > > '---------' '--' '-' '---------' '-------' > > > > [--- fixed components --] [----------- removable add-on -----------] > > > > This driver supports such devices, where the final segment of a MIPI DSI > > bus, including one or more bridges, can be physically disconnected and > > reconnected at runtime, possibly with a different model. > > > > This implementation supports a MIPI DSI bus only, but it is designed to be > > as far as possible generic and extendable to other busses that have no > > native hotplug and model ID discovery. > > > > This driver does not provide facilities to add and remove the hot-pluggable > > components from the kernel: this needs to be done by other means > > (e.g. device tree overlay runtime insertion and removal). The > > hotplug-bridge gets notified of hot-plugging by the DRM bridge notifier > > callbacks after they get added or before they get removed. > > > > The hotplug-bridge role is to implement the "hot-pluggable connector" in > > the bridge chain. In this position, what the hotplug-bridge should ideally > > do is: > > > > * communicate with the previous component (bridge or encoder) so that it > > believes it always has a connected bridge following it and the DRM card > > is always present > > * be notified of the addition and removal of the following bridge and > > attach/detach to/from it > > * communicate with the following bridge so that it will attach and detach > > using the normal procedure (as if the entire pipeline were being created > > or destroyed, not only the tail) > > * expose the "add-on connected/disconnected" status via the DRM connector > > connected/disconnected status, so that users of the DRM pipeline know > > when they can render output on the display > > > > However some aspects make it a bit more complex than that. Most notably: > > > > * the next bridge can be probed and removed at any moment and all probing > > sequences need to be handled > > * the DSI host/device registration process, which adds to the DRM bridge > > attach process, makes the initial card registration tricky > > * the need to register and deregister the following bridges at runtime > > without tearing down the whole DRM card prevents using the functions > > that are normally recommended > > * the automatic mechanism to call the appropriate .get_modes operation > > (typically provided by the panel bridge) cannot work as the panel can > > disappear and reappear as a different model, so an ad-hoc lookup is > > needed > > There's several additional hurdles there: > > - You mentioned the connector in your ideal scenario. But as soon as > you remove the last bridge, the connector will probably go away too. > There's two scenarii here then: > > - The driver is ok, and it will stay there until the last user its to > the main DRM device. Which means that if you create a new one, > you'll have the old one and the new one together, but you can't > tell which one you're supposed to use. > > - If the driver isn't ok, the connector will be freed immediately. > There's plenty of lingering pointers in the framework, and > especially the states though, leading to use-after-free errors. > > - So far, we told everyone that the graphics pipeline wasn't going to > change. How do you expect applications to deal with a connector going > away without any regression? I guess the natural thing here would be > to emit a uevent just like we do when the connection status change, > but the thing is: we're doing that for the connector, and the > connector is gone. Thanks for your feedback. I probably should have discussed this aspect in my cover letter, sorry about that, let me amend now. I think there are two possible approaches. The first approach is based on removing the drm_connector. My laptop uses the i915 driver, and I have observed that attaching/removing a USB-C dock with an HDMI connector connected to a monitor, a new drm_connector appears/disappears for the card. User space gets notified and the external monitor is enabled/disabled, just the way a desktop user would expect, so this is possible. I had a look at the driver but how this magic happens was not clear to me honestly. The second approach is simpler and based on keeping the drm_connector always instantiated, and it is what this driver does. The drm_connector is added by the hotplug-bridge driver in the drm_bridge_funcs.attach op, which happens initially, and only removed by drm_bridge_funcs.detach, so it is never removed when detaching the _following_ part of the pipeline (which the card is unaware of). So the encoder always has a drm_connector. Note when attaching to the downstream bridge we pass the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, which _should_ prevent creation of a second connector. I'd expect some drivers to not honour that flag, but they can be fixed if needed. When the tail of the pipeline is connected/removed, the hpb->next_bridge pointer becomes valid/NULL. And hotplug_bridge_detect() looks at exactly that pointer to return a connected or disconnected status. The result is that when the add-on is connected, 'modetest -c' shows: Connectors: id encoder status name size (mm) modes encoders 37 0 connected DSI-1 293x165 1 36 modes: index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot #0 1920x1080 60.00 1920 1978 2020 2108 1080 1088 1102 1116 141140 flags: ; type: preferred, driver props: ... and when it is disconnected, it shows: Connectors: id encoder status name size (mm) modes encoders 37 0 disconnected DSI-1 0x0 0 36 props: ... weston detects the HPD events from the connector and starts/stops using the removable display correctly. Does this clarify the approach? I could be missing some aspects of course, especially in case of more complex hardware setups than the one I have. However the code in this series has been tested for a long time and no memory-safety issue has appeared. > Between the userspace expectations and the memory-safety issue plaguing > way too many drivers, I'm not sure this approach can work. > > I guess one way to somewhat achieve what you're trying to do would be to > introduce the connection status at the bridge level, reflect the > aggregate connection status of all bridges on the connector, and make > each bridge driver probe its device in the connect hook through DCS or > I2C. I think you mean: keeping all the bridge drivers instantiated, even when the physical chip is removed. This is of course another possible approach. However it would be more invasive, forcing bridge drivers to change their current behaviour. And it would violate the design that a driver is probed when a device is there, and removed when the hardware goes away. The approach I took firstly allows to have zero modifications to existing bridge drivers -- not necessarily the right thing to do, but I didn't find any good reason to require that. Additionally, it is designed to allow removing an add-on having bridge XYZ and then plugging in another add-on with bridge ABC, having a different driver. Keeping alive the XYZ driver on unplug would not make sense in such a case. This is not a tested scenario as I have no hardware allowing that, but it is part of the design goals and I see no obvious reason it wouldn't work with this patch as is, since the downstream bridge driver is removed on disconnect and probed on connect for whatever bridge will be connected. Best regards, Luca
Hi Maxime, On Wed, 27 Mar 2024 17:08:49 +0100 Luca Ceresoli <luca.ceresoli@bootlin.com> wrote: [...] > > There's several additional hurdles there: > > > > - You mentioned the connector in your ideal scenario. But as soon as > > you remove the last bridge, the connector will probably go away too. > > There's two scenarii here then: > > > > - The driver is ok, and it will stay there until the last user its to > > the main DRM device. Which means that if you create a new one, > > you'll have the old one and the new one together, but you can't > > tell which one you're supposed to use. > > > > - If the driver isn't ok, the connector will be freed immediately. > > There's plenty of lingering pointers in the framework, and > > especially the states though, leading to use-after-free errors. > > > > - So far, we told everyone that the graphics pipeline wasn't going to > > change. How do you expect applications to deal with a connector going > > away without any regression? I guess the natural thing here would be > > to emit a uevent just like we do when the connection status change, > > but the thing is: we're doing that for the connector, and the > > connector is gone. > > Thanks for your feedback. I probably should have discussed this aspect > in my cover letter, sorry about that, let me amend now. > > I think there are two possible approaches. > > The first approach is based on removing the drm_connector. My laptop > uses the i915 driver, and I have observed that attaching/removing a > USB-C dock with an HDMI connector connected to a monitor, a new > drm_connector appears/disappears for the card. User space gets notified > and the external monitor is enabled/disabled, just the way a desktop > user would expect, so this is possible. I had a look at the driver but > how this magic happens was not clear to me honestly. > > The second approach is simpler and based on keeping the drm_connector > always instantiated, and it is what this driver does. The drm_connector > is added by the hotplug-bridge driver in the drm_bridge_funcs.attach op, > which happens initially, and only removed by drm_bridge_funcs.detach, > so it is never removed when detaching the _following_ part of the > pipeline (which the card is unaware of). So the encoder always has a > drm_connector. > > Note when attaching to the downstream bridge we pass the > DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, which _should_ prevent creation of a > second connector. I'd expect some drivers to not honour that flag, but > they can be fixed if needed. > > When the tail of the pipeline is connected/removed, the > hpb->next_bridge pointer becomes valid/NULL. And > hotplug_bridge_detect() looks at exactly that pointer to return a > connected or disconnected status. > > The result is that when the add-on is connected, 'modetest -c' shows: > > Connectors: > id encoder status name size (mm) modes encoders > 37 0 connected DSI-1 293x165 1 36 > modes: > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot > #0 1920x1080 60.00 1920 1978 2020 2108 1080 1088 1102 1116 141140 flags: ; type: preferred, driver > props: > ... > > and when it is disconnected, it shows: > > Connectors: > id encoder status name size (mm) modes encoders > 37 0 disconnected DSI-1 0x0 0 36 > props: > ... > > weston detects the HPD events from the connector and starts/stops using > the removable display correctly. > > Does this clarify the approach? > > I could be missing some aspects of course, especially in case of more > complex hardware setups than the one I have. However the code in this > series has been tested for a long time and no memory-safety issue has > appeared. > > > Between the userspace expectations and the memory-safety issue plaguing > > way too many drivers, I'm not sure this approach can work. > > > > I guess one way to somewhat achieve what you're trying to do would be to > > introduce the connection status at the bridge level, reflect the > > aggregate connection status of all bridges on the connector, and make > > each bridge driver probe its device in the connect hook through DCS or > > I2C. > > I think you mean: keeping all the bridge drivers instantiated, even > when the physical chip is removed. > > This is of course another possible approach. However it would be more > invasive, forcing bridge drivers to change their current behaviour. And > it would violate the design that a driver is probed when a device is > there, and removed when the hardware goes away. > > The approach I took firstly allows to have zero modifications to > existing bridge drivers -- not necessarily the right thing to do, but I > didn't find any good reason to require that. > > Additionally, it is designed to allow removing an add-on having bridge > XYZ and then plugging in another add-on with bridge ABC, having a > different driver. Keeping alive the XYZ driver on unplug would not make > sense in such a case. This is not a tested scenario as I have no > hardware allowing that, but it is part of the design goals and I see no > obvious reason it wouldn't work with this patch as is, since the > downstream bridge driver is removed on disconnect and probed on connect > for whatever bridge will be connected. Did you have a chance to think about this? I was wondering whether my comments addresses some of your concerns, and whether these additional clarifications make my approach look reasonable to you. Best regards, Luca
Hello, DRM natively supports pipelines whose display can be removed, but all the components preceding it (all the display controller and any bridges) are assumed to be fixed and cannot be plugged, removed or modified at runtime. This series adds support for DRM pipelines having a removable part after the encoder, thus also allowing bridges to be removed and reconnected at runtime, possibly with different components. In the overall ongoing work, this is going to be handled via device tree overlay insertion and removal. For many kernel driver frameworks, adding and removing devices via device tree overlays works already (albeit with some issues related to overlays in general), but this does not happen for DRM, so this serias aims at filling this gap. This series only covers the DRM aspects and not the overlay ones. See "Development roadmap" below for more details. Use case -------- The use case we are working on is to support professional products that have a portable "main" part running on battery, with the main SoC and able to work autonomously with limited features, and that can be connected to an "add-on" part that is not portable and adds more features. The add-on can be connected and disconnected at runtime at any moment by the end user, and add-on features need to be enabled and disabled automatically at runtime. The features provided by the add-on include a display and a battery charger to recharge the battery of the main part. The display on the add-on has an LVDS input but the connector between the base and the add-on has a MIPI DSI bus, so a DSI-to-LVDS bridge is present on the add-on. Targeted abstraction level -------------------------- This series aims at supporting both the use case described above and any similar use cases, e.g. using different video busses, up to a given level of generalization. This picture summarizes the DRM aspects of such devices: .------------------------. | DISPLAY CONTROLLER | | .---------. .------. | | | ENCODER |<--| CRTC | | | '---------' '------' | '------|-----------------' | |DSI HOTPLUG V CONNECTOR .---------. .--. .-. .---------. .-------. | 0 to N | | _| _| | | 1 to N | | | | BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | | | | | | | | | | | '---------' '--' '-' '---------' '-------' [--- fixed components --] [----------- removable add-on -----------] Fixed components include: * all components up to the DRM encoder, usually part of the SoC * optionally some bridges, in the SoC and/or as external chips Components on the removable add-on include: * one or more bridges * a fixed connector (not one natively supporting hotplug such as HDMI) * the panel Overall this looks like a fairly standard embedded device, except for the hot-pluggable connector allowing to remove a bridge and all the following components at runtime and without prior notice for the kernel. The video bus is MIPI DSI in the example and in the implementation provided by this series, but the implementation is meant to allow can be generalizedwgeneralization to other video busses without native hotplug support, such as parallel video and LVDS. The "hotplug connector" in picture is the mechanical connector that can be physically removed at runtime. All the video bus signals (DSI in the example) get connected or disconnected via that connector. Note that the term "connector" in this context has nothing to do with the "DRM connector" abstraction already present in the DRM subsystem (struct drm_connector). The existing "DRM connector" has been designed to support hotplug on a bus that physically supports both hotplug _and_ monitor identification (e.g. HDMI), and later also used to model the connection to a non-removable panel that is commonly found on embedded systems and supports neither hotplug nor panel identification. For this reason, the "DRM connector" is always physically located after all the bridges. The "hotplug connector" here described is physically hot-pluggable but does not support model identification, being meant for buses that do not support identification because they do not support hot-plugging natively. This is why at least 1 bridge is assumed to be present in the removable add-on: if there were no such bridge, the "hotplug connector" would be immediately followed by the "DRM connector" and the panel. In such a situation, hot-plugging could be implemented by the "DRM connector" in a much more straightforward way. So this work is mostly useful when there is at least one bridge on the removable add-on. The removable components form a unique assembly whose components can not be separated individually: at any given moment the add-on is either connected or disconencted -- it is never considered partially connected. After an add-on has been removed, an add-on of a different model can be connected, e.g. providing a different panel needing different timings. The technique to identify the model that gets connected is outside of the scope of this patch series, as described in "Development roadmap" below. Implementation -------------- In order to support the above use case while being reasonably generic and avoid unnecessary changes to the common DRM code, the implementation is based on the introduction of the "hotplug-bridge", a new bridge driver that represents the "hotplug connector" and should be positioned in the DRM pipeline exactly where the "hotplug connector" is. In this position the hotplug-bridge decouples the preceding and the following components so that each of them can be implemented normally, without having to even be aware of hot-plugging. The implementation is as transparent as possible in order to minimize the need of any changes to the design of other components: it is based on a few self-contained additions to drm_bridge and drm_encoder, and does not require any modification to other bridges. However the implementation has some tricky aspects that make it more complex than in an ideal design. See the last patch in the series for the details. Patch overview: * patch 1 adds device tree bindings for the "hotplug video connector" * patches 2 and 3 add some prerequisite new features to the DRM code * patch 4 adds the hotplug-bridge itself Development roadmap ------------------- This series is a part of a larger work to support embedded devices having a hot-pluggable add-on. The overall work includes: 1. a mechanism to detect add-on insertion/removal, read the model ID and load a corresponding device tree overlay 2. fixes to existing bugs that get exposed when loading/unloading device tree overlays at runtime 3. allowing the tail of a DRM pipeline to be hot-pluggable [this series] All of the above are under development at Bootlin, and patches for item 2 are already under discussion on the relevant mailing-lists. Item 1 is a prerequisite for production usage of the hotplug-bridge, but even though it is working well enough in internal testing, it is not yet ready for sending patches for review. This patch series covering item 3 is being sent anyway in order to start discussion with the kernel community as early as possible, expecially the DRM community as this work is changing some of the assumptions behind the DRM architecture. Testing ------- This series cannot be tested in public until the mechanism for add-on insertion and removal will be sent. It can however be tested by other means, even with a hardware that has no removable parts, "pretending" that one or more bridges can be removed: * remove and re-insert the driver module for the DRM bridge after the hotplug-bridge * unbind/bind the DRM bridge after the hotplug-bridge from its driver Thanks for you patience in reading this! Luca Co-developed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> --- Luca Ceresoli (3): dt-bindings: display: bridge: add the Hot-plug MIPI DSI connector drm/encoder: add drm_encoder_cleanup_from() drm/bridge: hotplug-bridge: add driver to support hot-pluggable DSI bridges Paul Kocialkowski (1): drm/bridge: add bridge notifier to be notified of bridge addition and removal .../bridge/hotplug-video-connector-dsi.yaml | 87 ++++ MAINTAINERS | 6 + drivers/gpu/drm/bridge/Kconfig | 15 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/hotplug-bridge.c | 561 +++++++++++++++++++++ drivers/gpu/drm/drm_bridge.c | 35 ++ drivers/gpu/drm/drm_encoder.c | 21 + include/drm/drm_bridge.h | 19 + include/drm/drm_encoder.h | 1 + 9 files changed, 746 insertions(+) --- base-commit: 30b26f75c864d1da39fe5e8628f1cbc3702a9add change-id: 20240319-hotplug-drm-bridge-16b86e67fe92 Best regards,