Re: [PATCH 2/2] drm/i915: Remove temporary allocation of dma addresses when rotating

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Tue, 2017-11-14 at 19:11 +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 <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

<SNIP>

> @@ -3665,27 +3665,32 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
>  }
>  
>  static struct scatterlist *
> -rotate_pages(const dma_addr_t *in, unsigned int offset,
> -	     unsigned int width, unsigned int height,
> -	     unsigned int stride,
> +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;

Wouldn't you rather do offset + ...? It's the most dominating factor.

> +		for (row = 0; row < p->height; row++) {
> +			struct scatterlist *src;
> +			unsigned int n;
> +
> +			src = i915_gem_object_get_sg(obj, src_idx, &n);

It's bit confusing that the arguments are "n" and "offset" and "n" is
passed as "offset", so maybe s/n/sg_offset/. With that sorted out;

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

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