On Mon, 14 Sep 2015 10:08:19 +0100
Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> On 09/12/2015 02:44 AM, Vivek Kasireddy wrote:
> > From: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> >
> > Currently, fb objects with rotated views are ignored while pinning.
> > Therefore, include the rotated view type and use the view size
> > instead of the object's size to determine if it is fenceable. And,
> > look at the view and its offset while writing and pinning to the
> > fence registers.
>
> I didn't figure out from the commit message if something is broken or?
>
> AFAIR rotated views deliberately skip on fencing since rotated view
> has shuffled pages in memory so it would be a weird view for
> userspace to handle.
Hi Tvrtko,
I apologize for the short and ambiguous commit message.
As I mentioned in my other reply to Chris, the reason for this patch is
because of this:
if (WARN_ON(!obj->map_and_fenceable))
return -EINVAL;
inside i915_find_fence_reg(). I am trying to enable 90 degree rotation
for the Weston compositor as part of which I need to allocate and flip
a Y-tiled fb obj. This fails because i915_gem_object_do_pin() ignores
the rotated view passed by i915_gem_object_pin_to_display_plane() and
hence obj->map_and_fenceable never gets set.
>
> Especially this below:
>
> > static void i965_write_fence_reg(struct drm_device *dev, int reg,
> > - struct drm_i915_gem_object *obj)
> > + struct drm_i915_gem_object *obj,
> > + const struct i915_ggtt_view *view)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > int fence_reg;
> > int fence_pitch_shift;
> > + const struct i915_ggtt_view *ggtt_view = view;
> >
> > if (INTEL_INFO(dev)->gen >= 6) {
> > fence_reg = FENCE_REG_SANDYBRIDGE_0;
> > @@ -95,9 +97,13 @@ static void i965_write_fence_reg(struct
> > drm_device *dev, int reg, size = (size / row_size) * row_size;
> > }
> >
> > - val = (uint64_t)((i915_gem_obj_ggtt_offset(obj) +
> > size - 4096) &
> > - 0xfffff000) << 32;
> > - val |= i915_gem_obj_ggtt_offset(obj) & 0xfffff000;
> > + if (!ggtt_view)
> > + ggtt_view = &i915_ggtt_view_normal;
> > +
> > + val =
> > (uint64_t)((i915_gem_obj_ggtt_offset_view((obj), ggtt_view)
> > + + size - 4096) & 0xfffff000) <<
> > 32;
> > + val |= i915_gem_obj_ggtt_offset_view((obj),
> > ggtt_view) & 0xfffff000; +
>
> Looks like the code can be setting up a fence with a rotated view
> GGTT address which looks wrong? Is this really what is wanted and why?
>
> Regards,
>
> Tvrtko
Well, i915_gem_obj_ggtt_offset() always calls
i915_gem_obj_ggtt_offset_view() with the default normal view type which
I suppose is incorrect right? When a rotated view is passed from
i915_gem_object_pin_to_display_plane(), i915_gem_obj_ggtt_offset_view()
will not be able to find the associated vma.
-Vivek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
