Re: [PATCH 2/4] drm/i915: Improve dynamic management/eviction of lrc backing objects

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

 



On 19/10/2015 10:48, Daniel Vetter wrote:
On Fri, Oct 16, 2015 at 03:42:53PM +0100, Nick Hoath wrote:
On 08/10/2015 14:35, Chris Wilson wrote:
On Wed, Oct 07, 2015 at 06:05:46PM +0200, Daniel Vetter wrote:
On Tue, Oct 06, 2015 at 03:52:02PM +0100, Nick Hoath wrote:
Shovel all context related objects through the active queue and obj
management.

- Added callback in vma_(un)bind to add CPU (un)mapping at same time
   if desired
- Inserted LRC hw context & ringbuf to vma active list

Issue: VIZ-4277
Signed-off-by: Nick Hoath <nicholas.hoath@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h         |  4 ++
  drivers/gpu/drm/i915/i915_gem.c         |  3 ++
  drivers/gpu/drm/i915/i915_gem_gtt.c     |  8 ++++
  drivers/gpu/drm/i915/intel_lrc.c        | 28 +++++++++++--
  drivers/gpu/drm/i915/intel_ringbuffer.c | 71 ++++++++++++++++++---------------
  drivers/gpu/drm/i915/intel_ringbuffer.h |  3 --
  6 files changed, 79 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3d217f9..d660ee3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2169,6 +2169,10 @@ struct drm_i915_gem_object {
  			struct work_struct *work;
  		} userptr;
  	};
+
+	/** Support for automatic CPU side mapping of object */
+	int (*mmap)(struct drm_i915_gem_object *obj, bool unmap);

I don't think we need a map hook, that can still be done (if not done so
I disagree - this keeps the interface symmetrical. Searching for the do/undo
code paths and finding they are in difference places, called via different
routes makes code harder to follow.
already) by the callers. Also it's better to rename this to vma_unbind
(and it should be at the vma level I think) since there's other potential
Nope - the obj is created first, at a point where the map/unamp function can
be known. Moving the map/unmap to the vma would mean having a callback path
to the object just to set up the callback path when the vma is created
anonymously at some later point.

One of the plans for this is to also use it for to-be-unpinned
framebuffers (4k buffers are huge ...). And in that case the unmap hook
only, and on the vma is the design we want. And since it also seems to
accomodate all the other users I do think it's the right choice.

I refer you to these words found on the mail list. The may be familiar:

As a rule of thumb for refactoring and share infastructure we use the following recipe in drm:
- first driver implements things as straightforward as possible
- 2nd user copypastes
- 3rd one has the duty to figure out whether some refactoring is in order
  or not.

The code as I have written it works best and simplest for my use case. If someone else wants to refactor it differently to shoe horn in their use case, that's up to them.



Like I said, explicit setup and lazy, implicit cleanup is kinda how a lot
of things in gem work.

The most dangerous phrase in the language is ‘we’ve always done it this way.’ - Grace Hopper

-Daniel

users. So explicit maping, lazy unmapping for the kmaps we need. That's
the same design we're using for binding objects into gpu address spaces.

Also Chris Wilson has something similar, please align with him on the
precise design of this callback.

We need the unbind hook because of the movement in the first patch (it
is a separate issue, the code should work without it albeit having to
remap the ring/context state more often). The changelog in this patch
simply explains the i915_vma_move_to_active() additions. But to get the
shrink accurate we do need the context unpin on retirement and to do the
pin_count check in i915_vma_unbind() after waiting (rather than before,
as we currently do). However, the eviction code will not inspect the
active contexts objects yet (as it will continue to skip over the
ggtt->pin_count on them). The way I allowed ctx objects to be evicted was
to only keep the ctx->state pinned for the duration of the request
construction.

Note that I think it should be a vma->unbind hook not an object level
one (it is i915_vma_unbind, without only a modicum of object level state
being modified in that function).
-Chris




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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux