Re: [PATCH v2 0/2] Fixes for omapdrm console
Hi
Am 27.02.24 um 11:19 schrieb Tony Lindgren:
* Tony Lindgren [240227 11:47]:
* Thomas Zimmermann [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
bool
depends onFB_CORE
selectFB_DEFERRED_IO
selectFB_DMAMEM_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
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 = >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
Re: [PATCH v2 0/2] Fixes for omapdrm console
* Tony Lindgren [240227 11:47]:
> * Thomas Zimmermann [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
> >
> > bool
> > depends onFB_CORE
> >
> > selectFB_DEFERRED_IO
> >
> > selectFB_DMAMEM_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 = >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
Re: [PATCH v2 0/2] Fixes for omapdrm console
* Thomas Zimmermann [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
>
> bool
> depends onFB_CORE
>
> selectFB_DEFERRED_IO
>
> selectFB_DMAMEM_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
Re: [PATCH v2 0/2] Fixes for omapdrm console
Hi
Am 27.02.24 um 09:01 schrieb Tony Lindgren:
* Thomas Zimmermann [240227 07:56]:
Am 27.02.24 um 08:06 schrieb Tony Lindgren:
* Tony Lindgren [240226 13:26]:
* Thomas Zimmermann [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
bool
depends onFB_CORE
selectFB_DEFERRED_IO
selectFB_DMAMEM_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
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH v2 0/2] Fixes for omapdrm console
* Thomas Zimmermann [240227 07:56]:
> Am 27.02.24 um 08:06 schrieb Tony Lindgren:
> > * Tony Lindgren [240226 13:26]:
> > > * Thomas Zimmermann [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
Re: [PATCH v2 0/2] Fixes for omapdrm console
Hi
Am 27.02.24 um 08:06 schrieb Tony Lindgren:
* Tony Lindgren [240226 13:26]:
* Thomas Zimmermann [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 = _deferred_io_vm_ops;
vm_flags_set(vma, VM_DONTEXPAND | VM_DONTDUMP);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH v2 0/2] Fixes for omapdrm console
* Tony Lindgren [240226 13:26]:
> * Thomas Zimmermann [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 = _deferred_io_vm_ops;
vm_flags_set(vma, VM_DONTEXPAND | VM_DONTDUMP);
Re: [PATCH v2 0/2] Fixes for omapdrm console
* Thomas Zimmermann [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
Re: [PATCH v2 0/2] Fixes for omapdrm console
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 Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Re: [PATCH v2 0/2] Fixes for omapdrm console
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
Re: [PATCH v2 0/2] Fixes for omapdrm console
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(-)
Re: [PATCH v2 0/2] Fixes for omapdrm console
Hi, looks good. For the series: Reviewed-by: Thomas Zimmermann 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(-) -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
[PATCH v2 0/2] Fixes for omapdrm console
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(-) -- 2.43.1
