From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Don't enable sprites on a disabled pipe Date: Fri, 13 Sep 2013 10:54:45 +0300 Message-ID: <20130913075445.GJ20128@intel.com> References: <1379015143-13822-1-git-send-email-ville.syrjala@linux.intel.com> <1379015143-13822-4-git-send-email-ville.syrjala@linux.intel.com> <20130912201356.GC14235@nuc-i3427.alporthouse.com> <20130913074015.GH20128@intel.com> <20130913075002.GI20128@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <20130913075002.GI20128@intel.com> Sender: stable-owner@vger.kernel.org To: Chris Wilson , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Sep 13, 2013 at 10:50:02AM +0300, Ville Syrj=E4l=E4 wrote: > On Fri, Sep 13, 2013 at 10:40:16AM +0300, Ville Syrj=E4l=E4 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.int= el.com wrote: > > > > From: Ville Syrj=E4l=E4 > > > >=20 > > > > 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. > > > >=20 > > > > Cc: stable@vger.kernel.org > > > > Signed-off-by: Ville Syrj=E4l=E4 > > > > --- > > > > drivers/gpu/drm/i915/intel_sprite.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > >=20 > > > > 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 =3D crtc_y + crtc_h, > > > > }; > > > > const struct drm_rect clip =3D { > > > > - .x2 =3D crtc->mode.hdisplay, > > > > - .y2 =3D crtc->mode.vdisplay, > > > > + .x2 =3D intel_crtc->active ? crtc->mode.hdisplay : 0, > > > > + .y2 =3D intel_crtc->active ? crtc->mode.vdisplay : 0, > > > > }; > > >=20 > > > 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: > > >=20 > > > if (WARN_ON(!intel_crtc->active)) > > > return -EMONKEY; > >=20 > > I'm actually perfectly happy to let users set up planes on a disabl= ed > > pipe. We allow it for cursors already, so why not all planes? Espec= ially > > as cursors will be just drm_planes in the future. >=20 > Oh and BTW the clipping/scaling stuff works perfectly well when stuff > gets fully clipped. dst region will come out as zero, and visible wil= l > 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. --=20 Ville Syrj=E4l=E4 Intel OTC