[PATCH 10/12] drm/{i915,xe}: Run DRM default client setup
Jocelyn Falempe
jfalempe at redhat.com
Tue Jan 7 12:10:15 UTC 2025
On 12/12/2024 18:08, Thomas Zimmermann wrote:
> Rework fbdev probing to support fbdev_probe in struct drm_driver
> and remove the old fb_probe callback. Provide an initializer macro
> that sets the callback in struct drm_driver according to the kernel
> configuration. Call drm_client_setup_with_color_mode() to run the
> kernel's default client setup for DRM.
>
> This commit also prepares support for the kernel's drm_log client
> (or any future client) in i915. Using drm_log will also require vmap
> support in GEM objects.
I've tested this series on an Alderlake laptop, and it effectively
breaks the boot with drm_log on i915. (I can still see the drm_log on
simpledrm, but when it switches to i915, I get a blackscreen, and the
laptop hard freezes).
Can we wait to have the proper vmap support in GEM objects, before
merging this series?
Or at least prevent drm_log to load on i915, until it is fixed?
Best regards,
--
Jocelyn
>
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
> .../gpu/drm/i915/display/intel_display_core.h | 1 -
> drivers/gpu/drm/i915/display/intel_fbdev.c | 194 +-----------------
> drivers/gpu/drm/i915/display/intel_fbdev.h | 17 +-
> drivers/gpu/drm/i915/i915_driver.c | 3 +
> drivers/gpu/drm/xe/display/xe_display.c | 5 +
> 5 files changed, 21 insertions(+), 199 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 554870d2494b..674913d6c11d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -386,7 +386,6 @@ struct intel_display {
> struct {
> /* list of fbdev register on this device */
> struct intel_fbdev *fbdev;
> - struct work_struct suspend_work;
> } fbdev;
>
> struct {
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index f5dc96a9f86d..de726a9c33c5 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -37,6 +37,7 @@
> #include <linux/tty.h>
> #include <linux/vga_switcheroo.h>
>
> +#include <drm/clients/drm_client_setup.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_fb_helper.h>
> @@ -54,9 +55,6 @@
> #include "intel_fbdev_fb.h"
> #include "intel_frontbuffer.h"
>
> -static int intelfb_create(struct drm_fb_helper *helper,
> - struct drm_fb_helper_surface_size *sizes);
> -
> struct intel_fbdev {
> struct intel_framebuffer *fb;
> struct i915_vma *vma;
> @@ -201,14 +199,13 @@ static void intelfb_set_suspend(struct drm_fb_helper *fb_helper, bool suspend)
> }
>
> static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
> - .fb_probe = intelfb_create,
> .fb_dirty = intelfb_dirty,
> .fb_restore = intelfb_restore,
> .fb_set_suspend = intelfb_set_suspend,
> };
>
> -static int intelfb_create(struct drm_fb_helper *helper,
> - struct drm_fb_helper_surface_size *sizes)
> +int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> + struct drm_fb_helper_surface_size *sizes)
> {
> struct intel_fbdev *ifbdev = to_intel_fbdev(helper);
> struct intel_framebuffer *fb = ifbdev->fb;
> @@ -272,6 +269,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> goto out_unpin;
> }
>
> + helper->funcs = &intel_fb_helper_funcs;
> helper->fb = &fb->base;
>
> info->fbops = &intelfb_ops;
> @@ -480,174 +478,11 @@ static unsigned int intel_fbdev_color_mode(const struct drm_format_info *info)
> }
> }
>
> -static void intel_fbdev_suspend_worker(struct work_struct *work)
> -{
> - intel_fbdev_set_suspend(&container_of(work,
> - struct drm_i915_private,
> - display.fbdev.suspend_work)->drm,
> - FBINFO_STATE_RUNNING,
> - true);
> -}
> -
> -void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous)
> -{
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
> -
> - if (!ifbdev)
> - return;
> -
> - if (drm_WARN_ON(&dev_priv->drm, !HAS_DISPLAY(dev_priv)))
> - return;
> -
> - if (!ifbdev->vma)
> - return;
> -
> - if (synchronous) {
> - /* Flush any pending work to turn the console on, and then
> - * wait to turn it off. It must be synchronous as we are
> - * about to suspend or unload the driver.
> - *
> - * Note that from within the work-handler, we cannot flush
> - * ourselves, so only flush outstanding work upon suspend!
> - */
> - if (state != FBINFO_STATE_RUNNING)
> - flush_work(&dev_priv->display.fbdev.suspend_work);
> -
> - console_lock();
> - } else {
> - /*
> - * The console lock can be pretty contented on resume due
> - * to all the printk activity. Try to keep it out of the hot
> - * path of resume if possible.
> - */
> - drm_WARN_ON(dev, state != FBINFO_STATE_RUNNING);
> - if (!console_trylock()) {
> - /* Don't block our own workqueue as this can
> - * be run in parallel with other i915.ko tasks.
> - */
> - queue_work(dev_priv->unordered_wq,
> - &dev_priv->display.fbdev.suspend_work);
> - return;
> - }
> - }
> -
> - drm_fb_helper_set_suspend(dev->fb_helper, state);
> - console_unlock();
> -}
> -
> -static int intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
> -{
> - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
> - struct drm_device *dev = &dev_priv->drm;
> - int ret;
> -
> - if (!ifbdev)
> - return -EINVAL;
> -
> - if (!ifbdev->vma)
> - return -ENOMEM;
> -
> - ret = drm_fb_helper_restore_fbdev_mode_unlocked(dev->fb_helper);
> - if (ret)
> - return ret;
> -
> - return 0;
> -}
> -
> -/*
> - * Fbdev client and struct drm_client_funcs
> - */
> -
> -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)
> -{
> - struct drm_i915_private *dev_priv = to_i915(client->dev);
> - int ret;
> -
> - ret = intel_fbdev_restore_mode(dev_priv);
> - if (ret)
> - return ret;
> -
> - vga_switcheroo_process_delayed_switch();
> -
> - return 0;
> -}
> -
> -static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
> -{
> - 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)
> - return drm_fb_helper_hotplug_event(dev->fb_helper);
> -
> - ret = drm_fb_helper_init(dev, fb_helper);
> - if (ret)
> - goto err_drm_err;
> -
> - 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 int intel_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
> -{
> - intel_fbdev_set_suspend(client->dev, FBINFO_STATE_SUSPENDED, true);
> -
> - return 0;
> -}
> -
> -static int intel_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock)
> -{
> - intel_fbdev_set_suspend(client->dev, FBINFO_STATE_RUNNING, false);
> -
> - return 0;
> -}
> -
> -static const struct drm_client_funcs intel_fbdev_client_funcs = {
> - .owner = THIS_MODULE,
> - .unregister = intel_fbdev_client_unregister,
> - .restore = intel_fbdev_client_restore,
> - .hotplug = intel_fbdev_client_hotplug,
> - .suspend = intel_fbdev_client_suspend,
> - .resume = intel_fbdev_client_resume,
> -};
> -
> void intel_fbdev_setup(struct drm_i915_private *i915)
> {
> struct drm_device *dev = &i915->drm;
> struct intel_fbdev *ifbdev;
> - struct drm_fb_helper *fb_helper;
> unsigned int preferred_bpp = 0;
> - int ret;
>
> if (!HAS_DISPLAY(i915))
> return;
> @@ -657,31 +492,12 @@ void intel_fbdev_setup(struct drm_i915_private *i915)
> return;
>
> i915->display.fbdev.fbdev = ifbdev;
> - INIT_WORK(&i915->display.fbdev.suspend_work, intel_fbdev_suspend_worker);
> if (intel_fbdev_init_bios(dev, ifbdev))
> preferred_bpp = intel_fbdev_color_mode(ifbdev->fb->base.format);
> if (!preferred_bpp)
> preferred_bpp = 32;
>
> - fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
> - if (!fb_helper)
> - return;
> - drm_fb_helper_prepare(dev, fb_helper, preferred_bpp, &intel_fb_helper_funcs);
> -
> - ret = drm_client_init(dev, &fb_helper->client, "intel-fbdev",
> - &intel_fbdev_client_funcs);
> - if (ret) {
> - drm_err(dev, "Failed to register client: %d\n", ret);
> - goto err_drm_fb_helper_unprepare;
> - }
> -
> - drm_client_register(&fb_helper->client);
> -
> - return;
> -
> -err_drm_fb_helper_unprepare:
> - drm_fb_helper_unprepare(dev->fb_helper);
> - kfree(fb_helper);
> + drm_client_setup_with_color_mode(dev, preferred_bpp);
> }
>
> 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 08de2d5b3433..b2ca1fe3c9ae 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
> @@ -6,26 +6,25 @@
> #ifndef __INTEL_FBDEV_H__
> #define __INTEL_FBDEV_H__
>
> -#include <linux/types.h>
> -
> -struct drm_device;
> +struct drm_fb_helper;
> +struct drm_fb_helper_surface_size;
> struct drm_i915_private;
> struct intel_fbdev;
> struct intel_framebuffer;
>
> #ifdef CONFIG_DRM_FBDEV_EMULATION
> +int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> + struct drm_fb_helper_surface_size *sizes);
> +#define INTEL_FBDEV_DRIVER_OPS \
> + .fbdev_probe = intel_fbdev_driver_fbdev_probe
> 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
> +#define INTEL_FBDEV_DRIVER_OPS \
> + .fbdev_probe = NULL
> static inline void intel_fbdev_setup(struct drm_i915_private *dev_priv)
> {
> }
> -
> -static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous)
> -{
> -}
> -
> static inline struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev)
> {
> return NULL;
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index e385e4947a91..1029bf8509b7 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -57,6 +57,7 @@
> #include "display/intel_dp.h"
> #include "display/intel_dpt.h"
> #include "display/intel_encoder.h"
> +#include "display/intel_fbdev.h"
> #include "display/intel_hotplug.h"
> #include "display/intel_overlay.h"
> #include "display/intel_pch_refclk.h"
> @@ -1798,6 +1799,8 @@ static const struct drm_driver i915_drm_driver = {
> .dumb_create = i915_gem_dumb_create,
> .dumb_map_offset = i915_gem_dumb_mmap_offset,
>
> + INTEL_FBDEV_DRIVER_OPS,
> +
> .ioctls = i915_ioctls,
> .num_ioctls = ARRAY_SIZE(i915_ioctls),
> .fops = &i915_driver_fops,
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index bc73c9999c57..03554cf4894a 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -27,6 +27,7 @@
> #include "intel_dmc_wl.h"
> #include "intel_dp.h"
> #include "intel_encoder.h"
> +#include "intel_fbdev.h"
> #include "intel_hdcp.h"
> #include "intel_hotplug.h"
> #include "intel_opregion.h"
> @@ -67,6 +68,10 @@ void xe_display_driver_set_hooks(struct drm_driver *driver)
> if (!xe_modparam.probe_display)
> return;
>
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> + driver->fbdev_probe = intel_fbdev_driver_fbdev_probe;
> +#endif
> +
> driver->driver_features |= DRIVER_MODESET | DRIVER_ATOMIC;
> }
>
More information about the Intel-gfx
mailing list