Re: [Intel-gfx] [PATCH v4 4/8] drm/i915/selftests: Add mock selftest for remapped vmas

2018-10-23 Thread Ville Syrjälä
On Tue, Oct 23, 2018 at 08:02:16PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-10-23 17:03:01)
> > From: Ville Syrjälä 
> > 
> > Extend the rotated vma mock selftest to cover remapped vmas as
> > well.
> > 
> > TODO: reindent the loops I guess? Left like this for now to
> > ease review
> > 
> > v2: Include the vma type in the error message (Chris)
> > 
> > Cc: Chris Wilson 
> > Signed-off-by: Ville Syrjälä 
> > ---
> > +static struct scatterlist *
> > +assert_remapped(struct drm_i915_gem_object *obj,
> > +   const struct intel_remapped_info *r, unsigned int n,
> > +   struct scatterlist *sg)
> > +{
> > +   unsigned int x, y;
> > +
> > +   for (y = 0; y < r->plane[n].height; y++) {
> > +   for (x = 0; x < r->plane[n].width; x++) {
> > +   unsigned long src_idx;
> > +   dma_addr_t src;
> > +
> > +   if (!sg) {
> > +   pr_err("Invalid sg table: too short at 
> > plane %d, (%d, %d)!\n",
> > +  n, x, y);
> > +   return ERR_PTR(-EINVAL);
> > +   }
> > +
> > +   src_idx = remapped_index(r, n, x, y);
> > +   src = i915_gem_object_get_dma_address(obj, src_idx);
> > +
> > +   if (sg_dma_len(sg) != PAGE_SIZE) {
> > +   pr_err("Invalid sg.length, found %d, 
> > expected %lu for remapped page (%d, %d) [src index %lu]\n",
> > +  sg_dma_len(sg), PAGE_SIZE,
> > +  x, y, src_idx);
> > +   return ERR_PTR(-EINVAL);
> > +   }
> 
> Meh, seems overly restrictive for a remapped view. But given we are
> iterating by pages and have restricted the construction to be page
> aligned (though didn't we use I915_GTT_PAGE_SIZE in the constructor?),
> seems reasonable for a first stab.

Hrm. I am calling sg_trim() now, so I guess this is actually bonkers.
Did I not actually run the selftests after that change?

> 
> ~o~ it fits the series, but it doesn't validate potential changes.
> 
> Acked-by: Chris Wilson 
> -Chris

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4 4/8] drm/i915/selftests: Add mock selftest for remapped vmas

2018-10-23 Thread Chris Wilson
Quoting Ville Syrjala (2018-10-23 17:03:01)
> From: Ville Syrjälä 
> 
> Extend the rotated vma mock selftest to cover remapped vmas as
> well.
> 
> TODO: reindent the loops I guess? Left like this for now to
> ease review
> 
> v2: Include the vma type in the error message (Chris)
> 
> Cc: Chris Wilson 
> Signed-off-by: Ville Syrjälä 
> ---
> +static struct scatterlist *
> +assert_remapped(struct drm_i915_gem_object *obj,
> +   const struct intel_remapped_info *r, unsigned int n,
> +   struct scatterlist *sg)
> +{
> +   unsigned int x, y;
> +
> +   for (y = 0; y < r->plane[n].height; y++) {
> +   for (x = 0; x < r->plane[n].width; x++) {
> +   unsigned long src_idx;
> +   dma_addr_t src;
> +
> +   if (!sg) {
> +   pr_err("Invalid sg table: too short at plane 
> %d, (%d, %d)!\n",
> +  n, x, y);
> +   return ERR_PTR(-EINVAL);
> +   }
> +
> +   src_idx = remapped_index(r, n, x, y);
> +   src = i915_gem_object_get_dma_address(obj, src_idx);
> +
> +   if (sg_dma_len(sg) != PAGE_SIZE) {
> +   pr_err("Invalid sg.length, found %d, expected 
> %lu for remapped page (%d, %d) [src index %lu]\n",
> +  sg_dma_len(sg), PAGE_SIZE,
> +  x, y, src_idx);
> +   return ERR_PTR(-EINVAL);
> +   }

Meh, seems overly restrictive for a remapped view. But given we are
iterating by pages and have restricted the construction to be page
aligned (though didn't we use I915_GTT_PAGE_SIZE in the constructor?),
seems reasonable for a first stab.

~o~ it fits the series, but it doesn't validate potential changes.

Acked-by: Chris Wilson 
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx