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

[Paulo Zanoni <przanoni@gmail.com>]

Hi

2012/10/23 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.

After testing this, I see we also need to patch ivb_disable_plane to
not write SPRSCALE. Everything else looks correct, so no problem to
include this in a separate patch.


> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni