public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: "Kandpal, Suraj" <suraj.kandpal@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Subject: RE: [PATCH 5/5] drm/{i915, xe}: move framebuffer bo to parent interface
Date: Thu, 12 Mar 2026 11:00:57 +0200	[thread overview]
Message-ID: <67ef5d21f50f0d9cba26fca7b197d37fbbd4877d@intel.com> (raw)
In-Reply-To: <DM3PPF208195D8D01AE643B28731FE6CFDAE344A@DM3PPF208195D8D.namprd11.prod.outlook.com>

On Thu, 12 Mar 2026, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> Subject: [PATCH 5/5] drm/{i915,xe}: move framebuffer bo to parent interface
>> 
>> Add .framebuffer_init, .framebuffer_fini and .framebuffer_lookup to the bo
>> parent interface. While they're about framebuffers, they're specifically about
>> framebuffer objects, so the bo interface is a good enough fit, and there's no
>> need to add another interface struct.
>
> Maybe it can also be mentioned that since we move all the functions from
> Intel_fb_bo to i915_bo.c we can safely remove it.

Sure.

>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/Makefile                |  1 -
>>  drivers/gpu/drm/i915/display/intel_bo.c      | 21 +++++
>>  drivers/gpu/drm/i915/display/intel_bo.h      |  9 ++
>>  drivers/gpu/drm/i915/display/intel_fb.c      | 12 +--
>>  drivers/gpu/drm/i915/display/intel_fb_bo.c   | 99 --------------------
>>  drivers/gpu/drm/i915/display/intel_fb_bo.h   | 25 -----
>>  drivers/gpu/drm/i915/i915_bo.c               | 92 ++++++++++++++++++
>>  drivers/gpu/drm/xe/Makefile                  |  1 -
>>  drivers/gpu/drm/xe/display/intel_fb_bo.c     | 91 ------------------
>>  drivers/gpu/drm/xe/display/xe_display_bo.c   | 84 +++++++++++++++++
>>  include/drm/intel/display_parent_interface.h |  6 ++
>>  11 files changed, 218 insertions(+), 223 deletions(-)  delete mode 100644
>> drivers/gpu/drm/i915/display/intel_fb_bo.c
>>  delete mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.h
>>  delete mode 100644 drivers/gpu/drm/xe/display/intel_fb_bo.c
>> 
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 425933fb26a5..be976a90c5a6 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -278,7 +278,6 @@ i915-y += \
>>  	display/intel_drrs.o \
>>  	display/intel_dsb.o \
>>  	display/intel_fb.o \
>> -	display/intel_fb_bo.o \
>>  	display/intel_fb_pin.o \
>>  	display/intel_fbc.o \
>>  	display/intel_fdi.o \
>> diff --git a/drivers/gpu/drm/i915/display/intel_bo.c
>> b/drivers/gpu/drm/i915/display/intel_bo.c
>> index e356ab4e0640..3b82d38a0504 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bo.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bo.c
>> @@ -64,3 +64,24 @@ void intel_bo_describe(struct seq_file *m, struct
>> drm_gem_object *obj)
>>  	if (display->parent->bo->describe)
>>  		display->parent->bo->describe(m, obj);  }
>> +
>> +int intel_bo_framebuffer_init(struct drm_gem_object *obj, struct
>> +drm_mode_fb_cmd2 *mode_cmd) {
>> +	struct intel_display *display = to_intel_display(obj->dev);
>> +
>> +	return display->parent->bo->framebuffer_init(obj, mode_cmd); }
>> +
>> +void intel_bo_framebuffer_fini(struct drm_gem_object *obj) {
>> +	struct intel_display *display = to_intel_display(obj->dev);
>> +
>> +	display->parent->bo->framebuffer_fini(obj);
>
> Should we be making this optional . This will help avoid creating dummy functions
> for fini like you have done previously.

I have this gnawing feeling that it should not be a dummy function in
the first place. I haven't actually looked into the bottom of it. That's
why I ended up keeping the dummy.

BR,
Jani.


>
> Regards,
> Suraj Kandpal
>
>> +}
>> +
>> +struct drm_gem_object *intel_bo_framebuffer_lookup(struct intel_display
>> *display,
>> +						   struct drm_file *filp,
>> +						   const struct
>> drm_mode_fb_cmd2 *user_mode_cmd) {
>> +	return display->parent->bo->framebuffer_lookup(display->drm, filp,
>> +user_mode_cmd); }
>> diff --git a/drivers/gpu/drm/i915/display/intel_bo.h
>> b/drivers/gpu/drm/i915/display/intel_bo.h
>> index 40390ed92ceb..aec188c706c2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bo.h
>> +++ b/drivers/gpu/drm/i915/display/intel_bo.h
>> @@ -6,8 +6,11 @@
>> 
>>  #include <linux/types.h>
>> 
>> +struct drm_file;
>>  struct drm_gem_object;
>> +struct drm_mode_fb_cmd2;
>>  struct drm_scanout_buffer;
>> +struct intel_display;
>>  struct intel_framebuffer;
>>  struct seq_file;
>>  struct vm_area_struct;
>> @@ -22,4 +25,10 @@ int intel_bo_read_from_page(struct drm_gem_object
>> *obj, u64 offset, void *dst, i
>> 
>>  void intel_bo_describe(struct seq_file *m, struct drm_gem_object *obj);
>> 
>> +void intel_bo_framebuffer_fini(struct drm_gem_object *obj); int
>> +intel_bo_framebuffer_init(struct drm_gem_object *obj, struct
>> +drm_mode_fb_cmd2 *mode_cmd); struct drm_gem_object
>> *intel_bo_framebuffer_lookup(struct intel_display *display,
>> +						   struct drm_file *filp,
>> +						   const struct
>> drm_mode_fb_cmd2 *user_mode_cmd);
>> +
>>  #endif /* __INTEL_BO__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
>> b/drivers/gpu/drm/i915/display/intel_fb.c
>> index 49c6ca9d94c6..5768619f840f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
>> @@ -17,7 +17,6 @@
>>  #include "intel_display_types.h"
>>  #include "intel_display_utils.h"
>>  #include "intel_fb.h"
>> -#include "intel_fb_bo.h"
>>  #include "intel_frontbuffer.h"
>>  #include "intel_parent.h"
>>  #include "intel_plane.h"
>> @@ -2111,7 +2110,7 @@ static void intel_user_framebuffer_destroy(struct
>> drm_framebuffer *fb)
>>  	if (intel_fb_uses_dpt(fb))
>>  		intel_parent_dpt_destroy(display, intel_fb->dpt);
>> 
>> -	intel_fb_bo_framebuffer_fini(intel_fb_bo(fb));
>> +	intel_bo_framebuffer_fini(intel_fb_bo(fb));
>> 
>>  	intel_parent_frontbuffer_put(display, intel_fb->frontbuffer);
>> 
>> @@ -2222,7 +2221,7 @@ int intel_framebuffer_init(struct intel_framebuffer
>> *intel_fb,
>> 
>>  	/*
>>  	 * intel_parent_frontbuffer_get() must be done before
>> -	 * intel_fb_bo_framebuffer_init() to avoid set_tiling vs. addfb race.
>> +	 * intel_bo_framebuffer_init() to avoid set_tiling vs. addfb race.
>>  	 */
>>  	intel_fb->frontbuffer = intel_parent_frontbuffer_get(display, obj);
>>  	if (!intel_fb->frontbuffer) {
>> @@ -2230,7 +2229,7 @@ int intel_framebuffer_init(struct intel_framebuffer
>> *intel_fb,
>>  		goto err_free_panic;
>>  	}
>> 
>> -	ret = intel_fb_bo_framebuffer_init(obj, mode_cmd);
>> +	ret = intel_bo_framebuffer_init(obj, mode_cmd);
>>  	if (ret)
>>  		goto err_frontbuffer_put;
>> 
>> @@ -2333,7 +2332,7 @@ int intel_framebuffer_init(struct intel_framebuffer
>> *intel_fb,
>>  	if (intel_fb_uses_dpt(fb))
>>  		intel_parent_dpt_destroy(display, intel_fb->dpt);
>>  err_bo_framebuffer_fini:
>> -	intel_fb_bo_framebuffer_fini(obj);
>> +	intel_bo_framebuffer_fini(obj);
>>  err_frontbuffer_put:
>>  	intel_parent_frontbuffer_put(display, intel_fb->frontbuffer);
>>  err_free_panic:
>> @@ -2348,11 +2347,12 @@ intel_user_framebuffer_create(struct
>> drm_device *dev,
>>  			      const struct drm_format_info *info,
>>  			      const struct drm_mode_fb_cmd2
>> *user_mode_cmd)  {
>> +	struct intel_display *display = to_intel_display(dev);
>>  	struct drm_framebuffer *fb;
>>  	struct drm_gem_object *obj;
>>  	struct drm_mode_fb_cmd2 mode_cmd = *user_mode_cmd;
>> 
>> -	obj = intel_fb_bo_lookup_valid_bo(dev, filp, &mode_cmd);
>> +	obj = intel_bo_framebuffer_lookup(display, filp, &mode_cmd);
>>  	if (IS_ERR(obj))
>>  		return ERR_CAST(obj);
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_fb_bo.c
>> b/drivers/gpu/drm/i915/display/intel_fb_bo.c
>> deleted file mode 100644
>> index a4d49ef450d9..000000000000
>> --- a/drivers/gpu/drm/i915/display/intel_fb_bo.c
>> +++ /dev/null
>> @@ -1,99 +0,0 @@
>> -/* SPDX-License-Identifier: MIT */
>> -/*
>> - * Copyright © 2021 Intel Corporation
>> - */
>> -
>> -#include <drm/drm_framebuffer.h>
>> -#include <drm/drm_print.h>
>> -
>> -#include "gem/i915_gem_object.h"
>> -
>> -#include "i915_drv.h"
>> -#include "intel_fb.h"
>> -#include "intel_fb_bo.h"
>> -
>> -void intel_fb_bo_framebuffer_fini(struct drm_gem_object *obj) -{
>> -	/* Nothing to do for i915 */
>> -}
>> -
>> -int intel_fb_bo_framebuffer_init(struct drm_gem_object *_obj,
>> -				 struct drm_mode_fb_cmd2 *mode_cmd)
>> -{
>> -	struct drm_i915_gem_object *obj = to_intel_bo(_obj);
>> -	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> -	unsigned int tiling, stride;
>> -
>> -	i915_gem_object_lock(obj, NULL);
>> -	tiling = i915_gem_object_get_tiling(obj);
>> -	stride = i915_gem_object_get_stride(obj);
>> -	i915_gem_object_unlock(obj);
>> -
>> -	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
>> -		/*
>> -		 * If there's a fence, enforce that
>> -		 * the fb modifier and tiling mode match.
>> -		 */
>> -		if (tiling != I915_TILING_NONE &&
>> -		    tiling != intel_fb_modifier_to_tiling(mode_cmd-
>> >modifier[0])) {
>> -			drm_dbg_kms(&i915->drm,
>> -				    "tiling_mode doesn't match fb modifier\n");
>> -			return -EINVAL;
>> -		}
>> -	} else {
>> -		if (tiling == I915_TILING_X) {
>> -			mode_cmd->modifier[0] =
>> I915_FORMAT_MOD_X_TILED;
>> -		} else if (tiling == I915_TILING_Y) {
>> -			drm_dbg_kms(&i915->drm,
>> -				    "No Y tiling for legacy addfb\n");
>> -			return -EINVAL;
>> -		}
>> -	}
>> -
>> -	/*
>> -	 * gen2/3 display engine uses the fence if present,
>> -	 * so the tiling mode must match the fb modifier exactly.
>> -	 */
>> -	if (GRAPHICS_VER(i915) < 4 &&
>> -	    tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
>> -		drm_dbg_kms(&i915->drm,
>> -			    "tiling_mode must match fb modifier exactly on
>> gen2/3\n");
>> -		return -EINVAL;
>> -	}
>> -
>> -	/*
>> -	 * If there's a fence, enforce that
>> -	 * the fb pitch and fence stride match.
>> -	 */
>> -	if (tiling != I915_TILING_NONE && mode_cmd->pitches[0] != stride) {
>> -		drm_dbg_kms(&i915->drm,
>> -			    "pitch (%d) must match tiling stride (%d)\n",
>> -			    mode_cmd->pitches[0], stride);
>> -		return -EINVAL;
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>> -struct drm_gem_object *
>> -intel_fb_bo_lookup_valid_bo(struct drm_device *drm,
>> -			    struct drm_file *filp,
>> -			    const struct drm_mode_fb_cmd2 *mode_cmd)
>> -{
>> -	struct drm_i915_private *i915 = to_i915(drm);
>> -	struct drm_i915_gem_object *obj;
>> -
>> -	obj = i915_gem_object_lookup(filp, mode_cmd->handles[0]);
>> -	if (!obj)
>> -		return ERR_PTR(-ENOENT);
>> -
>> -	/* object is backed with LMEM for discrete */
>> -	if (HAS_LMEM(i915) && !i915_gem_object_can_migrate(obj,
>> INTEL_REGION_LMEM_0)) {
>> -		/* object is "remote", not in local memory */
>> -		i915_gem_object_put(obj);
>> -		drm_dbg_kms(&i915->drm, "framebuffer must reside in local
>> memory\n");
>> -		return ERR_PTR(-EREMOTE);
>> -	}
>> -
>> -	return intel_bo_to_drm_bo(obj);
>> -}
>> diff --git a/drivers/gpu/drm/i915/display/intel_fb_bo.h
>> b/drivers/gpu/drm/i915/display/intel_fb_bo.h
>> deleted file mode 100644
>> index d775773c6c03..000000000000
>> --- a/drivers/gpu/drm/i915/display/intel_fb_bo.h
>> +++ /dev/null
>> @@ -1,25 +0,0 @@
>> -/* SPDX-License-Identifier: MIT */
>> -/*
>> - * Copyright © 2021 Intel Corporation
>> - */
>> -
>> -#ifndef __INTEL_FB_BO_H__
>> -#define __INTEL_FB_BO_H__
>> -
>> -struct drm_device;
>> -struct drm_file;
>> -struct drm_framebuffer;
>> -struct drm_gem_object;
>> -struct drm_mode_fb_cmd2;
>> -
>> -void intel_fb_bo_framebuffer_fini(struct drm_gem_object *obj);
>> -
>> -int intel_fb_bo_framebuffer_init(struct drm_gem_object *obj,
>> -				 struct drm_mode_fb_cmd2 *mode_cmd);
>> -
>> -struct drm_gem_object *
>> -intel_fb_bo_lookup_valid_bo(struct drm_device *drm,
>> -			    struct drm_file *filp,
>> -			    const struct drm_mode_fb_cmd2
>> *user_mode_cmd);
>> -
>> -#endif
>> diff --git a/drivers/gpu/drm/i915/i915_bo.c
>> b/drivers/gpu/drm/i915/i915_bo.c index 04fc0e3b7ef6..1789f7cab05c
>> 100644
>> --- a/drivers/gpu/drm/i915/i915_bo.c
>> +++ b/drivers/gpu/drm/i915/i915_bo.c
>> @@ -2,8 +2,10 @@
>>  /* Copyright © 2024 Intel Corporation */
>> 
>>  #include <drm/drm_panic.h>
>> +#include <drm/drm_print.h>
>>  #include <drm/intel/display_parent_interface.h>
>> 
>> +#include "display/intel_fb.h"
>>  #include "gem/i915_gem_mman.h"
>>  #include "gem/i915_gem_object.h"
>>  #include "gem/i915_gem_object_frontbuffer.h"
>> @@ -11,6 +13,7 @@
>> 
>>  #include "i915_bo.h"
>>  #include "i915_debugfs.h"
>> +#include "i915_drv.h"
>> 
>>  static bool i915_bo_is_tiled(struct drm_gem_object *obj)  { @@ -52,6 +55,92
>> @@ static void i915_bo_describe(struct seq_file *m, struct drm_gem_object
>> *obj)
>>  	i915_debugfs_describe_obj(m, to_intel_bo(obj));  }
>> 
>> +static int i915_bo_framebuffer_init(struct drm_gem_object *_obj,
>> +				    struct drm_mode_fb_cmd2 *mode_cmd) {
>> +	struct drm_i915_gem_object *obj = to_intel_bo(_obj);
>> +	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> +	unsigned int tiling, stride;
>> +
>> +	i915_gem_object_lock(obj, NULL);
>> +	tiling = i915_gem_object_get_tiling(obj);
>> +	stride = i915_gem_object_get_stride(obj);
>> +	i915_gem_object_unlock(obj);
>> +
>> +	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
>> +		/*
>> +		 * If there's a fence, enforce that
>> +		 * the fb modifier and tiling mode match.
>> +		 */
>> +		if (tiling != I915_TILING_NONE &&
>> +		    tiling != intel_fb_modifier_to_tiling(mode_cmd-
>> >modifier[0])) {
>> +			drm_dbg_kms(&i915->drm,
>> +				    "tiling_mode doesn't match fb modifier\n");
>> +			return -EINVAL;
>> +		}
>> +	} else {
>> +		if (tiling == I915_TILING_X) {
>> +			mode_cmd->modifier[0] =
>> I915_FORMAT_MOD_X_TILED;
>> +		} else if (tiling == I915_TILING_Y) {
>> +			drm_dbg_kms(&i915->drm,
>> +				    "No Y tiling for legacy addfb\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * gen2/3 display engine uses the fence if present,
>> +	 * so the tiling mode must match the fb modifier exactly.
>> +	 */
>> +	if (GRAPHICS_VER(i915) < 4 &&
>> +	    tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
>> +		drm_dbg_kms(&i915->drm,
>> +			    "tiling_mode must match fb modifier exactly on
>> gen2/3\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/*
>> +	 * If there's a fence, enforce that
>> +	 * the fb pitch and fence stride match.
>> +	 */
>> +	if (tiling != I915_TILING_NONE && mode_cmd->pitches[0] != stride) {
>> +		drm_dbg_kms(&i915->drm,
>> +			    "pitch (%d) must match tiling stride (%d)\n",
>> +			    mode_cmd->pitches[0], stride);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void i915_bo_framebuffer_fini(struct drm_gem_object *obj) {
>> +	/* Nothing to do for i915 */
>> +}
>> +
>> +static struct drm_gem_object *
>> +i915_bo_framebuffer_lookup(struct drm_device *drm,
>> +			   struct drm_file *filp,
>> +			   const struct drm_mode_fb_cmd2 *mode_cmd) {
>> +	struct drm_i915_private *i915 = to_i915(drm);
>> +	struct drm_i915_gem_object *obj;
>> +
>> +	obj = i915_gem_object_lookup(filp, mode_cmd->handles[0]);
>> +	if (!obj)
>> +		return ERR_PTR(-ENOENT);
>> +
>> +	/* object is backed with LMEM for discrete */
>> +	if (HAS_LMEM(i915) && !i915_gem_object_can_migrate(obj,
>> INTEL_REGION_LMEM_0)) {
>> +		/* object is "remote", not in local memory */
>> +		i915_gem_object_put(obj);
>> +		drm_dbg_kms(&i915->drm, "framebuffer must reside in local
>> memory\n");
>> +		return ERR_PTR(-EREMOTE);
>> +	}
>> +
>> +	return intel_bo_to_drm_bo(obj);
>> +}
>> +
>>  const struct intel_display_bo_interface i915_display_bo_interface = {
>>  	.is_tiled = i915_bo_is_tiled,
>>  	.is_userptr = i915_bo_is_userptr,
>> @@ -61,4 +150,7 @@ const struct intel_display_bo_interface
>> i915_display_bo_interface = {
>>  	.fb_mmap = i915_bo_fb_mmap,
>>  	.read_from_page = i915_bo_read_from_page,
>>  	.describe = i915_bo_describe,
>> +	.framebuffer_init = i915_bo_framebuffer_init,
>> +	.framebuffer_fini = i915_bo_framebuffer_fini,
>> +	.framebuffer_lookup = i915_bo_framebuffer_lookup,
>>  };
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index b16ed1ce2a85..dab979287a96 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -211,7 +211,6 @@ $(obj)/i915-display/%.o:
>> $(srctree)/drivers/gpu/drm/i915/display/%.c FORCE
>> 
>>  # Display code specific to xe
>>  xe-$(CONFIG_DRM_XE_DISPLAY) += \
>> -	display/intel_fb_bo.o \
>>  	display/intel_fbdev_fb.o \
>>  	display/xe_display.o \
>>  	display/xe_display_bo.o \
>> diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> b/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> deleted file mode 100644
>> index db8b1a27b4de..000000000000
>> --- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> +++ /dev/null
>> @@ -1,91 +0,0 @@
>> -/* SPDX-License-Identifier: MIT */
>> -/*
>> - * Copyright © 2021 Intel Corporation
>> - */
>> -
>> -#include <drm/drm_modeset_helper.h>
>> -#include <drm/ttm/ttm_bo.h>
>> -
>> -#include "intel_display_types.h"
>> -#include "intel_fb.h"
>> -#include "intel_fb_bo.h"
>> -#include "xe_bo.h"
>> -
>> -void intel_fb_bo_framebuffer_fini(struct drm_gem_object *obj) -{
>> -	struct xe_bo *bo = gem_to_xe_bo(obj);
>> -
>> -	if (bo->flags & XE_BO_FLAG_PINNED) {
>> -		/* Unpin our kernel fb first */
>> -		xe_bo_lock(bo, false);
>> -		xe_bo_unpin(bo);
>> -		xe_bo_unlock(bo);
>> -	}
>> -	xe_bo_put(bo);
>> -}
>> -
>> -int intel_fb_bo_framebuffer_init(struct drm_gem_object *obj,
>> -				 struct drm_mode_fb_cmd2 *mode_cmd)
>> -{
>> -	struct xe_bo *bo = gem_to_xe_bo(obj);
>> -	struct xe_device *xe = to_xe_device(bo->ttm.base.dev);
>> -	int ret;
>> -
>> -	/*
>> -	 * Some modifiers require physical alignment of 64KiB VRAM pages;
>> -	 * require that the BO in those cases is created correctly.
>> -	 */
>> -	if (XE_IOCTL_DBG(xe, intel_fb_needs_64k_phys(mode_cmd-
>> >modifier[0]) &&
>> -			     !(bo->flags & XE_BO_FLAG_NEEDS_64K)))
>> -		return -EINVAL;
>> -
>> -	xe_bo_get(bo);
>> -
>> -	ret = ttm_bo_reserve(&bo->ttm, true, false, NULL);
>> -	if (ret)
>> -		goto err;
>> -
>> -	if (!(bo->flags & XE_BO_FLAG_SCANOUT)) {
>> -		/*
>> -		 * XE_BO_FLAG_SCANOUT should ideally be set at creation,
>> or is
>> -		 * automatically set when creating FB. We cannot change
>> caching
>> -		 * mode when the bo is VM_BINDed, so we can only set
>> -		 * coherency with display when unbound.
>> -		 */
>> -		if (XE_IOCTL_DBG(xe, xe_bo_is_vm_bound(bo))) {
>> -			ttm_bo_unreserve(&bo->ttm);
>> -			ret = -EINVAL;
>> -			goto err;
>> -		}
>> -		bo->flags |= XE_BO_FLAG_SCANOUT;
>> -	}
>> -	ttm_bo_unreserve(&bo->ttm);
>> -	return 0;
>> -
>> -err:
>> -	xe_bo_put(bo);
>> -	return ret;
>> -}
>> -
>> -struct drm_gem_object *intel_fb_bo_lookup_valid_bo(struct drm_device
>> *drm,
>> -						   struct drm_file *filp,
>> -						   const struct
>> drm_mode_fb_cmd2 *mode_cmd)
>> -{
>> -	struct xe_device *xe = to_xe_device(drm);
>> -	struct xe_bo *bo;
>> -	struct drm_gem_object *gem = drm_gem_object_lookup(filp,
>> mode_cmd->handles[0]);
>> -
>> -	if (!gem)
>> -		return ERR_PTR(-ENOENT);
>> -
>> -	bo = gem_to_xe_bo(gem);
>> -	/* Require vram placement or dma-buf import */
>> -	if (IS_DGFX(xe) &&
>> -	    !xe_bo_can_migrate(bo, XE_PL_VRAM0) &&
>> -	    bo->ttm.type != ttm_bo_type_sg) {
>> -		drm_gem_object_put(gem);
>> -		return ERR_PTR(-EREMOTE);
>> -	}
>> -
>> -	return gem;
>> -}
>> diff --git a/drivers/gpu/drm/xe/display/xe_display_bo.c
>> b/drivers/gpu/drm/xe/display/xe_display_bo.c
>> index a53ba3f247ec..a689f71e7b14 100644
>> --- a/drivers/gpu/drm/xe/display/xe_display_bo.c
>> +++ b/drivers/gpu/drm/xe/display/xe_display_bo.c
>> @@ -4,6 +4,7 @@
>>  #include <drm/drm_gem.h>
>>  #include <drm/intel/display_parent_interface.h>
>> 
>> +#include "intel_fb.h"
>>  #include "xe_bo.h"
>>  #include "xe_display_bo.h"
>>  #include "xe_pxp.h"
>> @@ -20,9 +21,92 @@ static int xe_display_bo_read_from_page(struct
>> drm_gem_object *obj, u64 offset,
>>  	return xe_bo_read(bo, offset, dst, size);  }
>> 
>> +static int xe_display_bo_framebuffer_init(struct drm_gem_object *obj,
>> +					  struct drm_mode_fb_cmd2
>> *mode_cmd) {
>> +	struct xe_bo *bo = gem_to_xe_bo(obj);
>> +	struct xe_device *xe = to_xe_device(bo->ttm.base.dev);
>> +	int ret;
>> +
>> +	/*
>> +	 * Some modifiers require physical alignment of 64KiB VRAM pages;
>> +	 * require that the BO in those cases is created correctly.
>> +	 */
>> +	if (XE_IOCTL_DBG(xe, intel_fb_needs_64k_phys(mode_cmd-
>> >modifier[0]) &&
>> +			     !(bo->flags & XE_BO_FLAG_NEEDS_64K)))
>> +		return -EINVAL;
>> +
>> +	xe_bo_get(bo);
>> +
>> +	ret = ttm_bo_reserve(&bo->ttm, true, false, NULL);
>> +	if (ret)
>> +		goto err;
>> +
>> +	if (!(bo->flags & XE_BO_FLAG_SCANOUT)) {
>> +		/*
>> +		 * XE_BO_FLAG_SCANOUT should ideally be set at creation,
>> or is
>> +		 * automatically set when creating FB. We cannot change
>> caching
>> +		 * mode when the bo is VM_BINDed, so we can only set
>> +		 * coherency with display when unbound.
>> +		 */
>> +		if (XE_IOCTL_DBG(xe, xe_bo_is_vm_bound(bo))) {
>> +			ttm_bo_unreserve(&bo->ttm);
>> +			ret = -EINVAL;
>> +			goto err;
>> +		}
>> +		bo->flags |= XE_BO_FLAG_SCANOUT;
>> +	}
>> +	ttm_bo_unreserve(&bo->ttm);
>> +	return 0;
>> +
>> +err:
>> +	xe_bo_put(bo);
>> +	return ret;
>> +}
>> +
>> +static void xe_display_bo_framebuffer_fini(struct drm_gem_object *obj)
>> +{
>> +	struct xe_bo *bo = gem_to_xe_bo(obj);
>> +
>> +	if (bo->flags & XE_BO_FLAG_PINNED) {
>> +		/* Unpin our kernel fb first */
>> +		xe_bo_lock(bo, false);
>> +		xe_bo_unpin(bo);
>> +		xe_bo_unlock(bo);
>> +	}
>> +	xe_bo_put(bo);
>> +}
>> +
>> +static struct drm_gem_object *
>> +xe_display_bo_framebuffer_lookup(struct drm_device *drm,
>> +				 struct drm_file *filp,
>> +				 const struct drm_mode_fb_cmd2
>> *mode_cmd) {
>> +	struct xe_device *xe = to_xe_device(drm);
>> +	struct xe_bo *bo;
>> +	struct drm_gem_object *gem = drm_gem_object_lookup(filp,
>> +mode_cmd->handles[0]);
>> +
>> +	if (!gem)
>> +		return ERR_PTR(-ENOENT);
>> +
>> +	bo = gem_to_xe_bo(gem);
>> +	/* Require vram placement or dma-buf import */
>> +	if (IS_DGFX(xe) &&
>> +	    !xe_bo_can_migrate(bo, XE_PL_VRAM0) &&
>> +	    bo->ttm.type != ttm_bo_type_sg) {
>> +		drm_gem_object_put(gem);
>> +		return ERR_PTR(-EREMOTE);
>> +	}
>> +
>> +	return gem;
>> +}
>> +
>>  const struct intel_display_bo_interface xe_display_bo_interface = {
>>  	.is_protected = xe_display_bo_is_protected,
>>  	.key_check = xe_pxp_obj_key_check,
>>  	.fb_mmap = drm_gem_prime_mmap,
>>  	.read_from_page = xe_display_bo_read_from_page,
>> +	.framebuffer_init = xe_display_bo_framebuffer_init,
>> +	.framebuffer_fini = xe_display_bo_framebuffer_fini,
>> +	.framebuffer_lookup = xe_display_bo_framebuffer_lookup,
>>  };
>> diff --git a/include/drm/intel/display_parent_interface.h
>> b/include/drm/intel/display_parent_interface.h
>> index 2b53d12b0e0a..97ec94a2e749 100644
>> --- a/include/drm/intel/display_parent_interface.h
>> +++ b/include/drm/intel/display_parent_interface.h
>> @@ -12,6 +12,7 @@ struct drm_device;
>>  struct drm_file;
>>  struct drm_framebuffer;
>>  struct drm_gem_object;
>> +struct drm_mode_fb_cmd2;
>>  struct drm_plane_state;
>>  struct drm_scanout_buffer;
>>  struct i915_vma;
>> @@ -37,6 +38,11 @@ struct intel_display_bo_interface {
>>  	int (*fb_mmap)(struct drm_gem_object *obj, struct vm_area_struct
>> *vma);
>>  	int (*read_from_page)(struct drm_gem_object *obj, u64 offset, void
>> *dst, int size);
>>  	void (*describe)(struct seq_file *m, struct drm_gem_object *obj); /*
>> Optional */
>> +	int (*framebuffer_init)(struct drm_gem_object *obj, struct
>> drm_mode_fb_cmd2 *mode_cmd);
>> +	void (*framebuffer_fini)(struct drm_gem_object *obj);
>> +	struct drm_gem_object *(*framebuffer_lookup)(struct drm_device
>> *drm,
>> +						     struct drm_file *filp,
>> +						     const struct
>> drm_mode_fb_cmd2 *user_mode_cmd);
>>  };
>> 
>>  struct intel_display_dpt_interface {
>> --
>> 2.47.3
>

-- 
Jani Nikula, Intel

  reply	other threads:[~2026-03-12  9:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 14:18 [PATCH 0/5] drm/{i915,xe}: move bo stuff to parent interface Jani Nikula
2026-03-11 14:18 ` [PATCH 1/5] drm/i915: move i915 specific bo implementation to i915 Jani Nikula
2026-03-12  3:39   ` Kandpal, Suraj
2026-03-12  3:52     ` Kandpal, Suraj
2026-03-11 14:18 ` [PATCH 2/5] drm/xe: rename intel_bo.c to xe_display_bo.c Jani Nikula
2026-03-12  3:40   ` Kandpal, Suraj
2026-03-11 14:18 ` [PATCH 3/5] drm/{i915, xe}/bo: move display bo calls to parent interface Jani Nikula
2026-03-12  3:47   ` Kandpal, Suraj
2026-03-12  4:27     ` Kandpal, Suraj
2026-03-11 14:18 ` [PATCH 4/5] drm/i915/fb: make intel_fb_bo.c less dependent on display Jani Nikula
2026-03-12  3:51   ` Kandpal, Suraj
2026-03-11 14:18 ` [PATCH 5/5] drm/{i915,xe}: move framebuffer bo to parent interface Jani Nikula
2026-03-12  4:25   ` Kandpal, Suraj
2026-03-12  9:00     ` Jani Nikula [this message]
2026-03-13  5:12       ` Kandpal, Suraj
2026-03-11 15:33 ` ✓ i915.CI.BAT: success for drm/{i915,xe}: move bo stuff " Patchwork
2026-03-12  6:37 ` ✓ i915.CI.Full: " 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=67ef5d21f50f0d9cba26fca7b197d37fbbd4877d@intel.com \
    --to=jani.nikula@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=suraj.kandpal@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