| Message ID | 1427450584.4966.8.camel@jlahtine-mobl1 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 03/27/2015 10:03 AM, Joonas Lahtinen wrote: > To allow for views where the view type is not defined by the view type only, > like it is in stereo or rotated 90 degree view, change the semantic to require > the whole view structure for comparison when we match a GGTT view. > > This allows including parameters like offset to be included in the view which > is useful for eg. partial views. > > v3: > - Rely on ggtt_view type being 0 for non-GGTT vma's, which equals to > I915_GGTT_VIEW_NORMAL. (Daniel Vetter) > - Do not use potentially slower comparison when we only want to know if > something is or is not a normal view. > - Rebase on top of rotated view patches. Add rotated view singleton. > - If one view is missing in comparison they're equal only if both are missing. > > Cc: Daniel Vetter <daniel@ffwll.ch> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> (v2) > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 8 ++++---- > drivers/gpu/drm/i915/i915_gem.c | 13 +++++++------ > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > drivers/gpu/drm/i915/i915_gem_gtt.h | 8 ++++++++ > drivers/gpu/drm/i915/intel_display.c | 8 +++----- > 5 files changed, 25 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 256369f..5aeba60 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2794,19 +2794,19 @@ void i915_gem_restore_fences(struct drm_device *dev); > > unsigned long > i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o, > - enum i915_ggtt_view_type view); > + const struct i915_ggtt_view *view); > unsigned long > i915_gem_obj_offset(struct drm_i915_gem_object *o, > struct i915_address_space *vm); > static inline unsigned long > i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o) > { > - return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL); > + return i915_gem_obj_ggtt_offset_view(o, &i915_ggtt_view_normal); > } > > bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); > bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o, > - enum i915_ggtt_view_type view); > + const struct i915_ggtt_view *view); > bool i915_gem_obj_bound(struct drm_i915_gem_object *o, > struct i915_address_space *vm); > > @@ -2854,7 +2854,7 @@ i915_vm_to_ppgtt(struct i915_address_space *vm) > > static inline bool i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *obj) > { > - return i915_gem_obj_ggtt_bound_view(obj, I915_GGTT_VIEW_NORMAL); > + return i915_gem_obj_ggtt_bound_view(obj, &i915_ggtt_view_normal); > } > > static inline unsigned long > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 3300963..8c32b1d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4131,7 +4131,7 @@ i915_gem_object_do_pin(struct drm_i915_gem_object *obj, > > if (i915_vma_misplaced(vma, alignment, flags)) { > unsigned long offset; > - offset = ggtt_view ? i915_gem_obj_ggtt_offset_view(obj, ggtt_view->type) : > + offset = ggtt_view ? i915_gem_obj_ggtt_offset_view(obj, ggtt_view) : > i915_gem_obj_offset(obj, vm); > WARN(vma->pin_count, > "bo is already pinned in %s with incorrect alignment:" > @@ -4230,7 +4230,7 @@ i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, > > BUG_ON(!vma); > WARN_ON(vma->pin_count == 0); > - WARN_ON(!i915_gem_obj_ggtt_bound_view(obj, view->type)); > + WARN_ON(!i915_gem_obj_ggtt_bound_view(obj, view)); > > if (--vma->pin_count == 0 && view->type == I915_GGTT_VIEW_NORMAL) > obj->pin_mappable = false; No comparison helper in i915_gem_obj_to_ggtt_view()? > @@ -5109,13 +5109,14 @@ i915_gem_obj_offset(struct drm_i915_gem_object *o, > > unsigned long > i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o, > - enum i915_ggtt_view_type view) > + const struct i915_ggtt_view *view) > { > struct i915_address_space *ggtt = i915_obj_to_ggtt(o); > struct i915_vma *vma; > > list_for_each_entry(vma, &o->vma_list, vma_link) > - if (vma->vm == ggtt && vma->ggtt_view.type == view) > + if (vma->vm == ggtt && > + i915_ggtt_view_equal(&vma->ggtt_view, view)) > return vma->node.start; > > WARN(1, "global vma for this object not found.\n"); > @@ -5139,14 +5140,14 @@ bool i915_gem_obj_bound(struct drm_i915_gem_object *o, > } > > bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o, > - enum i915_ggtt_view_type view) > + const struct i915_ggtt_view *view) > { > struct i915_address_space *ggtt = i915_obj_to_ggtt(o); > struct i915_vma *vma; > > list_for_each_entry(vma, &o->vma_list, vma_link) > if (vma->vm == ggtt && > - vma->ggtt_view.type == view && > + i915_ggtt_view_equal(&vma->ggtt_view, view) && > drm_mm_node_allocated(&vma->node)) > return true; > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 9903bb0..13eb769 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -93,6 +93,9 @@ > */ > > const struct i915_ggtt_view i915_ggtt_view_normal; > +const struct i915_ggtt_view i915_ggtt_view_rotated = { > + .type = I915_GGTT_VIEW_ROTATED > +}; > > static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); > static void chv_setup_private_ppat(struct drm_i915_private *dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 0dad426..c1c426a 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -138,6 +138,7 @@ struct i915_ggtt_view { > }; > > extern const struct i915_ggtt_view i915_ggtt_view_normal; > +extern const struct i915_ggtt_view i915_ggtt_view_rotated; > > enum i915_cache_level; > > @@ -422,4 +423,11 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev); > int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj); > void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj); > > +static inline bool > +i915_ggtt_view_equal(const struct i915_ggtt_view *a, > + const struct i915_ggtt_view *b) > +{ > + return !!a && !!b ? a->type == b->type : !a && !b; > +} What's the background behind this both NULL check? Will you ever use it like that? I think the code elsewhere makes sure to always pass valid views to this internal helper so I don't get it. > + > #endif > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index cb50854..d6c2e34 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2293,8 +2293,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb, > const struct drm_plane_state *plane_state) > { > struct intel_rotation_info *info = &view->rotation_info; > - static const struct i915_ggtt_view rotated_view = > - { .type = I915_GGTT_VIEW_ROTATED }; > > *view = i915_ggtt_view_normal; > > @@ -2304,7 +2302,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb, > if (!intel_rotation_90_or_270(plane_state->rotation)) > return 0; > > - *view = rotated_view; > + *view = i915_ggtt_view_rotated; > > info->height = fb->height; > info->pixel_format = fb->pixel_format; > @@ -2904,10 +2902,10 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier, > unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane, > struct drm_i915_gem_object *obj) > { > - enum i915_ggtt_view_type view = I915_GGTT_VIEW_NORMAL; > + const struct i915_ggtt_view *view = &i915_ggtt_view_normal; > > if (intel_rotation_90_or_270(intel_plane->base.state->rotation)) > - view = I915_GGTT_VIEW_ROTATED; > + view = &i915_ggtt_view_rotated; > > return i915_gem_obj_ggtt_offset_view(obj, view); > } > Regards, Tvrtko
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6072
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 303/303 303/303
SNB 304/304 304/304
IVB -1 330/330 329/330
BYT 287/287 287/287
HSW 361/361 361/361
BDW 309/309 309/309
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*IVB igt@gem_pwrite_pread@snooped-copy-performance PASS(3) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
Note: You need to pay more attention to line start with '*'
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 256369f..5aeba60 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2794,19 +2794,19 @@ void i915_gem_restore_fences(struct drm_device *dev); unsigned long i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o, - enum i915_ggtt_view_type view); + const struct i915_ggtt_view *view); unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o, struct i915_address_space *vm); static inline unsigned long i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o) { - return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL); + return i915_gem_obj_ggtt_offset_view(o, &i915_ggtt_view_normal); } bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o, - enum i915_ggtt_view_type view); + const struct i915_ggtt_view *view); bool i915_gem_obj_bound(struct drm_i915_gem_object *o, struct i915_address_space *vm); @@ -2854,7 +2854,7 @@ i915_vm_to_ppgtt(struct i915_address_space *vm) static inline bool i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *obj) { - return i915_gem_obj_ggtt_bound_view(obj, I915_GGTT_VIEW_NORMAL); + return i915_gem_obj_ggtt_bound_view(obj, &i915_ggtt_view_normal); } static inline unsigned long diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3300963..8c32b1d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4131,7 +4131,7 @@ i915_gem_object_do_pin(struct drm_i915_gem_object *obj, if (i915_vma_misplaced(vma, alignment, flags)) { unsigned long offset; - offset = ggtt_view ? i915_gem_obj_ggtt_offset_view(obj, ggtt_view->type) : + offset = ggtt_view ? i915_gem_obj_ggtt_offset_view(obj, ggtt_view) : i915_gem_obj_offset(obj, vm); WARN(vma->pin_count, "bo is already pinned in %s with incorrect alignment:" @@ -4230,7 +4230,7 @@ i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, BUG_ON(!vma); WARN_ON(vma->pin_count == 0); - WARN_ON(!i915_gem_obj_ggtt_bound_view(obj, view->type)); + WARN_ON(!i915_gem_obj_ggtt_bound_view(obj, view)); if (--vma->pin_count == 0 && view->type == I915_GGTT_VIEW_NORMAL) obj->pin_mappable = false; @@ -5109,13 +5109,14 @@ i915_gem_obj_offset(struct drm_i915_gem_object *o, unsigned long i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o, - enum i915_ggtt_view_type view) + const struct i915_ggtt_view *view) { struct i915_address_space *ggtt = i915_obj_to_ggtt(o); struct i915_vma *vma; list_for_each_entry(vma, &o->vma_list, vma_link) - if (vma->vm == ggtt && vma->ggtt_view.type == view) + if (vma->vm == ggtt && + i915_ggtt_view_equal(&vma->ggtt_view, view)) return vma->node.start; WARN(1, "global vma for this object not found.\n"); @@ -5139,14 +5140,14 @@ bool i915_gem_obj_bound(struct drm_i915_gem_object *o, } bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o, - enum i915_ggtt_view_type view) + const struct i915_ggtt_view *view) { struct i915_address_space *ggtt = i915_obj_to_ggtt(o); struct i915_vma *vma; list_for_each_entry(vma, &o->vma_list, vma_link) if (vma->vm == ggtt && - vma->ggtt_view.type == view && + i915_ggtt_view_equal(&vma->ggtt_view, view) && drm_mm_node_allocated(&vma->node)) return true; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 9903bb0..13eb769 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -93,6 +93,9 @@ */ const struct i915_ggtt_view i915_ggtt_view_normal; +const struct i915_ggtt_view i915_ggtt_view_rotated = { + .type = I915_GGTT_VIEW_ROTATED +}; static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv); static void chv_setup_private_ppat(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 0dad426..c1c426a 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -138,6 +138,7 @@ struct i915_ggtt_view { }; extern const struct i915_ggtt_view i915_ggtt_view_normal; +extern const struct i915_ggtt_view i915_ggtt_view_rotated; enum i915_cache_level; @@ -422,4 +423,11 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev); int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj); void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj); +static inline bool +i915_ggtt_view_equal(const struct i915_ggtt_view *a, + const struct i915_ggtt_view *b) +{ + return !!a && !!b ? a->type == b->type : !a && !b; +} + #endif diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cb50854..d6c2e34 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2293,8 +2293,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb, const struct drm_plane_state *plane_state) { struct intel_rotation_info *info = &view->rotation_info; - static const struct i915_ggtt_view rotated_view = - { .type = I915_GGTT_VIEW_ROTATED }; *view = i915_ggtt_view_normal; @@ -2304,7 +2302,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb, if (!intel_rotation_90_or_270(plane_state->rotation)) return 0; - *view = rotated_view; + *view = i915_ggtt_view_rotated; info->height = fb->height; info->pixel_format = fb->pixel_format; @@ -2904,10 +2902,10 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier, unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane, struct drm_i915_gem_object *obj) { - enum i915_ggtt_view_type view = I915_GGTT_VIEW_NORMAL; + const struct i915_ggtt_view *view = &i915_ggtt_view_normal; if (intel_rotation_90_or_270(intel_plane->base.state->rotation)) - view = I915_GGTT_VIEW_ROTATED; + view = &i915_ggtt_view_rotated; return i915_gem_obj_ggtt_offset_view(obj, view); }