From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754558AbcEERGG (ORCPT ); Thu, 5 May 2016 13:06:06 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36451 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750865AbcEERGE (ORCPT ); Thu, 5 May 2016 13:06:04 -0400 Date: Thu, 5 May 2016 19:06:02 +0200 From: Daniel Vetter To: Robin Murphy Cc: airlied@linux.ie, liviu.dudau@arm.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/2] drm: hdlcd: Suspend/resume only active crtcs Message-ID: <20160505170601.GP1286@phenom.ffwll.local> Mail-Followup-To: Robin Murphy , airlied@linux.ie, liviu.dudau@arm.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <759fe8d3dde95091a9df83018051cba00f494e1e.1462464611.git.robin.murphy@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <759fe8d3dde95091a9df83018051cba00f494e1e.1462464611.git.robin.murphy@arm.com> X-Operating-System: Linux phenom 4.6.0-rc5+ User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 05, 2016 at 05:13:38PM +0100, Robin Murphy wrote: > The current PM ops simply unconditionally enable/disable the HDLCD, > which proves problematic when there is no display plugged in - since > without a crtc the hardware itself is still in an uninitialised state, > coming out of suspend results in it being enabled without a valid > framebuffer address, which typically results in it trying to scan out > from bus address 0 and flooding the system with error interrupts. > > Fix this by checking the crtc state on resume, and only enabling the > hardware if it's actually supposed to be. For the sake of consistency, > do the same on the suspend path as well, although there it's merely a > case of skipping unnecessary work. > > CC: Liviu Dudau > Signed-off-by: Robin Murphy > --- > drivers/gpu/drm/arm/hdlcd_crtc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c > index fef1b04c2aab..bf6ff5e48adc 100644 > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c > @@ -296,12 +296,14 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device *drm) > > void hdlcd_crtc_suspend(struct drm_crtc *crtc) > { > - hdlcd_crtc_disable(crtc); > + if (crtc->state->active) > + hdlcd_crtc_disable(crtc); > } > > void hdlcd_crtc_resume(struct drm_crtc *crtc) > { > - hdlcd_crtc_enable(crtc); > + if (crtc->state->active) > + hdlcd_crtc_enable(crtc); > } If you use the atomic helpers to suspend/resume your entire display pipeline these callbacks shouldn't even be needed at all. Tried just removing them? -Daniel > > int hdlcd_setup_crtc(struct drm_device *drm) > -- > 2.8.1.dirty > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch