| Message ID | 20240506125718.26001-8-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Plane fb refactoring | expand |
*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. ;) > > 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. 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);
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
On Mon, May 06, 2024 at 07:51:47PM +0300, Ville Syrjälä wrote: > 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. And the reason seems to be that xe has its own copy of that header for some reason. So xe's intel_fbdev_fb.c will include the xe header, but everything in i915 will include the i915 header. The two could be talking about apples vs. oranges and the build would never fail :/
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); + 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);