Re: [PATCH] drm/i915/ggtt: Move ballooning support to i915_ggtt

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

 



On Thu, 21 Nov 2019 19:05:25 +0100, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:

Quoting Michal Wajdeczko (2019-11-21 17:58:50)
Today VGT implements GGTT ballooning on its own, using some private
static structure to hold info about reserved GGTT nodes and then
manually update vm.reserved size owned by i915_ggtt.

As i915_ggtt already manages some custom reserved nodes (like uc_fw)
move VGT ballooning support to i915_ggtt and make it more generic
for possible future reuse in other scenarios.

As a bonus we can drop another place in driver that uses static data.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Xiong Zhang <xiong.y.zhang@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 47 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_gtt.h | 18 ++++++++
 drivers/gpu/drm/i915/i915_vgpu.c    | 72 ++++++-----------------------
 3 files changed, 78 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6239a9adbf14..d8b2084dab7e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3827,6 +3827,53 @@ int i915_gem_gtt_insert(struct i915_address_space *vm, start, end, DRM_MM_INSERT_EVICT);
 }

+int i915_ggtt_balloon_space(struct i915_ggtt *ggtt,
+                           enum i915_ggtt_balloon_id id,

Do we want to use a fixed number of slots?

Probably not, enum was just used as a starting point to provide explicit
name each of the available node and to avoid adding GEM_BUG_ON while
accessing fixed size array.


So what exactly is wrong with the clients reserving nodes themselves?

They still reserve nodes themselves. But now they will not do that
in a shadow and not use static data to hold actual nodes.


I'm not seeing why exactly we want to move to a much more centralised
system

I wanted to make it clear to all clients that i915_ggtt is a good place
to keep list of reserved some (instead of hiding them in private places)

And to provide wrapper function that can be easily reused and that each
ballooning behaves in the same way.

Extra bonus is that we have all such balloon tweaks in one place (both
code and data nodes).

and argue over ownership of i915_ggtt_balloon_id.

Now it's first come, first served order, but we should already know
all current usages and in case of clash either pick different id or
add the new one.

Note that today some scenarios are exclusive (gvt won't work with guc)
so some node placeholders can be shared - but this can be equally done
without enums but with union of explicit node definitions.

	I915_GGTT_BALLOON_GUC_TOP = I915_GGTT_BALLOON3,
	I915_GGTT_BALLOON_VGT_UNMAP_TOP = I915_GGTT_BALLOON3,

	union {
		struct {
			struct drm_mm_node uc_fw;
		} guc;
		struct {
			struct drm_mm_node space[4];
		} vgt;
	} balloons;

but direct access to nodes can be also wrapped into:

struct drm_mm_node *i915_ggtt_get_uc_fw_node(i915_ggtt *ggtt)
{
	return ggtt->balloon[I915_GGTT_BALLOON_GUC_TOP];
}

Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux