Re: [PATCH 7/9] drm/{i915,xe}/fbdev: deduplicate fbdev creation

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Sep 03, 2025 at 11:32:04PM +0300, Jani Nikula wrote:
> With the bo creation helper in place, we can lift
> intel_framebuffer_create() part to common code.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbdev.c    | 31 ++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_fbdev_fb.c | 29 ---------------
>  drivers/gpu/drm/i915/display/intel_fbdev_fb.h |  2 --
>  drivers/gpu/drm/xe/display/intel_fbdev_fb.c   | 35 -------------------
>  4 files changed, 28 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 5ac66fb88fcf..d7131de5c7ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -227,13 +227,38 @@ __intel_fbdev_fb_alloc(struct intel_display *display,
>  		       struct drm_fb_helper_surface_size *sizes)
>  {
>  	struct drm_mode_fb_cmd2 mode_cmd = {};
> -	struct intel_framebuffer *fb;
> +	struct drm_framebuffer *fb;
> +	struct drm_gem_object *obj;
> +	int size;
>  
>  	intel_fbdev_fill_mode_cmd(sizes, &mode_cmd);
>  
> -	fb = intel_fbdev_fb_alloc(display->drm, &mode_cmd);
> +	size = mode_cmd.pitches[0] * mode_cmd.height;
> +	size = PAGE_ALIGN(size);
> +
> +	obj = intel_fbdev_fb_bo_create(display->drm, size);
> +	if (IS_ERR(obj)) {
> +		fb = ERR_CAST(obj);
> +		goto err;

This use of goto doesn't make much sense to me in this function.

> +	}
> +
> +	fb = intel_framebuffer_create(obj,
> +				      drm_get_format_info(display->drm,
> +							  mode_cmd.pixel_format,
> +							  mode_cmd.modifier[0]),
> +				      &mode_cmd);
> +	if (IS_ERR(fb)) {
> +		intel_fbdev_fb_bo_destroy(obj);
> +		goto err;
> +	}
> +
> +	drm_gem_object_put(obj);
> +
> +	return to_intel_framebuffer(fb);
> +
> +err:
> +	return ERR_CAST(fb);
>  
> -	return fb;
>  }
>  
>  int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> index 6b70823ce5ef..1ac62a064936 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> @@ -10,7 +10,6 @@
>  #include "i915_drv.h"
>  #include "intel_display_core.h"
>  #include "intel_display_types.h"
> -#include "intel_fb.h"
>  #include "intel_fbdev_fb.h"
>  
>  u32 intel_fbdev_fb_pitch_align(u32 stride)
> @@ -56,34 +55,6 @@ void intel_fbdev_fb_bo_destroy(struct drm_gem_object *obj)
>  	/* nop? */
>  }
>  
> -struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
> -					       struct drm_mode_fb_cmd2 *mode_cmd)
> -{
> -	struct drm_framebuffer *fb;
> -	struct drm_gem_object *obj;
> -	int size;
> -
> -	size = mode_cmd->pitches[0] * mode_cmd->height;
> -	size = PAGE_ALIGN(size);
> -
> -	obj = intel_fbdev_fb_bo_create(drm, size);
> -	if (IS_ERR(obj)) {
> -		fb = ERR_CAST(obj);
> -		goto err;
> -	}
> -
> -	fb = intel_framebuffer_create(obj,
> -				      drm_get_format_info(drm,
> -							  mode_cmd->pixel_format,
> -							  mode_cmd->modifier[0]),
> -				      mode_cmd);
> -	drm_gem_object_put(obj);
> -
> -	return to_intel_framebuffer(fb);
> -err:
> -	return ERR_CAST(fb);
> -}
> -
>  int intel_fbdev_fb_fill_info(struct intel_display *display, struct fb_info *info,
>  			     struct drm_gem_object *_obj, struct i915_vma *vma)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> index 6d6f316834df..bb0ce3a85ff9 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> @@ -18,8 +18,6 @@ struct intel_display;
>  u32 intel_fbdev_fb_pitch_align(u32 stride);
>  struct drm_gem_object *intel_fbdev_fb_bo_create(struct drm_device *drm, int size);
>  void intel_fbdev_fb_bo_destroy(struct drm_gem_object *obj);
> -struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
> -					       struct drm_mode_fb_cmd2 *mode_cmd);
>  int intel_fbdev_fb_fill_info(struct intel_display *display, struct fb_info *info,
>  			     struct drm_gem_object *obj, struct i915_vma *vma);
>  
> diff --git a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> index 9a5cf50ea7de..2e8490a8cdb1 100644
> --- a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> +++ b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> @@ -7,7 +7,6 @@
>  
>  #include "intel_display_core.h"
>  #include "intel_display_types.h"
> -#include "intel_fb.h"
>  #include "intel_fbdev_fb.h"
>  #include "xe_bo.h"
>  #include "xe_ttm_stolen_mgr.h"
> @@ -59,40 +58,6 @@ void intel_fbdev_fb_bo_destroy(struct drm_gem_object *obj)
>  	xe_bo_unpin_map_no_vm(gem_to_xe_bo(obj));
>  }
>  
> -struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
> -					       struct drm_mode_fb_cmd2 *mode_cmd)
> -{
> -	struct drm_framebuffer *fb;
> -	struct drm_gem_object *obj;
> -	int size;
> -
> -	size = mode_cmd->pitches[0] * mode_cmd->height;
> -	size = PAGE_ALIGN(size);
> -
> -	obj = intel_fbdev_fb_bo_create(drm, size);
> -	if (IS_ERR(obj)) {
> -		fb = ERR_CAST(obj);
> -		goto err;
> -	}
> -
> -	fb = intel_framebuffer_create(obj,
> -				      drm_get_format_info(drm,
> -							  mode_cmd->pixel_format,
> -							  mode_cmd->modifier[0]),
> -				      mode_cmd);
> -	if (IS_ERR(fb)) {
> -		intel_fbdev_fb_bo_destroy(obj);
> -		goto err;
> -	}
> -
> -	drm_gem_object_put(obj);
> -
> -	return to_intel_framebuffer(fb);
> -
> -err:
> -	return ERR_CAST(fb);
> -}
> -
>  int intel_fbdev_fb_fill_info(struct intel_display *display, struct fb_info *info,
>  			     struct drm_gem_object *_obj, struct i915_vma *vma)
>  {
> -- 
> 2.47.2

-- 
Ville Syrjälä
Intel