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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 27.02.24 um 08:06 schrieb Tony Lindgren:
* Tony Lindgren <tony@xxxxxxxxxxx> [240226 13:26]:
* Thomas Zimmermann <tzimmermann@xxxxxxx> [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)





[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux