| Message ID | 20190703083302.2609-1-tzimmermann@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Unmappable DRM client buffers for fbdev emulation | expand |
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? 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 >
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. I'm not sure if we should check for the dirty() callback at all. 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >
Hi Am 04.07.19 um 12:18 schrieb Noralf Trønnes: > > > 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: I had something like that in mind, but maybe without a separate function. I'll post my variant as part of the patch set's next iteration. > 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? Drivers may already come with their own shadow buffer. Cirrus is an example of that. It uses shmem buffer objects as shadow fbs and internally updates the device frame buffer in its dirty callback. Using dirty() to select the shadow fbdev adds another buffer (and another memcpy) for no reason. Best regards Thomas > 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@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >>> >>
Den 04.07.2019 13.10, skrev Thomas Zimmermann: > Hi > > Am 04.07.19 um 12:18 schrieb Noralf Trønnes: >> >> >> 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: > > I had something like that in mind, but maybe without a separate > function. I'll post my variant as part of the patch set's next iteration. > <snip> >>> >>> I'm not sure if we should check for the dirty() callback at all. >>> >> >> Hm, why do you think that? > > Drivers may already come with their own shadow buffer. Cirrus is an > example of that. It uses shmem buffer objects as shadow fbs and > internally updates the device frame buffer in its dirty callback. Using > dirty() to select the shadow fbdev adds another buffer (and another > memcpy) for no reason. Cirruc uses shmem buffers and they won't work with fbdev defio (both use page->lru). shmem is the reason I added shadow buffering to generic fbdev in the first place. It will now work with whatever backing buffer the driver uses, as long as it can provide a virtual address on the dumb buffer (not the case with rockchip for instance, due to limited virtual address space). Noralf. > > Best regards > Thomas > >> 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@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >>>> >>> >
Hi Am 04.07.19 um 16:15 schrieb Noralf Trønnes: >>> Hm, why do you think that? >> >> Drivers may already come with their own shadow buffer. Cirrus is an >> example of that. It uses shmem buffer objects as shadow fbs and >> internally updates the device frame buffer in its dirty callback. Using >> dirty() to select the shadow fbdev adds another buffer (and another >> memcpy) for no reason. > > Cirruc uses shmem buffers and they won't work with fbdev defio (both use > page->lru). shmem is the reason I added shadow buffering to generic > fbdev in the first place. It will now work with whatever backing buffer > the driver uses, as long as it can provide a virtual address on the dumb > buffer (not the case with rockchip for instance, due to limited virtual > address space). OK, I see. Thanks or clarifying. Best regards Thomas > Noralf. > >> >> Best regards >> Thomas >> >>> 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@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 >
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. [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