On Wed, May 11, 2022 at 01:31:44PM +0200, Javier Martinez Canillas wrote: > The platform devices registered by sysfb match with firmware-based DRM or > fbdev drivers, that are used to have early graphics using a framebuffer > provided by the system firmware. > > DRM or fbdev drivers later are probed and remove all conflicting framebuffers, > leading to these platform devices for generic drivers to be unregistered. > > But the current solution has a race, since the sysfb_init() function could > be called after a DRM or fbdev driver is probed and request to unregister > the devices for drivers with conflicting framebuffes. > > To prevent this, disable any future sysfb platform device registration by > calling sysfb_disable(), if a driver requests to remove the conflicting > framebuffers. > > Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > > Changes in v5: > - Move the sysfb_disable() call at conflicting framebuffers again to > avoid the need of a DRIVER_FIRMWARE capability flag. > - Add Daniel Vetter's Reviewed-by tag again since reverted to the old > patch that he already reviewed in v2. > > Changes in v3: > - Call sysfb_disable() when a DRM dev and a fbdev are registered rather > than when conflicting framebuffers are removed (Thomas Zimmermann). > - Call sysfb_disable() when a fbdev framebuffer is registered rather > than when conflicting framebuffers are removed (Thomas Zimmermann). > - Drop Daniel Vetter's Reviewed-by tag since patch changed a lot. > > Changes in v2: > - Explain in the commit message that fbmem has to unregister the device > as fallback if a driver registered the device itself (Daniel Vetter). > - Also explain that fallback in a comment in the code (Daniel Vetter). > - Don't encode in fbmem the assumption that sysfb will always register > platform devices (Daniel Vetter). > - Add a FIXME comment about drivers registering devices (Daniel Vetter). > > drivers/video/fbdev/core/fbmem.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index 9b035ef4d552..265efa189bcc 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1789,6 +1789,17 @@ int remove_conflicting_framebuffers(struct apertures_struct *a, > if (do_free) > kfree(a); > > + /* > + * If a driver asked to unregister a platform device registered by > + * sysfb, then can be assumed that this is a driver for a display > + * that is set up by the system firmware and has a generic driver. > + * > + * Drivers for devices that don't have a generic driver will never > + * ask for this, so let's assume that a real driver for the display > + * was already probed and prevent sysfb to register devices later. > + */ > + sysfb_disable(); So the og version had (or should have had at least) the sysfb_disable() call before we go through the loop and try to unregister stuff. I think this needs to be done before we call do_remove_conflicting_framebuffer() instead. With that: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > + > return 0; > } > EXPORT_SYMBOL(remove_conflicting_framebuffers); > -- > 2.35.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
