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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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ä <ville.syrjala@linux.intel.com>
> > 
> > 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 <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > +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@chris-wilson.co.uk>
> -Chris

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