On Tue, Aug 30, 2016 at 2:30 PM, David Herrmann dh.herrmann@gmail.com wrote:
Hi Rob
On Fri, Aug 26, 2016 at 2:36 PM, Rob Herring robh+dt@kernel.org wrote:
On Thu, Aug 25, 2016 at 7:00 PM, David Herrmann dh.herrmann@gmail.com wrote:
Provide a generic DRM helper that evicts all conflicting firmware framebuffers, devices, and drivers. The new helper is called drm_evict_firmware(), and takes a flagset controlling which firmware to kick out.
This is meant to be used by drivers in their ->probe() callbacks of their parent bus, before calling into drm_dev_register().
Signed-off-by: David Herrmann dh.herrmann@gmail.com
Hey
This is just compile-tested for now. I just want to get some comments on the design. I decided to drop the sysfb infrastructure and rather just use this generic helper. It keeps things simple and should work just fine for all reasonable use-cases.
This will work with SimpleDRM out-of-the-box on x86.
Architectures with dynamic simple-framebuffer devices are not supported yet. I actually have no idea what the strategy there is? Can the DeviceTree people come up with something? Am I supposed to call of_platform_depopulate()?
If of_platform_populate was used to create the device, then yes call of_platform_depopulate. In this case, I think it wasn't. If of_platform_device_create was used, then platform_device_del.
Or of_detach_node()? Or what?
No. Only the struct device and its resources should need to be destroyed. The node should remain.
Looking at drivers/of/{platform,dynamic}.c, I cannot see how this is supposed to work at all. Who protects platform_device_del() from being called in parallel?
Not sure. In parallel to what? On most systems, nodes never go away and on those that do it is only a few things that get hotplugged. That's changing with DT overlays now, so there certainly could be some issues.
Two tasks calling platform_device_del() in parallel on the same device will not work. Meaning, calling of_platform_device_destroy() in parallel does not work either. Same for of_platform_depopulate(). Same for of_node_detach().
Changes to DT nodes and struct device's are completely separate from a DT core perspective ATM. A caller is responsible adding devices when nodes are added, removing the devices, then removing the nodes. The only overlays currently supported require a driver to load them and handle any transitions.
DT is still far from dynamic in the sense that any random node can come and go.
Sure, all those functions are not meant to be called in parallel by multiple tasks. They are rather meant to have a single holder which preferably is the one instantiating and destroying the node/device/foobar. But the framebuffer eviction code somehow needs to trigger the removal, and thus needs some hook, that can be called in parallel by any driver that is loaded.
I can make sure the simple-framebuffer eviction code is locked properly. That'll work, for now. But if someone ends up providing simple-framebuffer devices via overlays or any other dynamic mechanism, they will race the removal.
No doubt someone will come up with some usecase wanting to do that, but IMO that is not a supported usecase and should not be. simple-framebuffer is for providing a firmware setup framebuffer.
And it will be completely non-obvious to them. I really don't want to be responsible to have submitted that code. If anyone cares for proper device hand-over on ARM, please come up with an idea to fix it. Otherwise, I will just limit the simpledrm code to x86.
IOW the device handover code somehow needs to know who was responsible for the instantiation of the simple-framebuffer device, so it can tell them to remove it again. On x86 there is only one place where those can be instantiated. But on OF-based systems, it can be dynamically instantiated in many places right now.
What do you mean by all over the place? It is only in simplefb_init ATM. I haven't looked at what simpledrm is doing, but we can move the device creation to of_platform_default_populate_init if we need a single spot.
Rob