[Intel-gfx] [PATCH] drm/i915: Avoid pointer arithmetic in calculating plane surface offset

[Tvrtko Ursulin]

On 30/10/15 11:26, Mika Kuoppala wrote:
> VMA offsets are 64 bits. Plane surface offsets are in ggtt and
> the hardware register to set this is thus 32 bits. Be explicit
> about these and convert carefully to from vma to final size.
>
> This will make sparse happy by not creating 32bit pointers out
> of 64bit vma offsets.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 16 +++++++++-------
>   drivers/gpu/drm/i915/intel_drv.h     |  6 +++---
>   drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
>   3 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3f1b545..37e83f9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2926,13 +2926,13 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
>   	}
>   }
>
> -unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
> -				     struct drm_i915_gem_object *obj,
> -				     unsigned int plane)
> +u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
> +			   struct drm_i915_gem_object *obj,
> +			   unsigned int plane)
>   {
>   	const struct i915_ggtt_view *view = &i915_ggtt_view_normal;
>   	struct i915_vma *vma;
> -	unsigned char *offset;
> +	u64 offset;
>
>   	if (intel_rotation_90_or_270(intel_plane->base.state->rotation))
>   		view = &i915_ggtt_view_rotated;
> @@ -2942,14 +2942,16 @@ unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
>   		view->type))
>   		return -1;
>
> -	offset = (unsigned char *)vma->node.start;
> +	offset = vma->node.start;
>
>   	if (plane == 1) {
>   		offset += vma->ggtt_view.rotation_info.uv_start_page *
>   			  PAGE_SIZE;
>   	}
>
> -	return (unsigned long)offset;
> +	WARN_ON(upper_32_bits(offset));
> +
> +	return lower_32_bits(offset);
>   }
>
>   static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
> @@ -3075,7 +3077,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>   	u32 tile_height, plane_offset, plane_size;
>   	unsigned int rotation;
>   	int x_offset, y_offset;
> -	unsigned long surf_addr;
> +	u32 surf_addr;
>   	struct intel_crtc_state *crtc_state = intel_crtc->config;
>   	struct intel_plane_state *plane_state;
>   	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1a3bbdc..bc60385 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1196,9 +1196,9 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
>   int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
>   int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
>
> -unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
> -				     struct drm_i915_gem_object *obj,
> -				     unsigned int plane);
> +u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
> +			   struct drm_i915_gem_object *obj,
> +			   unsigned int plane);
>
>   u32 skl_plane_ctl_format(uint32_t pixel_format);
>   u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 4276c13..a2c15f8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -194,7 +194,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>   	u32 plane_ctl, stride_div, stride;
>   	const struct drm_intel_sprite_colorkey *key =
>   		&to_intel_plane_state(drm_plane->state)->ckey;
> -	unsigned long surf_addr;
> +	u32 surf_addr;
>   	u32 tile_height, plane_offset, plane_size;
>   	unsigned int rotation;
>   	int x_offset, y_offset;
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko