On 09/15/2015 03:09 AM, Vivek Kasireddy wrote:
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 use case itself has been exercised in general and it does work.
However I suspect you are using SET_TILING on the object to set it to Y
tiled? That would explain i915_gem_object_get_fence trying to enable
fencing.
This indeed looks like an omission in test coverage, needs a test case
in kms_addfb_basic/addfb25 test group.
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.
I think it is correct since for setting up fences we do want the normal
view. What would you do that in userspace with fenced rotated view and
would it even work? And it wouldn't even be predictable which type of
view you got for your mmap since it would depend on view instantiation
ordering, no?
I think what might work is that we skip fence creation on rotated view
pinning, and instead do it when mmap is requested, instantiating a
normal view at that time if not present.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
