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

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 05 Sep 2025, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

That would indeed be great, and allow even further cleanup. I'm just
trying to find the common parts here, and not so much dig into every
little detail that differs.

I did git blame this while writing the patch, and it's all in the big xe
driver enabling commit. I'm not sure where it comes from.

Cc: Maarten, any ideas?

BR,
Jani.

>
>> +}
>> +
>>  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

-- 
Jani Nikula, Intel