On 11/12/2014 05:02 PM, Daniel Vetter wrote:
On Thu, Nov 06, 2014 at 02:39:25PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Things like reliable GGTT mmaps and mirrored 2d-on-3d display will need
to map objects into the same address space multiple times.
This also means that objects now can have multiple VMA entries.
Added a GGTT view concept and linked it with the VMA to distinguish betwen
multiple instances per address space.
New objects and GEM functions which do not take this new view as a parameter
assume the default of zero (I915_GGTT_VIEW_NORMAL) which preserves the
previous behaviour.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Let's hope I've picked the latest version, not sure. Please either have
in-patch changelogs or coverletters with what's new to help confused
maintainers ;-)
I had a cover letter and version when it was a patch series _and_ the
concept split into multiple VMA and GGTT view. Once only one patch
remained from the patch series and it was redesigned to kind of merge
two concepts into one simplified approach I did not think cover letter
makes sense any more. Will definitely version as this design goes forward.
Looks good overal, just a few things around the lifetime rules for sg
tables and related stuff that we've discussed offline. And one nit.
I think with these addressed this is ready for detailed review and merging
(also applies to the -internal series).
Excellent, thank you! Just one question below:
-int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
+int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj,
+ const struct i915_ggtt_view *view)
{
- if (obj->has_dma_mapping)
+ if (obj->has_dma_mapping || view->type != I915_GGTT_VIEW_NORMAL)
This check and the one for finish_gtt are wrong. prepare/finish_gtt set up
the dma_addr fields in the sg table (if available also the iommu mappings
on big core). And that _must_ be done when the first view shows up, no
matter which one it is. E.g. userspace might display a rotate buffer right
aways without even rendering or writing into it (i.e. leaving it black).
The plan was to ensure that the "normal" view is always there first.
Otherwise we can't "steal" DMA addresses and the other views do not have
a proper SG table to generate DMA addresses from. So the internal code
was doing that. Am I missing something?
The change around finish_gtt is actually a leak since if the last view
around is the rotate one (which can happen if userspace drops the buffer
but leaves it displayed) we won't clean up (and so leak) the iommu
mapping.
So I somehow need to ensure finish_gtt on a "normal" view is done last,
correct? Move it from VMA destructon to object destruction? Would this
have any implications for the shrinker?
Thanks,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
