Hello Laurent,
On 5/2/22 18:17, Laurent Pinchart wrote:
Hi Javier,
Thank you for the patch.
On Mon, May 02, 2022 at 05:39:00PM +0200, Javier Martinez Canillas wrote:
Indicate to fbdev subsystem that the registered framebuffer is provided by the system firmware, so that it can handle accordingly. For example, would unregister the FB devices if asked to remove the conflicting framebuffers.
Add a new DRM_FB_FW field to drm_fbdev_generic_setup() options parameter. Drivers can use this to indicate the FB helper initialization that the FB registered is provided by the firmware, so it can be configured as such.
Suggested-by: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Javier Martinez Canillas javierm@redhat.com Reviewed-by: Thomas Zimmermann tzimmermann@suse.de
(no changes since v1)
drivers/gpu/drm/drm_fb_helper.c | 9 +++++++++ drivers/gpu/drm/tiny/simpledrm.c | 2 +- include/drm/drm_fb_helper.h | 10 ++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index fd0084ad77c3..775e47c5de1f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1891,6 +1891,10 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper, /* don't leak any physical addresses to userspace */ info->flags |= FBINFO_HIDE_SMEM_START;
- /* Indicate that the framebuffer is provided by the firmware */
- if (fb_helper->firmware)
info->flags |= FBINFO_MISC_FIRMWARE;- /* Need to drop locks to avoid recursive deadlock in
- register_framebuffer. This is ok because the only thing left to do is
- register the fbdev emulation instance in kernel_fb_helper_list. */
@@ -2512,6 +2516,8 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = {
- DRM_FB_BPP: bits per pixel for the device. If the field is not set,
- @dev->mode_config.preferred_depth is used instead.
- DRM_FB_FW: if the framebuffer for the device is provided by the
- system firmware.
- This function sets up generic fbdev emulation for drivers that supports
- dumb buffers with a virtual address and that can be mmap'ed.
@@ -2538,6 +2544,7 @@ void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int options) { struct drm_fb_helper *fb_helper; unsigned int preferred_bpp = DRM_FB_GET_OPTION(DRM_FB_BPP, options);
bool firmware = DRM_FB_GET_OPTION(DRM_FB_FW, options); int ret;
drm_WARN(dev, !dev->registered, "Device has not been registered.\n");
@@ -2570,6 +2577,8 @@ void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int options) preferred_bpp = 32; fb_helper->preferred_bpp = preferred_bpp;
- fb_helper->firmware = firmware;
I'd get rid of the local variable and write
I actually considered that but then decided to add a local variable to have both options set in the same place, since I thought that would be easier to read and also consistent with how preferred_bpp is handled.
Maybe I could go the other way around and rework patch 2/3 to also not require a preferred_bpp local variable ? That patch won't be as small as it's now though. -- Best regards,
Javier Martinez Canillas Linux Engineering Red Hat