[0/5] sysfb: Fix memory-region management

Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug
operation when removing fbdev firmware drivers.

After being unloaded by a hardware driver, simplefb leaves behind the
firmware framebuffer's platform device. This prevents other drivers
from acquiring the memory as reported at [1].

Patch 1 changes the removal code of remove_conflicting_framebuffers()
to remove the underlying device and the rsp memory region.

Patches 2 to 4 update sysfb and its drivers. The sysfb code does no
longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the
device drivers acquire the memory when they probe the device.

Patch 5 adds a todo item to acquire memory regions in all DRM drivers.

Tested with simpledrm and simplefb.

[1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/

Javier Martinez Canillas (1):
  drivers/firmware: Don't mark as busy the simple-framebuffer IO
    resource

Thomas Zimmermann (4):
  fbdev: Hot-unplug firmware fb devices on forced removal
  drm/simpledrm: Request memory region in driver
  fbdev/simplefb: Request memory region in driver
  drm: Add TODO item for requesting memory regions

 Documentation/gpu/todo.rst        | 15 ++++++++
 drivers/firmware/sysfb_simplefb.c |  2 +-
 drivers/gpu/drm/tiny/simpledrm.c  | 20 ++++++++---
 drivers/video/fbdev/core/fbmem.c  | 29 +++++++++++++--
 drivers/video/fbdev/simplefb.c    | 59 ++++++++++++++++++++++---------
 include/linux/fb.h                |  1 +
 6 files changed, 100 insertions(+), 26 deletions(-)


base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f

On 1/24/22 14:52, Javier Martinez Canillas wrote:

[snip]

>> @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer);
>>  void
>>  unregister_framebuffer(struct fb_info *fb_info)
>>  {
>> -	mutex_lock(&registration_lock);
>> +	bool forced_out = fb_info->forced_out;
>> +
>> +	if (!forced_out)
>> +		mutex_lock(&registration_lock);
>>  	do_unregister_framebuffer(fb_info);
>> -	mutex_unlock(&registration_lock);
>> +	if (!forced_out)
>> +		mutex_unlock(&registration_lock);
>>  }
> 
> I'm not sure to follow the logic here. The forced_out bool is set when the
> platform device is unregistered in do_remove_conflicting_framebuffers(),
> but shouldn't the struct platform_driver .remove callback be executed even
> in this case ?
> 
> That is, the platform_device_unregister() will trigger the call to the
> .remove callback that in turn will call unregister_framebuffer().
> 
> Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ?
> 

Scratch that, I got it now. That's exactly the reason why you skip the
mutext_lock(). After adding the check for dev, feel free to add:

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Best regards,

On 1/24/22 13:36, Thomas Zimmermann wrote:
> Add a TODO item about requesting  memory regions for each driver. The
> current DRM drivers don't do this consistently.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Best regards,

On Mon, 2022-01-24 at 13:36 +0100, Thomas Zimmermann wrote:
> From: Javier Martinez Canillas <javierm@redhat.com>
> 
> The sysfb_create_simplefb() function requests a IO memory resource for
> the
> simple-framebuffer platform device, but it also marks it as busy which
> can
> lead to drivers requesting the same memory resource to fail.
> 
> Let's drop the IORESOURCE_BUSY flag and let drivers to request it as
> busy
> instead.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Zack Rusin <zackr@vmware.com>