| Message ID | 20211029115014.264084-1-john@metanate.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/rockchip: use generic fbdev setup | expand |
Hi Am 29.10.21 um 13:50 schrieb John Keeping: > The Rockchip fbdev code does not add anything compared to > drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the > same thing as gem_prime_mmap which is called by the helper. > > Signed-off-by: John Keeping <john@metanate.com> > --- > drivers/gpu/drm/rockchip/Makefile | 1 - > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- > 5 files changed, 2 insertions(+), 199 deletions(-) > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > index 17a9e7eb2130..1a56f696558c 100644 > --- a/drivers/gpu/drm/rockchip/Makefile > +++ b/drivers/gpu/drm/rockchip/Makefile > @@ -5,7 +5,6 @@ > > rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index 69c699459dce..20d81ae69828 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -26,7 +26,6 @@ > > #include "rockchip_drm_drv.h" > #include "rockchip_drm_fb.h" > -#include "rockchip_drm_fbdev.h" > #include "rockchip_drm_gem.h" > > #define DRIVER_NAME "rockchip" > @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) > > drm_mode_config_reset(drm_dev); > > - ret = rockchip_drm_fbdev_init(drm_dev); > - if (ret) > - goto err_unbind_all; > - > /* init kms poll for handling hpd */ > drm_kms_helper_poll_init(drm_dev); > > @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) > if (ret) > goto err_kms_helper_poll_fini; > > + drm_fbdev_generic_setup(drm_dev, 32); Please pass 0 for the final argument. The fbdev helpers pick 32 by default. Maybe leave a comment if you require 32 here. For RGB colors, you should rather set drm_device.mode_config.preferred_depth to 24 instead. With these changes: Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Best regards Thomas > + > return 0; > err_kms_helper_poll_fini: > drm_kms_helper_poll_fini(drm_dev); > - rockchip_drm_fbdev_fini(drm_dev); > err_unbind_all: > component_unbind_all(dev, drm_dev); > err_iommu_cleanup: > @@ -189,7 +185,6 @@ static void rockchip_drm_unbind(struct device *dev) > > drm_dev_unregister(drm_dev); > > - rockchip_drm_fbdev_fini(drm_dev); > drm_kms_helper_poll_fini(drm_dev); > > drm_atomic_helper_shutdown(drm_dev); > @@ -203,7 +198,6 @@ DEFINE_DRM_GEM_FOPS(rockchip_drm_driver_fops); > > static const struct drm_driver rockchip_drm_driver = { > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, > - .lastclose = drm_fb_helper_lastclose, > .dumb_create = rockchip_gem_dumb_create, > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > index aa0909e8edf9..143a48330f84 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > @@ -43,8 +43,6 @@ struct rockchip_crtc_state { > * @mm_lock: protect drm_mm on multi-threads. > */ > struct rockchip_drm_private { > - struct drm_fb_helper fbdev_helper; > - struct drm_gem_object *fbdev_bo; > struct iommu_domain *domain; > struct mutex mm_lock; > struct drm_mm mm; > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > deleted file mode 100644 > index d8418dd39d0e..000000000000 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > +++ /dev/null > @@ -1,164 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -/* > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > - * Author:Mark Yao <mark.yao@rock-chips.com> > - */ > - > -#include <drm/drm.h> > -#include <drm/drm_fb_helper.h> > -#include <drm/drm_fourcc.h> > -#include <drm/drm_prime.h> > -#include <drm/drm_probe_helper.h> > - > -#include "rockchip_drm_drv.h" > -#include "rockchip_drm_gem.h" > -#include "rockchip_drm_fb.h" > -#include "rockchip_drm_fbdev.h" > - > -#define PREFERRED_BPP 32 > -#define to_drm_private(x) \ > - container_of(x, struct rockchip_drm_private, fbdev_helper) > - > -static int rockchip_fbdev_mmap(struct fb_info *info, > - struct vm_area_struct *vma) > -{ > - struct drm_fb_helper *helper = info->par; > - struct rockchip_drm_private *private = to_drm_private(helper); > - > - return drm_gem_prime_mmap(private->fbdev_bo, vma); > -} > - > -static const struct fb_ops rockchip_drm_fbdev_ops = { > - .owner = THIS_MODULE, > - DRM_FB_HELPER_DEFAULT_OPS, > - .fb_mmap = rockchip_fbdev_mmap, > - .fb_fillrect = drm_fb_helper_cfb_fillrect, > - .fb_copyarea = drm_fb_helper_cfb_copyarea, > - .fb_imageblit = drm_fb_helper_cfb_imageblit, > -}; > - > -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper, > - struct drm_fb_helper_surface_size *sizes) > -{ > - struct rockchip_drm_private *private = to_drm_private(helper); > - struct drm_mode_fb_cmd2 mode_cmd = { 0 }; > - struct drm_device *dev = helper->dev; > - struct rockchip_gem_object *rk_obj; > - struct drm_framebuffer *fb; > - unsigned int bytes_per_pixel; > - unsigned long offset; > - struct fb_info *fbi; > - size_t size; > - int ret; > - > - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8); > - > - mode_cmd.width = sizes->surface_width; > - mode_cmd.height = sizes->surface_height; > - mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel; > - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, > - sizes->surface_depth); > - > - size = mode_cmd.pitches[0] * mode_cmd.height; > - > - rk_obj = rockchip_gem_create_object(dev, size, true); > - if (IS_ERR(rk_obj)) > - return -ENOMEM; > - > - private->fbdev_bo = &rk_obj->base; > - > - fbi = drm_fb_helper_alloc_fbi(helper); > - if (IS_ERR(fbi)) { > - DRM_DEV_ERROR(dev->dev, "Failed to create framebuffer info.\n"); > - ret = PTR_ERR(fbi); > - goto out; > - } > - > - helper->fb = rockchip_drm_framebuffer_init(dev, &mode_cmd, > - private->fbdev_bo); > - if (IS_ERR(helper->fb)) { > - DRM_DEV_ERROR(dev->dev, > - "Failed to allocate DRM framebuffer.\n"); > - ret = PTR_ERR(helper->fb); > - goto out; > - } > - > - fbi->fbops = &rockchip_drm_fbdev_ops; > - > - fb = helper->fb; > - drm_fb_helper_fill_info(fbi, helper, sizes); > - > - offset = fbi->var.xoffset * bytes_per_pixel; > - offset += fbi->var.yoffset * fb->pitches[0]; > - > - dev->mode_config.fb_base = 0; > - fbi->screen_base = rk_obj->kvaddr + offset; > - fbi->screen_size = rk_obj->base.size; > - fbi->fix.smem_len = rk_obj->base.size; > - > - DRM_DEBUG_KMS("FB [%dx%d]-%d kvaddr=%p offset=%ld size=%zu\n", > - fb->width, fb->height, fb->format->depth, > - rk_obj->kvaddr, > - offset, size); > - > - return 0; > - > -out: > - rockchip_gem_free_object(&rk_obj->base); > - return ret; > -} > - > -static const struct drm_fb_helper_funcs rockchip_drm_fb_helper_funcs = { > - .fb_probe = rockchip_drm_fbdev_create, > -}; > - > -int rockchip_drm_fbdev_init(struct drm_device *dev) > -{ > - struct rockchip_drm_private *private = dev->dev_private; > - struct drm_fb_helper *helper; > - int ret; > - > - if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector) > - return -EINVAL; > - > - helper = &private->fbdev_helper; > - > - drm_fb_helper_prepare(dev, helper, &rockchip_drm_fb_helper_funcs); > - > - ret = drm_fb_helper_init(dev, helper); > - if (ret < 0) { > - DRM_DEV_ERROR(dev->dev, > - "Failed to initialize drm fb helper - %d.\n", > - ret); > - return ret; > - } > - > - ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP); > - if (ret < 0) { > - DRM_DEV_ERROR(dev->dev, > - "Failed to set initial hw config - %d.\n", > - ret); > - goto err_drm_fb_helper_fini; > - } > - > - return 0; > - > -err_drm_fb_helper_fini: > - drm_fb_helper_fini(helper); > - return ret; > -} > - > -void rockchip_drm_fbdev_fini(struct drm_device *dev) > -{ > - struct rockchip_drm_private *private = dev->dev_private; > - struct drm_fb_helper *helper; > - > - helper = &private->fbdev_helper; > - > - drm_fb_helper_unregister_fbi(helper); > - > - if (helper->fb) > - drm_framebuffer_put(helper->fb); > - > - drm_fb_helper_fini(helper); > -} > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > deleted file mode 100644 > index 5fb7ac2371a8..000000000000 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > +++ /dev/null > @@ -1,24 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0-only */ > -/* > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > - * Author:Mark Yao <mark.yao@rock-chips.com> > - */ > - > -#ifndef _ROCKCHIP_DRM_FBDEV_H > -#define _ROCKCHIP_DRM_FBDEV_H > - > -#ifdef CONFIG_DRM_FBDEV_EMULATION > -int rockchip_drm_fbdev_init(struct drm_device *dev); > -void rockchip_drm_fbdev_fini(struct drm_device *dev); > -#else > -static inline int rockchip_drm_fbdev_init(struct drm_device *dev) > -{ > - return 0; > -} > - > -static inline void rockchip_drm_fbdev_fini(struct drm_device *dev) > -{ > -} > -#endif > - > -#endif /* _ROCKCHIP_DRM_FBDEV_H */ >
Hi Thomas, On Fri, Oct 29, 2021 at 09:00:08PM +0200, Thomas Zimmermann wrote: > Am 29.10.21 um 13:50 schrieb John Keeping: > > The Rockchip fbdev code does not add anything compared to > > drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the > > same thing as gem_prime_mmap which is called by the helper. > > > > Signed-off-by: John Keeping <john@metanate.com> > > --- > > drivers/gpu/drm/rockchip/Makefile | 1 - > > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- > > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - > > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ > > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- > > 5 files changed, 2 insertions(+), 199 deletions(-) > > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > > > diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > > index 17a9e7eb2130..1a56f696558c 100644 > > --- a/drivers/gpu/drm/rockchip/Makefile > > +++ b/drivers/gpu/drm/rockchip/Makefile > > @@ -5,7 +5,6 @@ > > rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > > rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > > -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > > rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > index 69c699459dce..20d81ae69828 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > @@ -26,7 +26,6 @@ > > #include "rockchip_drm_drv.h" > > #include "rockchip_drm_fb.h" > > -#include "rockchip_drm_fbdev.h" > > #include "rockchip_drm_gem.h" > > #define DRIVER_NAME "rockchip" > > @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) > > drm_mode_config_reset(drm_dev); > > - ret = rockchip_drm_fbdev_init(drm_dev); > > - if (ret) > > - goto err_unbind_all; > > - > > /* init kms poll for handling hpd */ > > drm_kms_helper_poll_init(drm_dev); > > @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) > > if (ret) > > goto err_kms_helper_poll_fini; > > + drm_fbdev_generic_setup(drm_dev, 32); > > Please pass 0 for the final argument. The fbdev helpers pick 32 by default. > Maybe leave a comment if you require 32 here. I wanted to minimise the changes introduced here - passing 32 matches the value passed to drm_fb_helper_initial_config() in the deleted code from rockchip_drm_fbdev.c. What do you think about changing this to 0 in a follow-up patch?
Hi Am 30.10.21 um 14:05 schrieb John Keeping: > Hi Thomas, > > On Fri, Oct 29, 2021 at 09:00:08PM +0200, Thomas Zimmermann wrote: >> Am 29.10.21 um 13:50 schrieb John Keeping: >>> The Rockchip fbdev code does not add anything compared to >>> drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the >>> same thing as gem_prime_mmap which is called by the helper. >>> >>> Signed-off-by: John Keeping <john@metanate.com> >>> --- >>> drivers/gpu/drm/rockchip/Makefile | 1 - >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - >>> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ >>> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- >>> 5 files changed, 2 insertions(+), 199 deletions(-) >>> delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c >>> delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h >>> >>> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile >>> index 17a9e7eb2130..1a56f696558c 100644 >>> --- a/drivers/gpu/drm/rockchip/Makefile >>> +++ b/drivers/gpu/drm/rockchip/Makefile >>> @@ -5,7 +5,6 @@ >>> rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ >>> rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o >>> -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o >>> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o >>> rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>> index 69c699459dce..20d81ae69828 100644 >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>> @@ -26,7 +26,6 @@ >>> #include "rockchip_drm_drv.h" >>> #include "rockchip_drm_fb.h" >>> -#include "rockchip_drm_fbdev.h" >>> #include "rockchip_drm_gem.h" >>> #define DRIVER_NAME "rockchip" >>> @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) >>> drm_mode_config_reset(drm_dev); >>> - ret = rockchip_drm_fbdev_init(drm_dev); >>> - if (ret) >>> - goto err_unbind_all; >>> - >>> /* init kms poll for handling hpd */ >>> drm_kms_helper_poll_init(drm_dev); >>> @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) >>> if (ret) >>> goto err_kms_helper_poll_fini; >>> + drm_fbdev_generic_setup(drm_dev, 32); >> >> Please pass 0 for the final argument. The fbdev helpers pick 32 by default. >> Maybe leave a comment if you require 32 here. > > I wanted to minimise the changes introduced here - passing 32 matches > the value passed to drm_fb_helper_initial_config() in the deleted code > from rockchip_drm_fbdev.c. In that case Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > What do you think about changing this to 0 in a follow-up patch? > Yes. If possible, please provide a follow-up patch for this and set modeconfig.prefered_depth to 24. Best regards Thomas
On Sun, 31 Oct 2021 19:09:39 +0100 Thomas Zimmermann <tzimmermann@suse.de> wrote: > Am 30.10.21 um 14:05 schrieb John Keeping: > > On Fri, Oct 29, 2021 at 09:00:08PM +0200, Thomas Zimmermann wrote: > >> Am 29.10.21 um 13:50 schrieb John Keeping: > >>> The Rockchip fbdev code does not add anything compared to > >>> drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the > >>> same thing as gem_prime_mmap which is called by the helper. > >>> > >>> Signed-off-by: John Keeping <john@metanate.com> > >>> --- > >>> drivers/gpu/drm/rockchip/Makefile | 1 - > >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- > >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - > >>> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ > >>> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- > >>> 5 files changed, 2 insertions(+), 199 deletions(-) > >>> delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > >>> delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > >>> > >>> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > >>> index 17a9e7eb2130..1a56f696558c 100644 > >>> --- a/drivers/gpu/drm/rockchip/Makefile > >>> +++ b/drivers/gpu/drm/rockchip/Makefile > >>> @@ -5,7 +5,6 @@ > >>> rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > >>> rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > >>> -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > >>> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > >>> rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o > >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > >>> index 69c699459dce..20d81ae69828 100644 > >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > >>> @@ -26,7 +26,6 @@ > >>> #include "rockchip_drm_drv.h" > >>> #include "rockchip_drm_fb.h" > >>> -#include "rockchip_drm_fbdev.h" > >>> #include "rockchip_drm_gem.h" > >>> #define DRIVER_NAME "rockchip" > >>> @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) > >>> drm_mode_config_reset(drm_dev); > >>> - ret = rockchip_drm_fbdev_init(drm_dev); > >>> - if (ret) > >>> - goto err_unbind_all; > >>> - > >>> /* init kms poll for handling hpd */ > >>> drm_kms_helper_poll_init(drm_dev); > >>> @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) > >>> if (ret) > >>> goto err_kms_helper_poll_fini; > >>> + drm_fbdev_generic_setup(drm_dev, 32); > >> > >> Please pass 0 for the final argument. The fbdev helpers pick 32 by default. > >> Maybe leave a comment if you require 32 here. > > > > I wanted to minimise the changes introduced here - passing 32 matches > > the value passed to drm_fb_helper_initial_config() in the deleted code > > from rockchip_drm_fbdev.c. > > In that case > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks! > > > > What do you think about changing this to 0 in a follow-up patch? > > > > Yes. If possible, please provide a follow-up patch for this and set > modeconfig.prefered_depth to 24. I'll follow up with a patch setting this to zero, but I'm not convinced I understand mode_config.prefered_depth well enough to be confident setting it to 24 is the right thing to do. Looking at commits 23d4e55f7eeb ("drm/vkms: Unset preferred_depth") and 550f17441f53 ("drm/cirrus: flip default from 24bpp to 16bpp") it seems that this will have a wider impact beyond fbdev. 32bpp has been used since the Rockchip driver was added so I don't see any real need to change to 24 now. Regards, John
Hi Thomas, On Mon, Nov 01, 2021 at 11:34:15AM +0000, John Keeping wrote: > On Sun, 31 Oct 2021 19:09:39 +0100 > Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Am 30.10.21 um 14:05 schrieb John Keeping: > > > On Fri, Oct 29, 2021 at 09:00:08PM +0200, Thomas Zimmermann wrote: > > >> Am 29.10.21 um 13:50 schrieb John Keeping: > > >>> The Rockchip fbdev code does not add anything compared to > > >>> drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the > > >>> same thing as gem_prime_mmap which is called by the helper. > > >>> > > >>> Signed-off-by: John Keeping <john@metanate.com> > > >>> --- > > >>> drivers/gpu/drm/rockchip/Makefile | 1 - > > >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- > > >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - > > >>> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ > > >>> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- > > >>> 5 files changed, 2 insertions(+), 199 deletions(-) > > >>> delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > > >>> delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > >>> > > >>> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > > >>> index 17a9e7eb2130..1a56f696558c 100644 > > >>> --- a/drivers/gpu/drm/rockchip/Makefile > > >>> +++ b/drivers/gpu/drm/rockchip/Makefile > > >>> @@ -5,7 +5,6 @@ > > >>> rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > > >>> rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > > >>> -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > >>> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > > >>> rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o > > >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > >>> index 69c699459dce..20d81ae69828 100644 > > >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > >>> @@ -26,7 +26,6 @@ > > >>> #include "rockchip_drm_drv.h" > > >>> #include "rockchip_drm_fb.h" > > >>> -#include "rockchip_drm_fbdev.h" > > >>> #include "rockchip_drm_gem.h" > > >>> #define DRIVER_NAME "rockchip" > > >>> @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) > > >>> drm_mode_config_reset(drm_dev); > > >>> - ret = rockchip_drm_fbdev_init(drm_dev); > > >>> - if (ret) > > >>> - goto err_unbind_all; > > >>> - > > >>> /* init kms poll for handling hpd */ > > >>> drm_kms_helper_poll_init(drm_dev); > > >>> @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) > > >>> if (ret) > > >>> goto err_kms_helper_poll_fini; > > >>> + drm_fbdev_generic_setup(drm_dev, 32); > > >> > > >> Please pass 0 for the final argument. The fbdev helpers pick 32 by default. > > >> Maybe leave a comment if you require 32 here. > > > > > > I wanted to minimise the changes introduced here - passing 32 matches > > > the value passed to drm_fb_helper_initial_config() in the deleted code > > > from rockchip_drm_fbdev.c. > > > > In that case > > > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > Thanks! Are you able to pick this up (and [1])? Otherwise what is needed here and who should pick this up? [1] https://lore.kernel.org/dri-devel/20211101114622.813536-1-john@metanate.com/
Hi Am 07.12.21 um 12:54 schrieb John Keeping: > > Are you able to pick this up (and [1])? Otherwise what is needed here > and who should pick this up? > > [1] https://lore.kernel.org/dri-devel/20211101114622.813536-1-john@metanate.com/ > I added both patches to drm-misc-next. Best regards Thomas
Hi John, Your patch contribution causes a kernel panic on MK808 with Rockchip rk3066a SoC. Would you like to contribute to fix this issue? The assumtion that drm_fbdev_generic_setup() does what rockchip_drm_fbdev_init did is not true! A revert makes it work again. Johan ====== [ 7.975906] ------------[ cut here ]------------ [ 7.975929] WARNING: CPU: 0 PID: 35 at drivers/gpu/drm/drm_fb_helper.c:471 drm_fb_helper_damage_work+0x138/0x3b4 [ 7.976044] rockchip-drm display-subsystem: Damage blitter failed: ret=-12 [ 7.976064] Modules linked in: [ 7.976090] CPU: 0 PID: 35 Comm: kworker/0:4 Not tainted 6.0.0-next-20221013 #1 [ 7.976126] Hardware name: Rockchip (Device Tree) [ 7.976145] Workqueue: events drm_fb_helper_damage_work [ 7.976196] Backtrace: [ 7.976214] dump_backtrace from show_stack+0x20/0x24 [ 7.976276] r7:000001d7 r6:00000009 r5:c0b2bc78 r4:60000013 [ 7.976289] show_stack from dump_stack_lvl+0x48/0x54 [ 7.976357] dump_stack_lvl from dump_stack+0x18/0x1c [ 7.976426] r5:c0586054 r4:c0b63750 [ 7.976436] dump_stack from __warn+0xdc/0x154 [ 7.976525] __warn from warn_slowpath_fmt+0xa4/0xd8 [ 7.976588] r7:000001d7 r6:c0b63750 r5:c1004ec8 r4:c0b639ec [ 7.976598] warn_slowpath_fmt from drm_fb_helper_damage_work+0x138/0x3b4 [ 7.976670] r9:ef7cf105 r8:c15dfc00 r7:fffffff4 r6:c200a590 r5:c1004ec8 r4:c200a594 [ 7.976681] drm_fb_helper_damage_work from process_one_work+0x230/0x518 [ 7.976761] r10:c110d140 r9:ef7cf105 r8:00000000 r7:ef7cf100 r6:ef7cbf00 r5:c200f300 [ 7.976775] r4:c200a594 [ 7.976785] process_one_work from worker_thread+0x54/0x554 [ 7.976841] r10:ef7cbf00 r9:00000008 r8:c1003d40 r7:ef7cbf1c r6:c200f318 r5:ef7cbf00 [ 7.976855] r4:c200f300 [ 7.976864] worker_thread from kthread+0xe8/0x104 [ 7.976948] r10:f0929e84 r9:c200ea40 r8:c169aa80 r7:c200f300 r6:c01419e4 r5:00000000 [ 7.976962] r4:c200e800 [ 7.976971] kthread from ret_from_fork+0x14/0x2c [ 7.977026] Exception stack(0xf092dfb0 to 0xf092dff8) [ 7.977052] dfa0: 00000000 00000000 00000000 00000000 [ 7.977078] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 7.977100] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 7.977128] r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c01491a8 [ 7.977144] r4:c200e800 r3:00000001 [ 7.977155] ---[ end trace 0000000000000000 ]--- On 10/29/21 13:50, John Keeping wrote: > The Rockchip fbdev code does not add anything compared to > drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the > same thing as gem_prime_mmap which is called by the helper. > > Signed-off-by: John Keeping <john@metanate.com> > --- > drivers/gpu/drm/rockchip/Makefile | 1 - > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- > 5 files changed, 2 insertions(+), 199 deletions(-) > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > index 17a9e7eb2130..1a56f696558c 100644 > --- a/drivers/gpu/drm/rockchip/Makefile > +++ b/drivers/gpu/drm/rockchip/Makefile > @@ -5,7 +5,6 @@ > > rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index 69c699459dce..20d81ae69828 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -26,7 +26,6 @@ > > #include "rockchip_drm_drv.h" > #include "rockchip_drm_fb.h" > -#include "rockchip_drm_fbdev.h" > #include "rockchip_drm_gem.h" > > #define DRIVER_NAME "rockchip" > @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) > > drm_mode_config_reset(drm_dev); > > - ret = rockchip_drm_fbdev_init(drm_dev); > - if (ret) > - goto err_unbind_all; > - > /* init kms poll for handling hpd */ > drm_kms_helper_poll_init(drm_dev); > > @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) > if (ret) > goto err_kms_helper_poll_fini; > > + drm_fbdev_generic_setup(drm_dev, 32); > + > return 0; > err_kms_helper_poll_fini: > drm_kms_helper_poll_fini(drm_dev); > - rockchip_drm_fbdev_fini(drm_dev); > err_unbind_all: > component_unbind_all(dev, drm_dev); > err_iommu_cleanup: > @@ -189,7 +185,6 @@ static void rockchip_drm_unbind(struct device *dev) > > drm_dev_unregister(drm_dev); > > - rockchip_drm_fbdev_fini(drm_dev); > drm_kms_helper_poll_fini(drm_dev); > > drm_atomic_helper_shutdown(drm_dev); > @@ -203,7 +198,6 @@ DEFINE_DRM_GEM_FOPS(rockchip_drm_driver_fops); > > static const struct drm_driver rockchip_drm_driver = { > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, > - .lastclose = drm_fb_helper_lastclose, > .dumb_create = rockchip_gem_dumb_create, > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > index aa0909e8edf9..143a48330f84 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > @@ -43,8 +43,6 @@ struct rockchip_crtc_state { > * @mm_lock: protect drm_mm on multi-threads. > */ > struct rockchip_drm_private { > - struct drm_fb_helper fbdev_helper; > - struct drm_gem_object *fbdev_bo; > struct iommu_domain *domain; > struct mutex mm_lock; > struct drm_mm mm; > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > deleted file mode 100644 > index d8418dd39d0e..000000000000 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > +++ /dev/null > @@ -1,164 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -/* > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > - * Author:Mark Yao <mark.yao@rock-chips.com> > - */ > - > -#include <drm/drm.h> > -#include <drm/drm_fb_helper.h> > -#include <drm/drm_fourcc.h> > -#include <drm/drm_prime.h> > -#include <drm/drm_probe_helper.h> > - > -#include "rockchip_drm_drv.h" > -#include "rockchip_drm_gem.h" > -#include "rockchip_drm_fb.h" > -#include "rockchip_drm_fbdev.h" > - > -#define PREFERRED_BPP 32 > -#define to_drm_private(x) \ > - container_of(x, struct rockchip_drm_private, fbdev_helper) > - > -static int rockchip_fbdev_mmap(struct fb_info *info, > - struct vm_area_struct *vma) > -{ > - struct drm_fb_helper *helper = info->par; > - struct rockchip_drm_private *private = to_drm_private(helper); > - > - return drm_gem_prime_mmap(private->fbdev_bo, vma); > -} > - > -static const struct fb_ops rockchip_drm_fbdev_ops = { > - .owner = THIS_MODULE, > - DRM_FB_HELPER_DEFAULT_OPS, > - .fb_mmap = rockchip_fbdev_mmap, > - .fb_fillrect = drm_fb_helper_cfb_fillrect, > - .fb_copyarea = drm_fb_helper_cfb_copyarea, > - .fb_imageblit = drm_fb_helper_cfb_imageblit, > -}; > - > -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper, > - struct drm_fb_helper_surface_size *sizes) > -{ > - struct rockchip_drm_private *private = to_drm_private(helper); > - struct drm_mode_fb_cmd2 mode_cmd = { 0 }; > - struct drm_device *dev = helper->dev; > - struct rockchip_gem_object *rk_obj; > - struct drm_framebuffer *fb; > - unsigned int bytes_per_pixel; > - unsigned long offset; > - struct fb_info *fbi; > - size_t size; > - int ret; > - > - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8); > - > - mode_cmd.width = sizes->surface_width; > - mode_cmd.height = sizes->surface_height; > - mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel; > - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, > - sizes->surface_depth); > - > - size = mode_cmd.pitches[0] * mode_cmd.height; > - > - rk_obj = rockchip_gem_create_object(dev, size, true); > - if (IS_ERR(rk_obj)) > - return -ENOMEM; > - > - private->fbdev_bo = &rk_obj->base; > - > - fbi = drm_fb_helper_alloc_fbi(helper); > - if (IS_ERR(fbi)) { > - DRM_DEV_ERROR(dev->dev, "Failed to create framebuffer info.\n"); > - ret = PTR_ERR(fbi); > - goto out; > - } > - > - helper->fb = rockchip_drm_framebuffer_init(dev, &mode_cmd, > - private->fbdev_bo); > - if (IS_ERR(helper->fb)) { > - DRM_DEV_ERROR(dev->dev, > - "Failed to allocate DRM framebuffer.\n"); > - ret = PTR_ERR(helper->fb); > - goto out; > - } > - > - fbi->fbops = &rockchip_drm_fbdev_ops; > - > - fb = helper->fb; > - drm_fb_helper_fill_info(fbi, helper, sizes); > - > - offset = fbi->var.xoffset * bytes_per_pixel; > - offset += fbi->var.yoffset * fb->pitches[0]; > - > - dev->mode_config.fb_base = 0; > - fbi->screen_base = rk_obj->kvaddr + offset; > - fbi->screen_size = rk_obj->base.size; > - fbi->fix.smem_len = rk_obj->base.size; > - > - DRM_DEBUG_KMS("FB [%dx%d]-%d kvaddr=%p offset=%ld size=%zu\n", > - fb->width, fb->height, fb->format->depth, > - rk_obj->kvaddr, > - offset, size); > - > - return 0; > - > -out: > - rockchip_gem_free_object(&rk_obj->base); > - return ret; > -} > - > -static const struct drm_fb_helper_funcs rockchip_drm_fb_helper_funcs = { > - .fb_probe = rockchip_drm_fbdev_create, > -}; > - > -int rockchip_drm_fbdev_init(struct drm_device *dev) > -{ > - struct rockchip_drm_private *private = dev->dev_private; > - struct drm_fb_helper *helper; > - int ret; > - > - if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector) > - return -EINVAL; > - > - helper = &private->fbdev_helper; > - > - drm_fb_helper_prepare(dev, helper, &rockchip_drm_fb_helper_funcs); > - > - ret = drm_fb_helper_init(dev, helper); > - if (ret < 0) { > - DRM_DEV_ERROR(dev->dev, > - "Failed to initialize drm fb helper - %d.\n", > - ret); > - return ret; > - } > - > - ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP); > - if (ret < 0) { > - DRM_DEV_ERROR(dev->dev, > - "Failed to set initial hw config - %d.\n", > - ret); > - goto err_drm_fb_helper_fini; > - } > - > - return 0; > - > -err_drm_fb_helper_fini: > - drm_fb_helper_fini(helper); > - return ret; > -} > - > -void rockchip_drm_fbdev_fini(struct drm_device *dev) > -{ > - struct rockchip_drm_private *private = dev->dev_private; > - struct drm_fb_helper *helper; > - > - helper = &private->fbdev_helper; > - > - drm_fb_helper_unregister_fbi(helper); > - > - if (helper->fb) > - drm_framebuffer_put(helper->fb); > - > - drm_fb_helper_fini(helper); > -} > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > deleted file mode 100644 > index 5fb7ac2371a8..000000000000 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > +++ /dev/null > @@ -1,24 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0-only */ > -/* > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > - * Author:Mark Yao <mark.yao@rock-chips.com> > - */ > - > -#ifndef _ROCKCHIP_DRM_FBDEV_H > -#define _ROCKCHIP_DRM_FBDEV_H > - > -#ifdef CONFIG_DRM_FBDEV_EMULATION > -int rockchip_drm_fbdev_init(struct drm_device *dev); > -void rockchip_drm_fbdev_fini(struct drm_device *dev); > -#else > -static inline int rockchip_drm_fbdev_init(struct drm_device *dev) > -{ > - return 0; > -} > - > -static inline void rockchip_drm_fbdev_fini(struct drm_device *dev) > -{ > -} > -#endif > - > -#endif /* _ROCKCHIP_DRM_FBDEV_H */
Hi Johan, On Mon, Oct 17, 2022 at 10:11:32AM +0200, Johan Jonker wrote: > Your patch contribution causes a kernel panic on MK808 with Rockchip rk3066a SoC. > Would you like to contribute to fix this issue? > The assumtion that drm_fbdev_generic_setup() does what rockchip_drm_fbdev_init did is not true! > A revert makes it work again. It looks like there are 3 different ways to end up with -ENOMEM here, can you track down whether you're hitting one of the cases in rockchip_gem_prime_vmap() or if it's the iosys_map_is_null case in drm_gem_vmap()? I guess the memory usage increases slightly using the generic code and RK3066 has less memory available. > ====== > > [ 7.975906] ------------[ cut here ]------------ > [ 7.975929] WARNING: CPU: 0 PID: 35 at drivers/gpu/drm/drm_fb_helper.c:471 drm_fb_helper_damage_work+0x138/0x3b4 > [ 7.976044] rockchip-drm display-subsystem: Damage blitter failed: ret=-12 > [ 7.976064] Modules linked in: > [ 7.976090] CPU: 0 PID: 35 Comm: kworker/0:4 Not tainted 6.0.0-next-20221013 #1 > [ 7.976126] Hardware name: Rockchip (Device Tree) > [ 7.976145] Workqueue: events drm_fb_helper_damage_work > [ 7.976196] Backtrace: > [ 7.976214] dump_backtrace from show_stack+0x20/0x24 > [ 7.976276] r7:000001d7 r6:00000009 r5:c0b2bc78 r4:60000013 > [ 7.976289] show_stack from dump_stack_lvl+0x48/0x54 > [ 7.976357] dump_stack_lvl from dump_stack+0x18/0x1c > [ 7.976426] r5:c0586054 r4:c0b63750 > [ 7.976436] dump_stack from __warn+0xdc/0x154 > [ 7.976525] __warn from warn_slowpath_fmt+0xa4/0xd8 > [ 7.976588] r7:000001d7 r6:c0b63750 r5:c1004ec8 r4:c0b639ec > [ 7.976598] warn_slowpath_fmt from drm_fb_helper_damage_work+0x138/0x3b4 > [ 7.976670] r9:ef7cf105 r8:c15dfc00 r7:fffffff4 r6:c200a590 r5:c1004ec8 r4:c200a594 > [ 7.976681] drm_fb_helper_damage_work from process_one_work+0x230/0x518 > [ 7.976761] r10:c110d140 r9:ef7cf105 r8:00000000 r7:ef7cf100 r6:ef7cbf00 r5:c200f300 > [ 7.976775] r4:c200a594 > [ 7.976785] process_one_work from worker_thread+0x54/0x554 > [ 7.976841] r10:ef7cbf00 r9:00000008 r8:c1003d40 r7:ef7cbf1c r6:c200f318 r5:ef7cbf00 > [ 7.976855] r4:c200f300 > [ 7.976864] worker_thread from kthread+0xe8/0x104 > [ 7.976948] r10:f0929e84 r9:c200ea40 r8:c169aa80 r7:c200f300 r6:c01419e4 r5:00000000 > [ 7.976962] r4:c200e800 > [ 7.976971] kthread from ret_from_fork+0x14/0x2c > [ 7.977026] Exception stack(0xf092dfb0 to 0xf092dff8) > [ 7.977052] dfa0: 00000000 00000000 00000000 00000000 > [ 7.977078] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > [ 7.977100] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > [ 7.977128] r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c01491a8 > [ 7.977144] r4:c200e800 r3:00000001 > [ 7.977155] ---[ end trace 0000000000000000 ]--- > > On 10/29/21 13:50, John Keeping wrote: > > The Rockchip fbdev code does not add anything compared to > > drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the > > same thing as gem_prime_mmap which is called by the helper. > > > > Signed-off-by: John Keeping <john@metanate.com> > > --- > > drivers/gpu/drm/rockchip/Makefile | 1 - > > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- > > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - > > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ > > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- > > 5 files changed, 2 insertions(+), 199 deletions(-) > > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > > > diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > > index 17a9e7eb2130..1a56f696558c 100644 > > --- a/drivers/gpu/drm/rockchip/Makefile > > +++ b/drivers/gpu/drm/rockchip/Makefile > > @@ -5,7 +5,6 @@ > > > > rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > > rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > > -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > > > rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > > rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > index 69c699459dce..20d81ae69828 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > @@ -26,7 +26,6 @@ > > > > #include "rockchip_drm_drv.h" > > #include "rockchip_drm_fb.h" > > -#include "rockchip_drm_fbdev.h" > > #include "rockchip_drm_gem.h" > > > > #define DRIVER_NAME "rockchip" > > @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) > > > > drm_mode_config_reset(drm_dev); > > > > - ret = rockchip_drm_fbdev_init(drm_dev); > > - if (ret) > > - goto err_unbind_all; > > - > > /* init kms poll for handling hpd */ > > drm_kms_helper_poll_init(drm_dev); > > > > @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) > > if (ret) > > goto err_kms_helper_poll_fini; > > > > + drm_fbdev_generic_setup(drm_dev, 32); > > + > > return 0; > > err_kms_helper_poll_fini: > > drm_kms_helper_poll_fini(drm_dev); > > - rockchip_drm_fbdev_fini(drm_dev); > > err_unbind_all: > > component_unbind_all(dev, drm_dev); > > err_iommu_cleanup: > > @@ -189,7 +185,6 @@ static void rockchip_drm_unbind(struct device *dev) > > > > drm_dev_unregister(drm_dev); > > > > - rockchip_drm_fbdev_fini(drm_dev); > > drm_kms_helper_poll_fini(drm_dev); > > > > drm_atomic_helper_shutdown(drm_dev); > > @@ -203,7 +198,6 @@ DEFINE_DRM_GEM_FOPS(rockchip_drm_driver_fops); > > > > static const struct drm_driver rockchip_drm_driver = { > > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, > > - .lastclose = drm_fb_helper_lastclose, > > .dumb_create = rockchip_gem_dumb_create, > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > > index aa0909e8edf9..143a48330f84 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > > @@ -43,8 +43,6 @@ struct rockchip_crtc_state { > > * @mm_lock: protect drm_mm on multi-threads. > > */ > > struct rockchip_drm_private { > > - struct drm_fb_helper fbdev_helper; > > - struct drm_gem_object *fbdev_bo; > > struct iommu_domain *domain; > > struct mutex mm_lock; > > struct drm_mm mm; > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > > deleted file mode 100644 > > index d8418dd39d0e..000000000000 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > > +++ /dev/null > > @@ -1,164 +0,0 @@ > > -// SPDX-License-Identifier: GPL-2.0-only > > -/* > > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > > - * Author:Mark Yao <mark.yao@rock-chips.com> > > - */ > > - > > -#include <drm/drm.h> > > -#include <drm/drm_fb_helper.h> > > -#include <drm/drm_fourcc.h> > > -#include <drm/drm_prime.h> > > -#include <drm/drm_probe_helper.h> > > - > > -#include "rockchip_drm_drv.h" > > -#include "rockchip_drm_gem.h" > > -#include "rockchip_drm_fb.h" > > -#include "rockchip_drm_fbdev.h" > > - > > -#define PREFERRED_BPP 32 > > -#define to_drm_private(x) \ > > - container_of(x, struct rockchip_drm_private, fbdev_helper) > > - > > -static int rockchip_fbdev_mmap(struct fb_info *info, > > - struct vm_area_struct *vma) > > -{ > > - struct drm_fb_helper *helper = info->par; > > - struct rockchip_drm_private *private = to_drm_private(helper); > > - > > - return drm_gem_prime_mmap(private->fbdev_bo, vma); > > -} > > - > > -static const struct fb_ops rockchip_drm_fbdev_ops = { > > - .owner = THIS_MODULE, > > - DRM_FB_HELPER_DEFAULT_OPS, > > - .fb_mmap = rockchip_fbdev_mmap, > > - .fb_fillrect = drm_fb_helper_cfb_fillrect, > > - .fb_copyarea = drm_fb_helper_cfb_copyarea, > > - .fb_imageblit = drm_fb_helper_cfb_imageblit, > > -}; > > - > > -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper, > > - struct drm_fb_helper_surface_size *sizes) > > -{ > > - struct rockchip_drm_private *private = to_drm_private(helper); > > - struct drm_mode_fb_cmd2 mode_cmd = { 0 }; > > - struct drm_device *dev = helper->dev; > > - struct rockchip_gem_object *rk_obj; > > - struct drm_framebuffer *fb; > > - unsigned int bytes_per_pixel; > > - unsigned long offset; > > - struct fb_info *fbi; > > - size_t size; > > - int ret; > > - > > - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8); > > - > > - mode_cmd.width = sizes->surface_width; > > - mode_cmd.height = sizes->surface_height; > > - mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel; > > - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, > > - sizes->surface_depth); > > - > > - size = mode_cmd.pitches[0] * mode_cmd.height; > > - > > - rk_obj = rockchip_gem_create_object(dev, size, true); > > - if (IS_ERR(rk_obj)) > > - return -ENOMEM; > > - > > - private->fbdev_bo = &rk_obj->base; > > - > > - fbi = drm_fb_helper_alloc_fbi(helper); > > - if (IS_ERR(fbi)) { > > - DRM_DEV_ERROR(dev->dev, "Failed to create framebuffer info.\n"); > > - ret = PTR_ERR(fbi); > > - goto out; > > - } > > - > > - helper->fb = rockchip_drm_framebuffer_init(dev, &mode_cmd, > > - private->fbdev_bo); > > - if (IS_ERR(helper->fb)) { > > - DRM_DEV_ERROR(dev->dev, > > - "Failed to allocate DRM framebuffer.\n"); > > - ret = PTR_ERR(helper->fb); > > - goto out; > > - } > > - > > - fbi->fbops = &rockchip_drm_fbdev_ops; > > - > > - fb = helper->fb; > > - drm_fb_helper_fill_info(fbi, helper, sizes); > > - > > - offset = fbi->var.xoffset * bytes_per_pixel; > > - offset += fbi->var.yoffset * fb->pitches[0]; > > - > > - dev->mode_config.fb_base = 0; > > - fbi->screen_base = rk_obj->kvaddr + offset; > > - fbi->screen_size = rk_obj->base.size; > > - fbi->fix.smem_len = rk_obj->base.size; > > - > > - DRM_DEBUG_KMS("FB [%dx%d]-%d kvaddr=%p offset=%ld size=%zu\n", > > - fb->width, fb->height, fb->format->depth, > > - rk_obj->kvaddr, > > - offset, size); > > - > > - return 0; > > - > > -out: > > - rockchip_gem_free_object(&rk_obj->base); > > - return ret; > > -} > > - > > -static const struct drm_fb_helper_funcs rockchip_drm_fb_helper_funcs = { > > - .fb_probe = rockchip_drm_fbdev_create, > > -}; > > - > > -int rockchip_drm_fbdev_init(struct drm_device *dev) > > -{ > > - struct rockchip_drm_private *private = dev->dev_private; > > - struct drm_fb_helper *helper; > > - int ret; > > - > > - if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector) > > - return -EINVAL; > > - > > - helper = &private->fbdev_helper; > > - > > - drm_fb_helper_prepare(dev, helper, &rockchip_drm_fb_helper_funcs); > > - > > - ret = drm_fb_helper_init(dev, helper); > > - if (ret < 0) { > > - DRM_DEV_ERROR(dev->dev, > > - "Failed to initialize drm fb helper - %d.\n", > > - ret); > > - return ret; > > - } > > - > > - ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP); > > - if (ret < 0) { > > - DRM_DEV_ERROR(dev->dev, > > - "Failed to set initial hw config - %d.\n", > > - ret); > > - goto err_drm_fb_helper_fini; > > - } > > - > > - return 0; > > - > > -err_drm_fb_helper_fini: > > - drm_fb_helper_fini(helper); > > - return ret; > > -} > > - > > -void rockchip_drm_fbdev_fini(struct drm_device *dev) > > -{ > > - struct rockchip_drm_private *private = dev->dev_private; > > - struct drm_fb_helper *helper; > > - > > - helper = &private->fbdev_helper; > > - > > - drm_fb_helper_unregister_fbi(helper); > > - > > - if (helper->fb) > > - drm_framebuffer_put(helper->fb); > > - > > - drm_fb_helper_fini(helper); > > -} > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > deleted file mode 100644 > > index 5fb7ac2371a8..000000000000 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > +++ /dev/null > > @@ -1,24 +0,0 @@ > > -/* SPDX-License-Identifier: GPL-2.0-only */ > > -/* > > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > > - * Author:Mark Yao <mark.yao@rock-chips.com> > > - */ > > - > > -#ifndef _ROCKCHIP_DRM_FBDEV_H > > -#define _ROCKCHIP_DRM_FBDEV_H > > - > > -#ifdef CONFIG_DRM_FBDEV_EMULATION > > -int rockchip_drm_fbdev_init(struct drm_device *dev); > > -void rockchip_drm_fbdev_fini(struct drm_device *dev); > > -#else > > -static inline int rockchip_drm_fbdev_init(struct drm_device *dev) > > -{ > > - return 0; > > -} > > - > > -static inline void rockchip_drm_fbdev_fini(struct drm_device *dev) > > -{ > > -} > > -#endif > > - > > -#endif /* _ROCKCHIP_DRM_FBDEV_H */ > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Am Montag, 17. Oktober 2022, 12:05:16 CEST schrieb John Keeping: > Hi Johan, > > On Mon, Oct 17, 2022 at 10:11:32AM +0200, Johan Jonker wrote: > > Your patch contribution causes a kernel panic on MK808 with Rockchip rk3066a SoC. > > Would you like to contribute to fix this issue? > > The assumtion that drm_fbdev_generic_setup() does what rockchip_drm_fbdev_init did is not true! > > A revert makes it work again. > > It looks like there are 3 different ways to end up with -ENOMEM here, > can you track down whether you're hitting one of the cases in > rockchip_gem_prime_vmap() or if it's the iosys_map_is_null case in > drm_gem_vmap()? > > I guess the memory usage increases slightly using the generic code and > RK3066 has less memory available. also rk3066 and rk3188 do not have an iommu, so rely on cma allocations. Heiko
On 10/17/22 13:29, Heiko Stuebner wrote: > Am Montag, 17. Oktober 2022, 12:05:16 CEST schrieb John Keeping: >> Hi Johan, >> >> On Mon, Oct 17, 2022 at 10:11:32AM +0200, Johan Jonker wrote: >>> Your patch contribution causes a kernel panic on MK808 with Rockchip rk3066a SoC. >>> Would you like to contribute to fix this issue? >>> The assumtion that drm_fbdev_generic_setup() does what rockchip_drm_fbdev_init did is not true! >>> A revert makes it work again. >> >> It looks like there are 3 different ways to end up with -ENOMEM here, >> can you track down whether you're hitting one of the cases in >> rockchip_gem_prime_vmap() or if it's the iosys_map_is_null case in >> drm_gem_vmap()? It looks like it comes from rockchip_gem_prime_vmap() second return (2). ==== int rockchip_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map) { struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); if (rk_obj->pages) { void *vaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP, pgprot_writecombine(PAGE_KERNEL)); if (!vaddr) { printk("FBDEV rockchip_gem_prime_vmap 1"); return -ENOMEM; } iosys_map_set_vaddr(map, vaddr); return 0; } if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) { //////////////// printk("FBDEV rockchip_gem_prime_vmap 2"); //////////////// return -ENOMEM; } iosys_map_set_vaddr(map, rk_obj->kvaddr); return 0; } ==== [ 7.678392] [drm:drm_client_modeset_probe] connector 39 enabled? yes [ 7.678435] [drm:drm_client_modeset_probe] Not using firmware configuration [ 7.678465] [drm:drm_client_modeset_probe] looking for cmdline mode on connector 39 [ 7.678494] [drm:drm_client_modeset_probe] looking for preferred mode on connector 39 0 [ 7.678521] [drm:drm_client_modeset_probe] found mode 1920x1080 [ 7.678545] [drm:drm_client_modeset_probe] picking CRTCs for 1920x1080 config [ 7.678585] [drm:drm_client_modeset_probe] desired mode 1920x1080 set on crtc 35 (0,0) [ 7.801673] Console: switching to colour frame buffer device 240x67 [ 7.811047] FBDEV rockchip_gem_prime_vmap 2 [ 7.811071] ------------[ cut here ]------------ [ 7.811084] WARNING: CPU: 0 PID: 35 at drivers/gpu/drm/drm_fb_helper.c:471 drm_fb_helper_damage_work+0x138/0x3b4 [ 7.811198] rockchip-drm display-subsystem: Damage blitter failed: ret=-12 [ 7.811219] Modules linked in: [ 7.811244] CPU: 0 PID: 35 Comm: kworker/0:4 Not tainted 6.0.0-next-20221013+ #46 [ 7.811281] Hardware name: Rockchip (Device Tree) [ 7.811300] Workqueue: events drm_fb_helper_damage_work [ 7.811352] Backtrace: [ 7.811370] dump_backtrace from show_stack+0x20/0x24 [ 7.811431] r7:000001d7 r6:00000009 r5:c0b2bc60 r4:60000013 [ 7.811444] show_stack from dump_stack_lvl+0x48/0x54 [ 7.811512] dump_stack_lvl from dump_stack+0x18/0x1c [ 7.811580] r5:c0586064 r4:c0b6374c [ 7.811590] dump_stack from __warn+0xdc/0x154 [ 7.811677] __warn from warn_slowpath_fmt+0xa4/0xd8 [ 7.811740] r7:000001d7 r6:c0b6374c r5:c1004ec8 r4:c0b639e8 [ 7.811750] warn_slowpath_fmt from drm_fb_helper_damage_work+0x138/0x3b4 [ 7.811821] r9:ef7cf105 r8:c15dfc00 r7:fffffff4 r6:c200b490 r5:c1004ec8 r4:c200b494 [ 7.811833] drm_fb_helper_damage_work from process_one_work+0x230/0x518 [ 7.811912] r10:c110d140 r9:ef7cf105 r8:00000000 r7:ef7cf100 r6:ef7cbf00 r5:c200e300 [ 7.811927] r4:c200b494 [ 7.811936] process_one_work from worker_thread+0x54/0x554 [ 7.811991] r10:ef7cbf00 r9:00000008 r8:c1003d40 r7:ef7cbf1c r6:c200e318 r5:ef7cbf00 [ 7.812006] r4:c200e300 [ 7.812015] worker_thread from kthread+0xe8/0x104 [ 7.812100] r10:f0929e84 r9:c200da00 r8:c169aa80 r7:c200e300 r6:c01419e4 r5:00000000 [ 7.812114] r4:c200d780 [ 7.812124] kthread from ret_from_fork+0x14/0x2c [ 7.812178] Exception stack(0xf092dfb0 to 0xf092dff8) [ 7.812205] dfa0: 00000000 00000000 00000000 00000000 [ 7.812232] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 7.812255] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 7.812282] r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c01491a8 [ 7.812299] r4:c200d780 r3:00000001 [ 7.812309] ---[ end trace 0000000000000000 ]--- [ 7.812336] FBDEV rockchip_gem_prime_vmap 2 [ 7.889795] FBDEV rockchip_gem_prime_vmap 2 [ 7.890418] FBDEV rockchip_gem_prime_vmap 2 [ 7.899447] FBDEV rockchip_gem_prime_vmap 2 [ 7.905252] FBDEV rockchip_gem_prime_vmap 2 >> >> I guess the memory usage increases slightly using the generic code and >> RK3066 has less memory available. > > also rk3066 and rk3188 do not have an iommu, so rely > on cma allocations. > > > Heiko > >
On Mon, Oct 17, 2022 at 08:30:23PM +0200, Johan Jonker wrote: > > > On 10/17/22 13:29, Heiko Stuebner wrote: > > Am Montag, 17. Oktober 2022, 12:05:16 CEST schrieb John Keeping: > >> Hi Johan, > >> > >> On Mon, Oct 17, 2022 at 10:11:32AM +0200, Johan Jonker wrote: > >>> Your patch contribution causes a kernel panic on MK808 with Rockchip rk3066a SoC. > >>> Would you like to contribute to fix this issue? > >>> The assumtion that drm_fbdev_generic_setup() does what rockchip_drm_fbdev_init did is not true! > >>> A revert makes it work again. > >> > > >> It looks like there are 3 different ways to end up with -ENOMEM here, > >> can you track down whether you're hitting one of the cases in > >> rockchip_gem_prime_vmap() or if it's the iosys_map_is_null case in > >> drm_gem_vmap()? > > It looks like it comes from rockchip_gem_prime_vmap() second return (2). > > > if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) { > > //////////////// > > printk("FBDEV rockchip_gem_prime_vmap 2"); > > //////////////// > return -ENOMEM; > } Ah-ha, Heiko was right that this is because the no-iommu path is broken as a result of switching to the generic fbdev code. This patch should fix it, but I wonder if Thomas has any ideas about a better way to handle this since it feels a bit hacky to special-case the fb_helper inside the GEM code: -- >8 -- diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index 614e97aaac80..da8a69953706 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -364,9 +364,12 @@ rockchip_gem_create_with_handle(struct drm_file *file_priv, { struct rockchip_gem_object *rk_obj; struct drm_gem_object *obj; + bool is_framebuffer; int ret; - rk_obj = rockchip_gem_create_object(drm, size, false); + is_framebuffer = drm->fb_helper && file_priv == drm->fb_helper->client.file; + + rk_obj = rockchip_gem_create_object(drm, size, is_framebuffer); if (IS_ERR(rk_obj)) return ERR_CAST(rk_obj); -- 8< --
On 10/17/22 21:00, John Keeping wrote: > On Mon, Oct 17, 2022 at 08:30:23PM +0200, Johan Jonker wrote: >> >> >> On 10/17/22 13:29, Heiko Stuebner wrote: >>> Am Montag, 17. Oktober 2022, 12:05:16 CEST schrieb John Keeping: >>>> Hi Johan, >>>> >>>> On Mon, Oct 17, 2022 at 10:11:32AM +0200, Johan Jonker wrote: >>>>> Your patch contribution causes a kernel panic on MK808 with Rockchip rk3066a SoC. >>>>> Would you like to contribute to fix this issue? >>>>> The assumtion that drm_fbdev_generic_setup() does what rockchip_drm_fbdev_init did is not true! >>>>> A revert makes it work again. >>>> >> >>>> It looks like there are 3 different ways to end up with -ENOMEM here, >>>> can you track down whether you're hitting one of the cases in >>>> rockchip_gem_prime_vmap() or if it's the iosys_map_is_null case in >>>> drm_gem_vmap()? >> >> It looks like it comes from rockchip_gem_prime_vmap() second return (2). >> >> >> if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) { >> >> //////////////// >> >> printk("FBDEV rockchip_gem_prime_vmap 2"); >> >> //////////////// >> return -ENOMEM; >> } > > Ah-ha, Heiko was right that this is because the no-iommu path is broken > as a result of switching to the generic fbdev code. > > This patch should fix it, but I wonder if Thomas has any ideas about a > better way to handle this since it feels a bit hacky to special-case the > fb_helper inside the GEM code: The penguin is back on screen. Thanks! > > -- >8 -- > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index 614e97aaac80..da8a69953706 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -364,9 +364,12 @@ rockchip_gem_create_with_handle(struct drm_file *file_priv, > { > struct rockchip_gem_object *rk_obj; > struct drm_gem_object *obj; > + bool is_framebuffer; > int ret; > > - rk_obj = rockchip_gem_create_object(drm, size, false); > + is_framebuffer = drm->fb_helper && file_priv == drm->fb_helper->client.file; > + > + rk_obj = rockchip_gem_create_object(drm, size, is_framebuffer); > if (IS_ERR(rk_obj)) > return ERR_CAST(rk_obj); > -- 8< --
diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile index 17a9e7eb2130..1a56f696558c 100644 --- a/drivers/gpu/drm/rockchip/Makefile +++ b/drivers/gpu/drm/rockchip/Makefile @@ -5,7 +5,6 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 69c699459dce..20d81ae69828 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -26,7 +26,6 @@ #include "rockchip_drm_drv.h" #include "rockchip_drm_fb.h" -#include "rockchip_drm_fbdev.h" #include "rockchip_drm_gem.h" #define DRIVER_NAME "rockchip" @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) drm_mode_config_reset(drm_dev); - ret = rockchip_drm_fbdev_init(drm_dev); - if (ret) - goto err_unbind_all; - /* init kms poll for handling hpd */ drm_kms_helper_poll_init(drm_dev); @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) if (ret) goto err_kms_helper_poll_fini; + drm_fbdev_generic_setup(drm_dev, 32); + return 0; err_kms_helper_poll_fini: drm_kms_helper_poll_fini(drm_dev); - rockchip_drm_fbdev_fini(drm_dev); err_unbind_all: component_unbind_all(dev, drm_dev); err_iommu_cleanup: @@ -189,7 +185,6 @@ static void rockchip_drm_unbind(struct device *dev) drm_dev_unregister(drm_dev); - rockchip_drm_fbdev_fini(drm_dev); drm_kms_helper_poll_fini(drm_dev); drm_atomic_helper_shutdown(drm_dev); @@ -203,7 +198,6 @@ DEFINE_DRM_GEM_FOPS(rockchip_drm_driver_fops); static const struct drm_driver rockchip_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, - .lastclose = drm_fb_helper_lastclose, .dumb_create = rockchip_gem_dumb_create, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index aa0909e8edf9..143a48330f84 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -43,8 +43,6 @@ struct rockchip_crtc_state { * @mm_lock: protect drm_mm on multi-threads. */ struct rockchip_drm_private { - struct drm_fb_helper fbdev_helper; - struct drm_gem_object *fbdev_bo; struct iommu_domain *domain; struct mutex mm_lock; struct drm_mm mm; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c deleted file mode 100644 index d8418dd39d0e..000000000000 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c +++ /dev/null @@ -1,164 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd - * Author:Mark Yao <mark.yao@rock-chips.com> - */ - -#include <drm/drm.h> -#include <drm/drm_fb_helper.h> -#include <drm/drm_fourcc.h> -#include <drm/drm_prime.h> -#include <drm/drm_probe_helper.h> - -#include "rockchip_drm_drv.h" -#include "rockchip_drm_gem.h" -#include "rockchip_drm_fb.h" -#include "rockchip_drm_fbdev.h" - -#define PREFERRED_BPP 32 -#define to_drm_private(x) \ - container_of(x, struct rockchip_drm_private, fbdev_helper) - -static int rockchip_fbdev_mmap(struct fb_info *info, - struct vm_area_struct *vma) -{ - struct drm_fb_helper *helper = info->par; - struct rockchip_drm_private *private = to_drm_private(helper); - - return drm_gem_prime_mmap(private->fbdev_bo, vma); -} - -static const struct fb_ops rockchip_drm_fbdev_ops = { - .owner = THIS_MODULE, - DRM_FB_HELPER_DEFAULT_OPS, - .fb_mmap = rockchip_fbdev_mmap, - .fb_fillrect = drm_fb_helper_cfb_fillrect, - .fb_copyarea = drm_fb_helper_cfb_copyarea, - .fb_imageblit = drm_fb_helper_cfb_imageblit, -}; - -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper, - struct drm_fb_helper_surface_size *sizes) -{ - struct rockchip_drm_private *private = to_drm_private(helper); - struct drm_mode_fb_cmd2 mode_cmd = { 0 }; - struct drm_device *dev = helper->dev; - struct rockchip_gem_object *rk_obj; - struct drm_framebuffer *fb; - unsigned int bytes_per_pixel; - unsigned long offset; - struct fb_info *fbi; - size_t size; - int ret; - - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8); - - mode_cmd.width = sizes->surface_width; - mode_cmd.height = sizes->surface_height; - mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel; - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, - sizes->surface_depth); - - size = mode_cmd.pitches[0] * mode_cmd.height; - - rk_obj = rockchip_gem_create_object(dev, size, true); - if (IS_ERR(rk_obj)) - return -ENOMEM; - - private->fbdev_bo = &rk_obj->base; - - fbi = drm_fb_helper_alloc_fbi(helper); - if (IS_ERR(fbi)) { - DRM_DEV_ERROR(dev->dev, "Failed to create framebuffer info.\n"); - ret = PTR_ERR(fbi); - goto out; - } - - helper->fb = rockchip_drm_framebuffer_init(dev, &mode_cmd, - private->fbdev_bo); - if (IS_ERR(helper->fb)) { - DRM_DEV_ERROR(dev->dev, - "Failed to allocate DRM framebuffer.\n"); - ret = PTR_ERR(helper->fb); - goto out; - } - - fbi->fbops = &rockchip_drm_fbdev_ops; - - fb = helper->fb; - drm_fb_helper_fill_info(fbi, helper, sizes); - - offset = fbi->var.xoffset * bytes_per_pixel; - offset += fbi->var.yoffset * fb->pitches[0]; - - dev->mode_config.fb_base = 0; - fbi->screen_base = rk_obj->kvaddr + offset; - fbi->screen_size = rk_obj->base.size; - fbi->fix.smem_len = rk_obj->base.size; - - DRM_DEBUG_KMS("FB [%dx%d]-%d kvaddr=%p offset=%ld size=%zu\n", - fb->width, fb->height, fb->format->depth, - rk_obj->kvaddr, - offset, size); - - return 0; - -out: - rockchip_gem_free_object(&rk_obj->base); - return ret; -} - -static const struct drm_fb_helper_funcs rockchip_drm_fb_helper_funcs = { - .fb_probe = rockchip_drm_fbdev_create, -}; - -int rockchip_drm_fbdev_init(struct drm_device *dev) -{ - struct rockchip_drm_private *private = dev->dev_private; - struct drm_fb_helper *helper; - int ret; - - if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector) - return -EINVAL; - - helper = &private->fbdev_helper; - - drm_fb_helper_prepare(dev, helper, &rockchip_drm_fb_helper_funcs); - - ret = drm_fb_helper_init(dev, helper); - if (ret < 0) { - DRM_DEV_ERROR(dev->dev, - "Failed to initialize drm fb helper - %d.\n", - ret); - return ret; - } - - ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP); - if (ret < 0) { - DRM_DEV_ERROR(dev->dev, - "Failed to set initial hw config - %d.\n", - ret); - goto err_drm_fb_helper_fini; - } - - return 0; - -err_drm_fb_helper_fini: - drm_fb_helper_fini(helper); - return ret; -} - -void rockchip_drm_fbdev_fini(struct drm_device *dev) -{ - struct rockchip_drm_private *private = dev->dev_private; - struct drm_fb_helper *helper; - - helper = &private->fbdev_helper; - - drm_fb_helper_unregister_fbi(helper); - - if (helper->fb) - drm_framebuffer_put(helper->fb); - - drm_fb_helper_fini(helper); -} diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h deleted file mode 100644 index 5fb7ac2371a8..000000000000 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h +++ /dev/null @@ -1,24 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd - * Author:Mark Yao <mark.yao@rock-chips.com> - */ - -#ifndef _ROCKCHIP_DRM_FBDEV_H -#define _ROCKCHIP_DRM_FBDEV_H - -#ifdef CONFIG_DRM_FBDEV_EMULATION -int rockchip_drm_fbdev_init(struct drm_device *dev); -void rockchip_drm_fbdev_fini(struct drm_device *dev); -#else -static inline int rockchip_drm_fbdev_init(struct drm_device *dev) -{ - return 0; -} - -static inline void rockchip_drm_fbdev_fini(struct drm_device *dev) -{ -} -#endif - -#endif /* _ROCKCHIP_DRM_FBDEV_H */
The Rockchip fbdev code does not add anything compared to drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the same thing as gem_prime_mmap which is called by the helper. Signed-off-by: John Keeping <john@metanate.com> --- drivers/gpu/drm/rockchip/Makefile | 1 - drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 ------------------ drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- 5 files changed, 2 insertions(+), 199 deletions(-) delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h