On Thu, Sep 12, 2013 at 09:08:17PM +0100, Chris Wilson wrote: > On Thu, Sep 12, 2013 at 10:45:42PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxx > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 18043a2..d0137b6 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -6793,6 +6793,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, > > u32 base, pos; > > bool visible; > > > > + if (!intel_crtc->active) > > + return; > > This is misleading since we do expect to call this function whilst > turning off the crtc. This check makes it appear that such calls might > be wrong. crtc->active is true until the pipe has been fully turned off. > Also the !crtc->enabled following intel_crtc->active makes > ones question their sanity. I actually forgot we had that there. If you recall I'm actually removing it in my cursor visibility fixes. But I didn't realize that we don't actually clear crtc->config to zero for inactive pipes, so I actually would have introduced a bug there. > So I feel this check detracts from readability of the function. I just wanted something minimal for stable. But since we have the ->enabled check there currently I think this patch could just be dropped as ->enabled should match ->active outside modeset operations, and during modeset operations we're covered by Jani's patch to remove the extra cursor calls. But I need to amend my earlier visibility fixes to add either ->active or ->enabled check back. Or I need to make it so that crtc->config gets cleared for disabled pipes. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html