On Fri, 10 May 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> We use a mix of 'intel_fb' vs. 'ifbdev->fb' in the same function.
> Both should be pointing at the same thing. Make things less
> confusing by just getting existing fb from 'ifbdev->fb' at the
> start and then sticking with the local 'fb' (renamed from the
> 'intel_fb') until the very end.
>
> v2: rebase
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_fbdev.c | 38 ++++++++++++----------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 4bbbf481bb3a..e898018ab76a 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -175,7 +175,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> struct drm_fb_helper_surface_size *sizes)
> {
> struct intel_fbdev *ifbdev = to_intel_fbdev(helper);
> - struct intel_framebuffer *intel_fb = ifbdev->fb;
> + struct intel_framebuffer *fb = ifbdev->fb;
> struct drm_device *dev = helper->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> const struct i915_gtt_view view = {
> @@ -195,29 +195,30 @@ static int intelfb_create(struct drm_fb_helper *helper,
> if (ret)
> return ret;
>
> - if (intel_fb &&
> - (sizes->fb_width > intel_fb->base.width ||
> - sizes->fb_height > intel_fb->base.height)) {
> + ifbdev->fb = NULL;
> +
> + if (fb &&
> + (sizes->fb_width > fb->base.width ||
> + sizes->fb_height > fb->base.height)) {
> drm_dbg_kms(&dev_priv->drm,
> "BIOS fb too small (%dx%d), we require (%dx%d),"
> " releasing it\n",
> - intel_fb->base.width, intel_fb->base.height,
> + fb->base.width, fb->base.height,
> sizes->fb_width, sizes->fb_height);
> - drm_framebuffer_put(&intel_fb->base);
> - intel_fb = ifbdev->fb = NULL;
> + drm_framebuffer_put(&fb->base);
> + fb = NULL;
> }
> - if (!intel_fb || drm_WARN_ON(dev, !intel_fb_obj(&intel_fb->base))) {
> + if (!fb || drm_WARN_ON(dev, !intel_fb_obj(&fb->base))) {
> drm_dbg_kms(&dev_priv->drm,
> "no BIOS fb, allocating a new one\n");
> - intel_fb = intel_fbdev_fb_alloc(helper, sizes);
> - if (IS_ERR(intel_fb))
> - return PTR_ERR(intel_fb);
> - ifbdev->fb = intel_fb;
> + fb = intel_fbdev_fb_alloc(helper, sizes);
> + if (IS_ERR(fb))
> + return PTR_ERR(fb);
> } else {
> drm_dbg_kms(&dev_priv->drm, "re-using BIOS fb\n");
> prealloc = true;
> - sizes->fb_width = intel_fb->base.width;
> - sizes->fb_height = intel_fb->base.height;
> + sizes->fb_width = fb->base.width;
> + sizes->fb_height = fb->base.height;
> }
>
> wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> @@ -226,7 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> * This also validates that any existing fb inherited from the
> * BIOS is suitable for own access.
> */
> - vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base, false,
> + vma = intel_pin_and_fence_fb_obj(&fb->base, false,
> &view, false, &flags);
> if (IS_ERR(vma)) {
> ret = PTR_ERR(vma);
> @@ -240,11 +241,11 @@ static int intelfb_create(struct drm_fb_helper *helper,
> goto out_unpin;
> }
>
> - ifbdev->helper.fb = &ifbdev->fb->base;
> + ifbdev->helper.fb = &fb->base;
>
> info->fbops = &intelfb_ops;
>
> - obj = intel_fb_obj(&intel_fb->base);
> + obj = intel_fb_obj(&fb->base);
>
> ret = intel_fbdev_fb_fill_info(dev_priv, info, obj, vma);
> if (ret)
> @@ -262,8 +263,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
> /* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
>
> drm_dbg_kms(&dev_priv->drm, "allocated %dx%d fb: 0x%08x\n",
> - ifbdev->fb->base.width, ifbdev->fb->base.height,
> + fb->base.width, fb->base.height,
> i915_ggtt_offset(vma));
> + ifbdev->fb = fb;
> ifbdev->vma = vma;
> ifbdev->vma_flags = flags;
--
Jani Nikula, Intel
