Op 04-02-2022 om 19:29 schreef Christian König:
Oh, that's on my TODO list for years!
Am 04.02.22 um 18:48 schrieb Maarten Lankhorst:
Suballocating a buffer object is something that is not driver generic, and is useful for other drivers as well.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
drivers/gpu/drm/Makefile | 4 +- drivers/gpu/drm/drm_suballoc.c | 424 +++++++++++++++++++++++++++++++++ include/drm/drm_suballoc.h | 78 ++++++ 3 files changed, 505 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_suballoc.c create mode 100644 include/drm/drm_suballoc.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 8675c2af7ae1..b848bcf8790c 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -57,7 +57,9 @@ drm_kms_helper-y := drm_bridge_connector.o drm_crtc_helper.o \ drm_scdc_helper.o drm_gem_atomic_helper.o \ drm_gem_framebuffer_helper.o \ drm_atomic_state_helper.o drm_damage_helper.o \ - drm_format_helper.o drm_self_refresh_helper.o drm_rect.o + drm_format_helper.o drm_self_refresh_helper.o drm_rect.o \ + drm_suballoc.o
I think we should put that into a separate module like we now do with other helpers as well.
Can easily be done, it will likely be a very small helper. The code itself is just under a page. I felt the overhead wasn't worth it, but will do so.
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o diff --git a/drivers/gpu/drm/drm_suballoc.c b/drivers/gpu/drm/drm_suballoc.c new file mode 100644 index 000000000000..e0bb35367b71 --- /dev/null +++ b/drivers/gpu/drm/drm_suballoc.c @@ -0,0 +1,424 @@ +/*
- Copyright 2011 Red Hat Inc.
- All Rights Reserved.
- 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:
- * Jerome Glisse glisse@freedesktop.org
- */
That is hopelessly outdated. IIRC I completely rewrote that stuff in ~2012.
If you rewrote it, can you give me an updated copyright header please?
+/* Algorithm:
- We store the last allocated bo in "hole", we always try to allocate
- after the last allocated bo. Principle is that in a linear GPU ring
- progression was is after last is the oldest bo we allocated and thus
- the first one that should no longer be in use by the GPU.
- If it's not the case we skip over the bo after last to the closest
- done bo if such one exist. If none exist and we are not asked to
- block we report failure to allocate.
- If we are asked to block we wait on all the oldest fence of all
- rings. We just wait for any of those fence to complete.
- */
+#include <drm/drm_suballoc.h> +#include <drm/drm_print.h> +#include <linux/slab.h> +#include <linux/sched.h> +#include <linux/wait.h> +#include <linux/dma-fence.h>
+static void drm_suballoc_remove_locked(struct drm_suballoc *sa); +static void drm_suballoc_try_free(struct drm_suballoc_manager *sa_manager);
+/**
- drm_suballoc_manager_init - Initialise the drm_suballoc_manager
- @sa_manager: pointer to the sa_manager
- @size: number of bytes we want to suballocate
- @align: alignment for each suballocated chunk
- Prepares the suballocation manager for suballocations.
- */
+void drm_suballoc_manager_init(struct drm_suballoc_manager *sa_manager, + u32 size, u32 align) +{ + u32 i;
+ if (!align) + align = 1;
+ /* alignment must be a power of 2 */ + BUG_ON(align & (align - 1));
When we move that I think we should cleanup the code once more, e.g. use is_power_of_2() function here for example.
Yeah, I was looking for POW2 or something, I couldn't remember the macro name.
There are also a bunch of places with extra {} and constructs like "if (....) return true; else return false;" which could certainly be simplified.
Apart from that really great idea.
I copied this from the original implementation, I didn't want to do any major cleanups, as I wanted to keep it as identical to the current code as possible.
The only thing I changed is moving the alignment to init, because it removes dealing with differently aligned suballocations as simplification.
By the way, does this break amd's CI in any way?
Cheers,
Maarten