This patchset adds fbdev .last_close and .output_poll_changed helpers to reduce fbdev emulation footprint in drivers.
I don't know which drivers have their own tree or not, so if you want me to apply your patch to drm-misc, please let me know.
I will do a separate patchset for the cma helper drivers.
Noralf.
Changes since version 1: - drm_device.drm_fb_helper_private -> drm_device.fb_helper (Ville)
Noralf Trønnes (15): drm/fb-helper: Handle function NULL argument drm: Add drm_device->fb_helper pointer drm/fb-helper: Add .last_close and .output_poll_changed helpers drm/amdgpu: Use drm_fb_helper_lastclose() and _poll_changed() drm/armada: Use drm_fb_helper_lastclose() and _poll_changed() drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed() drm/gma500: Use drm_fb_helper_lastclose() and _poll_changed() drm/i915: Use drm_fb_helper_output_poll_changed() drm/msm: Use drm_fb_helper_lastclose() and _poll_changed() drm/nouveau: Use drm_fb_helper_output_poll_changed() drm/omap: Use drm_fb_helper_lastclose() and _poll_changed() drm/radeon: Use drm_fb_helper_lastclose() and _poll_changed() drm/rockchip: Use drm_fb_helper_lastclose() and _poll_changed() drm/tegra: Use drm_fb_helper_lastclose() and _poll_changed() staging: vboxvideo: Use drm_fb_helper_lastclose()
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +--- drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 27 ----------- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 -- drivers/gpu/drm/armada/armada_drm.h | 1 - drivers/gpu/drm/armada/armada_drv.c | 8 +--- drivers/gpu/drm/armada/armada_fb.c | 11 +---- drivers/gpu/drm/armada/armada_fbdev.c | 8 ---- drivers/gpu/drm/drm_fb_helper.c | 69 +++++++++++++++++++++++------ drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 +--- drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 -------- drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 2 - drivers/gpu/drm/gma500/framebuffer.c | 9 +--- drivers/gpu/drm/gma500/psb_drv.c | 15 +------ drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 5 --- drivers/gpu/drm/i915/intel_fbdev.c | 8 ---- drivers/gpu/drm/msm/msm_drv.c | 18 +------- drivers/gpu/drm/nouveau/nouveau_display.c | 3 +- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 8 ---- drivers/gpu/drm/nouveau/nouveau_fbcon.h | 2 - drivers/gpu/drm/nouveau/nouveau_vga.c | 3 +- drivers/gpu/drm/nouveau/nv50_display.c | 2 +- drivers/gpu/drm/omapdrm/omap_drv.c | 34 +------------- drivers/gpu/drm/radeon/radeon_display.c | 9 +--- drivers/gpu/drm/radeon/radeon_fb.c | 22 --------- drivers/gpu/drm/radeon/radeon_kms.c | 5 +-- drivers/gpu/drm/radeon/radeon_mode.h | 3 -- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 9 +--- drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 9 +--- drivers/gpu/drm/tegra/drm.c | 13 +----- drivers/gpu/drm/tegra/drm.h | 4 -- drivers/gpu/drm/tegra/fb.c | 14 ------ drivers/staging/vboxvideo/vbox_drv.c | 2 +- drivers/staging/vboxvideo/vbox_drv.h | 1 - drivers/staging/vboxvideo/vbox_main.c | 12 ----- include/drm/drm_device.h | 9 ++++ include/drm/drm_fb_helper.h | 11 +++++ 39 files changed, 106 insertions(+), 297 deletions(-)
Make functions tolerate that the drm_fb_helper argument is NULL. This is useful for drivers that continue probing when fbdev emulation fails and not having to do this check themselves. Update docs for functions that already handles this.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fb_helper.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 116d1f1337c7..954cdd48de92 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -150,6 +150,9 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, { int err;
+ if (!fb_helper) + return 0; + mutex_lock(&fb_helper->lock); err = __drm_fb_helper_add_one_connector(fb_helper, connector); mutex_unlock(&fb_helper->lock); @@ -161,7 +164,7 @@ EXPORT_SYMBOL(drm_fb_helper_add_one_connector); /** * drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev * emulation helper - * @fb_helper: fbdev initialized with drm_fb_helper_init + * @fb_helper: fbdev initialized with drm_fb_helper_init, can be NULL * * This functions adds all the available connectors for use with the given * fb_helper. This is a separate step to allow drivers to freely assign @@ -179,7 +182,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper) struct drm_connector_list_iter conn_iter; int i, ret = 0;
- if (!drm_fbdev_emulation) + if (!drm_fbdev_emulation || !fb_helper) return 0;
mutex_lock(&fb_helper->lock); @@ -245,6 +248,9 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, { int err;
+ if (!fb_helper) + return 0; + mutex_lock(&fb_helper->lock); err = __drm_fb_helper_remove_one_connector(fb_helper, connector); mutex_unlock(&fb_helper->lock); @@ -484,7 +490,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
/** * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration - * @fb_helper: fbcon to restore + * @fb_helper: driver-allocated fbdev helper, can be NULL * * This should be called from driver's drm &drm_driver.lastclose callback * when implementing an fbcon on top of kms using this helper. This ensures that @@ -498,7 +504,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) bool do_delayed; int ret;
- if (!drm_fbdev_emulation) + if (!drm_fbdev_emulation || !fb_helper) return -ENODEV;
if (READ_ONCE(fb_helper->deferred_setup)) @@ -883,7 +889,7 @@ EXPORT_SYMBOL(drm_fb_helper_alloc_fbi);
/** * drm_fb_helper_unregister_fbi - unregister fb_info framebuffer device - * @fb_helper: driver-allocated fbdev helper + * @fb_helper: driver-allocated fbdev helper, can be NULL * * A wrapper around unregister_framebuffer, to release the fb_info * framebuffer device. This must be called before releasing all resources for @@ -898,7 +904,7 @@ EXPORT_SYMBOL(drm_fb_helper_unregister_fbi);
/** * drm_fb_helper_fini - finialize a &struct drm_fb_helper - * @fb_helper: driver-allocated fbdev helper + * @fb_helper: driver-allocated fbdev helper, can be NULL * * This cleans up all remaining resources associated with @fb_helper. Must be * called after drm_fb_helper_unlink_fbi() was called. @@ -937,7 +943,7 @@ EXPORT_SYMBOL(drm_fb_helper_fini);
/** * drm_fb_helper_unlink_fbi - wrapper around unlink_framebuffer - * @fb_helper: driver-allocated fbdev helper + * @fb_helper: driver-allocated fbdev helper, can be NULL * * A wrapper around unlink_framebuffer implemented by fbdev core */ @@ -1138,7 +1144,7 @@ EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit);
/** * drm_fb_helper_set_suspend - wrapper around fb_set_suspend - * @fb_helper: driver-allocated fbdev helper + * @fb_helper: driver-allocated fbdev helper, can be NULL * @suspend: whether to suspend or resume * * A wrapper around fb_set_suspend implemented by fbdev core. @@ -1155,7 +1161,7 @@ EXPORT_SYMBOL(drm_fb_helper_set_suspend); /** * drm_fb_helper_set_suspend_unlocked - wrapper around fb_set_suspend that also * takes the console lock - * @fb_helper: driver-allocated fbdev helper + * @fb_helper: driver-allocated fbdev helper, can be NULL * @suspend: whether to suspend or resume * * A wrapper around fb_set_suspend() that takes the console lock. If the lock @@ -2568,7 +2574,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config); /** * drm_fb_helper_hotplug_event - respond to a hotplug notification by * probing all the outputs attached to the fb - * @fb_helper: the drm_fb_helper + * @fb_helper: driver-allocated fbdev helper, can be NULL * * Scan the connectors attached to the fb_helper and try to put together a * setup after notification of a change in output configuration. @@ -2590,7 +2596,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { int err = 0;
- if (!drm_fbdev_emulation) + if (!drm_fbdev_emulation || !fb_helper) return 0;
mutex_lock(&fb_helper->lock);
drm_fb_helper is *the* way of doing fbdev emulation so add a pointer to struct drm_device. This makes it possible to add callback helpers for .last_close and .output_poll_changed further reducing fbdev emulation footprint in drivers. The pointer is set by drm_fb_helper_init() and cleared by drm_fb_helper_fini().
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fb_helper.c | 13 +++++++++++-- include/drm/drm_device.h | 9 +++++++++ 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 954cdd48de92..19aaca9fea06 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -799,8 +799,10 @@ int drm_fb_helper_init(struct drm_device *dev, struct drm_mode_config *config = &dev->mode_config; int i;
- if (!drm_fbdev_emulation) + if (!drm_fbdev_emulation) { + dev->fb_helper = fb_helper; return 0; + }
if (!max_conn_count) return -EINVAL; @@ -835,6 +837,8 @@ int drm_fb_helper_init(struct drm_device *dev, i++; }
+ dev->fb_helper = fb_helper; + return 0; out_free: drm_fb_helper_crtc_free(fb_helper); @@ -913,7 +917,12 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper) { struct fb_info *info;
- if (!drm_fbdev_emulation || !fb_helper) + if (!fb_helper) + return; + + fb_helper->dev->fb_helper = NULL; + + if (!drm_fbdev_emulation) return;
cancel_work_sync(&fb_helper->resume_work); diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index e21af87a2f3c..7c4fa32f3fc6 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -17,6 +17,7 @@ struct drm_vblank_crtc; struct drm_sg_mem; struct drm_local_map; struct drm_vma_offset_manager; +struct drm_fb_helper;
struct inode;
@@ -185,6 +186,14 @@ struct drm_device { struct drm_vma_offset_manager *vma_offset_manager; /*@} */ int switch_power_state; + + /** + * @fb_helper: + * + * Pointer to the fbdev emulation structure. + * Set by drm_fb_helper_init() and cleared by drm_fb_helper_fini(). + */ + struct drm_fb_helper *fb_helper; };
#endif
This adds helpers for the drm_driver->last_close and the drm_mode_config_funcs->output_poll_changed callbacks.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fb_helper.c | 28 ++++++++++++++++++++++++++++ include/drm/drm_fb_helper.h | 11 +++++++++++ 2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 19aaca9fea06..defd6a76fef3 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2633,6 +2633,34 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) } EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
+/** + * drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev emulation + * @dev: DRM device + * + * This function can be used as the &drm_driver->lastclose callback for drivers + * that only need to call drm_fb_helper_restore_fbdev_mode_unlocked(). + */ +void drm_fb_helper_lastclose(struct drm_device *dev) +{ + drm_fb_helper_restore_fbdev_mode_unlocked(dev->fb_helper); +} +EXPORT_SYMBOL(drm_fb_helper_lastclose); + +/** + * drm_fb_helper_output_poll_changed - DRM mode config .output_poll_changed + * helper for fbdev emulation + * @dev: DRM device + * + * This function can be used as the + * &drm_mode_config_funcs.output_poll_changed callback for drivers that only + * need to call drm_fb_helper_hotplug_event(). + */ +void drm_fb_helper_output_poll_changed(struct drm_device *dev) +{ + drm_fb_helper_hotplug_event(dev->fb_helper); +} +EXPORT_SYMBOL(drm_fb_helper_output_poll_changed); + /* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT) * but the module doesn't depend on any fb console symbols. At least * attempt to load fbcon to avoid leaving the system without a usable console. diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 33fe95927742..877e5b395c02 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -310,6 +310,9 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn); int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector); int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector); + +void drm_fb_helper_lastclose(struct drm_device *dev); +void drm_fb_helper_output_poll_changed(struct drm_device *dev); #else static inline void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, @@ -507,6 +510,14 @@ drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, return 0; }
+static inline void drm_fb_helper_lastclose(struct drm_device *dev) +{ +} + +static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev) +{ +} + #endif
static inline int
This driver can use drm_fb_helper_lastclose() in its .lastclose function. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 ++------- drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 27 --------------------------- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +--- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 ---- 4 files changed, 3 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 6ad243293a78..c41262865616 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -34,6 +34,7 @@ #include <linux/pm_runtime.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_edid.h> +#include <drm/drm_fb_helper.h>
static void amdgpu_flip_callback(struct dma_fence *f, struct dma_fence_cb *cb) { @@ -556,15 +557,9 @@ amdgpu_user_framebuffer_create(struct drm_device *dev, return &amdgpu_fb->base; }
-static void amdgpu_output_poll_changed(struct drm_device *dev) -{ - struct amdgpu_device *adev = dev->dev_private; - amdgpu_fb_output_poll_changed(adev); -} - const struct drm_mode_config_funcs amdgpu_mode_funcs = { .fb_create = amdgpu_user_framebuffer_create, - .output_poll_changed = amdgpu_output_poll_changed + .output_poll_changed = drm_fb_helper_output_poll_changed, };
static const struct drm_prop_enum_list amdgpu_underscan_enum_list[] = diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c index 562930b17a6d..e38bf856f48b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c @@ -288,12 +288,6 @@ static int amdgpufb_create(struct drm_fb_helper *helper, return ret; }
-void amdgpu_fb_output_poll_changed(struct amdgpu_device *adev) -{ - if (adev->mode_info.rfbdev) - drm_fb_helper_hotplug_event(&adev->mode_info.rfbdev->helper); -} - static int amdgpu_fbdev_destroy(struct drm_device *dev, struct amdgpu_fbdev *rfbdev) { struct amdgpu_framebuffer *rfb = &rfbdev->rfb; @@ -397,24 +391,3 @@ bool amdgpu_fbdev_robj_is_fb(struct amdgpu_device *adev, struct amdgpu_bo *robj) return true; return false; } - -void amdgpu_fbdev_restore_mode(struct amdgpu_device *adev) -{ - struct amdgpu_fbdev *afbdev; - struct drm_fb_helper *fb_helper; - int ret; - - if (!adev) - return; - - afbdev = adev->mode_info.rfbdev; - - if (!afbdev) - return; - - fb_helper = &afbdev->helper; - - ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); - if (ret) - DRM_DEBUG("failed to restore crtc mode\n"); -} diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 4fd06f8d9768..db1e320be54a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -785,9 +785,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file */ void amdgpu_driver_lastclose_kms(struct drm_device *dev) { - struct amdgpu_device *adev = dev->dev_private; - - amdgpu_fbdev_restore_mode(adev); + drm_fb_helper_lastclose(dev); vga_switcheroo_process_delayed_switch(); }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index 2af2678ddaf6..9e9cd0e1c558 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -570,10 +570,6 @@ void amdgpu_fbdev_fini(struct amdgpu_device *adev); void amdgpu_fbdev_set_suspend(struct amdgpu_device *adev, int state); int amdgpu_fbdev_total_size(struct amdgpu_device *adev); bool amdgpu_fbdev_robj_is_fb(struct amdgpu_device *adev, struct amdgpu_bo *robj); -void amdgpu_fbdev_restore_mode(struct amdgpu_device *adev); - -void amdgpu_fb_output_poll_changed(struct amdgpu_device *adev); -
int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int bpp, bool tiled);
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Russell King linux@armlinux.org.uk Signed-off-by: Noralf Trønnes noralf@tronnes.org Acked-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_drm.h | 1 - drivers/gpu/drm/armada/armada_drv.c | 8 ++------ drivers/gpu/drm/armada/armada_fb.c | 11 +---------- drivers/gpu/drm/armada/armada_fbdev.c | 8 -------- 4 files changed, 3 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm/armada/armada_drm.h index b064879ecdbd..cc4c557c9f66 100644 --- a/drivers/gpu/drm/armada/armada_drm.h +++ b/drivers/gpu/drm/armada/armada_drm.h @@ -84,7 +84,6 @@ void armada_drm_queue_unref_work(struct drm_device *, extern const struct drm_mode_config_funcs armada_drm_mode_config_funcs;
int armada_fbdev_init(struct drm_device *); -void armada_fbdev_lastclose(struct drm_device *); void armada_fbdev_fini(struct drm_device *);
int armada_overlay_plane_create(struct drm_device *, unsigned long); diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index e857b88a9799..4b11b6b52f1d 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -10,6 +10,7 @@ #include <linux/module.h> #include <linux/of_graph.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_of.h> #include "armada_crtc.h" #include "armada_drm.h" @@ -54,15 +55,10 @@ static struct drm_ioctl_desc armada_ioctls[] = { DRM_IOCTL_DEF_DRV(ARMADA_GEM_PWRITE, armada_gem_pwrite_ioctl, 0), };
-static void armada_drm_lastclose(struct drm_device *dev) -{ - armada_fbdev_lastclose(dev); -} - DEFINE_DRM_GEM_FOPS(armada_drm_fops);
static struct drm_driver armada_drm_driver = { - .lastclose = armada_drm_lastclose, + .lastclose = drm_fb_helper_lastclose, .gem_free_object_unlocked = armada_gem_free_object, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, diff --git a/drivers/gpu/drm/armada/armada_fb.c b/drivers/gpu/drm/armada/armada_fb.c index a38d5a0892a9..ac92bce07ecd 100644 --- a/drivers/gpu/drm/armada/armada_fb.c +++ b/drivers/gpu/drm/armada/armada_fb.c @@ -154,16 +154,7 @@ static struct drm_framebuffer *armada_fb_create(struct drm_device *dev, return ERR_PTR(ret); }
-static void armada_output_poll_changed(struct drm_device *dev) -{ - struct armada_private *priv = dev->dev_private; - struct drm_fb_helper *fbh = priv->fbdev; - - if (fbh) - drm_fb_helper_hotplug_event(fbh); -} - const struct drm_mode_config_funcs armada_drm_mode_config_funcs = { .fb_create = armada_fb_create, - .output_poll_changed = armada_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, }; diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c index a2ce83f84800..2a59db0994b2 100644 --- a/drivers/gpu/drm/armada/armada_fbdev.c +++ b/drivers/gpu/drm/armada/armada_fbdev.c @@ -159,14 +159,6 @@ int armada_fbdev_init(struct drm_device *dev) return ret; }
-void armada_fbdev_lastclose(struct drm_device *dev) -{ - struct armada_private *priv = dev->dev_private; - - if (priv->fbdev) - drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); -} - void armada_fbdev_fini(struct drm_device *dev) { struct armada_private *priv = dev->dev_private;
On Mon, Oct 30, 2017 at 04:39:41PM +0100, Noralf Trønnes wrote:
-static void armada_output_poll_changed(struct drm_device *dev) -{
- struct armada_private *priv = dev->dev_private;
- struct drm_fb_helper *fbh = priv->fbdev;
- if (fbh)
drm_fb_helper_hotplug_event(fbh);-}
const struct drm_mode_config_funcs armada_drm_mode_config_funcs = { .fb_create = armada_fb_create,
- .output_poll_changed = armada_output_poll_changed,
- .output_poll_changed = drm_fb_helper_output_poll_changed,
I think this is fine, because although it gets rid of the NULL checks for the drm_fb_helper structure, the driver will fail to initialise unless priv->fbdev and the fb helper successfully initialises.
It would be nice to mention that in the commit message as to why removing those NULL checks is safe.
In any case,
Acked-by: Russell King rmk+kernel@armlinux.org.uk
Thanks.
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Inki Dae inki.dae@samsung.com Cc: Joonyoung Shim jy0922.shim@samsung.com Cc: Seung-Woo Kim sw0312.kim@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 ++------ drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 ------------------ drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 2 -- 4 files changed, 3 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e651a58c18cf..70f4895ac49c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -16,6 +16,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h>
#include <linux/component.h>
@@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file) file->driver_priv = NULL; }
-static void exynos_drm_lastclose(struct drm_device *dev) -{ - exynos_drm_fbdev_restore_mode(dev); -} - static const struct vm_operations_struct exynos_drm_gem_vm_ops = { .fault = exynos_drm_gem_fault, .open = drm_gem_vm_open, @@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC | DRIVER_RENDER, .open = exynos_drm_open, - .lastclose = exynos_drm_lastclose, + .lastclose = drm_fb_helper_lastclose, .postclose = exynos_drm_postclose, .gem_free_object_unlocked = exynos_drm_gem_free_object, .gem_vm_ops = &exynos_drm_gem_vm_ops, diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c index 8208df56a88f..0faaf829f5bf 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c @@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = { .fb_create = exynos_user_fb_create, - .output_poll_changed = exynos_drm_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = exynos_atomic_check, .atomic_commit = drm_atomic_helper_commit, }; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index dfb66ecf417b..132dd52d0ac7 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev) private->fb_helper = NULL; }
-void exynos_drm_fbdev_restore_mode(struct drm_device *dev) -{ - struct exynos_drm_private *private = dev->dev_private; - - if (!private || !private->fb_helper) - return; - - drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper); -} - -void exynos_drm_output_poll_changed(struct drm_device *dev) -{ - struct exynos_drm_private *private = dev->dev_private; - struct drm_fb_helper *fb_helper = private->fb_helper; - - drm_fb_helper_hotplug_event(fb_helper); -} - void exynos_drm_fbdev_suspend(struct drm_device *dev) { struct exynos_drm_private *private = dev->dev_private; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h index 645d1bb7f665..b33847223a85 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h @@ -19,8 +19,6 @@
int exynos_drm_fbdev_init(struct drm_device *dev); void exynos_drm_fbdev_fini(struct drm_device *dev); -void exynos_drm_fbdev_restore_mode(struct drm_device *dev); -void exynos_drm_output_poll_changed(struct drm_device *dev); void exynos_drm_fbdev_suspend(struct drm_device *drm); void exynos_drm_fbdev_resume(struct drm_device *drm);
On Mon, Oct 30, 2017 at 04:39:42PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Inki Dae inki.dae@samsung.com Cc: Joonyoung Shim jy0922.shim@samsung.com Cc: Seung-Woo Kim sw0312.kim@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 ++------ drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 ------------------ drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 2 -- 4 files changed, 3 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e651a58c18cf..70f4895ac49c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -16,6 +16,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h>
#include <linux/component.h>
@@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file) file->driver_priv = NULL; }
-static void exynos_drm_lastclose(struct drm_device *dev) -{
- exynos_drm_fbdev_restore_mode(dev);
-}
static const struct vm_operations_struct exynos_drm_gem_vm_ops = { .fault = exynos_drm_gem_fault, .open = drm_gem_vm_open, @@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC | DRIVER_RENDER, .open = exynos_drm_open,
- .lastclose = exynos_drm_lastclose,
- .lastclose = drm_fb_helper_lastclose, .postclose = exynos_drm_postclose, .gem_free_object_unlocked = exynos_drm_gem_free_object, .gem_vm_ops = &exynos_drm_gem_vm_ops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c index 8208df56a88f..0faaf829f5bf 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c @@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = { .fb_create = exynos_user_fb_create,
- .output_poll_changed = exynos_drm_output_poll_changed,
- .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = exynos_atomic_check, .atomic_commit = drm_atomic_helper_commit,
}; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index dfb66ecf417b..132dd52d0ac7 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev) private->fb_helper = NULL; }
-void exynos_drm_fbdev_restore_mode(struct drm_device *dev) -{
- struct exynos_drm_private *private = dev->dev_private;
- if (!private || !private->fb_helper)
Not sure this isn't risky, exynos has a strange load sequence ... Probably best if we get an ack from Inki. -Daniel
return;- drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper);
-}
-void exynos_drm_output_poll_changed(struct drm_device *dev) -{
- struct exynos_drm_private *private = dev->dev_private;
- struct drm_fb_helper *fb_helper = private->fb_helper;
- drm_fb_helper_hotplug_event(fb_helper);
-}
void exynos_drm_fbdev_suspend(struct drm_device *dev) { struct exynos_drm_private *private = dev->dev_private; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h index 645d1bb7f665..b33847223a85 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h @@ -19,8 +19,6 @@
int exynos_drm_fbdev_init(struct drm_device *dev); void exynos_drm_fbdev_fini(struct drm_device *dev); -void exynos_drm_fbdev_restore_mode(struct drm_device *dev); -void exynos_drm_output_poll_changed(struct drm_device *dev); void exynos_drm_fbdev_suspend(struct drm_device *drm); void exynos_drm_fbdev_resume(struct drm_device *drm);
-- 2.14.2
2017년 10월 31일 19:28에 Daniel Vetter 이(가) 쓴 글:
On Mon, Oct 30, 2017 at 04:39:42PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Inki Dae inki.dae@samsung.com Cc: Joonyoung Shim jy0922.shim@samsung.com Cc: Seung-Woo Kim sw0312.kim@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 ++------ drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 ------------------ drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 2 -- 4 files changed, 3 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e651a58c18cf..70f4895ac49c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -16,6 +16,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h>
#include <linux/component.h>
@@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file) file->driver_priv = NULL; }
-static void exynos_drm_lastclose(struct drm_device *dev) -{
- exynos_drm_fbdev_restore_mode(dev);
-}
static const struct vm_operations_struct exynos_drm_gem_vm_ops = { .fault = exynos_drm_gem_fault, .open = drm_gem_vm_open, @@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC | DRIVER_RENDER, .open = exynos_drm_open,
- .lastclose = exynos_drm_lastclose,
- .lastclose = drm_fb_helper_lastclose, .postclose = exynos_drm_postclose, .gem_free_object_unlocked = exynos_drm_gem_free_object, .gem_vm_ops = &exynos_drm_gem_vm_ops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c index 8208df56a88f..0faaf829f5bf 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c @@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = { .fb_create = exynos_user_fb_create,
- .output_poll_changed = exynos_drm_output_poll_changed,
- .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = exynos_atomic_check, .atomic_commit = drm_atomic_helper_commit,
}; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index dfb66ecf417b..132dd52d0ac7 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev) private->fb_helper = NULL; }
-void exynos_drm_fbdev_restore_mode(struct drm_device *dev) -{
- struct exynos_drm_private *private = dev->dev_private;
- if (!private || !private->fb_helper)
Not sure this isn't risky, exynos has a strange load sequence ... Probably best if we get an ack from Inki.
I didn't test this patch on real hardware due to below two issues, 1. many warning messages printed out at vblank period. - to finalize atomic flush drm_crtc_arm_vblank_event function is called but Exynos drm driver has no implementation of get_vblank_timestamp and get_scanout_position callbacks. 2. tranferring Panel commands to Panel device - s6e3ha2 Panel device - timed out when exynos_drm_fbdev_restore_mode is called.
So I just looked into this patch and looks good to me. Only a difference between old and new helper functions is above condition - checking if private and private->fb_helper are null or not. And dev->dev_private and private->fb_helper are cleared after calling drm_dev_unregister function which calls drm_fb_helpaer_lastclose function so it must be no problem.
Acked-by: Inki Dae inki.dae@samsung.com
Thanks, Inki Dae
-Daniel
return;- drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper);
-}
-void exynos_drm_output_poll_changed(struct drm_device *dev) -{
- struct exynos_drm_private *private = dev->dev_private;
- struct drm_fb_helper *fb_helper = private->fb_helper;
- drm_fb_helper_hotplug_event(fb_helper);
-}
void exynos_drm_fbdev_suspend(struct drm_device *dev) { struct exynos_drm_private *private = dev->dev_private; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h index 645d1bb7f665..b33847223a85 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h @@ -19,8 +19,6 @@
int exynos_drm_fbdev_init(struct drm_device *dev); void exynos_drm_fbdev_fini(struct drm_device *dev); -void exynos_drm_fbdev_restore_mode(struct drm_device *dev); -void exynos_drm_output_poll_changed(struct drm_device *dev); void exynos_drm_fbdev_suspend(struct drm_device *drm); void exynos_drm_fbdev_resume(struct drm_device *drm);
-- 2.14.2
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Patrik Jakobsson patrik.r.jakobsson@gmail.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/gma500/framebuffer.c | 9 +-------- drivers/gpu/drm/gma500/psb_drv.c | 15 +-------------- 2 files changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 2570c7f647a6..cb0a2ae916e0 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -576,13 +576,6 @@ static void psb_fbdev_fini(struct drm_device *dev) dev_priv->fbdev = NULL; }
-static void psbfb_output_poll_changed(struct drm_device *dev) -{ - struct drm_psb_private *dev_priv = dev->dev_private; - struct psb_fbdev *fbdev = (struct psb_fbdev *)dev_priv->fbdev; - drm_fb_helper_hotplug_event(&fbdev->psb_fb_helper); -} - /** * psb_user_framebuffer_create_handle - add hamdle to a framebuffer * @fb: framebuffer @@ -623,7 +616,7 @@ static void psb_user_framebuffer_destroy(struct drm_framebuffer *fb)
static const struct drm_mode_config_funcs psb_mode_funcs = { .fb_create = psb_user_framebuffer_create, - .output_poll_changed = psbfb_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, };
static void psb_setup_outputs(struct drm_device *dev) diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 37a3be71acd9..7ab4c532f1d4 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -107,19 +107,6 @@ MODULE_DEVICE_TABLE(pci, pciidlist); static const struct drm_ioctl_desc psb_ioctls[] = { };
-static void psb_driver_lastclose(struct drm_device *dev) -{ - int ret; - struct drm_psb_private *dev_priv = dev->dev_private; - struct psb_fbdev *fbdev = dev_priv->fbdev; - - ret = drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->psb_fb_helper); - if (ret) - DRM_DEBUG("failed to restore crtc mode\n"); - - return; -} - static int psb_do_init(struct drm_device *dev) { struct drm_psb_private *dev_priv = dev->dev_private; @@ -479,7 +466,7 @@ static struct drm_driver driver = { DRIVER_MODESET | DRIVER_GEM, .load = psb_driver_load, .unload = psb_driver_unload, - .lastclose = psb_driver_lastclose, + .lastclose = drm_fb_helper_lastclose,
.num_ioctls = ARRAY_SIZE(psb_ioctls), .irq_preinstall = psb_irq_preinstall,
On Mon, Oct 30, 2017 at 04:39:43PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Patrik Jakobsson patrik.r.jakobsson@gmail.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/gma500/framebuffer.c | 9 +-------- drivers/gpu/drm/gma500/psb_drv.c | 15 +-------------- 2 files changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 2570c7f647a6..cb0a2ae916e0 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -576,13 +576,6 @@ static void psb_fbdev_fini(struct drm_device *dev) dev_priv->fbdev = NULL; }
-static void psbfb_output_poll_changed(struct drm_device *dev) -{
- struct drm_psb_private *dev_priv = dev->dev_private;
- struct psb_fbdev *fbdev = (struct psb_fbdev *)dev_priv->fbdev;
- drm_fb_helper_hotplug_event(&fbdev->psb_fb_helper);
-}
/**
- psb_user_framebuffer_create_handle - add hamdle to a framebuffer
- @fb: framebuffer
@@ -623,7 +616,7 @@ static void psb_user_framebuffer_destroy(struct drm_framebuffer *fb)
static const struct drm_mode_config_funcs psb_mode_funcs = { .fb_create = psb_user_framebuffer_create,
- .output_poll_changed = psbfb_output_poll_changed,
- .output_poll_changed = drm_fb_helper_output_poll_changed,
};
static void psb_setup_outputs(struct drm_device *dev) diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 37a3be71acd9..7ab4c532f1d4 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -107,19 +107,6 @@ MODULE_DEVICE_TABLE(pci, pciidlist); static const struct drm_ioctl_desc psb_ioctls[] = { };
-static void psb_driver_lastclose(struct drm_device *dev) -{
- int ret;
- struct drm_psb_private *dev_priv = dev->dev_private;
- struct psb_fbdev *fbdev = dev_priv->fbdev;
- ret = drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->psb_fb_helper);
- if (ret)
DRM_DEBUG("failed to restore crtc mode\n");- return;
-}
static int psb_do_init(struct drm_device *dev) { struct drm_psb_private *dev_priv = dev->dev_private; @@ -479,7 +466,7 @@ static struct drm_driver driver = { DRIVER_MODESET | DRIVER_GEM, .load = psb_driver_load, .unload = psb_driver_unload,
- .lastclose = psb_driver_lastclose,
.lastclose = drm_fb_helper_lastclose,
.num_ioctls = ARRAY_SIZE(psb_ioctls), .irq_preinstall = psb_irq_preinstall,
-- 2.14.2
This driver can use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 5 ----- drivers/gpu/drm/i915/intel_fbdev.c | 8 -------- 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f780f39e0758..b205e2c782bb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state) static const struct drm_mode_config_funcs intel_mode_funcs = { .fb_create = intel_user_framebuffer_create, .get_format_info = intel_get_format_info, - .output_poll_changed = intel_fbdev_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = intel_atomic_check, .atomic_commit = intel_atomic_commit, .atomic_state_alloc = intel_atomic_state_alloc, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 463ed152e6b1..dfcf5ba220e8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev); extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv); extern void intel_fbdev_fini(struct drm_i915_private *dev_priv); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); -extern void intel_fbdev_output_poll_changed(struct drm_device *dev); extern void intel_fbdev_restore_mode(struct drm_device *dev); #else static inline int intel_fbdev_init(struct drm_device *dev) @@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo { }
-static inline void intel_fbdev_output_poll_changed(struct drm_device *dev) -{ -} - static inline void intel_fbdev_restore_mode(struct drm_device *dev) { } diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index f2bb8116227c..35babbadfc5a 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous console_unlock(); }
-void intel_fbdev_output_poll_changed(struct drm_device *dev) -{ - struct intel_fbdev *ifbdev = to_i915(dev)->fbdev; - - if (ifbdev) - drm_fb_helper_hotplug_event(&ifbdev->helper); -} - void intel_fbdev_restore_mode(struct drm_device *dev) { struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
On Mon, Oct 30, 2017 at 04:39:44PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 5 ----- drivers/gpu/drm/i915/intel_fbdev.c | 8 -------- 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f780f39e0758..b205e2c782bb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state) static const struct drm_mode_config_funcs intel_mode_funcs = { .fb_create = intel_user_framebuffer_create, .get_format_info = intel_get_format_info,
- .output_poll_changed = intel_fbdev_output_poll_changed,
- .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = intel_atomic_check, .atomic_commit = intel_atomic_commit, .atomic_state_alloc = intel_atomic_state_alloc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 463ed152e6b1..dfcf5ba220e8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev); extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv); extern void intel_fbdev_fini(struct drm_i915_private *dev_priv); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); -extern void intel_fbdev_output_poll_changed(struct drm_device *dev); extern void intel_fbdev_restore_mode(struct drm_device *dev); #else static inline int intel_fbdev_init(struct drm_device *dev) @@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo { }
-static inline void intel_fbdev_output_poll_changed(struct drm_device *dev) -{ -}
static inline void intel_fbdev_restore_mode(struct drm_device *dev) { } diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index f2bb8116227c..35babbadfc5a 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous console_unlock(); }
-void intel_fbdev_output_poll_changed(struct drm_device *dev) -{
- struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
- if (ifbdev)
This removes the NULL check here, and I think we still need that one for some slightly unclear but probably hilarious reasons.
I guess simplest if you just drop the i915 patch as too tricky. -Daniel
drm_fb_helper_hotplug_event(&ifbdev->helper);-}
void intel_fbdev_restore_mode(struct drm_device *dev) { struct intel_fbdev *ifbdev = to_i915(dev)->fbdev; -- 2.14.2
Den 31.10.2017 11.27, skrev Daniel Vetter:
On Mon, Oct 30, 2017 at 04:39:44PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 5 ----- drivers/gpu/drm/i915/intel_fbdev.c | 8 -------- 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f780f39e0758..b205e2c782bb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state) static const struct drm_mode_config_funcs intel_mode_funcs = { .fb_create = intel_user_framebuffer_create, .get_format_info = intel_get_format_info,
- .output_poll_changed = intel_fbdev_output_poll_changed,
- .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = intel_atomic_check, .atomic_commit = intel_atomic_commit, .atomic_state_alloc = intel_atomic_state_alloc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 463ed152e6b1..dfcf5ba220e8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev); extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv); extern void intel_fbdev_fini(struct drm_i915_private *dev_priv); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); -extern void intel_fbdev_output_poll_changed(struct drm_device *dev); extern void intel_fbdev_restore_mode(struct drm_device *dev); #else static inline int intel_fbdev_init(struct drm_device *dev) @@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo { }
-static inline void intel_fbdev_output_poll_changed(struct drm_device *dev) -{ -}
- static inline void intel_fbdev_restore_mode(struct drm_device *dev) { }
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index f2bb8116227c..35babbadfc5a 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous console_unlock(); }
-void intel_fbdev_output_poll_changed(struct drm_device *dev) -{
- struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
- if (ifbdev)
This removes the NULL check here, and I think we still need that one for some slightly unclear but probably hilarious reasons.
I guess simplest if you just drop the i915 patch as too tricky.
Ok.
Several drivers had this NULL check and I did a verification in the drivers fbdev code that conversion was OK. But I left it to the maintainer to know about any code paths that wasn't obvious to me by looking at the fbdev code.
On the surface i915 looks OK and here's the rationale:
dev->fb_helper is set in drm_fb_helper_init() and cleared in drm_fb_helper_fini(). All fbdev init error paths I've seen call drm_fb_helper_fini(). This means that dev->fb_helper is only set when fbdev is initialized.
drm_fb_helper_output_poll_changed() can handle dev->fb_helper == NULL:
void drm_fb_helper_output_poll_changed(struct drm_device *dev) { drm_fb_helper_hotplug_event(dev->fb_helper); }
int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { ... if (!drm_fbdev_emulation || !fb_helper) return 0; ... }
dev->fb_helper and dev_priv->fbdev are set and cleared in sync:
int intel_fbdev_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); struct intel_fbdev *ifbdev; int ret;
if (WARN_ON(INTEL_INFO(dev_priv)->num_pipes == 0)) return -ENODEV;
ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL); if (ifbdev == NULL) return -ENOMEM;
drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs);
if (!intel_fbdev_init_bios(dev, ifbdev)) ifbdev->preferred_bpp = 32;
ret = drm_fb_helper_init(dev, &ifbdev->helper, 4); if (ret) { kfree(ifbdev); return ret; }
dev->fb_helper is now set (by drm_fb_helper_init()).
dev_priv->fbdev = ifbdev;
dev_priv->fbdev is now set
INIT_WORK(&dev_priv->fbdev_suspend_work, intel_fbdev_suspend_worker);
drm_fb_helper_single_add_all_connectors(&ifbdev->helper);
return 0; }
void intel_fbdev_fini(struct drm_i915_private *dev_priv) { struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->fbdev);
dev_priv->fbdev is now NULL
if (!ifbdev) return;
intel_fbdev_destroy(ifbdev); }
static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) { /* We rely on the object-free to release the VMA pinning for * the info->screen_base mmaping. Leaking the VMA is simpler than * trying to rectify all the possible error paths leading here. */
drm_fb_helper_fini(&ifbdev->helper);
dev->fb_helper is now NULL (cleared by drm_fb_helper_fini())
if (ifbdev->vma) { mutex_lock(&ifbdev->helper.dev->struct_mutex); intel_unpin_fb_vma(ifbdev->vma); mutex_unlock(&ifbdev->helper.dev->struct_mutex); }
if (ifbdev->fb) drm_framebuffer_remove(&ifbdev->fb->base);
kfree(ifbdev); }
But I understand that it's better to be safe than sorry :-)
Hopefully all your CI work will make checking patches like this a breeze one day.
Noralf.
-Daniel
drm_fb_helper_hotplug_event(&ifbdev->helper);-}
- void intel_fbdev_restore_mode(struct drm_device *dev) { struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
-- 2.14.2
On Tue, Oct 31, 2017 at 03:26:50PM +0100, Noralf Trønnes wrote:
Den 31.10.2017 11.27, skrev Daniel Vetter:
On Mon, Oct 30, 2017 at 04:39:44PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 5 ----- drivers/gpu/drm/i915/intel_fbdev.c | 8 -------- 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f780f39e0758..b205e2c782bb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state) static const struct drm_mode_config_funcs intel_mode_funcs = { .fb_create = intel_user_framebuffer_create, .get_format_info = intel_get_format_info,
- .output_poll_changed = intel_fbdev_output_poll_changed,
- .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = intel_atomic_check, .atomic_commit = intel_atomic_commit, .atomic_state_alloc = intel_atomic_state_alloc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 463ed152e6b1..dfcf5ba220e8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev); extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv); extern void intel_fbdev_fini(struct drm_i915_private *dev_priv); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); -extern void intel_fbdev_output_poll_changed(struct drm_device *dev); extern void intel_fbdev_restore_mode(struct drm_device *dev); #else static inline int intel_fbdev_init(struct drm_device *dev) @@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo { } -static inline void intel_fbdev_output_poll_changed(struct drm_device *dev) -{ -}
- static inline void intel_fbdev_restore_mode(struct drm_device *dev) { }
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index f2bb8116227c..35babbadfc5a 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous console_unlock(); } -void intel_fbdev_output_poll_changed(struct drm_device *dev) -{
- struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
- if (ifbdev)
This removes the NULL check here, and I think we still need that one for some slightly unclear but probably hilarious reasons.
I guess simplest if you just drop the i915 patch as too tricky.
Ok.
Several drivers had this NULL check and I did a verification in the drivers fbdev code that conversion was OK. But I left it to the maintainer to know about any code paths that wasn't obvious to me by looking at the fbdev code.
On the surface i915 looks OK and here's the rationale:
dev->fb_helper is set in drm_fb_helper_init() and cleared in drm_fb_helper_fini(). All fbdev init error paths I've seen call drm_fb_helper_fini(). This means that dev->fb_helper is only set when fbdev is initialized.
drm_fb_helper_output_poll_changed() can handle dev->fb_helper == NULL:
void drm_fb_helper_output_poll_changed(struct drm_device *dev) { drm_fb_helper_hotplug_event(dev->fb_helper); }
int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { ... if (!drm_fbdev_emulation || !fb_helper) return 0; ... }
Argh, I totally missed that your patch series is adding these checks. So looks all good, on the driver patches (except i915):
Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
But probably good if you wait another week or so before pushing those. Core bits imo should land right away, that also gives more options to driver maintainers to push themselves.
dev->fb_helper and dev_priv->fbdev are set and cleared in sync:
int intel_fbdev_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); struct intel_fbdev *ifbdev; int ret;
if (WARN_ON(INTEL_INFO(dev_priv)->num_pipes == 0)) return -ENODEV;
ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL); if (ifbdev == NULL) return -ENOMEM;
drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs);
if (!intel_fbdev_init_bios(dev, ifbdev)) ifbdev->preferred_bpp = 32;
ret = drm_fb_helper_init(dev, &ifbdev->helper, 4); if (ret) { kfree(ifbdev); return ret; }
dev->fb_helper is now set (by drm_fb_helper_init()).
dev_priv->fbdev = ifbdev;
dev_priv->fbdev is now set
INIT_WORK(&dev_priv->fbdev_suspend_work, intel_fbdev_suspend_worker);
drm_fb_helper_single_add_all_connectors(&ifbdev->helper);
return 0; }
void intel_fbdev_fini(struct drm_i915_private *dev_priv) { struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->fbdev);
dev_priv->fbdev is now NULL
if (!ifbdev) return;
intel_fbdev_destroy(ifbdev); }
static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) { /* We rely on the object-free to release the VMA pinning for * the info->screen_base mmaping. Leaking the VMA is simpler than * trying to rectify all the possible error paths leading here. */
drm_fb_helper_fini(&ifbdev->helper);
dev->fb_helper is now NULL (cleared by drm_fb_helper_fini())
if (ifbdev->vma) { mutex_lock(&ifbdev->helper.dev->struct_mutex); intel_unpin_fb_vma(ifbdev->vma); mutex_unlock(&ifbdev->helper.dev->struct_mutex); }
if (ifbdev->fb) drm_framebuffer_remove(&ifbdev->fb->base);
kfree(ifbdev); }
But I understand that it's better to be safe than sorry :-)
Hopefully all your CI work will make checking patches like this a breeze one day.
Already works, just submit the i915-only patch (after you've merged the core work into drm-misc-next and drm-tip is rebuilt) to the intel-gfx mailing list, and CI will pick it up and stress test it for you.
Once that's done the i915 patch is also ready for merge, for that one:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Please include that tag when resubmitting.
Sorry for being a bit blind, I guess coffee didn't quite work yet :-)
Thanks, Daniel
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Rob Clark robdclark@gmail.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/msm/msm_drv.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 606df7bea97b..1bddbbc6fd3e 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -34,16 +34,9 @@ #define MSM_VERSION_MINOR 2 #define MSM_VERSION_PATCHLEVEL 0
-static void msm_fb_output_poll_changed(struct drm_device *dev) -{ - struct msm_drm_private *priv = dev->dev_private; - if (priv->fbdev) - drm_fb_helper_hotplug_event(priv->fbdev); -} - static const struct drm_mode_config_funcs mode_config_funcs = { .fb_create = msm_framebuffer_create, - .output_poll_changed = msm_fb_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = msm_atomic_check, .atomic_commit = msm_atomic_commit, .atomic_state_alloc = msm_atomic_state_alloc, @@ -545,13 +538,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file) kfree(ctx); }
-static void msm_lastclose(struct drm_device *dev) -{ - struct msm_drm_private *priv = dev->dev_private; - if (priv->fbdev) - drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); -} - static irqreturn_t msm_irq(int irq, void *arg) { struct drm_device *dev = arg; @@ -825,7 +811,7 @@ static struct drm_driver msm_driver = { DRIVER_MODESET, .open = msm_open, .postclose = msm_postclose, - .lastclose = msm_lastclose, + .lastclose = drm_fb_helper_lastclose, .irq_handler = msm_irq, .irq_preinstall = msm_irq_preinstall, .irq_postinstall = msm_irq_postinstall,
This driver can use drm_fb_helper_output_poll_changed() instead of its own nouveau_fbcon_output_poll_changed().
Cc: Ben Skeggs bskeggs@redhat.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/nouveau/nouveau_display.c | 3 ++- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 8 -------- drivers/gpu/drm/nouveau/nouveau_fbcon.h | 2 -- drivers/gpu/drm/nouveau/nouveau_vga.c | 3 ++- drivers/gpu/drm/nouveau/nv50_display.c | 2 +- 5 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 2e7785f49e6d..009713404cc4 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -29,6 +29,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h>
#include <nvif/class.h>
@@ -292,7 +293,7 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
static const struct drm_mode_config_funcs nouveau_mode_config_funcs = { .fb_create = nouveau_user_framebuffer_create, - .output_poll_changed = nouveau_fbcon_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, };
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index f7707849bb53..60ca03c27ec4 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -411,14 +411,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, return ret; }
-void -nouveau_fbcon_output_poll_changed(struct drm_device *dev) -{ - struct nouveau_drm *drm = nouveau_drm(dev); - if (drm->fbcon) - drm_fb_helper_hotplug_event(&drm->fbcon->helper); -} - static int nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon) { diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.h b/drivers/gpu/drm/nouveau/nouveau_fbcon.h index e2bca729721e..a6f192ea3fa6 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.h +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.h @@ -68,8 +68,6 @@ void nouveau_fbcon_set_suspend(struct drm_device *dev, int state); void nouveau_fbcon_accel_save_disable(struct drm_device *dev); void nouveau_fbcon_accel_restore(struct drm_device *dev);
-void nouveau_fbcon_output_poll_changed(struct drm_device *dev); - extern int nouveau_nofbaccel;
#endif /* __NV50_FBCON_H__ */ diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c index 48393a4f6331..ac97c30c5bbd 100644 --- a/drivers/gpu/drm/nouveau/nouveau_vga.c +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c @@ -3,6 +3,7 @@
#include <drm/drmP.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h>
#include "nouveau_drv.h" #include "nouveau_acpi.h" @@ -60,7 +61,7 @@ static void nouveau_switcheroo_reprobe(struct pci_dev *pdev) { struct drm_device *dev = pci_get_drvdata(pdev); - nouveau_fbcon_output_poll_changed(dev); + drm_fb_helper_output_poll_changed(dev); }
static bool diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 2dbf62a2ac41..5b3db08e50e4 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -4308,7 +4308,7 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev) static const struct drm_mode_config_funcs nv50_disp_func = { .fb_create = nouveau_user_framebuffer_create, - .output_poll_changed = nouveau_fbcon_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = nv50_disp_atomic_check, .atomic_commit = nv50_disp_atomic_commit, .atomic_state_alloc = nv50_disp_atomic_state_alloc,
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Tomi Valkeinen tomi.valkeinen@ti.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/omapdrm/omap_drv.c | 34 ++-------------------------------- 1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index cdf5b0601eba..96857c508ee0 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -46,14 +46,6 @@ * devices */
-static void omap_fb_output_poll_changed(struct drm_device *dev) -{ - struct omap_drm_private *priv = dev->dev_private; - DBG("dev=%p", dev); - if (priv->fbdev) - drm_fb_helper_hotplug_event(priv->fbdev); -} - static void omap_atomic_wait_for_completion(struct drm_device *dev, struct drm_atomic_state *old_state) { @@ -132,7 +124,7 @@ static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs =
static const struct drm_mode_config_funcs omap_mode_config_funcs = { .fb_create = omap_framebuffer_create, - .output_poll_changed = omap_fb_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, }; @@ -467,28 +459,6 @@ static int dev_open(struct drm_device *dev, struct drm_file *file) return 0; }
-/** - * lastclose - clean up after all DRM clients have exited - * @dev: DRM device - * - * Take care of cleaning up after all DRM clients have exited. In the - * mode setting case, we want to restore the kernel's initial mode (just - * in case the last client left us in a bad state). - */ -static void dev_lastclose(struct drm_device *dev) -{ - struct omap_drm_private *priv = dev->dev_private; - int ret; - - DBG("lastclose: dev=%p", dev); - - if (priv->fbdev) { - ret = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); - if (ret) - DBG("failed to restore crtc mode"); - } -} - static const struct vm_operations_struct omap_gem_vm_ops = { .fault = omap_gem_fault, .open = drm_gem_vm_open, @@ -511,7 +481,7 @@ static struct drm_driver omap_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC | DRIVER_RENDER, .open = dev_open, - .lastclose = dev_lastclose, + .lastclose = drm_fb_helper_lastclose, #ifdef CONFIG_DEBUG_FS .debugfs_init = omap_debugfs_init, #endif
This driver can use drm_fb_helper_lastclose() in its .lastclose function. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/radeon/radeon_display.c | 9 ++------- drivers/gpu/drm/radeon/radeon_fb.c | 22 ---------------------- drivers/gpu/drm/radeon/radeon_kms.c | 5 ++--- drivers/gpu/drm/radeon/radeon_mode.h | 3 --- 4 files changed, 4 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index ddfe91efa61e..dfda5e0ed166 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -32,6 +32,7 @@
#include <linux/pm_runtime.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_plane_helper.h> #include <drm/drm_edid.h>
@@ -1362,15 +1363,9 @@ radeon_user_framebuffer_create(struct drm_device *dev, return &radeon_fb->base; }
-static void radeon_output_poll_changed(struct drm_device *dev) -{ - struct radeon_device *rdev = dev->dev_private; - radeon_fb_output_poll_changed(rdev); -} - static const struct drm_mode_config_funcs radeon_mode_funcs = { .fb_create = radeon_user_framebuffer_create, - .output_poll_changed = radeon_output_poll_changed + .output_poll_changed = drm_fb_helper_output_poll_changed, };
static const struct drm_prop_enum_list radeon_tmds_pll_enum_list[] = diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index 2fcf805d3a16..8a582af52073 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -307,12 +307,6 @@ static int radeonfb_create(struct drm_fb_helper *helper, return ret; }
-void radeon_fb_output_poll_changed(struct radeon_device *rdev) -{ - if (rdev->mode_info.rfbdev) - drm_fb_helper_hotplug_event(&rdev->mode_info.rfbdev->helper); -} - static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfbdev) { struct radeon_framebuffer *rfb = &rfbdev->rfb; @@ -423,19 +417,3 @@ void radeon_fb_remove_connector(struct radeon_device *rdev, struct drm_connector if (rdev->mode_info.rfbdev) drm_fb_helper_remove_one_connector(&rdev->mode_info.rfbdev->helper, connector); } - -void radeon_fbdev_restore_mode(struct radeon_device *rdev) -{ - struct radeon_fbdev *rfbdev = rdev->mode_info.rfbdev; - struct drm_fb_helper *fb_helper; - int ret; - - if (!rfbdev) - return; - - fb_helper = &rfbdev->helper; - - ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); - if (ret) - DRM_DEBUG("failed to restore crtc mode\n"); -} diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index dfee8f7d94ae..e4c1bb8c21fa 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -26,6 +26,7 @@ * Jerome Glisse */ #include <drm/drmP.h> +#include <drm/drm_fb_helper.h> #include "radeon.h" #include <drm/radeon_drm.h> #include "radeon_asic.h" @@ -636,9 +637,7 @@ static int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file */ void radeon_driver_lastclose_kms(struct drm_device *dev) { - struct radeon_device *rdev = dev->dev_private; - - radeon_fbdev_restore_mode(rdev); + drm_fb_helper_lastclose(dev); vga_switcheroo_process_delayed_switch(); }
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index da44ac234f64..567156a7beae 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -988,9 +988,6 @@ int radeon_fbdev_init(struct radeon_device *rdev); void radeon_fbdev_fini(struct radeon_device *rdev); void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state); bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj); -void radeon_fbdev_restore_mode(struct radeon_device *rdev); - -void radeon_fb_output_poll_changed(struct radeon_device *rdev);
void radeon_crtc_handle_vblank(struct radeon_device *rdev, int crtc_id);
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Mark Yao mark.yao@rock-chips.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 9 +-------- drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 9 +-------- 2 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 76d63de5921d..d85431400a0d 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -207,13 +207,6 @@ static void rockchip_drm_unbind(struct device *dev) drm_dev_unref(drm_dev); }
-static void rockchip_drm_lastclose(struct drm_device *dev) -{ - struct rockchip_drm_private *priv = dev->dev_private; - - drm_fb_helper_restore_fbdev_mode_unlocked(&priv->fbdev_helper); -} - static const struct file_operations rockchip_drm_driver_fops = { .owner = THIS_MODULE, .open = drm_open, @@ -228,7 +221,7 @@ static const struct file_operations rockchip_drm_driver_fops = { static struct drm_driver rockchip_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC, - .lastclose = rockchip_drm_lastclose, + .lastclose = drm_fb_helper_lastclose, .gem_vm_ops = &drm_gem_cma_vm_ops, .gem_free_object_unlocked = rockchip_gem_free_object, .dumb_create = rockchip_gem_dumb_create, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index cd2ace0c3caa..e266539e04e5 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -167,20 +167,13 @@ rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, return ERR_PTR(ret); }
-static void rockchip_drm_output_poll_changed(struct drm_device *dev) -{ - struct rockchip_drm_private *private = dev->dev_private; - - drm_fb_helper_hotplug_event(&private->fbdev_helper); -} - static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = { .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, };
static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = { .fb_create = rockchip_user_fb_create, - .output_poll_changed = rockchip_drm_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, };
This driver can use drm_fb_helper_lastclose() as its .lastclose callback. It can also use drm_fb_helper_output_poll_changed() as its .output_poll_changed callback.
Cc: Thierry Reding thierry.reding@gmail.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tegra/drm.c | 13 ++----------- drivers/gpu/drm/tegra/drm.h | 4 ---- drivers/gpu/drm/tegra/fb.c | 14 -------------- 3 files changed, 2 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 597d563d636a..73bca7ac1271 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -120,7 +120,7 @@ static int tegra_atomic_commit(struct drm_device *drm, static const struct drm_mode_config_funcs tegra_drm_mode_funcs = { .fb_create = tegra_fb_create, #ifdef CONFIG_DRM_FBDEV_EMULATION - .output_poll_changed = tegra_fb_output_poll_changed, + .output_poll_changed = drm_fb_helper_output_poll_changed, #endif .atomic_check = drm_atomic_helper_check, .atomic_commit = tegra_atomic_commit, @@ -287,15 +287,6 @@ static void tegra_drm_context_free(struct tegra_drm_context *context) kfree(context); }
-static void tegra_drm_lastclose(struct drm_device *drm) -{ -#ifdef CONFIG_DRM_FBDEV_EMULATION - struct tegra_drm *tegra = drm->dev_private; - - tegra_fbdev_restore_mode(tegra->fbdev); -#endif -} - static struct host1x_bo * host1x_bo_lookup(struct drm_file *file, u32 handle) { @@ -1100,7 +1091,7 @@ static struct drm_driver tegra_drm_driver = { .unload = tegra_drm_unload, .open = tegra_drm_open, .postclose = tegra_drm_postclose, - .lastclose = tegra_drm_lastclose, + .lastclose = drm_fb_helper_lastclose,
#if defined(CONFIG_DEBUG_FS) .debugfs_init = tegra_debugfs_init, diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index 063f5d397526..55b6aff25b9b 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -290,10 +290,6 @@ int tegra_drm_fb_init(struct drm_device *drm); void tegra_drm_fb_exit(struct drm_device *drm); void tegra_drm_fb_suspend(struct drm_device *drm); void tegra_drm_fb_resume(struct drm_device *drm); -#ifdef CONFIG_DRM_FBDEV_EMULATION -void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev); -void tegra_fb_output_poll_changed(struct drm_device *drm); -#endif
extern struct platform_driver tegra_dc_driver; extern struct platform_driver tegra_hdmi_driver; diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c index 80540c1c66dc..8dfe3c6c217e 100644 --- a/drivers/gpu/drm/tegra/fb.c +++ b/drivers/gpu/drm/tegra/fb.c @@ -361,20 +361,6 @@ static void tegra_fbdev_exit(struct tegra_fbdev *fbdev) drm_fb_helper_fini(&fbdev->base); tegra_fbdev_free(fbdev); } - -void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev) -{ - if (fbdev) - drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->base); -} - -void tegra_fb_output_poll_changed(struct drm_device *drm) -{ - struct tegra_drm *tegra = drm->dev_private; - - if (tegra->fbdev) - drm_fb_helper_hotplug_event(&tegra->fbdev->base); -} #endif
int tegra_drm_fb_prepare(struct drm_device *drm)
This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
Cc: Hans de Goede hdegoede@redhat.com Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Hans de Goede hdegoede@redhat.com --- drivers/staging/vboxvideo/vbox_drv.c | 2 +- drivers/staging/vboxvideo/vbox_drv.h | 1 - drivers/staging/vboxvideo/vbox_main.c | 12 ------------ 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c index e18642e5027e..a4d8d7898e3d 100644 --- a/drivers/staging/vboxvideo/vbox_drv.c +++ b/drivers/staging/vboxvideo/vbox_drv.c @@ -229,7 +229,7 @@ static struct drm_driver driver = {
.load = vbox_driver_load, .unload = vbox_driver_unload, - .lastclose = vbox_driver_lastclose, + .lastclose = drm_fb_helper_lastclose, .master_set = vbox_master_set, .master_drop = vbox_master_drop,
diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h index 4b9302703b36..7273d7e9bc9b 100644 --- a/drivers/staging/vboxvideo/vbox_drv.h +++ b/drivers/staging/vboxvideo/vbox_drv.h @@ -128,7 +128,6 @@ struct vbox_private {
int vbox_driver_load(struct drm_device *dev, unsigned long flags); void vbox_driver_unload(struct drm_device *dev); -void vbox_driver_lastclose(struct drm_device *dev);
struct vbox_gem_object;
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..c3d756620fd5 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev) vbox_hw_fini(vbox); }
-/** - * @note this is described in the DRM framework documentation. AST does not - * have it, but we get an oops on driver unload if it is not present. - */ -void vbox_driver_lastclose(struct drm_device *dev) -{ - struct vbox_private *vbox = dev->dev_private; - - if (vbox->fbdev) - drm_fb_helper_restore_fbdev_mode_unlocked(&vbox->fbdev->helper); -} - int vbox_gem_create(struct drm_device *dev, u32 size, bool iskernel, struct drm_gem_object **obj) {
On Mon, Oct 30, 2017 at 04:39:51PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
Cc: Hans de Goede hdegoede@redhat.com Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Hans de Goede hdegoede@redhat.com
drivers/staging/vboxvideo/vbox_drv.c | 2 +- drivers/staging/vboxvideo/vbox_drv.h | 1 - drivers/staging/vboxvideo/vbox_main.c | 12 ------------ 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c index e18642e5027e..a4d8d7898e3d 100644 --- a/drivers/staging/vboxvideo/vbox_drv.c +++ b/drivers/staging/vboxvideo/vbox_drv.c @@ -229,7 +229,7 @@ static struct drm_driver driver = {
.load = vbox_driver_load, .unload = vbox_driver_unload,
- .lastclose = vbox_driver_lastclose,
- .lastclose = drm_fb_helper_lastclose, .master_set = vbox_master_set, .master_drop = vbox_master_drop,
diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h index 4b9302703b36..7273d7e9bc9b 100644 --- a/drivers/staging/vboxvideo/vbox_drv.h +++ b/drivers/staging/vboxvideo/vbox_drv.h @@ -128,7 +128,6 @@ struct vbox_private {
int vbox_driver_load(struct drm_device *dev, unsigned long flags); void vbox_driver_unload(struct drm_device *dev); -void vbox_driver_lastclose(struct drm_device *dev);
struct vbox_gem_object;
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..c3d756620fd5 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev) vbox_hw_fini(vbox); }
-/**
- @note this is described in the DRM framework documentation. AST does not
- have it, but we get an oops on driver unload if it is not present.
- */
-void vbox_driver_lastclose(struct drm_device *dev) -{
- struct vbox_private *vbox = dev->dev_private;
- if (vbox->fbdev)
Hm, except gma500 all the drivers have this NULL check here. I'm not sure whether that's just cargo-culted or whether that's needed, but I'm wary from slapping an Ack on the entire series as-is.
I'm pretty sure we still need it for i915, but everyone else might just have copied too much. Or it predates the helper support for disabling the fbdev emulation.
There's also the question that a bunch of drivers who set up fbdev forgot to add the lastclose call it seems, but that's a different story. -Daniel
drm_fb_helper_restore_fbdev_mode_unlocked(&vbox->fbdev->helper);-}
int vbox_gem_create(struct drm_device *dev, u32 size, bool iskernel, struct drm_gem_object **obj) { -- 2.14.2
Den 31.10.2017 11.32, skrev Daniel Vetter:
On Mon, Oct 30, 2017 at 04:39:51PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
Cc: Hans de Goede hdegoede@redhat.com Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Hans de Goede hdegoede@redhat.com
drivers/staging/vboxvideo/vbox_drv.c | 2 +- drivers/staging/vboxvideo/vbox_drv.h | 1 - drivers/staging/vboxvideo/vbox_main.c | 12 ------------ 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c index e18642e5027e..a4d8d7898e3d 100644 --- a/drivers/staging/vboxvideo/vbox_drv.c +++ b/drivers/staging/vboxvideo/vbox_drv.c @@ -229,7 +229,7 @@ static struct drm_driver driver = {
.load = vbox_driver_load, .unload = vbox_driver_unload,
- .lastclose = vbox_driver_lastclose,
- .lastclose = drm_fb_helper_lastclose, .master_set = vbox_master_set, .master_drop = vbox_master_drop,
diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h index 4b9302703b36..7273d7e9bc9b 100644 --- a/drivers/staging/vboxvideo/vbox_drv.h +++ b/drivers/staging/vboxvideo/vbox_drv.h @@ -128,7 +128,6 @@ struct vbox_private {
int vbox_driver_load(struct drm_device *dev, unsigned long flags); void vbox_driver_unload(struct drm_device *dev); -void vbox_driver_lastclose(struct drm_device *dev);
struct vbox_gem_object;
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..c3d756620fd5 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev) vbox_hw_fini(vbox); }
-/**
- @note this is described in the DRM framework documentation. AST does not
- have it, but we get an oops on driver unload if it is not present.
- */
-void vbox_driver_lastclose(struct drm_device *dev) -{
- struct vbox_private *vbox = dev->dev_private;
- if (vbox->fbdev)
Hm, except gma500 all the drivers have this NULL check here. I'm not sure whether that's just cargo-culted or whether that's needed, but I'm wary from slapping an Ack on the entire series as-is.
I think it's cargo-culted because IIRC almost every driver bails out if fbdev init fails. So the only time the pointer is NULL is when fbdev emulation is compiled out. And in that case the check is not needed.
vboxvideo for instance fails probing if fbdev init fails, and if it succeeds vbox->fbdev is always set.
But anyhow, yeah, I think it's better that the driver maintainers verify this.
Noralf.
I'm pretty sure we still need it for i915, but everyone else might just have copied too much. Or it predates the helper support for disabling the fbdev emulation.
There's also the question that a bunch of drivers who set up fbdev forgot to add the lastclose call it seems, but that's a different story. -Daniel
drm_fb_helper_restore_fbdev_mode_unlocked(&vbox->fbdev->helper);-}
- int vbox_gem_create(struct drm_device *dev, u32 size, bool iskernel, struct drm_gem_object **obj) {
-- 2.14.2
On Tue, Oct 31, 2017 at 03:40:40PM +0100, Noralf Trønnes wrote:
Den 31.10.2017 11.32, skrev Daniel Vetter:
On Mon, Oct 30, 2017 at 04:39:51PM +0100, Noralf Trønnes wrote:
This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
Cc: Hans de Goede hdegoede@redhat.com Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Hans de Goede hdegoede@redhat.com
drivers/staging/vboxvideo/vbox_drv.c | 2 +- drivers/staging/vboxvideo/vbox_drv.h | 1 - drivers/staging/vboxvideo/vbox_main.c | 12 ------------ 3 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c index e18642e5027e..a4d8d7898e3d 100644 --- a/drivers/staging/vboxvideo/vbox_drv.c +++ b/drivers/staging/vboxvideo/vbox_drv.c @@ -229,7 +229,7 @@ static struct drm_driver driver = { .load = vbox_driver_load, .unload = vbox_driver_unload,
- .lastclose = vbox_driver_lastclose,
- .lastclose = drm_fb_helper_lastclose, .master_set = vbox_master_set, .master_drop = vbox_master_drop,
diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h index 4b9302703b36..7273d7e9bc9b 100644 --- a/drivers/staging/vboxvideo/vbox_drv.h +++ b/drivers/staging/vboxvideo/vbox_drv.h @@ -128,7 +128,6 @@ struct vbox_private { int vbox_driver_load(struct drm_device *dev, unsigned long flags); void vbox_driver_unload(struct drm_device *dev); -void vbox_driver_lastclose(struct drm_device *dev); struct vbox_gem_object; diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..c3d756620fd5 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev) vbox_hw_fini(vbox); } -/**
- @note this is described in the DRM framework documentation. AST does not
- have it, but we get an oops on driver unload if it is not present.
- */
-void vbox_driver_lastclose(struct drm_device *dev) -{
- struct vbox_private *vbox = dev->dev_private;
- if (vbox->fbdev)
Hm, except gma500 all the drivers have this NULL check here. I'm not sure whether that's just cargo-culted or whether that's needed, but I'm wary from slapping an Ack on the entire series as-is.
I think it's cargo-culted because IIRC almost every driver bails out if fbdev init fails. So the only time the pointer is NULL is when fbdev emulation is compiled out. And in that case the check is not needed.
vboxvideo for instance fails probing if fbdev init fails, and if it succeeds vbox->fbdev is always set.
But anyhow, yeah, I think it's better that the driver maintainers verify this.
See my other mail, I was blind, I think it's all good. -Daniel
Den 30.10.2017 16.39, skrev Noralf Trønnes:
This patchset adds fbdev .last_close and .output_poll_changed helpers to reduce fbdev emulation footprint in drivers.
I don't know which drivers have their own tree or not, so if you want me to apply your patch to drm-misc, please let me know.
I will do a separate patchset for the cma helper drivers.
Noralf.
Changes since version 1:
- drm_device.drm_fb_helper_private -> drm_device.fb_helper (Ville)
Noralf Trønnes (15): drm/fb-helper: Handle function NULL argument drm: Add drm_device->fb_helper pointer drm/fb-helper: Add .last_close and .output_poll_changed helpers
Core patches 1-3 applied to drm-misc. Thanks for reviewing!
Noralf.
drm/amdgpu: Use drm_fb_helper_lastclose() and _poll_changed() drm/armada: Use drm_fb_helper_lastclose() and _poll_changed() drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed() drm/gma500: Use drm_fb_helper_lastclose() and _poll_changed() drm/i915: Use drm_fb_helper_output_poll_changed() drm/msm: Use drm_fb_helper_lastclose() and _poll_changed() drm/nouveau: Use drm_fb_helper_output_poll_changed() drm/omap: Use drm_fb_helper_lastclose() and _poll_changed() drm/radeon: Use drm_fb_helper_lastclose() and _poll_changed() drm/rockchip: Use drm_fb_helper_lastclose() and _poll_changed() drm/tegra: Use drm_fb_helper_lastclose() and _poll_changed() staging: vboxvideo: Use drm_fb_helper_lastclose()
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +--- drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 27 ----------- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 -- drivers/gpu/drm/armada/armada_drm.h | 1 - drivers/gpu/drm/armada/armada_drv.c | 8 +--- drivers/gpu/drm/armada/armada_fb.c | 11 +---- drivers/gpu/drm/armada/armada_fbdev.c | 8 ---- drivers/gpu/drm/drm_fb_helper.c | 69 +++++++++++++++++++++++------ drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 +--- drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 -------- drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 2 - drivers/gpu/drm/gma500/framebuffer.c | 9 +--- drivers/gpu/drm/gma500/psb_drv.c | 15 +------ drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 5 --- drivers/gpu/drm/i915/intel_fbdev.c | 8 ---- drivers/gpu/drm/msm/msm_drv.c | 18 +------- drivers/gpu/drm/nouveau/nouveau_display.c | 3 +- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 8 ---- drivers/gpu/drm/nouveau/nouveau_fbcon.h | 2 - drivers/gpu/drm/nouveau/nouveau_vga.c | 3 +- drivers/gpu/drm/nouveau/nv50_display.c | 2 +- drivers/gpu/drm/omapdrm/omap_drv.c | 34 +------------- drivers/gpu/drm/radeon/radeon_display.c | 9 +--- drivers/gpu/drm/radeon/radeon_fb.c | 22 --------- drivers/gpu/drm/radeon/radeon_kms.c | 5 +-- drivers/gpu/drm/radeon/radeon_mode.h | 3 -- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 9 +--- drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 9 +--- drivers/gpu/drm/tegra/drm.c | 13 +----- drivers/gpu/drm/tegra/drm.h | 4 -- drivers/gpu/drm/tegra/fb.c | 14 ------ drivers/staging/vboxvideo/vbox_drv.c | 2 +- drivers/staging/vboxvideo/vbox_drv.h | 1 - drivers/staging/vboxvideo/vbox_main.c | 12 ----- include/drm/drm_device.h | 9 ++++ include/drm/drm_fb_helper.h | 11 +++++ 39 files changed, 106 insertions(+), 297 deletions(-)
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Inki Dae -
Noralf Trønnes -
Russell King - ARM Linux