From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <dev@lankhorst.se>
Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Uma Shankar <uma.shankar@intel.com>
Subject: Re: [PATCH v5 5/5] drm/i915: Introduce intel_bo_fbdev_bios_fb_takeover()
Date: Fri, 13 Nov 2026 15:21:23 +0200 [thread overview]
Message-ID: <avcO035BB7RU_ow0@intel.com> (raw)
In-Reply-To: <20261113124453.144408-12-dev@lankhorst.se>
On Fri, Nov 13, 2026 at 01:44:59PM +0100, Maarten Lankhorst wrote:
> Pull the "do we want to use stolen for the BIOS FB?" checks into
> a new intel_bo_fbdev_bios_fb_takeover() helper, and defer that
> decision until we're ready to reallocate the fbdev FB. This way
> even if we don't want to ultimately use the BIOS FB we'll keep
> the plane enabled until the replacement FB is ready. Should
> hopefully result in fewer display blinks during boot.
>
> Additionally, on some platforms this allows us to re-create the
> framebuffer when stolen memory is not suitable, using the blitter
> engine to copy the contents to system memory without causing
> any flickering by not preserving the stolen memory allocation.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Did I do some part of this?
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
> drivers/gpu/drm/i915/display/intel_bo.c | 7 ++
> drivers/gpu/drm/i915/display/intel_bo.h | 1 +
> drivers/gpu/drm/i915/display/intel_fbdev.c | 68 ++++++++++++++++---
> drivers/gpu/drm/i915/i915_bo.c | 14 +++-
> drivers/gpu/drm/i915/i915_bo.h | 6 --
> drivers/gpu/drm/i915/i915_initial_plane.c | 14 ----
> drivers/gpu/drm/xe/display/xe_display_bo.c | 62 ++++++++++++++++-
> drivers/gpu/drm/xe/display/xe_display_bo.h | 6 --
> drivers/gpu/drm/xe/display/xe_initial_plane.c | 8 ---
> include/drm/intel/display_parent_interface.h | 1 +
> 10 files changed, 140 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bo.c b/drivers/gpu/drm/i915/display/intel_bo.c
> index 8ecdbb7e39f3d..878c62cb48b4f 100644
> --- a/drivers/gpu/drm/i915/display/intel_bo.c
> +++ b/drivers/gpu/drm/i915/display/intel_bo.c
> @@ -92,6 +92,13 @@ u32 intel_bo_fbdev_pitch_align(struct intel_display *display, u32 stride)
> return display->parent->bo->fbdev_pitch_align(stride);
> }
>
> +struct drm_gem_object *intel_bo_fbdev_bios_fb_takeover(struct drm_gem_object *obj)
> +{
> + struct intel_display *display = to_intel_display(obj->dev);
> +
> + return display->parent->bo->fbdev_bios_fb_takeover(obj);
> +}
> +
> struct drm_gem_object *intel_bo_fbdev_create(struct intel_display *display, int size)
> {
> return display->parent->bo->fbdev_create(display->drm, size);
> diff --git a/drivers/gpu/drm/i915/display/intel_bo.h b/drivers/gpu/drm/i915/display/intel_bo.h
> index 348f7fa669608..0410ee9c6d2e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_bo.h
> +++ b/drivers/gpu/drm/i915/display/intel_bo.h
> @@ -34,6 +34,7 @@ struct drm_gem_object *intel_bo_framebuffer_lookup(struct intel_display *display
> const struct drm_mode_fb_cmd2 *user_mode_cmd);
>
> u32 intel_bo_fbdev_pitch_align(struct intel_display *display, u32 stride);
> +struct drm_gem_object *intel_bo_fbdev_bios_fb_takeover(struct drm_gem_object *obj);
> struct drm_gem_object *intel_bo_fbdev_create(struct intel_display *display, int size);
> void intel_bo_fbdev_destroy(struct drm_gem_object *obj);
> int intel_bo_fbdev_fill_info(struct drm_gem_object *obj, struct fb_info *info,
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index c09246a9865cd..8ef522df7f172 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -229,8 +229,8 @@ __intel_fbdev_fb_alloc(struct intel_display *display,
> struct drm_fb_helper_surface_size *sizes)
> {
> struct drm_mode_fb_cmd2 mode_cmd = {};
> - struct drm_framebuffer *fb;
> struct drm_gem_object *obj;
> + struct drm_framebuffer *fb;
> int size;
>
> intel_fbdev_fill_mode_cmd(display, sizes, &mode_cmd);
> @@ -263,32 +263,68 @@ __intel_fbdev_fb_alloc(struct intel_display *display,
>
> }
>
> -static bool bios_fb_ok(const struct intel_framebuffer *fb,
> - const struct drm_fb_helper_surface_size *sizes)
> +static struct intel_framebuffer *
> +__intel_fbdev_fb_realloc(struct intel_display *display,
> + const struct drm_framebuffer *orig_fb,
> + struct drm_gem_object *obj)
> +{
> + struct drm_framebuffer *new_fb;
> + struct drm_mode_fb_cmd2 mode_cmd = {
> + .flags = DRM_MODE_FB_MODIFIERS,
> + .width = orig_fb->width,
> + .height = orig_fb->height,
> + .pitches[0] = orig_fb->pitches[0],
> + .offsets[0] = orig_fb->offsets[0],
> + .pixel_format = orig_fb->format->format,
> + .modifier[0] = orig_fb->modifier,
> + };
> +
> + new_fb = intel_framebuffer_create(obj, orig_fb->format, &mode_cmd);
> +
> + if (IS_ERR(new_fb)) {
> + intel_bo_fbdev_destroy(obj);
> + return ERR_CAST(new_fb);
> + }
> +
> + drm_gem_object_put(obj);
> + return to_intel_framebuffer(new_fb);
> +}
> +
> +static struct intel_framebuffer *bios_fb_pick(struct intel_framebuffer *fb,
> + const struct drm_fb_helper_surface_size *sizes)
> {
> struct intel_display *display = to_intel_display(fb->base.dev);
> int width = fb->base.width;
> int height = fb->base.height;
> int depth = fb->base.format->depth;
> int bpp = fb->base.format->cpp[0] * 8;
> + struct drm_gem_object *new;
'new' is a bit too generic a name for my taste.
>
> if (sizes->fb_width > width || sizes->fb_height > height) {
> drm_dbg_kms(display->drm,
> "BIOS fb too small (%dx%d), we require (%dx%d), releasing it\n",
> width, height, sizes->fb_width, sizes->fb_height);
> - return false;
> + return NULL;
> }
>
> if (sizes->surface_depth != depth || sizes->surface_bpp != bpp) {
> drm_dbg_kms(display->drm,
> "BIOS fb using wrong depth/bpp (%d/%d), we require (%d/%d), releasing it\n",
> depth, bpp, sizes->surface_depth, sizes->surface_bpp);
> - return false;
> + return NULL;
> }
I think you can leave bios_fb_ok() alone and just call it here?
The rest I like. Didn't look at the xe_migrate stuff though.
>
> - return true;
> + new = intel_bo_fbdev_bios_fb_takeover(fb->base.obj[0]);
> + if (IS_ERR(new))
> + return ERR_CAST(new);
> + if (fb->base.obj[0] == new)
> + return fb;
> +
> + /* Different object, re-create an intel_framebuffer around it */
> + return __intel_fbdev_fb_realloc(display, &fb->base, new);
> }
>
> +
> int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> struct drm_fb_helper_surface_size *sizes)
> {
> @@ -306,13 +342,23 @@ int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
>
> ifbdev->fb = NULL;
>
> - if (fb && !bios_fb_ok(fb, sizes)) {
> - drm_framebuffer_put(&fb->base);
> - fb = NULL;
> - }
> -
> wakeref = intel_display_rpm_get(display);
>
> + if (fb) {
> + struct intel_framebuffer *bios_fb =
> + bios_fb_pick(fb, sizes);
> +
> + if (IS_ERR(bios_fb)) {
> + drm_dbg_kms(display->drm, "Re-using BIOS fb failed (%pe).", bios_fb);
> + bios_fb = NULL;
> + }
> +
> + if (fb != bios_fb) {
> + drm_framebuffer_put(&fb->base);
> + fb = bios_fb;
> + }
> + }
> +
> if (!fb || drm_WARN_ON(display->drm, !intel_fb_bo(&fb->base))) {
> drm_dbg_kms(display->drm,
> "no BIOS fb, allocating a new one\n");
> diff --git a/drivers/gpu/drm/i915/i915_bo.c b/drivers/gpu/drm/i915/i915_bo.c
> index 559341103ca7d..fe11c0be81013 100644
> --- a/drivers/gpu/drm/i915/i915_bo.c
> +++ b/drivers/gpu/drm/i915/i915_bo.c
> @@ -150,7 +150,7 @@ static u32 i915_bo_fbdev_pitch_align(u32 stride)
> return ALIGN(stride, 64);
> }
>
> -bool i915_bo_fbdev_prefer_stolen(struct drm_i915_private *i915, unsigned int size)
> +static bool i915_bo_fbdev_prefer_stolen(struct drm_i915_private *i915, unsigned int size)
> {
> /* Skip stolen on MTL as Wa_22018444074 mitigation. */
> if (IS_METEORLAKE(i915))
> @@ -164,6 +164,17 @@ bool i915_bo_fbdev_prefer_stolen(struct drm_i915_private *i915, unsigned int siz
> return i915->dsm.usable_size >= size * 2;
> }
>
> +static struct drm_gem_object *i915_bo_fbdev_bios_fb_takeover(struct drm_gem_object *obj)
> +{
> + struct drm_i915_private *i915 = to_i915(obj->dev);
> +
> + if (HAS_LMEM(i915) ||
> + i915_bo_fbdev_prefer_stolen(i915, obj->size))
> + return obj;
> +
> + return ERR_PTR(-EIO);
> +}
> +
> static struct drm_gem_object *i915_bo_fbdev_create(struct drm_device *drm, int size)
> {
> struct drm_i915_private *i915 = to_i915(drm);
> @@ -262,6 +273,7 @@ const struct intel_display_bo_interface i915_display_bo_interface = {
> .framebuffer_fini = i915_bo_framebuffer_fini,
> .framebuffer_lookup = i915_bo_framebuffer_lookup,
> #if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
> + .fbdev_bios_fb_takeover = i915_bo_fbdev_bios_fb_takeover,
> .fbdev_create = i915_bo_fbdev_create,
> .fbdev_destroy = i915_bo_fbdev_destroy,
> .fbdev_fill_info = i915_bo_fbdev_fill_info,
> diff --git a/drivers/gpu/drm/i915/i915_bo.h b/drivers/gpu/drm/i915/i915_bo.h
> index 39ba62696550e..57255d052dd9a 100644
> --- a/drivers/gpu/drm/i915/i915_bo.h
> +++ b/drivers/gpu/drm/i915/i915_bo.h
> @@ -4,12 +4,6 @@
> #ifndef __I915_BO_H__
> #define __I915_BO_H__
>
> -#include <linux/types.h>
> -
> -struct drm_i915_private;
> -
> -bool i915_bo_fbdev_prefer_stolen(struct drm_i915_private *i915, unsigned int size);
> -
> extern const struct intel_display_bo_interface i915_display_bo_interface;
>
> #endif /* __I915_BO_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_initial_plane.c b/drivers/gpu/drm/i915/i915_initial_plane.c
> index 98858a78e46a3..d7792b65ec150 100644
> --- a/drivers/gpu/drm/i915/i915_initial_plane.c
> +++ b/drivers/gpu/drm/i915/i915_initial_plane.c
> @@ -12,7 +12,6 @@
> #include "gem/i915_gem_lmem.h"
> #include "gem/i915_gem_region.h"
>
> -#include "i915_bo.h"
> #include "i915_drv.h"
> #include "i915_initial_plane.h"
>
> @@ -102,19 +101,6 @@ initial_plane_vma(struct drm_i915_private *i915,
> mem->min_page_size);
> size -= base;
>
> - /*
> - * If the FB is too big, just don't use it since fbdev is not very
> - * important and we should probably use that space with FBC or other
> - * features.
> - */
> - if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
> - IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
> - mem == i915->mm.stolen_region &&
> - !i915_bo_fbdev_prefer_stolen(i915, size)) {
> - drm_dbg_kms(&i915->drm, "Initial FB size exceeds half of stolen, discarding\n");
> - return NULL;
> - }
> -
> obj = i915_gem_object_create_region_at(mem, phys_base, size,
> I915_BO_ALLOC_USER |
> I915_BO_PREALLOC);
> diff --git a/drivers/gpu/drm/xe/display/xe_display_bo.c b/drivers/gpu/drm/xe/display/xe_display_bo.c
> index d78d893210c4f..43d05a58bcb47 100644
> --- a/drivers/gpu/drm/xe/display/xe_display_bo.c
> +++ b/drivers/gpu/drm/xe/display/xe_display_bo.c
> @@ -9,6 +9,7 @@
> #include "intel_fb.h"
> #include "xe_bo.h"
> #include "xe_display_bo.h"
> +#include "xe_migrate.h"
> #include "xe_pxp.h"
> #include "xe_ttm_stolen_mgr.h"
> #include "xe_wa.h"
> @@ -120,7 +121,7 @@ static u32 xe_display_bo_fbdev_pitch_align(u32 stride)
> return ALIGN(stride, XE_PAGE_SIZE);
> }
>
> -bool xe_display_bo_fbdev_prefer_stolen(struct xe_device *xe, unsigned int size)
> +static bool xe_display_bo_fbdev_prefer_stolen(struct xe_device *xe, unsigned int size)
> {
> struct ttm_resource_manager *stolen;
>
> @@ -152,6 +153,64 @@ bool xe_display_bo_fbdev_prefer_stolen(struct xe_device *xe, unsigned int size)
> return stolen->size >= (size * 2) >> PAGE_SHIFT;
> }
>
> +static struct drm_gem_object *xe_display_bo_fbdev_bios_fb_takeover(struct drm_gem_object *obj)
> +{
> + struct xe_bo *orig_bo = gem_to_xe_bo(obj), *copy_bo;
> + struct xe_device *xe = xe_bo_device(orig_bo);
> + struct xe_validation_ctx ctx;
> + struct drm_exec exec;
> + int err;
> +
> + if (IS_DGFX(xe) ||
> + xe_display_bo_fbdev_prefer_stolen(xe, xe_bo_size(orig_bo)))
> + return obj;
> +
> + copy_bo = xe_bo_create_pin_map_novm(xe, xe_device_get_root_tile(xe), xe_bo_size(orig_bo),
> + ttm_bo_type_kernel,
> + XE_BO_FLAG_FORCE_WC | XE_BO_FLAG_SYSTEM | XE_BO_FLAG_GGTT,
> + false);
> + if (IS_ERR(copy_bo))
> + return ERR_CAST(copy_bo);
> +
> + xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {}, err) {
> + struct dma_fence *fence;
> +
> + err = drm_exec_lock_obj(&exec, &orig_bo->ttm.base);
> + drm_exec_retry_on_contention(&exec);
> + if (err)
> + break;
> +
> + err = drm_exec_lock_obj(&exec, ©_bo->ttm.base);
> + drm_exec_retry_on_contention(&exec);
> + if (err)
> + break;
> +
> + err = dma_resv_reserve_fences(orig_bo->ttm.base.resv, 1);
> + if (err)
> + break;
> +
> + err = dma_resv_reserve_fences(copy_bo->ttm.base.resv, 1);
> + if (err)
> + break;
> +
> + fence = xe_migrate_copy(xe_device_get_root_tile(xe)->migrate, orig_bo, copy_bo,
> + orig_bo->ttm.resource, copy_bo->ttm.resource, false);
> + if (IS_ERR(fence)) {
> + err = PTR_ERR(fence);
> + break;
> + }
> +
> + dma_resv_add_fence(copy_bo->ttm.base.resv, fence, DMA_RESV_USAGE_KERNEL);
> + dma_resv_add_fence(orig_bo->ttm.base.resv, fence, DMA_RESV_USAGE_BOOKKEEP);
> + dma_fence_put(fence);
> +
> + return ©_bo->ttm.base;
> + }
> +
> + xe_bo_unpin_map_no_vm(copy_bo);
> + return ERR_PTR(err);
> +}
> +
> static struct drm_gem_object *xe_display_bo_fbdev_create(struct drm_device *drm, int size)
> {
> struct xe_device *xe = to_xe_device(drm);
> @@ -214,6 +273,7 @@ const struct intel_display_bo_interface xe_display_bo_interface = {
> .framebuffer_fini = xe_display_bo_framebuffer_fini,
> .framebuffer_lookup = xe_display_bo_framebuffer_lookup,
> #if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
> + .fbdev_bios_fb_takeover = xe_display_bo_fbdev_bios_fb_takeover,
> .fbdev_create = xe_display_bo_fbdev_create,
> .fbdev_destroy = xe_display_bo_fbdev_destroy,
> .fbdev_fill_info = xe_display_bo_fbdev_fill_info,
> diff --git a/drivers/gpu/drm/xe/display/xe_display_bo.h b/drivers/gpu/drm/xe/display/xe_display_bo.h
> index c72056884ff41..6879c104b0b1f 100644
> --- a/drivers/gpu/drm/xe/display/xe_display_bo.h
> +++ b/drivers/gpu/drm/xe/display/xe_display_bo.h
> @@ -4,12 +4,6 @@
> #ifndef __XE_DISPLAY_BO_H__
> #define __XE_DISPLAY_BO_H__
>
> -#include <linux/types.h>
> -
> -struct xe_device;
> -
> -bool xe_display_bo_fbdev_prefer_stolen(struct xe_device *xe, unsigned int size);
> -
> extern const struct intel_display_bo_interface xe_display_bo_interface;
>
> #endif
> diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> index 0f86b73036d03..8988c07e61934 100644
> --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
> +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> @@ -14,7 +14,6 @@
> #include "intel_display_types.h"
>
> #include "xe_bo.h"
> -#include "xe_display_bo.h"
> #include "xe_display_vma.h"
> #include "xe_fb_pin.h"
> #include "xe_ggtt.h"
> @@ -102,13 +101,6 @@ initial_plane_bo(struct xe_device *xe,
>
> phys_base = base;
> flags |= XE_BO_FLAG_STOLEN;
> -
> - if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
> - IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
> - !xe_display_bo_fbdev_prefer_stolen(xe, plane_config->size)) {
> - drm_info(&xe->drm, "Initial FB size exceeds half of stolen, discarding\n");
> - return NULL;
> - }
> }
>
> bo = xe_bo_create_pin_map_at_novm(xe, tile0, size, phys_base,
> diff --git a/include/drm/intel/display_parent_interface.h b/include/drm/intel/display_parent_interface.h
> index 464fb588cb60f..fd799980c9419 100644
> --- a/include/drm/intel/display_parent_interface.h
> +++ b/include/drm/intel/display_parent_interface.h
> @@ -58,6 +58,7 @@ struct intel_display_bo_interface {
> struct drm_file *filp,
> const struct drm_mode_fb_cmd2 *user_mode_cmd);
> #if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
> + struct drm_gem_object *(*fbdev_bios_fb_takeover)(struct drm_gem_object *obj);
> struct drm_gem_object *(*fbdev_create)(struct drm_device *drm, int size);
> void (*fbdev_destroy)(struct drm_gem_object *obj);
> int (*fbdev_fill_info)(struct drm_gem_object *obj, struct fb_info *info, struct i915_vma *vma);
> --
> 2.55.0
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2026-11-13 13:21 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-13 12:44 [PATCH v5 0/5] drm/xe/display: Transparant fallback from stolen to sysmem Maarten Lankhorst
2026-11-13 12:44 ` [PATCH v5 1/5] drm/xe/display: Avoid using stolen memory for framebuffer when media gt exists Maarten Lankhorst
2026-11-13 12:44 ` [PATCH v5 2/5] drm/xe/migrate: Support copying between sysmem and stolen Maarten Lankhorst
2026-11-13 13:05 ` sashiko-bot
2026-11-13 12:44 ` [PATCH v5 3/5] drm/xe: Raise gt frequency slightly earlier Maarten Lankhorst
2026-11-13 13:30 ` Ville Syrjälä
2026-11-13 12:44 ` [PATCH v5 4/5] HACK: Always fallback Maarten Lankhorst
2026-11-13 12:50 ` sashiko-bot
2026-11-13 12:44 ` [PATCH v5 5/5] drm/i915: Introduce intel_bo_fbdev_bios_fb_takeover() Maarten Lankhorst
2026-11-13 13:21 ` Ville Syrjälä [this message]
2026-11-13 13:22 ` sashiko-bot
2026-11-13 13:22 ` ✗ CI.checkpatch: warning for drm/xe/display: Transparant fallback from stolen to sysmem. (rev5) Patchwork
2026-11-13 13:24 ` ✓ CI.KUnit: success " Patchwork
2026-11-13 13:56 ` ✗ Xe.CI.BAT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=avcO035BB7RU_ow0@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=dev@lankhorst.se \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=uma.shankar@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox