[Intel-gfx] [PATCH 3/3] drm/i915: Don't enable sprites on a disabled pipe

[Chris Wilson]

On Fri, Sep 13, 2013 at 10:54:45AM +0300, Ville Syrjälä wrote:
> On Fri, Sep 13, 2013 at 10:50:02AM +0300, Ville Syrjälä wrote:
> > On Fri, Sep 13, 2013 at 10:40:16AM +0300, Ville Syrjälä wrote:
> > > On Thu, Sep 12, 2013 at 09:13:56PM +0100, Chris Wilson wrote:
> > > > On Thu, Sep 12, 2013 at 10:45:43PM +0300, ville.syrjala at linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > 
> > > > > On HSW enabling a plane on a disabled pipe may hang the entire system.
> > > > > And there's no good reason for doing it ever, so just don't.
> > > > > 
> > > > > Cc: stable at vger.kernel.org
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > > index d9c7a66..4f11eb1 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > > @@ -652,8 +652,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > > >  		.y2 = crtc_y + crtc_h,
> > > > >  	};
> > > > >  	const struct drm_rect clip = {
> > > > > -		.x2 = crtc->mode.hdisplay,
> > > > > -		.y2 = crtc->mode.vdisplay,
> > > > > +		.x2 = intel_crtc->active ? crtc->mode.hdisplay : 0,
> > > > > +		.y2 = intel_crtc->active ? crtc->mode.vdisplay : 0,
> > > > >  	};
> > > > 
> > > > Too much magic that looks like it would have interesting effects later
> > > > in the function. This function should only be called on an active CRTC,
> > > > so declare it so:
> > > > 
> > > >   if (WARN_ON(!intel_crtc->active))
> > > >     return -EMONKEY;
> > > 
> > > I'm actually perfectly happy to let users set up planes on a disabled
> > > pipe. We allow it for cursors already, so why not all planes? Especially
> > > as cursors will be just drm_planes in the future.
> > 
> > Oh and BTW the clipping/scaling stuff works perfectly well when stuff
> > gets fully clipped. dst region will come out as zero, and visible will
> > be false. That already happens when you simply move the plane outside
> > the pipe src dimensions.
> 
> Hmm. Except the primary disable logic is fscked up in that case. Oh
> well, I already knew I have to fix that stuff up.

That's what caught my eye. I wasn't convinced it would always do the
right thing and erroring out earlier was easier than thinking. :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre