Re: [PATCH 04/10] drm/i915: Make plane_config->fb a struct drm_framebuffer*

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

On Fri, Apr 10, 2026 at 06:48:02PM +0300, Jani Nikula wrote:
> On Fri, 10 Apr 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > There's no need to use the intel specific struct intel_framebuffer*
> > type in the initial plane_config structure. Just make it a
> > struct drm_framebuffer*.
> 
> I guess a few words on the "why?" would be nice. I mean they're still
> pointers to struct intel_framebuffer objects, and could be
> container_of'd back.

Yeah, meant to put something more sensible here. Basically I think the
less special types we expose in the interface (even if just pointers)
the better. At least then there is no need to change the interface
definition if we ever change the internal types. And I hope that the
existing examples will generally encourage people to not expose more
special types unless absolutely necessary.

> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/i9xx_plane.c          |  2 +-
> >  drivers/gpu/drm/i915/display/intel_display_types.h |  2 +-
> >  drivers/gpu/drm/i915/display/intel_initial_plane.c | 10 +++++-----
> >  drivers/gpu/drm/i915/display/skl_universal_plane.c |  2 +-
> >  drivers/gpu/drm/i915/i915_initial_plane.c          |  6 +++---
> >  drivers/gpu/drm/xe/display/xe_initial_plane.c      |  2 +-
> >  6 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > index 9c16753a1f3b..70734d32a409 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > @@ -1240,7 +1240,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> >  		    fb->width, fb->height, fb->format->cpp[0] * 8,
> >  		    base, fb->pitches[0], plane_config->size);
> >  
> > -	plane_config->fb = intel_fb;
> > +	plane_config->fb = &intel_fb->base;
> >  }
> >  
> >  bool i9xx_fixup_initial_plane_config(struct intel_crtc *crtc,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index eaa37b8d9584..82c7e03427ff 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -755,7 +755,7 @@ struct intel_plane_state {
> >  };
> >  
> >  struct intel_initial_plane_config {
> > -	struct intel_framebuffer *fb;
> > +	struct drm_framebuffer *fb;
> >  	struct i915_vma *vma;
> >  	int size;
> >  	u32 base;
> > diff --git a/drivers/gpu/drm/i915/display/intel_initial_plane.c b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> > index 4f51083dbd11..911d67dceba9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_initial_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> > @@ -55,9 +55,9 @@ static struct drm_gem_object *
> >  intel_alloc_initial_plane_obj(struct intel_display *display,
> >  			      struct intel_initial_plane_config *plane_config)
> >  {
> > -	struct intel_framebuffer *fb = plane_config->fb;
> > +	struct drm_framebuffer *fb = plane_config->fb;
> >  
> > -	switch (fb->base.modifier) {
> > +	switch (fb->modifier) {
> >  	case DRM_FORMAT_MOD_LINEAR:
> >  	case I915_FORMAT_MOD_X_TILED:
> >  	case I915_FORMAT_MOD_Y_TILED:
> > @@ -65,7 +65,7 @@ intel_alloc_initial_plane_obj(struct intel_display *display,
> >  		break;
> >  	default:
> >  		drm_dbg_kms(display->drm, "Unsupported modifier for initial FB: 0x%llx\n",
> > -			    fb->base.modifier);
> > +			    fb->modifier);
> >  		return NULL;
> >  	}
> >  
> > @@ -93,7 +93,7 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc,
> >  		return;
> >  
> >  	if (intel_alloc_initial_plane_obj(display, plane_config)) {
> > -		fb = &plane_config->fb->base;
> > +		fb = plane_config->fb;
> >  		vma = plane_config->vma;
> >  	} else {
> >  		const struct intel_plane_state *other_plane_state;
> > @@ -149,7 +149,7 @@ static void plane_config_fini(struct intel_display *display,
> >  			      struct intel_initial_plane_config *plane_config)
> >  {
> >  	if (plane_config->fb) {
> > -		struct drm_framebuffer *fb = &plane_config->fb->base;
> > +		struct drm_framebuffer *fb = plane_config->fb;
> >  
> >  		/* We may only have the stub and not a full framebuffer */
> >  		if (drm_framebuffer_read_refcount(fb))
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 7a9d494334b5..00c863e378a1 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -3206,7 +3206,7 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
> >  		    fb->width, fb->height, fb->format->cpp[0] * 8,
> >  		    base, fb->pitches[0], plane_config->size);
> >  
> > -	plane_config->fb = intel_fb;
> > +	plane_config->fb = &intel_fb->base;
> >  	return;
> >  
> >  error:
> > diff --git a/drivers/gpu/drm/i915/i915_initial_plane.c b/drivers/gpu/drm/i915/i915_initial_plane.c
> > index 6df57db9b62a..5cb1adde67b6 100644
> > --- a/drivers/gpu/drm/i915/i915_initial_plane.c
> > +++ b/drivers/gpu/drm/i915/i915_initial_plane.c
> > @@ -137,7 +137,7 @@ initial_plane_vma(struct drm_i915_private *i915,
> >  	i915_gem_object_set_cache_coherency(obj, HAS_WT(i915) ?
> >  					    I915_CACHE_WT : I915_CACHE_NONE);
> >  
> > -	tiling = intel_fb_modifier_to_tiling(plane_config->fb->base.modifier);
> > +	tiling = intel_fb_modifier_to_tiling(plane_config->fb->modifier);
> >  
> >  	switch (tiling) {
> >  	case I915_TILING_NONE:
> > @@ -145,7 +145,7 @@ initial_plane_vma(struct drm_i915_private *i915,
> >  	case I915_TILING_X:
> >  	case I915_TILING_Y:
> >  		obj->tiling_and_stride =
> > -			plane_config->fb->base.pitches[0] |
> > +			plane_config->fb->pitches[0] |
> >  			tiling;
> >  		break;
> >  	default:
> > @@ -223,7 +223,7 @@ i915_alloc_initial_plane_obj(struct drm_device *drm,
> >  {
> >  	struct drm_i915_private *i915 = to_i915(drm);
> >  	struct drm_mode_fb_cmd2 mode_cmd = {};
> > -	struct drm_framebuffer *fb = &plane_config->fb->base;
> > +	struct drm_framebuffer *fb = plane_config->fb;
> >  	struct i915_vma *vma;
> >  
> >  	vma = initial_plane_vma(i915, plane_config);
> > diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> > index 1c7a4e8c390c..8f2d0244c03f 100644
> > --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
> > +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> > @@ -115,7 +115,7 @@ xe_alloc_initial_plane_obj(struct drm_device *drm,
> >  {
> >  	struct xe_device *xe = to_xe_device(drm);
> >  	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> > -	struct drm_framebuffer *fb = &plane_config->fb->base;
> > +	struct drm_framebuffer *fb = plane_config->fb;
> >  	struct xe_bo *bo;
> >  
> >  	mode_cmd.pixel_format = fb->format->format;
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel