Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or less overhead. With this in place, add support for virtual screens (i.e., framebuffers that are larger than the display resolution). This also enables fbdev panning and page flipping.
After the discussion and bug fixing wrt to fbdev overallocation, I decided to add full support for this to simpledrm. Patches #1 to #5 change the format-helper functions accordingly. Destination buffers are now clipped by the caller and all functions support a similar feature set. This has some fallout in various drivers.
Patch #6 change fbdev emulation to support overallocation with shadow buffers, even if the hardware buffer would be too small.
Patch #7 and #8 update simpledrm to enable damage clipping and virtual screen sizes. Both feature go hand in hand, sort of. For shadow- buffered planes, the DRM framebuffer lives in system memory. So the maximum size of the virtual screen is somewhat arbitrary. We add two constants for resonable maximum width and height of 4096 each.
Patch #9 adds documentation and a TODO item.
Tested with simpledrm. I also ran the recently posted fbdev panning tests to make sure that the fbdev overallocation works correctly. [1]
[1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html
Thomas Zimmermann (9): drm/format-helper: Export drm_fb_clip_offset() drm/format-helper: Rework format-helper memcpy functions drm/format-helper: Add destination-buffer pitch to drm_fb_swab() drm/format-helper: Rework format-helper conversion functions drm/format-helper: Streamline blit-helper interface drm/fb-helper: Allocate shadow buffer of surface height drm/simpledrm: Enable FB_DAMAGE_CLIPS property drm/simpledrm: Support virtual screen sizes drm: Clarify semantics of struct drm_mode_config.{min,max}_{width,height}
Documentation/gpu/todo.rst | 15 ++ drivers/gpu/drm/drm_fb_helper.c | 2 +- drivers/gpu/drm/drm_format_helper.c | 236 ++++++++++---------- drivers/gpu/drm/drm_mipi_dbi.c | 6 +- drivers/gpu/drm/gud/gud_pipe.c | 14 +- drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 5 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 4 +- drivers/gpu/drm/tiny/cirrus.c | 24 +- drivers/gpu/drm/tiny/repaper.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 41 +++- drivers/gpu/drm/tiny/st7586.c | 2 +- include/drm/drm_format_helper.h | 58 ++--- include/drm/drm_gem_atomic_helper.h | 18 ++ include/drm/drm_mode_config.h | 13 ++ 14 files changed, 254 insertions(+), 186 deletions(-)
-- 2.33.0
Provide a function that computes the offset into a blit destination buffer. This will allow to move destination-buffer clipping into the format-helper callers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_format_helper.c | 10 ++++++++-- include/drm/drm_format_helper.h | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 69fde60e36b3..28e9d0d89270 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -17,12 +17,18 @@ #include <drm/drm_fourcc.h> #include <drm/drm_rect.h>
-static unsigned int clip_offset(struct drm_rect *clip, - unsigned int pitch, unsigned int cpp) +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp) { return clip->y1 * pitch + clip->x1 * cpp; }
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format, + const struct drm_rect *clip) +{ + return clip_offset(clip, pitch, format->cpp[0]); +} +EXPORT_SYMBOL(drm_fb_clip_offset); + /** * drm_fb_memcpy - Copy clip buffer * @dst: Destination buffer diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index e86925cf07b9..90b9bd9ecb83 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -6,9 +6,13 @@ #ifndef __LINUX_DRM_FORMAT_HELPER_H #define __LINUX_DRM_FORMAT_HELPER_H
+struct drm_format_info; struct drm_framebuffer; struct drm_rect;
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format, + const struct drm_rect *clip); + void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb, struct drm_rect *clip); void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
Hi Thomas,
On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
Provide a function that computes the offset into a blit destination buffer. This will allow to move destination-buffer clipping into the format-helper callers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_format_helper.c | 10 ++++++++-- include/drm/drm_format_helper.h | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 69fde60e36b3..28e9d0d89270 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -17,12 +17,18 @@ #include <drm/drm_fourcc.h> #include <drm/drm_rect.h>
-static unsigned int clip_offset(struct drm_rect *clip,
unsigned int pitch, unsigned int cpp)+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp) { return clip->y1 * pitch + clip->x1 * cpp; }
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
const struct drm_rect *clip)+{
- return clip_offset(clip, pitch, format->cpp[0]);
+} +EXPORT_SYMBOL(drm_fb_clip_offset);
Exported functions are expected to have kernel-doc documentation. Just copy more or less from the changelog I think.
Anywhere else (I looked in struct drm_framebuffer) we only need unsigned int for offsets and width/length - so I cannot see why we do an unsigned int => unsigned long conversion here.
Sam
/**
- drm_fb_memcpy - Copy clip buffer
- @dst: Destination buffer
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index e86925cf07b9..90b9bd9ecb83 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -6,9 +6,13 @@ #ifndef __LINUX_DRM_FORMAT_HELPER_H #define __LINUX_DRM_FORMAT_HELPER_H
+struct drm_format_info; struct drm_framebuffer; struct drm_rect;
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
const struct drm_rect *clip);void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb, struct drm_rect *clip); void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr, -- 2.33.0
Hi
Am 23.10.21 um 09:49 schrieb Sam Ravnborg:
Hi Thomas,
On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
Provide a function that computes the offset into a blit destination buffer. This will allow to move destination-buffer clipping into the format-helper callers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_format_helper.c | 10 ++++++++-- include/drm/drm_format_helper.h | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 69fde60e36b3..28e9d0d89270 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -17,12 +17,18 @@ #include <drm/drm_fourcc.h> #include <drm/drm_rect.h>
-static unsigned int clip_offset(struct drm_rect *clip,
unsigned int pitch, unsigned int cpp)+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp) { return clip->y1 * pitch + clip->x1 * cpp; }
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
const struct drm_rect *clip)+{
- return clip_offset(clip, pitch, format->cpp[0]);
+} +EXPORT_SYMBOL(drm_fb_clip_offset);
Exported functions are expected to have kernel-doc documentation. Just copy more or less from the changelog I think.
That's an oversight. Sorry.
Anywhere else (I looked in struct drm_framebuffer) we only need unsigned int for offsets and width/length - so I cannot see why we do an unsigned int => unsigned long conversion here.
On ancient platforms, int was 16 bit wide. So for values that are array indices or buffer indices, I naturally use long, which is 32-bit at least. Never mind, it's not relevant any longer. I'll convert this code to unsigned int.
Best regards Thomas
Sam
- /**
- drm_fb_memcpy - Copy clip buffer
- @dst: Destination buffer
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index e86925cf07b9..90b9bd9ecb83 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -6,9 +6,13 @@ #ifndef __LINUX_DRM_FORMAT_HELPER_H #define __LINUX_DRM_FORMAT_HELPER_H
+struct drm_format_info; struct drm_framebuffer; struct drm_rect;
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
const struct drm_rect *clip);- void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb, struct drm_rect *clip); void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
-- 2.33.0
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Provide a function that computes the offset into a blit destination buffer. This will allow to move destination-buffer clipping into the format-helper callers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_format_helper.c | 10 ++++++++-- include/drm/drm_format_helper.h | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 69fde60e36b3..28e9d0d89270 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -17,12 +17,18 @@ #include <drm/drm_fourcc.h> #include <drm/drm_rect.h>
-static unsigned int clip_offset(struct drm_rect *clip,
unsigned int pitch, unsigned int cpp)+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp) { return clip->y1 * pitch + clip->x1 * cpp; }
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
Like Sam I wonder about the unsigned long here.
Noralf.
const struct drm_rect *clip)+{
- return clip_offset(clip, pitch, format->cpp[0]);
+} +EXPORT_SYMBOL(drm_fb_clip_offset);
Move destination-buffer clipping from all format-helper memcpy function into callers. Support destination-buffer pitch. Only distinguish between system and I/O memory, but use same logic everywhere.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_format_helper.c | 35 ++++++++++++--------- drivers/gpu/drm/drm_mipi_dbi.c | 2 +- drivers/gpu/drm/gud/gud_pipe.c | 2 +- drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 5 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++- drivers/gpu/drm/tiny/cirrus.c | 14 +++++---- include/drm/drm_format_helper.h | 9 +++--- 7 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 28e9d0d89270..38c8055f6fa8 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -32,58 +32,62 @@ EXPORT_SYMBOL(drm_fb_clip_offset); /** * drm_fb_memcpy - Copy clip buffer * @dst: Destination buffer + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @vaddr: Source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy * * This function does not apply clipping on dst, i.e. the destination - * is a small buffer containing the clip rect only. + * is at the top-left corner. */ -void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip) +void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip) { unsigned int cpp = fb->format->cpp[0]; size_t len = (clip->x2 - clip->x1) * cpp; unsigned int y, lines = clip->y2 - clip->y1;
+ if (!dst_pitch) + dst_pitch = len; + vaddr += clip_offset(clip, fb->pitches[0], cpp); for (y = 0; y < lines; y++) { memcpy(dst, vaddr, len); vaddr += fb->pitches[0]; - dst += len; + dst += dst_pitch; } } EXPORT_SYMBOL(drm_fb_memcpy);
/** - * drm_fb_memcpy_dstclip - Copy clip buffer + * drm_fb_memcpy_toio - Copy clip buffer * @dst: Destination buffer (iomem) * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @vaddr: Source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy * - * This function applies clipping on dst, i.e. the destination is a - * full (iomem) framebuffer but only the clip rect content is copied over. + * This function does not apply clipping on dst, i.e. the destination + * is at the top-left corner. */ -void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, - void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip) +void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip) { unsigned int cpp = fb->format->cpp[0]; - unsigned int offset = clip_offset(clip, dst_pitch, cpp); size_t len = (clip->x2 - clip->x1) * cpp; unsigned int y, lines = clip->y2 - clip->y1;
- vaddr += offset; - dst += offset; + if (!dst_pitch) + dst_pitch = len; + + vaddr += clip_offset(clip, fb->pitches[0], cpp); for (y = 0; y < lines; y++) { memcpy_toio(dst, vaddr, len); vaddr += fb->pitches[0]; dst += dst_pitch; } } -EXPORT_SYMBOL(drm_fb_memcpy_dstclip); +EXPORT_SYMBOL(drm_fb_memcpy_toio);
/** * drm_fb_swab - Swap bytes into clip buffer @@ -472,7 +476,8 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch, dst_format = DRM_FORMAT_XRGB8888;
if (dst_format == fb_format) { - drm_fb_memcpy_dstclip(dst, dst_pitch, vmap, fb, clip); + dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]); + drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip); return 0;
} else if (dst_format == DRM_FORMAT_RGB565) { diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c index 71b646c4131f..c09df8b06c7a 100644 --- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -213,7 +213,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb, if (swap) drm_fb_swab(dst, src, fb, clip, !gem->import_attach); else - drm_fb_memcpy(dst, src, fb, clip); + drm_fb_memcpy(dst, 0, src, fb, clip); break; case DRM_FORMAT_XRGB8888: drm_fb_xrgb8888_to_rgb565(dst, src, fb, clip, swap); diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c index daf75c178c2b..a92112ffd37a 100644 --- a/drivers/gpu/drm/gud/gud_pipe.c +++ b/drivers/gpu/drm/gud/gud_pipe.c @@ -206,7 +206,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb, /* can compress directly from the framebuffer */ buf = vaddr + rect->y1 * pitch; } else { - drm_fb_memcpy(buf, vaddr, fb, rect); + drm_fb_memcpy(buf, 0, vaddr, fb, rect); }
memset(req, 0, sizeof(*req)); diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c index 8c97a20dfe23..93f51e70a951 100644 --- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c +++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c @@ -23,13 +23,16 @@ static int hyperv_blit_to_vram_rect(struct drm_framebuffer *fb, struct drm_rect *rect) { struct hyperv_drm_device *hv = to_hv(fb->dev); + void __iomem *dst = hv->vram; void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */ int idx;
if (!drm_dev_enter(&hv->dev, &idx)) return -ENODEV;
- drm_fb_memcpy_dstclip(hv->vram, fb->pitches[0], vmap, fb, rect); + dst += drm_fb_clip_offset(fb->pitches[0], fb->format, rect); + drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect); + drm_dev_exit(idx);
return 0; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index fd98e8bbc550..b983541a4c53 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -847,9 +847,11 @@ static void mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb, struct drm_rect *clip, const struct dma_buf_map *map) { + void __iomem *dst = mdev->vram; void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
- drm_fb_memcpy_dstclip(mdev->vram, fb->pitches[0], vmap, fb, clip); + dst += drm_fb_clip_offset(fb->pitches[0], fb->format, clip); + drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, clip);
/* Always scanout image at VRAM offset 0 */ mgag200_set_startadd(mdev, (u32)0); diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index 4611ec408506..5344e506e8a9 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -317,28 +317,30 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, const struct dma_buf_ struct drm_rect *rect) { struct cirrus_device *cirrus = to_cirrus(fb->dev); + void __iomem *dst = cirrus->vram; void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */ int idx;
if (!drm_dev_enter(&cirrus->dev, &idx)) return -ENODEV;
- if (cirrus->cpp == fb->format->cpp[0]) - drm_fb_memcpy_dstclip(cirrus->vram, fb->pitches[0], - vmap, fb, rect); + if (cirrus->cpp == fb->format->cpp[0]) { + dst += drm_fb_clip_offset(fb->pitches[0], fb->format, rect); + drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
- else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) + } else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) { drm_fb_xrgb8888_to_rgb565_dstclip(cirrus->vram, cirrus->pitch, vmap, fb, rect, false);
- else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) + } else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) { drm_fb_xrgb8888_to_rgb888_dstclip(cirrus->vram, cirrus->pitch, vmap, fb, rect);
- else + } else { WARN_ON_ONCE("cpp mismatch"); + }
drm_dev_exit(idx);
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index 90b9bd9ecb83..8d72f6fd27e9 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -13,11 +13,10 @@ struct drm_rect; unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format, const struct drm_rect *clip);
-void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip); -void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr, - struct drm_framebuffer *fb, - struct drm_rect *clip); +void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip); +void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip); void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb, struct drm_rect *clip, bool cached); void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Move destination-buffer clipping from all format-helper memcpy function into callers. Support destination-buffer pitch. Only distinguish between system and I/O memory, but use same logic everywhere.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Tested-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Noralf Trønnes noralf@tronnes.org
Add destination-buffer pitch as argument to drm_fb_swab(). Done for consistency with the rest of the interface.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_format_helper.c | 19 +++++++++++++++---- drivers/gpu/drm/drm_mipi_dbi.c | 2 +- drivers/gpu/drm/gud/gud_pipe.c | 2 +- include/drm/drm_format_helper.h | 5 +++-- 4 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 38c8055f6fa8..79869ed553d9 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio); /** * drm_fb_swab - Swap bytes into clip buffer * @dst: Destination buffer + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @src: Source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy @@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio); * This function does not apply clipping on dst, i.e. the destination * is a small buffer containing the clip rect only. */ -void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb, - struct drm_rect *clip, bool cached) +void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src, + const struct drm_framebuffer *fb, const struct drm_rect *clip, + bool cached) { u8 cpp = fb->format->cpp[0]; size_t len = drm_rect_width(clip) * cpp; - u16 *src16, *dst16 = dst; - u32 *src32, *dst32 = dst; + const u16 *src16; + const u32 *src32; + u16 *dst16; + u32 *dst32; unsigned int x, y; void *buf = NULL;
if (WARN_ON_ONCE(cpp != 2 && cpp != 4)) return;
+ if (!dst_pitch) + dst_pitch = len; + if (!cached) buf = kmalloc(len, GFP_KERNEL);
@@ -131,6 +138,9 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb, src32 = src; }
+ dst16 = dst; + dst32 = dst; + for (x = clip->x1; x < clip->x2; x++) { if (cpp == 4) *dst32++ = swab32(*src32++); @@ -139,6 +149,7 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb, }
src += fb->pitches[0]; + dst += dst_pitch; }
kfree(buf); diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c index c09df8b06c7a..7ce89917d761 100644 --- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -211,7 +211,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb, switch (fb->format->format) { case DRM_FORMAT_RGB565: if (swap) - drm_fb_swab(dst, src, fb, clip, !gem->import_attach); + drm_fb_swab(dst, 0, src, fb, clip, !gem->import_attach); else drm_fb_memcpy(dst, 0, src, fb, clip); break; diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c index a92112ffd37a..e0b117b2559f 100644 --- a/drivers/gpu/drm/gud/gud_pipe.c +++ b/drivers/gpu/drm/gud/gud_pipe.c @@ -201,7 +201,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb, len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect); } } else if (gud_is_big_endian() && format->cpp[0] > 1) { - drm_fb_swab(buf, vaddr, fb, rect, !import_attach); + drm_fb_swab(buf, 0, vaddr, fb, rect, !import_attach); } else if (compression && !import_attach && pitch == fb->pitches[0]) { /* can compress directly from the framebuffer */ buf = vaddr + rect->y1 * pitch; diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index 8d72f6fd27e9..c4c3c845e119 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -17,8 +17,9 @@ void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr, const struct drm_framebuffer *fb, const struct drm_rect *clip); void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr, const struct drm_framebuffer *fb, const struct drm_rect *clip); -void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb, - struct drm_rect *clip, bool cached); +void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src, + const struct drm_framebuffer *fb, const struct drm_rect *clip, + bool cached); void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb, struct drm_rect *clip); void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Add destination-buffer pitch as argument to drm_fb_swab(). Done for consistency with the rest of the interface.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_format_helper.c | 19 +++++++++++++++---- drivers/gpu/drm/drm_mipi_dbi.c | 2 +- drivers/gpu/drm/gud/gud_pipe.c | 2 +- include/drm/drm_format_helper.h | 5 +++-- 4 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 38c8055f6fa8..79869ed553d9 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio); /**
- drm_fb_swab - Swap bytes into clip buffer
- @dst: Destination buffer
- @dst_pitch: Number of bytes between two consecutive scanlines within dst
- @src: Source buffer
- @fb: DRM framebuffer
- @clip: Clip rectangle area to copy
@@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
- This function does not apply clipping on dst, i.e. the destination
You have changed this line on the other functions, maybe you just missed it here:
- is a small buffer containing the clip rect only.
*/ -void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
struct drm_rect *clip, bool cached)+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
const struct drm_framebuffer *fb, const struct drm_rect *clip,bool cached)
Tested-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Noralf Trønnes noralf@tronnes.org
Move destination-buffer clipping from all format-helper conversion functions into callers. Support destination-buffer pitch. Only distinguish between system and I/O memory, but use same logic everywhere.
Simply harmonize the interface and semantics of the existing code. Not all conversion helpers support all combinations of parameters. We have to add additional features when we need them.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_format_helper.c | 131 +++++++++++++++------------- drivers/gpu/drm/drm_mipi_dbi.c | 2 +- drivers/gpu/drm/gud/gud_pipe.c | 10 +-- drivers/gpu/drm/tiny/cirrus.c | 10 +-- drivers/gpu/drm/tiny/repaper.c | 2 +- drivers/gpu/drm/tiny/st7586.c | 2 +- include/drm/drm_format_helper.h | 30 +++---- 7 files changed, 97 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 79869ed553d9..260dc587c1d7 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -156,7 +156,7 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src, } EXPORT_SYMBOL(drm_fb_swab);
-static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int pixels) +static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, const __le32 *sbuf, unsigned int pixels) { unsigned int x; u32 pix; @@ -172,23 +172,24 @@ static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int /** * drm_fb_xrgb8888_to_rgb332 - Convert XRGB8888 to RGB332 clip buffer * @dst: RGB332 destination buffer + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @src: XRGB8888 source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy * * Drivers can use this function for RGB332 devices that don't natively support XRGB8888. - * - * This function does not apply clipping on dst, i.e. the destination is a small buffer - * containing the clip rect only. */ -void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb, - struct drm_rect *clip) +void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *src, + const struct drm_framebuffer *fb, const struct drm_rect *clip) { size_t width = drm_rect_width(clip); size_t src_len = width * sizeof(u32); unsigned int y; void *sbuf;
+ if (!dst_pitch) + dst_pitch = width; + /* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */ sbuf = kmalloc(src_len, GFP_KERNEL); if (!sbuf) @@ -199,14 +200,14 @@ void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb, memcpy(sbuf, src, src_len); drm_fb_xrgb8888_to_rgb332_line(dst, sbuf, width); src += fb->pitches[0]; - dst += width; + dst += dst_pitch; }
kfree(sbuf); } EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
-static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf, +static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, const u32 *sbuf, unsigned int pixels, bool swab) { @@ -227,6 +228,7 @@ static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf, /** * drm_fb_xrgb8888_to_rgb565 - Convert XRGB8888 to RGB565 clip buffer * @dst: RGB565 destination buffer + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @vaddr: XRGB8888 source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy @@ -234,13 +236,10 @@ static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf, * * Drivers can use this function for RGB565 devices that don't natively * support XRGB8888. - * - * This function does not apply clipping on dst, i.e. the destination - * is a small buffer containing the clip rect only. */ -void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr, - struct drm_framebuffer *fb, - struct drm_rect *clip, bool swab) +void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip, + bool swab) { size_t linepixels = clip->x2 - clip->x1; size_t src_len = linepixels * sizeof(u32); @@ -248,6 +247,9 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr, unsigned y, lines = clip->y2 - clip->y1; void *sbuf;
+ if (!dst_pitch) + dst_pitch = dst_len; + /* * The cma memory is write-combined so reads are uncached. * Speed up by fetching one line at a time. @@ -261,7 +263,7 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr, memcpy(sbuf, vaddr, src_len); drm_fb_xrgb8888_to_rgb565_line(dst, sbuf, linepixels, swab); vaddr += fb->pitches[0]; - dst += dst_len; + dst += dst_pitch; }
kfree(sbuf); @@ -269,9 +271,9 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr, EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
/** - * drm_fb_xrgb8888_to_rgb565_dstclip - Convert XRGB8888 to RGB565 clip buffer + * drm_fb_xrgb8888_to_rgb565_toio - Convert XRGB8888 to RGB565 clip buffer * @dst: RGB565 destination buffer (iomem) - * @dst_pitch: destination buffer pitch + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @vaddr: XRGB8888 source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy @@ -279,37 +281,36 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565); * * Drivers can use this function for RGB565 devices that don't natively * support XRGB8888. - * - * This function applies clipping on dst, i.e. the destination is a - * full (iomem) framebuffer but only the clip rect content is copied over. */ -void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch, - void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip, bool swab) +void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch, + const void *vaddr, const struct drm_framebuffer *fb, + const struct drm_rect *clip, bool swab) { size_t linepixels = clip->x2 - clip->x1; size_t dst_len = linepixels * sizeof(u16); unsigned y, lines = clip->y2 - clip->y1; void *dbuf;
+ if (!dst_pitch) + dst_pitch = dst_len; + dbuf = kmalloc(dst_len, GFP_KERNEL); if (!dbuf) return;
vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32)); - dst += clip_offset(clip, dst_pitch, sizeof(u16)); for (y = 0; y < lines; y++) { drm_fb_xrgb8888_to_rgb565_line(dbuf, vaddr, linepixels, swab); memcpy_toio(dst, dbuf, dst_len); vaddr += fb->pitches[0]; - dst += dst_len; + dst += dst_pitch; }
kfree(dbuf); } -EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_dstclip); +EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_toio);
-static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, u32 *sbuf, +static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, const u32 *sbuf, unsigned int pixels) { unsigned int x; @@ -324,24 +325,25 @@ static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, u32 *sbuf, /** * drm_fb_xrgb8888_to_rgb888 - Convert XRGB8888 to RGB888 clip buffer * @dst: RGB888 destination buffer + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @src: XRGB8888 source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy * * Drivers can use this function for RGB888 devices that don't natively * support XRGB8888. - * - * This function does not apply clipping on dst, i.e. the destination - * is a small buffer containing the clip rect only. */ -void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb, - struct drm_rect *clip) +void drm_fb_xrgb8888_to_rgb888(void *dst, unsigned int dst_pitch, const void *src, + const struct drm_framebuffer *fb, const struct drm_rect *clip) { size_t width = drm_rect_width(clip); size_t src_len = width * sizeof(u32); unsigned int y; void *sbuf;
+ if (!dst_pitch) + dst_pitch = width * 3; + /* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */ sbuf = kmalloc(src_len, GFP_KERNEL); if (!sbuf) @@ -352,7 +354,7 @@ void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb, memcpy(sbuf, src, src_len); drm_fb_xrgb8888_to_rgb888_line(dst, sbuf, width); src += fb->pitches[0]; - dst += width * 3; + dst += dst_pitch; }
kfree(sbuf); @@ -360,48 +362,48 @@ void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb, EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
/** - * drm_fb_xrgb8888_to_rgb888_dstclip - Convert XRGB8888 to RGB888 clip buffer + * drm_fb_xrgb8888_to_rgb888_toio - Convert XRGB8888 to RGB888 clip buffer * @dst: RGB565 destination buffer (iomem) - * @dst_pitch: destination buffer pitch + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @vaddr: XRGB8888 source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy * * Drivers can use this function for RGB888 devices that don't natively * support XRGB8888. - * - * This function applies clipping on dst, i.e. the destination is a - * full (iomem) framebuffer but only the clip rect content is copied over. */ -void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst, unsigned int dst_pitch, - void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip) +void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, + const void *vaddr, const struct drm_framebuffer *fb, + const struct drm_rect *clip) { size_t linepixels = clip->x2 - clip->x1; size_t dst_len = linepixels * 3; unsigned y, lines = clip->y2 - clip->y1; void *dbuf;
+ if (!dst_pitch) + dst_pitch = dst_len; + dbuf = kmalloc(dst_len, GFP_KERNEL); if (!dbuf) return;
vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32)); - dst += clip_offset(clip, dst_pitch, sizeof(u16)); for (y = 0; y < lines; y++) { drm_fb_xrgb8888_to_rgb888_line(dbuf, vaddr, linepixels); memcpy_toio(dst, dbuf, dst_len); vaddr += fb->pitches[0]; - dst += dst_len; + dst += dst_pitch; }
kfree(dbuf); } -EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip); +EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_toio);
/** * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale * @dst: 8-bit grayscale destination buffer + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @vaddr: XRGB8888 source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy @@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip); * * ITU BT.601 is used for the RGB -> luma (brightness) conversion. */ -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip) +void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip) { unsigned int len = (clip->x2 - clip->x1) * sizeof(u32); unsigned int x, y; void *buf; - u32 *src; + u8 *dst8; + u32 *src32;
if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888)) return; + + if (!dst_pitch) + dst_pitch = len; + /* * The cma memory is write-combined so reads are uncached. * Speed up by fetching one line at a time. @@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, if (!buf) return;
+ vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32)); for (y = clip->y1; y < clip->y2; y++) { - src = vaddr + (y * fb->pitches[0]); - src += clip->x1; - memcpy(buf, src, len); - src = buf; + dst8 = dst; + src32 = memcpy(buf, vaddr, len); for (x = clip->x1; x < clip->x2; x++) { - u8 r = (*src & 0x00ff0000) >> 16; - u8 g = (*src & 0x0000ff00) >> 8; - u8 b = *src & 0x000000ff; + u8 r = (*src32 & 0x00ff0000) >> 16; + u8 g = (*src32 & 0x0000ff00) >> 8; + u8 b = *src32 & 0x000000ff;
/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */ - *dst++ = (3 * r + 6 * g + b) / 10; - src++; + *dst8++ = (3 * r + 6 * g + b) / 10; + src32++; } + + vaddr += fb->pitches[0]; + dst += dst_pitch; }
kfree(buf); @@ -493,15 +502,15 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
} else if (dst_format == DRM_FORMAT_RGB565) { if (fb_format == DRM_FORMAT_XRGB8888) { - drm_fb_xrgb8888_to_rgb565_dstclip(dst, dst_pitch, - vmap, fb, clip, - false); + dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]); + drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip, + false); return 0; } } else if (dst_format == DRM_FORMAT_RGB888) { if (fb_format == DRM_FORMAT_XRGB8888) { - drm_fb_xrgb8888_to_rgb888_dstclip(dst, dst_pitch, - vmap, fb, clip); + dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]); + drm_fb_xrgb8888_to_rgb888_toio(dst, dst_pitch, vmap, fb, clip); return 0; } } diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c index 7ce89917d761..b75403f3251a 100644 --- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -216,7 +216,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb, drm_fb_memcpy(dst, 0, src, fb, clip); break; case DRM_FORMAT_XRGB8888: - drm_fb_xrgb8888_to_rgb565(dst, src, fb, clip, swap); + drm_fb_xrgb8888_to_rgb565(dst, 0, src, fb, clip, swap); break; default: drm_err_once(fb->dev, "Format is not supported: %p4cc\n", diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c index e0b117b2559f..a150a5a4b5d4 100644 --- a/drivers/gpu/drm/gud/gud_pipe.c +++ b/drivers/gpu/drm/gud/gud_pipe.c @@ -74,7 +74,7 @@ static size_t gud_xrgb8888_to_r124(u8 *dst, const struct drm_format_info *format if (!buf) return 0;
- drm_fb_xrgb8888_to_gray8(buf, src, fb, rect); + drm_fb_xrgb8888_to_gray8(buf, 0, src, fb, rect); pix8 = buf;
for (y = 0; y < height; y++) { @@ -190,13 +190,13 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb, goto end_cpu_access; } } else if (format->format == DRM_FORMAT_R8) { - drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, rect); + drm_fb_xrgb8888_to_gray8(buf, 0, vaddr, fb, rect); } else if (format->format == DRM_FORMAT_RGB332) { - drm_fb_xrgb8888_to_rgb332(buf, vaddr, fb, rect); + drm_fb_xrgb8888_to_rgb332(buf, 0, vaddr, fb, rect); } else if (format->format == DRM_FORMAT_RGB565) { - drm_fb_xrgb8888_to_rgb565(buf, vaddr, fb, rect, gud_is_big_endian()); + drm_fb_xrgb8888_to_rgb565(buf, 0, vaddr, fb, rect, gud_is_big_endian()); } else if (format->format == DRM_FORMAT_RGB888) { - drm_fb_xrgb8888_to_rgb888(buf, vaddr, fb, rect); + drm_fb_xrgb8888_to_rgb888(buf, 0, vaddr, fb, rect); } else { len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect); } diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index 5344e506e8a9..9327001d35dd 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -329,14 +329,12 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, const struct dma_buf_ drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) { - drm_fb_xrgb8888_to_rgb565_dstclip(cirrus->vram, - cirrus->pitch, - vmap, fb, rect, false); + dst += drm_fb_clip_offset(cirrus->pitch, fb->format, rect); + drm_fb_xrgb8888_to_rgb565_toio(dst, cirrus->pitch, vmap, fb, rect, false);
} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) { - drm_fb_xrgb8888_to_rgb888_dstclip(cirrus->vram, - cirrus->pitch, - vmap, fb, rect); + dst += drm_fb_clip_offset(cirrus->pitch, fb->format, rect); + drm_fb_xrgb8888_to_rgb888_toio(dst, cirrus->pitch, vmap, fb, rect);
} else { WARN_ON_ONCE("cpp mismatch"); diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c index 4d07b21a16e6..97a775c48cea 100644 --- a/drivers/gpu/drm/tiny/repaper.c +++ b/drivers/gpu/drm/tiny/repaper.c @@ -560,7 +560,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb) if (ret) goto out_free;
- drm_fb_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip); + drm_fb_xrgb8888_to_gray8(buf, 0, cma_obj->vaddr, fb, &clip);
drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c index ad0faa8723c2..51b9b9fb3ead 100644 --- a/drivers/gpu/drm/tiny/st7586.c +++ b/drivers/gpu/drm/tiny/st7586.c @@ -73,7 +73,7 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr, if (!buf) return;
- drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, clip); + drm_fb_xrgb8888_to_gray8(buf, 0, vaddr, fb, clip); src = buf;
for (y = clip->y1; y < clip->y2; y++) { diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index c4c3c845e119..8af9b8f35dd6 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -20,21 +20,21 @@ void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *v void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src, const struct drm_framebuffer *fb, const struct drm_rect *clip, bool cached); -void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip); -void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr, - struct drm_framebuffer *fb, - struct drm_rect *clip, bool swab); -void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch, - void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip, bool swab); -void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb, - struct drm_rect *clip); -void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst, unsigned int dst_pitch, - void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip); -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, - struct drm_rect *clip); +void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip); +void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip, + bool swab); +void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch, + const void *vaddr, const struct drm_framebuffer *fb, + const struct drm_rect *clip, bool swab); +void drm_fb_xrgb8888_to_rgb888(void *dst, unsigned int dst_pitch, const void *src, + const struct drm_framebuffer *fb, const struct drm_rect *clip); +void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, + const void *vaddr, const struct drm_framebuffer *fb, + const struct drm_rect *clip); +void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip);
int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_format, void *vmap,
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Move destination-buffer clipping from all format-helper conversion functions into callers. Support destination-buffer pitch. Only distinguish between system and I/O memory, but use same logic everywhere.
Simply harmonize the interface and semantics of the existing code. Not all conversion helpers support all combinations of parameters. We have to add additional features when we need them.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
/**
- drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
- @dst: 8-bit grayscale destination buffer
- @dst_pitch: Number of bytes between two consecutive scanlines within dst
- @vaddr: XRGB8888 source buffer
- @fb: DRM framebuffer
- @clip: Clip rectangle area to copy
@@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
- ITU BT.601 is used for the RGB -> luma (brightness) conversion.
*/ -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip)+void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
const struct drm_framebuffer *fb, const struct drm_rect *clip){ unsigned int len = (clip->x2 - clip->x1) * sizeof(u32); unsigned int x, y; void *buf;
- u32 *src;
u8 *dst8;
u32 *src32;
if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888)) return;
if (!dst_pitch)
len is source length (should really have been named src_len) which results in a kernel crash:
dst_pitch = len;
This works:
dst_pitch = drm_rect_width(clip);
With that fixed:
Tested-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Noralf Trønnes noralf@tronnes.org
- /*
- The cma memory is write-combined so reads are uncached.
- Speed up by fetching one line at a time.
@@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, if (!buf) return;
- vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32)); for (y = clip->y1; y < clip->y2; y++) {
src = vaddr + (y * fb->pitches[0]);src += clip->x1;memcpy(buf, src, len);src = buf;
dst8 = dst;src32 = memcpy(buf, vaddr, len);
u8 r = (*src & 0x00ff0000) >> 16;u8 g = (*src & 0x0000ff00) >> 8;u8 b = *src & 0x000000ff;
u8 r = (*src32 & 0x00ff0000) >> 16;u8 g = (*src32 & 0x0000ff00) >> 8;u8 b = *src32 & 0x000000ff; /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
*dst++ = (3 * r + 6 * g + b) / 10;src++;
*dst8++ = (3 * r + 6 * g + b) / 10;src32++;}
vaddr += fb->pitches[0];dst += dst_pitch;}
kfree(buf);
Hi
Am 24.10.21 um 13:32 schrieb Noralf Trønnes:
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Move destination-buffer clipping from all format-helper conversion functions into callers. Support destination-buffer pitch. Only distinguish between system and I/O memory, but use same logic everywhere.
Simply harmonize the interface and semantics of the existing code. Not all conversion helpers support all combinations of parameters. We have to add additional features when we need them.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
/**
- drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
- @dst: 8-bit grayscale destination buffer
- @dst_pitch: Number of bytes between two consecutive scanlines within dst
- @vaddr: XRGB8888 source buffer
- @fb: DRM framebuffer
- @clip: Clip rectangle area to copy
@@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
- ITU BT.601 is used for the RGB -> luma (brightness) conversion.
*/ -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip)+void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
const struct drm_framebuffer *fb, const struct drm_rect *clip)
- u32 *src;
u8 *dst8;
u32 *src32;
if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888)) return;
if (!dst_pitch)
len is source length (should really have been named src_len) which results in a kernel crash:
dst_pitch = len;This works:
dst_pitch = drm_rect_width(clip);
Fixed in the next revision. Thank you so much!
Best regards Thomas
With that fixed:
Tested-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Noralf Trønnes noralf@tronnes.org
- /*
- The cma memory is write-combined so reads are uncached.
- Speed up by fetching one line at a time.
@@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, if (!buf) return;
- vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32)); for (y = clip->y1; y < clip->y2; y++) {
src = vaddr + (y * fb->pitches[0]);src += clip->x1;memcpy(buf, src, len);src = buf;
dst8 = dst;src32 = memcpy(buf, vaddr, len);
u8 r = (*src & 0x00ff0000) >> 16;u8 g = (*src & 0x0000ff00) >> 8;u8 b = *src & 0x000000ff;
u8 r = (*src32 & 0x00ff0000) >> 16;u8 g = (*src32 & 0x0000ff00) >> 8;u8 b = *src32 & 0x000000ff; /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
*dst++ = (3 * r + 6 * g + b) / 10;src++;
*dst8++ = (3 * r + 6 * g + b) / 10;src32++;}
vaddr += fb->pitches[0];dst += dst_pitch;}
kfree(buf);
Move destination-buffer clipping from format-helper blit function into caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for consistency with the rest of the interface. Remove drm_fb_blit_dstclip(), which isn't required.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_format_helper.c | 51 ++++------------------------- drivers/gpu/drm/tiny/simpledrm.c | 14 +++++--- include/drm/drm_format_helper.h | 10 ++---- 3 files changed, 19 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 260dc587c1d7..5ca9abc65510 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -463,7 +463,7 @@ void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vad EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
/** - * drm_fb_blit_rect_dstclip - Copy parts of a framebuffer to display memory + * drm_fb_blit_toio - Copy parts of a framebuffer to display memory * @dst: The display memory to copy to * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @dst_format: FOURCC code of the display's color format @@ -475,17 +475,14 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8); * formats of the display and the framebuffer mismatch, the blit function * will attempt to convert between them. * - * Use drm_fb_blit_dstclip() to copy the full framebuffer. - * * Returns: * 0 on success, or * -EINVAL if the color-format conversion failed, or * a negative error code otherwise. */ -int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch, - uint32_t dst_format, void *vmap, - struct drm_framebuffer *fb, - struct drm_rect *clip) +int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_format, + const void *vmap, const struct drm_framebuffer *fb, + const struct drm_rect *clip) { uint32_t fb_format = fb->format->format;
@@ -496,20 +493,16 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch, dst_format = DRM_FORMAT_XRGB8888;
if (dst_format == fb_format) { - dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]); drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip); return 0;
} else if (dst_format == DRM_FORMAT_RGB565) { if (fb_format == DRM_FORMAT_XRGB8888) { - dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]); - drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip, - false); + drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip, false); return 0; } } else if (dst_format == DRM_FORMAT_RGB888) { if (fb_format == DRM_FORMAT_XRGB8888) { - dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]); drm_fb_xrgb8888_to_rgb888_toio(dst, dst_pitch, vmap, fb, clip); return 0; } @@ -517,36 +510,4 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
return -EINVAL; } -EXPORT_SYMBOL(drm_fb_blit_rect_dstclip); - -/** - * drm_fb_blit_dstclip - Copy framebuffer to display memory - * @dst: The display memory to copy to - * @dst_pitch: Number of bytes between two consecutive scanlines within dst - * @dst_format: FOURCC code of the display's color format - * @vmap: The framebuffer memory to copy from - * @fb: The framebuffer to copy from - * - * This function copies a full framebuffer to display memory. If the formats - * of the display and the framebuffer mismatch, the copy function will - * attempt to convert between them. - * - * See drm_fb_blit_rect_dstclip() for more information. - * - * Returns: - * 0 on success, or a negative error code otherwise. - */ -int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch, - uint32_t dst_format, void *vmap, - struct drm_framebuffer *fb) -{ - struct drm_rect fullscreen = { - .x1 = 0, - .x2 = fb->width, - .y1 = 0, - .y2 = fb->height, - }; - return drm_fb_blit_rect_dstclip(dst, dst_pitch, dst_format, vmap, fb, - &fullscreen); -} -EXPORT_SYMBOL(drm_fb_blit_dstclip); +EXPORT_SYMBOL(drm_fb_blit_toio); diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 481b48bde047..571f716ff427 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -641,6 +641,8 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = plane_state->fb; void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ struct drm_device *dev = &sdev->dev; + void __iomem *dst = sdev->screen_base; + struct drm_rect clip; int idx;
if (!fb) @@ -649,8 +651,11 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, if (!drm_dev_enter(dev, &idx)) return;
- drm_fb_blit_dstclip(sdev->screen_base, sdev->pitch, - sdev->format->format, vmap, fb); + drm_rect_init(&clip, 0, 0, fb->width, fb->height); + + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); + drm_dev_exit(idx); }
@@ -680,6 +685,7 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = &sdev->dev; + void __iomem *dst = sdev->screen_base; struct drm_rect clip; int idx;
@@ -692,8 +698,8 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, if (!drm_dev_enter(dev, &idx)) return;
- drm_fb_blit_rect_dstclip(sdev->screen_base, sdev->pitch, - sdev->format->format, vmap, fb, &clip); + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
drm_dev_exit(idx); } diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index 8af9b8f35dd6..c91b7c553878 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -36,12 +36,8 @@ void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr, const struct drm_framebuffer *fb, const struct drm_rect *clip);
-int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch, - uint32_t dst_format, void *vmap, - struct drm_framebuffer *fb, - struct drm_rect *rect); -int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch, - uint32_t dst_format, void *vmap, - struct drm_framebuffer *fb); +int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_format, + const void *vmap, const struct drm_framebuffer *fb, + const struct drm_rect *rect);
#endif /* __LINUX_DRM_FORMAT_HELPER_H */
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Move destination-buffer clipping from format-helper blit function into caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for consistency with the rest of the interface. Remove drm_fb_blit_dstclip(), which isn't required.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Reviewed-by: Noralf Trønnes noralf@tronnes.org
Allocating a shadow buffer of the height of the buffer object does not support fbdev overallocation. Use surface height instead.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 8e7a124d6c5a..9727a59d35fd 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2338,7 +2338,7 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, return PTR_ERR(fbi);
fbi->fbops = &drm_fbdev_fb_ops; - fbi->screen_size = fb->height * fb->pitches[0]; + fbi->screen_size = sizes->surface_height * fb->pitches[0]; fbi->fix.smem_len = fbi->screen_size;
drm_fb_helper_fill_info(fbi, fb_helper, sizes);
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Allocating a shadow buffer of the height of the buffer object does not support fbdev overallocation. Use surface height instead.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Reviewed-by: Noralf Trønnes noralf@tronnes.org
Enable the FB_DAMAGE_CLIPS property to reduce display-update overhead. Also fixes a warning in the kernel log.
simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
Fix the computation of the blit rectangle. This wasn't an issue so far, as simpledrm always blitted the full framebuffer. The code now supports damage clipping and virtual screen sizes.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 571f716ff427..e872121e9fb0 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base; - struct drm_rect clip; + struct drm_rect src_clip, dst_clip; int idx;
if (!fb) @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, if (!drm_dev_enter(dev, &idx)) return;
- drm_rect_init(&clip, 0, 0, fb->width, fb->height); + drm_rect_fp_to_int(&src_clip, &plane_state->src);
- dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); + dst_clip = plane_state->dst; + if (!drm_rect_intersect(&dst_clip, &src_clip)) + return; + + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
drm_dev_exit(idx); } @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base; - struct drm_rect clip; + struct drm_rect damage_clip, src_clip, dst_clip; int idx;
if (!fb) return;
- if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip)) + if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip)) + return; + + drm_rect_fp_to_int(&src_clip, &plane_state->src); + if (!drm_rect_intersect(&src_clip, &damage_clip)) + return; + + dst_clip = plane_state->dst; + if (!drm_rect_intersect(&dst_clip, &src_clip)) return;
if (!drm_dev_enter(dev, &idx)) return;
- dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
drm_dev_exit(idx); } @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) if (ret) return ret;
+ drm_plane_enable_fb_damage_clips(&pipe->plane); + drm_mode_config_reset(dev);
return 0;
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Enable the FB_DAMAGE_CLIPS property to reduce display-update overhead. Also fixes a warning in the kernel log.
simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
Fix the computation of the blit rectangle. This wasn't an issue so far, as simpledrm always blitted the full framebuffer. The code now supports damage clipping and virtual screen sizes.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 571f716ff427..e872121e9fb0 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base;
- struct drm_rect clip;
struct drm_rect src_clip, dst_clip; int idx;
if (!fb)
@@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, if (!drm_dev_enter(dev, &idx)) return;
- drm_rect_init(&clip, 0, 0, fb->width, fb->height);
- drm_rect_fp_to_int(&src_clip, &plane_state->src);
- dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
- drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
dst_clip = plane_state->dst;
if (!drm_rect_intersect(&dst_clip, &src_clip))
return;dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
drm_dev_exit(idx);
} @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base;
- struct drm_rect clip;
struct drm_rect damage_clip, src_clip, dst_clip; int idx;
if (!fb) return;
- if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
- if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
return;
I'm afraid I don't understand what's going on here, but isn't it possible to put this logic into drm_atomic_helper_damage_merged()?
Noralf.
drm_rect_fp_to_int(&src_clip, &plane_state->src);
if (!drm_rect_intersect(&src_clip, &damage_clip))
return;dst_clip = plane_state->dst;
if (!drm_rect_intersect(&dst_clip, &src_clip)) return;
if (!drm_dev_enter(dev, &idx)) return;
- dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
- drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
drm_dev_exit(idx);
} @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) if (ret) return ret;
drm_plane_enable_fb_damage_clips(&pipe->plane);
drm_mode_config_reset(dev);
return 0;
Hi
Am 24.10.21 um 17:20 schrieb Noralf Trønnes:
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
Enable the FB_DAMAGE_CLIPS property to reduce display-update overhead. Also fixes a warning in the kernel log.
simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
Fix the computation of the blit rectangle. This wasn't an issue so far, as simpledrm always blitted the full framebuffer. The code now supports damage clipping and virtual screen sizes.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 571f716ff427..e872121e9fb0 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base;
- struct drm_rect clip;
struct drm_rect src_clip, dst_clip; int idx;
if (!fb)
@@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, if (!drm_dev_enter(dev, &idx)) return;
- drm_rect_init(&clip, 0, 0, fb->width, fb->height);
- drm_rect_fp_to_int(&src_clip, &plane_state->src);
- dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
- drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
dst_clip = plane_state->dst;
if (!drm_rect_intersect(&dst_clip, &src_clip))
return;dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
drm_dev_exit(idx); }
@@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = &sdev->dev; void __iomem *dst = sdev->screen_base;
- struct drm_rect clip;
struct drm_rect damage_clip, src_clip, dst_clip; int idx;
if (!fb) return;
- if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
- if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
return;I'm afraid I don't understand what's going on here, but isn't it possible to put this logic into drm_atomic_helper_damage_merged()?
The code above gets the damage rectangle (i.e., the plane's area that needs to be updated). The code below get's the framebuffer area. If they don't overlap, return. (I think this can really only fail with the next patch in the series, which adds virtual screens.)
Noralf.
- drm_rect_fp_to_int(&src_clip, &plane_state->src);
- if (!drm_rect_intersect(&src_clip, &damage_clip))
return;- dst_clip = plane_state->dst;
- if (!drm_rect_intersect(&dst_clip, &src_clip)) return;
And here we check if the updated plane/framebuffer area is actually visible on screen; otherwise return. It could be in an area that is off-screen. (Again, this probably only fails with the virtual-screen patch.)
I don't think this is all generic enough to be within drm_atomic_helper_damage_merged(). But once we have multiple SHMEM-based drivers with virtual screens, we can move it into a helper for shadow-buffered planes. Your gud driver would be a candidate for this feature as well.
Best regards Thomas
if (!drm_dev_enter(dev, &idx)) return;
- dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
- drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
drm_dev_exit(idx); }
@@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) if (ret) return ret;
drm_plane_enable_fb_damage_clips(&pipe->plane);
drm_mode_config_reset(dev);
return 0;
Add constants for the maximum size of the shadow-plane surface size. Useful for shadow planes with virtual screen sizes. The current sizes are 4096 scanlines with 4096 pixels each. This seems reasonable for current hardware, but can be increased as necessary.
In simpledrm, set the maximum framebuffer size from the constants for shadow planes. Implements support for virtual screen sizes and page flipping on the fbdev console.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/tiny/simpledrm.c | 9 +++++++-- include/drm/drm_gem_atomic_helper.h | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index e872121e9fb0..e42ae1c6ebcd 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -2,6 +2,7 @@
#include <linux/clk.h> #include <linux/of_clk.h> +#include <linux/minmax.h> #include <linux/platform_data/simplefb.h> #include <linux/platform_device.h> #include <linux/regulator/consumer.h> @@ -776,6 +777,7 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) struct drm_display_mode *mode = &sdev->mode; struct drm_connector *connector = &sdev->connector; struct drm_simple_display_pipe *pipe = &sdev->pipe; + unsigned long max_width, max_height; const uint32_t *formats; size_t nformats; int ret; @@ -784,10 +786,13 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) if (ret) return ret;
+ max_width = max_t(unsigned long, mode->hdisplay, DRM_SHADOW_PLANE_MAX_WIDTH); + max_height = max_t(unsigned long, mode->vdisplay, DRM_SHADOW_PLANE_MAX_HEIGHT); + dev->mode_config.min_width = mode->hdisplay; - dev->mode_config.max_width = mode->hdisplay; + dev->mode_config.max_width = max_width; dev->mode_config.min_height = mode->vdisplay; - dev->mode_config.max_height = mode->vdisplay; + dev->mode_config.max_height = max_height; dev->mode_config.prefer_shadow_fbdev = true; dev->mode_config.preferred_depth = sdev->format->cpp[0] * 8; dev->mode_config.funcs = &simpledrm_mode_config_funcs; diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h index 48222a107873..54983ecf641a 100644 --- a/include/drm/drm_gem_atomic_helper.h +++ b/include/drm/drm_gem_atomic_helper.h @@ -22,6 +22,24 @@ int drm_gem_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe, * Helpers for planes with shadow buffers */
+/** + * DRM_SHADOW_PLANE_MAX_WIDTH - Maximum width of a plane's shadow buffer in pixels + * + * For drivers with shadow planes, the maximum width of the framebuffer is + * usually independent from hardware limitations. Drivers can initialize struct + * drm_mode_config.max_width from DRM_SHADOW_PLANE_MAX_WIDTH. + */ +#define DRM_SHADOW_PLANE_MAX_WIDTH (1ul << 12) + +/** + * DRM_SHADOW_PLANE_MAX_HEIGHT - Maximum height of a plane's shadow buffer in scanlines + * + * For drivers with shadow planes, the maximum height of the framebuffer is + * usually independent from hardware limitations. Drivers can initialize struct + * drm_mode_config.max_height from DRM_SHADOW_PLANE_MAX_HEIGHT. + */ +#define DRM_SHADOW_PLANE_MAX_HEIGHT (1ul << 12) + /** * struct drm_shadow_plane_state - plane state for planes with shadow buffers *
Add additional information on the semantics of the size fields in struct drm_mode_config. Also add a TODO to review all driver for correct usage of these fields.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- Documentation/gpu/todo.rst | 15 +++++++++++++++ include/drm/drm_mode_config.h | 13 +++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 60d1d7ee0719..f4e1d72149f7 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -463,6 +463,21 @@ Contact: Thomas Zimmermann tzimmermann@suse.de, Christian König, Daniel Vette
Level: Intermediate
+Review all drivers for setting struct drm_mode_config.{max_width,max_height} correctly +-------------------------------------------------------------------------------------- + +The values in struct drm_mode_config.{max_width,max_height} describe the +maximum supported framebuffer size. It's the virtual screen size, but many +drivers treat it like limitations of the physical resolution. + +The maximum width depends on the hardware's maximum scanline pitch. The +maximum height depends on the amount of addressable video memory. Review all +drivers to initialize the fields to the correct values. + +Contact: Thomas Zimmermann tzimmermann@suse.de + +Level: Intermediate +
Core refactorings ================= diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 48b7de80daf5..91ca575a78de 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -359,6 +359,19 @@ struct drm_mode_config_funcs { * Core mode resource tracking structure. All CRTC, encoders, and connectors * enumerated by the driver are added here, as are global properties. Some * global restrictions are also here, e.g. dimension restrictions. + * + * Framebuffer sizes refer to the virtual screen that can be displayed by + * the CRTC. This can be different from the physical resolution programmed. + * The minimum width and height, stored in @min_width and @min_height, + * describe the smallest size of the framebuffer. It correlates to the + * minimum programmable resolution. + * The maximum width, stored in @max_width, is typically limited by the + * maximum pitch between two adjacent scanlines. The maximum height, stored + * in @max_height, is usually only limited by the amount of addressable video + * memory. For hardware that has no real maximum, drivers should pick a + * reasonable default. + * + * See also @DRM_SHADOW_PLANE_MAX_WIDTH and @DRM_SHADOW_PLANE_MAX_HEIGHT. */ struct drm_mode_config { /**
Hi Thomas,
On Fri, Oct 22, 2021 at 03:28:20PM +0200, Thomas Zimmermann wrote:
Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or less overhead. With this in place, add support for virtual screens (i.e., framebuffers that are larger than the display resolution). This also enables fbdev panning and page flipping.
After the discussion and bug fixing wrt to fbdev overallocation, I decided to add full support for this to simpledrm. Patches #1 to #5 change the format-helper functions accordingly. Destination buffers are now clipped by the caller and all functions support a similar feature set. This has some fallout in various drivers.
Patch #6 change fbdev emulation to support overallocation with shadow buffers, even if the hardware buffer would be too small.
This change is very welcome - I hope it will solve the problem Alistair experience - see: https://lore.kernel.org/all/CAKmqyKPCP45O5_gjCFwUs8jU4NrDnjAeLs7OYAE4j-LEUw+...
Sam
Patch #7 and #8 update simpledrm to enable damage clipping and virtual screen sizes. Both feature go hand in hand, sort of. For shadow- buffered planes, the DRM framebuffer lives in system memory. So the maximum size of the virtual screen is somewhat arbitrary. We add two constants for resonable maximum width and height of 4096 each.
Patch #9 adds documentation and a TODO item.
Tested with simpledrm. I also ran the recently posted fbdev panning tests to make sure that the fbdev overallocation works correctly. [1]
[1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html
Thomas Zimmermann (9): drm/format-helper: Export drm_fb_clip_offset() drm/format-helper: Rework format-helper memcpy functions drm/format-helper: Add destination-buffer pitch to drm_fb_swab() drm/format-helper: Rework format-helper conversion functions drm/format-helper: Streamline blit-helper interface drm/fb-helper: Allocate shadow buffer of surface height drm/simpledrm: Enable FB_DAMAGE_CLIPS property drm/simpledrm: Support virtual screen sizes drm: Clarify semantics of struct drm_mode_config.{min,max}_{width,height}
Documentation/gpu/todo.rst | 15 ++ drivers/gpu/drm/drm_fb_helper.c | 2 +- drivers/gpu/drm/drm_format_helper.c | 236 ++++++++++---------- drivers/gpu/drm/drm_mipi_dbi.c | 6 +- drivers/gpu/drm/gud/gud_pipe.c | 14 +- drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 5 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 4 +- drivers/gpu/drm/tiny/cirrus.c | 24 +- drivers/gpu/drm/tiny/repaper.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 41 +++- drivers/gpu/drm/tiny/st7586.c | 2 +- include/drm/drm_format_helper.h | 58 ++--- include/drm/drm_gem_atomic_helper.h | 18 ++ include/drm/drm_mode_config.h | 13 ++ 14 files changed, 254 insertions(+), 186 deletions(-)
-- 2.33.0
dri-devel@lists.freedesktop.org
-
Noralf Trønnes -
Sam Ravnborg -
Thomas Zimmermann