[PATCH v5 7/7] drm/i915: Implement fbdev emulation as in-kernel client
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Nov 1 09:25:43 UTC 2023
On Wed, Nov 01, 2023 at 09:33:41AM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 25.10.23 um 13:36 schrieb Hogander, Jouni:
> [...]
> >> +
> >> + if (!drm_drv_uses_atomic_modeset(dev))
> >> + drm_helper_disable_unused_functions(dev);
> >
> > Can you please explain why this is needed here?
>
> This disables some parts of the mode-setting pipeline and is required
> for drivers with non-atomic commits. AFAICT atomic mode setting is not
> supported on some very old Intel chips. [1] I'll leave a short comment
> on the code.
We don't expose the atomic uapi because the watermark code is
kinda sketchy for the old chips, but internally i915 is 100% atomic.
>
> [1]
> https://elixir.bootlin.com/linux/v6.6/source/drivers/gpu/drm/i915/intel_device_info.c#L399
>
> Best regard
> Thomas
>
> >
> >> +
> >> + 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 i915 fbdev emulation
> >> (ret=%d)\n", ret);
> >> + return ret;
> >> }
> >>
> >> static const struct drm_client_funcs intel_fbdev_client_funcs = {
> >> @@ -703,22 +729,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)
> >
> > Use i915 rather than dev_priv.
> >
> > BR,
> >
> > Jouni Högander
> >
> >> {
> >> - 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;
> >> + if (!HAS_DISPLAY(dev_priv))
> >> + return;
> >>
> >> ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
> >> if (!ifbdev)
> >> - return -ENOMEM;
> >> -
> >> - mutex_init(&ifbdev->hpd_lock);
> >> + 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
> >> @@ -726,68 +753,19 @@ 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);
> >> - if (ret)
> >> - goto err_drm_client_release;
> >> -
> >> - 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_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 8c953f102ba22..08de2d5b34338 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)
> >> +static inline 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 86460cd8167d1..53663c0cc3be4 100644
> >> --- a/drivers/gpu/drm/i915/i915_driver.c
> >> +++ b/drivers/gpu/drm/i915/i915_driver.c
> >> @@ -817,6 +817,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:
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
--
Ville Syrjälä
Intel
More information about the dri-devel
mailing list