Re: [PATCH] drm/i915: Don't try to use SPR_SCALE when we don't have a sprite scaler

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 22 Oct 2012 18:19:27 +0100
Damien Lespiau <damien.lespiau@gmail.com> wrote:

> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> Haswell does not have a scaler in the sprite pipeline anymore, so let's
> ensure:
>   1/ We bail out of update_plate() when someone is trying to ask to
>      display a scaled framebuffer,
>   2/ We never write to the nonexistent SPR_SCALE register
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  1 +
>  drivers/gpu/drm/i915/intel_sprite.c | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0942fb4..97c714d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -221,6 +221,7 @@ struct intel_plane {
>  	struct drm_plane base;
>  	enum pipe pipe;
>  	struct drm_i915_gem_object *obj;
> +	bool can_scale;
>  	int max_downscale;
>  	u32 lut_r[1024], lut_g[1024], lut_b[1024];
>  	void (*update_plane)(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index f2ca943..ccfaa6f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -136,7 +136,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  		I915_WRITE(SPRLINOFF(pipe), offset);
>  	}
>  	I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
> -	I915_WRITE(SPRSCALE(pipe), sprscale);
> +	if (intel_plane->can_scale)
> +		I915_WRITE(SPRSCALE(pipe), sprscale);
>  	I915_WRITE(SPRCTL(pipe), sprctl);
>  	I915_MODIFY_DISPBASE(SPRSURF(pipe), obj->gtt_offset);
>  	POSTING_READ(SPRSURF(pipe));
> @@ -474,6 +475,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		goto out;
>  
>  	/*
> +	 * We may not have a scaler, eg. HSW does not have it any more
> +	 */
> +	if (!intel_plane->can_scale && (crtc_w != src_w || crtc_h != src_h))
> +		return -EINVAL;
> +
> +	/*
>  	 * We can take a larger source and scale it down, but
>  	 * only so much...  16x is the max on SNB.
>  	 */
> @@ -666,6 +673,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
>  	switch (INTEL_INFO(dev)->gen) {
>  	case 5:
>  	case 6:
> +		intel_plane->can_scale = true;
>  		intel_plane->max_downscale = 16;
>  		intel_plane->update_plane = ilk_update_plane;
>  		intel_plane->disable_plane = ilk_disable_plane;
> @@ -682,6 +690,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
>  		break;
>  
>  	case 7:
> +		if (IS_HASWELL(dev))
> +			intel_plane->can_scale = false;
> +		else
> +			intel_plane->can_scale = true;
>  		intel_plane->max_downscale = 2;
>  		intel_plane->update_plane = ivb_update_plane;
>  		intel_plane->disable_plane = ivb_disable_plane;

Yeah, looks fine.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center