Re: [PATCH 7/9] drm/i915: Change intel_fbdev_fb_alloc() reuturn type

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

On Mon, May 06, 2024 at 05:16:50PM +0300, Jani Nikula wrote:
> 
> *return in subject
> 
> On Mon, 06 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Change intel_fbdev_fb_alloc() to return struct intel_fb instead
> > of struct drm_framebuffer. Let's us eliminate some annoying
> > aliasing variables in the fbdev setup code.
> 
> You'll need to enable DRM_XE=m and DRM_XE_DISPLAY=y configs, this will
> fail the build there. ;)

$ grep DRM_XE build_test/.config
CONFIG_DRM_XE=m
CONFIG_DRM_XE_DISPLAY=y

It actually builds just fine here, which is a bit
surprising.

> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbdev.c    | 10 +++++-----
> >  drivers/gpu/drm/i915/display/intel_fbdev_fb.c |  6 +++---
> >  drivers/gpu/drm/i915/display/intel_fbdev_fb.h |  5 +++--
> >  3 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > index bda702c2cab8..0d79ec1a6427 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > @@ -207,13 +207,13 @@ static int intelfb_create(struct drm_fb_helper *helper,
> >  		intel_fb = ifbdev->fb = NULL;
> >  	}
> >  	if (!intel_fb || drm_WARN_ON(dev, !intel_fb_obj(&intel_fb->base))) {
> > -		struct drm_framebuffer *fb;
> > +		struct intel_framebuffer *fb;
> >  		drm_dbg_kms(&dev_priv->drm,
> >  			    "no BIOS fb, allocating a new one\n");
> > -		fb = intel_fbdev_fb_alloc(helper, sizes);
> > -		if (IS_ERR(fb))
> > -			return PTR_ERR(fb);
> > -		intel_fb = ifbdev->fb = to_intel_framebuffer(fb);
> > +		intel_fb = intel_fbdev_fb_alloc(helper, sizes);
> 
> Now you have both fb in block context and intel_fb in the function
> context, and I think there's some confusion here.

Aye. Probably a rebase fail or something. The next patch
fixes it all up, but this intermediate thing does look
completely borked.

> 
> BR,
> Jani.
> 
> > +		if (IS_ERR(intel_fb))
> > +			return PTR_ERR(intel_fb);
> > +		ifbdev->fb = fb;
> >  	} else {
> >  		drm_dbg_kms(&dev_priv->drm, "re-using BIOS fb\n");
> >  		prealloc = true;
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> > index 0665f943f65f..497525ef9668 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> > @@ -11,8 +11,8 @@
> >  #include "intel_display_types.h"
> >  #include "intel_fbdev_fb.h"
> >  
> > -struct drm_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > -					     struct drm_fb_helper_surface_size *sizes)
> > +struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > +					       struct drm_fb_helper_surface_size *sizes)
> >  {
> >  	struct drm_framebuffer *fb;
> >  	struct drm_device *dev = helper->dev;
> > @@ -63,7 +63,7 @@ struct drm_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> >  	fb = intel_framebuffer_create(obj, &mode_cmd);
> >  	i915_gem_object_put(obj);
> >  
> > -	return fb;
> > +	return to_intel_framebuffer(fb);
> >  }
> >  
> >  int intel_fbdev_fb_fill_info(struct drm_i915_private *i915, struct fb_info *info,
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> > index a395b2c65d33..82e8e7cc007b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> > @@ -12,9 +12,10 @@ struct drm_i915_gem_object;
> >  struct drm_i915_private;
> >  struct fb_info;
> >  struct i915_vma;
> > +struct intel_framebuffer;
> >  
> > -struct drm_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > -					     struct drm_fb_helper_surface_size *sizes);
> > +struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > +					       struct drm_fb_helper_surface_size *sizes);
> >  int intel_fbdev_fb_fill_info(struct drm_i915_private *i915, struct fb_info *info,
> >  			     struct drm_i915_gem_object *obj, struct i915_vma *vma);
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel