Here's a number of fb-helper patches that have been piling up recently.
Patches 1 to 3 fix bugs that I spotted while going through the code. Because of the way the fbdev code works, they have been avoided so far.
Patches 4 to 10 cleanup damage handling for fbdev's shadow buffer and fix a few issues.
Specifically, the final patch adds locking to the code that flushes the shadow framebuffer into BO memory. During the conversion of radeon to generic fbdev, the question came up about interference with concurrent modesets. If fbdev has the BO pinned in system memory for flushing while the modeset wants to pin it to VRAM for scanout, the modeset would most likely fail. We haven't seen that so far, but it's possible at least. Acquiring the fb-helper lock during the flush operation prevents concurrent modesets from taking place.
The code has been tested with SHMEM and TTM BOs; with atomic and non- atomic modesetting.
[1] https://patchwork.freedesktop.org/patch/400054/?series=83765&rev=1
Thomas Zimmermann (10): drm/fb-helper: Call dirty helper after writing to fbdev drm/fb-helper: Unmap client buffer during shutdown drm/client: Depend on GEM object kmap ref-counting drm/fb-helper: Rename dirty worker to damage worker drm/fb-helper: Return early in dirty worker drm/fb-helper: Separate shadow-buffer flushing and calling dirty callback drm/fb-helper: Move damage blit code and its setup into separate routine drm/fb-helper: Restore damage area upon errors drm/fb-helper: Copy dma-buf map before flushing shadow fb drm/fb-helper: Acquire modeset lock around shadow-buffer flushing
drivers/gpu/drm/drm_client.c | 4 - drivers/gpu/drm/drm_fb_helper.c | 155 +++++++++++++++++++++----------- include/drm/drm_fb_helper.h | 14 +-- 3 files changed, 108 insertions(+), 65 deletions(-)
-- 2.29.2
If fbdev uses a shadow framebuffer, call the damage handler. Otherwise the update might not make it to the screen.
v2: * mark virtual screen as dirty (Ville)
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: 222ec45f4c69 ("drm/fb_helper: Support framebuffers in I/O memory") Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Sam Ravnborg sam@ravnborg.org Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: Gerd Hoffmann kraxel@redhat.com Cc: dri-devel@lists.freedesktop.org Cc: virtualization@lists.linux-foundation.org --- drivers/gpu/drm/drm_fb_helper.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 25edf670867c..9c673f33d222 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2189,6 +2189,9 @@ static ssize_t drm_fbdev_fb_write(struct fb_info *info, const char __user *buf, if (ret > 0) *ppos += ret;
+ if (ret > 0) + drm_fb_helper_dirty(info, 0, 0, info->var.xres_virtual, info->var.yres_virtual); + return ret ? ret : err; }
On Fri, Nov 20, 2020 at 11:25:36AM +0100, Thomas Zimmermann wrote:
If fbdev uses a shadow framebuffer, call the damage handler. Otherwise the update might not make it to the screen.
v2:
- mark virtual screen as dirty (Ville)
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: 222ec45f4c69 ("drm/fb_helper: Support framebuffers in I/O memory") Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Sam Ravnborg sam@ravnborg.org Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: Gerd Hoffmann kraxel@redhat.com Cc: dri-devel@lists.freedesktop.org Cc: virtualization@lists.linux-foundation.org
Acked-by: Sam Ravnborg sam@ravnborg.org
The fbdev helper's generic probe function establishes a mapping for framebuffers without shadow buffer. The clean-up function did not unmap the buffer object. Add the unmap operation.
As fbdev devices are usally released during system shutdown, this has not been a problem in practice.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 9c673f33d222..aa7af463c50d 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1988,14 +1988,19 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper) if (!fb_helper->dev) return;
- if (fbi && fbi->fbdefio) { - fb_deferred_io_cleanup(fbi); - shadow = fbi->screen_buffer; + if (fbi) { + if (fbi->fbdefio) + fb_deferred_io_cleanup(fbi); + if (drm_fbdev_use_shadow_fb(fb_helper)) + shadow = fbi->screen_buffer; }
drm_fb_helper_fini(fb_helper);
- vfree(shadow); + if (shadow) + vfree(shadow); + else + drm_client_buffer_vunmap(fb_helper->buffer);
drm_client_framebuffer_delete(fb_helper->buffer); }
On Fri, Nov 20, 2020 at 11:25:37AM +0100, Thomas Zimmermann wrote:
The fbdev helper's generic probe function establishes a mapping for framebuffers without shadow buffer. The clean-up function did not unmap the buffer object. Add the unmap operation.
As fbdev devices are usally released during system shutdown, this has not been a problem in practice.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Reviewed-by: Sam Ravnborg sam@ravnborg.org
DRM client's vmap/vunmap functions don't allow for multiple vmap operations. Calling drm_client_buffer_vmap() twice returns the same mapping, then calling drm_client_buffer_vunmap() twice already unmaps on the first call. This leads to unbalanced vmap refcounts. Fix this by calling drm_gem_vmap() unconditionally in drm_client_buffer_vmap().
All drivers that support DRM clients have to implement correct ref- counting for their vmap operations, or not vunmap at all. This is the case for drivers that use CMA, SHMEM and VRAM helpers, and QXL. Other drivers are not affected.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_client.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index fe573acf1067..ce45e380f4a2 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -314,9 +314,6 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map *map struct dma_buf_map *map = &buffer->map; int ret;
- if (dma_buf_map_is_set(map)) - goto out; - /* * FIXME: The dependency on GEM here isn't required, we could * convert the driver handle to a dma-buf instead and use the @@ -329,7 +326,6 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map *map if (ret) return ret;
-out: *map_copy = *map;
return 0;
The dirty worker handles all damage updates, instead of just calling the framebuffer's dirty callback. Rename it to damage worker. Also rename related variables accordingly. No functional changes are made.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 65 +++++++++++++++------------------ include/drm/drm_fb_helper.h | 14 +++---- 2 files changed, 36 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index aa7af463c50d..87d4759de04a 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -371,9 +371,9 @@ static void drm_fb_helper_resume_worker(struct work_struct *work) console_unlock(); }
-static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper, - struct drm_clip_rect *clip, - struct dma_buf_map *dst) +static void drm_fb_helper_damage_blit_real(struct drm_fb_helper *fb_helper, + struct drm_clip_rect *clip, + struct dma_buf_map *dst) { struct drm_framebuffer *fb = fb_helper->fb; unsigned int cpp = fb->format->cpp[0]; @@ -391,21 +391,21 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper, } }
-static void drm_fb_helper_dirty_work(struct work_struct *work) +static void drm_fb_helper_damage_work(struct work_struct *work) { struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, - dirty_work); - struct drm_clip_rect *clip = &helper->dirty_clip; + damage_work); + struct drm_clip_rect *clip = &helper->damage_clip; struct drm_clip_rect clip_copy; unsigned long flags; struct dma_buf_map map; int ret;
- spin_lock_irqsave(&helper->dirty_lock, flags); + spin_lock_irqsave(&helper->damage_lock, flags); clip_copy = *clip; clip->x1 = clip->y1 = ~0; clip->x2 = clip->y2 = 0; - spin_unlock_irqrestore(&helper->dirty_lock, flags); + spin_unlock_irqrestore(&helper->damage_lock, flags);
/* call dirty callback only when it has been really touched */ if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2) { @@ -415,7 +415,7 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) ret = drm_client_buffer_vmap(helper->buffer, &map); if (ret) return; - drm_fb_helper_dirty_blit_real(helper, &clip_copy, &map); + drm_fb_helper_damage_blit_real(helper, &clip_copy, &map); }
if (helper->fb->funcs->dirty) @@ -440,10 +440,10 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, const struct drm_fb_helper_funcs *funcs) { INIT_LIST_HEAD(&helper->kernel_fb_list); - spin_lock_init(&helper->dirty_lock); + spin_lock_init(&helper->damage_lock); INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker); - INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); - helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; + INIT_WORK(&helper->damage_work, drm_fb_helper_damage_work); + helper->damage_clip.x1 = helper->damage_clip.y1 = ~0; mutex_init(&helper->lock); helper->funcs = funcs; helper->dev = dev; @@ -579,7 +579,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper) return;
cancel_work_sync(&fb_helper->resume_work); - cancel_work_sync(&fb_helper->dirty_work); + cancel_work_sync(&fb_helper->damage_work);
info = fb_helper->fbdev; if (info) { @@ -614,30 +614,30 @@ static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper) fb->funcs->dirty; }
-static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, - u32 width, u32 height) +static void drm_fb_helper_damage(struct fb_info *info, u32 x, u32 y, + u32 width, u32 height) { struct drm_fb_helper *helper = info->par; - struct drm_clip_rect *clip = &helper->dirty_clip; + struct drm_clip_rect *clip = &helper->damage_clip; unsigned long flags;
if (!drm_fbdev_use_shadow_fb(helper)) return;
- spin_lock_irqsave(&helper->dirty_lock, flags); + spin_lock_irqsave(&helper->damage_lock, flags); clip->x1 = min_t(u32, clip->x1, x); clip->y1 = min_t(u32, clip->y1, y); clip->x2 = max_t(u32, clip->x2, x + width); clip->y2 = max_t(u32, clip->y2, y + height); - spin_unlock_irqrestore(&helper->dirty_lock, flags); + spin_unlock_irqrestore(&helper->damage_lock, flags);
- schedule_work(&helper->dirty_work); + schedule_work(&helper->damage_work); }
/** * drm_fb_helper_deferred_io() - fbdev deferred_io callback function * @info: fb_info struct pointer - * @pagelist: list of dirty mmap framebuffer pages + * @pagelist: list of mmap framebuffer pages that have to be flushed * * This function is used as the &fb_deferred_io.deferred_io * callback function for flushing the fbdev mmap writes. @@ -662,7 +662,7 @@ void drm_fb_helper_deferred_io(struct fb_info *info, y1 = min / info->fix.line_length; y2 = min_t(u32, DIV_ROUND_UP(max, info->fix.line_length), info->var.yres); - drm_fb_helper_dirty(info, 0, y1, info->var.xres, y2 - y1); + drm_fb_helper_damage(info, 0, y1, info->var.xres, y2 - y1); } } EXPORT_SYMBOL(drm_fb_helper_deferred_io); @@ -699,8 +699,7 @@ ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf,
ret = fb_sys_write(info, buf, count, ppos); if (ret > 0) - drm_fb_helper_dirty(info, 0, 0, info->var.xres, - info->var.yres); + drm_fb_helper_damage(info, 0, 0, info->var.xres, info->var.yres);
return ret; } @@ -717,8 +716,7 @@ void drm_fb_helper_sys_fillrect(struct fb_info *info, const struct fb_fillrect *rect) { sys_fillrect(info, rect); - drm_fb_helper_dirty(info, rect->dx, rect->dy, - rect->width, rect->height); + drm_fb_helper_damage(info, rect->dx, rect->dy, rect->width, rect->height); } EXPORT_SYMBOL(drm_fb_helper_sys_fillrect);
@@ -733,8 +731,7 @@ void drm_fb_helper_sys_copyarea(struct fb_info *info, const struct fb_copyarea *area) { sys_copyarea(info, area); - drm_fb_helper_dirty(info, area->dx, area->dy, - area->width, area->height); + drm_fb_helper_damage(info, area->dx, area->dy, area->width, area->height); } EXPORT_SYMBOL(drm_fb_helper_sys_copyarea);
@@ -749,8 +746,7 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info, const struct fb_image *image) { sys_imageblit(info, image); - drm_fb_helper_dirty(info, image->dx, image->dy, - image->width, image->height); + drm_fb_helper_damage(info, image->dx, image->dy, image->width, image->height); } EXPORT_SYMBOL(drm_fb_helper_sys_imageblit);
@@ -765,8 +761,7 @@ void drm_fb_helper_cfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect) { cfb_fillrect(info, rect); - drm_fb_helper_dirty(info, rect->dx, rect->dy, - rect->width, rect->height); + drm_fb_helper_damage(info, rect->dx, rect->dy, rect->width, rect->height); } EXPORT_SYMBOL(drm_fb_helper_cfb_fillrect);
@@ -781,8 +776,7 @@ void drm_fb_helper_cfb_copyarea(struct fb_info *info, const struct fb_copyarea *area) { cfb_copyarea(info, area); - drm_fb_helper_dirty(info, area->dx, area->dy, - area->width, area->height); + drm_fb_helper_damage(info, area->dx, area->dy, area->width, area->height); } EXPORT_SYMBOL(drm_fb_helper_cfb_copyarea);
@@ -797,8 +791,7 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info, const struct fb_image *image) { cfb_imageblit(info, image); - drm_fb_helper_dirty(info, image->dx, image->dy, - image->width, image->height); + drm_fb_helper_damage(info, image->dx, image->dy, image->width, image->height); } EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit);
@@ -2195,7 +2188,7 @@ static ssize_t drm_fbdev_fb_write(struct fb_info *info, const char __user *buf, *ppos += ret;
if (ret > 0) - drm_fb_helper_dirty(info, 0, 0, info->var.xres_virtual, info->var.yres_virtual); + drm_fb_helper_damage(info, 0, 0, info->var.xres_virtual, info->var.yres_virtual);
return ret ? ret : err; } diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 306aa3a60be9..3b273f9ca39a 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -100,10 +100,10 @@ struct drm_fb_helper_funcs { * @funcs: driver callbacks for fb helper * @fbdev: emulated fbdev device info struct * @pseudo_palette: fake palette of 16 colors - * @dirty_clip: clip rectangle used with deferred_io to accumulate damage to - * the screen buffer - * @dirty_lock: spinlock protecting @dirty_clip - * @dirty_work: worker used to flush the framebuffer + * @damage_clip: clip rectangle used with deferred_io to accumulate damage to + * the screen buffer + * @damage_lock: spinlock protecting @damage_clip + * @damage_work: worker used to flush the framebuffer * @resume_work: worker used during resume if the console lock is already taken * * This is the main structure used by the fbdev helpers. Drivers supporting @@ -131,9 +131,9 @@ struct drm_fb_helper { const struct drm_fb_helper_funcs *funcs; struct fb_info *fbdev; u32 pseudo_palette[17]; - struct drm_clip_rect dirty_clip; - spinlock_t dirty_lock; - struct work_struct dirty_work; + struct drm_clip_rect damage_clip; + spinlock_t damage_lock; + struct work_struct damage_work; struct work_struct resume_work;
/**
On Fri, Nov 20, 2020 at 11:25:39AM +0100, Thomas Zimmermann wrote:
The dirty worker handles all damage updates, instead of just calling the framebuffer's dirty callback. Rename it to damage worker. Also rename related variables accordingly. No functional changes are made.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Reviewed-by: Sam Ravnborg sam@ravnborg.org
It had been nice to move to inline comments in struct drm_fb_helper_funcs but that can be another day.
Sam
Returning early in the dirty worker if no update is required makes the code more readable. No functional changes are made.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 87d4759de04a..c9018ffff5f9 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -407,24 +407,23 @@ static void drm_fb_helper_damage_work(struct work_struct *work) clip->x2 = clip->y2 = 0; spin_unlock_irqrestore(&helper->damage_lock, flags);
- /* call dirty callback only when it has been really touched */ - if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2) { - - /* Generic fbdev uses a shadow buffer */ - if (helper->buffer) { - ret = drm_client_buffer_vmap(helper->buffer, &map); - if (ret) - return; - drm_fb_helper_damage_blit_real(helper, &clip_copy, &map); - } - - if (helper->fb->funcs->dirty) - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, - &clip_copy, 1); + /* Call damage handlers only if necessary */ + if (!(clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2)) + return;
- if (helper->buffer) - drm_client_buffer_vunmap(helper->buffer); + /* Generic fbdev uses a shadow buffer */ + if (helper->buffer) { + ret = drm_client_buffer_vmap(helper->buffer, &map); + if (ret) + return; + drm_fb_helper_damage_blit_real(helper, &clip_copy, &map); } + + if (helper->fb->funcs->dirty) + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1); + + if (helper->buffer) + drm_client_buffer_vunmap(helper->buffer); }
/**
On Fri, Nov 20, 2020 at 11:25:40AM +0100, Thomas Zimmermann wrote:
Returning early in the dirty worker if no update is required makes the code more readable. No functional changes are made.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
It is a damage worker (both subject and changelog).
Reviewed-by: Sam Ravnborg sam@ravnborg.org
drivers/gpu/drm/drm_fb_helper.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 87d4759de04a..c9018ffff5f9 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -407,24 +407,23 @@ static void drm_fb_helper_damage_work(struct work_struct *work) clip->x2 = clip->y2 = 0; spin_unlock_irqrestore(&helper->damage_lock, flags);
- /* call dirty callback only when it has been really touched */
- if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2) {
/* Generic fbdev uses a shadow buffer */if (helper->buffer) {ret = drm_client_buffer_vmap(helper->buffer, &map);if (ret)return;drm_fb_helper_damage_blit_real(helper, &clip_copy, &map);}if (helper->fb->funcs->dirty)helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,&clip_copy, 1);
- /* Call damage handlers only if necessary */
- if (!(clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2))
return;
if (helper->buffer)drm_client_buffer_vunmap(helper->buffer);
- /* Generic fbdev uses a shadow buffer */
- if (helper->buffer) {
ret = drm_client_buffer_vmap(helper->buffer, &map);if (ret)return;drm_fb_helper_damage_blit_real(helper, &clip_copy, &map);- if (helper->fb->funcs->dirty)
helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);- if (helper->buffer)
drm_client_buffer_vunmap(helper->buffer);}
/**
2.29.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi
Am 23.11.20 um 20:23 schrieb Sam Ravnborg:
On Fri, Nov 20, 2020 at 11:25:40AM +0100, Thomas Zimmermann wrote:
Returning early in the dirty worker if no update is required makes the code more readable. No functional changes are made.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
It is a damage worker (both subject and changelog).
I changed this before pushing the branches. Thanks!
Best regards Thomas
Reviewed-by: Sam Ravnborg sam@ravnborg.org
drivers/gpu/drm/drm_fb_helper.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 87d4759de04a..c9018ffff5f9 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -407,24 +407,23 @@ static void drm_fb_helper_damage_work(struct work_struct *work) clip->x2 = clip->y2 = 0; spin_unlock_irqrestore(&helper->damage_lock, flags);
- /* call dirty callback only when it has been really touched */
- if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2) {
/* Generic fbdev uses a shadow buffer */if (helper->buffer) {ret = drm_client_buffer_vmap(helper->buffer, &map);if (ret)return;drm_fb_helper_damage_blit_real(helper, &clip_copy, &map);}if (helper->fb->funcs->dirty)helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,&clip_copy, 1);
- /* Call damage handlers only if necessary */
- if (!(clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2))
return;
if (helper->buffer)drm_client_buffer_vunmap(helper->buffer);
/* Generic fbdev uses a shadow buffer */
if (helper->buffer) {
ret = drm_client_buffer_vmap(helper->buffer, &map);if (ret)return;drm_fb_helper_damage_blit_real(helper, &clip_copy, &map);}
if (helper->fb->funcs->dirty)
helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);if (helper->buffer)
drm_client_buffer_vunmap(helper->buffer);}
/**
-- 2.29.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Flushing the shadow framebuffer and invoking the dirty callback are two separate operations, so do them seprately. The flush operation is paired with calls to vmap and vunmap. They are not needed for the dirty callback, which performs its own invocations if necessary.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index c9018ffff5f9..bdfdf60e7bd8 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -417,13 +417,11 @@ static void drm_fb_helper_damage_work(struct work_struct *work) if (ret) return; drm_fb_helper_damage_blit_real(helper, &clip_copy, &map); + drm_client_buffer_vunmap(helper->buffer); }
if (helper->fb->funcs->dirty) helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1); - - if (helper->buffer) - drm_client_buffer_vunmap(helper->buffer); }
/**
On Fri, Nov 20, 2020 at 11:25:41AM +0100, Thomas Zimmermann wrote:
Flushing the shadow framebuffer and invoking the dirty callback are two separate operations, so do them seprately. The flush operation is paired with calls to vmap and vunmap. They are not needed for the dirty callback, which performs its own invocations if necessary.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Reviewed-by: Sam Ravnborg sam@ravnborg.org
Introduce a separate function for the blit code and its vmap setup. Done in preparation of additional changes. No functional changes are made.
v2: * print a single warning if damage blitter fails
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index bdfdf60e7bd8..b0bde894bf39 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -391,14 +391,32 @@ static void drm_fb_helper_damage_blit_real(struct drm_fb_helper *fb_helper, } }
+static int drm_fb_helper_damage_blit(struct drm_fb_helper *fb_helper, + struct drm_clip_rect *clip) +{ + struct drm_client_buffer *buffer = fb_helper->buffer; + struct dma_buf_map map; + int ret; + + ret = drm_client_buffer_vmap(buffer, &map); + if (ret) + return ret; + + drm_fb_helper_damage_blit_real(fb_helper, clip, &map); + + drm_client_buffer_vunmap(buffer); + + return 0; +} + static void drm_fb_helper_damage_work(struct work_struct *work) { struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, damage_work); + struct drm_device *dev = helper->dev; struct drm_clip_rect *clip = &helper->damage_clip; struct drm_clip_rect clip_copy; unsigned long flags; - struct dma_buf_map map; int ret;
spin_lock_irqsave(&helper->damage_lock, flags); @@ -411,13 +429,10 @@ static void drm_fb_helper_damage_work(struct work_struct *work) if (!(clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2)) return;
- /* Generic fbdev uses a shadow buffer */ if (helper->buffer) { - ret = drm_client_buffer_vmap(helper->buffer, &map); - if (ret) + ret = drm_fb_helper_damage_blit(helper, &clip_copy); + if (drm_WARN_ON_ONCE(dev, ret)) return; - drm_fb_helper_damage_blit_real(helper, &clip_copy, &map); - drm_client_buffer_vunmap(helper->buffer); }
if (helper->fb->funcs->dirty)
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 6a1b34c0a339fdc75d7932ad5702f2177c9d7a1c ("drm/fb-helper: Move damage blit code and its setup into separate routine") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-fb-helper-Var...
in testcase: trinity version: trinity-static-i386-x86_64-f93256fb_2019-08-28 with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+-----------------------------------------------------------------------+------------+------------+ | | 154f2d1afd | 6a1b34c0a3 | +-----------------------------------------------------------------------+------------+------------+ | WARNING:at_drivers/gpu/drm/drm_fb_helper.c:#drm_fb_helper_damage_work | 0 | 36 | | EIP:drm_fb_helper_damage_work | 0 | 36 | +-----------------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 106.616652] WARNING: CPU: 1 PID: 173 at drivers/gpu/drm/drm_fb_helper.c:434 drm_fb_helper_damage_work+0x371/0x390 [ 106.627732] Modules linked in: [ 106.632419] CPU: 1 PID: 173 Comm: kworker/1:2 Not tainted 5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 #3 [ 106.637806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 106.642853] Workqueue: events drm_fb_helper_damage_work [ 106.647664] EIP: drm_fb_helper_damage_work+0x371/0x390 [ 106.652305] Code: b1 17 c7 01 68 bd 5b 2d c5 53 50 68 55 21 2d c5 83 15 44 b1 17 c7 00 e8 ae bc b1 01 83 05 48 b1 17 c7 01 83 15 4c b1 17 c7 00 <0f> 0b 83 05 50 b1 17 c7 01 83 15 54 b1 17 c7 00 83 c4 10 e9 78 fd [ 106.663517] EAX: 0000002d EBX: c8730520 ECX: 00000847 EDX: 00000000 [ 106.668423] ESI: ca987000 EDI: cab274d8 EBP: f62f5f20 ESP: f62f5ee8 [ 106.673214] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010246 [ 106.678295] CR0: 80050033 CR2: 00000000 CR3: 063a7000 CR4: 000406d0 [ 106.683160] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 [ 106.687967] DR6: fffe0ff0 DR7: 00000400 [ 106.690763] Call Trace: [ 106.693394] process_one_work+0x3ea/0xaa0 [ 106.693501] ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver [ 106.695300] worker_thread+0x330/0x900 [ 106.697406] ixgbevf: Copyright (c) 2009 - 2018 Intel Corporation. [ 106.702963] kthread+0x190/0x210 [ 106.705709] ? rescuer_thread+0x650/0x650 [ 106.708379] ? kthread_insert_work_sanity_check+0x120/0x120 [ 106.711271] ret_from_fork+0x1c/0x30 [ 106.713973] ---[ end trace dd528799d3369ac1 ]---
To reproduce:
# build kernel cd linux cp config-5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 .config make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks, Oliver Sang
Hi
Am 22.11.20 um 15:18 schrieb kernel test robot:
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 6a1b34c0a339fdc75d7932ad5702f2177c9d7a1c ("drm/fb-helper: Move damage blit code and its setup into separate routine") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-fb-helper-Var...
in testcase: trinity version: trinity-static-i386-x86_64-f93256fb_2019-08-28 with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
That dmesg is full of messages like
[ 696.323556] alloc_vmap_area: 24 callbacks suppressed [ 696.323562] vmap allocation for size 3149824 failed: use vmalloc=<size> to increase size
I think the test system needs to be reconfigured first.
Best regards Thomas
+-----------------------------------------------------------------------+------------+------------+ | | 154f2d1afd | 6a1b34c0a3 | +-----------------------------------------------------------------------+------------+------------+ | WARNING:at_drivers/gpu/drm/drm_fb_helper.c:#drm_fb_helper_damage_work | 0 | 36 | | EIP:drm_fb_helper_damage_work | 0 | 36 | +-----------------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 106.616652] WARNING: CPU: 1 PID: 173 at drivers/gpu/drm/drm_fb_helper.c:434 drm_fb_helper_damage_work+0x371/0x390 [ 106.627732] Modules linked in: [ 106.632419] CPU: 1 PID: 173 Comm: kworker/1:2 Not tainted 5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 #3 [ 106.637806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 106.642853] Workqueue: events drm_fb_helper_damage_work [ 106.647664] EIP: drm_fb_helper_damage_work+0x371/0x390 [ 106.652305] Code: b1 17 c7 01 68 bd 5b 2d c5 53 50 68 55 21 2d c5 83 15 44 b1 17 c7 00 e8 ae bc b1 01 83 05 48 b1 17 c7 01 83 15 4c b1 17 c7 00 <0f> 0b 83 05 50 b1 17 c7 01 83 15 54 b1 17 c7 00 83 c4 10 e9 78 fd [ 106.663517] EAX: 0000002d EBX: c8730520 ECX: 00000847 EDX: 00000000 [ 106.668423] ESI: ca987000 EDI: cab274d8 EBP: f62f5f20 ESP: f62f5ee8 [ 106.673214] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010246 [ 106.678295] CR0: 80050033 CR2: 00000000 CR3: 063a7000 CR4: 000406d0 [ 106.683160] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 [ 106.687967] DR6: fffe0ff0 DR7: 00000400 [ 106.690763] Call Trace: [ 106.693394] process_one_work+0x3ea/0xaa0 [ 106.693501] ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver [ 106.695300] worker_thread+0x330/0x900 [ 106.697406] ixgbevf: Copyright (c) 2009 - 2018 Intel Corporation. [ 106.702963] kthread+0x190/0x210 [ 106.705709] ? rescuer_thread+0x650/0x650 [ 106.708379] ? kthread_insert_work_sanity_check+0x120/0x120 [ 106.711271] ret_from_fork+0x1c/0x30 [ 106.713973] ---[ end trace dd528799d3369ac1 ]---
To reproduce:
# build kernelcd linux cp config-5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 .config make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> job-script # job-script is attached in this emailThanks, Oliver Sang
On 11/23/2020 4:04 PM, Thomas Zimmermann wrote:
Hi
Am 22.11.20 um 15:18 schrieb kernel test robot:
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 6a1b34c0a339fdc75d7932ad5702f2177c9d7a1c ("drm/fb-helper: Move damage blit code and its setup into separate routine") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-fb-helper-Var...
in testcase: trinity version: trinity-static-i386-x86_64-f93256fb_2019-08-28 with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
That dmesg is full of messages like
[ 696.323556] alloc_vmap_area: 24 callbacks suppressed [ 696.323562] vmap allocation for size 3149824 failed: use vmalloc=<size> to increase size
I think the test system needs to be reconfigured first.
We have tried "vmalloc=256M" and "vmalloc=512M", the same warning still happened.
Best regards Thomas
+-----------------------------------------------------------------------+------------+------------+
| | 154f2d1afd | 6a1b34c0a3 | +-----------------------------------------------------------------------+------------+------------+
| WARNING:at_drivers/gpu/drm/drm_fb_helper.c:#drm_fb_helper_damage_work | 0 | 36 | | EIP:drm_fb_helper_damage_work | 0 | 36 | +-----------------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 106.616652] WARNING: CPU: 1 PID: 173 at drivers/gpu/drm/drm_fb_helper.c:434 drm_fb_helper_damage_work+0x371/0x390 [ 106.627732] Modules linked in: [ 106.632419] CPU: 1 PID: 173 Comm: kworker/1:2 Not tainted 5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 #3 [ 106.637806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 106.642853] Workqueue: events drm_fb_helper_damage_work [ 106.647664] EIP: drm_fb_helper_damage_work+0x371/0x390 [ 106.652305] Code: b1 17 c7 01 68 bd 5b 2d c5 53 50 68 55 21 2d c5 83 15 44 b1 17 c7 00 e8 ae bc b1 01 83 05 48 b1 17 c7 01 83 15 4c b1 17 c7 00 <0f> 0b 83 05 50 b1 17 c7 01 83 15 54 b1 17 c7 00 83 c4 10 e9 78 fd [ 106.663517] EAX: 0000002d EBX: c8730520 ECX: 00000847 EDX: 00000000 [ 106.668423] ESI: ca987000 EDI: cab274d8 EBP: f62f5f20 ESP: f62f5ee8 [ 106.673214] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010246 [ 106.678295] CR0: 80050033 CR2: 00000000 CR3: 063a7000 CR4: 000406d0 [ 106.683160] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 [ 106.687967] DR6: fffe0ff0 DR7: 00000400 [ 106.690763] Call Trace: [ 106.693394] process_one_work+0x3ea/0xaa0 [ 106.693501] ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver [ 106.695300] worker_thread+0x330/0x900 [ 106.697406] ixgbevf: Copyright (c) 2009 - 2018 Intel Corporation. [ 106.702963] kthread+0x190/0x210 [ 106.705709] ? rescuer_thread+0x650/0x650 [ 106.708379] ? kthread_insert_work_sanity_check+0x120/0x120 [ 106.711271] ret_from_fork+0x1c/0x30 [ 106.713973] ---[ end trace dd528799d3369ac1 ]---
To reproduce:
# build kernel cd linux cp config-5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 .config make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks, Oliver Sang
LKP mailing list -- lkp@lists.01.org To unsubscribe send an email to lkp-leave@lists.01.org
Hi
Am 24.11.20 um 02:58 schrieb Xing Zhengjun:
On 11/23/2020 4:04 PM, Thomas Zimmermann wrote:
Hi
Am 22.11.20 um 15:18 schrieb kernel test robot:
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 6a1b34c0a339fdc75d7932ad5702f2177c9d7a1c ("drm/fb-helper: Move damage blit code and its setup into separate routine") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-fb-helper-Var...
in testcase: trinity version: trinity-static-i386-x86_64-f93256fb_2019-08-28 with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
That dmesg is full of messages like
[ 696.323556] alloc_vmap_area: 24 callbacks suppressed [ 696.323562] vmap allocation for size 3149824 failed: use vmalloc=<size> to increase size
I think the test system needs to be reconfigured first.
We have tried "vmalloc=256M" and "vmalloc=512M", the same warning still happened.
OK, then I don't know. Thanks for testing.
Best regards Thomas
Best regards Thomas
+-----------------------------------------------------------------------+------------+------------+
| | 154f2d1afd | 6a1b34c0a3 | +-----------------------------------------------------------------------+------------+------------+
| WARNING:at_drivers/gpu/drm/drm_fb_helper.c:#drm_fb_helper_damage_work | 0 | 36 | | EIP:drm_fb_helper_damage_work | 0 | 36 | +-----------------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 106.616652] WARNING: CPU: 1 PID: 173 at drivers/gpu/drm/drm_fb_helper.c:434 drm_fb_helper_damage_work+0x371/0x390 [ 106.627732] Modules linked in: [ 106.632419] CPU: 1 PID: 173 Comm: kworker/1:2 Not tainted 5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 #3 [ 106.637806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 106.642853] Workqueue: events drm_fb_helper_damage_work [ 106.647664] EIP: drm_fb_helper_damage_work+0x371/0x390 [ 106.652305] Code: b1 17 c7 01 68 bd 5b 2d c5 53 50 68 55 21 2d c5 83 15 44 b1 17 c7 00 e8 ae bc b1 01 83 05 48 b1 17 c7 01 83 15 4c b1 17 c7 00 <0f> 0b 83 05 50 b1 17 c7 01 83 15 54 b1 17 c7 00 83 c4 10 e9 78 fd [ 106.663517] EAX: 0000002d EBX: c8730520 ECX: 00000847 EDX: 00000000 [ 106.668423] ESI: ca987000 EDI: cab274d8 EBP: f62f5f20 ESP: f62f5ee8 [ 106.673214] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010246 [ 106.678295] CR0: 80050033 CR2: 00000000 CR3: 063a7000 CR4: 000406d0 [ 106.683160] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 [ 106.687967] DR6: fffe0ff0 DR7: 00000400 [ 106.690763] Call Trace: [ 106.693394] process_one_work+0x3ea/0xaa0 [ 106.693501] ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver [ 106.695300] worker_thread+0x330/0x900 [ 106.697406] ixgbevf: Copyright (c) 2009 - 2018 Intel Corporation. [ 106.702963] kthread+0x190/0x210 [ 106.705709] ? rescuer_thread+0x650/0x650 [ 106.708379] ? kthread_insert_work_sanity_check+0x120/0x120 [ 106.711271] ret_from_fork+0x1c/0x30 [ 106.713973] ---[ end trace dd528799d3369ac1 ]---
To reproduce:
# build kernel cd linux cp config-5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 .config make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks, Oliver Sang
LKP mailing list -- lkp@lists.01.org To unsubscribe send an email to lkp-leave@lists.01.org
Hi
Am 24.11.20 um 02:58 schrieb Xing Zhengjun:
On 11/23/2020 4:04 PM, Thomas Zimmermann wrote:
Hi
Am 22.11.20 um 15:18 schrieb kernel test robot:
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 6a1b34c0a339fdc75d7932ad5702f2177c9d7a1c ("drm/fb-helper: Move damage blit code and its setup into separate routine") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-fb-helper-Var...
in testcase: trinity version: trinity-static-i386-x86_64-f93256fb_2019-08-28 with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
That dmesg is full of messages like
[ 696.323556] alloc_vmap_area: 24 callbacks suppressed [ 696.323562] vmap allocation for size 3149824 failed: use vmalloc=<size> to increase size
I think the test system needs to be reconfigured first.
We have tried "vmalloc=256M" and "vmalloc=512M", the same warning still happened.
I added a more descriptive error message before pushing the patch. This may help to find the problem's cause.
Best regards Thomas
Best regards Thomas
+-----------------------------------------------------------------------+------------+------------+
| | 154f2d1afd | 6a1b34c0a3 | +-----------------------------------------------------------------------+------------+------------+
| WARNING:at_drivers/gpu/drm/drm_fb_helper.c:#drm_fb_helper_damage_work | 0 | 36 | | EIP:drm_fb_helper_damage_work | 0 | 36 | +-----------------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 106.616652] WARNING: CPU: 1 PID: 173 at drivers/gpu/drm/drm_fb_helper.c:434 drm_fb_helper_damage_work+0x371/0x390 [ 106.627732] Modules linked in: [ 106.632419] CPU: 1 PID: 173 Comm: kworker/1:2 Not tainted 5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 #3 [ 106.637806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 106.642853] Workqueue: events drm_fb_helper_damage_work [ 106.647664] EIP: drm_fb_helper_damage_work+0x371/0x390 [ 106.652305] Code: b1 17 c7 01 68 bd 5b 2d c5 53 50 68 55 21 2d c5 83 15 44 b1 17 c7 00 e8 ae bc b1 01 83 05 48 b1 17 c7 01 83 15 4c b1 17 c7 00 <0f> 0b 83 05 50 b1 17 c7 01 83 15 54 b1 17 c7 00 83 c4 10 e9 78 fd [ 106.663517] EAX: 0000002d EBX: c8730520 ECX: 00000847 EDX: 00000000 [ 106.668423] ESI: ca987000 EDI: cab274d8 EBP: f62f5f20 ESP: f62f5ee8 [ 106.673214] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010246 [ 106.678295] CR0: 80050033 CR2: 00000000 CR3: 063a7000 CR4: 000406d0 [ 106.683160] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 [ 106.687967] DR6: fffe0ff0 DR7: 00000400 [ 106.690763] Call Trace: [ 106.693394] process_one_work+0x3ea/0xaa0 [ 106.693501] ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver [ 106.695300] worker_thread+0x330/0x900 [ 106.697406] ixgbevf: Copyright (c) 2009 - 2018 Intel Corporation. [ 106.702963] kthread+0x190/0x210 [ 106.705709] ? rescuer_thread+0x650/0x650 [ 106.708379] ? kthread_insert_work_sanity_check+0x120/0x120 [ 106.711271] ret_from_fork+0x1c/0x30 [ 106.713973] ---[ end trace dd528799d3369ac1 ]---
To reproduce:
# build kernel cd linux cp config-5.10.0-rc4-next-20201120-00007-g6a1b34c0a339 .config make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks, Oliver Sang
LKP mailing list -- lkp@lists.01.org To unsubscribe send an email to lkp-leave@lists.01.org
On Fri, Nov 20, 2020 at 11:25:42AM +0100, Thomas Zimmermann wrote:
Introduce a separate function for the blit code and its vmap setup. Done in preparation of additional changes. No functional changes are made.
v2:
- print a single warning if damage blitter fails
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Reviewed-by: Sam Ravnborg sam@ravnborg.org
If the damage handling fails, restore the damage area. The next invocation of the damage worker will then perform the update.
v2: * print a single warning if dirty callback fails (Daniel, Sebastian) * update comment
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b0bde894bf39..b36d9852cdf7 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -432,11 +432,28 @@ static void drm_fb_helper_damage_work(struct work_struct *work) if (helper->buffer) { ret = drm_fb_helper_damage_blit(helper, &clip_copy); if (drm_WARN_ON_ONCE(dev, ret)) - return; + goto err; }
- if (helper->fb->funcs->dirty) - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1); + if (helper->fb->funcs->dirty) { + ret = helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1); + if (drm_WARN_ON_ONCE(dev, ret)) + goto err; + } + + return; + +err: + /* + * Restore damage clip rectangle on errors. The next run + * of the damage worker will perform the update. + */ + spin_lock_irqsave(&helper->damage_lock, flags); + clip->x1 = min_t(u32, clip->x1, clip_copy.x1); + clip->y1 = min_t(u32, clip->y1, clip_copy.y1); + clip->x2 = max_t(u32, clip->x2, clip_copy.x2); + clip->y2 = max_t(u32, clip->y2, clip_copy.y2); + spin_unlock_irqrestore(&helper->damage_lock, flags); }
/**
On Fri, Nov 20, 2020 at 11:25:43AM +0100, Thomas Zimmermann wrote:
If the damage handling fails, restore the damage area. The next invocation of the damage worker will then perform the update.
v2:
- print a single warning if dirty callback fails (Daniel, Sebastian)
- update comment
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Acked-by: Sam Ravnborg sam@ravnborg.org
Copy the vmap()'ed instance of struct dma_buf_map before modifying it, in case the implementation of vunmap() depends on the exact address.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b36d9852cdf7..d972ce75d180 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -395,14 +395,15 @@ static int drm_fb_helper_damage_blit(struct drm_fb_helper *fb_helper, struct drm_clip_rect *clip) { struct drm_client_buffer *buffer = fb_helper->buffer; - struct dma_buf_map map; + struct dma_buf_map map, dst; int ret;
ret = drm_client_buffer_vmap(buffer, &map); if (ret) return ret;
- drm_fb_helper_damage_blit_real(fb_helper, clip, &map); + dst = map; + drm_fb_helper_damage_blit_real(fb_helper, clip, &dst);
drm_client_buffer_vunmap(buffer);
On Fri, Nov 20, 2020 at 11:25:44AM +0100, Thomas Zimmermann wrote:
Copy the vmap()'ed instance of struct dma_buf_map before modifying it, in case the implementation of vunmap() depends on the exact address.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Acked-by: Sam Ravnborg sam@ravnborg.org
Flushing the fbdev's shadow buffer requires vmap'ing the BO memory, which in turn requires pinning the BO. While being pinned, the BO cannot be moved into VRAM for scanout. Consequently, a concurrent modeset operation that involves the fbdev framebuffer would likely fail.
Resolve this problem be acquiring the modeset lock of the planes that use the fbdev framebuffer. On non-atomic drivers, also acquire the mode-config lock. This serializes the flushing of the framebuffer with concurrent modeset operations.
v2: * only acquire struct drm_fb_helper.lock in damage blitter (Daniel, Christian)
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index d972ce75d180..6f2763467e99 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -398,16 +398,32 @@ static int drm_fb_helper_damage_blit(struct drm_fb_helper *fb_helper, struct dma_buf_map map, dst; int ret;
+ /* + * We have to pin the client buffer to its current location while + * flushing the shadow buffer. In the general case, concurrent + * modesetting operations could try to move the buffer and would + * fail. The modeset has to be serialized by acquiring the reservation + * object of the underlying BO here. + * + * For fbdev emulation, we only have to protect against fbdev modeset + * operations. Nothing else will involve the client buffer's BO. So it + * is sufficient to acquire struct drm_fb_helper.lock here. + */ + mutex_lock(&fb_helper->lock); + ret = drm_client_buffer_vmap(buffer, &map); if (ret) - return ret; + goto out;
dst = map; drm_fb_helper_damage_blit_real(fb_helper, clip, &dst);
drm_client_buffer_vunmap(buffer);
- return 0; +out: + mutex_unlock(&fb_helper->lock); + + return ret; }
static void drm_fb_helper_damage_work(struct work_struct *work)
On Fri, Nov 20, 2020 at 11:25:45AM +0100, Thomas Zimmermann wrote:
Flushing the fbdev's shadow buffer requires vmap'ing the BO memory, which in turn requires pinning the BO. While being pinned, the BO cannot be moved into VRAM for scanout. Consequently, a concurrent modeset operation that involves the fbdev framebuffer would likely fail.
Resolve this problem be acquiring the modeset lock of the planes that use the fbdev framebuffer. On non-atomic drivers, also acquire the mode-config lock. This serializes the flushing of the framebuffer with concurrent modeset operations.
v2:
- only acquire struct drm_fb_helper.lock in damage blitter (Daniel, Christian)
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Acked-by: Sam Ravnborg sam@ravnborg.org
On Fri, Nov 20, 2020 at 11:25:35AM +0100, Thomas Zimmermann wrote:
Here's a number of fb-helper patches that have been piling up recently.
Patches 1 to 3 fix bugs that I spotted while going through the code. Because of the way the fbdev code works, they have been avoided so far.
Patches 4 to 10 cleanup damage handling for fbdev's shadow buffer and fix a few issues.
Specifically, the final patch adds locking to the code that flushes the shadow framebuffer into BO memory. During the conversion of radeon to generic fbdev, the question came up about interference with concurrent modesets. If fbdev has the BO pinned in system memory for flushing while the modeset wants to pin it to VRAM for scanout, the modeset would most likely fail. We haven't seen that so far, but it's possible at least. Acquiring the fb-helper lock during the flush operation prevents concurrent modesets from taking place.
The code has been tested with SHMEM and TTM BOs; with atomic and non- atomic modesetting.
For the whole series Acked-by: Maxime Ripard mripard@kernel.org
Maxime
dri-devel@lists.freedesktop.org
-
kernel test robot -
Maxime Ripard -
Sam Ravnborg -
Thomas Zimmermann -
Xing Zhengjun