On pe, 2017-02-17 at 15:10 +0000, Chris Wilson wrote:
> The object already stores (computed on the fly) the index to dma address
> so use it instead of reallocating a large temporary array every time we
> bind a rotated framebuffer.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.a...@gmail.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>

<SNIP>

> +rotate_pages(struct drm_i915_gem_object *obj,
> +          const struct intel_rotation_plane_info *p,
>            struct sg_table *st, struct scatterlist *sg)
>  {
>       unsigned int column, row;
> -     unsigned int src_idx;
>  
> -     for (column = 0; column < width; column++) {
> -             src_idx = stride * (height - 1) + column;
> -             for (row = 0; row < height; row++) {
> -                     st->nents++;
> +     for (column = 0; column < p->width; column++) {
> +             unsigned long src_idx =
> +                     p->stride * (p->height - 1) + column + p->offset;
> +             for (row = 0; row < p->height; row++) {
> +                     struct scatterlist *src;
> +                     unsigned int n;
> +
> +                     src = i915_gem_object_get_sg(obj, src_idx, &n);

i915_gem_object_get_sg has variable names obj, n, *offset, so I'd be
little concerned of sidetracking reader. Rename n into offset?

> +                     src_idx -= p->stride;
> +
>                       /* We don't need the pages, but need to initialize
>                        * the entries so the sg list can be happily traversed.
>                        * The only thing we need are DMA addresses.
>                        */
>                       sg_set_page(sg, NULL, PAGE_SIZE, 0);
> -                     sg_dma_address(sg) = in[offset + src_idx];
> +                     sg_dma_address(sg) = sg_dma_address(src) + n*PAGE_SIZE;
>                       sg_dma_len(sg) = PAGE_SIZE;
> -                     sg = sg_next(sg);
> -                     src_idx -= stride;

I'm not sure why moving this line, might as well hoist all these to the
for() line.

> +                     sg = __sg_next(sg);
> +
> +                     st->nents++;
>               }
>       }
>  
> @@ -3074,62 +3079,30 @@ static noinline struct sg_table *
>  intel_rotate_pages(struct intel_rotation_info *rot_info,
>                  struct drm_i915_gem_object *obj)
>  {
> -     const unsigned long n_pages = obj->base.size / PAGE_SIZE;
> -     unsigned int size = intel_rotation_info_size(rot_info);
> -     struct sgt_iter sgt_iter;
> -     dma_addr_t dma_addr;
> -     unsigned long i;
> -     dma_addr_t *page_addr_list;
> -     struct sg_table *st;
> +     const unsigned int size = intel_rotation_info_size(rot_info);

This is only used once, just inline it.

Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>

Could use an A-b from Tvrtko.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to