* [PATCH 0/3] Multiple GGTT views
@ 2014-12-03 14:59 Tvrtko Ursulin
2014-12-03 14:59 ` [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list Tvrtko Ursulin
` (4 more replies)
0 siblings, 5 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2014-12-03 14:59 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
This series continues what was previously a single patch called "drm/i915:
Infrastructure for supporting different GGTT views per object".
To start with we break the assumption GGTT VMA is at the head of the list to
smoke out any potential hidden assumptions in the code. I have run a few IGTs
here and things look OK.
Trailing the series is a small DocBook section describing the new feature.
And the main patch has it's own change log.
Tvrtko Ursulin (3):
drm/i915: Stop putting GGTT VMA at the head of the list
drm/i915: Infrastructure for supporting different GGTT views per
object
drm/i915: Documentation for multiple GGTT views.
Documentation/DocBook/drm.tmpl | 5 ++
drivers/gpu/drm/i915/i915_debugfs.c | 5 +-
drivers/gpu/drm/i915/i915_drv.h | 46 +++++++++-
drivers/gpu/drm/i915/i915_gem.c | 81 ++++++++++--------
drivers/gpu/drm/i915/i915_gem_context.c | 4 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 130 ++++++++++++++++++++++++-----
drivers/gpu/drm/i915/i915_gem_gtt.h | 22 ++++-
drivers/gpu/drm/i915/i915_gpu_error.c | 8 +-
9 files changed, 232 insertions(+), 73 deletions(-)
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-03 14:59 [PATCH 0/3] Multiple GGTT views Tvrtko Ursulin @ 2014-12-03 14:59 ` Tvrtko Ursulin 2014-12-04 9:48 ` Chris Wilson 2014-12-03 14:59 ` [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin ` (3 subsequent siblings) 4 siblings, 1 reply; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-03 14:59 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Multiple GGTT VMAs per object will be introduced in the near future which will make it impossible to guarantee normal GGTT view is at the head of the list. Purpose of this patch is to break this assumption straight away so any potential hidden assumptions in the code base can be bisected to this simple patch. For: VIZ-4544 Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Suggested-by: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/i915/i915_gem.c | 10 ++++++---- drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 9d362d3..c1c1141 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5303,11 +5303,13 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr) struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj) { + struct i915_address_space *ggtt = i915_obj_to_ggtt(obj); struct i915_vma *vma; - vma = list_first_entry(&obj->vma_list, typeof(*vma), vma_link); - if (vma->vm != i915_obj_to_ggtt(obj)) - return NULL; + list_for_each_entry(vma, &obj->vma_list, vma_link) { + if (vma->vm == ggtt) + return vma; + } - return vma; + return NULL; } diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 171f6ea..ac03a38 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2202,13 +2202,9 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, BUG(); } - /* Keep GGTT vmas first to make debug easier */ - if (i915_is_ggtt(vm)) - list_add(&vma->vma_link, &obj->vma_list); - else { - list_add_tail(&vma->vma_link, &obj->vma_list); + list_add_tail(&vma->vma_link, &obj->vma_list); + if (!i915_is_ggtt(vm)) i915_ppgtt_get(i915_vm_to_ppgtt(vm)); - } return vma; } -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-03 14:59 ` [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list Tvrtko Ursulin @ 2014-12-04 9:48 ` Chris Wilson 2014-12-04 10:02 ` Tvrtko Ursulin 2014-12-04 10:54 ` Daniel Vetter 0 siblings, 2 replies; 37+ messages in thread From: Chris Wilson @ 2014-12-04 9:48 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx On Wed, Dec 03, 2014 at 02:59:24PM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Multiple GGTT VMAs per object will be introduced in the near future which will > make it impossible to guarantee normal GGTT view is at the head of the list. > > Purpose of this patch is to break this assumption straight away so any > potential hidden assumptions in the code base can be bisected to this > simple patch. No, please don't. The rationale for putting ggtt first is because it puts the one vma that every object is likely to use at the front, and makes it also the first in the list for debugging. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-04 9:48 ` Chris Wilson @ 2014-12-04 10:02 ` Tvrtko Ursulin 2014-12-04 10:17 ` Chris Wilson 2014-12-04 10:54 ` Daniel Vetter 1 sibling, 1 reply; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-04 10:02 UTC (permalink / raw) To: Chris Wilson, Intel-gfx On 12/04/2014 09:48 AM, Chris Wilson wrote: > On Wed, Dec 03, 2014 at 02:59:24PM +0000, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Multiple GGTT VMAs per object will be introduced in the near future which will >> make it impossible to guarantee normal GGTT view is at the head of the list. >> >> Purpose of this patch is to break this assumption straight away so any >> potential hidden assumptions in the code base can be bisected to this >> simple patch. > > No, please don't. The rationale for putting ggtt first is because it > puts the one vma that every object is likely to use at the front, and > makes it also the first in the list for debugging. This came from talking with Daniel, but I don't understand why every object is likely to have a GGTT mapping? With multiple GGTT views it may happen that only single GGTT VMA exists in the list but it is not the one current code would expect. So the idea was to break that to flesh out any potential hidden assumptions in the code. (I did not find any by just looking though.) What kind of debugging you have in mind, error capture? Or actually inspecting memory of a live kernel? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-04 10:02 ` Tvrtko Ursulin @ 2014-12-04 10:17 ` Chris Wilson 2014-12-04 10:30 ` Tvrtko Ursulin 0 siblings, 1 reply; 37+ messages in thread From: Chris Wilson @ 2014-12-04 10:17 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx On Thu, Dec 04, 2014 at 10:02:19AM +0000, Tvrtko Ursulin wrote: > > On 12/04/2014 09:48 AM, Chris Wilson wrote: > >On Wed, Dec 03, 2014 at 02:59:24PM +0000, Tvrtko Ursulin wrote: > >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >>Multiple GGTT VMAs per object will be introduced in the near future which will > >>make it impossible to guarantee normal GGTT view is at the head of the list. > >> > >>Purpose of this patch is to break this assumption straight away so any > >>potential hidden assumptions in the code base can be bisected to this > >>simple patch. > > > >No, please don't. The rationale for putting ggtt first is because it > >puts the one vma that every object is likely to use at the front, and > >makes it also the first in the list for debugging. > > This came from talking with Daniel, but I don't understand why every > object is likely to have a GGTT mapping? Because the userspace usage pattern is such that it is typical that every object is accessed through the GTT at some point in its life. It's actually becoming less so as we use alternative mmappings, but it will remain so for quite some time yet. > With multiple GGTT views it may happen that only single GGTT VMA > exists in the list but it is not the one current code would expect. > So the idea was to break that to flesh out any potential hidden > assumptions in the code. (I did not find any by just looking > though.) > > What kind of debugging you have in mind, error capture? Or actually > inspecting memory of a live kernel? Error capture inspection of memory of a live kernel. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-04 10:17 ` Chris Wilson @ 2014-12-04 10:30 ` Tvrtko Ursulin 2014-12-04 10:39 ` Chris Wilson 0 siblings, 1 reply; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-04 10:30 UTC (permalink / raw) To: Chris Wilson, Intel-gfx On 12/04/2014 10:17 AM, Chris Wilson wrote: > On Thu, Dec 04, 2014 at 10:02:19AM +0000, Tvrtko Ursulin wrote: >> >> On 12/04/2014 09:48 AM, Chris Wilson wrote: >>> On Wed, Dec 03, 2014 at 02:59:24PM +0000, Tvrtko Ursulin wrote: >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> >>>> Multiple GGTT VMAs per object will be introduced in the near future which will >>>> make it impossible to guarantee normal GGTT view is at the head of the list. >>>> >>>> Purpose of this patch is to break this assumption straight away so any >>>> potential hidden assumptions in the code base can be bisected to this >>>> simple patch. >>> >>> No, please don't. The rationale for putting ggtt first is because it >>> puts the one vma that every object is likely to use at the front, and >>> makes it also the first in the list for debugging. >> >> This came from talking with Daniel, but I don't understand why every >> object is likely to have a GGTT mapping? > > Because the userspace usage pattern is such that it is typical that > every object is accessed through the GTT at some point in its life. > > It's actually becoming less so as we use alternative mmappings, but it > will remain so for quite some time yet. > >> With multiple GGTT views it may happen that only single GGTT VMA >> exists in the list but it is not the one current code would expect. >> So the idea was to break that to flesh out any potential hidden >> assumptions in the code. (I did not find any by just looking >> though.) >> >> What kind of debugging you have in mind, error capture? Or actually >> inspecting memory of a live kernel? > > Error capture inspection of memory of a live kernel. So personally you don't think it should be of any concern if a GGTT VMA is at the head of the list, but it is not the same GGTT VMA which you would find there in majority of cases? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-04 10:30 ` Tvrtko Ursulin @ 2014-12-04 10:39 ` Chris Wilson 2014-12-04 10:48 ` Tvrtko Ursulin 0 siblings, 1 reply; 37+ messages in thread From: Chris Wilson @ 2014-12-04 10:39 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx On Thu, Dec 04, 2014 at 10:30:30AM +0000, Tvrtko Ursulin wrote: > > On 12/04/2014 10:17 AM, Chris Wilson wrote: > >On Thu, Dec 04, 2014 at 10:02:19AM +0000, Tvrtko Ursulin wrote: > >> > >>On 12/04/2014 09:48 AM, Chris Wilson wrote: > >>>On Wed, Dec 03, 2014 at 02:59:24PM +0000, Tvrtko Ursulin wrote: > >>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>> > >>>>Multiple GGTT VMAs per object will be introduced in the near future which will > >>>>make it impossible to guarantee normal GGTT view is at the head of the list. > >>>> > >>>>Purpose of this patch is to break this assumption straight away so any > >>>>potential hidden assumptions in the code base can be bisected to this > >>>>simple patch. > >>> > >>>No, please don't. The rationale for putting ggtt first is because it > >>>puts the one vma that every object is likely to use at the front, and > >>>makes it also the first in the list for debugging. > >> > >>This came from talking with Daniel, but I don't understand why every > >>object is likely to have a GGTT mapping? > > > >Because the userspace usage pattern is such that it is typical that > >every object is accessed through the GTT at some point in its life. > > > >It's actually becoming less so as we use alternative mmappings, but it > >will remain so for quite some time yet. > > > >>With multiple GGTT views it may happen that only single GGTT VMA > >>exists in the list but it is not the one current code would expect. > >>So the idea was to break that to flesh out any potential hidden > >>assumptions in the code. (I did not find any by just looking > >>though.) > >> > >>What kind of debugging you have in mind, error capture? Or actually > >>inspecting memory of a live kernel? > > > >Error capture inspection of memory of a live kernel. > > So personally you don't think it should be of any concern if a GGTT > VMA is at the head of the list, but it is not the same GGTT VMA > which you would find there in majority of cases? Actually, regarding error capture it is sorting of the vm list that is important - so a different list. I am still confused about why having different GGTT views makes it difficult putting the normal GGTT vm first. I would very much like the standard GGTT view remain first, and I would actually like the MRU ppgtt last (so that the two most common uses are easily found). -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-04 10:39 ` Chris Wilson @ 2014-12-04 10:48 ` Tvrtko Ursulin 0 siblings, 0 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-04 10:48 UTC (permalink / raw) To: Chris Wilson, Intel-gfx, Daniel Vetter On 12/04/2014 10:39 AM, Chris Wilson wrote: > On Thu, Dec 04, 2014 at 10:30:30AM +0000, Tvrtko Ursulin wrote: >> So personally you don't think it should be of any concern if a GGTT >> VMA is at the head of the list, but it is not the same GGTT VMA >> which you would find there in majority of cases? > > Actually, regarding error capture it is sorting of the vm list that is > important - so a different list. > > I am still confused about why having different GGTT views makes it > difficult putting the normal GGTT vm first. I would very much like the > standard GGTT view remain first, and I would actually like the MRU > ppgtt last (so that the two most common uses are easily found). It is not difficult, but it is not guaranteed that, when there is a single GGTT vma on the list, it is a normal one. + Daniel since this is at least how I interpreted our discussion about this. Maybe I got it wrong, who knows. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list 2014-12-04 9:48 ` Chris Wilson 2014-12-04 10:02 ` Tvrtko Ursulin @ 2014-12-04 10:54 ` Daniel Vetter 1 sibling, 0 replies; 37+ messages in thread From: Daniel Vetter @ 2014-12-04 10:54 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx On Thu, Dec 04, 2014 at 09:48:11AM +0000, Chris Wilson wrote: > On Wed, Dec 03, 2014 at 02:59:24PM +0000, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > > Multiple GGTT VMAs per object will be introduced in the near future which will > > make it impossible to guarantee normal GGTT view is at the head of the list. > > > > Purpose of this patch is to break this assumption straight away so any > > potential hidden assumptions in the code base can be bisected to this > > simple patch. > > No, please don't. The rationale for putting ggtt first is because it > puts the one vma that every object is likely to use at the front, and > makes it also the first in the list for debugging. With hindsight I think this was just premature optimization (and way too many obj->vma lookups to boot). Imo we should just fix the remaining spurious obj->vma lookups we have and if there's still a problem then we need to have actual benchmark data or profiles to support this trick. Hence I pulled this patch in, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-03 14:59 [PATCH 0/3] Multiple GGTT views Tvrtko Ursulin 2014-12-03 14:59 ` [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list Tvrtko Ursulin @ 2014-12-03 14:59 ` Tvrtko Ursulin 2014-12-04 9:53 ` Chris Wilson 2014-12-03 14:59 ` [PATCH 3/3] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin ` (2 subsequent siblings) 4 siblings, 1 reply; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-03 14:59 UTC (permalink / raw) To: Intel-gfx; +Cc: Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Things like reliable GGTT mappings and mirrored 2d-on-3d display will need to map objects into the same address space multiple times. Added a GGTT view concept and linked it with the VMA to distinguish between 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. This now means that objects can have multiple VMA entries so the code which assumed there will only be one also had to be modified. Alternative GGTT views are supposed to borrow DMA addresses from obj->pages which is DMA mapped on first VMA instantiation and unmapped on the last one going away. v2: * Removed per view special casing in i915_gem_ggtt_prepare / finish_object in favour of creating and destroying DMA mappings on first VMA instantiation and last VMA destruction. (Daniel Vetter) * Simplified i915_vma_unbind which does not need to count the GGTT views. (Daniel Vetter) * Also moved obj->map_and_fenceable reset under the same check. * Checkpatch cleanups. v3: * Only retire objects once the last VMA is unbound. For: VIZ-4544 Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_debugfs.c | 5 +- drivers/gpu/drm/i915/i915_drv.h | 46 +++++++++++++++++-- drivers/gpu/drm/i915/i915_gem.c | 73 ++++++++++++++++-------------- drivers/gpu/drm/i915/i915_gem_context.c | 4 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 61 +++++++++++++++++++------ drivers/gpu/drm/i915/i915_gem_gtt.h | 22 ++++++++- drivers/gpu/drm/i915/i915_gpu_error.c | 8 +--- 8 files changed, 159 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 6c16939..bd08289 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -152,8 +152,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) seq_puts(m, " (pp"); else seq_puts(m, " (g"); - seq_printf(m, "gtt offset: %08lx, size: %08lx)", - vma->node.start, vma->node.size); + seq_printf(m, "gtt offset: %08lx, size: %08lx, type: %u)", + vma->node.start, vma->node.size, + vma->ggtt_view.type); } if (obj->stolen) seq_printf(m, " (stolen: %08lx)", obj->stolen->start); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 049482f..03c8132 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2514,10 +2514,23 @@ void i915_gem_vma_destroy(struct i915_vma *vma); #define PIN_GLOBAL 0x4 #define PIN_OFFSET_BIAS 0x8 #define PIN_OFFSET_MASK (~4095) +int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + uint32_t alignment, + uint64_t flags, + const struct i915_ggtt_view *view); +static inline int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm, uint32_t alignment, - uint64_t flags); + uint64_t flags) +{ + return i915_gem_object_pin_view(obj, vm, alignment, flags, + &i915_ggtt_view_normal); +} + +void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, + u32 flags); int __must_check i915_vma_unbind(struct i915_vma *vma); int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv); @@ -2679,18 +2692,43 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, void i915_gem_restore_fences(struct drm_device *dev); +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view); +static inline unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL); +} bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); bool i915_gem_obj_bound(struct drm_i915_gem_object *o, struct i915_address_space *vm); unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, struct i915_address_space *vm); +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view); +static inline struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal); +} + +struct i915_vma * +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view); + +static inline struct i915_vma * i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_lookup_or_create_vma_view(obj, vm, + &i915_ggtt_view_normal); +} struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj); static inline bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c1c1141..976597c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2087,8 +2087,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, /* For the unbound phase, this should be a no-op! */ list_for_each_entry_safe(vma, v, &obj->vma_list, vma_link) - if (i915_vma_unbind(vma)) - break; + i915_vma_unbind(vma); if (i915_gem_object_put_pages(obj) == 0) count += obj->base.size >> PAGE_SHIFT; @@ -2297,17 +2296,14 @@ void i915_vma_move_to_active(struct i915_vma *vma, static void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) { - struct drm_i915_private *dev_priv = obj->base.dev->dev_private; - struct i915_address_space *vm; struct i915_vma *vma; BUG_ON(obj->base.write_domain & ~I915_GEM_GPU_DOMAINS); BUG_ON(!obj->active); - list_for_each_entry(vm, &dev_priv->vm_list, global_link) { - vma = i915_gem_obj_to_vma(obj, vm); - if (vma && !list_empty(&vma->mm_list)) - list_move_tail(&vma->mm_list, &vm->inactive_list); + list_for_each_entry(vma, &obj->vma_list, vma_link) { + if (!list_empty(&vma->mm_list)) + list_move_tail(&vma->mm_list, &vma->vm->inactive_list); } intel_fb_obj_flush(obj, true); @@ -3062,10 +3058,8 @@ int i915_vma_unbind(struct i915_vma *vma) * cause memory corruption through use-after-free. */ - /* Throw away the active reference before moving to the unbound list */ - i915_gem_object_retire(obj); - - if (i915_is_ggtt(vma->vm)) { + if (i915_is_ggtt(vma->vm) && + vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { i915_gem_object_finish_gtt(obj); /* release the fence reg _after_ flushing */ @@ -3079,7 +3073,8 @@ int i915_vma_unbind(struct i915_vma *vma) vma->unbind_vma(vma); list_del_init(&vma->mm_list); - if (i915_is_ggtt(vma->vm)) + if (i915_is_ggtt(vma->vm) && + vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) obj->map_and_fenceable = false; drm_mm_remove_node(&vma->node); @@ -3088,6 +3083,10 @@ int i915_vma_unbind(struct i915_vma *vma) /* Since the unbound list is global, only move to that list if * no more VMAs exist. */ if (list_empty(&obj->vma_list)) { + /* Throw away the active reference before + * moving to the unbound list. */ + i915_gem_object_retire(obj); + i915_gem_gtt_finish_object(obj); list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list); } @@ -3498,7 +3497,8 @@ static struct i915_vma * i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, struct i915_address_space *vm, unsigned alignment, - uint64_t flags) + uint64_t flags, + const struct i915_ggtt_view *view) { struct drm_device *dev = obj->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -3548,7 +3548,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, i915_gem_object_pin_pages(obj); - vma = i915_gem_obj_lookup_or_create_vma(obj, vm); + vma = i915_gem_obj_lookup_or_create_vma_view(obj, vm, view); if (IS_ERR(vma)) goto err_unpin; @@ -3582,7 +3582,7 @@ search_free: list_add_tail(&vma->mm_list, &vm->inactive_list); trace_i915_vma_bind(vma, flags); - vma->bind_vma(vma, obj->cache_level, + i915_vma_bind(vma, obj->cache_level, flags & PIN_GLOBAL ? GLOBAL_BIND : 0); return vma; @@ -3790,8 +3790,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, list_for_each_entry(vma, &obj->vma_list, vma_link) if (drm_mm_node_allocated(&vma->node)) - vma->bind_vma(vma, cache_level, - vma->bound & GLOBAL_BIND); + i915_vma_bind(vma, cache_level, + vma->bound & GLOBAL_BIND); } list_for_each_entry(vma, &obj->vma_list, vma_link) @@ -4144,10 +4144,11 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags) } int -i915_gem_object_pin(struct drm_i915_gem_object *obj, - struct i915_address_space *vm, - uint32_t alignment, - uint64_t flags) +i915_gem_object_pin_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + uint32_t alignment, + uint64_t flags, + const struct i915_ggtt_view *view) { struct drm_i915_private *dev_priv = obj->base.dev->dev_private; struct i915_vma *vma; @@ -4163,7 +4164,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, if (WARN_ON((flags & (PIN_MAPPABLE | PIN_GLOBAL)) == PIN_MAPPABLE)) return -EINVAL; - vma = i915_gem_obj_to_vma(obj, vm); + vma = i915_gem_obj_to_vma_view(obj, vm, view); if (vma) { if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)) return -EBUSY; @@ -4173,7 +4174,8 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, "bo is already pinned with incorrect alignment:" " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d," " obj->map_and_fenceable=%d\n", - i915_gem_obj_offset(obj, vm), alignment, + i915_gem_obj_offset_view(obj, vm, view->type), + alignment, !!(flags & PIN_MAPPABLE), obj->map_and_fenceable); ret = i915_vma_unbind(vma); @@ -4186,13 +4188,14 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, bound = vma ? vma->bound : 0; if (vma == NULL || !drm_mm_node_allocated(&vma->node)) { - vma = i915_gem_object_bind_to_vm(obj, vm, alignment, flags); + vma = i915_gem_object_bind_to_vm(obj, vm, alignment, + flags, view); if (IS_ERR(vma)) return PTR_ERR(vma); } if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND)) - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); + i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND); if ((bound ^ vma->bound) & GLOBAL_BIND) { bool mappable, fenceable; @@ -4528,12 +4531,13 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) intel_runtime_pm_put(dev_priv); } -struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma; list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm) + if (vma->vm == vm && vma->ggtt_view.type == view->type) return vma; return NULL; @@ -5147,8 +5151,9 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) } /* All the new VM stuff */ -unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, - struct i915_address_space *vm) +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view) { struct drm_i915_private *dev_priv = o->base.dev->dev_private; struct i915_vma *vma; @@ -5156,7 +5161,7 @@ unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base); list_for_each_entry(vma, &o->vma_list, vma_link) { - if (vma->vm == vm) + if (vma->vm == vm && vma->ggtt_view.type == view) return vma->node.start; } @@ -5307,7 +5312,9 @@ struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj) struct i915_vma *vma; list_for_each_entry(vma, &obj->vma_list, vma_link) { - if (vma->vm == ggtt) + if (vma->vm != ggtt) + continue; + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) return vma; } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 5cd2b97..c2e6f2e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -591,8 +591,8 @@ static int do_switch(struct intel_engine_cs *ring, vma = i915_gem_obj_to_ggtt(to->legacy_hw_ctx.rcs_state); if (!(vma->bound & GLOBAL_BIND)) - vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level, - GLOBAL_BIND); + i915_vma_bind(vma, to->legacy_hw_ctx.rcs_state->cache_level, + GLOBAL_BIND); if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) hw_flags |= MI_RESTORE_INHIBIT; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0c25f62..56ac483 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -361,8 +361,8 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, if (unlikely(IS_GEN6(dev) && reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && !(target_vma->bound & GLOBAL_BIND))) - target_vma->bind_vma(target_vma, target_i915_obj->cache_level, - GLOBAL_BIND); + i915_vma_bind(target_vma, target_i915_obj->cache_level, + GLOBAL_BIND); /* Validate that the target is in a valid r/w GPU domain */ if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) { diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ac03a38..02eabf9 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -30,6 +30,8 @@ #include "i915_trace.h" #include "intel_drv.h" +const struct i915_ggtt_view i915_ggtt_view_normal; + static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); static void chv_setup_private_ppat(struct drm_i915_private *dev_priv); @@ -76,7 +78,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) } -static void ppgtt_bind_vma(struct i915_vma *vma, +static void ppgtt_bind_vma(struct i915_vma *vma, struct sg_table *pages, enum i915_cache_level cache_level, u32 flags); static void ppgtt_unbind_vma(struct i915_vma *vma); @@ -1200,7 +1202,7 @@ void i915_ppgtt_release(struct kref *kref) } static void -ppgtt_bind_vma(struct i915_vma *vma, +ppgtt_bind_vma(struct i915_vma *vma, struct sg_table *pages, enum i915_cache_level cache_level, u32 flags) { @@ -1208,7 +1210,7 @@ ppgtt_bind_vma(struct i915_vma *vma, if (vma->obj->gt_ro) flags |= PTE_READ_ONLY; - vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start, + vma->vm->insert_entries(vma->vm, pages, vma->node.start, cache_level, flags); } @@ -1343,7 +1345,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev) * without telling our object about it. So we need to fake it. */ vma->bound &= ~GLOBAL_BIND; - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); + i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND); } @@ -1529,7 +1531,7 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm, } -static void i915_ggtt_bind_vma(struct i915_vma *vma, +static void i915_ggtt_bind_vma(struct i915_vma *vma, struct sg_table *pages, enum i915_cache_level cache_level, u32 unused) { @@ -1538,7 +1540,7 @@ static void i915_ggtt_bind_vma(struct i915_vma *vma, AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY; BUG_ON(!i915_is_ggtt(vma->vm)); - intel_gtt_insert_sg_entries(vma->obj->pages, entry, flags); + intel_gtt_insert_sg_entries(pages, entry, flags); vma->bound = GLOBAL_BIND; } @@ -1562,7 +1564,7 @@ static void i915_ggtt_unbind_vma(struct i915_vma *vma) intel_gtt_clear_range(first, size); } -static void ggtt_bind_vma(struct i915_vma *vma, +static void ggtt_bind_vma(struct i915_vma *vma, struct sg_table *pages, enum i915_cache_level cache_level, u32 flags) { @@ -1588,7 +1590,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) { if (!(vma->bound & GLOBAL_BIND) || (cache_level != obj->cache_level)) { - vma->vm->insert_entries(vma->vm, obj->pages, + vma->vm->insert_entries(vma->vm, pages, vma->node.start, cache_level, flags); vma->bound |= GLOBAL_BIND; @@ -1600,7 +1602,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, (cache_level != obj->cache_level))) { struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt; appgtt->base.insert_entries(&appgtt->base, - vma->obj->pages, + pages, vma->node.start, cache_level, flags); vma->bound |= LOCAL_BIND; @@ -2165,7 +2167,8 @@ int i915_gem_gtt_init(struct drm_device *dev) } static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); if (vma == NULL) @@ -2176,6 +2179,7 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, INIT_LIST_HEAD(&vma->exec_list); vma->vm = vm; vma->obj = obj; + vma->ggtt_view = *view; switch (INTEL_INFO(vm->dev)->gen) { case 9: @@ -2210,14 +2214,43 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, } struct i915_vma * -i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma; - vma = i915_gem_obj_to_vma(obj, vm); + vma = i915_gem_obj_to_vma_view(obj, vm, view); if (!vma) - vma = __i915_gem_vma_create(obj, vm); + vma = __i915_gem_vma_create(obj, vm, view); return vma; } + +static inline +struct sg_table *i915_ggtt_view_pages(struct i915_vma *vma) +{ + struct sg_table *pages = ERR_PTR(-EINVAL); + + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) + pages = vma->obj->pages; + else + WARN(1, "Not implemented!\n"); + + return pages; +} + +void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, + u32 flags) +{ + struct sg_table *pages = i915_ggtt_view_pages(vma); + + if (pages && !IS_ERR(pages)) { + vma->bind_vma(vma, pages, cache_level, flags); + + if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) { + sg_free_table(pages); + kfree(pages); + } + } +} diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index dd849df..a6a0e27 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -109,7 +109,18 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; #define GEN8_PPAT_ELLC_OVERRIDE (0<<2) #define GEN8_PPAT(i, x) ((uint64_t) (x) << ((i) * 8)) +enum i915_ggtt_view_type { + I915_GGTT_VIEW_NORMAL = 0, +}; + +struct i915_ggtt_view { + enum i915_ggtt_view_type type; +}; + +extern const struct i915_ggtt_view i915_ggtt_view_normal; + enum i915_cache_level; + /** * A VMA represents a GEM BO that is bound into an address space. Therefore, a * VMA's presence cannot be guaranteed before binding, or after unbinding the @@ -129,6 +140,15 @@ struct i915_vma { #define PTE_READ_ONLY (1<<2) unsigned int bound : 4; + /** + * Support different GGTT views into the same object. + * This means there can be multiple VMA mappings per object and per VM. + * i915_ggtt_view_type is used to distinguish between those entries. + * The default one of zero (I915_GGTT_VIEW_NORMAL) is default and also + * assumed in GEM functions which take no ggtt view parameter. + */ + struct i915_ggtt_view ggtt_view; + /** This object's place on the active/inactive lists */ struct list_head mm_list; @@ -160,7 +180,7 @@ struct i915_vma { * setting the valid PTE entries to a reserved scratch page. */ void (*unbind_vma)(struct i915_vma *vma); /* Map an object into an address space with the given cache flags. */ - void (*bind_vma)(struct i915_vma *vma, + void (*bind_vma)(struct i915_vma *vma, struct sg_table *pages, enum i915_cache_level cache_level, u32 flags); }; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c4536e1..f97479a 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -718,10 +718,8 @@ static u32 capture_pinned_bo(struct drm_i915_error_buffer *err, break; list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm && vma->pin_count > 0) { + if (vma->vm == vm && vma->pin_count > 0) capture_bo(err++, vma); - break; - } } return err - first; @@ -1096,10 +1094,8 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv, list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm && vma->pin_count > 0) { + if (vma->vm == vm && vma->pin_count > 0) i++; - break; - } } error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx]; -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-03 14:59 ` [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin @ 2014-12-04 9:53 ` Chris Wilson 2014-12-04 10:19 ` Tvrtko Ursulin 0 siblings, 1 reply; 37+ messages in thread From: Chris Wilson @ 2014-12-04 9:53 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx On Wed, Dec 03, 2014 at 02:59:25PM +0000, Tvrtko Ursulin wrote: > +void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > + u32 flags) > +{ > + struct sg_table *pages = i915_ggtt_view_pages(vma); > + > + if (pages && !IS_ERR(pages)) { > + vma->bind_vma(vma, pages, cache_level, flags); > + > + if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) { > + sg_free_table(pages); > + kfree(pages); > + } > + } > +} Stop. Even if the failure path is impossible with the present implementation, here you are masking the error only to go and pretend the binding succeeded. Don't be lazy, this is a very nasty bug that should be hit during igt - or else you are not testing well enough. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-04 9:53 ` Chris Wilson @ 2014-12-04 10:19 ` Tvrtko Ursulin 2014-12-04 10:26 ` Chris Wilson 0 siblings, 1 reply; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-04 10:19 UTC (permalink / raw) To: Chris Wilson, Intel-gfx, Daniel Vetter On 12/04/2014 09:53 AM, Chris Wilson wrote: > On Wed, Dec 03, 2014 at 02:59:25PM +0000, Tvrtko Ursulin wrote: >> +void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, >> + u32 flags) >> +{ >> + struct sg_table *pages = i915_ggtt_view_pages(vma); >> + >> + if (pages && !IS_ERR(pages)) { >> + vma->bind_vma(vma, pages, cache_level, flags); >> + >> + if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) { >> + sg_free_table(pages); >> + kfree(pages); >> + } >> + } >> +} > > Stop. Even if the failure path is impossible with the present > implementation, here you are masking the error only to go and pretend > the binding succeeded. > > Don't be lazy, this is a very nasty bug that should be hit during igt - > or else you are not testing well enough. Fair comment, even if a bit too assuming. I actually had this as TODO but somehow lost it. I don't have any ideas on how to provoke this to fail from an IGT? Even with future implementations it boils down to a couple of small allocations which would have to fail reliably. But will add the error path for it. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-04 10:19 ` Tvrtko Ursulin @ 2014-12-04 10:26 ` Chris Wilson 2014-12-04 10:59 ` Daniel Vetter 0 siblings, 1 reply; 37+ messages in thread From: Chris Wilson @ 2014-12-04 10:26 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx On Thu, Dec 04, 2014 at 10:19:09AM +0000, Tvrtko Ursulin wrote: > > On 12/04/2014 09:53 AM, Chris Wilson wrote: > >On Wed, Dec 03, 2014 at 02:59:25PM +0000, Tvrtko Ursulin wrote: > >>+void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > >>+ u32 flags) > >>+{ > >>+ struct sg_table *pages = i915_ggtt_view_pages(vma); > >>+ > >>+ if (pages && !IS_ERR(pages)) { > >>+ vma->bind_vma(vma, pages, cache_level, flags); > >>+ > >>+ if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) { > >>+ sg_free_table(pages); > >>+ kfree(pages); > >>+ } > >>+ } > >>+} > > > >Stop. Even if the failure path is impossible with the present > >implementation, here you are masking the error only to go and pretend > >the binding succeeded. > > > >Don't be lazy, this is a very nasty bug that should be hit during igt - > >or else you are not testing well enough. > > Fair comment, even if a bit too assuming. I actually had this as > TODO but somehow lost it. > > I don't have any ideas on how to provoke this to fail from an IGT? > Even with future implementations it boils down to a couple of small > allocations which would have to fail reliably. We have quite a few thrash tests now that are fairly good at getting even the small allocations to fail. What we don't have is a single-fd, multi-ctx thrash test (well except for some GL tests...) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-04 10:26 ` Chris Wilson @ 2014-12-04 10:59 ` Daniel Vetter 2014-12-04 12:17 ` Tvrtko Ursulin 0 siblings, 1 reply; 37+ messages in thread From: Daniel Vetter @ 2014-12-04 10:59 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Daniel Vetter On Thu, Dec 04, 2014 at 10:26:14AM +0000, Chris Wilson wrote: > On Thu, Dec 04, 2014 at 10:19:09AM +0000, Tvrtko Ursulin wrote: > > > > On 12/04/2014 09:53 AM, Chris Wilson wrote: > > >On Wed, Dec 03, 2014 at 02:59:25PM +0000, Tvrtko Ursulin wrote: > > >>+void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > > >>+ u32 flags) > > >>+{ > > >>+ struct sg_table *pages = i915_ggtt_view_pages(vma); > > >>+ > > >>+ if (pages && !IS_ERR(pages)) { > > >>+ vma->bind_vma(vma, pages, cache_level, flags); > > >>+ > > >>+ if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) { > > >>+ sg_free_table(pages); > > >>+ kfree(pages); > > >>+ } > > >>+ } > > >>+} > > > > > >Stop. Even if the failure path is impossible with the present > > >implementation, here you are masking the error only to go and pretend > > >the binding succeeded. > > > > > >Don't be lazy, this is a very nasty bug that should be hit during igt - > > >or else you are not testing well enough. > > > > Fair comment, even if a bit too assuming. I actually had this as > > TODO but somehow lost it. > > > > I don't have any ideas on how to provoke this to fail from an IGT? > > Even with future implementations it boils down to a couple of small > > allocations which would have to fail reliably. > > We have quite a few thrash tests now that are fairly good at getting > even the small allocations to fail. > > What we don't have is a single-fd, multi-ctx thrash test (well except > for some GL tests...) But none of these tests result in permanent memory failures (only the occasional ioctl restart when waiting for gpu rendering). And sg table alloc only recurses through the shrinker so that can't happen. So I think we just have to get by with review. We did have issues with sg table allocations in stress tests though, before we've added the recursive shrinker locking, hence sg table alloc can indeed go south. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-04 10:59 ` Daniel Vetter @ 2014-12-04 12:17 ` Tvrtko Ursulin 2014-12-04 12:27 ` Chris Wilson 2014-12-04 13:01 ` Daniel Vetter 0 siblings, 2 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-04 12:17 UTC (permalink / raw) To: Daniel Vetter, Chris Wilson, Intel-gfx, Daniel Vetter On 12/04/2014 10:59 AM, Daniel Vetter wrote: > On Thu, Dec 04, 2014 at 10:26:14AM +0000, Chris Wilson wrote: >> On Thu, Dec 04, 2014 at 10:19:09AM +0000, Tvrtko Ursulin wrote: >>> >>> On 12/04/2014 09:53 AM, Chris Wilson wrote: >>>> On Wed, Dec 03, 2014 at 02:59:25PM +0000, Tvrtko Ursulin wrote: >>>>> +void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, >>>>> + u32 flags) >>>>> +{ >>>>> + struct sg_table *pages = i915_ggtt_view_pages(vma); >>>>> + >>>>> + if (pages && !IS_ERR(pages)) { >>>>> + vma->bind_vma(vma, pages, cache_level, flags); >>>>> + >>>>> + if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) { >>>>> + sg_free_table(pages); >>>>> + kfree(pages); >>>>> + } >>>>> + } >>>>> +} >>>> >>>> Stop. Even if the failure path is impossible with the present >>>> implementation, here you are masking the error only to go and pretend >>>> the binding succeeded. >>>> >>>> Don't be lazy, this is a very nasty bug that should be hit during igt - >>>> or else you are not testing well enough. >>> >>> Fair comment, even if a bit too assuming. I actually had this as >>> TODO but somehow lost it. >>> >>> I don't have any ideas on how to provoke this to fail from an IGT? >>> Even with future implementations it boils down to a couple of small >>> allocations which would have to fail reliably. >> >> We have quite a few thrash tests now that are fairly good at getting >> even the small allocations to fail. >> >> What we don't have is a single-fd, multi-ctx thrash test (well except >> for some GL tests...) > > But none of these tests result in permanent memory failures (only the > occasional ioctl restart when waiting for gpu rendering). And sg table > alloc only recurses through the shrinker so that can't happen. So I think > we just have to get by with review. > > We did have issues with sg table allocations in stress tests though, > before we've added the recursive shrinker locking, hence sg table alloc > can indeed go south. I looked at propagating errors from i915_vma_bind() out to callers and it is mostly all fine apart from the i915_gem_restore_gtt_mappings during i915_drm_resume. I don't see how this is fixable apart by going back and having sgls stay around for the lifetime of their VMAs. It shouldn't be such a big deal - they are not so big even with non-coalesced entries. Thoughts? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-04 12:17 ` Tvrtko Ursulin @ 2014-12-04 12:27 ` Chris Wilson 2014-12-04 13:01 ` Daniel Vetter 1 sibling, 0 replies; 37+ messages in thread From: Chris Wilson @ 2014-12-04 12:27 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx On Thu, Dec 04, 2014 at 12:17:51PM +0000, Tvrtko Ursulin wrote: > On 12/04/2014 10:59 AM, Daniel Vetter wrote: > >On Thu, Dec 04, 2014 at 10:26:14AM +0000, Chris Wilson wrote: > >>On Thu, Dec 04, 2014 at 10:19:09AM +0000, Tvrtko Ursulin wrote: > >>> > >>>On 12/04/2014 09:53 AM, Chris Wilson wrote: > >>>>On Wed, Dec 03, 2014 at 02:59:25PM +0000, Tvrtko Ursulin wrote: > >>>>>+void i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > >>>>>+ u32 flags) > >>>>>+{ > >>>>>+ struct sg_table *pages = i915_ggtt_view_pages(vma); > >>>>>+ > >>>>>+ if (pages && !IS_ERR(pages)) { > >>>>>+ vma->bind_vma(vma, pages, cache_level, flags); > >>>>>+ > >>>>>+ if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) { > >>>>>+ sg_free_table(pages); > >>>>>+ kfree(pages); > >>>>>+ } > >>>>>+ } > >>>>>+} > >>>> > >>>>Stop. Even if the failure path is impossible with the present > >>>>implementation, here you are masking the error only to go and pretend > >>>>the binding succeeded. > >>>> > >>>>Don't be lazy, this is a very nasty bug that should be hit during igt - > >>>>or else you are not testing well enough. > >>> > >>>Fair comment, even if a bit too assuming. I actually had this as > >>>TODO but somehow lost it. > >>> > >>>I don't have any ideas on how to provoke this to fail from an IGT? > >>>Even with future implementations it boils down to a couple of small > >>>allocations which would have to fail reliably. > >> > >>We have quite a few thrash tests now that are fairly good at getting > >>even the small allocations to fail. > >> > >>What we don't have is a single-fd, multi-ctx thrash test (well except > >>for some GL tests...) > > > >But none of these tests result in permanent memory failures (only the > >occasional ioctl restart when waiting for gpu rendering). And sg table > >alloc only recurses through the shrinker so that can't happen. So I think > >we just have to get by with review. > > > >We did have issues with sg table allocations in stress tests though, > >before we've added the recursive shrinker locking, hence sg table alloc > >can indeed go south. > > I looked at propagating errors from i915_vma_bind() out to callers > and it is mostly all fine apart from the > i915_gem_restore_gtt_mappings during i915_drm_resume. > > I don't see how this is fixable apart by going back and having sgls > stay around for the lifetime of their VMAs. It shouldn't be such a > big deal - they are not so big even with non-coalesced entries. The good news is that for a rebind, you already have the sg lists already allocated and converted to dma addresses, and so you can just do something like: static inline void i915_vma_rebind(struct i915_vma *vma, enum i915_cache_level cache_level) { unsigned rebind; rebind = vma->bound; vma->bound = 0; WARN_ON(vma->bind_vma(vma, cache_level, rebind)); } -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-04 12:17 ` Tvrtko Ursulin 2014-12-04 12:27 ` Chris Wilson @ 2014-12-04 13:01 ` Daniel Vetter 1 sibling, 0 replies; 37+ messages in thread From: Daniel Vetter @ 2014-12-04 13:01 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx On Thu, Dec 4, 2014 at 1:17 PM, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > I looked at propagating errors from i915_vma_bind() out to callers and it is > mostly all fine apart from the i915_gem_restore_gtt_mappings during > i915_drm_resume. Bummer. > I don't see how this is fixable apart by going back and having sgls stay > around for the lifetime of their VMAs. It shouldn't be such a big deal - > they are not so big even with non-coalesced entries. > > Thoughts? I guess we'll just have to eat the additional pointer in vma structs and that's it. If you want to go fancy we could have a new subtype: struct i915_vma_special_view { struct i915_vma base; struct sg_table *sgt; } And then typecast accordingly for non-normal views. Feels a bit like overkill though, at least until we start using vmas for other crazy things which need special handling. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 3/3] drm/i915: Documentation for multiple GGTT views. 2014-12-03 14:59 [PATCH 0/3] Multiple GGTT views Tvrtko Ursulin 2014-12-03 14:59 ` [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list Tvrtko Ursulin 2014-12-03 14:59 ` [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin @ 2014-12-03 14:59 ` Tvrtko Ursulin 2014-12-04 7:12 ` shuang.he 2014-12-05 12:11 ` [PATCH v2 0/2] Multiple " Tvrtko Ursulin 2014-12-10 17:27 ` [PATCH v3 0/2] Multiple " Tvrtko Ursulin 4 siblings, 1 reply; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-03 14:59 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> A short section describing background, implementation and intended usage. For: VIZ-4544 Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- Documentation/DocBook/drm.tmpl | 5 +++ drivers/gpu/drm/i915/i915_gem_gtt.c | 61 +++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 56e2a9b..c975f26 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4033,6 +4033,11 @@ int num_ioctls;</synopsis> !Pdrivers/gpu/drm/i915/intel_lrc.c Logical Rings, Logical Ring Contexts and Execlists !Idrivers/gpu/drm/i915/intel_lrc.c </sect2> + <sect2> + <title>Global GTT views</title> +!Pdrivers/gpu/drm/i915/i915_gem_gtt.c Global GTT views +!Idrivers/gpu/drm/i915/i915_gem_gtt.c + </sect2> </sect1> <sect1> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 02eabf9..633f063 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -30,6 +30,67 @@ #include "i915_trace.h" #include "intel_drv.h" +/** + * DOC: Global GTT views (GGTT) + * + * Background and previous state + * + * Historically objects could exists (be bound) in global GTT space only as + * singular instances with a view representing all of the object's backing pages + * in a linear fashion. This view will be called a normal view. + * + * To support multiple views of the same object, where the number of mapped + * pages is not equal to the backing store, or where the layout of the pages + * is not linear, concept of a GGTT view was added. + * + * One example of an alternative view is a stereo display driven by a single + * image. In this case we would have a framebuffer looking like this + * (2x2 pages): + * + * 12 + * 34 + * + * Above would represent a normal GGTT view as normally mapped for GPU or CPU + * rendering. In contrast, fed to the display engine would be an alternative + * view which could look something like this: + * + * 1212 + * 3434 + * + * In this example both the size and layout of pages in the alternative view is + * different from the normal view. + * + * Implementation and usage + * + * GGTT views are implemented using VMAs and are distinguished via enum + * i915_ggtt_view_type and struct i915_ggtt_view. + * + * A new flavour of core GEM functions which work with GGTT bound objects were + * added with the _view suffix. They take the struct i915_ggtt_view parameter + * encapsulating all metadata required to implement a view. + * + * As a helper for callers which are only interested in the normal view, + * globally const i915_ggtt_view_normal singleton instance exists. All old core + * GEM API functions, the ones not taking the view parameter, are operating on, + * or with the normal GGTT view. + * + * Code wanting to add or use a new GGTT view needs to: + * + * 1. Add a new enum. + * 2. Extend the metadata in the i915_ggtt_view structure if required. + * 3. Add support to i915_ggtt_view_pages(). + * + * New views are required to build a throw-away scatter-gather table + * at the time they are bound into the GGTT space (3rd item from above). This is + * intended to simplify the code and is not considered a performance sensitive + * path at the moment. + * + * Core API is designed to have copy semantics which means that passed in + * struct i915_ggtt_view does not need to be persistent (left around after + * calling the core API functions). + * + */ + const struct i915_ggtt_view i915_ggtt_view_normal; static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 3/3] drm/i915: Documentation for multiple GGTT views. 2014-12-03 14:59 ` [PATCH 3/3] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin @ 2014-12-04 7:12 ` shuang.he 0 siblings, 0 replies; 37+ messages in thread From: shuang.he @ 2014-12-04 7:12 UTC (permalink / raw) To: shuang.he, intel-gfx, tvrtko.ursulin Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK -1 365/366 364/366 SNB 450/450 450/450 IVB 498/498 498/498 BYT 289/289 289/289 HSW -1 564/564 563/564 BDW 417/417 417/417 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied ILK igt_kms_flip_plain-flip-ts-check-interruptible DMESG_WARN(2, M26)PASS(4, M26) DMESG_WARN(1, M26) *HSW igt_kms_plane_plane-panning-bottom-right-pipe-B-plane-2 PASS(2, M19M40) DMESG_WARN(1, M40) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 0/2] Multiple GGTT views 2014-12-03 14:59 [PATCH 0/3] Multiple GGTT views Tvrtko Ursulin ` (2 preceding siblings ...) 2014-12-03 14:59 ` [PATCH 3/3] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin @ 2014-12-05 12:11 ` Tvrtko Ursulin 2014-12-05 12:11 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin 2014-12-05 12:11 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 2014-12-10 17:27 ` [PATCH v3 0/2] Multiple " Tvrtko Ursulin 4 siblings, 2 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-05 12:11 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> This series continues what was previously a single patch called "drm/i915: Infrastructure for supporting different GGTT views per object". v2: * One less patch due early prep work getting merged. * Main patch reworked to keep the pages pointer around for the lifetime of the VMA. More detailed changelog in the patch itself. Tvrtko Ursulin (2): drm/i915: Infrastructure for supporting different GGTT views per object drm/i915: Documentation for multiple GGTT views Documentation/DocBook/drm.tmpl | 5 ++ drivers/gpu/drm/i915/i915_debugfs.c | 5 +- drivers/gpu/drm/i915/i915_drv.h | 46 +++++++++- drivers/gpu/drm/i915/i915_gem.c | 101 +++++++++++++--------- drivers/gpu/drm/i915/i915_gem_context.c | 11 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 130 +++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_gem_gtt.h | 22 +++++ drivers/gpu/drm/i915/i915_gpu_error.c | 8 +- 9 files changed, 271 insertions(+), 66 deletions(-) -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-05 12:11 ` [PATCH v2 0/2] Multiple " Tvrtko Ursulin @ 2014-12-05 12:11 ` Tvrtko Ursulin 2014-12-05 14:25 ` Daniel Vetter 2014-12-09 15:23 ` Michel Thierry 2014-12-05 12:11 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 1 sibling, 2 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-05 12:11 UTC (permalink / raw) To: Intel-gfx; +Cc: Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Things like reliable GGTT mappings and mirrored 2d-on-3d display will need to map objects into the same address space multiple times. Added a GGTT view concept and linked it with the VMA to distinguish between 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. This now means that objects can have multiple VMA entries so the code which assumed there will only be one also had to be modified. Alternative GGTT views are supposed to borrow DMA addresses from obj->pages which is DMA mapped on first VMA instantiation and unmapped on the last one going away. v2: * Removed per view special casing in i915_gem_ggtt_prepare / finish_object in favour of creating and destroying DMA mappings on first VMA instantiation and last VMA destruction. (Daniel Vetter) * Simplified i915_vma_unbind which does not need to count the GGTT views. (Daniel Vetter) * Also moved obj->map_and_fenceable reset under the same check. * Checkpatch cleanups. v3: * Only retire objects once the last VMA is unbound. v4: * Keep scatter-gather table for alternative views persistent for the lifetime of the VMA. * Propagate binding errors to callers and handle appropriately. For: VIZ-4544 Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_debugfs.c | 5 +- drivers/gpu/drm/i915/i915_drv.h | 46 +++++++++++-- drivers/gpu/drm/i915/i915_gem.c | 101 ++++++++++++++++++----------- drivers/gpu/drm/i915/i915_gem_context.c | 11 +++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++- drivers/gpu/drm/i915/i915_gem_gtt.c | 70 +++++++++++++++++--- drivers/gpu/drm/i915/i915_gem_gtt.h | 22 +++++++ drivers/gpu/drm/i915/i915_gpu_error.c | 8 +-- 8 files changed, 206 insertions(+), 66 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 6c16939..bd08289 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -152,8 +152,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) seq_puts(m, " (pp"); else seq_puts(m, " (g"); - seq_printf(m, "gtt offset: %08lx, size: %08lx)", - vma->node.start, vma->node.size); + seq_printf(m, "gtt offset: %08lx, size: %08lx, type: %u)", + vma->node.start, vma->node.size, + vma->ggtt_view.type); } if (obj->stolen) seq_printf(m, " (stolen: %08lx)", obj->stolen->start); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 049482f..b2f6f7d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2514,10 +2514,23 @@ void i915_gem_vma_destroy(struct i915_vma *vma); #define PIN_GLOBAL 0x4 #define PIN_OFFSET_BIAS 0x8 #define PIN_OFFSET_MASK (~4095) +int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + uint32_t alignment, + uint64_t flags, + const struct i915_ggtt_view *view); +static inline int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm, uint32_t alignment, - uint64_t flags); + uint64_t flags) +{ + return i915_gem_object_pin_view(obj, vm, alignment, flags, + &i915_ggtt_view_normal); +} + +int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, + u32 flags); int __must_check i915_vma_unbind(struct i915_vma *vma); int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv); @@ -2679,18 +2692,43 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, void i915_gem_restore_fences(struct drm_device *dev); +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view); +static inline unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL); +} bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); bool i915_gem_obj_bound(struct drm_i915_gem_object *o, struct i915_address_space *vm); unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, struct i915_address_space *vm); +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view); +static inline struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal); +} + +struct i915_vma * +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view); + +static inline struct i915_vma * i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_lookup_or_create_vma_view(obj, vm, + &i915_ggtt_view_normal); +} struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj); static inline bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c1c1141..9244f0b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2087,8 +2087,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, /* For the unbound phase, this should be a no-op! */ list_for_each_entry_safe(vma, v, &obj->vma_list, vma_link) - if (i915_vma_unbind(vma)) - break; + i915_vma_unbind(vma); if (i915_gem_object_put_pages(obj) == 0) count += obj->base.size >> PAGE_SHIFT; @@ -2297,17 +2296,14 @@ void i915_vma_move_to_active(struct i915_vma *vma, static void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) { - struct drm_i915_private *dev_priv = obj->base.dev->dev_private; - struct i915_address_space *vm; struct i915_vma *vma; BUG_ON(obj->base.write_domain & ~I915_GEM_GPU_DOMAINS); BUG_ON(!obj->active); - list_for_each_entry(vm, &dev_priv->vm_list, global_link) { - vma = i915_gem_obj_to_vma(obj, vm); - if (vma && !list_empty(&vma->mm_list)) - list_move_tail(&vma->mm_list, &vm->inactive_list); + list_for_each_entry(vma, &obj->vma_list, vma_link) { + if (!list_empty(&vma->mm_list)) + list_move_tail(&vma->mm_list, &vma->vm->inactive_list); } intel_fb_obj_flush(obj, true); @@ -3062,10 +3058,8 @@ int i915_vma_unbind(struct i915_vma *vma) * cause memory corruption through use-after-free. */ - /* Throw away the active reference before moving to the unbound list */ - i915_gem_object_retire(obj); - - if (i915_is_ggtt(vma->vm)) { + if (i915_is_ggtt(vma->vm) && + vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { i915_gem_object_finish_gtt(obj); /* release the fence reg _after_ flushing */ @@ -3079,8 +3073,15 @@ int i915_vma_unbind(struct i915_vma *vma) vma->unbind_vma(vma); list_del_init(&vma->mm_list); - if (i915_is_ggtt(vma->vm)) - obj->map_and_fenceable = false; + if (i915_is_ggtt(vma->vm)) { + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { + obj->map_and_fenceable = false; + } else if (vma->ggtt_view.pages) { + sg_free_table(vma->ggtt_view.pages); + kfree(vma->ggtt_view.pages); + vma->ggtt_view.pages = NULL; + } + } drm_mm_remove_node(&vma->node); i915_gem_vma_destroy(vma); @@ -3088,6 +3089,10 @@ int i915_vma_unbind(struct i915_vma *vma) /* Since the unbound list is global, only move to that list if * no more VMAs exist. */ if (list_empty(&obj->vma_list)) { + /* Throw away the active reference before + * moving to the unbound list. */ + i915_gem_object_retire(obj); + i915_gem_gtt_finish_object(obj); list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list); } @@ -3498,7 +3503,8 @@ static struct i915_vma * i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, struct i915_address_space *vm, unsigned alignment, - uint64_t flags) + uint64_t flags, + const struct i915_ggtt_view *view) { struct drm_device *dev = obj->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -3548,7 +3554,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, i915_gem_object_pin_pages(obj); - vma = i915_gem_obj_lookup_or_create_vma(obj, vm); + vma = i915_gem_obj_lookup_or_create_vma_view(obj, vm, view); if (IS_ERR(vma)) goto err_unpin; @@ -3578,15 +3584,19 @@ search_free: if (ret) goto err_remove_node; + trace_i915_vma_bind(vma, flags); + ret = i915_vma_bind(vma, obj->cache_level, + flags & PIN_GLOBAL ? GLOBAL_BIND : 0); + if (ret) + goto err_finish_gtt; + list_move_tail(&obj->global_list, &dev_priv->mm.bound_list); list_add_tail(&vma->mm_list, &vm->inactive_list); - trace_i915_vma_bind(vma, flags); - vma->bind_vma(vma, obj->cache_level, - flags & PIN_GLOBAL ? GLOBAL_BIND : 0); - return vma; +err_finish_gtt: + i915_gem_gtt_finish_object(obj); err_remove_node: drm_mm_remove_node(&vma->node); err_free_vma: @@ -3789,9 +3799,12 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, } list_for_each_entry(vma, &obj->vma_list, vma_link) - if (drm_mm_node_allocated(&vma->node)) - vma->bind_vma(vma, cache_level, - vma->bound & GLOBAL_BIND); + if (drm_mm_node_allocated(&vma->node)) { + ret = i915_vma_bind(vma, cache_level, + vma->bound & GLOBAL_BIND); + if (ret); + return ret; + } } list_for_each_entry(vma, &obj->vma_list, vma_link) @@ -4144,10 +4157,11 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags) } int -i915_gem_object_pin(struct drm_i915_gem_object *obj, - struct i915_address_space *vm, - uint32_t alignment, - uint64_t flags) +i915_gem_object_pin_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + uint32_t alignment, + uint64_t flags, + const struct i915_ggtt_view *view) { struct drm_i915_private *dev_priv = obj->base.dev->dev_private; struct i915_vma *vma; @@ -4163,7 +4177,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, if (WARN_ON((flags & (PIN_MAPPABLE | PIN_GLOBAL)) == PIN_MAPPABLE)) return -EINVAL; - vma = i915_gem_obj_to_vma(obj, vm); + vma = i915_gem_obj_to_vma_view(obj, vm, view); if (vma) { if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)) return -EBUSY; @@ -4173,7 +4187,8 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, "bo is already pinned with incorrect alignment:" " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d," " obj->map_and_fenceable=%d\n", - i915_gem_obj_offset(obj, vm), alignment, + i915_gem_obj_offset_view(obj, vm, view->type), + alignment, !!(flags & PIN_MAPPABLE), obj->map_and_fenceable); ret = i915_vma_unbind(vma); @@ -4186,13 +4201,17 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, bound = vma ? vma->bound : 0; if (vma == NULL || !drm_mm_node_allocated(&vma->node)) { - vma = i915_gem_object_bind_to_vm(obj, vm, alignment, flags); + vma = i915_gem_object_bind_to_vm(obj, vm, alignment, + flags, view); if (IS_ERR(vma)) return PTR_ERR(vma); } - if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND)) - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); + if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND)) { + ret = i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND); + if (ret) + return ret; + } if ((bound ^ vma->bound) & GLOBAL_BIND) { bool mappable, fenceable; @@ -4528,12 +4547,13 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) intel_runtime_pm_put(dev_priv); } -struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma; list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm) + if (vma->vm == vm && vma->ggtt_view.type == view->type) return vma; return NULL; @@ -5147,8 +5167,9 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) } /* All the new VM stuff */ -unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, - struct i915_address_space *vm) +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view) { struct drm_i915_private *dev_priv = o->base.dev->dev_private; struct i915_vma *vma; @@ -5156,7 +5177,7 @@ unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base); list_for_each_entry(vma, &o->vma_list, vma_link) { - if (vma->vm == vm) + if (vma->vm == vm && vma->ggtt_view.type == view) return vma->node.start; } @@ -5307,7 +5328,9 @@ struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj) struct i915_vma *vma; list_for_each_entry(vma, &obj->vma_list, vma_link) { - if (vma->vm == ggtt) + if (vma->vm != ggtt) + continue; + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) return vma; } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 5cd2b97..4ef8815 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -590,9 +590,14 @@ static int do_switch(struct intel_engine_cs *ring, goto unpin_out; vma = i915_gem_obj_to_ggtt(to->legacy_hw_ctx.rcs_state); - if (!(vma->bound & GLOBAL_BIND)) - vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level, - GLOBAL_BIND); + if (!(vma->bound & GLOBAL_BIND)) { + ret = i915_vma_bind(vma, + to->legacy_hw_ctx.rcs_state->cache_level, + GLOBAL_BIND); + /* This shouldn't ever fail. */ + if (WARN_ONCE(ret, "GGTT context bind failed!")) + goto unpin_out; + } if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) hw_flags |= MI_RESTORE_INHIBIT; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0c25f62..3927d93 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -360,9 +360,12 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, * through the ppgtt for non_secure batchbuffers. */ if (unlikely(IS_GEN6(dev) && reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && - !(target_vma->bound & GLOBAL_BIND))) - target_vma->bind_vma(target_vma, target_i915_obj->cache_level, - GLOBAL_BIND); + !(target_vma->bound & GLOBAL_BIND))) { + ret = i915_vma_bind(target_vma, target_i915_obj->cache_level, + GLOBAL_BIND); + if (WARN_ONCE(ret, "Unexpected failure to bind target VMA!")) + return ret; + } /* Validate that the target is in a valid r/w GPU domain */ if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) { diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ac03a38..73c1c0b 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -30,6 +30,8 @@ #include "i915_trace.h" #include "intel_drv.h" +const struct i915_ggtt_view i915_ggtt_view_normal; + static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); static void chv_setup_private_ppat(struct drm_i915_private *dev_priv); @@ -1341,9 +1343,12 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev) /* The bind_vma code tries to be smart about tracking mappings. * Unfortunately above, we've just wiped out the mappings * without telling our object about it. So we need to fake it. + * + * Bind is not expected to fail since this is only called on + * resume and assumption is all requirements exist already. */ vma->bound &= ~GLOBAL_BIND; - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); + WARN_ON(i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND)); } @@ -1538,7 +1543,7 @@ static void i915_ggtt_bind_vma(struct i915_vma *vma, AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY; BUG_ON(!i915_is_ggtt(vma->vm)); - intel_gtt_insert_sg_entries(vma->obj->pages, entry, flags); + intel_gtt_insert_sg_entries(vma->ggtt_view.pages, entry, flags); vma->bound = GLOBAL_BIND; } @@ -1588,7 +1593,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) { if (!(vma->bound & GLOBAL_BIND) || (cache_level != obj->cache_level)) { - vma->vm->insert_entries(vma->vm, obj->pages, + vma->vm->insert_entries(vma->vm, vma->ggtt_view.pages, vma->node.start, cache_level, flags); vma->bound |= GLOBAL_BIND; @@ -1600,7 +1605,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, (cache_level != obj->cache_level))) { struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt; appgtt->base.insert_entries(&appgtt->base, - vma->obj->pages, + vma->ggtt_view.pages, vma->node.start, cache_level, flags); vma->bound |= LOCAL_BIND; @@ -2165,7 +2170,8 @@ int i915_gem_gtt_init(struct drm_device *dev) } static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); if (vma == NULL) @@ -2176,6 +2182,7 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, INIT_LIST_HEAD(&vma->exec_list); vma->vm = vm; vma->obj = obj; + vma->ggtt_view = *view; switch (INTEL_INFO(vm->dev)->gen) { case 9: @@ -2210,14 +2217,59 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, } struct i915_vma * -i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma; - vma = i915_gem_obj_to_vma(obj, vm); + vma = i915_gem_obj_to_vma_view(obj, vm, view); if (!vma) - vma = __i915_gem_vma_create(obj, vm); + vma = __i915_gem_vma_create(obj, vm, view); return vma; } + +static inline +int i915_get_vma_pages(struct i915_vma *vma) +{ + if (vma->ggtt_view.pages) + return 0; + + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) + vma->ggtt_view.pages = vma->obj->pages; + else + WARN_ONCE(1, "GGTT view %u not implemented!\n", + vma->ggtt_view.type); + + if (!vma->ggtt_view.pages) { + DRM_ERROR("Failed to get pages for VMA view type %u!\n", + vma->ggtt_view.type); + return -EINVAL; + } + + return 0; +} + +/** + * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space. + * @vma: VMA to map + * @cache_level: mapping cache level + * @flags: flags like global or local mapping + * + * DMA addresses are taken from the scatter-gather table of this object (or of + * this VMA in case of non-default GGTT views) and PTE entries set up. + * Note that DMA addresses are also the only part of the SG table we care about. + */ +int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, + u32 flags) +{ + int ret = i915_get_vma_pages(vma); + + if (ret) + return ret; + + vma->bind_vma(vma, cache_level, flags); + + return 0; +} diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index dd849df..e377c7d 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -109,7 +109,20 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; #define GEN8_PPAT_ELLC_OVERRIDE (0<<2) #define GEN8_PPAT(i, x) ((uint64_t) (x) << ((i) * 8)) +enum i915_ggtt_view_type { + I915_GGTT_VIEW_NORMAL = 0, +}; + +struct i915_ggtt_view { + enum i915_ggtt_view_type type; + + struct sg_table *pages; +}; + +extern const struct i915_ggtt_view i915_ggtt_view_normal; + enum i915_cache_level; + /** * A VMA represents a GEM BO that is bound into an address space. Therefore, a * VMA's presence cannot be guaranteed before binding, or after unbinding the @@ -129,6 +142,15 @@ struct i915_vma { #define PTE_READ_ONLY (1<<2) unsigned int bound : 4; + /** + * Support different GGTT views into the same object. + * This means there can be multiple VMA mappings per object and per VM. + * i915_ggtt_view_type is used to distinguish between those entries. + * The default one of zero (I915_GGTT_VIEW_NORMAL) is default and also + * assumed in GEM functions which take no ggtt view parameter. + */ + struct i915_ggtt_view ggtt_view; + /** This object's place on the active/inactive lists */ struct list_head mm_list; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c4536e1..f97479a 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -718,10 +718,8 @@ static u32 capture_pinned_bo(struct drm_i915_error_buffer *err, break; list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm && vma->pin_count > 0) { + if (vma->vm == vm && vma->pin_count > 0) capture_bo(err++, vma); - break; - } } return err - first; @@ -1096,10 +1094,8 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv, list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm && vma->pin_count > 0) { + if (vma->vm == vm && vma->pin_count > 0) i++; - break; - } } error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx]; -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-05 12:11 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin @ 2014-12-05 14:25 ` Daniel Vetter 2014-12-09 15:23 ` Michel Thierry 1 sibling, 0 replies; 37+ messages in thread From: Daniel Vetter @ 2014-12-05 14:25 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx On Fri, Dec 05, 2014 at 12:11:45PM +0000, Tvrtko Ursulin wrote: > +struct i915_ggtt_view { > + enum i915_ggtt_view_type type; > + > + struct sg_table *pages; > +}; Minor nit on semantics, not really worth a resend (except when you need one anyway because of the detailed review): Imo the sg_table should be in the vma directly, not in the gtt_view. Otherwise we can't just do a memcmp if the view paramaters grow more fancy than just the type key. And maybe call it sg_table, not pages, to avoid confusion with the real obj->pages backing storage pointer. Anyway just bikesheds ;-) -Daniel > + > +extern const struct i915_ggtt_view i915_ggtt_view_normal; > + > enum i915_cache_level; > + > /** > * A VMA represents a GEM BO that is bound into an address space. Therefore, a > * VMA's presence cannot be guaranteed before binding, or after unbinding the > @@ -129,6 +142,15 @@ struct i915_vma { > #define PTE_READ_ONLY (1<<2) > unsigned int bound : 4; > > + /** > + * Support different GGTT views into the same object. > + * This means there can be multiple VMA mappings per object and per VM. > + * i915_ggtt_view_type is used to distinguish between those entries. > + * The default one of zero (I915_GGTT_VIEW_NORMAL) is default and also > + * assumed in GEM functions which take no ggtt view parameter. > + */ > + struct i915_ggtt_view ggtt_view; > + > /** This object's place on the active/inactive lists */ > struct list_head mm_list; > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index c4536e1..f97479a 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -718,10 +718,8 @@ static u32 capture_pinned_bo(struct drm_i915_error_buffer *err, > break; > > list_for_each_entry(vma, &obj->vma_list, vma_link) > - if (vma->vm == vm && vma->pin_count > 0) { > + if (vma->vm == vm && vma->pin_count > 0) > capture_bo(err++, vma); > - break; > - } > } > > return err - first; > @@ -1096,10 +1094,8 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv, > > list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { > list_for_each_entry(vma, &obj->vma_list, vma_link) > - if (vma->vm == vm && vma->pin_count > 0) { > + if (vma->vm == vm && vma->pin_count > 0) > i++; > - break; > - } > } > error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx]; > > -- > 2.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-05 12:11 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin 2014-12-05 14:25 ` Daniel Vetter @ 2014-12-09 15:23 ` Michel Thierry 2014-12-10 9:16 ` Daniel Vetter 1 sibling, 1 reply; 37+ messages in thread From: Michel Thierry @ 2014-12-09 15:23 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx@lists.freedesktop.org; +Cc: Daniel Vetter [-- Attachment #1.1: Type: text/plain, Size: 24992 bytes --] On 12/5/2014 12:11 PM, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Things like reliable GGTT mappings and mirrored 2d-on-3d display will need > to map objects into the same address space multiple times. > > Added a GGTT view concept and linked it with the VMA to distinguish between > 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. > > This now means that objects can have multiple VMA entries so the code which > assumed there will only be one also had to be modified. > > Alternative GGTT views are supposed to borrow DMA addresses from obj->pages > which is DMA mapped on first VMA instantiation and unmapped on the last one > going away. > > v2: > * Removed per view special casing in i915_gem_ggtt_prepare / > finish_object in favour of creating and destroying DMA mappings > on first VMA instantiation and last VMA destruction. (Daniel Vetter) > * Simplified i915_vma_unbind which does not need to count the GGTT views. > (Daniel Vetter) > * Also moved obj->map_and_fenceable reset under the same check. > * Checkpatch cleanups. > > v3: > * Only retire objects once the last VMA is unbound. > > v4: > * Keep scatter-gather table for alternative views persistent for the > lifetime of the VMA. > * Propagate binding errors to callers and handle appropriately. > > For: VIZ-4544 > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 5 +- > drivers/gpu/drm/i915/i915_drv.h | 46 +++++++++++-- > drivers/gpu/drm/i915/i915_gem.c | 101 ++++++++++++++++++----------- > drivers/gpu/drm/i915/i915_gem_context.c | 11 +++- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++- > drivers/gpu/drm/i915/i915_gem_gtt.c | 70 +++++++++++++++++--- > drivers/gpu/drm/i915/i915_gem_gtt.h | 22 +++++++ > drivers/gpu/drm/i915/i915_gpu_error.c | 8 +-- > 8 files changed, 206 insertions(+), 66 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 6c16939..bd08289 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -152,8 +152,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) > seq_puts(m, " (pp"); > else > seq_puts(m, " (g"); > - seq_printf(m, "gtt offset: %08lx, size: %08lx)", > - vma->node.start, vma->node.size); > + seq_printf(m, "gtt offset: %08lx, size: %08lx, type: %u)", > + vma->node.start, vma->node.size, > + vma->ggtt_view.type); > } > if (obj->stolen) > seq_printf(m, " (stolen: %08lx)", obj->stolen->start); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 049482f..b2f6f7d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2514,10 +2514,23 @@ void i915_gem_vma_destroy(struct i915_vma *vma); > #define PIN_GLOBAL 0x4 > #define PIN_OFFSET_BIAS 0x8 > #define PIN_OFFSET_MASK (~4095) > +int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + uint32_t alignment, > + uint64_t flags, > + const struct i915_ggtt_view *view); > +static inline > int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj, > struct i915_address_space *vm, > uint32_t alignment, > - uint64_t flags); > + uint64_t flags) > +{ > + return i915_gem_object_pin_view(obj, vm, alignment, flags, > + &i915_ggtt_view_normal); > +} > + > +int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > + u32 flags); > int __must_check i915_vma_unbind(struct i915_vma *vma); > int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); > void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv); > @@ -2679,18 +2692,43 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, > > void i915_gem_restore_fences(struct drm_device *dev); > > +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, > + struct i915_address_space *vm, > + enum i915_ggtt_view_type view); > +static inline > unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, > - struct i915_address_space *vm); > + struct i915_address_space *vm) > +{ > + return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL); > +} > bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); > bool i915_gem_obj_bound(struct drm_i915_gem_object *o, > struct i915_address_space *vm); > unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, > struct i915_address_space *vm); > +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_ggtt_view *view); > +static inline > struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, > - struct i915_address_space *vm); > + struct i915_address_space *vm) > +{ > + return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal); > +} > + > +struct i915_vma * > +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_ggtt_view *view); > + > +static inline > struct i915_vma * > i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, > - struct i915_address_space *vm); > + struct i915_address_space *vm) > +{ > + return i915_gem_obj_lookup_or_create_vma_view(obj, vm, > + &i915_ggtt_view_normal); > +} Hi, We also need a _vma_view version of i915_gem_obj_bound; i915_gem_object_ggtt_unpin checks if the obj is ggtt_bound and it could be that the normal view is gone but a different view is still active (it is also used in gpu_error and debug_fs, but I don't think it's a problem there). > > struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj); > static inline bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj) { > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c1c1141..9244f0b 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2087,8 +2087,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, > /* For the unbound phase, this should be a no-op! */ > list_for_each_entry_safe(vma, v, > &obj->vma_list, vma_link) > - if (i915_vma_unbind(vma)) > - break; > + i915_vma_unbind(vma); > > if (i915_gem_object_put_pages(obj) == 0) > count += obj->base.size >> PAGE_SHIFT; > @@ -2297,17 +2296,14 @@ void i915_vma_move_to_active(struct i915_vma *vma, > static void > i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) > { > - struct drm_i915_private *dev_priv = obj->base.dev->dev_private; > - struct i915_address_space *vm; > struct i915_vma *vma; > > BUG_ON(obj->base.write_domain & ~I915_GEM_GPU_DOMAINS); > BUG_ON(!obj->active); > > - list_for_each_entry(vm, &dev_priv->vm_list, global_link) { > - vma = i915_gem_obj_to_vma(obj, vm); > - if (vma && !list_empty(&vma->mm_list)) > - list_move_tail(&vma->mm_list, &vm->inactive_list); > + list_for_each_entry(vma, &obj->vma_list, vma_link) { > + if (!list_empty(&vma->mm_list)) > + list_move_tail(&vma->mm_list, &vma->vm->inactive_list); > } > > intel_fb_obj_flush(obj, true); > @@ -3062,10 +3058,8 @@ int i915_vma_unbind(struct i915_vma *vma) > * cause memory corruption through use-after-free. > */ > > - /* Throw away the active reference before moving to the unbound list */ > - i915_gem_object_retire(obj); > - > - if (i915_is_ggtt(vma->vm)) { > + if (i915_is_ggtt(vma->vm) && > + vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { > i915_gem_object_finish_gtt(obj); > > /* release the fence reg _after_ flushing */ > @@ -3079,8 +3073,15 @@ int i915_vma_unbind(struct i915_vma *vma) > vma->unbind_vma(vma); > > list_del_init(&vma->mm_list); > - if (i915_is_ggtt(vma->vm)) > - obj->map_and_fenceable = false; > + if (i915_is_ggtt(vma->vm)) { > + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { > + obj->map_and_fenceable = false; > + } else if (vma->ggtt_view.pages) { > + sg_free_table(vma->ggtt_view.pages); > + kfree(vma->ggtt_view.pages); > + vma->ggtt_view.pages = NULL; > + } > + } > > drm_mm_remove_node(&vma->node); > i915_gem_vma_destroy(vma); > @@ -3088,6 +3089,10 @@ int i915_vma_unbind(struct i915_vma *vma) > /* Since the unbound list is global, only move to that list if > * no more VMAs exist. */ > if (list_empty(&obj->vma_list)) { > + /* Throw away the active reference before > + * moving to the unbound list. */ > + i915_gem_object_retire(obj); > + > i915_gem_gtt_finish_object(obj); > list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list); > } > @@ -3498,7 +3503,8 @@ static struct i915_vma * > i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > struct i915_address_space *vm, > unsigned alignment, > - uint64_t flags) > + uint64_t flags, > + const struct i915_ggtt_view *view) > { > struct drm_device *dev = obj->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -3548,7 +3554,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > > i915_gem_object_pin_pages(obj); > > - vma = i915_gem_obj_lookup_or_create_vma(obj, vm); > + vma = i915_gem_obj_lookup_or_create_vma_view(obj, vm, view); > if (IS_ERR(vma)) > goto err_unpin; > > @@ -3578,15 +3584,19 @@ search_free: > if (ret) > goto err_remove_node; > > + trace_i915_vma_bind(vma, flags); > + ret = i915_vma_bind(vma, obj->cache_level, > + flags & PIN_GLOBAL ? GLOBAL_BIND : 0); > + if (ret) > + goto err_finish_gtt; > + > list_move_tail(&obj->global_list, &dev_priv->mm.bound_list); > list_add_tail(&vma->mm_list, &vm->inactive_list); > > - trace_i915_vma_bind(vma, flags); > - vma->bind_vma(vma, obj->cache_level, > - flags & PIN_GLOBAL ? GLOBAL_BIND : 0); > - > return vma; > > +err_finish_gtt: > + i915_gem_gtt_finish_object(obj); > err_remove_node: > drm_mm_remove_node(&vma->node); > err_free_vma: > @@ -3789,9 +3799,12 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, > } > > list_for_each_entry(vma, &obj->vma_list, vma_link) > - if (drm_mm_node_allocated(&vma->node)) > - vma->bind_vma(vma, cache_level, > - vma->bound & GLOBAL_BIND); > + if (drm_mm_node_allocated(&vma->node)) { > + ret = i915_vma_bind(vma, cache_level, > + vma->bound & GLOBAL_BIND); > + if (ret); > + return ret; > + } > } > > list_for_each_entry(vma, &obj->vma_list, vma_link) > @@ -4144,10 +4157,11 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags) > } > > int > -i915_gem_object_pin(struct drm_i915_gem_object *obj, > - struct i915_address_space *vm, > - uint32_t alignment, > - uint64_t flags) > +i915_gem_object_pin_view(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + uint32_t alignment, > + uint64_t flags, > + const struct i915_ggtt_view *view) > { > struct drm_i915_private *dev_priv = obj->base.dev->dev_private; > struct i915_vma *vma; > @@ -4163,7 +4177,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, > if (WARN_ON((flags & (PIN_MAPPABLE | PIN_GLOBAL)) == PIN_MAPPABLE)) > return -EINVAL; > > - vma = i915_gem_obj_to_vma(obj, vm); > + vma = i915_gem_obj_to_vma_view(obj, vm, view); > if (vma) { > if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)) > return -EBUSY; > @@ -4173,7 +4187,8 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, > "bo is already pinned with incorrect alignment:" > " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d," > " obj->map_and_fenceable=%d\n", > - i915_gem_obj_offset(obj, vm), alignment, > + i915_gem_obj_offset_view(obj, vm, view->type), > + alignment, > !!(flags & PIN_MAPPABLE), > obj->map_and_fenceable); > ret = i915_vma_unbind(vma); > @@ -4186,13 +4201,17 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, > > bound = vma ? vma->bound : 0; > if (vma == NULL || !drm_mm_node_allocated(&vma->node)) { > - vma = i915_gem_object_bind_to_vm(obj, vm, alignment, flags); > + vma = i915_gem_object_bind_to_vm(obj, vm, alignment, > + flags, view); > if (IS_ERR(vma)) > return PTR_ERR(vma); > } > > - if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND)) > - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); > + if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND)) { > + ret = i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND); > + if (ret) > + return ret; > + } > > if ((bound ^ vma->bound) & GLOBAL_BIND) { > bool mappable, fenceable; > @@ -4528,12 +4547,13 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) > intel_runtime_pm_put(dev_priv); > } > > -struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, > - struct i915_address_space *vm) > +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_ggtt_view *view) > { > struct i915_vma *vma; > list_for_each_entry(vma, &obj->vma_list, vma_link) > - if (vma->vm == vm) > + if (vma->vm == vm && vma->ggtt_view.type == view->type) > return vma; > > return NULL; > @@ -5147,8 +5167,9 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) > } > > /* All the new VM stuff */ > -unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, > - struct i915_address_space *vm) > +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, > + struct i915_address_space *vm, > + enum i915_ggtt_view_type view) > { > struct drm_i915_private *dev_priv = o->base.dev->dev_private; > struct i915_vma *vma; > @@ -5156,7 +5177,7 @@ unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, > WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base); > > list_for_each_entry(vma, &o->vma_list, vma_link) { > - if (vma->vm == vm) > + if (vma->vm == vm && vma->ggtt_view.type == view) > return vma->node.start; > > } > @@ -5307,7 +5328,9 @@ struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj) > struct i915_vma *vma; > > list_for_each_entry(vma, &obj->vma_list, vma_link) { > - if (vma->vm == ggtt) > + if (vma->vm != ggtt) > + continue; > + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) > return vma; > } I'd prefer to have a single if (vma->vm == ggtt && type = normal view). Everything else looks good to me. -Michel > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 5cd2b97..4ef8815 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -590,9 +590,14 @@ static int do_switch(struct intel_engine_cs *ring, > goto unpin_out; > > vma = i915_gem_obj_to_ggtt(to->legacy_hw_ctx.rcs_state); > - if (!(vma->bound & GLOBAL_BIND)) > - vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level, > - GLOBAL_BIND); > + if (!(vma->bound & GLOBAL_BIND)) { > + ret = i915_vma_bind(vma, > + to->legacy_hw_ctx.rcs_state->cache_level, > + GLOBAL_BIND); > + /* This shouldn't ever fail. */ > + if (WARN_ONCE(ret, "GGTT context bind failed!")) > + goto unpin_out; > + } > > if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) > hw_flags |= MI_RESTORE_INHIBIT; > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 0c25f62..3927d93 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -360,9 +360,12 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, > * through the ppgtt for non_secure batchbuffers. */ > if (unlikely(IS_GEN6(dev) && > reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && > - !(target_vma->bound & GLOBAL_BIND))) > - target_vma->bind_vma(target_vma, target_i915_obj->cache_level, > - GLOBAL_BIND); > + !(target_vma->bound & GLOBAL_BIND))) { > + ret = i915_vma_bind(target_vma, target_i915_obj->cache_level, > + GLOBAL_BIND); > + if (WARN_ONCE(ret, "Unexpected failure to bind target VMA!")) > + return ret; > + } > > /* Validate that the target is in a valid r/w GPU domain */ > if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) { > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index ac03a38..73c1c0b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -30,6 +30,8 @@ > #include "i915_trace.h" > #include "intel_drv.h" > > +const struct i915_ggtt_view i915_ggtt_view_normal; > + > static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); > static void chv_setup_private_ppat(struct drm_i915_private *dev_priv); > > @@ -1341,9 +1343,12 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev) > /* The bind_vma code tries to be smart about tracking mappings. > * Unfortunately above, we've just wiped out the mappings > * without telling our object about it. So we need to fake it. > + * > + * Bind is not expected to fail since this is only called on > + * resume and assumption is all requirements exist already. > */ > vma->bound &= ~GLOBAL_BIND; > - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); > + WARN_ON(i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND)); > } > > > @@ -1538,7 +1543,7 @@ static void i915_ggtt_bind_vma(struct i915_vma *vma, > AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY; > > BUG_ON(!i915_is_ggtt(vma->vm)); > - intel_gtt_insert_sg_entries(vma->obj->pages, entry, flags); > + intel_gtt_insert_sg_entries(vma->ggtt_view.pages, entry, flags); > vma->bound = GLOBAL_BIND; > } > > @@ -1588,7 +1593,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, > if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) { > if (!(vma->bound & GLOBAL_BIND) || > (cache_level != obj->cache_level)) { > - vma->vm->insert_entries(vma->vm, obj->pages, > + vma->vm->insert_entries(vma->vm, vma->ggtt_view.pages, > vma->node.start, > cache_level, flags); > vma->bound |= GLOBAL_BIND; > @@ -1600,7 +1605,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, > (cache_level != obj->cache_level))) { > struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt; > appgtt->base.insert_entries(&appgtt->base, > - vma->obj->pages, > + vma->ggtt_view.pages, > vma->node.start, > cache_level, flags); > vma->bound |= LOCAL_BIND; > @@ -2165,7 +2170,8 @@ int i915_gem_gtt_init(struct drm_device *dev) > } > > static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, > - struct i915_address_space *vm) > + struct i915_address_space *vm, > + const struct i915_ggtt_view *view) > { > struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); > if (vma == NULL) > @@ -2176,6 +2182,7 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, > INIT_LIST_HEAD(&vma->exec_list); > vma->vm = vm; > vma->obj = obj; > + vma->ggtt_view = *view; > > switch (INTEL_INFO(vm->dev)->gen) { > case 9: > @@ -2210,14 +2217,59 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, > } > > struct i915_vma * > -i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, > - struct i915_address_space *vm) > +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_ggtt_view *view) > { > struct i915_vma *vma; > > - vma = i915_gem_obj_to_vma(obj, vm); > + vma = i915_gem_obj_to_vma_view(obj, vm, view); > if (!vma) > - vma = __i915_gem_vma_create(obj, vm); > + vma = __i915_gem_vma_create(obj, vm, view); > > return vma; > } > + > +static inline > +int i915_get_vma_pages(struct i915_vma *vma) > +{ > + if (vma->ggtt_view.pages) > + return 0; > + > + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) > + vma->ggtt_view.pages = vma->obj->pages; > + else > + WARN_ONCE(1, "GGTT view %u not implemented!\n", > + vma->ggtt_view.type); > + > + if (!vma->ggtt_view.pages) { > + DRM_ERROR("Failed to get pages for VMA view type %u!\n", > + vma->ggtt_view.type); > + return -EINVAL; > + } > + > + return 0; > +} > + > +/** > + * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space. > + * @vma: VMA to map > + * @cache_level: mapping cache level > + * @flags: flags like global or local mapping > + * > + * DMA addresses are taken from the scatter-gather table of this object (or of > + * this VMA in case of non-default GGTT views) and PTE entries set up. > + * Note that DMA addresses are also the only part of the SG table we care about. > + */ > +int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > + u32 flags) > +{ > + int ret = i915_get_vma_pages(vma); > + > + if (ret) > + return ret; > + > + vma->bind_vma(vma, cache_level, flags); > + > + return 0; > +} > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index dd849df..e377c7d 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -109,7 +109,20 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; > #define GEN8_PPAT_ELLC_OVERRIDE (0<<2) > #define GEN8_PPAT(i, x) ((uint64_t) (x) << ((i) * 8)) > > +enum i915_ggtt_view_type { > + I915_GGTT_VIEW_NORMAL = 0, > +}; > + > +struct i915_ggtt_view { > + enum i915_ggtt_view_type type; > + > + struct sg_table *pages; > +}; > + > +extern const struct i915_ggtt_view i915_ggtt_view_normal; > + > enum i915_cache_level; > + > /** > * A VMA represents a GEM BO that is bound into an address space. Therefore, a > * VMA's presence cannot be guaranteed before binding, or after unbinding the > @@ -129,6 +142,15 @@ struct i915_vma { > #define PTE_READ_ONLY (1<<2) > unsigned int bound : 4; > > + /** > + * Support different GGTT views into the same object. > + * This means there can be multiple VMA mappings per object and per VM. > + * i915_ggtt_view_type is used to distinguish between those entries. > + * The default one of zero (I915_GGTT_VIEW_NORMAL) is default and also > + * assumed in GEM functions which take no ggtt view parameter. > + */ > + struct i915_ggtt_view ggtt_view; > + > /** This object's place on the active/inactive lists */ > struct list_head mm_list; > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index c4536e1..f97479a 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -718,10 +718,8 @@ static u32 capture_pinned_bo(struct drm_i915_error_buffer *err, > break; > > list_for_each_entry(vma, &obj->vma_list, vma_link) > - if (vma->vm == vm && vma->pin_count > 0) { > + if (vma->vm == vm && vma->pin_count > 0) > capture_bo(err++, vma); > - break; > - } > } > > return err - first; > @@ -1096,10 +1094,8 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv, > > list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { > list_for_each_entry(vma, &obj->vma_list, vma_link) > - if (vma->vm == vm && vma->pin_count > 0) { > + if (vma->vm == vm && vma->pin_count > 0) > i++; > - break; > - } > } > error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx]; > [-- Attachment #1.2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 5510 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-09 15:23 ` Michel Thierry @ 2014-12-10 9:16 ` Daniel Vetter 2014-12-10 10:17 ` Tvrtko Ursulin 0 siblings, 1 reply; 37+ messages in thread From: Daniel Vetter @ 2014-12-10 9:16 UTC (permalink / raw) To: Michel Thierry; +Cc: Intel-gfx@lists.freedesktop.org, Daniel Vetter On Tue, Dec 09, 2014 at 03:23:35PM +0000, Michel Thierry wrote: > On 12/5/2014 12:11 PM, Tvrtko Ursulin wrote: > >From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > >Things like reliable GGTT mappings and mirrored 2d-on-3d display will need > >to map objects into the same address space multiple times. > > > >Added a GGTT view concept and linked it with the VMA to distinguish between > >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. > > > >This now means that objects can have multiple VMA entries so the code which > >assumed there will only be one also had to be modified. > > > >Alternative GGTT views are supposed to borrow DMA addresses from obj->pages > >which is DMA mapped on first VMA instantiation and unmapped on the last one > >going away. > > > >v2: > > * Removed per view special casing in i915_gem_ggtt_prepare / > > finish_object in favour of creating and destroying DMA mappings > > on first VMA instantiation and last VMA destruction. (Daniel Vetter) > > * Simplified i915_vma_unbind which does not need to count the GGTT views. > > (Daniel Vetter) > > * Also moved obj->map_and_fenceable reset under the same check. > > * Checkpatch cleanups. > > > >v3: > > * Only retire objects once the last VMA is unbound. > > > >v4: > > * Keep scatter-gather table for alternative views persistent for the > > lifetime of the VMA. > > * Propagate binding errors to callers and handle appropriately. > > > >For: VIZ-4544 > >Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >--- > > drivers/gpu/drm/i915/i915_debugfs.c | 5 +- > > drivers/gpu/drm/i915/i915_drv.h | 46 +++++++++++-- > > drivers/gpu/drm/i915/i915_gem.c | 101 ++++++++++++++++++----------- > > drivers/gpu/drm/i915/i915_gem_context.c | 11 +++- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 70 +++++++++++++++++--- > > drivers/gpu/drm/i915/i915_gem_gtt.h | 22 +++++++ > > drivers/gpu/drm/i915/i915_gpu_error.c | 8 +-- > > 8 files changed, 206 insertions(+), 66 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > >index 6c16939..bd08289 100644 > >--- a/drivers/gpu/drm/i915/i915_debugfs.c > >+++ b/drivers/gpu/drm/i915/i915_debugfs.c > >@@ -152,8 +152,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) > > seq_puts(m, " (pp"); > > else > > seq_puts(m, " (g"); > >- seq_printf(m, "gtt offset: %08lx, size: %08lx)", > >- vma->node.start, vma->node.size); > >+ seq_printf(m, "gtt offset: %08lx, size: %08lx, type: %u)", > >+ vma->node.start, vma->node.size, > >+ vma->ggtt_view.type); > > } > > if (obj->stolen) > > seq_printf(m, " (stolen: %08lx)", obj->stolen->start); > >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >index 049482f..b2f6f7d 100644 > >--- a/drivers/gpu/drm/i915/i915_drv.h > >+++ b/drivers/gpu/drm/i915/i915_drv.h > >@@ -2514,10 +2514,23 @@ void i915_gem_vma_destroy(struct i915_vma *vma); > > #define PIN_GLOBAL 0x4 > > #define PIN_OFFSET_BIAS 0x8 > > #define PIN_OFFSET_MASK (~4095) > >+int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj, > >+ struct i915_address_space *vm, > >+ uint32_t alignment, > >+ uint64_t flags, > >+ const struct i915_ggtt_view *view); > >+static inline > > int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj, > > struct i915_address_space *vm, > > uint32_t alignment, > >- uint64_t flags); > >+ uint64_t flags) > >+{ > >+ return i915_gem_object_pin_view(obj, vm, alignment, flags, > >+ &i915_ggtt_view_normal); > >+} > >+ > >+int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > >+ u32 flags); > > int __must_check i915_vma_unbind(struct i915_vma *vma); > > int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); > > void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv); > >@@ -2679,18 +2692,43 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, > > void i915_gem_restore_fences(struct drm_device *dev); > >+unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, > >+ struct i915_address_space *vm, > >+ enum i915_ggtt_view_type view); > >+static inline > > unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, > >- struct i915_address_space *vm); > >+ struct i915_address_space *vm) > >+{ > >+ return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL); > >+} > > bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); > > bool i915_gem_obj_bound(struct drm_i915_gem_object *o, > > struct i915_address_space *vm); > > unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, > > struct i915_address_space *vm); > >+struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, > >+ struct i915_address_space *vm, > >+ const struct i915_ggtt_view *view); > >+static inline > > struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, > >- struct i915_address_space *vm); > >+ struct i915_address_space *vm) > >+{ > >+ return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal); > >+} > >+ > >+struct i915_vma * > >+i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, > >+ struct i915_address_space *vm, > >+ const struct i915_ggtt_view *view); > >+ > >+static inline > > struct i915_vma * > > i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, > >- struct i915_address_space *vm); > >+ struct i915_address_space *vm) > >+{ > >+ return i915_gem_obj_lookup_or_create_vma_view(obj, vm, > >+ &i915_ggtt_view_normal); > >+} > > We also need a _vma_view version of i915_gem_obj_bound; > i915_gem_object_ggtt_unpin checks if the obj is ggtt_bound and it could be > that the normal view is gone but a different view is still active (it is > also used in gpu_error and debug_fs, but I don't think it's a problem > there). Where did you see the need for the new obj_bound variant? Probably best to reply to the patch newly with just the relevant part quoted. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-10 9:16 ` Daniel Vetter @ 2014-12-10 10:17 ` Tvrtko Ursulin 0 siblings, 0 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-10 10:17 UTC (permalink / raw) To: Daniel Vetter, Michel Thierry Cc: Daniel Vetter, Intel-gfx@lists.freedesktop.org On 12/10/2014 09:16 AM, Daniel Vetter wrote: > On Tue, Dec 09, 2014 at 03:23:35PM +0000, Michel Thierry wrote: >> We also need a _vma_view version of i915_gem_obj_bound; >> i915_gem_object_ggtt_unpin checks if the obj is ggtt_bound and it could be >> that the normal view is gone but a different view is still active (it is >> also used in gpu_error and debug_fs, but I don't think it's a problem >> there). > > Where did you see the need for the new obj_bound variant? Probably best to > reply to the patch newly with just the relevant part quoted. It is not in the patch but in the i915_gem_object_ggtt_unpin. Which is: i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj) { struct i915_vma *vma = i915_gem_obj_to_ggtt(obj); BUG_ON(!vma); BUG_ON(vma->pin_count == 0); BUG_ON(!i915_gem_obj_ggtt_bound(obj)); if (--vma->pin_count == 0) obj->pin_mappable = false; } The concern is the mismatch in semantics between i915_gem_obj_to_ggtt and i915_gem_obj_ggtt_bound. Former implies normal VMA while the latter hasn't been touched so it returns true on _any_ GGTT bound VMA. I don't think this BUG_ON can trigger since normal VMA exists by the virtue of BUG_ON(!vma), but I do agree that there is a mismatch in "documentation" (BUG_ONs). So making i915_gem_obj_ggtt_bound also imply a normal view would be correct it seems. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 2/2] drm/i915: Documentation for multiple GGTT views 2014-12-05 12:11 ` [PATCH v2 0/2] Multiple " Tvrtko Ursulin 2014-12-05 12:11 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin @ 2014-12-05 12:11 ` Tvrtko Ursulin 2014-12-09 15:41 ` Michel Thierry 1 sibling, 1 reply; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-05 12:11 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> A short section describing background, implementation and intended usage. For: VIZ-4544 Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- Documentation/DocBook/drm.tmpl | 5 ++++ drivers/gpu/drm/i915/i915_gem_gtt.c | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 56e2a9b..c975f26 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4033,6 +4033,11 @@ int num_ioctls;</synopsis> !Pdrivers/gpu/drm/i915/intel_lrc.c Logical Rings, Logical Ring Contexts and Execlists !Idrivers/gpu/drm/i915/intel_lrc.c </sect2> + <sect2> + <title>Global GTT views</title> +!Pdrivers/gpu/drm/i915/i915_gem_gtt.c Global GTT views +!Idrivers/gpu/drm/i915/i915_gem_gtt.c + </sect2> </sect1> <sect1> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 73c1c0b..0953369 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -30,6 +30,66 @@ #include "i915_trace.h" #include "intel_drv.h" +/** + * DOC: Global GTT views (GGTT) + * + * Background and previous state + * + * Historically objects could exists (be bound) in global GTT space only as + * singular instances with a view representing all of the object's backing pages + * in a linear fashion. This view will be called a normal view. + * + * To support multiple views of the same object, where the number of mapped + * pages is not equal to the backing store, or where the layout of the pages + * is not linear, concept of a GGTT view was added. + * + * One example of an alternative view is a stereo display driven by a single + * image. In this case we would have a framebuffer looking like this + * (2x2 pages): + * + * 12 + * 34 + * + * Above would represent a normal GGTT view as normally mapped for GPU or CPU + * rendering. In contrast, fed to the display engine would be an alternative + * view which could look something like this: + * + * 1212 + * 3434 + * + * In this example both the size and layout of pages in the alternative view is + * different from the normal view. + * + * Implementation and usage + * + * GGTT views are implemented using VMAs and are distinguished via enum + * i915_ggtt_view_type and struct i915_ggtt_view. + * + * A new flavour of core GEM functions which work with GGTT bound objects were + * added with the _view suffix. They take the struct i915_ggtt_view parameter + * encapsulating all metadata required to implement a view. + * + * As a helper for callers which are only interested in the normal view, + * globally const i915_ggtt_view_normal singleton instance exists. All old core + * GEM API functions, the ones not taking the view parameter, are operating on, + * or with the normal GGTT view. + * + * Code wanting to add or use a new GGTT view needs to: + * + * 1. Add a new enum with a suitable name. + * 2. Extend the metadata in the i915_ggtt_view structure if required. + * 3. Add support to i915_get_vma_pages(). + * + * New views are required to build a scatter-gather table from within the + * i915_get_vma_pages function. This table is stored in the vma.ggtt_view and + * exists for the lifetime of an VMA. + * + * Core API is designed to have copy semantics which means that passed in + * struct i915_ggtt_view does not need to be persistent (left around after + * calling the core API functions). + * + */ + const struct i915_ggtt_view i915_ggtt_view_normal; static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] drm/i915: Documentation for multiple GGTT views 2014-12-05 12:11 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin @ 2014-12-09 15:41 ` Michel Thierry 0 siblings, 0 replies; 37+ messages in thread From: Michel Thierry @ 2014-12-09 15:41 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx@lists.freedesktop.org [-- Attachment #1.1: Type: text/plain, Size: 4231 bytes --] On 12/5/2014 12:11 PM, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > A short section describing background, implementation and intended usage. > > For: VIZ-4544 > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > Documentation/DocBook/drm.tmpl | 5 ++++ > drivers/gpu/drm/i915/i915_gem_gtt.c | 60 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 65 insertions(+) > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 56e2a9b..c975f26 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -4033,6 +4033,11 @@ int num_ioctls;</synopsis> > !Pdrivers/gpu/drm/i915/intel_lrc.c Logical Rings, Logical Ring Contexts and Execlists > !Idrivers/gpu/drm/i915/intel_lrc.c > </sect2> > + <sect2> > + <title>Global GTT views</title> > +!Pdrivers/gpu/drm/i915/i915_gem_gtt.c Global GTT views Isn't it missing the _(GGTT)_ of the DOC: section title? i.e. !Pdrivers/gpu/drm/i915/i915_gem_gtt.c Global GTT views (GGTT) > +!Idrivers/gpu/drm/i915/i915_gem_gtt.c > + </sect2> > </sect1> > > <sect1> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 73c1c0b..0953369 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -30,6 +30,66 @@ > #include "i915_trace.h" > #include "intel_drv.h" > > +/** > + * DOC: Global GTT views (GGTT) > + * > + * Background and previous state > + * > + * Historically objects could exists (be bound) in global GTT space only as > + * singular instances with a view representing all of the object's backing pages > + * in a linear fashion. This view will be called a normal view. > + * > + * To support multiple views of the same object, where the number of mapped > + * pages is not equal to the backing store, or where the layout of the pages > + * is not linear, concept of a GGTT view was added. > + * > + * One example of an alternative view is a stereo display driven by a single > + * image. In this case we would have a framebuffer looking like this > + * (2x2 pages): > + * > + * 12 > + * 34 > + * > + * Above would represent a normal GGTT view as normally mapped for GPU or CPU > + * rendering. In contrast, fed to the display engine would be an alternative > + * view which could look something like this: > + * > + * 1212 > + * 3434 > + * > + * In this example both the size and layout of pages in the alternative view is > + * different from the normal view. > + * > + * Implementation and usage > + * > + * GGTT views are implemented using VMAs and are distinguished via enum > + * i915_ggtt_view_type and struct i915_ggtt_view. > + * > + * A new flavour of core GEM functions which work with GGTT bound objects were > + * added with the _view suffix. They take the struct i915_ggtt_view parameter > + * encapsulating all metadata required to implement a view. > + * > + * As a helper for callers which are only interested in the normal view, > + * globally const i915_ggtt_view_normal singleton instance exists. All old core > + * GEM API functions, the ones not taking the view parameter, are operating on, > + * or with the normal GGTT view. > + * > + * Code wanting to add or use a new GGTT view needs to: > + * > + * 1. Add a new enum with a suitable name. > + * 2. Extend the metadata in the i915_ggtt_view structure if required. > + * 3. Add support to i915_get_vma_pages(). > + * > + * New views are required to build a scatter-gather table from within the > + * i915_get_vma_pages function. This table is stored in the vma.ggtt_view and > + * exists for the lifetime of an VMA. > + * > + * Core API is designed to have copy semantics which means that passed in > + * struct i915_ggtt_view does not need to be persistent (left around after > + * calling the core API functions). > + * > + */ > + > const struct i915_ggtt_view i915_ggtt_view_normal; > > static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); Apart from that, Reviewed-by: Michel Thierry <michel.thierry@intel.com> [-- Attachment #1.2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 5510 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 0/2] Multiple GGTT views 2014-12-03 14:59 [PATCH 0/3] Multiple GGTT views Tvrtko Ursulin ` (3 preceding siblings ...) 2014-12-05 12:11 ` [PATCH v2 0/2] Multiple " Tvrtko Ursulin @ 2014-12-10 17:27 ` Tvrtko Ursulin 2014-12-10 17:27 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin 2014-12-10 17:27 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 4 siblings, 2 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-10 17:27 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> This series continues what was previously a single patch called "drm/i915: Infrastructure for supporting different GGTT views per object". v2: * One less patch due early prep work getting merged. * Main patch reworked to keep the pages pointer around for the lifetime of the VMA. More detailed changelog in the patch itself. v3: * Addressing review comments - see individual changelogs. Tvrtko Ursulin (2): drm/i915: Infrastructure for supporting different GGTT views per object drm/i915: Documentation for multiple GGTT views Documentation/DocBook/drm.tmpl | 5 ++ drivers/gpu/drm/i915/i915_debugfs.c | 5 +- drivers/gpu/drm/i915/i915_drv.h | 56 +++++++++++-- drivers/gpu/drm/i915/i915_gem.c | 112 +++++++++++++++---------- drivers/gpu/drm/i915/i915_gem_context.c | 11 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 130 +++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_gem_gtt.h | 22 +++++ drivers/gpu/drm/i915/i915_gpu_error.c | 8 +- 9 files changed, 286 insertions(+), 72 deletions(-) -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-10 17:27 ` [PATCH v3 0/2] Multiple " Tvrtko Ursulin @ 2014-12-10 17:27 ` Tvrtko Ursulin 2014-12-11 10:18 ` Michel Thierry 2014-12-15 10:24 ` Daniel Vetter 2014-12-10 17:27 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 1 sibling, 2 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-10 17:27 UTC (permalink / raw) To: Intel-gfx; +Cc: Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Things like reliable GGTT mappings and mirrored 2d-on-3d display will need to map objects into the same address space multiple times. Added a GGTT view concept and linked it with the VMA to distinguish between 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. This now means that objects can have multiple VMA entries so the code which assumed there will only be one also had to be modified. Alternative GGTT views are supposed to borrow DMA addresses from obj->pages which is DMA mapped on first VMA instantiation and unmapped on the last one going away. v2: * Removed per view special casing in i915_gem_ggtt_prepare / finish_object in favour of creating and destroying DMA mappings on first VMA instantiation and last VMA destruction. (Daniel Vetter) * Simplified i915_vma_unbind which does not need to count the GGTT views. (Daniel Vetter) * Also moved obj->map_and_fenceable reset under the same check. * Checkpatch cleanups. v3: * Only retire objects once the last VMA is unbound. v4: * Keep scatter-gather table for alternative views persistent for the lifetime of the VMA. * Propagate binding errors to callers and handle appropriately. v5: * Explicitly look for normal GGTT view in i915_gem_obj_bound to align usage in i915_gem_object_ggtt_unpin. (Michel Thierry) * Change to single if statement in i915_gem_obj_to_ggtt. (Michel Thierry) * Removed stray semi-colon in i915_gem_object_set_cache_level. For: VIZ-4544 Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_debugfs.c | 5 +- drivers/gpu/drm/i915/i915_drv.h | 56 +++++++++++++-- drivers/gpu/drm/i915/i915_gem.c | 112 +++++++++++++++++------------ drivers/gpu/drm/i915/i915_gem_context.c | 11 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++- drivers/gpu/drm/i915/i915_gem_gtt.c | 70 +++++++++++++++--- drivers/gpu/drm/i915/i915_gem_gtt.h | 22 ++++++ drivers/gpu/drm/i915/i915_gpu_error.c | 8 +-- 8 files changed, 221 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d0e445e..a8af667 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -152,8 +152,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) seq_puts(m, " (pp"); else seq_puts(m, " (g"); - seq_printf(m, "gtt offset: %08lx, size: %08lx)", - vma->node.start, vma->node.size); + seq_printf(m, "gtt offset: %08lx, size: %08lx, type: %u)", + vma->node.start, vma->node.size, + vma->ggtt_view.type); } if (obj->stolen) seq_printf(m, " (stolen: %08lx)", obj->stolen->start); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 11e85cb..451bb6a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2518,10 +2518,23 @@ void i915_gem_vma_destroy(struct i915_vma *vma); #define PIN_GLOBAL 0x4 #define PIN_OFFSET_BIAS 0x8 #define PIN_OFFSET_MASK (~4095) +int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + uint32_t alignment, + uint64_t flags, + const struct i915_ggtt_view *view); +static inline int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm, uint32_t alignment, - uint64_t flags); + uint64_t flags) +{ + return i915_gem_object_pin_view(obj, vm, alignment, flags, + &i915_ggtt_view_normal); +} + +int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, + u32 flags); int __must_check i915_vma_unbind(struct i915_vma *vma); int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv); @@ -2683,18 +2696,51 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, void i915_gem_restore_fences(struct drm_device *dev); +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view); +static inline unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL); +} bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); +bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view); +static inline bool i915_gem_obj_bound(struct drm_i915_gem_object *o, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL); +} + unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o, struct i915_address_space *vm); +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view); +static inline struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal); +} + +struct i915_vma * +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view); + +static inline struct i915_vma * i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm); + struct i915_address_space *vm) +{ + return i915_gem_obj_lookup_or_create_vma_view(obj, vm, + &i915_ggtt_view_normal); +} struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj); static inline bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index de241eb..c6610bc 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2098,8 +2098,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, /* For the unbound phase, this should be a no-op! */ list_for_each_entry_safe(vma, v, &obj->vma_list, vma_link) - if (i915_vma_unbind(vma)) - break; + i915_vma_unbind(vma); if (i915_gem_object_put_pages(obj) == 0) count += obj->base.size >> PAGE_SHIFT; @@ -2308,17 +2307,14 @@ void i915_vma_move_to_active(struct i915_vma *vma, static void i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) { - struct drm_i915_private *dev_priv = obj->base.dev->dev_private; - struct i915_address_space *vm; struct i915_vma *vma; BUG_ON(obj->base.write_domain & ~I915_GEM_GPU_DOMAINS); BUG_ON(!obj->active); - list_for_each_entry(vm, &dev_priv->vm_list, global_link) { - vma = i915_gem_obj_to_vma(obj, vm); - if (vma && !list_empty(&vma->mm_list)) - list_move_tail(&vma->mm_list, &vm->inactive_list); + list_for_each_entry(vma, &obj->vma_list, vma_link) { + if (!list_empty(&vma->mm_list)) + list_move_tail(&vma->mm_list, &vma->vm->inactive_list); } intel_fb_obj_flush(obj, true); @@ -3073,10 +3069,8 @@ int i915_vma_unbind(struct i915_vma *vma) * cause memory corruption through use-after-free. */ - /* Throw away the active reference before moving to the unbound list */ - i915_gem_object_retire(obj); - - if (i915_is_ggtt(vma->vm)) { + if (i915_is_ggtt(vma->vm) && + vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { i915_gem_object_finish_gtt(obj); /* release the fence reg _after_ flushing */ @@ -3090,8 +3084,15 @@ int i915_vma_unbind(struct i915_vma *vma) vma->unbind_vma(vma); list_del_init(&vma->mm_list); - if (i915_is_ggtt(vma->vm)) - obj->map_and_fenceable = false; + if (i915_is_ggtt(vma->vm)) { + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { + obj->map_and_fenceable = false; + } else if (vma->ggtt_view.pages) { + sg_free_table(vma->ggtt_view.pages); + kfree(vma->ggtt_view.pages); + vma->ggtt_view.pages = NULL; + } + } drm_mm_remove_node(&vma->node); i915_gem_vma_destroy(vma); @@ -3099,6 +3100,10 @@ int i915_vma_unbind(struct i915_vma *vma) /* Since the unbound list is global, only move to that list if * no more VMAs exist. */ if (list_empty(&obj->vma_list)) { + /* Throw away the active reference before + * moving to the unbound list. */ + i915_gem_object_retire(obj); + i915_gem_gtt_finish_object(obj); list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list); } @@ -3509,7 +3514,8 @@ static struct i915_vma * i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, struct i915_address_space *vm, unsigned alignment, - uint64_t flags) + uint64_t flags, + const struct i915_ggtt_view *view) { struct drm_device *dev = obj->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -3559,7 +3565,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, i915_gem_object_pin_pages(obj); - vma = i915_gem_obj_lookup_or_create_vma(obj, vm); + vma = i915_gem_obj_lookup_or_create_vma_view(obj, vm, view); if (IS_ERR(vma)) goto err_unpin; @@ -3589,15 +3595,19 @@ search_free: if (ret) goto err_remove_node; + trace_i915_vma_bind(vma, flags); + ret = i915_vma_bind(vma, obj->cache_level, + flags & PIN_GLOBAL ? GLOBAL_BIND : 0); + if (ret) + goto err_finish_gtt; + list_move_tail(&obj->global_list, &dev_priv->mm.bound_list); list_add_tail(&vma->mm_list, &vm->inactive_list); - trace_i915_vma_bind(vma, flags); - vma->bind_vma(vma, obj->cache_level, - flags & PIN_GLOBAL ? GLOBAL_BIND : 0); - return vma; +err_finish_gtt: + i915_gem_gtt_finish_object(obj); err_remove_node: drm_mm_remove_node(&vma->node); err_free_vma: @@ -3800,9 +3810,12 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, } list_for_each_entry(vma, &obj->vma_list, vma_link) - if (drm_mm_node_allocated(&vma->node)) - vma->bind_vma(vma, cache_level, - vma->bound & GLOBAL_BIND); + if (drm_mm_node_allocated(&vma->node)) { + ret = i915_vma_bind(vma, cache_level, + vma->bound & GLOBAL_BIND); + if (ret) + return ret; + } } list_for_each_entry(vma, &obj->vma_list, vma_link) @@ -4155,10 +4168,11 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags) } int -i915_gem_object_pin(struct drm_i915_gem_object *obj, - struct i915_address_space *vm, - uint32_t alignment, - uint64_t flags) +i915_gem_object_pin_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + uint32_t alignment, + uint64_t flags, + const struct i915_ggtt_view *view) { struct drm_i915_private *dev_priv = obj->base.dev->dev_private; struct i915_vma *vma; @@ -4174,7 +4188,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, if (WARN_ON((flags & (PIN_MAPPABLE | PIN_GLOBAL)) == PIN_MAPPABLE)) return -EINVAL; - vma = i915_gem_obj_to_vma(obj, vm); + vma = i915_gem_obj_to_vma_view(obj, vm, view); if (vma) { if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)) return -EBUSY; @@ -4184,7 +4198,8 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, "bo is already pinned with incorrect alignment:" " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d," " obj->map_and_fenceable=%d\n", - i915_gem_obj_offset(obj, vm), alignment, + i915_gem_obj_offset_view(obj, vm, view->type), + alignment, !!(flags & PIN_MAPPABLE), obj->map_and_fenceable); ret = i915_vma_unbind(vma); @@ -4197,13 +4212,17 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, bound = vma ? vma->bound : 0; if (vma == NULL || !drm_mm_node_allocated(&vma->node)) { - vma = i915_gem_object_bind_to_vm(obj, vm, alignment, flags); + vma = i915_gem_object_bind_to_vm(obj, vm, alignment, + flags, view); if (IS_ERR(vma)) return PTR_ERR(vma); } - if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND)) - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); + if (flags & PIN_GLOBAL && !(vma->bound & GLOBAL_BIND)) { + ret = i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND); + if (ret) + return ret; + } if ((bound ^ vma->bound) & GLOBAL_BIND) { bool mappable, fenceable; @@ -4539,12 +4558,13 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) intel_runtime_pm_put(dev_priv); } -struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) +struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma; list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm) + if (vma->vm == vm && vma->ggtt_view.type == view->type) return vma; return NULL; @@ -5156,8 +5176,9 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) } /* All the new VM stuff */ -unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, - struct i915_address_space *vm) +unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view) { struct drm_i915_private *dev_priv = o->base.dev->dev_private; struct i915_vma *vma; @@ -5165,7 +5186,7 @@ unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base); list_for_each_entry(vma, &o->vma_list, vma_link) { - if (vma->vm == vm) + if (vma->vm == vm && vma->ggtt_view.type == view) return vma->node.start; } @@ -5174,13 +5195,16 @@ unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, return -1; } -bool i915_gem_obj_bound(struct drm_i915_gem_object *o, - struct i915_address_space *vm) +bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o, + struct i915_address_space *vm, + enum i915_ggtt_view_type view) { struct i915_vma *vma; list_for_each_entry(vma, &o->vma_list, vma_link) - if (vma->vm == vm && drm_mm_node_allocated(&vma->node)) + if (vma->vm == vm && + vma->ggtt_view.type == view && + drm_mm_node_allocated(&vma->node)) return true; return false; @@ -5315,10 +5339,10 @@ struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj) struct i915_address_space *ggtt = i915_obj_to_ggtt(obj); struct i915_vma *vma; - list_for_each_entry(vma, &obj->vma_list, vma_link) { - if (vma->vm == ggtt) + list_for_each_entry(vma, &obj->vma_list, vma_link) + if (vma->vm == ggtt && + vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) return vma; - } return NULL; } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 5cd2b97..4ef8815 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -590,9 +590,14 @@ static int do_switch(struct intel_engine_cs *ring, goto unpin_out; vma = i915_gem_obj_to_ggtt(to->legacy_hw_ctx.rcs_state); - if (!(vma->bound & GLOBAL_BIND)) - vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level, - GLOBAL_BIND); + if (!(vma->bound & GLOBAL_BIND)) { + ret = i915_vma_bind(vma, + to->legacy_hw_ctx.rcs_state->cache_level, + GLOBAL_BIND); + /* This shouldn't ever fail. */ + if (WARN_ONCE(ret, "GGTT context bind failed!")) + goto unpin_out; + } if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) hw_flags |= MI_RESTORE_INHIBIT; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0c25f62..3927d93 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -360,9 +360,12 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, * through the ppgtt for non_secure batchbuffers. */ if (unlikely(IS_GEN6(dev) && reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && - !(target_vma->bound & GLOBAL_BIND))) - target_vma->bind_vma(target_vma, target_i915_obj->cache_level, - GLOBAL_BIND); + !(target_vma->bound & GLOBAL_BIND))) { + ret = i915_vma_bind(target_vma, target_i915_obj->cache_level, + GLOBAL_BIND); + if (WARN_ONCE(ret, "Unexpected failure to bind target VMA!")) + return ret; + } /* Validate that the target is in a valid r/w GPU domain */ if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) { diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ac03a38..73c1c0b 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -30,6 +30,8 @@ #include "i915_trace.h" #include "intel_drv.h" +const struct i915_ggtt_view i915_ggtt_view_normal; + static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); static void chv_setup_private_ppat(struct drm_i915_private *dev_priv); @@ -1341,9 +1343,12 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev) /* The bind_vma code tries to be smart about tracking mappings. * Unfortunately above, we've just wiped out the mappings * without telling our object about it. So we need to fake it. + * + * Bind is not expected to fail since this is only called on + * resume and assumption is all requirements exist already. */ vma->bound &= ~GLOBAL_BIND; - vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); + WARN_ON(i915_vma_bind(vma, obj->cache_level, GLOBAL_BIND)); } @@ -1538,7 +1543,7 @@ static void i915_ggtt_bind_vma(struct i915_vma *vma, AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY; BUG_ON(!i915_is_ggtt(vma->vm)); - intel_gtt_insert_sg_entries(vma->obj->pages, entry, flags); + intel_gtt_insert_sg_entries(vma->ggtt_view.pages, entry, flags); vma->bound = GLOBAL_BIND; } @@ -1588,7 +1593,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) { if (!(vma->bound & GLOBAL_BIND) || (cache_level != obj->cache_level)) { - vma->vm->insert_entries(vma->vm, obj->pages, + vma->vm->insert_entries(vma->vm, vma->ggtt_view.pages, vma->node.start, cache_level, flags); vma->bound |= GLOBAL_BIND; @@ -1600,7 +1605,7 @@ static void ggtt_bind_vma(struct i915_vma *vma, (cache_level != obj->cache_level))) { struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt; appgtt->base.insert_entries(&appgtt->base, - vma->obj->pages, + vma->ggtt_view.pages, vma->node.start, cache_level, flags); vma->bound |= LOCAL_BIND; @@ -2165,7 +2170,8 @@ int i915_gem_gtt_init(struct drm_device *dev) } static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); if (vma == NULL) @@ -2176,6 +2182,7 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, INIT_LIST_HEAD(&vma->exec_list); vma->vm = vm; vma->obj = obj; + vma->ggtt_view = *view; switch (INTEL_INFO(vm->dev)->gen) { case 9: @@ -2210,14 +2217,59 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, } struct i915_vma * -i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) +i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_ggtt_view *view) { struct i915_vma *vma; - vma = i915_gem_obj_to_vma(obj, vm); + vma = i915_gem_obj_to_vma_view(obj, vm, view); if (!vma) - vma = __i915_gem_vma_create(obj, vm); + vma = __i915_gem_vma_create(obj, vm, view); return vma; } + +static inline +int i915_get_vma_pages(struct i915_vma *vma) +{ + if (vma->ggtt_view.pages) + return 0; + + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) + vma->ggtt_view.pages = vma->obj->pages; + else + WARN_ONCE(1, "GGTT view %u not implemented!\n", + vma->ggtt_view.type); + + if (!vma->ggtt_view.pages) { + DRM_ERROR("Failed to get pages for VMA view type %u!\n", + vma->ggtt_view.type); + return -EINVAL; + } + + return 0; +} + +/** + * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space. + * @vma: VMA to map + * @cache_level: mapping cache level + * @flags: flags like global or local mapping + * + * DMA addresses are taken from the scatter-gather table of this object (or of + * this VMA in case of non-default GGTT views) and PTE entries set up. + * Note that DMA addresses are also the only part of the SG table we care about. + */ +int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, + u32 flags) +{ + int ret = i915_get_vma_pages(vma); + + if (ret) + return ret; + + vma->bind_vma(vma, cache_level, flags); + + return 0; +} diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index dd849df..e377c7d 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -109,7 +109,20 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; #define GEN8_PPAT_ELLC_OVERRIDE (0<<2) #define GEN8_PPAT(i, x) ((uint64_t) (x) << ((i) * 8)) +enum i915_ggtt_view_type { + I915_GGTT_VIEW_NORMAL = 0, +}; + +struct i915_ggtt_view { + enum i915_ggtt_view_type type; + + struct sg_table *pages; +}; + +extern const struct i915_ggtt_view i915_ggtt_view_normal; + enum i915_cache_level; + /** * A VMA represents a GEM BO that is bound into an address space. Therefore, a * VMA's presence cannot be guaranteed before binding, or after unbinding the @@ -129,6 +142,15 @@ struct i915_vma { #define PTE_READ_ONLY (1<<2) unsigned int bound : 4; + /** + * Support different GGTT views into the same object. + * This means there can be multiple VMA mappings per object and per VM. + * i915_ggtt_view_type is used to distinguish between those entries. + * The default one of zero (I915_GGTT_VIEW_NORMAL) is default and also + * assumed in GEM functions which take no ggtt view parameter. + */ + struct i915_ggtt_view ggtt_view; + /** This object's place on the active/inactive lists */ struct list_head mm_list; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c4536e1..f97479a 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -718,10 +718,8 @@ static u32 capture_pinned_bo(struct drm_i915_error_buffer *err, break; list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm && vma->pin_count > 0) { + if (vma->vm == vm && vma->pin_count > 0) capture_bo(err++, vma); - break; - } } return err - first; @@ -1096,10 +1094,8 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv, list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { list_for_each_entry(vma, &obj->vma_list, vma_link) - if (vma->vm == vm && vma->pin_count > 0) { + if (vma->vm == vm && vma->pin_count > 0) i++; - break; - } } error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx]; -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-10 17:27 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin @ 2014-12-11 10:18 ` Michel Thierry 2014-12-15 10:24 ` Daniel Vetter 1 sibling, 0 replies; 37+ messages in thread From: Michel Thierry @ 2014-12-11 10:18 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx@lists.freedesktop.org; +Cc: Daniel Vetter [-- Attachment #1.1: Type: text/plain, Size: 2104 bytes --] On 12/10/2014 5:27 PM, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Things like reliable GGTT mappings and mirrored 2d-on-3d display will need > to map objects into the same address space multiple times. > > Added a GGTT view concept and linked it with the VMA to distinguish between > 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. > > This now means that objects can have multiple VMA entries so the code which > assumed there will only be one also had to be modified. > > Alternative GGTT views are supposed to borrow DMA addresses from obj->pages > which is DMA mapped on first VMA instantiation and unmapped on the last one > going away. > > v2: > * Removed per view special casing in i915_gem_ggtt_prepare / > finish_object in favour of creating and destroying DMA mappings > on first VMA instantiation and last VMA destruction. (Daniel Vetter) > * Simplified i915_vma_unbind which does not need to count the GGTT views. > (Daniel Vetter) > * Also moved obj->map_and_fenceable reset under the same check. > * Checkpatch cleanups. > > v3: > * Only retire objects once the last VMA is unbound. > > v4: > * Keep scatter-gather table for alternative views persistent for the > lifetime of the VMA. > * Propagate binding errors to callers and handle appropriately. > > v5: > * Explicitly look for normal GGTT view in i915_gem_obj_bound to align > usage in i915_gem_object_ggtt_unpin. (Michel Thierry) > * Change to single if statement in i915_gem_obj_to_ggtt. (Michel Thierry) > * Removed stray semi-colon in i915_gem_object_set_cache_level. > > For: VIZ-4544 > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Good catch in i915_gem_object_set_cache_level. Reviewed-by: Michel Thierry <michel.thierry@intel.com> [-- Attachment #1.2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 5510 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-10 17:27 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin 2014-12-11 10:18 ` Michel Thierry @ 2014-12-15 10:24 ` Daniel Vetter 2014-12-15 11:23 ` Tvrtko Ursulin 1 sibling, 1 reply; 37+ messages in thread From: Daniel Vetter @ 2014-12-15 10:24 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx On Wed, Dec 10, 2014 at 05:27:58PM +0000, Tvrtko Ursulin wrote: > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index de241eb..c6610bc 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2098,8 +2098,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, > /* For the unbound phase, this should be a no-op! */ > list_for_each_entry_safe(vma, v, > &obj->vma_list, vma_link) > - if (i915_vma_unbind(vma)) > - break; > + i915_vma_unbind(vma); This hunk adds a new warning. It's the 2nd time a __must_check warning from someone from vpg london crept through all the premerge testing. Can you please double-check whether you have some crazy gcc flags settings or something in your builders? I've dropped the hunk locally. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object 2014-12-15 10:24 ` Daniel Vetter @ 2014-12-15 11:23 ` Tvrtko Ursulin 0 siblings, 0 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-15 11:23 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, Intel-gfx On 12/15/2014 10:24 AM, Daniel Vetter wrote: > On Wed, Dec 10, 2014 at 05:27:58PM +0000, Tvrtko Ursulin wrote: >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index de241eb..c6610bc 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -2098,8 +2098,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, >> /* For the unbound phase, this should be a no-op! */ >> list_for_each_entry_safe(vma, v, >> &obj->vma_list, vma_link) >> - if (i915_vma_unbind(vma)) >> - break; >> + i915_vma_unbind(vma); > > This hunk adds a new warning. It's the 2nd time a __must_check warning > from someone from vpg london crept through all the premerge testing. Can > you please double-check whether you have some crazy gcc flags settings or > something in your builders? > > I've dropped the hunk locally. Yes sorry, somehow I've lost the relevant flag from Kconfig. Doesn't matter for this hunk I suppose since it was in the air anyway. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 2/2] drm/i915: Documentation for multiple GGTT views 2014-12-10 17:27 ` [PATCH v3 0/2] Multiple " Tvrtko Ursulin 2014-12-10 17:27 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin @ 2014-12-10 17:27 ` Tvrtko Ursulin 2014-12-11 3:41 ` shuang.he 2014-12-11 10:19 ` Michel Thierry 1 sibling, 2 replies; 37+ messages in thread From: Tvrtko Ursulin @ 2014-12-10 17:27 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> A short section describing background, implementation and intended usage. v2: * Align section name between template and DOC comment. (Michel Thierry) For: VIZ-4544 Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- Documentation/DocBook/drm.tmpl | 5 ++++ drivers/gpu/drm/i915/i915_gem_gtt.c | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 85287cb..eb84da73 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4033,6 +4033,11 @@ int num_ioctls;</synopsis> !Pdrivers/gpu/drm/i915/intel_lrc.c Logical Rings, Logical Ring Contexts and Execlists !Idrivers/gpu/drm/i915/intel_lrc.c </sect2> + <sect2> + <title>Global GTT views</title> +!Pdrivers/gpu/drm/i915/i915_gem_gtt.c Global GTT views +!Idrivers/gpu/drm/i915/i915_gem_gtt.c + </sect2> </sect1> <sect1> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 73c1c0b..534ebc1 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -30,6 +30,66 @@ #include "i915_trace.h" #include "intel_drv.h" +/** + * DOC: Global GTT views + * + * Background and previous state + * + * Historically objects could exists (be bound) in global GTT space only as + * singular instances with a view representing all of the object's backing pages + * in a linear fashion. This view will be called a normal view. + * + * To support multiple views of the same object, where the number of mapped + * pages is not equal to the backing store, or where the layout of the pages + * is not linear, concept of a GGTT view was added. + * + * One example of an alternative view is a stereo display driven by a single + * image. In this case we would have a framebuffer looking like this + * (2x2 pages): + * + * 12 + * 34 + * + * Above would represent a normal GGTT view as normally mapped for GPU or CPU + * rendering. In contrast, fed to the display engine would be an alternative + * view which could look something like this: + * + * 1212 + * 3434 + * + * In this example both the size and layout of pages in the alternative view is + * different from the normal view. + * + * Implementation and usage + * + * GGTT views are implemented using VMAs and are distinguished via enum + * i915_ggtt_view_type and struct i915_ggtt_view. + * + * A new flavour of core GEM functions which work with GGTT bound objects were + * added with the _view suffix. They take the struct i915_ggtt_view parameter + * encapsulating all metadata required to implement a view. + * + * As a helper for callers which are only interested in the normal view, + * globally const i915_ggtt_view_normal singleton instance exists. All old core + * GEM API functions, the ones not taking the view parameter, are operating on, + * or with the normal GGTT view. + * + * Code wanting to add or use a new GGTT view needs to: + * + * 1. Add a new enum with a suitable name. + * 2. Extend the metadata in the i915_ggtt_view structure if required. + * 3. Add support to i915_get_vma_pages(). + * + * New views are required to build a scatter-gather table from within the + * i915_get_vma_pages function. This table is stored in the vma.ggtt_view and + * exists for the lifetime of an VMA. + * + * Core API is designed to have copy semantics which means that passed in + * struct i915_ggtt_view does not need to be persistent (left around after + * calling the core API functions). + * + */ + const struct i915_ggtt_view i915_ggtt_view_normal; static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] drm/i915: Documentation for multiple GGTT views 2014-12-10 17:27 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin @ 2014-12-11 3:41 ` shuang.he 2014-12-11 10:19 ` Michel Thierry 1 sibling, 0 replies; 37+ messages in thread From: shuang.he @ 2014-12-11 3:41 UTC (permalink / raw) To: shuang.he, intel-gfx, tvrtko.ursulin Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK +1-2 364/366 363/366 SNB 448/450 448/450 IVB 497/498 497/498 BYT 289/289 289/289 HSW 563/564 563/564 BDW 417/417 417/417 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied *ILK igt_kms_setmode_invalid-clone-exclusive-crtc PASS(5, M26) DMESG_WARN(1, M26) *ILK igt_kms_flip_rcs-flip-vs-modeset NSPT(1, M26)PASS(4, M26) DMESG_WARN(1, M26) ILK igt_kms_flip_wf_vblank-ts-check DMESG_WARN(5, M26)PASS(20, M26M37) PASS(1, M26) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] drm/i915: Documentation for multiple GGTT views 2014-12-10 17:27 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 2014-12-11 3:41 ` shuang.he @ 2014-12-11 10:19 ` Michel Thierry 2014-12-15 8:30 ` Daniel Vetter 1 sibling, 1 reply; 37+ messages in thread From: Michel Thierry @ 2014-12-11 10:19 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx@lists.freedesktop.org [-- Attachment #1.1: Type: text/plain, Size: 407 bytes --] On 12/10/2014 5:27 PM, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > A short section describing background, implementation and intended usage. > > v2: > * Align section name between template and DOC comment. (Michel Thierry) > > For: VIZ-4544 > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Michel Thierry <michel.thierry@intel.com> [-- Attachment #1.2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 5510 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] drm/i915: Documentation for multiple GGTT views 2014-12-11 10:19 ` Michel Thierry @ 2014-12-15 8:30 ` Daniel Vetter 2014-12-15 10:01 ` Michel Thierry 0 siblings, 1 reply; 37+ messages in thread From: Daniel Vetter @ 2014-12-15 8:30 UTC (permalink / raw) To: Michel Thierry; +Cc: Intel-gfx@lists.freedesktop.org On Thu, Dec 11, 2014 at 10:19:39AM +0000, Michel Thierry wrote: > On 12/10/2014 5:27 PM, Tvrtko Ursulin wrote: > >From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > >A short section describing background, implementation and intended usage. > > > >v2: > > * Align section name between template and DOC comment. (Michel Thierry) > > > >For: VIZ-4544 > >Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Reviewed-by: Michel Thierry <michel.thierry@intel.com> Both merged, thanks for patches and review. Michel, I guess you'll also review the -internal skl patches which are based on top of this now? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] drm/i915: Documentation for multiple GGTT views 2014-12-15 8:30 ` Daniel Vetter @ 2014-12-15 10:01 ` Michel Thierry 0 siblings, 0 replies; 37+ messages in thread From: Michel Thierry @ 2014-12-15 10:01 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel-gfx@lists.freedesktop.org [-- Attachment #1.1: Type: text/plain, Size: 786 bytes --] On 12/15/2014 8:30 AM, Daniel Vetter wrote: > On Thu, Dec 11, 2014 at 10:19:39AM +0000, Michel Thierry wrote: >> On 12/10/2014 5:27 PM, Tvrtko Ursulin wrote: >>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> >>> A short section describing background, implementation and intended usage. >>> >>> v2: >>> * Align section name between template and DOC comment. (Michel Thierry) >>> >>> For: VIZ-4544 >>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Reviewed-by: Michel Thierry <michel.thierry@intel.com> > Both merged, thanks for patches and review. Michel, I guess you'll also > review the -internal skl patches which are based on top of this now? > -Daniel Yes, I already started with them, will reply to -internal soon. Thanks, -Michel [-- Attachment #1.2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 5510 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2014-12-15 11:23 UTC | newest] Thread overview: 37+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-03 14:59 [PATCH 0/3] Multiple GGTT views Tvrtko Ursulin 2014-12-03 14:59 ` [PATCH 1/3] drm/i915: Stop putting GGTT VMA at the head of the list Tvrtko Ursulin 2014-12-04 9:48 ` Chris Wilson 2014-12-04 10:02 ` Tvrtko Ursulin 2014-12-04 10:17 ` Chris Wilson 2014-12-04 10:30 ` Tvrtko Ursulin 2014-12-04 10:39 ` Chris Wilson 2014-12-04 10:48 ` Tvrtko Ursulin 2014-12-04 10:54 ` Daniel Vetter 2014-12-03 14:59 ` [PATCH 2/3] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin 2014-12-04 9:53 ` Chris Wilson 2014-12-04 10:19 ` Tvrtko Ursulin 2014-12-04 10:26 ` Chris Wilson 2014-12-04 10:59 ` Daniel Vetter 2014-12-04 12:17 ` Tvrtko Ursulin 2014-12-04 12:27 ` Chris Wilson 2014-12-04 13:01 ` Daniel Vetter 2014-12-03 14:59 ` [PATCH 3/3] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 2014-12-04 7:12 ` shuang.he 2014-12-05 12:11 ` [PATCH v2 0/2] Multiple " Tvrtko Ursulin 2014-12-05 12:11 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin 2014-12-05 14:25 ` Daniel Vetter 2014-12-09 15:23 ` Michel Thierry 2014-12-10 9:16 ` Daniel Vetter 2014-12-10 10:17 ` Tvrtko Ursulin 2014-12-05 12:11 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 2014-12-09 15:41 ` Michel Thierry 2014-12-10 17:27 ` [PATCH v3 0/2] Multiple " Tvrtko Ursulin 2014-12-10 17:27 ` [PATCH 1/2] drm/i915: Infrastructure for supporting different GGTT views per object Tvrtko Ursulin 2014-12-11 10:18 ` Michel Thierry 2014-12-15 10:24 ` Daniel Vetter 2014-12-15 11:23 ` Tvrtko Ursulin 2014-12-10 17:27 ` [PATCH 2/2] drm/i915: Documentation for multiple GGTT views Tvrtko Ursulin 2014-12-11 3:41 ` shuang.he 2014-12-11 10:19 ` Michel Thierry 2014-12-15 8:30 ` Daniel Vetter 2014-12-15 10:01 ` Michel Thierry
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox