| Message ID | 20240225064700.48035-1-tony@atomide.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fixes for omapdrm console | expand |
Hi, looks good. For the series: Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Am 25.02.24 um 07:46 schrieb Tony Lindgren: > Here are two fixes for omapdrm console. > > Regards, > > Tony > > Changes since v1: > > - Add FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS to use with > FB_DEFAULT_DEFERRED_OPS as suggested by Thomas > > Tony Lindgren (2): > drm/omapdrm: Fix console by implementing fb_dirty > drm/omapdrm: Fix console with deferred ops > > drivers/gpu/drm/omapdrm/omap_fbdev.c | 39 +++++++++++++++++++--------- > include/linux/fb.h | 4 +++ > 2 files changed, 31 insertions(+), 12 deletions(-) >
Hi Tony, On 25/02/2024 08:46, Tony Lindgren wrote: > Here are two fixes for omapdrm console. How is it broken? I don't usually use the console (or fbdev) but enabling it now, it seems to work fine for me, on DRA76 EVM with HDMI output. Tomi > > Regards, > > Tony > > Changes since v1: > > - Add FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS to use with > FB_DEFAULT_DEFERRED_OPS as suggested by Thomas > > Tony Lindgren (2): > drm/omapdrm: Fix console by implementing fb_dirty > drm/omapdrm: Fix console with deferred ops > > drivers/gpu/drm/omapdrm/omap_fbdev.c | 39 +++++++++++++++++++--------- > include/linux/fb.h | 4 +++ > 2 files changed, 31 insertions(+), 12 deletions(-) >
On 26/02/2024 10:26, Tomi Valkeinen wrote: > Hi Tony, > > On 25/02/2024 08:46, Tony Lindgren wrote: >> Here are two fixes for omapdrm console. > > How is it broken? I don't usually use the console (or fbdev) but > enabling it now, it seems to work fine for me, on DRA76 EVM with HDMI > output. After applying your patches, I see a lot of cache-related artifacts on the screen when updating the fb. Tomi
Hi Am 26.02.24 um 10:01 schrieb Tomi Valkeinen: > On 26/02/2024 10:26, Tomi Valkeinen wrote: >> Hi Tony, >> >> On 25/02/2024 08:46, Tony Lindgren wrote: >>> Here are two fixes for omapdrm console. >> >> How is it broken? I don't usually use the console (or fbdev) but >> enabling it now, it seems to work fine for me, on DRA76 EVM with HDMI >> output. Omapdrm implements drm_framebuffer_funcs.dirty withomap_framebuffer_dirty(). AFAIK DRM semantics requires to run the dirty helper after writing to the framebuffer's memory. Userspace does this via the DIRTYFB ioctl. [1] But (at least) for correctness the console needs to do the same. [1] https://elixir.bootlin.com/linux/v6.7.6/source/drivers/gpu/drm/drm_ioctl.c#L679 > > After applying your patches, I see a lot of cache-related artifacts on > the screen when updating the fb. I guess we might need a dma-specific mmap helper to make this work correctly. Best regards Thomas > > Tomi >
* Thomas Zimmermann <tzimmermann@suse.de> [240226 09:10]: > Hi > > Am 26.02.24 um 10:01 schrieb Tomi Valkeinen: > > On 26/02/2024 10:26, Tomi Valkeinen wrote: > > > Hi Tony, > > > > > > On 25/02/2024 08:46, Tony Lindgren wrote: > > > > Here are two fixes for omapdrm console. > > > > > > How is it broken? I don't usually use the console (or fbdev) but > > > enabling it now, it seems to work fine for me, on DRA76 EVM with > > > HDMI output. > > Omapdrm implements drm_framebuffer_funcs.dirty withomap_framebuffer_dirty(). > AFAIK DRM semantics requires to run the dirty helper after writing to the > framebuffer's memory. Userspace does this via the DIRTYFB ioctl. [1] But (at > least) for correctness the console needs to do the same. > > [1] https://elixir.bootlin.com/linux/v6.7.6/source/drivers/gpu/drm/drm_ioctl.c#L679 Yes I noticed console not updating and bisected it down to the two commits listed. I did the bisect on a droid4 though with command mode LCD. I did not test with HDMI, will give that a try too. > > After applying your patches, I see a lot of cache-related artifacts on > > the screen when updating the fb. > > I guess we might need a dma-specific mmap helper to make this work > correctly. I can easily test this if you have some suggested patch to try. Hmm so I wonder if we now have double updates happening on HDMI? Regards, Tony
* Tony Lindgren <tony@atomide.com> [240226 13:26]: > * Thomas Zimmermann <tzimmermann@suse.de> [240226 09:10]: > > Am 26.02.24 um 10:01 schrieb Tomi Valkeinen: > > > On 26/02/2024 10:26, Tomi Valkeinen wrote: > > > > How is it broken? I don't usually use the console (or fbdev) but > > > > enabling it now, it seems to work fine for me, on DRA76 EVM with > > > > HDMI output. > > > > Omapdrm implements drm_framebuffer_funcs.dirty withomap_framebuffer_dirty(). > > AFAIK DRM semantics requires to run the dirty helper after writing to the > > framebuffer's memory. Userspace does this via the DIRTYFB ioctl. [1] But (at > > least) for correctness the console needs to do the same. > > > > [1] https://elixir.bootlin.com/linux/v6.7.6/source/drivers/gpu/drm/drm_ioctl.c#L679 > > Yes I noticed console not updating and bisected it down to the two > commits listed. I did the bisect on a droid4 though with command mode > LCD. I did not test with HDMI, will give that a try too. I can reproduce the cache issue with Tomi's omapfb-tests [2] below: while true; do dd if=/dev/urandom of=/dev/fb0 ~/src/omapfb-tests/test sleep 1 done That produces short random data stripes on the test image. > > > After applying your patches, I see a lot of cache-related artifacts on > > > the screen when updating the fb. > > > > I guess we might need a dma-specific mmap helper to make this work > > correctly. Comparing the difference between drm_gem_mmap_obj() and fb_deferred_io_mmap(), the following test patch makes the cache issue go away for me. Not sure if this can be set based on some flag, or if we need a separate fb_deferred_io_wc_mmap() or something like that? Regards, Tony [2] https://github.com/tomba/omapfb-tests 8< -------------------- diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c --- a/drivers/video/fbdev/core/fb_defio.c +++ b/drivers/video/fbdev/core/fb_defio.c @@ -224,6 +224,7 @@ static const struct address_space_operations fb_deferred_io_aops = { int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) { vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); vma->vm_ops = &fb_deferred_io_vm_ops; vm_flags_set(vma, VM_DONTEXPAND | VM_DONTDUMP);
Hi Am 27.02.24 um 08:06 schrieb Tony Lindgren: > * Tony Lindgren <tony@atomide.com> [240226 13:26]: >> * Thomas Zimmermann <tzimmermann@suse.de> [240226 09:10]: >>> Am 26.02.24 um 10:01 schrieb Tomi Valkeinen: >>>> On 26/02/2024 10:26, Tomi Valkeinen wrote: >>>>> How is it broken? I don't usually use the console (or fbdev) but >>>>> enabling it now, it seems to work fine for me, on DRA76 EVM with >>>>> HDMI output. >>> Omapdrm implements drm_framebuffer_funcs.dirty withomap_framebuffer_dirty(). >>> AFAIK DRM semantics requires to run the dirty helper after writing to the >>> framebuffer's memory. Userspace does this via the DIRTYFB ioctl. [1] But (at >>> least) for correctness the console needs to do the same. >>> >>> [1] https://elixir.bootlin.com/linux/v6.7.6/source/drivers/gpu/drm/drm_ioctl.c#L679 >> Yes I noticed console not updating and bisected it down to the two >> commits listed. I did the bisect on a droid4 though with command mode >> LCD. I did not test with HDMI, will give that a try too. > I can reproduce the cache issue with Tomi's omapfb-tests [2] below: > > while true; > do dd if=/dev/urandom of=/dev/fb0 > ~/src/omapfb-tests/test > sleep 1 > done > > That produces short random data stripes on the test image. > >>>> After applying your patches, I see a lot of cache-related artifacts on >>>> the screen when updating the fb. >>> I guess we might need a dma-specific mmap helper to make this work >>> correctly. > Comparing the difference between drm_gem_mmap_obj() and > fb_deferred_io_mmap(), the following test patch makes the cache issue > go away for me. Not sure if this can be set based on some flag, or if > we need a separate fb_deferred_io_wc_mmap() or something like that? > > Regards, > > Tony > > [2] https://github.com/tomba/omapfb-tests > > 8< -------------------- > diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c > --- a/drivers/video/fbdev/core/fb_defio.c > +++ b/drivers/video/fbdev/core/fb_defio.c > @@ -224,6 +224,7 @@ static const struct address_space_operations fb_deferred_io_aops = { > int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) > { > vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); > + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); Great, that's exactly what I had in mind! My proposal is to add this mmap function directly to omapdrm. I'll later take care of integrating this into the overall framework. I have a few other ideas in mind that are related to this issue. Ok? Best regards Thomas > > vma->vm_ops = &fb_deferred_io_vm_ops; > vm_flags_set(vma, VM_DONTEXPAND | VM_DONTDUMP);
* Thomas Zimmermann <tzimmermann@suse.de> [240227 07:56]: > Am 27.02.24 um 08:06 schrieb Tony Lindgren: > > * Tony Lindgren <tony@atomide.com> [240226 13:26]: > > > * Thomas Zimmermann <tzimmermann@suse.de> [240226 09:10]: > > > > Am 26.02.24 um 10:01 schrieb Tomi Valkeinen: > > > > > On 26/02/2024 10:26, Tomi Valkeinen wrote: > > > > > > How is it broken? I don't usually use the console (or fbdev) but > > > > > > enabling it now, it seems to work fine for me, on DRA76 EVM with > > > > > > HDMI output. > > > > Omapdrm implements drm_framebuffer_funcs.dirty withomap_framebuffer_dirty(). > > > > AFAIK DRM semantics requires to run the dirty helper after writing to the > > > > framebuffer's memory. Userspace does this via the DIRTYFB ioctl. [1] But (at > > > > least) for correctness the console needs to do the same. > > > > > > > > [1] https://elixir.bootlin.com/linux/v6.7.6/source/drivers/gpu/drm/drm_ioctl.c#L679 > > > Yes I noticed console not updating and bisected it down to the two > > > commits listed. I did the bisect on a droid4 though with command mode > > > LCD. I did not test with HDMI, will give that a try too. > > I can reproduce the cache issue with Tomi's omapfb-tests [2] below: > > > > while true; > > do dd if=/dev/urandom of=/dev/fb0 > > ~/src/omapfb-tests/test > > sleep 1 > > done > > > > That produces short random data stripes on the test image. > > > > > > > After applying your patches, I see a lot of cache-related artifacts on > > > > > the screen when updating the fb. > > > > I guess we might need a dma-specific mmap helper to make this work > > > > correctly. > > Comparing the difference between drm_gem_mmap_obj() and > > fb_deferred_io_mmap(), the following test patch makes the cache issue > > go away for me. Not sure if this can be set based on some flag, or if > > we need a separate fb_deferred_io_wc_mmap() or something like that? > > > > [2] https://github.com/tomba/omapfb-tests > > > > 8< -------------------- > > diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c > > --- a/drivers/video/fbdev/core/fb_defio.c > > +++ b/drivers/video/fbdev/core/fb_defio.c > > @@ -224,6 +224,7 @@ static const struct address_space_operations fb_deferred_io_aops = { > > int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) > > { > > vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); > > + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); > > Great, that's exactly what I had in mind! OK :) > My proposal is to add this mmap function directly to omapdrm. I'll later > take care of integrating this into the overall framework. I have a few other > ideas in mind that are related to this issue. Ok? OK that sounds good to me, I'll post v3 set of patches. Regards, Tony
Hi Am 27.02.24 um 09:01 schrieb Tony Lindgren: > * Thomas Zimmermann <tzimmermann@suse.de> [240227 07:56]: >> Am 27.02.24 um 08:06 schrieb Tony Lindgren: >>> * Tony Lindgren <tony@atomide.com> [240226 13:26]: >>>> * Thomas Zimmermann <tzimmermann@suse.de> [240226 09:10]: >>>>> Am 26.02.24 um 10:01 schrieb Tomi Valkeinen: >>>>>> On 26/02/2024 10:26, Tomi Valkeinen wrote: >>>>>>> How is it broken? I don't usually use the console (or fbdev) but >>>>>>> enabling it now, it seems to work fine for me, on DRA76 EVM with >>>>>>> HDMI output. >>>>> Omapdrm implements drm_framebuffer_funcs.dirty withomap_framebuffer_dirty(). >>>>> AFAIK DRM semantics requires to run the dirty helper after writing to the >>>>> framebuffer's memory. Userspace does this via the DIRTYFB ioctl. [1] But (at >>>>> least) for correctness the console needs to do the same. >>>>> >>>>> [1] https://elixir.bootlin.com/linux/v6.7.6/source/drivers/gpu/drm/drm_ioctl.c#L679 >>>> Yes I noticed console not updating and bisected it down to the two >>>> commits listed. I did the bisect on a droid4 though with command mode >>>> LCD. I did not test with HDMI, will give that a try too. >>> I can reproduce the cache issue with Tomi's omapfb-tests [2] below: >>> >>> while true; >>> do dd if=/dev/urandom of=/dev/fb0 >>> ~/src/omapfb-tests/test >>> sleep 1 >>> done >>> >>> That produces short random data stripes on the test image. >>> >>>>>> After applying your patches, I see a lot of cache-related artifacts on >>>>>> the screen when updating the fb. >>>>> I guess we might need a dma-specific mmap helper to make this work >>>>> correctly. >>> Comparing the difference between drm_gem_mmap_obj() and >>> fb_deferred_io_mmap(), the following test patch makes the cache issue >>> go away for me. Not sure if this can be set based on some flag, or if >>> we need a separate fb_deferred_io_wc_mmap() or something like that? >>> >>> [2] https://github.com/tomba/omapfb-tests >>> >>> 8< -------------------- >>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c >>> --- a/drivers/video/fbdev/core/fb_defio.c >>> +++ b/drivers/video/fbdev/core/fb_defio.c >>> @@ -224,6 +224,7 @@ static const struct address_space_operations fb_deferred_io_aops = { >>> int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) >>> { >>> vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); >>> + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); >> Great, that's exactly what I had in mind! > OK :) > >> My proposal is to add this mmap function directly to omapdrm. I'll later >> take care of integrating this into the overall framework. I have a few other >> ideas in mind that are related to this issue. Ok? > OK that sounds good to me, I'll post v3 set of patches. I just realized the fb_deferred_io_mmap() is already exported. So please use it instead of duplicating the code in omapdrm. [1] https://elixir.bootlin.com/linux/v6.7/source/drivers/video/fbdev/core/fb_defio.c#L237 I also noticed that omapdrm does not yet select the correct Kconfig symbols. That can be fixed by 1) creating Kconfig FB_DMAMEM_HELPERS_DEFERRED that are similar to their SYSMEM equivalent at [2]. The tokens should look like this configFB_DMAMEM_HELPERS_DEFERRED <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS_DEFERRED> bool depends onFB_CORE <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_CORE> selectFB_DEFERRED_IO <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_DEFERRED_IO> selectFB_DMAMEM_HELPERS <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS> 2) and selecting it instead of FB_DMAMEM_HELPERS under omapdrm's Kconfig symbol. [2] https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/Kconfig#L147 Best regards Thomas > > Regards, > > Tony >
* Thomas Zimmermann <tzimmermann@suse.de> [240227 09:16]: > I just realized the fb_deferred_io_mmap() is already exported. So please use > it instead of duplicating the code in omapdrm. > > [1] https://elixir.bootlin.com/linux/v6.7/source/drivers/video/fbdev/core/fb_defio.c#L237 Yeah I have now: static int omap_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) { vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); return fb_deferred_io_mmap(info, vma); } > I also noticed that omapdrm does not yet select the correct Kconfig symbols. > That can be fixed by > > 1) creating Kconfig FB_DMAMEM_HELPERS_DEFERRED that are similar to their > SYSMEM equivalent at [2]. The tokens should look like this > > configFB_DMAMEM_HELPERS_DEFERRED <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS_DEFERRED> > bool > depends onFB_CORE <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_CORE> > selectFB_DEFERRED_IO <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_DEFERRED_IO> > selectFB_DMAMEM_HELPERS <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS> OK > 2) and selecting it instead of FB_DMAMEM_HELPERS under omapdrm's Kconfig > symbol. OK Regards, Tony > [2] https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/Kconfig#L147
* Tony Lindgren <tony@atomide.com> [240227 11:47]: > * Thomas Zimmermann <tzimmermann@suse.de> [240227 09:16]: > > I just realized the fb_deferred_io_mmap() is already exported. So please use > > it instead of duplicating the code in omapdrm. > > > > [1] https://elixir.bootlin.com/linux/v6.7/source/drivers/video/fbdev/core/fb_defio.c#L237 > > Yeah I have now: > > static int omap_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) > { > vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); > > return fb_deferred_io_mmap(info, vma); > } > > > I also noticed that omapdrm does not yet select the correct Kconfig symbols. > > That can be fixed by > > > > 1) creating Kconfig FB_DMAMEM_HELPERS_DEFERRED that are similar to their > > SYSMEM equivalent at [2]. The tokens should look like this > > > > configFB_DMAMEM_HELPERS_DEFERRED <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS_DEFERRED> > > bool > > depends onFB_CORE <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_CORE> > > selectFB_DEFERRED_IO <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_DEFERRED_IO> > > selectFB_DMAMEM_HELPERS <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS> > > OK > > > 2) and selecting it instead of FB_DMAMEM_HELPERS under omapdrm's Kconfig > > symbol. > > OK So here's what I have now, does that look OK? Regards, Tony 8< ------------------------- diff --git a/drivers/gpu/drm/omapdrm/Kconfig b/drivers/gpu/drm/omapdrm/Kconfig --- a/drivers/gpu/drm/omapdrm/Kconfig +++ b/drivers/gpu/drm/omapdrm/Kconfig @@ -4,7 +4,7 @@ config DRM_OMAP depends on DRM && OF depends on ARCH_OMAP2PLUS select DRM_KMS_HELPER - select FB_DMAMEM_HELPERS if DRM_FBDEV_EMULATION + select FB_DMAMEM_HELPERS_DEFERRED if DRM_FBDEV_EMULATION select VIDEOMODE_HELPERS select HDMI default n diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c @@ -51,6 +51,10 @@ static void pan_worker(struct work_struct *work) omap_gem_roll(bo, fbi->var.yoffset * npages); } +FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(omap_fbdev, + drm_fb_helper_damage_range, + drm_fb_helper_damage_area) + static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi) { @@ -78,11 +82,9 @@ static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, static int omap_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) { - struct drm_fb_helper *helper = info->par; - struct drm_framebuffer *fb = helper->fb; - struct drm_gem_object *bo = drm_gem_fb_get_obj(fb, 0); + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); - return drm_gem_mmap_obj(bo, omap_gem_mmap_size(bo), vma); + return fb_deferred_io_mmap(info, vma); } static void omap_fbdev_fb_destroy(struct fb_info *info) @@ -94,6 +96,7 @@ static void omap_fbdev_fb_destroy(struct fb_info *info) DBG(); + fb_deferred_io_cleanup(info); drm_fb_helper_fini(helper); omap_gem_unpin(bo); @@ -104,15 +107,19 @@ static void omap_fbdev_fb_destroy(struct fb_info *info) kfree(fbdev); } +/* + * For now, we cannot use FB_DEFAULT_DEFERRED_OPS and fb_deferred_io_mmap() + * because we use write-combine. + */ static const struct fb_ops omap_fb_ops = { .owner = THIS_MODULE, - __FB_DEFAULT_DMAMEM_OPS_RDWR, + __FB_DEFAULT_DEFERRED_OPS_RDWR(omap_fbdev), .fb_check_var = drm_fb_helper_check_var, .fb_set_par = drm_fb_helper_set_par, .fb_setcmap = drm_fb_helper_setcmap, .fb_blank = drm_fb_helper_blank, .fb_pan_display = omap_fbdev_pan_display, - __FB_DEFAULT_DMAMEM_OPS_DRAW, + __FB_DEFAULT_DEFERRED_OPS_DRAW(omap_fbdev), .fb_ioctl = drm_fb_helper_ioctl, .fb_mmap = omap_fbdev_fb_mmap, .fb_destroy = omap_fbdev_fb_destroy, @@ -213,6 +220,15 @@ static int omap_fbdev_create(struct drm_fb_helper *helper, fbi->fix.smem_start = dma_addr; fbi->fix.smem_len = bo->size; + /* deferred I/O */ + helper->fbdefio.delay = HZ / 20; + helper->fbdefio.deferred_io = drm_fb_helper_deferred_io; + + fbi->fbdefio = &helper->fbdefio; + ret = fb_deferred_io_init(fbi); + if (ret) + goto fail; + /* if we have DMM, then we can use it for scrolling by just * shuffling pages around in DMM rather than doing sw blit. */ diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig --- a/drivers/video/fbdev/core/Kconfig +++ b/drivers/video/fbdev/core/Kconfig @@ -144,6 +144,12 @@ config FB_DMAMEM_HELPERS select FB_SYS_IMAGEBLIT select FB_SYSMEM_FOPS +config FB_DMAMEM_HELPERS_DEFERRED + bool + depends on FB_CORE + select FB_DEFERRED_IO + select FB_DMAMEM_HELPERS + config FB_IOMEM_FOPS tristate depends on FB_CORE diff --git a/include/linux/fb.h b/include/linux/fb.h --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -686,6 +686,10 @@ extern int fb_deferred_io_fsync(struct file *file, loff_t start, __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, sys) \ __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, sys) +#define FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(__prefix, __damage_range, __damage_area) \ + __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, sys) \ + __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, sys) + /* * Initializes struct fb_ops for deferred I/O. */
Hi Am 27.02.24 um 11:19 schrieb Tony Lindgren: > * Tony Lindgren <tony@atomide.com> [240227 11:47]: >> * Thomas Zimmermann <tzimmermann@suse.de> [240227 09:16]: >>> I just realized the fb_deferred_io_mmap() is already exported. So please use >>> it instead of duplicating the code in omapdrm. >>> >>> [1] https://elixir.bootlin.com/linux/v6.7/source/drivers/video/fbdev/core/fb_defio.c#L237 >> Yeah I have now: >> >> static int omap_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) >> { >> vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); >> >> return fb_deferred_io_mmap(info, vma); >> } >> >>> I also noticed that omapdrm does not yet select the correct Kconfig symbols. >>> That can be fixed by >>> >>> 1) creating Kconfig FB_DMAMEM_HELPERS_DEFERRED that are similar to their >>> SYSMEM equivalent at [2]. The tokens should look like this >>> >>> configFB_DMAMEM_HELPERS_DEFERRED <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS_DEFERRED> >>> bool >>> depends onFB_CORE <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_CORE> >>> selectFB_DEFERRED_IO <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_DEFERRED_IO> >>> selectFB_DMAMEM_HELPERS <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS> >> OK >> >>> 2) and selecting it instead of FB_DMAMEM_HELPERS under omapdrm's Kconfig >>> symbol. >> OK > So here's what I have now, does that look OK? Looks good. You can add Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> to the final patch. Best regards Thomas > > Regards, > > Tony > > 8< ------------------------- > diff --git a/drivers/gpu/drm/omapdrm/Kconfig b/drivers/gpu/drm/omapdrm/Kconfig > --- a/drivers/gpu/drm/omapdrm/Kconfig > +++ b/drivers/gpu/drm/omapdrm/Kconfig > @@ -4,7 +4,7 @@ config DRM_OMAP > depends on DRM && OF > depends on ARCH_OMAP2PLUS > select DRM_KMS_HELPER > - select FB_DMAMEM_HELPERS if DRM_FBDEV_EMULATION > + select FB_DMAMEM_HELPERS_DEFERRED if DRM_FBDEV_EMULATION > select VIDEOMODE_HELPERS > select HDMI > default n > diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c > --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c > +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c > @@ -51,6 +51,10 @@ static void pan_worker(struct work_struct *work) > omap_gem_roll(bo, fbi->var.yoffset * npages); > } > > +FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(omap_fbdev, > + drm_fb_helper_damage_range, > + drm_fb_helper_damage_area) > + > static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, > struct fb_info *fbi) > { > @@ -78,11 +82,9 @@ static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, > > static int omap_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) > { > - struct drm_fb_helper *helper = info->par; > - struct drm_framebuffer *fb = helper->fb; > - struct drm_gem_object *bo = drm_gem_fb_get_obj(fb, 0); > + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); > > - return drm_gem_mmap_obj(bo, omap_gem_mmap_size(bo), vma); > + return fb_deferred_io_mmap(info, vma); > } > > static void omap_fbdev_fb_destroy(struct fb_info *info) > @@ -94,6 +96,7 @@ static void omap_fbdev_fb_destroy(struct fb_info *info) > > DBG(); > > + fb_deferred_io_cleanup(info); > drm_fb_helper_fini(helper); > > omap_gem_unpin(bo); > @@ -104,15 +107,19 @@ static void omap_fbdev_fb_destroy(struct fb_info *info) > kfree(fbdev); > } > > +/* > + * For now, we cannot use FB_DEFAULT_DEFERRED_OPS and fb_deferred_io_mmap() > + * because we use write-combine. > + */ > static const struct fb_ops omap_fb_ops = { > .owner = THIS_MODULE, > - __FB_DEFAULT_DMAMEM_OPS_RDWR, > + __FB_DEFAULT_DEFERRED_OPS_RDWR(omap_fbdev), > .fb_check_var = drm_fb_helper_check_var, > .fb_set_par = drm_fb_helper_set_par, > .fb_setcmap = drm_fb_helper_setcmap, > .fb_blank = drm_fb_helper_blank, > .fb_pan_display = omap_fbdev_pan_display, > - __FB_DEFAULT_DMAMEM_OPS_DRAW, > + __FB_DEFAULT_DEFERRED_OPS_DRAW(omap_fbdev), > .fb_ioctl = drm_fb_helper_ioctl, > .fb_mmap = omap_fbdev_fb_mmap, > .fb_destroy = omap_fbdev_fb_destroy, > @@ -213,6 +220,15 @@ static int omap_fbdev_create(struct drm_fb_helper *helper, > fbi->fix.smem_start = dma_addr; > fbi->fix.smem_len = bo->size; > > + /* deferred I/O */ > + helper->fbdefio.delay = HZ / 20; > + helper->fbdefio.deferred_io = drm_fb_helper_deferred_io; > + > + fbi->fbdefio = &helper->fbdefio; > + ret = fb_deferred_io_init(fbi); > + if (ret) > + goto fail; > + > /* if we have DMM, then we can use it for scrolling by just > * shuffling pages around in DMM rather than doing sw blit. > */ > diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig > --- a/drivers/video/fbdev/core/Kconfig > +++ b/drivers/video/fbdev/core/Kconfig > @@ -144,6 +144,12 @@ config FB_DMAMEM_HELPERS > select FB_SYS_IMAGEBLIT > select FB_SYSMEM_FOPS > > +config FB_DMAMEM_HELPERS_DEFERRED > + bool > + depends on FB_CORE > + select FB_DEFERRED_IO > + select FB_DMAMEM_HELPERS > + > config FB_IOMEM_FOPS > tristate > depends on FB_CORE > diff --git a/include/linux/fb.h b/include/linux/fb.h > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -686,6 +686,10 @@ extern int fb_deferred_io_fsync(struct file *file, loff_t start, > __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, sys) \ > __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, sys) > > +#define FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(__prefix, __damage_range, __damage_area) \ > + __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, sys) \ > + __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, sys) > + > /* > * Initializes struct fb_ops for deferred I/O. > */