Re: [PATCH v2 0/2] Fixes for omapdrm console

2024-02-27 Thread Thomas Zimmermann

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 = &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
se

Re: [PATCH v2 0/2] Fixes for omapdrm console

2024-02-27 Thread 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?

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

Re: [PATCH v2 0/2] Fixes for omapdrm console

2024-02-27 Thread Tony Lindgren
* 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

2024-02-27 Thread Thomas Zimmermann

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

2024-02-27 Thread 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.

Regards,

Tony


Re: [PATCH v2 0/2] Fixes for omapdrm console

2024-02-26 Thread Thomas Zimmermann

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 = &fb_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

2024-02-26 Thread 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));
 
vma->vm_ops = &fb_deferred_io_vm_ops;
vm_flags_set(vma, VM_DONTEXPAND | VM_DONTDUMP);


Re: [PATCH v2 0/2] Fixes for omapdrm console

2024-02-26 Thread Tony Lindgren
* 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

2024-02-26 Thread Thomas Zimmermann

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

2024-02-26 Thread 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.


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

2024-02-26 Thread Tomi Valkeinen

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

2024-02-26 Thread Thomas Zimmermann

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

2024-02-24 Thread 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(-)

-- 
2.43.1