DRM client buffers are permanently mapped throughout their lifetime. This prevents us from using generic framebuffer emulation for devices with small dedicated video memory, such as ast or mgag200. With fb buffers permanently mapped, such devices often won't have enough space left to display other content (e.g., X11).
This patch set introduces unmappable DRM client buffers for framebuffer emulation with shadow buffers. While the shadow buffer remains in system memory permanently, the respective buffer object will only be mapped briefly during updates from the shadow buffer. Hence, the driver can relocate he buffer object among memory regions as needed.
HW-based framebuffer consoles will still map the buffer permanently. This is a special case required by the fbdev interface, which allows for mmaping video memory to userspace. Some userspace clients rely on this functionality. Hence, the patch set also changes DRM clients to not map the buffer by default. Future DRM clients are expected to map buffers as needed.
The patch set converts ast and mgag200 to generic framebuffer emulation and removes a large amount of framebuffer code from these drivers. For bochs, a problem was reported where the driver could not display the console because it was pinned in system memory. [1] The patch set fixes this bug by converting bochs to use the shadow fb.
The patch set has been tested on ast and mga200 HW.
[1] https://lists.freedesktop.org/archives/dri-devel/2019-June/224423.html
Thomas Zimmermann (6): drm/client: Support unmapping of DRM client buffers drm/fb-helper: Map DRM client buffer only when required drm/fb-helper: Instanciate shadow FB if configured in device's mode_config drm/ast: Replace struct ast_fbdev with generic framebuffer emulation drm/bochs: Use shadow buffer for bochs framebuffer console drm/mgag200: Replace struct mga_fbdev with generic framebuffer emulation
drivers/gpu/drm/ast/Makefile | 2 +- drivers/gpu/drm/ast/ast_drv.c | 13 +- drivers/gpu/drm/ast/ast_drv.h | 17 -- drivers/gpu/drm/ast/ast_fb.c | 341 ------------------------- drivers/gpu/drm/ast/ast_main.c | 3 +- drivers/gpu/drm/ast/ast_mode.c | 21 -- drivers/gpu/drm/bochs/bochs_kms.c | 1 + drivers/gpu/drm/drm_client.c | 61 ++++- drivers/gpu/drm/drm_fb_helper.c | 52 +++- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 19 -- drivers/gpu/drm/mgag200/mgag200_fb.c | 309 ---------------------- drivers/gpu/drm/mgag200/mgag200_main.c | 31 +-- drivers/gpu/drm/mgag200/mgag200_mode.c | 27 -- include/drm/drm_client.h | 3 + include/drm/drm_mode_config.h | 5 + 16 files changed, 109 insertions(+), 798 deletions(-) delete mode 100644 drivers/gpu/drm/ast/ast_fb.c delete mode 100644 drivers/gpu/drm/mgag200/mgag200_fb.c
-- 2.21.0
DRM clients, such as the fbdev emulation, have their buffer objects mapped by default. Mapping a buffer implicitly prevents its relocation. Hence, the buffer may permanently consume video memory while it's allocated. This is a problem for drivers of low-memory devices, such as ast, mgag200 or older framebuffer hardware, which will then not have enough memory to display other content (e.g., X11).
This patch introduces drm_client_buffer_vmap() and _vunmap(). Internal DRM clients can use these functions to unmap and remap buffer objects as needed.
There's no reference counting for vmap operations. Callers are expected to either keep buffers mapped (as it is now), or call vmap and vunmap in pairs around code that accesses the mapped memory.
v2: * remove several duplicated NULL-pointer checks
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_client.c | 67 ++++++++++++++++++++++++++++++------ include/drm/drm_client.h | 3 ++ 2 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 410572f14257..66d8d645ac79 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -281,6 +281,43 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
buffer->gem = obj;
+ vaddr = drm_client_buffer_vmap(buffer); + if (IS_ERR(vaddr)) { + ret = PTR_ERR(vaddr); + goto err_delete; + } + + return buffer; + +err_delete: + drm_client_buffer_delete(buffer); + + return ERR_PTR(ret); +} + +/** + * drm_client_buffer_vmap - Map DRM client buffer into address space + * @buffer: DRM client buffer + * + * This function maps a client buffer into kernel address space. If the + * buffer is already mapped, it returns the mapping's address. + * + * Client buffer mappings are not ref'counted. Each call to + * drm_client_buffer_vmap() should be followed by a call to + * drm_client_buffer_vunmap(); or the client buffer should be mapped + * throughout its lifetime. The latter is the default. + * + * Returns: + * The mapped memory's address + */ +void * +drm_client_buffer_vmap(struct drm_client_buffer *buffer) +{ + void *vaddr; + + if (buffer->vaddr) + return buffer->vaddr; + /* * FIXME: The dependency on GEM here isn't required, we could * convert the driver handle to a dma-buf instead and use the @@ -289,21 +326,31 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u * fd_install step out of the driver backend hooks, to make that * final step optional for internal users. */ - vaddr = drm_gem_vmap(obj); - if (IS_ERR(vaddr)) { - ret = PTR_ERR(vaddr); - goto err_delete; - } + vaddr = drm_gem_vmap(buffer->gem); + if (IS_ERR(vaddr)) + return vaddr;
buffer->vaddr = vaddr;
- return buffer; - -err_delete: - drm_client_buffer_delete(buffer); + return vaddr; +} +EXPORT_SYMBOL(drm_client_buffer_vmap);
- return ERR_PTR(ret); +/** + * drm_client_buffer_vunmap - Unmap DRM client buffer + * @buffer: DRM client buffer + * + * This function removes a client buffer's memory mmapping. This + * function is only required by clients that manage their buffers + * by themselves. By default, DRM client buffers are mapped throughout + * their entire lifetime. + */ +void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) +{ + drm_gem_vunmap(buffer->gem, buffer->vaddr); + buffer->vaddr = NULL; } +EXPORT_SYMBOL(drm_client_buffer_vunmap);
static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer) { diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h index 72d51d1e9dd9..e1db1d9da0bf 100644 --- a/include/drm/drm_client.h +++ b/include/drm/drm_client.h @@ -149,6 +149,9 @@ struct drm_client_buffer { struct drm_client_buffer * drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format); void drm_client_framebuffer_delete(struct drm_client_buffer *buffer); +void * +drm_client_buffer_vmap(struct drm_client_buffer *buffer); +void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
int drm_client_modeset_create(struct drm_client_dev *client); void drm_client_modeset_free(struct drm_client_dev *client);
Den 05.07.2019 11.26, skrev Thomas Zimmermann:
DRM clients, such as the fbdev emulation, have their buffer objects mapped by default. Mapping a buffer implicitly prevents its relocation. Hence, the buffer may permanently consume video memory while it's allocated. This is a problem for drivers of low-memory devices, such as ast, mgag200 or older framebuffer hardware, which will then not have enough memory to display other content (e.g., X11).
This patch introduces drm_client_buffer_vmap() and _vunmap(). Internal DRM clients can use these functions to unmap and remap buffer objects as needed.
There's no reference counting for vmap operations. Callers are expected to either keep buffers mapped (as it is now), or call vmap and vunmap in pairs around code that accesses the mapped memory.
v2:
- remove several duplicated NULL-pointer checks
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_client.c | 67 ++++++++++++++++++++++++++++++------ include/drm/drm_client.h | 3 ++ 2 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 410572f14257..66d8d645ac79 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -281,6 +281,43 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
buffer->gem = obj;
- vaddr = drm_client_buffer_vmap(buffer);
- if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);goto err_delete;- }
- return buffer;
+err_delete:
- drm_client_buffer_delete(buffer);
- return ERR_PTR(ret);
+}
+/**
- drm_client_buffer_vmap - Map DRM client buffer into address space
- @buffer: DRM client buffer
- This function maps a client buffer into kernel address space. If the
- buffer is already mapped, it returns the mapping's address.
- Client buffer mappings are not ref'counted. Each call to
- drm_client_buffer_vmap() should be followed by a call to
- drm_client_buffer_vunmap(); or the client buffer should be mapped
- throughout its lifetime. The latter is the default.
- Returns:
- The mapped memory's address
- */
+void * +drm_client_buffer_vmap(struct drm_client_buffer *buffer)
I prefer to have this on one line.
+{
- void *vaddr;
- if (buffer->vaddr)
return buffer->vaddr;- /*
- FIXME: The dependency on GEM here isn't required, we could
- convert the driver handle to a dma-buf instead and use the
@@ -289,21 +326,31 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u * fd_install step out of the driver backend hooks, to make that * final step optional for internal users. */
- vaddr = drm_gem_vmap(obj);
- if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);goto err_delete;- }
vaddr = drm_gem_vmap(buffer->gem);
if (IS_ERR(vaddr))
return vaddr;buffer->vaddr = vaddr;
- return buffer;
-err_delete:
- drm_client_buffer_delete(buffer);
- return vaddr;
+} +EXPORT_SYMBOL(drm_client_buffer_vmap);
- return ERR_PTR(ret);
+/**
- drm_client_buffer_vunmap - Unmap DRM client buffer
- @buffer: DRM client buffer
- This function removes a client buffer's memory mmapping. This
s/mmapping/mapping/
- function is only required by clients that manage their buffers
- by themselves. By default, DRM client buffers are mapped throughout
- their entire lifetime.
- */
+void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) +{
- drm_gem_vunmap(buffer->gem, buffer->vaddr);
- buffer->vaddr = NULL;
} +EXPORT_SYMBOL(drm_client_buffer_vunmap);
static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer) { diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h index 72d51d1e9dd9..e1db1d9da0bf 100644 --- a/include/drm/drm_client.h +++ b/include/drm/drm_client.h @@ -149,6 +149,9 @@ struct drm_client_buffer { struct drm_client_buffer * drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format); void drm_client_framebuffer_delete(struct drm_client_buffer *buffer); +void * +drm_client_buffer_vmap(struct drm_client_buffer *buffer);
Prefer to have this on one line.
Reviewed-by: Noralf Trønnes noralf@tronnes.org
+void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
int drm_client_modeset_create(struct drm_client_dev *client); void drm_client_modeset_free(struct drm_client_dev *client);
This patch changes DRM clients to not map the buffer by default. The buffer, like any buffer object, should be mapped and unmapped when needed.
An unmapped buffer object can be evicted to system memory and does not consume video ram until displayed. This allows to use generic fbdev emulation with drivers for low-memory devices, such as ast and mgag200.
This change affects the generic framebuffer console. HW-based consoles map their console buffer once and keep it mapped. Userspace can mmap this buffer into its address space. The shadow-buffered framebuffer console only needs the buffer object to be mapped during updates. While not being updated from the shadow buffer, the buffer object can remain unmapped. Userspace will always mmap the shadow buffer.
v2: * change DRM client to not map buffer by default * manually map client buffer for fbdev with HW framebuffer
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_client.c | 16 ++++------------ drivers/gpu/drm/drm_fb_helper.c | 33 +++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 66d8d645ac79..7d23c834f503 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -254,7 +254,6 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u struct drm_device *dev = client->dev; struct drm_client_buffer *buffer; struct drm_gem_object *obj; - void *vaddr; int ret;
buffer = kzalloc(sizeof(*buffer), GFP_KERNEL); @@ -281,12 +280,6 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
buffer->gem = obj;
- vaddr = drm_client_buffer_vmap(buffer); - if (IS_ERR(vaddr)) { - ret = PTR_ERR(vaddr); - goto err_delete; - } - return buffer;
err_delete: @@ -305,7 +298,7 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u * Client buffer mappings are not ref'counted. Each call to * drm_client_buffer_vmap() should be followed by a call to * drm_client_buffer_vunmap(); or the client buffer should be mapped - * throughout its lifetime. The latter is the default. + * throughout its lifetime. * * Returns: * The mapped memory's address @@ -340,10 +333,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap); * drm_client_buffer_vunmap - Unmap DRM client buffer * @buffer: DRM client buffer * - * This function removes a client buffer's memory mmapping. This - * function is only required by clients that manage their buffers - * by themselves. By default, DRM client buffers are mapped throughout - * their entire lifetime. + * This function removes a client buffer's memory mmapping. Calling this + * function is only required by clients that manage their buffer mappings + * by themselves. */ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) { diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 1984e5c54d58..7ba6a0255821 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -403,6 +403,7 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) struct drm_clip_rect *clip = &helper->dirty_clip; struct drm_clip_rect clip_copy; unsigned long flags; + void *vaddr;
spin_lock_irqsave(&helper->dirty_lock, flags); clip_copy = *clip; @@ -412,10 +413,18 @@ static void drm_fb_helper_dirty_work(struct work_struct *work)
/* 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) + if (helper->buffer) { + vaddr = drm_client_buffer_vmap(helper->buffer); + if (IS_ERR(vaddr)) + return; drm_fb_helper_dirty_blit_real(helper, &clip_copy); + } helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1); + + if (helper->buffer) + drm_client_buffer_vunmap(helper->buffer); } }
@@ -2178,6 +2187,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, struct drm_framebuffer *fb; struct fb_info *fbi; u32 format; + void *vaddr;
DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n", sizes->surface_width, sizes->surface_height, @@ -2200,13 +2210,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, fbi->fbops = &drm_fbdev_fb_ops; fbi->screen_size = fb->height * fb->pitches[0]; fbi->fix.smem_len = fbi->screen_size; - fbi->screen_buffer = buffer->vaddr; - /* Shamelessly leak the physical address to user-space */ -#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) - if (drm_leak_fbdev_smem && fbi->fix.smem_start == 0) - fbi->fix.smem_start = - page_to_phys(virt_to_page(fbi->screen_buffer)); -#endif + drm_fb_helper_fill_info(fbi, fb_helper, sizes);
if (fb->funcs->dirty) { @@ -2231,6 +2235,19 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper, fbi->fbdefio = &drm_fbdev_defio;
fb_deferred_io_init(fbi); + } else { + /* buffer is mapped for HW framebuffer */ + vaddr = drm_client_buffer_vmap(fb_helper->buffer); + if (IS_ERR(vaddr)) + return PTR_ERR(vaddr); + + fbi->screen_buffer = vaddr; + /* Shamelessly leak the physical address to user-space */ +#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) + if (drm_leak_fbdev_smem && fbi->fix.smem_start == 0) + fbi->fix.smem_start = + page_to_phys(virt_to_page(fbi->screen_buffer)); +#endif }
return 0;
Den 05.07.2019 11.26, skrev Thomas Zimmermann:
This patch changes DRM clients to not map the buffer by default. The buffer, like any buffer object, should be mapped and unmapped when needed.
An unmapped buffer object can be evicted to system memory and does not consume video ram until displayed. This allows to use generic fbdev emulation with drivers for low-memory devices, such as ast and mgag200.
This change affects the generic framebuffer console. HW-based consoles map their console buffer once and keep it mapped. Userspace can mmap this buffer into its address space. The shadow-buffered framebuffer console only needs the buffer object to be mapped during updates. While not being updated from the shadow buffer, the buffer object can remain unmapped. Userspace will always mmap the shadow buffer.
v2:
- change DRM client to not map buffer by default
- manually map client buffer for fbdev with HW framebuffer
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
Reviewed-by: Noralf Trønnes noralf@tronnes.org
Generic framebuffer emulation uses a shadow buffer for framebuffers with dirty() function. If drivers want to use the shadow FB without such a function, they can now set prefer_shadow or prefer_shadow_fbdev in their mode_config structures. The former flag is exported to userspace, the latter flag is fbdev-only.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++----- include/drm/drm_mode_config.h | 5 +++++ 2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 7ba6a0255821..56ef169e1814 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -421,7 +421,9 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) return; drm_fb_helper_dirty_blit_real(helper, &clip_copy); } - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1); + 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); @@ -620,9 +622,6 @@ static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, struct drm_clip_rect *clip = &helper->dirty_clip; unsigned long flags;
- if (!helper->fb->funcs->dirty) - return; - spin_lock_irqsave(&helper->dirty_lock, flags); clip->x1 = min_t(u32, clip->x1, x); clip->y1 = min_t(u32, clip->y1, y); @@ -2166,6 +2165,16 @@ static struct fb_deferred_io drm_fbdev_defio = { .deferred_io = drm_fb_helper_deferred_io, };
+static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper) +{ + struct drm_device *dev = fb_helper->dev; + struct drm_framebuffer *fb = fb_helper->fb; + + return dev->mode_config.prefer_shadow_fbdev | + dev->mode_config.prefer_shadow | + !!fb->funcs->dirty; +} + /** * drm_fb_helper_generic_probe - Generic fbdev emulation probe helper * @fb_helper: fbdev helper structure @@ -2213,7 +2222,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
drm_fb_helper_fill_info(fbi, fb_helper, sizes);
- if (fb->funcs->dirty) { + if (drm_fbdev_use_shadow_fb(fb_helper)) { struct fb_ops *fbops; void *shadow;
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 759d462d028b..e1c751aca353 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -347,6 +347,8 @@ struct drm_mode_config_funcs { * @output_poll_work: delayed work for polling in process context * @preferred_depth: preferred RBG pixel depth, used by fb helpers * @prefer_shadow: hint to userspace to prefer shadow-fb rendering + * @prefer_shadow_fbdev: hint to framebuffer emulation to prefer shadow-fb \ + rendering * @cursor_width: hint to userspace for max cursor width * @cursor_height: hint to userspace for max cursor height * @helper_private: mid-layer private data @@ -852,6 +854,9 @@ struct drm_mode_config { /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
+ /* fbdev parameters */ + uint32_t prefer_shadow_fbdev; + /** * @quirk_addfb_prefer_xbgr_30bpp: *
Den 05.07.2019 11.26, skrev Thomas Zimmermann:
Generic framebuffer emulation uses a shadow buffer for framebuffers with dirty() function. If drivers want to use the shadow FB without such a function, they can now set prefer_shadow or prefer_shadow_fbdev in their mode_config structures. The former flag is exported to userspace, the latter flag is fbdev-only.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++----- include/drm/drm_mode_config.h | 5 +++++ 2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 7ba6a0255821..56ef169e1814 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -421,7 +421,9 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) return; drm_fb_helper_dirty_blit_real(helper, &clip_copy); }
helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
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);
@@ -620,9 +622,6 @@ static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, struct drm_clip_rect *clip = &helper->dirty_clip; unsigned long flags;
- if (!helper->fb->funcs->dirty)
return;
drm_fb_helper_dirty() is called unconditionally by drm_fb_helper_sys_imageblit() et al, so we need check with drm_fbdev_use_shadow_fb() here.
- spin_lock_irqsave(&helper->dirty_lock, flags); clip->x1 = min_t(u32, clip->x1, x); clip->y1 = min_t(u32, clip->y1, y);
@@ -2166,6 +2165,16 @@ static struct fb_deferred_io drm_fbdev_defio = { .deferred_io = drm_fb_helper_deferred_io, };
+static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper) +{
- struct drm_device *dev = fb_helper->dev;
- struct drm_framebuffer *fb = fb_helper->fb;
- return dev->mode_config.prefer_shadow_fbdev |
dev->mode_config.prefer_shadow |
Use logical OR here
!!fb->funcs->dirty;
and you can drop the the double NOT here.
+}
/**
- drm_fb_helper_generic_probe - Generic fbdev emulation probe helper
- @fb_helper: fbdev helper structure
@@ -2213,7 +2222,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
drm_fb_helper_fill_info(fbi, fb_helper, sizes);
- if (fb->funcs->dirty) {
- if (drm_fbdev_use_shadow_fb(fb_helper)) { struct fb_ops *fbops; void *shadow;
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 759d462d028b..e1c751aca353 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -347,6 +347,8 @@ struct drm_mode_config_funcs {
- @output_poll_work: delayed work for polling in process context
- @preferred_depth: preferred RBG pixel depth, used by fb helpers
- @prefer_shadow: hint to userspace to prefer shadow-fb rendering
- @prefer_shadow_fbdev: hint to framebuffer emulation to prefer shadow-fb \
- rendering
It's preferred to have the doc together with the struct member. This way it's less likely to be forgotten when things change. And we don't use line cont. when the doc line is too long. Just continue on the next line after an asterix.
- @cursor_width: hint to userspace for max cursor width
- @cursor_height: hint to userspace for max cursor height
- @helper_private: mid-layer private data
@@ -852,6 +854,9 @@ struct drm_mode_config { /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
- /* fbdev parameters */
No need for this comment.
Doc can look like this, I've done s/framebuffer/fbdev/: /** * @prefer_shadow_fbdev: * * Hint to fbdev emulation to prefer shadow-fb rendering. */
- uint32_t prefer_shadow_fbdev;
Use bool here.
With that:
Reviewed-by: Noralf Trønnes noralf@tronnes.org
I have tested this on 2 drivers that use generic fbdev: vc4 (no shadow buf) and mi0283qt which has a dirty callback.
Tested-by: Noralf Trønnes noralf@tronnes.org
- /**
- @quirk_addfb_prefer_xbgr_30bpp:
Hi
Am 07.07.19 um 16:37 schrieb Noralf Trønnes:
Den 05.07.2019 11.26, skrev Thomas Zimmermann:
Generic framebuffer emulation uses a shadow buffer for framebuffers with dirty() function. If drivers want to use the shadow FB without such a function, they can now set prefer_shadow or prefer_shadow_fbdev in their mode_config structures. The former flag is exported to userspace, the latter flag is fbdev-only.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++----- include/drm/drm_mode_config.h | 5 +++++ 2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 7ba6a0255821..56ef169e1814 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -421,7 +421,9 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) return; drm_fb_helper_dirty_blit_real(helper, &clip_copy); }
helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
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);
@@ -620,9 +622,6 @@ static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, struct drm_clip_rect *clip = &helper->dirty_clip; unsigned long flags;
- if (!helper->fb->funcs->dirty)
return;drm_fb_helper_dirty() is called unconditionally by drm_fb_helper_sys_imageblit() et al, so we need check with drm_fbdev_use_shadow_fb() here.
- spin_lock_irqsave(&helper->dirty_lock, flags); clip->x1 = min_t(u32, clip->x1, x); clip->y1 = min_t(u32, clip->y1, y);
@@ -2166,6 +2165,16 @@ static struct fb_deferred_io drm_fbdev_defio = { .deferred_io = drm_fb_helper_deferred_io, };
+static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper) +{
- struct drm_device *dev = fb_helper->dev;
- struct drm_framebuffer *fb = fb_helper->fb;
- return dev->mode_config.prefer_shadow_fbdev |
dev->mode_config.prefer_shadow |Use logical OR here
!!fb->funcs->dirty;and you can drop the the double NOT here.
+}
/**
- drm_fb_helper_generic_probe - Generic fbdev emulation probe helper
- @fb_helper: fbdev helper structure
@@ -2213,7 +2222,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
drm_fb_helper_fill_info(fbi, fb_helper, sizes);
- if (fb->funcs->dirty) {
- if (drm_fbdev_use_shadow_fb(fb_helper)) { struct fb_ops *fbops; void *shadow;
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 759d462d028b..e1c751aca353 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -347,6 +347,8 @@ struct drm_mode_config_funcs {
- @output_poll_work: delayed work for polling in process context
- @preferred_depth: preferred RBG pixel depth, used by fb helpers
- @prefer_shadow: hint to userspace to prefer shadow-fb rendering
- @prefer_shadow_fbdev: hint to framebuffer emulation to prefer shadow-fb \
- rendering
It's preferred to have the doc together with the struct member.
I just tried to follow the file's existing style, but OK, I don't mind.
it's less likely to be forgotten when things change. And we don't use line cont. when the doc line is too long. Just continue on the next line after an asterix.
- @cursor_width: hint to userspace for max cursor width
- @cursor_height: hint to userspace for max cursor height
- @helper_private: mid-layer private data
@@ -852,6 +854,9 @@ struct drm_mode_config { /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
- /* fbdev parameters */
No need for this comment.
Doc can look like this, I've done s/framebuffer/fbdev/: /** * @prefer_shadow_fbdev: * * Hint to fbdev emulation to prefer shadow-fb rendering. */
- uint32_t prefer_shadow_fbdev;
Use bool here.
With that:
Reviewed-by: Noralf Trønnes noralf@tronnes.org
I have tested this on 2 drivers that use generic fbdev: vc4 (no shadow buf) and mi0283qt which has a dirty callback.
Tested-by: Noralf Trønnes noralf@tronnes.org
Thanks for reviewing and testing the patches.
Best regards Thomas
- /**
- @quirk_addfb_prefer_xbgr_30bpp:
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Sun, Jul 07, 2019 at 06:14:50PM +0200, Thomas Zimmermann wrote:
Hi
Am 07.07.19 um 16:37 schrieb Noralf Trønnes:
Den 05.07.2019 11.26, skrev Thomas Zimmermann:
Generic framebuffer emulation uses a shadow buffer for framebuffers with dirty() function. If drivers want to use the shadow FB without such a function, they can now set prefer_shadow or prefer_shadow_fbdev in their mode_config structures. The former flag is exported to userspace, the latter flag is fbdev-only.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++----- include/drm/drm_mode_config.h | 5 +++++ 2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 7ba6a0255821..56ef169e1814 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -421,7 +421,9 @@ static void drm_fb_helper_dirty_work(struct work_struct *work) return; drm_fb_helper_dirty_blit_real(helper, &clip_copy); }
helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
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);
@@ -620,9 +622,6 @@ static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, struct drm_clip_rect *clip = &helper->dirty_clip; unsigned long flags;
- if (!helper->fb->funcs->dirty)
return;drm_fb_helper_dirty() is called unconditionally by drm_fb_helper_sys_imageblit() et al, so we need check with drm_fbdev_use_shadow_fb() here.
- spin_lock_irqsave(&helper->dirty_lock, flags); clip->x1 = min_t(u32, clip->x1, x); clip->y1 = min_t(u32, clip->y1, y);
@@ -2166,6 +2165,16 @@ static struct fb_deferred_io drm_fbdev_defio = { .deferred_io = drm_fb_helper_deferred_io, };
+static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper) +{
- struct drm_device *dev = fb_helper->dev;
- struct drm_framebuffer *fb = fb_helper->fb;
- return dev->mode_config.prefer_shadow_fbdev |
dev->mode_config.prefer_shadow |Use logical OR here
!!fb->funcs->dirty;and you can drop the the double NOT here.
+}
/**
- drm_fb_helper_generic_probe - Generic fbdev emulation probe helper
- @fb_helper: fbdev helper structure
@@ -2213,7 +2222,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
drm_fb_helper_fill_info(fbi, fb_helper, sizes);
- if (fb->funcs->dirty) {
- if (drm_fbdev_use_shadow_fb(fb_helper)) { struct fb_ops *fbops; void *shadow;
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 759d462d028b..e1c751aca353 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -347,6 +347,8 @@ struct drm_mode_config_funcs {
- @output_poll_work: delayed work for polling in process context
- @preferred_depth: preferred RBG pixel depth, used by fb helpers
- @prefer_shadow: hint to userspace to prefer shadow-fb rendering
- @prefer_shadow_fbdev: hint to framebuffer emulation to prefer shadow-fb \
- rendering
It's preferred to have the doc together with the struct member.
I just tried to follow the file's existing style, but OK, I don't mind.
If it annoys you too much, convert all the member docs in the header comment into the inline style. That's what I usually do when the first inline kerneldoc comment is warranted (like here), but not yet rolled out.
Or just mix&match if you feel like not doing that, we have lots of that already :-) -Daniel
it's less likely to be forgotten when things change. And we don't use line cont. when the doc line is too long. Just continue on the next line after an asterix.
- @cursor_width: hint to userspace for max cursor width
- @cursor_height: hint to userspace for max cursor height
- @helper_private: mid-layer private data
@@ -852,6 +854,9 @@ struct drm_mode_config { /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
- /* fbdev parameters */
No need for this comment.
Doc can look like this, I've done s/framebuffer/fbdev/: /** * @prefer_shadow_fbdev: * * Hint to fbdev emulation to prefer shadow-fb rendering. */
- uint32_t prefer_shadow_fbdev;
Use bool here.
With that:
Reviewed-by: Noralf Trønnes noralf@tronnes.org
I have tested this on 2 drivers that use generic fbdev: vc4 (no shadow buf) and mi0283qt which has a dirty callback.
Tested-by: Noralf Trønnes noralf@tronnes.org
Thanks for reviewing and testing the patches.
Best regards Thomas
- /**
- @quirk_addfb_prefer_xbgr_30bpp:
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)
This patch replaces ast's framebuffer console with DRM's generic implememtation. All respective code is being removed from the driver.
The console is set up with a shadow buffer. The actual buffer object is not permanently pinned in video ram, but just another buffer object that the driver moves in and out of vram as necessary. The driver's function ast_crtc_do_set_base() used to contain special handling for the framebuffer console. With the new generic framebuffer, the driver does not need this code an longer.
v2: * use drm_fb_helper_set_suspend_unlocked() in ast_drm_{thaw,freeze}() * dirty function no longer required
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Acked-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/ast/Makefile | 2 +- drivers/gpu/drm/ast/ast_drv.c | 13 +- drivers/gpu/drm/ast/ast_drv.h | 17 -- drivers/gpu/drm/ast/ast_fb.c | 341 --------------------------------- drivers/gpu/drm/ast/ast_main.c | 3 +- drivers/gpu/drm/ast/ast_mode.c | 21 -- 6 files changed, 5 insertions(+), 392 deletions(-) delete mode 100644 drivers/gpu/drm/ast/ast_fb.c
diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile index b086dae17013..561f7c4199e4 100644 --- a/drivers/gpu/drm/ast/Makefile +++ b/drivers/gpu/drm/ast/Makefile @@ -3,6 +3,6 @@ # Makefile for the drm device driver. This driver provides support for the # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
-ast-y := ast_drv.o ast_main.o ast_mode.o ast_fb.o ast_ttm.o ast_post.o ast_dp501.o +ast-y := ast_drv.o ast_main.o ast_mode.o ast_ttm.o ast_post.o ast_dp501.o
obj-$(CONFIG_DRM_AST) := ast.o diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index 3811997e78c4..2c977d0ac8fe 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -100,28 +100,21 @@ ast_pci_remove(struct pci_dev *pdev) static int ast_drm_freeze(struct drm_device *dev) { drm_kms_helper_poll_disable(dev); - pci_save_state(dev->pdev); + drm_fb_helper_set_suspend_unlocked(dev->fb_helper, true);
- console_lock(); - ast_fbdev_set_suspend(dev, 1); - console_unlock(); return 0; }
static int ast_drm_thaw(struct drm_device *dev) { - int error = 0; - ast_post_gpu(dev);
drm_mode_config_reset(dev); drm_helper_resume_force_mode(dev); + drm_fb_helper_set_suspend_unlocked(dev->fb_helper, false);
- console_lock(); - ast_fbdev_set_suspend(dev, 0); - console_unlock(); - return error; + return 0; }
static int ast_drm_resume(struct drm_device *dev) diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index ca794a3fcf8f..907d7480b7ba 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -81,8 +81,6 @@ enum ast_tx_chip { #define AST_DRAM_4Gx16 7 #define AST_DRAM_8Gx16 8
-struct ast_fbdev; - struct ast_private { struct drm_device *dev;
@@ -96,8 +94,6 @@ struct ast_private { uint32_t mclk; uint32_t vram_size;
- struct ast_fbdev *fbdev; - int fb_mtrr;
struct drm_gem_object *cursor_cache; @@ -239,14 +235,6 @@ struct ast_encoder { struct drm_encoder base; };
-struct ast_fbdev { - struct drm_fb_helper helper; /* must be first */ - void *sysram; - int size; - int x1, y1, x2, y2; /* dirty rect */ - spinlock_t dirty_lock; -}; - #define to_ast_crtc(x) container_of(x, struct ast_crtc, base) #define to_ast_connector(x) container_of(x, struct ast_connector, base) #define to_ast_encoder(x) container_of(x, struct ast_encoder, base) @@ -289,11 +277,6 @@ struct ast_vbios_mode_info { extern int ast_mode_init(struct drm_device *dev); extern void ast_mode_fini(struct drm_device *dev);
-int ast_fbdev_init(struct drm_device *dev); -void ast_fbdev_fini(struct drm_device *dev); -void ast_fbdev_set_suspend(struct drm_device *dev, int state); -void ast_fbdev_set_base(struct ast_private *ast, unsigned long gpu_addr); - #define AST_MM_ALIGN_SHIFT 4 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c deleted file mode 100644 index a849e58b40bc..000000000000 --- a/drivers/gpu/drm/ast/ast_fb.c +++ /dev/null @@ -1,341 +0,0 @@ -/* - * Copyright 2012 Red Hat Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the - * "Software"), to deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, merge, publish, - * distribute, sub license, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to - * the following conditions: - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, - * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE - * USE OR OTHER DEALINGS IN THE SOFTWARE. - * - * The above copyright notice and this permission notice (including the - * next paragraph) shall be included in all copies or substantial portions - * of the Software. - * - */ -/* - * Authors: Dave Airlie airlied@redhat.com - */ -#include <linux/module.h> -#include <linux/kernel.h> -#include <linux/errno.h> -#include <linux/string.h> -#include <linux/mm.h> -#include <linux/tty.h> -#include <linux/sysrq.h> -#include <linux/delay.h> -#include <linux/init.h> - - -#include <drm/drmP.h> -#include <drm/drm_crtc.h> -#include <drm/drm_crtc_helper.h> -#include <drm/drm_fb_helper.h> -#include <drm/drm_gem_framebuffer_helper.h> -#include <drm/drm_util.h> - -#include "ast_drv.h" - -static void ast_dirty_update(struct ast_fbdev *afbdev, - int x, int y, int width, int height) -{ - int i; - struct drm_gem_vram_object *gbo; - int src_offset, dst_offset; - int ret; - u8 *dst; - bool unmap = false; - bool store_for_later = false; - int x2, y2; - unsigned long flags; - struct drm_framebuffer *fb = afbdev->helper.fb; - int bpp = fb->format->cpp[0]; - - gbo = drm_gem_vram_of_gem(fb->obj[0]); - - if (drm_can_sleep()) { - /* We pin the BO so it won't be moved during the - * update. The actual location, video RAM or system - * memory, is not important. - */ - ret = drm_gem_vram_pin(gbo, 0); - if (ret) { - if (ret != -EBUSY) - return; - store_for_later = true; - } - } else { - store_for_later = true; - } - - x2 = x + width - 1; - y2 = y + height - 1; - spin_lock_irqsave(&afbdev->dirty_lock, flags); - - if (afbdev->y1 < y) - y = afbdev->y1; - if (afbdev->y2 > y2) - y2 = afbdev->y2; - if (afbdev->x1 < x) - x = afbdev->x1; - if (afbdev->x2 > x2) - x2 = afbdev->x2; - - if (store_for_later) { - afbdev->x1 = x; - afbdev->x2 = x2; - afbdev->y1 = y; - afbdev->y2 = y2; - spin_unlock_irqrestore(&afbdev->dirty_lock, flags); - return; - } - - afbdev->x1 = afbdev->y1 = INT_MAX; - afbdev->x2 = afbdev->y2 = 0; - spin_unlock_irqrestore(&afbdev->dirty_lock, flags); - - dst = drm_gem_vram_kmap(gbo, false, NULL); - if (IS_ERR(dst)) { - DRM_ERROR("failed to kmap fb updates\n"); - goto out; - } else if (!dst) { - dst = drm_gem_vram_kmap(gbo, true, NULL); - if (IS_ERR(dst)) { - DRM_ERROR("failed to kmap fb updates\n"); - goto out; - } - unmap = true; - } - - for (i = y; i <= y2; i++) { - /* assume equal stride for now */ - src_offset = dst_offset = i * fb->pitches[0] + (x * bpp); - memcpy_toio(dst + dst_offset, afbdev->sysram + src_offset, - (x2 - x + 1) * bpp); - } - - if (unmap) - drm_gem_vram_kunmap(gbo); - -out: - drm_gem_vram_unpin(gbo); -} - -static void ast_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct ast_fbdev *afbdev = info->par; - drm_fb_helper_sys_fillrect(info, rect); - ast_dirty_update(afbdev, rect->dx, rect->dy, rect->width, - rect->height); -} - -static void ast_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - struct ast_fbdev *afbdev = info->par; - drm_fb_helper_sys_copyarea(info, area); - ast_dirty_update(afbdev, area->dx, area->dy, area->width, - area->height); -} - -static void ast_imageblit(struct fb_info *info, - const struct fb_image *image) -{ - struct ast_fbdev *afbdev = info->par; - drm_fb_helper_sys_imageblit(info, image); - ast_dirty_update(afbdev, image->dx, image->dy, image->width, - image->height); -} - -static struct fb_ops astfb_ops = { - .owner = THIS_MODULE, - .fb_check_var = drm_fb_helper_check_var, - .fb_set_par = drm_fb_helper_set_par, - .fb_fillrect = ast_fillrect, - .fb_copyarea = ast_copyarea, - .fb_imageblit = ast_imageblit, - .fb_pan_display = drm_fb_helper_pan_display, - .fb_blank = drm_fb_helper_blank, - .fb_setcmap = drm_fb_helper_setcmap, -}; - -static int astfb_create_object(struct ast_fbdev *afbdev, - const struct drm_mode_fb_cmd2 *mode_cmd, - struct drm_gem_object **gobj_p) -{ - struct drm_device *dev = afbdev->helper.dev; - u32 size; - struct drm_gem_object *gobj; - int ret = 0; - - size = mode_cmd->pitches[0] * mode_cmd->height; - ret = ast_gem_create(dev, size, true, &gobj); - if (ret) - return ret; - - *gobj_p = gobj; - return ret; -} - -static int astfb_create(struct drm_fb_helper *helper, - struct drm_fb_helper_surface_size *sizes) -{ - struct ast_fbdev *afbdev = - container_of(helper, struct ast_fbdev, helper); - struct drm_device *dev = afbdev->helper.dev; - struct drm_mode_fb_cmd2 mode_cmd; - struct drm_framebuffer *fb; - struct fb_info *info; - int size, ret; - void *sysram; - struct drm_gem_object *gobj = NULL; - mode_cmd.width = sizes->surface_width; - mode_cmd.height = sizes->surface_height; - mode_cmd.pitches[0] = mode_cmd.width * ((sizes->surface_bpp + 7)/8); - - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, - sizes->surface_depth); - - size = mode_cmd.pitches[0] * mode_cmd.height; - - ret = astfb_create_object(afbdev, &mode_cmd, &gobj); - if (ret) { - DRM_ERROR("failed to create fbcon backing object %d\n", ret); - return ret; - } - - sysram = vmalloc(size); - if (!sysram) - return -ENOMEM; - - info = drm_fb_helper_alloc_fbi(helper); - if (IS_ERR(info)) { - ret = PTR_ERR(info); - goto out; - } - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, gobj, NULL); - if (IS_ERR(fb)) { - ret = PTR_ERR(fb); - goto out; - } - - afbdev->sysram = sysram; - afbdev->size = size; - - afbdev->helper.fb = fb; - - info->fbops = &astfb_ops; - - info->apertures->ranges[0].base = pci_resource_start(dev->pdev, 0); - info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0); - - drm_fb_helper_fill_info(info, &afbdev->helper, sizes); - - info->screen_base = sysram; - info->screen_size = size; - - info->pixmap.flags = FB_PIXMAP_SYSTEM; - - DRM_DEBUG_KMS("allocated %dx%d\n", - fb->width, fb->height); - - return 0; - -out: - vfree(sysram); - return ret; -} - -static const struct drm_fb_helper_funcs ast_fb_helper_funcs = { - .fb_probe = astfb_create, -}; - -static void ast_fbdev_destroy(struct drm_device *dev, - struct ast_fbdev *afbdev) -{ - drm_helper_force_disable_all(dev); - drm_fb_helper_unregister_fbi(&afbdev->helper); - - drm_fb_helper_fini(&afbdev->helper); - drm_framebuffer_put(afbdev->helper.fb); - - vfree(afbdev->sysram); -} - -int ast_fbdev_init(struct drm_device *dev) -{ - struct ast_private *ast = dev->dev_private; - struct ast_fbdev *afbdev; - int ret; - - afbdev = kzalloc(sizeof(struct ast_fbdev), GFP_KERNEL); - if (!afbdev) - return -ENOMEM; - - ast->fbdev = afbdev; - spin_lock_init(&afbdev->dirty_lock); - - drm_fb_helper_prepare(dev, &afbdev->helper, &ast_fb_helper_funcs); - - ret = drm_fb_helper_init(dev, &afbdev->helper, 1); - if (ret) - goto free; - - ret = drm_fb_helper_single_add_all_connectors(&afbdev->helper); - if (ret) - goto fini; - - /* disable all the possible outputs/crtcs before entering KMS mode */ - drm_helper_disable_unused_functions(dev); - - ret = drm_fb_helper_initial_config(&afbdev->helper, 32); - if (ret) - goto fini; - - return 0; - -fini: - drm_fb_helper_fini(&afbdev->helper); -free: - kfree(afbdev); - return ret; -} - -void ast_fbdev_fini(struct drm_device *dev) -{ - struct ast_private *ast = dev->dev_private; - - if (!ast->fbdev) - return; - - ast_fbdev_destroy(dev, ast->fbdev); - kfree(ast->fbdev); - ast->fbdev = NULL; -} - -void ast_fbdev_set_suspend(struct drm_device *dev, int state) -{ - struct ast_private *ast = dev->dev_private; - - if (!ast->fbdev) - return; - - drm_fb_helper_set_suspend(&ast->fbdev->helper, state); -} - -void ast_fbdev_set_base(struct ast_private *ast, unsigned long gpu_addr) -{ - ast->fbdev->helper.fbdev->fix.smem_start = - ast->fbdev->helper.fbdev->apertures->ranges[0].base + gpu_addr; - ast->fbdev->helper.fbdev->fix.smem_len = ast->vram_size - gpu_addr; -} diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 1bd61696e509..8dff7672560c 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -502,7 +502,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags) if (ret) goto out_free;
- ret = ast_fbdev_init(dev); + ret = drm_fbdev_generic_setup(dev, 32); if (ret) goto out_free;
@@ -520,7 +520,6 @@ void ast_driver_unload(struct drm_device *dev) ast_release_firmware(dev); kfree(ast->dp501_fw_addr); ast_mode_fini(dev); - ast_fbdev_fini(dev); drm_mode_config_cleanup(dev);
ast_mm_fini(ast); diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index c48249df758e..3c0716891b29 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -525,18 +525,12 @@ static int ast_crtc_do_set_base(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y, int atomic) { - struct ast_private *ast = crtc->dev->dev_private; struct drm_gem_vram_object *gbo; int ret; s64 gpu_addr; - void *base;
if (!atomic && fb) { gbo = drm_gem_vram_of_gem(fb->obj[0]); - - /* unmap if console */ - if (ast->fbdev->helper.fb == fb) - drm_gem_vram_kunmap(gbo); drm_gem_vram_unpin(gbo); }
@@ -551,17 +545,6 @@ static int ast_crtc_do_set_base(struct drm_crtc *crtc, goto err_drm_gem_vram_unpin; }
- if (ast->fbdev->helper.fb == crtc->primary->fb) { - /* if pushing console in kmap it */ - base = drm_gem_vram_kmap(gbo, true, NULL); - if (IS_ERR(base)) { - ret = PTR_ERR(base); - DRM_ERROR("failed to kmap fbcon\n"); - } else { - ast_fbdev_set_base(ast, gpu_addr); - } - } - ast_set_offset_reg(crtc); ast_set_start_address_crt1(crtc, (u32)gpu_addr);
@@ -618,14 +601,10 @@ static void ast_crtc_disable(struct drm_crtc *crtc) DRM_DEBUG_KMS("\n"); ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); if (crtc->primary->fb) { - struct ast_private *ast = crtc->dev->dev_private; struct drm_framebuffer *fb = crtc->primary->fb; struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(fb->obj[0]);
- /* unmap if console */ - if (ast->fbdev->helper.fb == fb) - drm_gem_vram_kunmap(gbo); drm_gem_vram_unpin(gbo); } crtc->primary->fb = NULL;
The bochs driver (and virtual hardware) requires buffer objects to reside in video ram to display them to the screen. So it can not display the framebuffer console because the respective buffer object is permanently pinned in system memory.
Using a shadow buffer for the console solves this problem. The console emulation will pin the buffer object only during updates from the shadow buffer. Otherwise, the bochs driver can freely relocated the buffer between system memory and video ram.
v2: * select shadow FB via struct drm_mode_config.prefer_shadow_fbdev
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Acked-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/bochs/bochs_kms.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c index bc19dbd531ef..359030d5d818 100644 --- a/drivers/gpu/drm/bochs/bochs_kms.c +++ b/drivers/gpu/drm/bochs/bochs_kms.c @@ -191,6 +191,7 @@ int bochs_kms_init(struct bochs_device *bochs) bochs->dev->mode_config.fb_base = bochs->fb_base; bochs->dev->mode_config.preferred_depth = 24; bochs->dev->mode_config.prefer_shadow = 0; + bochs->dev->mode_config.prefer_shadow_fbdev = 1; bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
bochs->dev->mode_config.funcs = &bochs_mode_funcs;
This patch replaces mgag200's framebuffer console with DRM's generic implememtation. All respective code is being removed from the driver.
The console is set up with a shadow buffer. The actual buffer object is not permanently pinned in video ram, but just another buffer object that the driver moves in and out of vram as necessary. The driver's function mga_crtc_do_set_base() used to contain special handling for the framebuffer console. With the new generic framebuffer, the driver does not need this code an longer.
For consistency, this patch also changes the preferred framebuffer depth. The original code used 24 bpp by default and 32 bpp for the framebuffer. As 24 bpp is not well supported by userspace anyway, setting 32 bpp as default makes sense.
v2: * rely on fbdev helpers error messages * document changes to preferred depth * dirty function no longer required
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Acked-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 19 -- drivers/gpu/drm/mgag200/mgag200_fb.c | 309 ------------------------- drivers/gpu/drm/mgag200/mgag200_main.c | 31 +-- drivers/gpu/drm/mgag200/mgag200_mode.c | 27 --- 5 files changed, 6 insertions(+), 382 deletions(-) delete mode 100644 drivers/gpu/drm/mgag200/mgag200_fb.c
diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile index 98d204408bd0..04b281bcf655 100644 --- a/drivers/gpu/drm/mgag200/Makefile +++ b/drivers/gpu/drm/mgag200/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only mgag200-y := mgag200_main.o mgag200_mode.o mgag200_cursor.o \ - mgag200_drv.o mgag200_fb.o mgag200_i2c.o mgag200_ttm.o + mgag200_drv.o mgag200_i2c.o mgag200_ttm.o
obj-$(CONFIG_DRM_MGAG200) += mgag200.o diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 3ab27f1053c1..1c93f8dc08c7 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -99,14 +99,6 @@ #define to_mga_encoder(x) container_of(x, struct mga_encoder, base) #define to_mga_connector(x) container_of(x, struct mga_connector, base)
-struct mga_fbdev { - struct drm_fb_helper helper; /* must be first */ - void *sysram; - int size; - int x1, y1, x2, y2; /* dirty rect */ - spinlock_t dirty_lock; -}; - struct mga_crtc { struct drm_crtc base; u8 lut_r[256], lut_g[256], lut_b[256]; @@ -180,7 +172,6 @@ struct mga_device { struct mga_mc mc; struct mga_mode_info mode_info;
- struct mga_fbdev *mfbdev; struct mga_cursor cursor;
bool suspended; @@ -201,19 +192,9 @@ struct mga_device { int mgag200_modeset_init(struct mga_device *mdev); void mgag200_modeset_fini(struct mga_device *mdev);
- /* mgag200_fb.c */ -int mgag200_fbdev_init(struct mga_device *mdev); -void mgag200_fbdev_fini(struct mga_device *mdev); - /* mgag200_main.c */ int mgag200_driver_load(struct drm_device *dev, unsigned long flags); void mgag200_driver_unload(struct drm_device *dev); -int mgag200_gem_create(struct drm_device *dev, - u32 size, bool iskernel, - struct drm_gem_object **obj); -int mgag200_dumb_create(struct drm_file *file, - struct drm_device *dev, - struct drm_mode_create_dumb *args);
/* mgag200_i2c.c */ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev); diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c deleted file mode 100644 index c77cf1b34c98..000000000000 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ /dev/null @@ -1,309 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * Copyright 2010 Matt Turner. - * Copyright 2012 Red Hat - * - * Authors: Matthew Garrett - * Matt Turner - * Dave Airlie - */ - -#include <linux/module.h> -#include <linux/vmalloc.h> - -#include <drm/drm_crtc_helper.h> -#include <drm/drm_fb_helper.h> -#include <drm/drm_fourcc.h> -#include <drm/drm_gem_framebuffer_helper.h> -#include <drm/drm_util.h> - -#include "mgag200_drv.h" - -static void mga_dirty_update(struct mga_fbdev *mfbdev, - int x, int y, int width, int height) -{ - int i; - struct drm_gem_vram_object *gbo; - int src_offset, dst_offset; - int ret; - u8 *dst; - bool unmap = false; - bool store_for_later = false; - int x2, y2; - unsigned long flags; - struct drm_framebuffer *fb = mfbdev->helper.fb; - int bpp = fb->format->cpp[0]; - - gbo = drm_gem_vram_of_gem(fb->obj[0]); - - if (drm_can_sleep()) { - /* We pin the BO so it won't be moved during the - * update. The actual location, video RAM or system - * memory, is not important. - */ - ret = drm_gem_vram_pin(gbo, 0); - if (ret) { - if (ret != -EBUSY) - return; - store_for_later = true; - } - } else { - store_for_later = true; - } - - x2 = x + width - 1; - y2 = y + height - 1; - spin_lock_irqsave(&mfbdev->dirty_lock, flags); - - if (mfbdev->y1 < y) - y = mfbdev->y1; - if (mfbdev->y2 > y2) - y2 = mfbdev->y2; - if (mfbdev->x1 < x) - x = mfbdev->x1; - if (mfbdev->x2 > x2) - x2 = mfbdev->x2; - - if (store_for_later) { - mfbdev->x1 = x; - mfbdev->x2 = x2; - mfbdev->y1 = y; - mfbdev->y2 = y2; - spin_unlock_irqrestore(&mfbdev->dirty_lock, flags); - return; - } - - mfbdev->x1 = mfbdev->y1 = INT_MAX; - mfbdev->x2 = mfbdev->y2 = 0; - spin_unlock_irqrestore(&mfbdev->dirty_lock, flags); - - dst = drm_gem_vram_kmap(gbo, false, NULL); - if (IS_ERR(dst)) { - DRM_ERROR("failed to kmap fb updates\n"); - goto out; - } else if (!dst) { - dst = drm_gem_vram_kmap(gbo, true, NULL); - if (IS_ERR(dst)) { - DRM_ERROR("failed to kmap fb updates\n"); - goto out; - } - unmap = true; - } - - for (i = y; i <= y2; i++) { - /* assume equal stride for now */ - src_offset = dst_offset = i * fb->pitches[0] + (x * bpp); - memcpy_toio(dst + dst_offset, mfbdev->sysram + src_offset, - (x2 - x + 1) * bpp); - } - - if (unmap) - drm_gem_vram_kunmap(gbo); - -out: - drm_gem_vram_unpin(gbo); -} - -static void mga_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct mga_fbdev *mfbdev = info->par; - drm_fb_helper_sys_fillrect(info, rect); - mga_dirty_update(mfbdev, rect->dx, rect->dy, rect->width, - rect->height); -} - -static void mga_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - struct mga_fbdev *mfbdev = info->par; - drm_fb_helper_sys_copyarea(info, area); - mga_dirty_update(mfbdev, area->dx, area->dy, area->width, - area->height); -} - -static void mga_imageblit(struct fb_info *info, - const struct fb_image *image) -{ - struct mga_fbdev *mfbdev = info->par; - drm_fb_helper_sys_imageblit(info, image); - mga_dirty_update(mfbdev, image->dx, image->dy, image->width, - image->height); -} - - -static struct fb_ops mgag200fb_ops = { - .owner = THIS_MODULE, - .fb_check_var = drm_fb_helper_check_var, - .fb_set_par = drm_fb_helper_set_par, - .fb_fillrect = mga_fillrect, - .fb_copyarea = mga_copyarea, - .fb_imageblit = mga_imageblit, - .fb_pan_display = drm_fb_helper_pan_display, - .fb_blank = drm_fb_helper_blank, - .fb_setcmap = drm_fb_helper_setcmap, -}; - -static int mgag200fb_create_object(struct mga_fbdev *afbdev, - const struct drm_mode_fb_cmd2 *mode_cmd, - struct drm_gem_object **gobj_p) -{ - struct drm_device *dev = afbdev->helper.dev; - u32 size; - struct drm_gem_object *gobj; - int ret = 0; - - size = mode_cmd->pitches[0] * mode_cmd->height; - ret = mgag200_gem_create(dev, size, true, &gobj); - if (ret) - return ret; - - *gobj_p = gobj; - return ret; -} - -static int mgag200fb_create(struct drm_fb_helper *helper, - struct drm_fb_helper_surface_size *sizes) -{ - struct mga_fbdev *mfbdev = - container_of(helper, struct mga_fbdev, helper); - struct drm_device *dev = mfbdev->helper.dev; - struct drm_mode_fb_cmd2 mode_cmd; - struct mga_device *mdev = dev->dev_private; - struct fb_info *info; - struct drm_framebuffer *fb; - struct drm_gem_object *gobj = NULL; - int ret; - void *sysram; - int size; - - mode_cmd.width = sizes->surface_width; - mode_cmd.height = sizes->surface_height; - mode_cmd.pitches[0] = mode_cmd.width * ((sizes->surface_bpp + 7) / 8); - - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, - sizes->surface_depth); - size = mode_cmd.pitches[0] * mode_cmd.height; - - ret = mgag200fb_create_object(mfbdev, &mode_cmd, &gobj); - if (ret) { - DRM_ERROR("failed to create fbcon backing object %d\n", ret); - return ret; - } - - sysram = vmalloc(size); - if (!sysram) { - ret = -ENOMEM; - goto err_drm_gem_object_put_unlocked; - } - - info = drm_fb_helper_alloc_fbi(helper); - if (IS_ERR(info)) { - ret = PTR_ERR(info); - goto err_vfree; - } - - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, gobj, NULL); - if (IS_ERR(fb)) { - ret = PTR_ERR(fb); - goto err_vfree; - } - - mfbdev->sysram = sysram; - mfbdev->size = size; - - /* setup helper */ - mfbdev->helper.fb = fb; - - info->fbops = &mgag200fb_ops; - - /* setup aperture base/size for vesafb takeover */ - info->apertures->ranges[0].base = mdev->dev->mode_config.fb_base; - info->apertures->ranges[0].size = mdev->mc.vram_size; - - drm_fb_helper_fill_info(info, &mfbdev->helper, sizes); - - info->screen_base = sysram; - info->screen_size = size; - info->pixmap.flags = FB_PIXMAP_SYSTEM; - - DRM_DEBUG_KMS("allocated %dx%d\n", - fb->width, fb->height); - - return 0; - -err_vfree: - vfree(sysram); -err_drm_gem_object_put_unlocked: - drm_gem_object_put_unlocked(gobj); - return ret; -} - -static int mga_fbdev_destroy(struct drm_device *dev, - struct mga_fbdev *mfbdev) -{ - drm_fb_helper_unregister_fbi(&mfbdev->helper); - drm_fb_helper_fini(&mfbdev->helper); - drm_framebuffer_put(mfbdev->helper.fb); - - vfree(mfbdev->sysram); - - return 0; -} - -static const struct drm_fb_helper_funcs mga_fb_helper_funcs = { - .fb_probe = mgag200fb_create, -}; - -int mgag200_fbdev_init(struct mga_device *mdev) -{ - struct mga_fbdev *mfbdev; - int ret; - int bpp_sel = 32; - - /* prefer 16bpp on low end gpus with limited VRAM */ - if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024)) - bpp_sel = 16; - - mfbdev = devm_kzalloc(mdev->dev->dev, sizeof(struct mga_fbdev), GFP_KERNEL); - if (!mfbdev) - return -ENOMEM; - - mdev->mfbdev = mfbdev; - spin_lock_init(&mfbdev->dirty_lock); - - drm_fb_helper_prepare(mdev->dev, &mfbdev->helper, &mga_fb_helper_funcs); - - ret = drm_fb_helper_init(mdev->dev, &mfbdev->helper, - MGAG200FB_CONN_LIMIT); - if (ret) - goto err_fb_helper; - - ret = drm_fb_helper_single_add_all_connectors(&mfbdev->helper); - if (ret) - goto err_fb_setup; - - /* disable all the possible outputs/crtcs before entering KMS mode */ - drm_helper_disable_unused_functions(mdev->dev); - - ret = drm_fb_helper_initial_config(&mfbdev->helper, bpp_sel); - if (ret) - goto err_fb_setup; - - return 0; - -err_fb_setup: - drm_fb_helper_fini(&mfbdev->helper); -err_fb_helper: - mdev->mfbdev = NULL; - - return ret; -} - -void mgag200_fbdev_fini(struct mga_device *mdev) -{ - if (!mdev->mfbdev) - return; - - mga_fbdev_destroy(mdev->dev, mdev->mfbdev); -} diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index b10f7265b5c4..a9773334dedf 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -162,7 +162,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024)) dev->mode_config.preferred_depth = 16; else - dev->mode_config.preferred_depth = 24; + dev->mode_config.preferred_depth = 32; dev->mode_config.prefer_shadow = 1;
r = mgag200_modeset_init(mdev); @@ -186,6 +186,10 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) } mdev->cursor.pixels_current = NULL;
+ r = drm_fbdev_generic_setup(mdev->dev, 0); + if (r) + goto err_modeset; + return 0;
err_modeset: @@ -204,32 +208,7 @@ void mgag200_driver_unload(struct drm_device *dev) if (mdev == NULL) return; mgag200_modeset_fini(mdev); - mgag200_fbdev_fini(mdev); drm_mode_config_cleanup(dev); mgag200_mm_fini(mdev); dev->dev_private = NULL; } - -int mgag200_gem_create(struct drm_device *dev, - u32 size, bool iskernel, - struct drm_gem_object **obj) -{ - struct drm_gem_vram_object *gbo; - int ret; - - *obj = NULL; - - size = roundup(size, PAGE_SIZE); - if (size == 0) - return -EINVAL; - - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0, false); - if (IS_ERR(gbo)) { - ret = PTR_ERR(gbo); - if (ret != -ERESTARTSYS) - DRM_ERROR("failed to allocate GEM object\n"); - return ret; - } - *obj = &gbo->gem; - return 0; -} diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index a7cef78d426f..822f2a13748f 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -860,18 +860,12 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y, int atomic) { - struct mga_device *mdev = crtc->dev->dev_private; struct drm_gem_vram_object *gbo; int ret; s64 gpu_addr; - void *base;
if (!atomic && fb) { gbo = drm_gem_vram_of_gem(fb->obj[0]); - - /* unmap if console */ - if (mdev->mfbdev->helper.fb == fb) - drm_gem_vram_kunmap(gbo); drm_gem_vram_unpin(gbo); }
@@ -886,15 +880,6 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, goto err_drm_gem_vram_unpin; }
- if (mdev->mfbdev->helper.fb == crtc->primary->fb) { - /* if pushing console in kmap it */ - base = drm_gem_vram_kmap(gbo, true, NULL); - if (IS_ERR(base)) { - ret = PTR_ERR(base); - DRM_ERROR("failed to kmap fbcon\n"); - } - } - mga_set_start_address(crtc, (u32)gpu_addr);
return 0; @@ -1418,14 +1403,9 @@ static void mga_crtc_disable(struct drm_crtc *crtc) DRM_DEBUG_KMS("\n"); mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); if (crtc->primary->fb) { - struct mga_device *mdev = crtc->dev->dev_private; struct drm_framebuffer *fb = crtc->primary->fb; struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(fb->obj[0]); - - /* unmap if console */ - if (mdev->mfbdev->helper.fb == fb) - drm_gem_vram_kunmap(gbo); drm_gem_vram_unpin(gbo); } crtc->primary->fb = NULL; @@ -1718,7 +1698,6 @@ int mgag200_modeset_init(struct mga_device *mdev) { struct drm_encoder *encoder; struct drm_connector *connector; - int ret;
mdev->mode_info.mode_config_initialized = true;
@@ -1743,12 +1722,6 @@ int mgag200_modeset_init(struct mga_device *mdev)
drm_connector_attach_encoder(connector, encoder);
- ret = mgag200_fbdev_init(mdev); - if (ret) { - DRM_ERROR("mga_fbdev_init failed\n"); - return ret; - } - return 0; }
Hi,
and removes a large amount of framebuffer code from these drivers. For bochs, a problem was reported where the driver could not display the console because it was pinned in system memory. [1] The patch set fixes this bug by converting bochs to use the shadow fb.
So, bochs in 5.3-rcX is broken, and patches 1+2+3+5 of this series unbreak it. Series already landed in drm-misc-next, but that doesn't help bochs in 5.3.
What is the usual way to handle this? Just cherry-pick the four patches into drm-misc-fixes and push?
thanks, Gerd
On Thu, Aug 1, 2019 at 9:04 AM Gerd Hoffmann kraxel@redhat.com wrote:
Hi,
and removes a large amount of framebuffer code from these drivers. For bochs, a problem was reported where the driver could not display the console because it was pinned in system memory. [1] The patch set fixes this bug by converting bochs to use the shadow fb.
So, bochs in 5.3-rcX is broken, and patches 1+2+3+5 of this series unbreak it. Series already landed in drm-misc-next, but that doesn't help bochs in 5.3.
What is the usual way to handle this? Just cherry-pick the four patches into drm-misc-fixes and push?
Best would be to put the patches into -fixes from the start. Cherry pick works too, if it's not too invasive. Otherwise revert in -fixes, and remember to not carry that revert over when backmerging. -Daniel
On Thu, Aug 01, 2019 at 01:38:18PM +0200, Daniel Vetter wrote:
On Thu, Aug 1, 2019 at 9:04 AM Gerd Hoffmann kraxel@redhat.com wrote:
Hi,
and removes a large amount of framebuffer code from these drivers. For bochs, a problem was reported where the driver could not display the console because it was pinned in system memory. [1] The patch set fixes this bug by converting bochs to use the shadow fb.
So, bochs in 5.3-rcX is broken, and patches 1+2+3+5 of this series unbreak it. Series already landed in drm-misc-next, but that doesn't help bochs in 5.3.
What is the usual way to handle this? Just cherry-pick the four patches into drm-misc-fixes and push?
Best would be to put the patches into -fixes from the start.
Sure, but that ship is sailed in this case.
Cherry pick works too, if it's not too invasive.
All 4 patches are clean cherry-picks, so I did that.
thanks, Gerd
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Gerd Hoffmann -
Noralf Trønnes -
Thomas Zimmermann