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@xxxxxxxxx> > > --- > > 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@xxxxxxxxx> > > > > > 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 >