public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4)
Date: Wed, 23 Sep 2015 11:16:51 +0200	[thread overview]
Message-ID: <20150923091651.GC3383@phenom.ffwll.local> (raw)
In-Reply-To: <55FABEF0.3060507@linux.intel.com>

On Thu, Sep 17, 2015 at 03:24:00PM +0200, Maarten Lankhorst wrote:
> Op 17-09-15 om 15:05 schreef Ville Syrjälä:
> > On Thu, Sep 17, 2015 at 02:38:12PM +0200, Maarten Lankhorst wrote:
> >> Hey,
> >>
> >> It's worh nothing this series may have a soft dependency on Ville's cleanups,
> >> because else pixel_rate == 0 during boot. :(
> >>
> >> This patch breaks when the initial sprite watermarks are set on ironlake, but no sprite is enabled.
> >>
> >> A naive fix is below, without it my laptop won't power the screen because intermediate wm calculation fails. :(
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 41a4dbaf600b..bf874ff97024 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -2457,7 +2457,7 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
> >>  
> >>  		a_wm->enable &= b_wm->enable;
> >>  		a_wm->pri_val = max(a_wm->pri_val, b_wm->pri_val);
> >> -		a_wm->spr_val = max(a_wm->spr_val, b_wm->spr_val);
> >> +		a_wm->spr_val = a->sprites_enabled ? max(a_wm->spr_val, b_wm->spr_val) : 0;
> >>  		a_wm->cur_val = max(a_wm->cur_val, b_wm->cur_val);
> >>  		a_wm->fbc_val = max(a_wm->fbc_val, b_wm->fbc_val);
> >>  	}
> > Hmm. That could work, but feels a bit wrong. Would be nice if we could
> > always keep the watermarks totally consistent with the plane
> > configuration.
> >
> > One thing we could do during init is simply calculate the optimal
> > watermarks for the current config and just blast them in. Then when
> > the first plane update/modeset happens the current state would
> > actually be sane wrt. the current plane config.
> >
> That would work too, as long as we end up with something sane I'm all for it. :-)

Yeah blasting in our own preferred config is probably simpler - adding all
kinds of hw state readout for this seems overkill to me. And adding
special cases all over is imo bad since we're really not any good at
testing all the various corner-cases ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

      reply	other threads:[~2015-09-23  9:13 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-15  2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
2015-09-15  2:19 ` [PATCH 01/14] drm/i915: Drop redundant watermark programming Matt Roper
2015-09-15  2:19 ` [PATCH 02/14] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2) Matt Roper
2015-09-15  2:19 ` [PATCH 03/14] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM (v2) Matt Roper
2015-09-15  2:19 ` [PATCH 04/14] drm/i915: Determine I915_MAX_PLANES from plane enum Matt Roper
2015-09-15  2:19 ` [PATCH 05/14] drm/i915/skl: Simplify wm structures slightly (v2) Matt Roper
2015-09-15  2:19 ` [PATCH 06/14] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v2) Matt Roper
2015-09-17 13:24   ` Maarten Lankhorst
2015-09-15  2:19 ` [PATCH 07/14] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check Matt Roper
2015-09-15  2:19 ` [PATCH 08/14] drm/i915: Drop intel_update_sprite_watermarks Matt Roper
2015-09-15  2:19 ` [PATCH 09/14] drm/i915: Refactor ilk_update_wm (v3) Matt Roper
2015-09-15  2:19 ` [PATCH 10/14] drm/i915: Calculate pipe watermarks into CRTC state (v3) Matt Roper
2015-09-15  2:19 ` [PATCH 11/14] drm/i915: Calculate ILK-style watermarks during atomic check (v3) Matt Roper
2015-09-15  2:19 ` [PATCH 12/14] drm/i915: Don't set plane visible during HW readout if CRTC is off Matt Roper
2015-09-15  2:19 ` [PATCH 13/14] drm/i915: Calculate watermark configuration during atomic check (v2) Matt Roper
2015-09-15  2:19 ` [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4) Matt Roper
2015-09-17 12:38   ` Maarten Lankhorst
2015-09-17 13:05     ` Ville Syrjälä
2015-09-17 13:24       ` Maarten Lankhorst
2015-09-23  9:16         ` Daniel Vetter [this message]

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=20150923091651.GC3383@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox