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

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 23, 2012 at 07:16:03AM -0700, Jesse Barnes wrote:
> 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.

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch