Re: [PATCH 2/9] drm/{i915, xe}/fbdev: add intel_fbdev_fb_pitch_align()

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

On Wed, Sep 03, 2025 at 11:31:59PM +0300, Jani Nikula wrote:
> Add new helper intel_fbdev_fb_pitch_align() in preparation for further
> refactoring. The alignment is different for i915 and xe.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbdev_fb.c | 8 ++++++--
>  drivers/gpu/drm/i915/display/intel_fbdev_fb.h | 3 +++
>  drivers/gpu/drm/xe/display/intel_fbdev_fb.c   | 8 ++++++--
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> index 9c557917d781..d1c03d7b9bdc 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> @@ -13,6 +13,11 @@
>  #include "intel_fb.h"
>  #include "intel_fbdev_fb.h"
>  
> +u32 intel_fbdev_fb_pitch_align(u32 stride)
> +{
> +	return ALIGN(stride, 64);
> +}
> +
>  struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
>  					       struct drm_fb_helper_surface_size *sizes)
>  {
> @@ -30,8 +35,7 @@ struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
>  	mode_cmd.width = sizes->surface_width;
>  	mode_cmd.height = sizes->surface_height;
>  
> -	mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
> -				    DIV_ROUND_UP(sizes->surface_bpp, 8), 64);
> +	mode_cmd.pitches[0] = intel_fbdev_fb_pitch_align(mode_cmd.width * DIV_ROUND_UP(sizes->surface_bpp, 8));
>  	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
>  							  sizes->surface_depth);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> index 668ae355f5e5..caeb543d5efc 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> @@ -6,6 +6,8 @@
>  #ifndef __INTEL_FBDEV_FB_H__
>  #define __INTEL_FBDEV_FB_H__
>  
> +#include <linux/types.h>
> +
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
>  struct drm_gem_object;
> @@ -13,6 +15,7 @@ struct fb_info;
>  struct i915_vma;
>  struct intel_display;
>  
> +u32 intel_fbdev_fb_pitch_align(u32 stride);
>  struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
>  					       struct drm_fb_helper_surface_size *sizes);
>  int intel_fbdev_fb_fill_info(struct intel_display *display, struct fb_info *info,
> diff --git a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> index bafca1059a40..fd2c40020eea 100644
> --- a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> +++ b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
> @@ -15,6 +15,11 @@
>  
>  #include <generated/xe_wa_oob.h>
>  
> +u32 intel_fbdev_fb_pitch_align(u32 stride)
> +{
> +	return ALIGN(stride, XE_PAGE_SIZE);

I think someone needs to explain what the heck this extra alignment
is trying to achieve? I suspect it just needs to get nuked.

> +}
> +
>  struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
>  					       struct drm_fb_helper_surface_size *sizes)
>  {
> @@ -31,8 +36,7 @@ struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_device *drm,
>  	mode_cmd.width = sizes->surface_width;
>  	mode_cmd.height = sizes->surface_height;
>  
> -	mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
> -				    DIV_ROUND_UP(sizes->surface_bpp, 8), XE_PAGE_SIZE);
> +	mode_cmd.pitches[0] = intel_fbdev_fb_pitch_align(mode_cmd.width * DIV_ROUND_UP(sizes->surface_bpp, 8));
>  	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
>  							  sizes->surface_depth);
>  
> -- 
> 2.47.2

-- 
Ville Syrjälä
Intel