Re: [PATCH v2 21/43] drm/fbdev-dma: Implement damage handling and deferred I/O

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

 



Hi

Am 16.04.24 um 14:18 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> writes:

Add support for damage handling and deferred I/O to fbdev-dma. This
enables fbdev-dma to support all DMA-memory-based DRM drivers, even
such with a dirty callback in their framebuffers.

The patch adds the code for deferred I/O and also sets a dedicated
helper for struct fb_ops.fb_mmap that support coherent mappings.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  drivers/gpu/drm/drm_fbdev_dma.c | 65 ++++++++++++++++++++++++++-------
  1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index 6c9427bb4053b..8ffd072368bca 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -4,6 +4,7 @@
#include <drm/drm_crtc_helper.h>
  #include <drm/drm_drv.h>
+#include <drm/drm_fb_dma_helper.h>
  #include <drm/drm_fb_helper.h>
  #include <drm/drm_framebuffer.h>
  #include <drm/drm_gem_dma_helper.h>
@@ -35,6 +36,22 @@ static int drm_fbdev_dma_fb_release(struct fb_info *info, int user)
  	return 0;
  }
+FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(drm_fbdev_dma,
+				   drm_fb_helper_damage_range,
+				   drm_fb_helper_damage_area);
+
Shouldn't this be FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS() instead ?

I know that right now the macros are the same but I believe that it was
added it for a reason ?

Oh, thanks for noticing! I asked for that macro specifically for this reason. It went through the omap tree and hadn't arrived in drm-misc-next when I first made these patches. I'll update the patch accordingly.


+static int drm_fbdev_dma_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_framebuffer *fb = fb_helper->fb;
+	struct drm_gem_dma_object *dma = drm_fb_dma_get_gem_obj(fb, 0);
+
+	if (!dma->map_noncoherent)
+		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
I noticed that some drivers do:

                  vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));

I see that vm_get_page_prot() is a per-architecture function, but I don't
know about the implications of getting the pgprot_t from the vma->vm_flags
set or just using the current vma->vm_page_prot value...

That's in interesting observation. The code in the patch adds a WC flag to the existing vm_page_prot. The code in your example first creates a new vm_page_prot from the vm_flags field. Fbdev drivers generally use the former approach. So where does the original vm_page_prot value come from? (I think that's also the question behind your comment.)

I've looked through the kernel's mmap code from the syscall [1] to the place where it invokes the mmap callback. [2] Shortly before doing so, mmap_region() set's vm_page_prot from vm_flags like in your example. [3] I would assume there's no reason for drivers to call vm_get_page_prot() by themselves. DRM drivers specially seem to have the habit of doing so.

Best regards
Thomas

[1] https://elixir.bootlin.com/linux/v6.8/source/arch/x86/kernel/sys_x86_64.c#L86
[2] https://elixir.bootlin.com/linux/v6.8/source/mm/mmap.c#L2829
[3] https://elixir.bootlin.com/linux/v6.8/source/mm/mmap.c#L2824



Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


--
--
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]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux