Generic fbdev emulation is a DRM client. If possible, it should behave like userspace clients. Therefore it should not run before the driver registered the new DRM device. If the setup function fails, the driver should not report an error.
This is a follow-up patchset to the discussion at [1]. I went through all calls to drm_fbdev_generic_setup(), moved them to the final operation of their driver's probe function, and removed the return value.
Built on x86-64, aarch64 and arm. Tested with mgag200.
v2: * warn in drm_fbdev_generic_setup() if device is unregistered (Jani) * document the new behavior (Sam) * fix mediatek subject (Noralf) * keep kirin patch for now, even though the patched code will probably be removed
Thomas Zimmermann (10): drm/ast: Set up fbdev after registering device; remove error checks drm/hibmc: Remove error check from fbdev setup drm/kirin: Set up fbdev after fully registering device drm/ingenic: Remove error check from fbdev setup drm/mediatek: Remove error check from fbdev setup drm/mgag200: Set up fbdev after registering device; remove error checks drm/tilcdc: Set up fbdev after fully registering device drm/udl: Remove error check from fbdev setup drm/vboxvideo: Set up fbdev after registering device; remove error checks drm/fb-helper: Remove return value from drm_fbdev_generic_setup()
drivers/gpu/drm/ast/ast_drv.c | 3 +++ drivers/gpu/drm/ast/ast_main.c | 5 ---- drivers/gpu/drm/drm_fb_helper.c | 25 ++++++++++--------- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +---- .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 +-- drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +-- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ drivers/gpu/drm/mgag200/mgag200_main.c | 4 --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +-- drivers/gpu/drm/udl/udl_drv.c | 6 +---- drivers/gpu/drm/vboxvideo/vbox_drv.c | 6 ++--- include/drm/drm_fb_helper.h | 5 ++-- 13 files changed, 30 insertions(+), 47 deletions(-)
-- 2.26.0
Generic fbdev support is a DRM client. Set it up after registering the new DRM device. Remove the error checks as the driver's probe function should not depend on a DRM client's state.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/ast/ast_drv.c | 3 +++ drivers/gpu/drm/ast/ast_main.c | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index 30aa73a5d9b72..b7ba22dddcad9 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -32,6 +32,7 @@
#include <drm/drm_crtc_helper.h> #include <drm/drm_drv.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_gem_vram_helper.h> #include <drm/drm_probe_helper.h>
@@ -111,6 +112,8 @@ static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_ast_driver_unload;
+ drm_fbdev_generic_setup(dev, 32); + return 0;
err_ast_driver_unload: diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 18a0a4ce00f6e..e5398e3dabe70 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -30,7 +30,6 @@
#include <drm/drm_atomic_helper.h> #include <drm/drm_crtc_helper.h> -#include <drm/drm_fb_helper.h> #include <drm/drm_gem.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_gem_vram_helper.h> @@ -512,10 +511,6 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
drm_mode_config_reset(dev);
- ret = drm_fbdev_generic_setup(dev, 32); - if (ret) - goto out_free; - return 0; out_free: kfree(ast);
Generic fbdev support is a DRM client. Remove the error check as the driver's probe function should not depend on a DRM client's state.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 79a180ae4509f..a6fd0c29e5b89 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -307,11 +307,7 @@ static int hibmc_load(struct drm_device *dev) /* reset all the states of crtc/plane/encoder/connector */ drm_mode_config_reset(dev);
- ret = drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); - if (ret) { - DRM_ERROR("failed to initialize fbdev: %d\n", ret); - goto err; - } + drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth);
return 0;
Generic fbdev support is a DRM client. Set it up after fully registering the new DRM device.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c index d3145ae877d74..981858cc8d2b5 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c @@ -277,8 +277,6 @@ static int kirin_drm_bind(struct device *dev) if (ret) goto err_kms_cleanup;
- drm_fbdev_generic_setup(drm_dev, 32); - /* connectors should be registered after drm device register */ if (driver_data->register_connects) { ret = kirin_drm_connectors_register(drm_dev); @@ -286,6 +284,8 @@ static int kirin_drm_bind(struct device *dev) goto err_drm_dev_unregister; }
+ drm_fbdev_generic_setup(drm_dev, 32); + return 0;
err_drm_dev_unregister:
Remove the error check from the fbdev setup function. The function will print a warning.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Paul Cercueil paul@crapouillou.net Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index 7f3f869f57b3f..d938f2b1a96f1 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -783,9 +783,7 @@ static int ingenic_drm_probe(struct platform_device *pdev) goto err_devclk_disable; }
- ret = drm_fbdev_generic_setup(drm, 32); - if (ret) - dev_warn(dev, "Unable to start fbdev emulation: %i", ret); + drm_fbdev_generic_setup(drm, 32);
return 0;
Remove the error check from the fbdev setup function. The function will print a warning.
v2: * fix subject line
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 2eaa9080d2505..ce570283b55f7 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -347,9 +347,7 @@ static int mtk_drm_bind(struct device *dev) if (ret < 0) goto err_deinit;
- ret = drm_fbdev_generic_setup(drm, 32); - if (ret) - DRM_ERROR("Failed to initialize fbdev: %d\n", ret); + drm_fbdev_generic_setup(drm, 32);
return 0;
Generic fbdev support is a DRM client. Set it up after registering the new DRM device. Remove the error checks as the driver's probe function should not depend on a DRM client's state.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ drivers/gpu/drm/mgag200/mgag200_main.c | 4 ---- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 7a5bad2f57d70..3298b7ef18b03 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -77,6 +77,8 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mgag200_driver_unload;
+ drm_fbdev_generic_setup(dev, 0); + return 0;
err_mgag200_driver_unload: diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index e278b6a547bde..b680cf47cbb94 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -181,10 +181,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) dev_warn(&dev->pdev->dev, "Could not initialize cursors. Not doing hardware cursors.\n");
- r = drm_fbdev_generic_setup(mdev->dev, 0); - if (r) - goto err_modeset; - return 0;
err_modeset:
Generic fbdev support is a DRM client. Set it up after fully registering the new DRM device.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Acked-by: Jyri Sarha jsarha@ti.com Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 78c1877d13a83..a5e9ee4c7fbf4 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -390,10 +390,9 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) ret = drm_dev_register(ddev, 0); if (ret) goto init_failed; + priv->is_registered = true;
drm_fbdev_generic_setup(ddev, bpp); - - priv->is_registered = true; return 0;
init_failed:
Remove the error check from the fbdev setup function. The driver's probe function should not depend on a DRM client's state.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/udl/udl_drv.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index 1ce2d865c36dc..9cc6d075cb402 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -97,14 +97,10 @@ static int udl_usb_probe(struct usb_interface *interface,
DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
- r = drm_fbdev_generic_setup(&udl->drm, 0); - if (r) - goto err_drm_dev_unregister; + drm_fbdev_generic_setup(&udl->drm, 0);
return 0;
-err_drm_dev_unregister: - drm_dev_unregister(&udl->drm); err_free: drm_dev_put(&udl->drm); return r;
Generic fbdev support is a DRM client. Set it up after registering the new DRM device. Remove the error checks as the driver's probe function should not depend on a DRM client's state.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/vboxvideo/vbox_drv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index d685ec197fa05..282348e071fe3 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -82,14 +82,12 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
- ret = drm_fbdev_generic_setup(&vbox->ddev, 32); - if (ret) - goto err_irq_fini; - ret = drm_dev_register(&vbox->ddev, 0); if (ret) goto err_irq_fini;
+ drm_fbdev_generic_setup(&vbox->ddev, 32); + return 0;
err_irq_fini:
Generic fbdev emulation is a DRM client. Drivers should invoke the setup function, but not depend on its success. Hence remove the return value.
v2: * warn if fbdev device has not been registered yet * document the new behavior * convert the existing warning to the new dev_ interface
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Sam Ravnborg sam@ravnborg.org Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/drm_fb_helper.c | 25 +++++++++++++------------ include/drm/drm_fb_helper.h | 5 +++-- 2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 165c8dab50797..97f5e43837486 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2169,7 +2169,9 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = { * @dev->mode_config.preferred_depth is used if this is zero. * * This function sets up generic fbdev emulation for drivers that supports - * dumb buffers with a virtual address and that can be mmap'ed. + * dumb buffers with a virtual address and that can be mmap'ed. It's supposed + * to run after the DRM driver registered the new DRM device with + * drm_dev_register(). * * Restore, hotplug events and teardown are all taken care of. Drivers that do * suspend/resume need to call drm_fb_helper_set_suspend_unlocked() themselves. @@ -2186,29 +2188,30 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = { * Setup will be retried on the next hotplug event. * * The fbdev is destroyed by drm_dev_unregister(). - * - * Returns: - * Zero on success or negative error code on failure. */ -int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) +void drm_fbdev_generic_setup(struct drm_device *dev, + unsigned int preferred_bpp) { struct drm_fb_helper *fb_helper; int ret;
- WARN(dev->fb_helper, "fb_helper is already set!\n"); + drm_WARN(dev, !dev->registered, "Device has not been registered.\n"); + drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n");
if (!drm_fbdev_emulation) - return 0; + return;
fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL); - if (!fb_helper) - return -ENOMEM; + if (!fb_helper) { + drm_err(dev, "Failed to allocate fb_helper\n"); + return; + }
ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs); if (ret) { kfree(fb_helper); drm_err(dev, "Failed to register client: %d\n", ret); - return ret; + return; }
if (!preferred_bpp) @@ -2222,8 +2225,6 @@ int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
drm_client_register(&fb_helper->client); - - return 0; } EXPORT_SYMBOL(drm_fbdev_generic_setup);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 208dbf87afa3e..fb037be83997d 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -269,7 +269,8 @@ int drm_fb_helper_debug_leave(struct fb_info *info); void drm_fb_helper_lastclose(struct drm_device *dev); void drm_fb_helper_output_poll_changed(struct drm_device *dev);
-int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp); +void drm_fbdev_generic_setup(struct drm_device *dev, + unsigned int preferred_bpp); #else static inline void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, @@ -443,7 +444,7 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev) { }
-static inline int +static inline void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) { return 0;
Hi Thomas.
You missed my ack on the first 9 patches: https://lore.kernel.org/dri-devel/20200407101354.GA12686@ravnborg.org/ Not that it matters as others have acked/reviewed them.
On Wed, Apr 08, 2020 at 10:26:41AM +0200, Thomas Zimmermann wrote:
Generic fbdev emulation is a DRM client. Drivers should invoke the setup function, but not depend on its success. Hence remove the return value.
v2:
- warn if fbdev device has not been registered yet
- document the new behavior
- convert the existing warning to the new dev_ interface
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Sam Ravnborg sam@ravnborg.org Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com
drivers/gpu/drm/drm_fb_helper.c | 25 +++++++++++++------------ include/drm/drm_fb_helper.h | 5 +++-- 2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 165c8dab50797..97f5e43837486 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2169,7 +2169,9 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = {
@dev->mode_config.preferred_depth is used if this is zero.- This function sets up generic fbdev emulation for drivers that supports
- dumb buffers with a virtual address and that can be mmap'ed.
- dumb buffers with a virtual address and that can be mmap'ed. It's supposed
- to run after the DRM driver registered the new DRM device with
- drm_dev_register().
OR maybe be more explicit - something like: drm_fbdev_generic_setup() shall be called after the DRM is registered with drm_dev_register().
Either way is fine.
Sam
- Restore, hotplug events and teardown are all taken care of. Drivers that do
- suspend/resume need to call drm_fb_helper_set_suspend_unlocked() themselves.
@@ -2186,29 +2188,30 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = {
- Setup will be retried on the next hotplug event.
- The fbdev is destroyed by drm_dev_unregister().
- Returns:
*/
- Zero on success or negative error code on failure.
-int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) +void drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp){ struct drm_fb_helper *fb_helper; int ret;
- WARN(dev->fb_helper, "fb_helper is already set!\n");
drm_WARN(dev, !dev->registered, "Device has not been registered.\n");
drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n");
if (!drm_fbdev_emulation)
return 0;
return;fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
- if (!fb_helper)
return -ENOMEM;
if (!fb_helper) {
drm_err(dev, "Failed to allocate fb_helper\n");return;}
ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs); if (ret) { kfree(fb_helper); drm_err(dev, "Failed to register client: %d\n", ret);
return ret;
return;}
if (!preferred_bpp)
@@ -2222,8 +2225,6 @@ int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
drm_client_register(&fb_helper->client);
- return 0;
} EXPORT_SYMBOL(drm_fbdev_generic_setup);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 208dbf87afa3e..fb037be83997d 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -269,7 +269,8 @@ int drm_fb_helper_debug_leave(struct fb_info *info); void drm_fb_helper_lastclose(struct drm_device *dev); void drm_fb_helper_output_poll_changed(struct drm_device *dev);
-int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp); +void drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp);#else static inline void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, @@ -443,7 +444,7 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev) { }
-static inline int +static inline void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) { return 0; -- 2.26.0
Hi Sam
Am 08.04.20 um 10:50 schrieb Sam Ravnborg:
Hi Thomas.
You missed my ack on the first 9 patches: https://lore.kernel.org/dri-devel/20200407101354.GA12686@ravnborg.org/ Not that it matters as others have acked/reviewed them.
This got lost. I'll add you acks. Thanks for taking another look at the patches.
On Wed, Apr 08, 2020 at 10:26:41AM +0200, Thomas Zimmermann wrote:
Generic fbdev emulation is a DRM client. Drivers should invoke the setup function, but not depend on its success. Hence remove the return value.
v2:
- warn if fbdev device has not been registered yet
- document the new behavior
- convert the existing warning to the new dev_ interface
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Sam Ravnborg sam@ravnborg.org Reviewed-by: Noralf Trønnes noralf@tronnes.org Acked-by: Gerd Hoffmann kraxel@redhat.com
drivers/gpu/drm/drm_fb_helper.c | 25 +++++++++++++------------ include/drm/drm_fb_helper.h | 5 +++-- 2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 165c8dab50797..97f5e43837486 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2169,7 +2169,9 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = {
@dev->mode_config.preferred_depth is used if this is zero.- This function sets up generic fbdev emulation for drivers that supports
- dumb buffers with a virtual address and that can be mmap'ed.
- dumb buffers with a virtual address and that can be mmap'ed. It's supposed
- to run after the DRM driver registered the new DRM device with
- drm_dev_register().
OR maybe be more explicit - something like: drm_fbdev_generic_setup() shall be called after the DRM is registered with drm_dev_register().
I think this one is even better.
Best regards Thomas
Either way is fine.
Sam
- Restore, hotplug events and teardown are all taken care of. Drivers that do
- suspend/resume need to call drm_fb_helper_set_suspend_unlocked() themselves.
@@ -2186,29 +2188,30 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = {
- Setup will be retried on the next hotplug event.
- The fbdev is destroyed by drm_dev_unregister().
- Returns:
*/
- Zero on success or negative error code on failure.
-int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) +void drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp){ struct drm_fb_helper *fb_helper; int ret;
- WARN(dev->fb_helper, "fb_helper is already set!\n");
drm_WARN(dev, !dev->registered, "Device has not been registered.\n");
drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n");
if (!drm_fbdev_emulation)
return 0;
return;fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
- if (!fb_helper)
return -ENOMEM;
if (!fb_helper) {
drm_err(dev, "Failed to allocate fb_helper\n");return;}
ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs); if (ret) { kfree(fb_helper); drm_err(dev, "Failed to register client: %d\n", ret);
return ret;
return;}
if (!preferred_bpp)
@@ -2222,8 +2225,6 @@ int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
drm_client_register(&fb_helper->client);
- return 0;
} EXPORT_SYMBOL(drm_fbdev_generic_setup);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 208dbf87afa3e..fb037be83997d 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -269,7 +269,8 @@ int drm_fb_helper_debug_leave(struct fb_info *info); void drm_fb_helper_lastclose(struct drm_device *dev); void drm_fb_helper_output_poll_changed(struct drm_device *dev);
-int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp); +void drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp);#else static inline void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, @@ -443,7 +444,7 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev) { }
-static inline int +static inline void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp) { return 0; -- 2.26.0
dri-devel@lists.freedesktop.org
-
Sam Ravnborg -
Thomas Zimmermann