From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/6] drm/i915: Error out if we are trying to use VGA with SPLL already in use Date: Sat, 23 Mar 2013 13:27:18 +0100 Message-ID: <20130323122718.GY9021@phenom.ffwll.local> References: <1362670228-19494-1-git-send-email-damien.lespiau@intel.com> <1362670228-19494-3-git-send-email-damien.lespiau@intel.com> <20130308210656.GD31789@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f178.google.com (mail-ea0-f178.google.com [209.85.215.178]) by gabe.freedesktop.org (Postfix) with ESMTP id DFD77E612B for ; Sat, 23 Mar 2013 05:24:30 -0700 (PDT) Received: by mail-ea0-f178.google.com with SMTP id g14so1771872eak.37 for ; Sat, 23 Mar 2013 05:24:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Paulo Zanoni Cc: Ben Widawsky , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Mar 22, 2013 at 06:14:25PM -0300, Paulo Zanoni wrote: > Hi > > 2013/3/8 Ben Widawsky : > > On Thu, Mar 07, 2013 at 03:30:25PM +0000, Damien Lespiau wrote: > >> Our static analysis tool noticed that 'reg' could be used uninitialized if > >> we are trying to get a PLL to drive VGA and SPLL is already in use > >> (plls->spll_refcoung != 0). > > This was also reported by a human a few weeks ago and went to my TODO > list. Thanks for clearing my TODO list :) > > >> > >> In the (error) case above, let's return false to the caller and emit an > >> error. > >> > >> Signed-off-by: Damien Lespiau > >> --- > >> drivers/gpu/drm/i915/intel_ddi.c | 3 +++ > >> 1 files changed, 3 insertions(+), 0 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > >> index a1505e3..0eab200 100644 > >> --- a/drivers/gpu/drm/i915/intel_ddi.c > >> +++ b/drivers/gpu/drm/i915/intel_ddi.c > >> @@ -898,6 +898,9 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) > >> plls->spll_refcount++; > >> reg = SPLL_CTL; > >> intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL; > >> + } else { > >> + DRM_ERROR("SPLL already in use\n"); > >> + return false; > >> } > >> > >> WARN(I915_READ(reg) & SPLL_PLL_ENABLE, > > > > I know very little about this code, but the warning still seems valuable > > to me. > > The WARN will still be hit in case no one is using SPLL but it's > enabled. It's a different problem. > > The code added by Damien is correct, but it should never happen with > the current code because only the analog encoder tries to use SPLL and > there's only 1 analog encoder. But we can always introduce bugs to > trigger these errors :) > > Reviewed-by: Paulo Zanoni Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch