| Message ID | 20230328111422.23986-5-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Convert fbdev to DRM client | expand |
On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Replace all code that initializes or releases fbdev emulation > throughout the driver. Instead initialize the fbdev client by a > single call to i915_fbdev_setup() after i915 has registered its > DRM device. Just like in most drivers, i915 fbdev emulation now > acts like a regular DRM client. > > The fbdev client setup consists of the initial preparation and the > hot-plugging of the display. The latter creates the fbdev device > and sets up the fbdev framebuffer. The setup performs display > hot-plugging once. If no display can be detected, DRM probe helpers > re-run the detection on each hotplug event. > > A call to drm_dev_unregister() releases the client automatically. > No further action is required within i915. If the fbdev framebuffer > has been fully set up, struct fb_ops.fb_destroy implements the > release. For partially initialized emulation, the fbdev client > reverts the initial setup. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> I like the direction in the series, but I'm afraid I don't have the time for proper detailed review of this right now. Just a couple of nits I stumbled on inline. Anyone else have a chance to get this reviewed? BR, Jani. > --- > drivers/gpu/drm/i915/display/intel_display.c | 30 --- > drivers/gpu/drm/i915/display/intel_fbdev.c | 184 +++++++++---------- > drivers/gpu/drm/i915/display/intel_fbdev.h | 20 +- > drivers/gpu/drm/i915/i915_driver.c | 2 + > 4 files changed, 87 insertions(+), 149 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 430a1016e013..1bbeb7a061e3 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -85,7 +85,6 @@ > #include "intel_dvo.h" > #include "intel_fb.h" > #include "intel_fbc.h" > -#include "intel_fbdev.h" > #include "intel_fdi.h" > #include "intel_fifo_underrun.h" > #include "intel_frontbuffer.h" > @@ -8588,11 +8587,6 @@ int intel_modeset_init(struct drm_i915_private *i915) > > intel_overlay_setup(i915); > > - ret = intel_fbdev_init(&i915->drm); > - if (ret) > - return ret; > - > - /* Only enable hotplug handling once the fbdev is fully set up. */ > intel_hpd_init(i915); > intel_hpd_poll_disable(i915); > > @@ -8796,9 +8790,6 @@ void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915) > */ > intel_hpd_poll_fini(i915); > > - /* poll work can call into fbdev, hence clean that up afterwards */ > - intel_fbdev_fini(i915); > - > intel_unregister_dsm_handler(); > > /* flush any delayed tasks or pending work */ > @@ -8864,21 +8855,6 @@ void intel_display_driver_register(struct drm_i915_private *i915) > > intel_display_debugfs_register(i915); > > - /* > - * Some ports require correctly set-up hpd registers for > - * detection to work properly (leading to ghost connected > - * connector status), e.g. VGA on gm45. Hence we can only set > - * up the initial fbdev config after hpd irqs are fully > - * enabled. We do it last so that the async config cannot run > - * before the connectors are registered. > - */ > - intel_fbdev_initial_config_async(i915); > - > - /* > - * We need to coordinate the hotplugs with the asynchronous > - * fbdev configuration, for which we use the > - * fbdev->async_cookie. > - */ > drm_kms_helper_poll_init(&i915->drm); > } > > @@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915) > if (!HAS_DISPLAY(i915)) > return; > > - intel_fbdev_unregister(i915); > intel_audio_deinit(i915); > > - /* > - * After flushing the fbdev (incl. a late async config which > - * will have delayed queuing of a hotplug event), then flush > - * the hotplug events. > - */ > drm_kms_helper_poll_fini(&i915->drm); > drm_atomic_helper_shutdown(&i915->drm); > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c > index bdb9e6f43602..8b618db30ee5 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c > @@ -24,7 +24,6 @@ > * David Airlie > */ > > -#include <linux/async.h> > #include <linux/console.h> > #include <linux/delay.h> > #include <linux/errno.h> > @@ -38,6 +37,7 @@ > #include <linux/vga_switcheroo.h> > > #include <drm/drm_crtc.h> > +#include <drm/drm_crtc_helper.h> > #include <drm/drm_fb_helper.h> > #include <drm/drm_fourcc.h> > > @@ -55,7 +55,6 @@ struct intel_fbdev { > struct intel_framebuffer *fb; > struct i915_vma *vma; > unsigned long vma_flags; > - async_cookie_t cookie; > int preferred_bpp; > > /* Whether or not fbdev hpd processing is temporarily suspended */ > @@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, > return ret; > } > > +static void intel_fbdev_fb_destroy(struct fb_info *info) > +{ > + struct drm_fb_helper *fb_helper = info->par; > + struct intel_fbdev *ifbdev = container_of(fb_helper, struct intel_fbdev, helper); > + > + drm_fb_helper_fini(&ifbdev->helper); > + > + /* > + * 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. > + */ > + intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags); > + drm_framebuffer_remove(&ifbdev->fb->base); > + > + drm_client_release(&fb_helper->client); > + drm_fb_helper_unprepare(&ifbdev->helper); > + kfree(ifbdev); > +} > + > static const struct fb_ops intelfb_ops = { > .owner = THIS_MODULE, > DRM_FB_HELPER_DEFAULT_OPS, > @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = { > .fb_imageblit = drm_fb_helper_cfb_imageblit, > .fb_pan_display = intel_fbdev_pan_display, > .fb_blank = intel_fbdev_blank, > + .fb_destroy = intel_fbdev_fb_destroy, > }; > > static int intelfb_alloc(struct drm_fb_helper *helper, > @@ -198,7 +218,6 @@ static int intelfb_create(struct drm_fb_helper *helper, > struct intel_framebuffer *intel_fb = ifbdev->fb; > struct drm_device *dev = helper->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > - struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt; > const struct i915_gtt_view view = { > .type = I915_GTT_VIEW_NORMAL, > @@ -323,7 +342,7 @@ static int intelfb_create(struct drm_fb_helper *helper, > ifbdev->vma_flags = flags; > > intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref); > - vga_switcheroo_client_fb_set(pdev, info); > + > return 0; > > out_unpin: > @@ -349,26 +368,6 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { > .fb_dirty = intelfb_dirty, > }; > > -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); > - > - if (ifbdev->vma) > - intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags); > - > - if (ifbdev->fb) > - drm_framebuffer_remove(&ifbdev->fb->base); > - > - drm_client_release(&ifbdev->helper.client); > - drm_fb_helper_unprepare(&ifbdev->helper); > - kfree(ifbdev); > -} > - > /* > * Build an intel_fbdev struct using a BIOS allocated framebuffer, if possible. > * The core display code will have read out the current plane configuration, > @@ -532,16 +531,6 @@ static void intel_fbdev_suspend_worker(struct work_struct *work) > true); > } > > -static void intel_fbdev_sync(struct intel_fbdev *ifbdev) > -{ > - if (!ifbdev->cookie) > - return; > - > - /* Only serialises with all preceding async calls, hence +1 */ > - async_synchronize_cookie(ifbdev->cookie + 1); > - ifbdev->cookie = 0; > -} > - > /* Suspends/resumes fbdev processing of incoming HPD events. When resuming HPD > * processing, fbdev will perform a full connector reprobe if a hotplug event > * was received while HPD was suspended. > @@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev) > if (!ifbdev) > return; > > - intel_fbdev_sync(ifbdev); > - > mutex_lock(&ifbdev->hpd_lock); > send_hpd = !ifbdev->hpd_suspended; > ifbdev->hpd_waiting = true; > @@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv) > if (!ifbdev) > return; > > - intel_fbdev_sync(ifbdev); > if (!ifbdev->vma) > return; > > @@ -662,7 +648,20 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv) > */ > > static void intel_fbdev_client_unregister(struct drm_client_dev *client) > -{ } > +{ > + struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); > + struct drm_device *dev = fb_helper->dev; > + struct pci_dev *pdev = to_pci_dev(dev->dev); > + > + if (fb_helper->info) { > + vga_switcheroo_client_fb_set(pdev, NULL); > + drm_fb_helper_unregister_info(fb_helper); > + } else { > + drm_fb_helper_unprepare(fb_helper); > + drm_client_release(&fb_helper->client); > + kfree(fb_helper); > + } > +} > > static int intel_fbdev_client_restore(struct drm_client_dev *client) > { > @@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client) > > static int intel_fbdev_client_hotplug(struct drm_client_dev *client) > { > - intel_fbdev_output_poll_changed(client->dev); > + struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); > + struct drm_device *dev = client->dev; > + struct pci_dev *pdev = to_pci_dev(dev->dev); > + int ret; > + > + if (dev->fb_helper) { > + intel_fbdev_output_poll_changed(dev); > + return 0; > + } > + > + ret = drm_fb_helper_init(dev, fb_helper); > + if (ret) > + goto err_drm_err; > + > + if (!drm_drv_uses_atomic_modeset(dev)) > + drm_helper_disable_unused_functions(dev); > + > + ret = drm_fb_helper_initial_config(fb_helper); > + if (ret) > + goto err_drm_fb_helper_fini; > + > + vga_switcheroo_client_fb_set(pdev, fb_helper->info); > > return 0; > + > +err_drm_fb_helper_fini: > + drm_fb_helper_fini(fb_helper); > +err_drm_err: > + drm_err(dev, "Failed to setup radeon fbdev emulation (ret=%d)\n", ret); ^^^^^^ Hrrmmh. ;) > + return ret; > } > > static const struct drm_client_funcs intel_fbdev_client_funcs = { > @@ -688,22 +714,23 @@ static const struct drm_client_funcs intel_fbdev_client_funcs = { > .hotplug = intel_fbdev_client_hotplug, > }; > > -int intel_fbdev_init(struct drm_device *dev) > +void intel_fbdev_setup(struct drm_i915_private *dev_priv) > { > - struct drm_i915_private *dev_priv = to_i915(dev); > + struct drm_device *dev = &dev_priv->drm; > struct intel_fbdev *ifbdev; > int ret; > > if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv))) > - return -ENODEV; > - > - ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL); > - if (ifbdev == NULL) > - return -ENOMEM; > + return; > > - mutex_init(&ifbdev->hpd_lock); > + ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL); > + if (!ifbdev) > + return; > drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs); > > + dev_priv->display.fbdev.fbdev = ifbdev; > + INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker); > + mutex_init(&ifbdev->hpd_lock); > if (intel_fbdev_init_bios(dev, ifbdev)) > ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp; > else > @@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev) > > ret = drm_client_init(dev, &ifbdev->helper.client, "i915-fbdev", > &intel_fbdev_client_funcs); > - if (ret) > + if (ret) { > + drm_err(dev, "Failed to register client: %d\n", ret); > goto err_drm_fb_helper_unprepare; > + } > > - ret = drm_fb_helper_init(dev, &ifbdev->helper); > + ret = intel_fbdev_client_hotplug(&ifbdev->helper.client); > if (ret) > - goto err_drm_client_release; > + drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); > > - dev_priv->display.fbdev.fbdev = ifbdev; > - INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker); > + drm_client_register(&ifbdev->helper.client); > > - return 0; > + return; > > -err_drm_client_release: > - drm_client_release(&ifbdev->helper.client); > err_drm_fb_helper_unprepare: > - drm_client_release(&ifbdev->helper.client); > + drm_fb_helper_unprepare(&ifbdev->helper); > + mutex_destroy(&ifbdev->hpd_lock); > kfree(ifbdev); > - return ret; > -} > - > -static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) > -{ > - struct intel_fbdev *ifbdev = data; > - > - /* Due to peculiar init order wrt to hpd handling this is separate. */ > - if (drm_fb_helper_initial_config(&ifbdev->helper)) > - intel_fbdev_unregister(to_i915(ifbdev->helper.dev)); > -} > - > -void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv) > -{ > - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev; > - > - if (!ifbdev) > - return; > - > - ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev); > -} > - > -void intel_fbdev_unregister(struct drm_i915_private *dev_priv) > -{ > - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev; > - > - if (!ifbdev) > - return; > - > - intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true); > - > - if (!current_is_async()) > - intel_fbdev_sync(ifbdev); > - > - drm_fb_helper_unregister_info(&ifbdev->helper); > -} > - > -void intel_fbdev_fini(struct drm_i915_private *dev_priv) > -{ > - struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->display.fbdev.fbdev); > - > - if (!ifbdev) > - return; > - > - intel_fbdev_destroy(ifbdev); > } > > struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev) > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h > index 8c953f102ba2..8123c813caaf 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbdev.h > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h > @@ -14,27 +14,11 @@ struct intel_fbdev; > struct intel_framebuffer; > > #ifdef CONFIG_DRM_FBDEV_EMULATION > -int intel_fbdev_init(struct drm_device *dev); > -void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv); > -void intel_fbdev_unregister(struct drm_i915_private *dev_priv); > -void intel_fbdev_fini(struct drm_i915_private *dev_priv); > +void intel_fbdev_setup(struct drm_i915_private *dev_priv); > void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); > struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev); > #else > -static inline int intel_fbdev_init(struct drm_device *dev) > -{ > - return 0; > -} > - > -static inline void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv) > -{ > -} > - > -static inline void intel_fbdev_unregister(struct drm_i915_private *dev_priv) > -{ > -} > - > -static inline void intel_fbdev_fini(struct drm_i915_private *dev_priv) > +void intel_fbdev_setup(struct drm_i915_private *dev_priv); > { > } > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index be804fd4f79f..0b6722bcfbb2 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > i915->do_release = true; > > + intel_fbdev_setup(i915); > + Maybe we could call this from intel_display_driver_register()? We're trying to separate display related stuff from the rest, and call as few display functions from top level driver code as possible. (Clearly, we're nowhere near this goal yet.) > return 0; > > out_cleanup_gem:
Hi Thomas,
I love your patch! Yet something to improve:
[auto build test ERROR on 6e5f96153989e454041848f66a5227be9bd0bbc3]
url: https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-i915-Move-fbdev-functions/20230328-191627
base: 6e5f96153989e454041848f66a5227be9bd0bbc3
patch link: https://lore.kernel.org/r/20230328111422.23986-5-tzimmermann%40suse.de
patch subject: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230329/202303290420.6rd5p4br-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/6db03fde4587438371c6f7871675cba6389a1319
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Thomas-Zimmermann/drm-i915-Move-fbdev-functions/20230328-191627
git checkout 6db03fde4587438371c6f7871675cba6389a1319
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 olddefconfig
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303290420.6rd5p4br-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/i915/i915_driver.c:55:
>> drivers/gpu/drm/i915/display/intel_fbdev.h:22:1: error: expected identifier or '(' before '{' token
22 | {
| ^
vim +22 drivers/gpu/drm/i915/display/intel_fbdev.h
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h Jani Nikula 2019-04-05 15
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h Jani Nikula 2019-04-05 16 #ifdef CONFIG_DRM_FBDEV_EMULATION
6db03fde458743 drivers/gpu/drm/i915/display/intel_fbdev.h Thomas Zimmermann 2023-03-28 17 void intel_fbdev_setup(struct drm_i915_private *dev_priv);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h Jani Nikula 2019-04-05 18 void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
c1c04560ac038d drivers/gpu/drm/i915/display/intel_fbdev.h Jani Nikula 2022-02-15 19 struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h Jani Nikula 2019-04-05 20 #else
6db03fde458743 drivers/gpu/drm/i915/display/intel_fbdev.h Thomas Zimmermann 2023-03-28 21 void intel_fbdev_setup(struct drm_i915_private *dev_priv);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h Jani Nikula 2019-04-05 @22 {
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h Jani Nikula 2019-04-05 23 }
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h Jani Nikula 2019-04-05 24
Hi Am 28.03.23 um 17:31 schrieb Jani Nikula: > On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Replace all code that initializes or releases fbdev emulation >> throughout the driver. Instead initialize the fbdev client by a >> single call to i915_fbdev_setup() after i915 has registered its >> DRM device. Just like in most drivers, i915 fbdev emulation now >> acts like a regular DRM client. >> >> The fbdev client setup consists of the initial preparation and the >> hot-plugging of the display. The latter creates the fbdev device >> and sets up the fbdev framebuffer. The setup performs display >> hot-plugging once. If no display can be detected, DRM probe helpers >> re-run the detection on each hotplug event. >> >> A call to drm_dev_unregister() releases the client automatically. >> No further action is required within i915. If the fbdev framebuffer >> has been fully set up, struct fb_ops.fb_destroy implements the >> release. For partially initialized emulation, the fbdev client >> reverts the initial setup. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > I like the direction in the series, but I'm afraid I don't have the time > for proper detailed review of this right now. Just a couple of nits I > stumbled on inline. Thanks a look for looking at it. I keep this patchset up for a little longer and maybe someone sends in further comments. Otherwise I'll send out an update and ask around on irc. Best regards Thomas > > Anyone else have a chance to get this reviewed? > > BR, > Jani. > > >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 30 --- >> drivers/gpu/drm/i915/display/intel_fbdev.c | 184 +++++++++---------- >> drivers/gpu/drm/i915/display/intel_fbdev.h | 20 +- >> drivers/gpu/drm/i915/i915_driver.c | 2 + >> 4 files changed, 87 insertions(+), 149 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 430a1016e013..1bbeb7a061e3 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -85,7 +85,6 @@ >> #include "intel_dvo.h" >> #include "intel_fb.h" >> #include "intel_fbc.h" >> -#include "intel_fbdev.h" >> #include "intel_fdi.h" >> #include "intel_fifo_underrun.h" >> #include "intel_frontbuffer.h" >> @@ -8588,11 +8587,6 @@ int intel_modeset_init(struct drm_i915_private *i915) >> >> intel_overlay_setup(i915); >> >> - ret = intel_fbdev_init(&i915->drm); >> - if (ret) >> - return ret; >> - >> - /* Only enable hotplug handling once the fbdev is fully set up. */ >> intel_hpd_init(i915); >> intel_hpd_poll_disable(i915); >> >> @@ -8796,9 +8790,6 @@ void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915) >> */ >> intel_hpd_poll_fini(i915); >> >> - /* poll work can call into fbdev, hence clean that up afterwards */ >> - intel_fbdev_fini(i915); >> - >> intel_unregister_dsm_handler(); >> >> /* flush any delayed tasks or pending work */ >> @@ -8864,21 +8855,6 @@ void intel_display_driver_register(struct drm_i915_private *i915) >> >> intel_display_debugfs_register(i915); >> >> - /* >> - * Some ports require correctly set-up hpd registers for >> - * detection to work properly (leading to ghost connected >> - * connector status), e.g. VGA on gm45. Hence we can only set >> - * up the initial fbdev config after hpd irqs are fully >> - * enabled. We do it last so that the async config cannot run >> - * before the connectors are registered. >> - */ >> - intel_fbdev_initial_config_async(i915); >> - >> - /* >> - * We need to coordinate the hotplugs with the asynchronous >> - * fbdev configuration, for which we use the >> - * fbdev->async_cookie. >> - */ >> drm_kms_helper_poll_init(&i915->drm); >> } >> >> @@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915) >> if (!HAS_DISPLAY(i915)) >> return; >> >> - intel_fbdev_unregister(i915); >> intel_audio_deinit(i915); >> >> - /* >> - * After flushing the fbdev (incl. a late async config which >> - * will have delayed queuing of a hotplug event), then flush >> - * the hotplug events. >> - */ >> drm_kms_helper_poll_fini(&i915->drm); >> drm_atomic_helper_shutdown(&i915->drm); >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c >> index bdb9e6f43602..8b618db30ee5 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >> @@ -24,7 +24,6 @@ >> * David Airlie >> */ >> >> -#include <linux/async.h> >> #include <linux/console.h> >> #include <linux/delay.h> >> #include <linux/errno.h> >> @@ -38,6 +37,7 @@ >> #include <linux/vga_switcheroo.h> >> >> #include <drm/drm_crtc.h> >> +#include <drm/drm_crtc_helper.h> >> #include <drm/drm_fb_helper.h> >> #include <drm/drm_fourcc.h> >> >> @@ -55,7 +55,6 @@ struct intel_fbdev { >> struct intel_framebuffer *fb; >> struct i915_vma *vma; >> unsigned long vma_flags; >> - async_cookie_t cookie; >> int preferred_bpp; >> >> /* Whether or not fbdev hpd processing is temporarily suspended */ >> @@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, >> return ret; >> } >> >> +static void intel_fbdev_fb_destroy(struct fb_info *info) >> +{ >> + struct drm_fb_helper *fb_helper = info->par; >> + struct intel_fbdev *ifbdev = container_of(fb_helper, struct intel_fbdev, helper); >> + >> + drm_fb_helper_fini(&ifbdev->helper); >> + >> + /* >> + * 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. >> + */ >> + intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags); >> + drm_framebuffer_remove(&ifbdev->fb->base); >> + >> + drm_client_release(&fb_helper->client); >> + drm_fb_helper_unprepare(&ifbdev->helper); >> + kfree(ifbdev); >> +} >> + >> static const struct fb_ops intelfb_ops = { >> .owner = THIS_MODULE, >> DRM_FB_HELPER_DEFAULT_OPS, >> @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = { >> .fb_imageblit = drm_fb_helper_cfb_imageblit, >> .fb_pan_display = intel_fbdev_pan_display, >> .fb_blank = intel_fbdev_blank, >> + .fb_destroy = intel_fbdev_fb_destroy, >> }; >> >> static int intelfb_alloc(struct drm_fb_helper *helper, >> @@ -198,7 +218,6 @@ static int intelfb_create(struct drm_fb_helper *helper, >> struct intel_framebuffer *intel_fb = ifbdev->fb; >> struct drm_device *dev = helper->dev; >> struct drm_i915_private *dev_priv = to_i915(dev); >> - struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); >> struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt; >> const struct i915_gtt_view view = { >> .type = I915_GTT_VIEW_NORMAL, >> @@ -323,7 +342,7 @@ static int intelfb_create(struct drm_fb_helper *helper, >> ifbdev->vma_flags = flags; >> >> intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref); >> - vga_switcheroo_client_fb_set(pdev, info); >> + >> return 0; >> >> out_unpin: >> @@ -349,26 +368,6 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { >> .fb_dirty = intelfb_dirty, >> }; >> >> -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); >> - >> - if (ifbdev->vma) >> - intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags); >> - >> - if (ifbdev->fb) >> - drm_framebuffer_remove(&ifbdev->fb->base); >> - >> - drm_client_release(&ifbdev->helper.client); >> - drm_fb_helper_unprepare(&ifbdev->helper); >> - kfree(ifbdev); >> -} >> - >> /* >> * Build an intel_fbdev struct using a BIOS allocated framebuffer, if possible. >> * The core display code will have read out the current plane configuration, >> @@ -532,16 +531,6 @@ static void intel_fbdev_suspend_worker(struct work_struct *work) >> true); >> } >> >> -static void intel_fbdev_sync(struct intel_fbdev *ifbdev) >> -{ >> - if (!ifbdev->cookie) >> - return; >> - >> - /* Only serialises with all preceding async calls, hence +1 */ >> - async_synchronize_cookie(ifbdev->cookie + 1); >> - ifbdev->cookie = 0; >> -} >> - >> /* Suspends/resumes fbdev processing of incoming HPD events. When resuming HPD >> * processing, fbdev will perform a full connector reprobe if a hotplug event >> * was received while HPD was suspended. >> @@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev) >> if (!ifbdev) >> return; >> >> - intel_fbdev_sync(ifbdev); >> - >> mutex_lock(&ifbdev->hpd_lock); >> send_hpd = !ifbdev->hpd_suspended; >> ifbdev->hpd_waiting = true; >> @@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv) >> if (!ifbdev) >> return; >> >> - intel_fbdev_sync(ifbdev); >> if (!ifbdev->vma) >> return; >> >> @@ -662,7 +648,20 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv) >> */ >> >> static void intel_fbdev_client_unregister(struct drm_client_dev *client) >> -{ } >> +{ >> + struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); >> + struct drm_device *dev = fb_helper->dev; >> + struct pci_dev *pdev = to_pci_dev(dev->dev); >> + >> + if (fb_helper->info) { >> + vga_switcheroo_client_fb_set(pdev, NULL); >> + drm_fb_helper_unregister_info(fb_helper); >> + } else { >> + drm_fb_helper_unprepare(fb_helper); >> + drm_client_release(&fb_helper->client); >> + kfree(fb_helper); >> + } >> +} >> >> static int intel_fbdev_client_restore(struct drm_client_dev *client) >> { >> @@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client) >> >> static int intel_fbdev_client_hotplug(struct drm_client_dev *client) >> { >> - intel_fbdev_output_poll_changed(client->dev); >> + struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); >> + struct drm_device *dev = client->dev; >> + struct pci_dev *pdev = to_pci_dev(dev->dev); >> + int ret; >> + >> + if (dev->fb_helper) { >> + intel_fbdev_output_poll_changed(dev); >> + return 0; >> + } >> + >> + ret = drm_fb_helper_init(dev, fb_helper); >> + if (ret) >> + goto err_drm_err; >> + >> + if (!drm_drv_uses_atomic_modeset(dev)) >> + drm_helper_disable_unused_functions(dev); >> + >> + ret = drm_fb_helper_initial_config(fb_helper); >> + if (ret) >> + goto err_drm_fb_helper_fini; >> + >> + vga_switcheroo_client_fb_set(pdev, fb_helper->info); >> >> return 0; >> + >> +err_drm_fb_helper_fini: >> + drm_fb_helper_fini(fb_helper); >> +err_drm_err: >> + drm_err(dev, "Failed to setup radeon fbdev emulation (ret=%d)\n", ret); > ^^^^^^ > > Hrrmmh. ;) > >> + return ret; >> } >> >> static const struct drm_client_funcs intel_fbdev_client_funcs = { >> @@ -688,22 +714,23 @@ static const struct drm_client_funcs intel_fbdev_client_funcs = { >> .hotplug = intel_fbdev_client_hotplug, >> }; >> >> -int intel_fbdev_init(struct drm_device *dev) >> +void intel_fbdev_setup(struct drm_i915_private *dev_priv) >> { >> - struct drm_i915_private *dev_priv = to_i915(dev); >> + struct drm_device *dev = &dev_priv->drm; >> struct intel_fbdev *ifbdev; >> int ret; >> >> if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv))) >> - return -ENODEV; >> - >> - ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL); >> - if (ifbdev == NULL) >> - return -ENOMEM; >> + return; >> >> - mutex_init(&ifbdev->hpd_lock); >> + ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL); >> + if (!ifbdev) >> + return; >> drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs); >> >> + dev_priv->display.fbdev.fbdev = ifbdev; >> + INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker); >> + mutex_init(&ifbdev->hpd_lock); >> if (intel_fbdev_init_bios(dev, ifbdev)) >> ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp; >> else >> @@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev) >> >> ret = drm_client_init(dev, &ifbdev->helper.client, "i915-fbdev", >> &intel_fbdev_client_funcs); >> - if (ret) >> + if (ret) { >> + drm_err(dev, "Failed to register client: %d\n", ret); >> goto err_drm_fb_helper_unprepare; >> + } >> >> - ret = drm_fb_helper_init(dev, &ifbdev->helper); >> + ret = intel_fbdev_client_hotplug(&ifbdev->helper.client); >> if (ret) >> - goto err_drm_client_release; >> + drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); >> >> - dev_priv->display.fbdev.fbdev = ifbdev; >> - INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker); >> + drm_client_register(&ifbdev->helper.client); >> >> - return 0; >> + return; >> >> -err_drm_client_release: >> - drm_client_release(&ifbdev->helper.client); >> err_drm_fb_helper_unprepare: >> - drm_client_release(&ifbdev->helper.client); >> + drm_fb_helper_unprepare(&ifbdev->helper); >> + mutex_destroy(&ifbdev->hpd_lock); >> kfree(ifbdev); >> - return ret; >> -} >> - >> -static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) >> -{ >> - struct intel_fbdev *ifbdev = data; >> - >> - /* Due to peculiar init order wrt to hpd handling this is separate. */ >> - if (drm_fb_helper_initial_config(&ifbdev->helper)) >> - intel_fbdev_unregister(to_i915(ifbdev->helper.dev)); >> -} >> - >> -void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv) >> -{ >> - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev; >> - >> - if (!ifbdev) >> - return; >> - >> - ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev); >> -} >> - >> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv) >> -{ >> - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev; >> - >> - if (!ifbdev) >> - return; >> - >> - intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true); >> - >> - if (!current_is_async()) >> - intel_fbdev_sync(ifbdev); >> - >> - drm_fb_helper_unregister_info(&ifbdev->helper); >> -} >> - >> -void intel_fbdev_fini(struct drm_i915_private *dev_priv) >> -{ >> - struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->display.fbdev.fbdev); >> - >> - if (!ifbdev) >> - return; >> - >> - intel_fbdev_destroy(ifbdev); >> } >> >> struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev) >> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h >> index 8c953f102ba2..8123c813caaf 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h >> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h >> @@ -14,27 +14,11 @@ struct intel_fbdev; >> struct intel_framebuffer; >> >> #ifdef CONFIG_DRM_FBDEV_EMULATION >> -int intel_fbdev_init(struct drm_device *dev); >> -void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv); >> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv); >> -void intel_fbdev_fini(struct drm_i915_private *dev_priv); >> +void intel_fbdev_setup(struct drm_i915_private *dev_priv); >> void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); >> struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev); >> #else >> -static inline int intel_fbdev_init(struct drm_device *dev) >> -{ >> - return 0; >> -} >> - >> -static inline void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv) >> -{ >> -} >> - >> -static inline void intel_fbdev_unregister(struct drm_i915_private *dev_priv) >> -{ >> -} >> - >> -static inline void intel_fbdev_fini(struct drm_i915_private *dev_priv) >> +void intel_fbdev_setup(struct drm_i915_private *dev_priv); >> { >> } >> >> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c >> index be804fd4f79f..0b6722bcfbb2 100644 >> --- a/drivers/gpu/drm/i915/i915_driver.c >> +++ b/drivers/gpu/drm/i915/i915_driver.c >> @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >> >> i915->do_release = true; >> >> + intel_fbdev_setup(i915); >> + > > Maybe we could call this from intel_display_driver_register()? > > We're trying to separate display related stuff from the rest, and call > as few display functions from top level driver code as > possible. (Clearly, we're nowhere near this goal yet.) > > >> return 0; >> >> out_cleanup_gem: >
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 430a1016e013..1bbeb7a061e3 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -85,7 +85,6 @@ #include "intel_dvo.h" #include "intel_fb.h" #include "intel_fbc.h" -#include "intel_fbdev.h" #include "intel_fdi.h" #include "intel_fifo_underrun.h" #include "intel_frontbuffer.h" @@ -8588,11 +8587,6 @@ int intel_modeset_init(struct drm_i915_private *i915) intel_overlay_setup(i915); - ret = intel_fbdev_init(&i915->drm); - if (ret) - return ret; - - /* Only enable hotplug handling once the fbdev is fully set up. */ intel_hpd_init(i915); intel_hpd_poll_disable(i915); @@ -8796,9 +8790,6 @@ void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915) */ intel_hpd_poll_fini(i915); - /* poll work can call into fbdev, hence clean that up afterwards */ - intel_fbdev_fini(i915); - intel_unregister_dsm_handler(); /* flush any delayed tasks or pending work */ @@ -8864,21 +8855,6 @@ void intel_display_driver_register(struct drm_i915_private *i915) intel_display_debugfs_register(i915); - /* - * Some ports require correctly set-up hpd registers for - * detection to work properly (leading to ghost connected - * connector status), e.g. VGA on gm45. Hence we can only set - * up the initial fbdev config after hpd irqs are fully - * enabled. We do it last so that the async config cannot run - * before the connectors are registered. - */ - intel_fbdev_initial_config_async(i915); - - /* - * We need to coordinate the hotplugs with the asynchronous - * fbdev configuration, for which we use the - * fbdev->async_cookie. - */ drm_kms_helper_poll_init(&i915->drm); } @@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915) if (!HAS_DISPLAY(i915)) return; - intel_fbdev_unregister(i915); intel_audio_deinit(i915); - /* - * After flushing the fbdev (incl. a late async config which - * will have delayed queuing of a hotplug event), then flush - * the hotplug events. - */ drm_kms_helper_poll_fini(&i915->drm); drm_atomic_helper_shutdown(&i915->drm); diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index bdb9e6f43602..8b618db30ee5 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -24,7 +24,6 @@ * David Airlie */ -#include <linux/async.h> #include <linux/console.h> #include <linux/delay.h> #include <linux/errno.h> @@ -38,6 +37,7 @@ #include <linux/vga_switcheroo.h> #include <drm/drm_crtc.h> +#include <drm/drm_crtc_helper.h> #include <drm/drm_fb_helper.h> #include <drm/drm_fourcc.h> @@ -55,7 +55,6 @@ struct intel_fbdev { struct intel_framebuffer *fb; struct i915_vma *vma; unsigned long vma_flags; - async_cookie_t cookie; int preferred_bpp; /* Whether or not fbdev hpd processing is temporarily suspended */ @@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, return ret; } +static void intel_fbdev_fb_destroy(struct fb_info *info) +{ + struct drm_fb_helper *fb_helper = info->par; + struct intel_fbdev *ifbdev = container_of(fb_helper, struct intel_fbdev, helper); + + drm_fb_helper_fini(&ifbdev->helper); + + /* + * 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. + */ + intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags); + drm_framebuffer_remove(&ifbdev->fb->base); + + drm_client_release(&fb_helper->client); + drm_fb_helper_unprepare(&ifbdev->helper); + kfree(ifbdev); +} + static const struct fb_ops intelfb_ops = { .owner = THIS_MODULE, DRM_FB_HELPER_DEFAULT_OPS, @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = { .fb_imageblit = drm_fb_helper_cfb_imageblit, .fb_pan_display = intel_fbdev_pan_display, .fb_blank = intel_fbdev_blank, + .fb_destroy = intel_fbdev_fb_destroy, }; static int intelfb_alloc(struct drm_fb_helper *helper, @@ -198,7 +218,6 @@ static int intelfb_create(struct drm_fb_helper *helper, struct intel_framebuffer *intel_fb = ifbdev->fb; struct drm_device *dev = helper->dev; struct drm_i915_private *dev_priv = to_i915(dev); - struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt; const struct i915_gtt_view view = { .type = I915_GTT_VIEW_NORMAL, @@ -323,7 +342,7 @@ static int intelfb_create(struct drm_fb_helper *helper, ifbdev->vma_flags = flags; intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref); - vga_switcheroo_client_fb_set(pdev, info); + return 0; out_unpin: @@ -349,26 +368,6 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_dirty = intelfb_dirty, }; -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); - - if (ifbdev->vma) - intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags); - - if (ifbdev->fb) - drm_framebuffer_remove(&ifbdev->fb->base); - - drm_client_release(&ifbdev->helper.client); - drm_fb_helper_unprepare(&ifbdev->helper); - kfree(ifbdev); -} - /* * Build an intel_fbdev struct using a BIOS allocated framebuffer, if possible. * The core display code will have read out the current plane configuration, @@ -532,16 +531,6 @@ static void intel_fbdev_suspend_worker(struct work_struct *work) true); } -static void intel_fbdev_sync(struct intel_fbdev *ifbdev) -{ - if (!ifbdev->cookie) - return; - - /* Only serialises with all preceding async calls, hence +1 */ - async_synchronize_cookie(ifbdev->cookie + 1); - ifbdev->cookie = 0; -} - /* Suspends/resumes fbdev processing of incoming HPD events. When resuming HPD * processing, fbdev will perform a full connector reprobe if a hotplug event * was received while HPD was suspended. @@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev) if (!ifbdev) return; - intel_fbdev_sync(ifbdev); - mutex_lock(&ifbdev->hpd_lock); send_hpd = !ifbdev->hpd_suspended; ifbdev->hpd_waiting = true; @@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv) if (!ifbdev) return; - intel_fbdev_sync(ifbdev); if (!ifbdev->vma) return; @@ -662,7 +648,20 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv) */ static void intel_fbdev_client_unregister(struct drm_client_dev *client) -{ } +{ + struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); + struct drm_device *dev = fb_helper->dev; + struct pci_dev *pdev = to_pci_dev(dev->dev); + + if (fb_helper->info) { + vga_switcheroo_client_fb_set(pdev, NULL); + drm_fb_helper_unregister_info(fb_helper); + } else { + drm_fb_helper_unprepare(fb_helper); + drm_client_release(&fb_helper->client); + kfree(fb_helper); + } +} static int intel_fbdev_client_restore(struct drm_client_dev *client) { @@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client) static int intel_fbdev_client_hotplug(struct drm_client_dev *client) { - intel_fbdev_output_poll_changed(client->dev); + struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); + struct drm_device *dev = client->dev; + struct pci_dev *pdev = to_pci_dev(dev->dev); + int ret; + + if (dev->fb_helper) { + intel_fbdev_output_poll_changed(dev); + return 0; + } + + ret = drm_fb_helper_init(dev, fb_helper); + if (ret) + goto err_drm_err; + + if (!drm_drv_uses_atomic_modeset(dev)) + drm_helper_disable_unused_functions(dev); + + ret = drm_fb_helper_initial_config(fb_helper); + if (ret) + goto err_drm_fb_helper_fini; + + vga_switcheroo_client_fb_set(pdev, fb_helper->info); return 0; + +err_drm_fb_helper_fini: + drm_fb_helper_fini(fb_helper); +err_drm_err: + drm_err(dev, "Failed to setup radeon fbdev emulation (ret=%d)\n", ret); + return ret; } static const struct drm_client_funcs intel_fbdev_client_funcs = { @@ -688,22 +714,23 @@ static const struct drm_client_funcs intel_fbdev_client_funcs = { .hotplug = intel_fbdev_client_hotplug, }; -int intel_fbdev_init(struct drm_device *dev) +void intel_fbdev_setup(struct drm_i915_private *dev_priv) { - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_device *dev = &dev_priv->drm; struct intel_fbdev *ifbdev; int ret; if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv))) - return -ENODEV; - - ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL); - if (ifbdev == NULL) - return -ENOMEM; + return; - mutex_init(&ifbdev->hpd_lock); + ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL); + if (!ifbdev) + return; drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs); + dev_priv->display.fbdev.fbdev = ifbdev; + INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker); + mutex_init(&ifbdev->hpd_lock); if (intel_fbdev_init_bios(dev, ifbdev)) ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp; else @@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev) ret = drm_client_init(dev, &ifbdev->helper.client, "i915-fbdev", &intel_fbdev_client_funcs); - if (ret) + if (ret) { + drm_err(dev, "Failed to register client: %d\n", ret); goto err_drm_fb_helper_unprepare; + } - ret = drm_fb_helper_init(dev, &ifbdev->helper); + ret = intel_fbdev_client_hotplug(&ifbdev->helper.client); if (ret) - goto err_drm_client_release; + drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); - dev_priv->display.fbdev.fbdev = ifbdev; - INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker); + drm_client_register(&ifbdev->helper.client); - return 0; + return; -err_drm_client_release: - drm_client_release(&ifbdev->helper.client); err_drm_fb_helper_unprepare: - drm_client_release(&ifbdev->helper.client); + drm_fb_helper_unprepare(&ifbdev->helper); + mutex_destroy(&ifbdev->hpd_lock); kfree(ifbdev); - return ret; -} - -static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) -{ - struct intel_fbdev *ifbdev = data; - - /* Due to peculiar init order wrt to hpd handling this is separate. */ - if (drm_fb_helper_initial_config(&ifbdev->helper)) - intel_fbdev_unregister(to_i915(ifbdev->helper.dev)); -} - -void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv) -{ - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev; - - if (!ifbdev) - return; - - ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev); -} - -void intel_fbdev_unregister(struct drm_i915_private *dev_priv) -{ - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev; - - if (!ifbdev) - return; - - intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true); - - if (!current_is_async()) - intel_fbdev_sync(ifbdev); - - drm_fb_helper_unregister_info(&ifbdev->helper); -} - -void intel_fbdev_fini(struct drm_i915_private *dev_priv) -{ - struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->display.fbdev.fbdev); - - if (!ifbdev) - return; - - intel_fbdev_destroy(ifbdev); } struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev) diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h index 8c953f102ba2..8123c813caaf 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.h +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h @@ -14,27 +14,11 @@ struct intel_fbdev; struct intel_framebuffer; #ifdef CONFIG_DRM_FBDEV_EMULATION -int intel_fbdev_init(struct drm_device *dev); -void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv); -void intel_fbdev_unregister(struct drm_i915_private *dev_priv); -void intel_fbdev_fini(struct drm_i915_private *dev_priv); +void intel_fbdev_setup(struct drm_i915_private *dev_priv); void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev); #else -static inline int intel_fbdev_init(struct drm_device *dev) -{ - return 0; -} - -static inline void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv) -{ -} - -static inline void intel_fbdev_unregister(struct drm_i915_private *dev_priv) -{ -} - -static inline void intel_fbdev_fini(struct drm_i915_private *dev_priv) +void intel_fbdev_setup(struct drm_i915_private *dev_priv); { } diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index be804fd4f79f..0b6722bcfbb2 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) i915->do_release = true; + intel_fbdev_setup(i915); + return 0; out_cleanup_gem:
Replace all code that initializes or releases fbdev emulation throughout the driver. Instead initialize the fbdev client by a single call to i915_fbdev_setup() after i915 has registered its DRM device. Just like in most drivers, i915 fbdev emulation now acts like a regular DRM client. The fbdev client setup consists of the initial preparation and the hot-plugging of the display. The latter creates the fbdev device and sets up the fbdev framebuffer. The setup performs display hot-plugging once. If no display can be detected, DRM probe helpers re-run the detection on each hotplug event. A call to drm_dev_unregister() releases the client automatically. No further action is required within i915. If the fbdev framebuffer has been fully set up, struct fb_ops.fb_destroy implements the release. For partially initialized emulation, the fbdev client reverts the initial setup. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/i915/display/intel_display.c | 30 --- drivers/gpu/drm/i915/display/intel_fbdev.c | 184 +++++++++---------- drivers/gpu/drm/i915/display/intel_fbdev.h | 20 +- drivers/gpu/drm/i915/i915_driver.c | 2 + 4 files changed, 87 insertions(+), 149 deletions(-)