Re: [PATCH 00/24] device link, bridge supplier <-> drm device
On Fri, Apr 27, 2018 at 01:40:21AM +0300, Laurent Pinchart wrote: > Hi Peter, > > Thank you for the patches. > > On Friday, 27 April 2018 01:31:15 EEST Peter Rosin wrote: > > Hi! > > > > It was noted by Russel King [1] that bridges (not using components) > > might disappear unexpectedly if the owner of the bridge was unbound. > > Jyri Sarha had previously noted the same thing with panels [2]. Jyri > > came up with using device links to resolve the panel issue, which > > was also my (independent) reaction to the note from Russel. > > > > This series builds up to the addition of that link in the last > > patch, but in my opinion the other 23 patches do have merit on their > > own. > > > > The last patch needs testing, while the others look trivial. That > > said, I might have missed some subtlety. > > I don't think this is the way to go. We have a real lifetime management > problem with bridges, and device links are just trying to hide the problem > under the carpet. They will further corner us by making a real fix much more > difficult to implement. I'll try to comment further in the next few days on > what I think a better solution would be, but in a nutshell I believe that > drm_bridge objects need to be refcounted, with a .release() operation to free > the bridge resources when the reference count drops to zero. This shouldn't > be > difficult to implement and I'm willing to help. I agree that refcounting is the proper fix if bridges can be hot-unplugged, independently of the overall drm_device. And yes retro-shoehorning refcounting is "fun", but it's also not impossible. I've done it a few times by now in drm. Otoh, refcounting has a pretty serious cost - we're still blowing up drm_framebuffer refcounting in corner cases at shocking regularity, and that's something that's been refcounted for years by now. As long as we don't have a clearly defined need to make bridges hotpluggable then I think not paying the hefty bill just yet is the correct technical decision. This patch series here looks like a reasonable way to tie the lifetimes together a bit more closely and should fix the bugs we have right now. Cheers, Daniel PS: Yes there's cases where you can hotplug display blocks on a SoC. To my knowledge they're all covered by hotplugging the entire drm_device (plus all the statically associated bits like drm_bridge/crtc/plane/encoder). And we're already working on fixing up drm_device to at least make it possible to write a correct hot-unpluggable driver in theory. We'll see whether anyone manages to pull that off in practice :-) > > > [1] https://lkml.org/lkml/2018/4/23/769 > > [2] https://www.spinics.net/lists/dri-devel/msg174275.html > > > > Peter Rosin (24): > > drm/bridge: allow optionally specifying an .owner device > > drm/bridge: adv7511: provide an .owner device > > drm/bridge/analogix: core: specify the .owner of the bridge > > drm/bridge: analogix-anx78xx: provide an .owner device > > drm/bridge: vga-dac: provide an .owner device > > drm/bridge: lvds-encoder: provide an .owner device > > drm/bridge: megachips-stdp-ge-b850v3-fw: provide an .owner device > > drm/bridge: nxp-ptn3460: provide an .owner device > > drm/bridge: panel: provide an .owner device > > drm/bridge: ps8622: provide an .owner device > > drm/bridge: sii902x: provide an .owner device > > drm/bridge: sii9234: provide an .owner device > > drm/bridge: sii8620: provide an .owner device > > drm/bridge: synopsys: provide an .owner device for the bridges > > drm/bridge: tc358767: provide an .owner device > > drm/bridge: ti-tfp410: provide an .owner device > > drm/exynos: mic: provide an .owner device for the bridge > > drm/mediatek: hdmi: provide an .owner device for the bridge > > drm/msm: specify the .owner of the bridges > > drm/rcar-du: lvds: provide an .owner device for the bridge > > drm/sti: provide an .owner device for the bridges > > drm/bridge: remove the .of_node member > > drm/bridge: require the .owner to be filled in on drm_bridge_attach > > drm/bridge: establish a link between the bridge supplier and consumer > > > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > > drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 + > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + > > drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +- > > drivers/gpu/drm/bridge/lvds-encoder.c | 2 +- > > .../drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- > > drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- > > drivers/gpu/drm/bridge/panel.c | 4 +--- > > drivers/gpu/drm/bridge/parade-ps8622.c | 2 +- > > drivers/gpu/drm/bridge/sii902x.c | 2 +- > > drivers/gpu/drm/bridge/sii9234.c | 2 +- > > drivers/gpu/drm/bridge/sil-sii8620.c | 2 +- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +--- > > drivers/
Re: [PATCH 00/24] device link, bridge supplier <-> drm device
On 2018-04-27 09:37, Peter Rosin wrote: > On 2018-04-27 09:11, Andrzej Hajda wrote: >> Hi Peter, >> >> On 27.04.2018 00:31, Peter Rosin wrote: >>> Hi! >>> >>> It was noted by Russel King [1] that bridges (not using components) >>> might disappear unexpectedly if the owner of the bridge was unbound. >>> Jyri Sarha had previously noted the same thing with panels [2]. Jyri >>> came up with using device links to resolve the panel issue, which >>> was also my (independent) reaction to the note from Russel. >>> >>> This series builds up to the addition of that link in the last >>> patch, but in my opinion the other 23 patches do have merit on their >>> own. >>> >>> The last patch needs testing, while the others look trivial. That >>> said, I might have missed some subtlety. >> >> of_node is used as an identifier of the bridge in the kernel. If you >> replace it with device pointer there will be potential problem with >> devices having two or more bridges, how do you differentiate bridges if >> the owner is the same? If I remember correctly current bridge code does >> not allow to have multiple bridges in one device, but that should be >> quite easy to fix if necessary. After this change it will become more >> difficult. > > I don't see how it will be more difficult? > >> Anyway I remember discussion that in DT world bridge should be >> identified rather by of_graph port node, not by parent node as it is >> now. If you want to translate this relation to device owner, you should >> add also port number to have full identification of the bridge, ie pair >> (owner, port_number) would be equivalent of port node. > > You even state the trivial solution here, just add the port/endpoint ID > when/if it is needed. So, what is the significant difference? Or, since this is apparently a rare requirement, you could make the owners that do need it fix it themselves. E.g. by embedding the struct drm_bridge in another struct that contains the needed ID, and use container_of to get to that containing struct with the ID. Cheers, Peter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/24] device link, bridge supplier <-> drm device
On 2018-04-27 09:11, Andrzej Hajda wrote: > Hi Peter, > > On 27.04.2018 00:31, Peter Rosin wrote: >> Hi! >> >> It was noted by Russel King [1] that bridges (not using components) >> might disappear unexpectedly if the owner of the bridge was unbound. >> Jyri Sarha had previously noted the same thing with panels [2]. Jyri >> came up with using device links to resolve the panel issue, which >> was also my (independent) reaction to the note from Russel. >> >> This series builds up to the addition of that link in the last >> patch, but in my opinion the other 23 patches do have merit on their >> own. >> >> The last patch needs testing, while the others look trivial. That >> said, I might have missed some subtlety. > > of_node is used as an identifier of the bridge in the kernel. If you > replace it with device pointer there will be potential problem with > devices having two or more bridges, how do you differentiate bridges if > the owner is the same? If I remember correctly current bridge code does > not allow to have multiple bridges in one device, but that should be > quite easy to fix if necessary. After this change it will become more > difficult. I don't see how it will be more difficult? > Anyway I remember discussion that in DT world bridge should be > identified rather by of_graph port node, not by parent node as it is > now. If you want to translate this relation to device owner, you should > add also port number to have full identification of the bridge, ie pair > (owner, port_number) would be equivalent of port node. You even state the trivial solution here, just add the port/endpoint ID when/if it is needed. So, what is the significant difference? Cheers, Peter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/24] device link, bridge supplier <-> drm device
On 2018-04-27 00:40, Laurent Pinchart wrote: > Hi Peter, > > Thank you for the patches. > > On Friday, 27 April 2018 01:31:15 EEST Peter Rosin wrote: >> Hi! >> >> It was noted by Russel King [1] that bridges (not using components) >> might disappear unexpectedly if the owner of the bridge was unbound. >> Jyri Sarha had previously noted the same thing with panels [2]. Jyri >> came up with using device links to resolve the panel issue, which >> was also my (independent) reaction to the note from Russel. >> >> This series builds up to the addition of that link in the last >> patch, but in my opinion the other 23 patches do have merit on their >> own. >> >> The last patch needs testing, while the others look trivial. That >> said, I might have missed some subtlety. > > I don't think this is the way to go. We have a real lifetime management > problem with bridges, and device links are just trying to hide the problem > under the carpet. They will further corner us by making a real fix much more > difficult to implement. I'll try to comment further in the next few days on > what I think a better solution would be, but in a nutshell I believe that > drm_bridge objects need to be refcounted, with a .release() operation to free > the bridge resources when the reference count drops to zero. This shouldn't > be > difficult to implement and I'm willing to help. Ok, sp 24/24 is dead, and maybe 23/24 too. But how do you feel about dropping .of_node in favour of .owner? That gets rid of ugly #ifdefs... I also have the nagging feeling that .driver_private serves very little real purpose if there is a .owner so that you can do dev_get_drvdata(bridge->owner) for the cases where the container_of macro is not appropriate. Cheers, Peter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/24] device link, bridge supplier <-> drm device
On 2018-04-27 01:18, Laurent Pinchart wrote: > Hi Peter, > > On Friday, 27 April 2018 02:09:14 EEST Peter Rosin wrote: >> On 2018-04-27 00:40, Laurent Pinchart wrote: >>> On Friday, 27 April 2018 01:31:15 EEST Peter Rosin wrote: Hi! It was noted by Russel King [1] that bridges (not using components) might disappear unexpectedly if the owner of the bridge was unbound. Jyri Sarha had previously noted the same thing with panels [2]. Jyri came up with using device links to resolve the panel issue, which was also my (independent) reaction to the note from Russel. This series builds up to the addition of that link in the last patch, but in my opinion the other 23 patches do have merit on their own. The last patch needs testing, while the others look trivial. That said, I might have missed some subtlety. >>> >>> I don't think this is the way to go. We have a real lifetime management >>> problem with bridges, and device links are just trying to hide the problem >>> under the carpet. They will further corner us by making a real fix much >>> more difficult to implement. I'll try to comment further in the next few >>> days on what I think a better solution would be, but in a nutshell I >>> believe that drm_bridge objects need to be refcounted, with a .release() >>> operation to free the bridge resources when the reference count drops to >>> zero. This shouldn't be difficult to implement and I'm willing to help. >> >> Ok, sp 24/24 is dead, and maybe 23/24 too. > > Not necessarily, maybe I'll get proven wrong :-) Let's discuss, but I first > need some sleep. Ok, I'll add my view... I don't see how a refcount is going to resolve anything. Let's take some device that allocs a bridge that is later attached to some encoder. In doing so it adds hooks to some of the drm_bridge_funcs. If you add a refcount to the bridge itself then yes, the bridge object might remain but the code backing the hook functions will go away the moment the owner device goes away. So, you'd have to refcount the owner device itself to prevent it from going away. But, you can't stop a device from going away IIUC, you can only bring other stuff down with it in an orderly fashion. Components, that some bridges use, takes care of bringing the whole cluster down *before* any device goes away, so that is obviously a solution. But that solution is not in place everywhere. Now, my device-link is pretty light-weight. And it should only matter if the owner goes away before the consuming drm device has brought down the encoder properly. If that ever happens, we're in trouble. So, the link can at worst only substitute one problem with another, and at best it prevents the fireworks. Sure, there's the reprobe problem if the bridge's owner device shows up again, but that's pretty minor compared to a hard crash. And there was a patch for that, so in the end that may be a non-issue. If some other solution is found, removing the device-link is trivial. It is localized to bridge attach/detach and nothing else is affected (in terms of code). Cheers, Peter >> But how do you feel about dropping .of_node in favour of .owner? That gets >> rid of ugly #ifdefs... > > I'll review that part and reply, I have nothing against it on principle at > the > moment. The more generic the code is, the better in my opinion. We just need > to make sure that the OF node we're interested in will always be .owner- >> of_node in all cases. > >> I also have the nagging feeling that .driver_private serves very little real >> purpose if there is a .owner so that you can do >> >> dev_get_drvdata(bridge->owner) >> >> for the cases where the container_of macro is not appropriate. > > I'll review that too, it's a good point. > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/24] device link, bridge supplier <-> drm device
Hi! It was noted by Russel King [1] that bridges (not using components) might disappear unexpectedly if the owner of the bridge was unbound. Jyri Sarha had previously noted the same thing with panels [2]. Jyri came up with using device links to resolve the panel issue, which was also my (independent) reaction to the note from Russel. This series builds up to the addition of that link in the last patch, but in my opinion the other 23 patches do have merit on their own. The last patch needs testing, while the others look trivial. That said, I might have missed some subtlety. Cheers, Peter [1] https://lkml.org/lkml/2018/4/23/769 [2] https://www.spinics.net/lists/dri-devel/msg174275.html Peter Rosin (24): drm/bridge: allow optionally specifying an .owner device drm/bridge: adv7511: provide an .owner device drm/bridge/analogix: core: specify the .owner of the bridge drm/bridge: analogix-anx78xx: provide an .owner device drm/bridge: vga-dac: provide an .owner device drm/bridge: lvds-encoder: provide an .owner device drm/bridge: megachips-stdp-ge-b850v3-fw: provide an .owner device drm/bridge: nxp-ptn3460: provide an .owner device drm/bridge: panel: provide an .owner device drm/bridge: ps8622: provide an .owner device drm/bridge: sii902x: provide an .owner device drm/bridge: sii9234: provide an .owner device drm/bridge: sii8620: provide an .owner device drm/bridge: synopsys: provide an .owner device for the bridges drm/bridge: tc358767: provide an .owner device drm/bridge: ti-tfp410: provide an .owner device drm/exynos: mic: provide an .owner device for the bridge drm/mediatek: hdmi: provide an .owner device for the bridge drm/msm: specify the .owner of the bridges drm/rcar-du: lvds: provide an .owner device for the bridge drm/sti: provide an .owner device for the bridges drm/bridge: remove the .of_node member drm/bridge: require the .owner to be filled in on drm_bridge_attach drm/bridge: establish a link between the bridge supplier and consumer drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 + drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +- drivers/gpu/drm/bridge/lvds-encoder.c | 2 +- .../drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- drivers/gpu/drm/bridge/panel.c | 4 +--- drivers/gpu/drm/bridge/parade-ps8622.c | 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/sii9234.c | 2 +- drivers/gpu/drm/bridge/sil-sii8620.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +--- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 4 +--- drivers/gpu/drm/bridge/tc358767.c | 2 +- drivers/gpu/drm/bridge/ti-tfp410.c | 2 +- drivers/gpu/drm/drm_bridge.c | 23 +- drivers/gpu/drm/exynos/exynos_drm_mic.c| 2 +- drivers/gpu/drm/mediatek/mtk_hdmi.c| 2 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 1 + drivers/gpu/drm/msm/edp/edp_bridge.c | 1 + drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 1 + drivers/gpu/drm/rcar-du/rcar_lvds.c| 2 +- drivers/gpu/drm/sti/sti_dvo.c | 2 +- drivers/gpu/drm/sti/sti_hda.c | 1 + drivers/gpu/drm/sti/sti_hdmi.c | 1 + include/drm/drm_bridge.h | 8 27 files changed, 51 insertions(+), 33 deletions(-) -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/24] device link, bridge supplier <-> drm device
Hi Peter, On 27.04.2018 00:31, Peter Rosin wrote: > Hi! > > It was noted by Russel King [1] that bridges (not using components) > might disappear unexpectedly if the owner of the bridge was unbound. > Jyri Sarha had previously noted the same thing with panels [2]. Jyri > came up with using device links to resolve the panel issue, which > was also my (independent) reaction to the note from Russel. > > This series builds up to the addition of that link in the last > patch, but in my opinion the other 23 patches do have merit on their > own. > > The last patch needs testing, while the others look trivial. That > said, I might have missed some subtlety. of_node is used as an identifier of the bridge in the kernel. If you replace it with device pointer there will be potential problem with devices having two or more bridges, how do you differentiate bridges if the owner is the same? If I remember correctly current bridge code does not allow to have multiple bridges in one device, but that should be quite easy to fix if necessary. After this change it will become more difficult. Anyway I remember discussion that in DT world bridge should be identified rather by of_graph port node, not by parent node as it is now. If you want to translate this relation to device owner, you should add also port number to have full identification of the bridge, ie pair (owner, port_number) would be equivalent of port node. Regards Andrzej > > Cheers, > Peter > > [1] https://lkml.org/lkml/2018/4/23/769 > [2] https://www.spinics.net/lists/dri-devel/msg174275.html > > Peter Rosin (24): > drm/bridge: allow optionally specifying an .owner device > drm/bridge: adv7511: provide an .owner device > drm/bridge/analogix: core: specify the .owner of the bridge > drm/bridge: analogix-anx78xx: provide an .owner device > drm/bridge: vga-dac: provide an .owner device > drm/bridge: lvds-encoder: provide an .owner device > drm/bridge: megachips-stdp-ge-b850v3-fw: provide an .owner device > drm/bridge: nxp-ptn3460: provide an .owner device > drm/bridge: panel: provide an .owner device > drm/bridge: ps8622: provide an .owner device > drm/bridge: sii902x: provide an .owner device > drm/bridge: sii9234: provide an .owner device > drm/bridge: sii8620: provide an .owner device > drm/bridge: synopsys: provide an .owner device for the bridges > drm/bridge: tc358767: provide an .owner device > drm/bridge: ti-tfp410: provide an .owner device > drm/exynos: mic: provide an .owner device for the bridge > drm/mediatek: hdmi: provide an .owner device for the bridge > drm/msm: specify the .owner of the bridges > drm/rcar-du: lvds: provide an .owner device for the bridge > drm/sti: provide an .owner device for the bridges > drm/bridge: remove the .of_node member > drm/bridge: require the .owner to be filled in on drm_bridge_attach > drm/bridge: establish a link between the bridge supplier and consumer > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 + > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + > drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +- > drivers/gpu/drm/bridge/lvds-encoder.c | 2 +- > .../drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- > drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- > drivers/gpu/drm/bridge/panel.c | 4 +--- > drivers/gpu/drm/bridge/parade-ps8622.c | 2 +- > drivers/gpu/drm/bridge/sii902x.c | 2 +- > drivers/gpu/drm/bridge/sii9234.c | 2 +- > drivers/gpu/drm/bridge/sil-sii8620.c | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +--- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 4 +--- > drivers/gpu/drm/bridge/tc358767.c | 2 +- > drivers/gpu/drm/bridge/ti-tfp410.c | 2 +- > drivers/gpu/drm/drm_bridge.c | 23 > +- > drivers/gpu/drm/exynos/exynos_drm_mic.c| 2 +- > drivers/gpu/drm/mediatek/mtk_hdmi.c| 2 +- > drivers/gpu/drm/msm/dsi/dsi_manager.c | 1 + > drivers/gpu/drm/msm/edp/edp_bridge.c | 1 + > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 1 + > drivers/gpu/drm/rcar-du/rcar_lvds.c| 2 +- > drivers/gpu/drm/sti/sti_dvo.c | 2 +- > drivers/gpu/drm/sti/sti_hda.c | 1 + > drivers/gpu/drm/sti/sti_hdmi.c | 1 + > include/drm/drm_bridge.h | 8 > 27 files changed, 51 insertions(+), 33 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/24] device link, bridge supplier <-> drm device
Hi Peter, On Friday, 27 April 2018 02:09:14 EEST Peter Rosin wrote: > On 2018-04-27 00:40, Laurent Pinchart wrote: > > On Friday, 27 April 2018 01:31:15 EEST Peter Rosin wrote: > >> Hi! > >> > >> It was noted by Russel King [1] that bridges (not using components) > >> might disappear unexpectedly if the owner of the bridge was unbound. > >> Jyri Sarha had previously noted the same thing with panels [2]. Jyri > >> came up with using device links to resolve the panel issue, which > >> was also my (independent) reaction to the note from Russel. > >> > >> This series builds up to the addition of that link in the last > >> patch, but in my opinion the other 23 patches do have merit on their > >> own. > >> > >> The last patch needs testing, while the others look trivial. That > >> said, I might have missed some subtlety. > > > > I don't think this is the way to go. We have a real lifetime management > > problem with bridges, and device links are just trying to hide the problem > > under the carpet. They will further corner us by making a real fix much > > more difficult to implement. I'll try to comment further in the next few > > days on what I think a better solution would be, but in a nutshell I > > believe that drm_bridge objects need to be refcounted, with a .release() > > operation to free the bridge resources when the reference count drops to > > zero. This shouldn't be difficult to implement and I'm willing to help. > > Ok, sp 24/24 is dead, and maybe 23/24 too. Not necessarily, maybe I'll get proven wrong :-) Let's discuss, but I first need some sleep. > But how do you feel about dropping .of_node in favour of .owner? That gets > rid of ugly #ifdefs... I'll review that part and reply, I have nothing against it on principle at the moment. The more generic the code is, the better in my opinion. We just need to make sure that the OF node we're interested in will always be .owner- >of_node in all cases. > I also have the nagging feeling that .driver_private serves very little real > purpose if there is a .owner so that you can do > > dev_get_drvdata(bridge->owner) > > for the cases where the container_of macro is not appropriate. I'll review that too, it's a good point. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/24] device link, bridge supplier <-> drm device
Hi Peter, Thank you for the patches. On Friday, 27 April 2018 01:31:15 EEST Peter Rosin wrote: > Hi! > > It was noted by Russel King [1] that bridges (not using components) > might disappear unexpectedly if the owner of the bridge was unbound. > Jyri Sarha had previously noted the same thing with panels [2]. Jyri > came up with using device links to resolve the panel issue, which > was also my (independent) reaction to the note from Russel. > > This series builds up to the addition of that link in the last > patch, but in my opinion the other 23 patches do have merit on their > own. > > The last patch needs testing, while the others look trivial. That > said, I might have missed some subtlety. I don't think this is the way to go. We have a real lifetime management problem with bridges, and device links are just trying to hide the problem under the carpet. They will further corner us by making a real fix much more difficult to implement. I'll try to comment further in the next few days on what I think a better solution would be, but in a nutshell I believe that drm_bridge objects need to be refcounted, with a .release() operation to free the bridge resources when the reference count drops to zero. This shouldn't be difficult to implement and I'm willing to help. > [1] https://lkml.org/lkml/2018/4/23/769 > [2] https://www.spinics.net/lists/dri-devel/msg174275.html > > Peter Rosin (24): > drm/bridge: allow optionally specifying an .owner device > drm/bridge: adv7511: provide an .owner device > drm/bridge/analogix: core: specify the .owner of the bridge > drm/bridge: analogix-anx78xx: provide an .owner device > drm/bridge: vga-dac: provide an .owner device > drm/bridge: lvds-encoder: provide an .owner device > drm/bridge: megachips-stdp-ge-b850v3-fw: provide an .owner device > drm/bridge: nxp-ptn3460: provide an .owner device > drm/bridge: panel: provide an .owner device > drm/bridge: ps8622: provide an .owner device > drm/bridge: sii902x: provide an .owner device > drm/bridge: sii9234: provide an .owner device > drm/bridge: sii8620: provide an .owner device > drm/bridge: synopsys: provide an .owner device for the bridges > drm/bridge: tc358767: provide an .owner device > drm/bridge: ti-tfp410: provide an .owner device > drm/exynos: mic: provide an .owner device for the bridge > drm/mediatek: hdmi: provide an .owner device for the bridge > drm/msm: specify the .owner of the bridges > drm/rcar-du: lvds: provide an .owner device for the bridge > drm/sti: provide an .owner device for the bridges > drm/bridge: remove the .of_node member > drm/bridge: require the .owner to be filled in on drm_bridge_attach > drm/bridge: establish a link between the bridge supplier and consumer > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 + > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + > drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +- > drivers/gpu/drm/bridge/lvds-encoder.c | 2 +- > .../drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- > drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- > drivers/gpu/drm/bridge/panel.c | 4 +--- > drivers/gpu/drm/bridge/parade-ps8622.c | 2 +- > drivers/gpu/drm/bridge/sii902x.c | 2 +- > drivers/gpu/drm/bridge/sii9234.c | 2 +- > drivers/gpu/drm/bridge/sil-sii8620.c | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +--- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 4 +--- > drivers/gpu/drm/bridge/tc358767.c | 2 +- > drivers/gpu/drm/bridge/ti-tfp410.c | 2 +- > drivers/gpu/drm/drm_bridge.c | 23 ++- > drivers/gpu/drm/exynos/exynos_drm_mic.c| 2 +- > drivers/gpu/drm/mediatek/mtk_hdmi.c| 2 +- > drivers/gpu/drm/msm/dsi/dsi_manager.c | 1 + > drivers/gpu/drm/msm/edp/edp_bridge.c | 1 + > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 1 + > drivers/gpu/drm/rcar-du/rcar_lvds.c| 2 +- > drivers/gpu/drm/sti/sti_dvo.c | 2 +- > drivers/gpu/drm/sti/sti_hda.c | 1 + > drivers/gpu/drm/sti/sti_hdmi.c | 1 + > include/drm/drm_bridge.h | 8 > 27 files changed, 51 insertions(+), 33 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
