Koenig, Christian
2019-Apr-08 11:10 UTC
[PATCH 00/15] Share TTM code among framebuffer drivers
Well first problem is I'm not sure if that is a good idea. Essentially we want to get rid of TTM in the long run. On the other hand this work might aid with that goal, so it might be worth a try. Second is that this might actually not work of hand. The problem is here:> + /* TODO: This test used to be performed by drivers, but can > + * this actually happen? If so, should we put the check into > + * drm_gem_ttm_of_gem()? */ > + if (!drm_is_gem_ttm(bo)) > + return;Yeah, this test is mandatory because TTM on itself can present buffer object which doesn't belong to the driver called. E.g. we sometimes have BOs which don't belong to the current drivers on a driver specific LRU. A totally brain dead? design if you ask me, but that's how it is. Not 100% sure, but by converting all drivers to use a common GEM_TTM backend you might actually break that test. I'm not sure if that is actually a problem in the real world, it most likely isn't. But I still won't bet on it without being able to test this. Regards, Christian. Am 08.04.19 um 11:21 schrieb Thomas Zimmermann:> Several simple framebuffer drivers copy most of the TTM code from each > other. The implementation is always the same; except for the name of > some data structures. > > As recently discussed, this patch set provides generic TTM memory- > management code for framebuffers with dedicated video memory. It further > converts the respective drivers to the generic code. The shared code > is basically the same implementation as the one copied among individual > drivers. > > The patch set contains two major changes: first, it introduces > |struct drm_gem_ttm_object| and helpers. It's a GEM object that is > backed by TTM-managed memory. The type's purpose is somewhat similar > to |struct drm_gem_{cma, shmem}_object|. Second, the patch set > introduces |struct drm_simple_ttm| (for the lack of a better name) and > helpers. It's an implementation of a basic TTM-based memory manager. > > Both, GEM TTM and Simple TTM, support VRAM and SYSTEM placements. Support > for TT could probably be added if necessary. Both can be used independedly > from each other if desired by the DRM driver. > > Currently ast, bochs, mgag200, vboxvideo and hisilicon/hibmc can use > these helpers. Cirrus would also be a candidate, but as it's being > rewrtten from scratch, I didn't bother doing the conversion. > > Future directions: with these changes, the respective drivers can also > share some of their mode-setting or fbdev code. GEM TTM could implement > PRIME helpers, which would allow for using the generic fbcon. > > The patch set is against a recent drm-tip. > > Thomas Zimmermann (15): > drm: Add |struct drm_gem_ttm_object| and helpers > drm: Add |struct drm_gem_ttm_object| callbacks for |struct > ttm_bo_driver| > drm: Add |struct drm_gem_ttm_object| callbacks for |struct drm_driver| > drm: Add drm_gem_ttm_fill_create_dumb() to create dumb buffers > drm: Add Simple TTM, a memory manager for dedicated VRAM > drm/ast: Convert AST driver to |struct drm_gem_ttm_object| > drm/ast: Convert AST driver to Simple TTM > drm/bochs: Convert Bochs driver to |struct drm_gem_ttm_object| > drm/bochs: Convert Bochs driver to Simple TTM > drm/mgag200: Convert mgag200 driver to |struct drm_gem_ttm_object| > drm/mgag200: Convert mgag200 driver to Simple TTM > drm/vboxvideo: Convert vboxvideo driver to |struct drm_gem_ttm_object| > drm/vboxvideo: Convert vboxvideo driver to Simple TTM > drm/hisilicon: Convert hibmc-drm driver to |struct drm_gem_ttm_object| > drm/hisilicon: Convert hibmc-drm driver to Simple TTM > > Documentation/gpu/drm-mm.rst | 23 + > drivers/gpu/drm/Kconfig | 20 + > drivers/gpu/drm/Makefile | 5 + > drivers/gpu/drm/ast/Kconfig | 3 +- > drivers/gpu/drm/ast/ast_drv.c | 4 +- > drivers/gpu/drm/ast/ast_drv.h | 58 +- > drivers/gpu/drm/ast/ast_fb.c | 18 +- > drivers/gpu/drm/ast/ast_main.c | 74 +-- > drivers/gpu/drm/ast/ast_mode.c | 78 +-- > drivers/gpu/drm/ast/ast_ttm.c | 290 +--------- > drivers/gpu/drm/bochs/Kconfig | 2 + > drivers/gpu/drm/bochs/bochs.h | 42 +- > drivers/gpu/drm/bochs/bochs_drv.c | 4 +- > drivers/gpu/drm/bochs/bochs_kms.c | 18 +- > drivers/gpu/drm/bochs/bochs_mm.c | 392 +------------- > drivers/gpu/drm/drm_gem_ttm_helper.c | 507 ++++++++++++++++++ > drivers/gpu/drm/drm_simple_ttm_helper.c | 191 +++++++ > drivers/gpu/drm/drm_ttm_helper_common.c | 6 + > drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 + > .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 19 +- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 4 +- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 28 +- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 30 +- > drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 328 +---------- > drivers/gpu/drm/mgag200/Kconfig | 2 + > drivers/gpu/drm/mgag200/mgag200_cursor.c | 61 ++- > drivers/gpu/drm/mgag200/mgag200_drv.c | 4 +- > drivers/gpu/drm/mgag200/mgag200_drv.h | 68 +-- > drivers/gpu/drm/mgag200/mgag200_fb.c | 18 +- > drivers/gpu/drm/mgag200/mgag200_main.c | 84 +-- > drivers/gpu/drm/mgag200/mgag200_mode.c | 45 +- > drivers/gpu/drm/mgag200/mgag200_ttm.c | 290 +--------- > drivers/gpu/drm/vboxvideo/Kconfig | 2 + > drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- > drivers/gpu/drm/vboxvideo/vbox_drv.h | 64 +-- > drivers/gpu/drm/vboxvideo/vbox_fb.c | 22 +- > drivers/gpu/drm/vboxvideo/vbox_main.c | 70 +-- > drivers/gpu/drm/vboxvideo/vbox_mode.c | 36 +- > drivers/gpu/drm/vboxvideo/vbox_ttm.c | 344 +----------- > include/drm/drm_gem_ttm_helper.h | 119 ++++ > include/drm/drm_simple_ttm_helper.h | 74 +++ > 41 files changed, 1292 insertions(+), 2162 deletions(-) > create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c > create mode 100644 drivers/gpu/drm/drm_simple_ttm_helper.c > create mode 100644 drivers/gpu/drm/drm_ttm_helper_common.c > create mode 100644 include/drm/drm_gem_ttm_helper.h > create mode 100644 include/drm/drm_simple_ttm_helper.h > > -- > 2.21.0 >