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

Ville Syrjälä
2013-09-13T07:50:02Z

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@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@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@vger.kernel.org
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@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.

--
Ville Syrjälä
Intel OTC