[Intel-gfx] [PATCH 1/2] drm/i915/display: split i915 specific code from intel_fbdev

Hogander, Jouni jouni.hogander at intel.com
Fri Dec 1 15:23:46 UTC 2023


On Fri, 2023-12-01 at 13:53 +0000, Govindapillai, Vinod wrote:
> On Wed, 2023-11-15 at 11:07 +0200, Jouni Högander wrote:
> > Split out code from intel_fbdev that can not be share between i915
> > and
> > xe. Create new i915 specific source/header file intel_fbdev_fb.[ch]
> > which
> > contains this code.
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile                 |   3 +-
> >  drivers/gpu/drm/i915/display/intel_fbdev.c    | 108 ++------------
> > --
> >  drivers/gpu/drm/i915/display/intel_fbdev_fb.c | 115
> > ++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_fbdev_fb.h |  21 ++++
> >  4 files changed, 146 insertions(+), 101 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> 
> Pls have a look at the checkpatch warnings for ->
> drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> 
> -:209: WARNING:SPDX_LICENSE_TAG: Improper SPDX comment style for
> 'drivers/gpu/drm/i915/display/intel_fbdev_fb.c', please use '//'
> instead
> #209: FILE: drivers/gpu/drm/i915/display/intel_fbdev_fb.c:1:
> +/* SPDX-License-Identifier: MIT */
> 
> -:209: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-
> Identifier tag in line 1
> #209: FILE: drivers/gpu/drm/i915/display/intel_fbdev_fb.c:1:
> +/* SPDX-License-Identifier: MIT */

As discusses offline changing these will just change the complaint
being about "//" in comments.

BR,

Jouni Högander
> 
> With that,
> 
> Reviewed-by: Vinod Govindapillai <vinod.govindapillai at intel.com>
> 
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile
> > index 239da40a401f..38cb7e984930 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -316,7 +316,8 @@ i915-$(CONFIG_ACPI) += \
> >         display/intel_acpi.o \
> >         display/intel_opregion.o
> >  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > -       display/intel_fbdev.o
> > +       display/intel_fbdev.o \
> > +       display/intel_fbdev_fb.o
> >  i915-$(CONFIG_DEBUG_FS) += \
> >         display/intel_display_debugfs.o \
> >         display/intel_display_debugfs_params.o \
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > index 31d0d695d567..b7e4b7830e45 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > @@ -43,7 +43,6 @@
> >  #include <drm/drm_fourcc.h>
> >  #include <drm/drm_gem_framebuffer_helper.h>
> >  
> > -#include "gem/i915_gem_lmem.h"
> >  #include "gem/i915_gem_mman.h"
> >  
> >  #include "i915_drv.h"
> > @@ -51,6 +50,7 @@
> >  #include "intel_fb.h"
> >  #include "intel_fb_pin.h"
> >  #include "intel_fbdev.h"
> > +#include "intel_fbdev_fb.h"
> >  #include "intel_frontbuffer.h"
> >  
> >  struct intel_fbdev {
> > @@ -146,65 +146,6 @@ static const struct fb_ops intelfb_ops = {
> >         .fb_mmap = intel_fbdev_mmap,
> >  };
> >  
> > -static int intelfb_alloc(struct drm_fb_helper *helper,
> > -                        struct drm_fb_helper_surface_size *sizes)
> > -{
> > -       struct intel_fbdev *ifbdev = to_intel_fbdev(helper);
> > -       struct drm_framebuffer *fb;
> > -       struct drm_device *dev = helper->dev;
> > -       struct drm_i915_private *dev_priv = to_i915(dev);
> > -       struct drm_mode_fb_cmd2 mode_cmd = {};
> > -       struct drm_i915_gem_object *obj;
> > -       int size;
> > -
> > -       /* we don't do packed 24bpp */
> > -       if (sizes->surface_bpp == 24)
> > -               sizes->surface_bpp = 32;
> > -
> > -       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.pixel_format = drm_mode_legacy_fb_format(sizes-
> > >surface_bpp,
> > -                                                         sizes-
> > >surface_depth);
> > -
> > -       size = mode_cmd.pitches[0] * mode_cmd.height;
> > -       size = PAGE_ALIGN(size);
> > -
> > -       obj = ERR_PTR(-ENODEV);
> > -       if (HAS_LMEM(dev_priv)) {
> > -               obj = i915_gem_object_create_lmem(dev_priv, size,
> > -                                                
> > I915_BO_ALLOC_CONTIGUOUS |
> > -                                                
> > I915_BO_ALLOC_USER);
> > -       } else {
> > -               /*
> > -                * If the FB is too big, just don't use it since
> > fbdev is not very
> > -                * important and we should probably use that space
> > with FBC or other
> > -                * features.
> > -                *
> > -                * Also skip stolen on MTL as Wa_22018444074
> > mitigation.
> > -                */
> > -               if (!(IS_METEORLAKE(dev_priv)) && size * 2 <
> > dev_priv->dsm.usable_size)
> > -                       obj =
> > i915_gem_object_create_stolen(dev_priv, size);
> > -               if (IS_ERR(obj))
> > -                       obj =
> > i915_gem_object_create_shmem(dev_priv, size);
> > -       }
> > -
> > -       if (IS_ERR(obj)) {
> > -               drm_err(&dev_priv->drm, "failed to allocate
> > framebuffer (%pe)\n", obj);
> > -               return PTR_ERR(obj);
> > -       }
> > -
> > -       fb = intel_framebuffer_create(obj, &mode_cmd);
> > -       i915_gem_object_put(obj);
> > -       if (IS_ERR(fb))
> > -               return PTR_ERR(fb);
> > -
> > -       ifbdev->fb = to_intel_framebuffer(fb);
> > -       return 0;
> > -}
> > -
> >  static int intelfb_create(struct drm_fb_helper *helper,
> >                           struct drm_fb_helper_surface_size *sizes)
> >  {
> > @@ -213,7 +154,6 @@ static int intelfb_create(struct drm_fb_helper
> > *helper,
> >         struct drm_device *dev = helper->dev;
> >         struct drm_i915_private *dev_priv = to_i915(dev);
> >         struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> > -       struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt;
> >         const struct i915_gtt_view view = {
> >                 .type = I915_GTT_VIEW_NORMAL,
> >         };
> > @@ -222,9 +162,7 @@ static int intelfb_create(struct drm_fb_helper
> > *helper,
> >         struct i915_vma *vma;
> >         unsigned long flags = 0;
> >         bool prealloc = false;
> > -       void __iomem *vaddr;
> >         struct drm_i915_gem_object *obj;
> > -       struct i915_gem_ww_ctx ww;
> >         int ret;
> >  
> >         mutex_lock(&ifbdev->hpd_lock);
> > @@ -245,12 +183,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;
> >                 drm_dbg_kms(&dev_priv->drm,
> >                             "no BIOS fb, allocating a new one\n");
> > -               ret = intelfb_alloc(helper, sizes);
> > -               if (ret)
> > -                       return ret;
> > -               intel_fb = ifbdev->fb;
> > +               fb = intel_fbdev_fb_alloc(helper, sizes);
> > +               if (IS_ERR(fb))
> > +                       return PTR_ERR(fb);
> > +               intel_fb = ifbdev->fb = to_intel_framebuffer(fb);
> >         } else {
> >                 drm_dbg_kms(&dev_priv->drm, "re-using BIOS fb\n");
> >                 prealloc = true;
> > @@ -283,49 +222,18 @@ static int intelfb_create(struct
> > drm_fb_helper *helper,
> >         info->fbops = &intelfb_ops;
> >  
> >         obj = intel_fb_obj(&intel_fb->base);
> > -       if (i915_gem_object_is_lmem(obj)) {
> > -               struct intel_memory_region *mem = obj->mm.region;
> > -
> > -               /* Use fbdev's framebuffer from lmem for discrete
> > */
> > -               info->fix.smem_start =
> > -                       (unsigned long)(mem->io_start +
> > -
> >                                        i915_gem_object_get_dma_addre
> > ss(obj, 0));
> > -               info->fix.smem_len = obj->base.size;
> > -       } else {
> > -               /* Our framebuffer is the entirety of fbdev's
> > system memory */
> > -               info->fix.smem_start =
> > -                       (unsigned long)(ggtt->gmadr.start +
> > i915_ggtt_offset(vma));
> > -               info->fix.smem_len = vma->size;
> > -       }
> > -
> > -       for_i915_gem_ww(&ww, ret, false) {
> > -               ret = i915_gem_object_lock(vma->obj, &ww);
> > -
> > -               if (ret)
> > -                       continue;
> > -
> > -               vaddr = i915_vma_pin_iomap(vma);
> > -               if (IS_ERR(vaddr)) {
> > -                       drm_err(&dev_priv->drm,
> > -                               "Failed to remap framebuffer into
> > virtual memory (%pe)\n", vaddr);
> > -                       ret = PTR_ERR(vaddr);
> > -                       continue;
> > -               }
> > -       }
> >  
> > +       ret = intel_fbdev_fb_fill_info(dev_priv, info, obj, vma);
> >         if (ret)
> >                 goto out_unpin;
> >  
> > -       info->screen_base = vaddr;
> > -       info->screen_size = vma->size;
> > -
> >         drm_fb_helper_fill_info(info, &ifbdev->helper, sizes);
> >  
> >         /* If the object is shmemfs backed, it will have given us
> > zeroed pages.
> >          * If the object is stolen however, it will be full of
> > whatever
> >          * garbage was left in there.
> >          */
> > -       if (!i915_gem_object_is_shmem(vma->obj) && !prealloc)
> > +       if (!i915_gem_object_is_shmem(obj) && !prealloc)
> >                 memset_io(info->screen_base, 0, info->screen_size);
> >  
> >         /* Use default scratch pixmap (info->pixmap.flags =
> > FB_PIXMAP_SYSTEM) */
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> > b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> > new file mode 100644
> > index 000000000000..717c3a3237c4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> > @@ -0,0 +1,115 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + */
> > +
> > +#include <drm/drm_fb_helper.h>
> > +
> > +#include "gem/i915_gem_lmem.h"
> > +
> > +#include "i915_drv.h"
> > +#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 drm_framebuffer *fb;
> > +       struct drm_device *dev = helper->dev;
> > +       struct drm_i915_private *dev_priv = to_i915(dev);
> > +       struct drm_mode_fb_cmd2 mode_cmd = {};
> > +       struct drm_i915_gem_object *obj;
> > +       int size;
> > +
> > +       /* we don't do packed 24bpp */
> > +       if (sizes->surface_bpp == 24)
> > +               sizes->surface_bpp = 32;
> > +
> > +       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.pixel_format = drm_mode_legacy_fb_format(sizes-
> > >surface_bpp,
> > +                                                         sizes-
> > >surface_depth);
> > +
> > +       size = mode_cmd.pitches[0] * mode_cmd.height;
> > +       size = PAGE_ALIGN(size);
> > +
> > +       obj = ERR_PTR(-ENODEV);
> > +       if (HAS_LMEM(dev_priv)) {
> > +               obj = i915_gem_object_create_lmem(dev_priv, size,
> > +                                                
> > I915_BO_ALLOC_CONTIGUOUS |
> > +                                                
> > I915_BO_ALLOC_USER);
> > +       } else {
> > +               /*
> > +                * If the FB is too big, just don't use it since
> > fbdev is not very
> > +                * important and we should probably use that space
> > with FBC or other
> > +                * features.
> > +                *
> > +                * Also skip stolen on MTL as Wa_22018444074
> > mitigation.
> > +                */
> > +               if (!(IS_METEORLAKE(dev_priv)) && size * 2 <
> > dev_priv->dsm.usable_size)
> > +                       obj =
> > i915_gem_object_create_stolen(dev_priv, size);
> > +               if (IS_ERR(obj))
> > +                       obj =
> > i915_gem_object_create_shmem(dev_priv, size);
> > +       }
> > +
> > +       if (IS_ERR(obj)) {
> > +               drm_err(&dev_priv->drm, "failed to allocate
> > framebuffer (%pe)\n", obj);
> > +               return ERR_PTR(-ENOMEM);
> > +       }
> > +
> > +       fb = intel_framebuffer_create(obj, &mode_cmd);
> > +       i915_gem_object_put(obj);
> > +
> > +       return fb;
> > +}
> > +
> > +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)
> > +{
> > +       struct i915_gem_ww_ctx ww;
> > +       void __iomem *vaddr;
> > +       int ret;
> > +
> > +       if (i915_gem_object_is_lmem(obj)) {
> > +               struct intel_memory_region *mem = obj->mm.region;
> > +
> > +               /* Use fbdev's framebuffer from lmem for discrete
> > */
> > +               info->fix.smem_start =
> > +                       (unsigned long)(mem->io_start +
> > +                                       i915_gem_object_get_dma_add
> > ress(obj, 0));
> > +               info->fix.smem_len = obj->base.size;
> > +       } else {
> > +               struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
> > +
> > +               /* Our framebuffer is the entirety of fbdev's
> > system memory */
> > +               info->fix.smem_start =
> > +                       (unsigned long)(ggtt->gmadr.start +
> > i915_ggtt_offset(vma));
> > +               info->fix.smem_len = vma->size;
> > +       }
> > +
> > +       for_i915_gem_ww(&ww, ret, false) {
> > +               ret = i915_gem_object_lock(vma->obj, &ww);
> > +
> > +               if (ret)
> > +                       continue;
> > +
> > +               vaddr = i915_vma_pin_iomap(vma);
> > +               if (IS_ERR(vaddr)) {
> > +                       drm_err(&i915->drm,
> > +                               "Failed to remap framebuffer into
> > virtual memory (%pe)\n", vaddr);
> > +                       ret = PTR_ERR(vaddr);
> > +                       continue;
> > +               }
> > +       }
> > +
> > +       if (ret)
> > +               return ret;
> > +
> > +       info->screen_base = vaddr;
> > +       info->screen_size = intel_bo_to_drm_bo(obj)->size;
> > +
> > +       return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> > b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> > new file mode 100644
> > index 000000000000..a395b2c65d33
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + */
> > +
> > +#ifndef __INTEL_FBDEV_FB_H__
> > +#define __INTEL_FBDEV_FB_H__
> > +
> > +struct drm_fb_helper;
> > +struct drm_fb_helper_surface_size;
> > +struct drm_i915_gem_object;
> > +struct drm_i915_private;
> > +struct fb_info;
> > +struct i915_vma;
> > +
> > +struct drm_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);
> > +
> > +#endif
> 



More information about the Intel-gfx mailing list