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 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.