Hi Andrzej, On Wednesday 30 Nov 2016 10:26:24 Andrzej Hajda wrote: > On 30.11.2016 09:16, Laurent Pinchart wrote: > > On Wednesday 30 Nov 2016 09:11:56 Andrzej Hajda wrote: > >> On 29.11.2016 22:12, Jyri Sarha wrote: > >>> Store the module that provides the bridge and adjust its refcount > >>> accordingly. The bridge module unload should not be allowed while the > >>> bridge is attached. > >> > >> Module refcounting as a way of preventing driver from unbinding > >> is quite popular, but as other developers said already is incorrect > >> solution - it does not really prevent from unbinding and is a hack. > > > > Absolutely, module refcounting must not be used as a way to prevent > > unbinding, but it's still necessary to prevent functions from > > disappearing. The unbinding race has to be fixed through reference > > counting to prevent objects from being freed, but if an object contains > > function pointers that refer to code part of a module, object refcounting > > won't prevent the code from being removed. Only module refcounting helps > > there. The two techniques are thus complementary. > > It is not true. There at least two existing and proper solutions, which > do not use refcounting: > > 1. components - if you put the bridge into component framework, it will > bring down drm device before detaching the bridge. Don't get me started on that one. The component concept is fine, its implementation less so. It's on my (long) to-do list of things to fix. > 2. proper callbacks from the provider (bridge in this case) to the > consumer (encoder or drm device). Such callbacks exists for example in > case of MIPI-DSI panels attached to encoders. On removal > panel calls mipi_dsi_detach, which calls .detach ops in associated > encoder, encoder safely disables the video pipeline and the panel > continue its removal. *safely* is the keyword here. I have yet to see a solution based on a removal notification callback that is both race-free and easy to use in drivers. > Of course both solutions have some pitfalls, the first one removes whole > drmdev instead of disabling one pipeline, The DRM subsystem doesn't have hotplug support (except for displayed connected to connectors of course), let alone hot-unplug support. That should be fixed, but will be a very long term goal. Regardless of that, the component framework also relies on a removal notification callback, which has the same drawback as the DSI one. Handling this in a race-free way is complex. Seeing how drivers fail at simple things (such as calling to drm_bridge_detach(), which all but one driver fail to do), I'd be surprised if even a single driver got the component unbind handling right. > the second one is specific for DSI bus, but they clearly shows refcounting > is not the only option. Sorry, I meant the only working and (more or less) simple option ;-) -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
