[PATCH 0/5] Unmappable DRM client buffers for fbdev emulation
Noralf Trønnes
noralf at tronnes.org
Thu Jul 4 10:18:06 UTC 2019
Den 04.07.2019 09.43, skrev Thomas Zimmermann:
> Hi
>
> Am 03.07.19 um 21:27 schrieb Noralf Trønnes:
>>
>>
>> Den 03.07.2019 10.32, skrev Thomas Zimmermann:
>>> 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 enougth 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.
>>>
>>> The default behoviour for DRM client buffers is still to be permanently
>>> mapped.
>>>
>>> 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.
>>>
>>
>> I've been thinking, this enables dirty tracking in DRM userspace for
>> these drivers since the dirty callback is now set on all framebuffers.
>> Is this OK? Should we add a drm_fbdev_generic_shadow_setup() that sets a
>> flag enabling the shadow buffer instead?
>
> Fbdev emulation is special wrt framebuffer setup and there's no way to
> distinguish a regular FB from the fbdev's FB. I've been trying
> drm_fbdev_generic_shadow_setup(), but ended up duplicating
> functionality. The problem was that we cannot get state-flag arguments
> into drm_fb_helper_generic_probe().
>
> There already is struct mode_config.prefer_shadow. It signals shadow FB
> rendering to userspace. The easiest solution is to add
> prefer_shadow_fbdev as well. If either flag is true, fbdev emulation
> would enable shadow buffering.
How about something like this:
diff --git a/drivers/gpu/drm/drm_fb_helper.c
b/drivers/gpu/drm/drm_fb_helper.c
index 1984e5c54d58..723fe56aa5f5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -415,7 +415,8 @@ static void drm_fb_helper_dirty_work(struct
work_struct *work)
/* Generic fbdev uses a shadow buffer */
if (helper->buffer)
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);
}
}
@@ -2209,7 +2210,7 @@ int drm_fb_helper_generic_probe(struct
drm_fb_helper *fb_helper,
#endif
drm_fb_helper_fill_info(fbi, fb_helper, sizes);
- if (fb->funcs->dirty) {
+ if (fb->funcs->dirty || fb_helper->use_shadow) {
struct fb_ops *fbops;
void *shadow;
@@ -2310,6 +2311,44 @@ static const struct drm_client_funcs
drm_fbdev_client_funcs = {
.hotplug = drm_fbdev_client_hotplug,
};
+static int _drm_fbdev_generic_setup(struct drm_device *dev, unsigned
int preferred_bpp, bool use_shadow)
+{
+ struct drm_fb_helper *fb_helper;
+ int ret;
+
+ WARN(dev->fb_helper, "fb_helper is already set!\n");
+
+ if (!drm_fbdev_emulation)
+ return 0;
+
+ fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
+ if (!fb_helper)
+ return -ENOMEM;
+
+ ret = drm_client_init(dev, &fb_helper->client, "fbdev",
&drm_fbdev_client_funcs);
+ if (ret) {
+ kfree(fb_helper);
+ DRM_DEV_ERROR(dev->dev, "Failed to register client: %d\n", ret);
+ return ret;
+ }
+
+ if (!preferred_bpp)
+ preferred_bpp = dev->mode_config.preferred_depth;
+ if (!preferred_bpp)
+ preferred_bpp = 32;
+ fb_helper->preferred_bpp = preferred_bpp;
+
+ fb_helper->use_shadow = use_shadow;
+
+ ret = drm_fbdev_client_hotplug(&fb_helper->client);
+ if (ret)
+ DRM_DEV_DEBUG(dev->dev, "client hotplug ret=%d\n", ret);
+
+ drm_client_register(&fb_helper->client);
+
+ return 0;
+}
+
/**
* drm_fbdev_generic_setup() - Setup generic fbdev emulation
* @dev: DRM device
@@ -2338,38 +2377,13 @@ static const struct drm_client_funcs
drm_fbdev_client_funcs = {
*/
int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int
preferred_bpp)
{
- struct drm_fb_helper *fb_helper;
- int ret;
+ return _drm_fbdev_generic_setup(dev, preferred_bpp, false);
+}
+EXPORT_SYMBOL(drm_fbdev_generic_setup);
- WARN(dev->fb_helper, "fb_helper is already set!\n");
-
- if (!drm_fbdev_emulation)
- return 0;
-
- fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
- if (!fb_helper)
- return -ENOMEM;
-
- ret = drm_client_init(dev, &fb_helper->client, "fbdev",
&drm_fbdev_client_funcs);
- if (ret) {
- kfree(fb_helper);
- DRM_DEV_ERROR(dev->dev, "Failed to register client: %d\n", ret);
- return ret;
- }
-
- if (!preferred_bpp)
- preferred_bpp = dev->mode_config.preferred_depth;
- if (!preferred_bpp)
- preferred_bpp = 32;
- fb_helper->preferred_bpp = preferred_bpp;
-
- ret = drm_fbdev_client_hotplug(&fb_helper->client);
- if (ret)
- DRM_DEV_DEBUG(dev->dev, "client hotplug ret=%d\n", ret);
-
- drm_client_register(&fb_helper->client);
-
- return 0;
+int drm_fbdev_generic_shadow_setup(struct drm_device *dev, unsigned int
preferred_bpp)
+{
+ return _drm_fbdev_generic_setup(dev, preferred_bpp, true);
}
EXPORT_SYMBOL(drm_fbdev_generic_setup);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index c8a8ae2a678a..39f063de8cbc 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -186,6 +186,8 @@ struct drm_fb_helper {
* See also: @deferred_setup
*/
int preferred_bpp;
+
+ bool use_shadow;
};
static inline struct drm_fb_helper *
>
> I'm not sure if we should check for the dirty() callback at all.
>
Hm, why do you think that?
The thing with fbdev defio is that it only supports kmalloc and vmalloc
allocated memory (page->lru is avail.). This means that only the CMA
drivers can use defio without shadow memory. To keep things simple
everyone with a dirty() callback gets a shadow buffer.
Noralf.
> Best regards
> Thomas
>
>> Really nice diffstat by the way :-)
>>
>> Noralf.
>>
>>> [1] https://lists.freedesktop.org/archives/dri-devel/2019-June/224423.html
>>>
>>> Thomas Zimmermann (5):
>>> drm/client: Support unmapping of DRM client buffers
>>> drm/fb-helper: Unmap BO for shadow-buffered framebuffer console
>>> 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 | 22 +-
>>> drivers/gpu/drm/ast/ast_drv.h | 17 --
>>> drivers/gpu/drm/ast/ast_fb.c | 341 -------------------------
>>> drivers/gpu/drm/ast/ast_main.c | 30 ++-
>>> drivers/gpu/drm/ast/ast_mode.c | 21 --
>>> drivers/gpu/drm/bochs/bochs_kms.c | 2 +-
>>> drivers/gpu/drm/drm_client.c | 71 ++++-
>>> drivers/gpu/drm/drm_fb_helper.c | 14 +-
>>> 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 | 61 +++--
>>> drivers/gpu/drm/mgag200/mgag200_mode.c | 27 --
>>> include/drm/drm_client.h | 3 +
>>> 15 files changed, 154 insertions(+), 787 deletions(-)
>>> delete mode 100644 drivers/gpu/drm/ast/ast_fb.c
>>> delete mode 100644 drivers/gpu/drm/mgag200/mgag200_fb.c
>>>
>>> --
>>> 2.21.0
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
More information about the dri-devel
mailing list