Re: [PATCH 0/5] Unmappable DRM client buffers for fbdev emulation

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

 




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@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux