From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH 3/5] drm/i915/vlv: warn on bad VLV PLL
 divider values
Date: Fri, 27 Sep 2013 22:11:33 +0200
Message-ID: <20130927201133.GJ26592@phenom.ffwll.local>
References: <1380311846-1581-1-git-send-email-jbarnes@virtuousgeek.org>
	<1380311846-1581-3-git-send-email-jbarnes@virtuousgeek.org>
	<20130927200524.GC744@nuc-i3427.alporthouse.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from mail-ee0-f42.google.com (mail-ee0-f42.google.com [74.125.83.42])
	by gabe.freedesktop.org (Postfix) with ESMTP id 1E385E5C8A
	for <intel-gfx@lists.freedesktop.org>;
	Fri, 27 Sep 2013 13:11:16 -0700 (PDT)
Received: by mail-ee0-f42.google.com with SMTP id b45so1444626eek.15
	for <intel-gfx@lists.freedesktop.org>;
	Fri, 27 Sep 2013 13:11:16 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <20130927200524.GC744@nuc-i3427.alporthouse.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=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: Chris Wilson <chris@chris-wilson.co.uk>, Jesse Barnes <jbarnes@virtuousgeek.org>, intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Fri, Sep 27, 2013 at 09:05:24PM +0100, Chris Wilson wrote:
> On Fri, Sep 27, 2013 at 12:57:24PM -0700, Jesse Barnes wrote:
> > This avoids a divide by zero and warns appropriately on this serious bug.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 8da1c96..9a83236 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5109,6 +5109,11 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
> >  	clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
> >  	clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
> >  
> > +	if (!clock.n || !(clock.p1 * clock.p2)) {
> 
> if ((clock.n & clock.p1 & clock.p2) == 0) {
> 
> or just
> 
> if (!clock.n || !clock.p1 || !clock.p2)
> 
> > +		WARN(1, "bad divider values on pipe %d\n", crtc->pipe);
> > +		return;
> > +	}
> > +
> 
> So I think you want
> 
> if (WARN_ONCE(clock.n == 0 || clock.p1 == 0 || clock.p2 == 0,
>               "bad divider values on pipe %d (m1=%d, m2=%d, p1=%d, p2=%d, n=%d)\n",
> 	      crtc->pipe, clock.m1, clock.m2, clock.p1, clock.p2, clock.n))
>    return;

So the bios leaves a broken setup behind or is our hw state readout not
quite careful enough?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch