All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Widawsky <ben@bwidawsk.net>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 04/15] drm/i915: don't touch the PF regs if the power well is down
Date: Fri, 15 Mar 2013 11:41:45 -0700	[thread overview]
Message-ID: <20130315184144.GE17773@bwidawsk.net> (raw)
In-Reply-To: <20130306232832.GF9021@phenom.ffwll.local>

On Thu, Mar 07, 2013 at 12:28:32AM +0100, Daniel Vetter wrote:
> On Wed, Mar 06, 2013 at 08:03:11PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > This solves some "unclaimed register" messages when booting the
> > machine with eDP attached.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bd27336..9a9f6d7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3584,8 +3584,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >  	intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
> >  
> >  	/* Disable PF */
> > -	I915_WRITE(PF_CTL(pipe), 0);
> > -	I915_WRITE(PF_WIN_SZ(pipe), 0);
> > +	if (!intel_power_well_is_down(dev)) {
> > +		I915_WRITE(PF_CTL(pipe), 0);
> > +		I915_WRITE(PF_WIN_SZ(pipe), 0);
> > +	}
> 
> I'd vote for a /* XXX: Once we have proper pfit state tracking implemented
> with hw state read/check support we should switch to only disable the pfit
> when we know it's used */
> 
> The idea is that the power well code here irks me a bit, after all if
> something is on and we want to disable it, it can't be also off due to the
> power well being down ;-)
> -Daniel


Daniel, your second paragraph doesn't make sense to me. I agree with the
first paragraph, but I like that this fixes a real error message in the
meantime. As best I can tell, there is nothing wrong with the code on
the existing d-i-n.

Paulo: P.S. This doesn't help my previous bikeshed about not needing an
IS_HASWELL check in intel_power_well_is_down() ;-).


> 
> >  
> >  	intel_ddi_disable_pipe_clock(intel_crtc);
> >  
> > -- 
> > 1.7.10.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

  reply	other threads:[~2013-03-15 18:41 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-06 23:03 [PATCH 00/15] More "unclaimed register" fixes Paulo Zanoni
2013-03-06 23:03 ` [PATCH 01/15] drm/i915: only disable DDI sound if intel_crtc->eld_vld Paulo Zanoni
2013-03-07  9:31   ` Ville Syrjälä
2013-03-17 20:17     ` Daniel Vetter
2013-03-17 20:23     ` Daniel Vetter
2013-03-20 22:03       ` Paulo Zanoni
2013-03-20 22:24         ` Daniel Vetter
2013-03-22 17:11   ` [PATCH 2/7] " Paulo Zanoni
2013-03-22 18:11     ` Ville Syrjälä
2013-03-06 23:03 ` [PATCH 02/15] drm/i915: disable sound first on intel_disable_ddi Paulo Zanoni
2013-03-15 18:24   ` Ben Widawsky
2013-03-17 20:25     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 03/15] drm/i915: add intel_power_well_is_down Paulo Zanoni
2013-03-06 23:26   ` Daniel Vetter
2013-03-06 23:31     ` Daniel Vetter
2013-03-15 18:31     ` Ben Widawsky
2013-03-20 22:21       ` Paulo Zanoni
2013-03-22 17:14   ` [PATCH 3/7] drm/i915: add intel_using_power_well Paulo Zanoni
2013-04-17  9:04     ` Daniel Vetter
2013-04-17 12:27       ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 04/15] drm/i915: don't touch the PF regs if the power well is down Paulo Zanoni
2013-03-06 23:28   ` Daniel Vetter
2013-03-15 18:41     ` Ben Widawsky [this message]
2013-03-22 17:16   ` [PATCH 4/7] " Paulo Zanoni
2013-04-12 20:23     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 05/15] drm/i915: capture the correct cursor registers on IVB Paulo Zanoni
2013-03-07  9:34   ` Ville Syrjälä
2013-03-15 18:45     ` Ben Widawsky
2013-03-17 20:26       ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 06/15] drm/i915: there's no DSPSIZE register on gen4+ Paulo Zanoni
2013-03-07  9:38   ` Ville Syrjälä
2013-03-15 19:04     ` Ben Widawsky
2013-03-15 19:08       ` Ben Widawsky
2013-03-17 20:29         ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 07/15] drm/i915: there's no DSPADDR register on Haswell Paulo Zanoni
2013-03-15 19:10   ` Ben Widawsky
2013-03-17 20:33     ` Daniel Vetter
2013-03-20 18:01       ` Paulo Zanoni
2013-03-20 21:44         ` Daniel Vetter
2013-03-22 17:19   ` [PATCH 5/7] drm/i915: fix DSPADDR Gen check Paulo Zanoni
2013-03-23 12:33     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 08/15] drm/i915: remove DSPPOS register Paulo Zanoni
2013-03-07  9:43   ` Ville Syrjälä
2013-03-17 20:39     ` Daniel Vetter
2013-03-15 19:13   ` Ben Widawsky
2013-03-22 17:20   ` [PATCH 6/7] drm/i915: there's no DSPPOS register on gen4+ Paulo Zanoni
2013-03-23 12:33     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 09/15] drm/i915: there's no PIPESTAT on Gen5+ Paulo Zanoni
2013-03-06 23:22   ` Daniel Vetter
2013-03-07  9:19     ` Ville Syrjälä
2013-03-22 17:24   ` [PATCH 7/7] drm/i915: there's no PIPESTAT on HAS_PCH_SPLIT platforms Paulo Zanoni
2013-03-23 12:33     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 10/15] drm/i915: check the power well when capturing error state Paulo Zanoni
2013-03-15 19:22   ` Ben Widawsky
2013-03-17 20:46   ` Daniel Vetter
2013-03-17 20:51     ` Daniel Vetter
2013-03-21 22:12     ` Paulo Zanoni
2013-03-22  8:45       ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 11/15] drm/i915: add HAS_POWER_WELL Paulo Zanoni
2013-03-07 17:14   ` Jesse Barnes
2013-03-17 20:49     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 12/15] drm/i915: reorganize intel_lvds_supported Paulo Zanoni
2013-03-15 20:57   ` Ben Widawsky
2013-03-17 20:52     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 13/15] drm/i915: don't save/restore PCH_LVDS on LPT Paulo Zanoni
2013-03-15 21:04   ` Ben Widawsky
2013-03-17 20:54     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 14/15] drm/i915: check the power well on i915_pipe_enabled Paulo Zanoni
2013-03-15 21:06   ` Ben Widawsky
2013-03-17 20:55   ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 15/15] drm/i915: add missing space in error message Paulo Zanoni
2013-03-15 21:10   ` Ben Widawsky
2013-03-17 20:57     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130315184144.GE17773@bwidawsk.net \
    --to=ben@bwidawsk.net \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.