From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: yetundex.adebisi@intel.com, isg-gms@eclists.intel.com,
Chi Ding <chix.ding@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/vlv: Fix off-by-1 error in calculating num_levels.
Date: Tue, 19 Jul 2016 18:25:42 +0300 [thread overview]
Message-ID: <20160719152542.GN4329@intel.com> (raw)
In-Reply-To: <d73f9def-caec-25ca-16cb-226549467e93@linux.intel.com>
On Tue, Jul 19, 2016 at 05:14:23PM +0200, Maarten Lankhorst wrote:
> num_levels should be level+1, not level, else num_levels - 1 becomes
> negative. This resulted in bogus watermarks being written to the first
> 255 levels like below:
>
> [drm] Setting FIFO watermarks - C: plane=0, cursor=0, sprite0=0, sprite1=0, SR: plane=0, cursor=0 level=255 cxsr=0
> [drm:chv_set_memory_dvfs [i915]] *ERROR* timed out waiting for Punit DDR DVFS request
> [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe C FIFO underrun
> [drm:chv_set_memory_dvfs [i915]] *ERROR* timed out waiting for Punit DDR DVFS request
>
> Testcase: kms_atomic_transition
> Fixes: 262cd2e154c2 ("drm/i915: CHV DDR DVFS support and another watermark rewrite")
> Cc: stable@vger.kernel.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> Urgent fix for watermark support. This is definitely a pre-requisite for this series.
> With this I've noticed that patch "[RFC 3/8] drm/i915/vlv: Move fifo_size from
> intel_plane_wm_parameters to vlv_wm_state" introduces a regression with invalid FIFO split.
>
> I need to find out what's going wrong in that patch before this series can be applied.
>
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 376c60b98515..8defdcc54529 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1148,7 +1148,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
> }
> }
>
> - wm_state->num_levels = level;
> + wm_state->num_levels = level + 1;
Nope. The loop above breaks when the current level is bad, hence level-1
is actually the higher usable level.
>
> if (!wm_state->cxsr)
> continue;
> --
> 2.7.4
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-07-19 15:25 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-23 12:36 [RFC 0/8] Add two-stage watermark programming for VLV/CHV (v5) Chi Ding
2016-06-23 12:36 ` [RFC 1/8] drm/i915: Remove unused parameters from intel_plane_wm_parameters Chi Ding
2016-06-23 12:36 ` [RFC 2/8] drm/i915: Rename skl_wm_plane_id to wm_plane_id Chi Ding
2016-06-23 12:36 ` [RFC 3/8] drm/i915/vlv: Move fifo_size from intel_plane_wm_parameters to vlv_wm_state Chi Ding
2016-06-23 12:36 ` [RFC 4/8] drm/i915/vlv: return EINVAL when computed watermark exceeds system limitation Chi Ding
2016-06-23 12:36 ` [RFC 5/8] drm/i915/vlv: Change to use intel_crtc_state instead of base CRTC object Chi Ding
2016-06-23 12:36 ` [RFC 6/8] drm/i915/vlv: Add optimal field in intel_crtc_wm_state Chi Ding
2016-06-23 12:36 ` [RFC 7/8] drm/i915/vlv: Move active watermarks into intel_crtc->wm.active.vlv Chi Ding
2016-06-23 12:36 ` [RFC 8/8] drm/i915/vlv: Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark Chi Ding
2016-06-28 10:44 ` Maarten Lankhorst
2016-06-24 10:11 ` ✗ Ro.CI.BAT: failure for Add two-stage watermark programming for VLV/CHV (v5) Patchwork
2016-07-19 15:14 ` [PATCH] drm/i915/vlv: Fix off-by-1 error in calculating num_levels Maarten Lankhorst
2016-07-19 15:25 ` Ville Syrjälä [this message]
2016-07-19 15:50 ` Chris Wilson
2016-07-19 16:21 ` Ville Syrjälä
2016-07-25 11:32 ` Maarten Lankhorst
2016-07-25 11:51 ` Ville Syrjälä
2016-07-25 12:46 ` Maarten Lankhorst
2016-07-25 13:42 ` Ville Syrjälä
2016-07-20 9:16 ` Maarten Lankhorst
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=20160719152542.GN4329@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=chix.ding@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=isg-gms@eclists.intel.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=yetundex.adebisi@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