public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
@ 2016-03-03 22:18 Matt Roper
  2016-03-04 10:45 ` Ville Syrjälä
  2016-03-07  9:13 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 7+ messages in thread
From: Matt Roper @ 2016-03-03 22:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

As of commit d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
it's now possible to have non-zero values for watermark levels that are
disabled (e.g., in the higher LP levels that can only be used when the sprite
is disabled).  Stop WARN()'ing when we see these non-zero sprite values in LP2
or LP3.

Fixes the SNB warning:

   WARNING: CPU: 1 PID: 25405 at /home/mattrope/work/kernel/gms/drivers/gpu/drm/i915/intel_pm.c:2580 ilk_program_watermarks+0x7b2/0x9d0 [i915]()
   WARN_ON(wm_lp != 1)
   Modules linked in: i915 drm_kms_helper drm bluetooth fuse iTCO_wdt iTCO_vendor_support syscopyarea sysfillrect sysimgblt fb_sys_fops tpm_tis mei_me e1000e snd_hda_codec_hdmi pcspkr tpm mei i2c_i801 lpc_ich snd_hda_codec snd_hda_core [last unloaded: drm]
   CPU: 1 PID: 25405 Comm: kms_universal_p Tainted: G     U  W       4.5.0-rc6apollolake+ #462
   Hardware name:                  /DH67GD, BIOS BLH6710H.86A.0160.2012.1204.1156 12/04/2012
    0000000000000000 ffff88009d42b918 ffffffff8143cfab ffff88009d42b960
    ffffffffa0363580 ffff88009d42b950 ffffffff81082746 ffff8800b9a24928
    ffff88009d42ba00 ffff88009d4a0000 0000000000000000 ffff88009d42ba6c
   Call Trace:
    [<ffffffff8143cfab>] dump_stack+0x4d/0x72
    [<ffffffff81082746>] warn_slowpath_common+0x86/0xc0
    [<ffffffff810827cc>] warn_slowpath_fmt+0x4c/0x50
    [<ffffffffa0292862>] ilk_program_watermarks+0x7b2/0x9d0 [i915]
    [<ffffffffa0292cb7>] ilk_initial_watermarks+0x107/0x120 [i915]
    [<ffffffffa02feffa>] intel_pre_plane_update+0x12a/0x190 [i915]
    [<ffffffffa02ffb36>] intel_atomic_commit+0x546/0xd50 [i915]
    [<ffffffffa012c9e7>] drm_atomic_commit+0x37/0x60 [drm]
    [<ffffffffa0217361>] drm_atomic_helper_disable_plane+0xb1/0xf0 [drm_kms_helper]
    [<ffffffffa011cdb4>] __setplane_internal+0x184/0x280 [drm]
    [<ffffffffa012b57a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
    [<ffffffffa012010f>] drm_mode_setplane+0x13f/0x1c0 [drm]
    [<ffffffffa0111b52>] drm_ioctl+0x142/0x590 [drm]
    [<ffffffffa011ffd0>] ? drm_plane_check_pixel_format+0x50/0x50 [drm]
    [<ffffffff811f2744>] ? mntput+0x24/0x40
    [<ffffffff811d28d4>] ? __fput+0x194/0x200
    [<ffffffffa012dec3>] drm_compat_ioctl+0x33/0x40 [drm]
    [<ffffffffa029e1c2>] i915_compat_ioctl+0x32/0x40 [i915]
    [<ffffffff81228d72>] compat_SyS_ioctl+0xc2/0x330
    [<ffffffff810021d5>] ? exit_to_usermode_loop+0x95/0xb0
    [<ffffffff81002d2e>] do_fast_syscall_32+0x9e/0x210
    [<ffffffff8197faf2>] entry_SYSENTER_compat+0x52/0x70

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Testcase: kms_universal_plane
Fixes: d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
The comment above this block of code indicates that we need to turn on
WM1S_LP_EN, even if the LP level is disabled, to avoid underruns.  It's not
clear to me from the bspec why this is, so I'm trusting that the comment
is still accurate, even though we wind up with non-zero values more often
now than we previously did.

 drivers/gpu/drm/i915/intel_pm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 823437f..9698360 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2576,10 +2576,9 @@ static void ilk_compute_wm_results(struct drm_device *dev,
 		 * Always set WM1S_LP_EN when spr_val != 0, even if the
 		 * level is disabled. Doing otherwise could cause underruns.
 		 */
-		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
-			WARN_ON(wm_lp != 1);
+		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val)
 			results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
-		} else
+		else
 			results->wm_lp_spr[wm_lp - 1] = r->spr_val;
 	}
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
  2016-03-03 22:18 [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels Matt Roper
@ 2016-03-04 10:45 ` Ville Syrjälä
  2016-03-04 17:37   ` Matt Roper
  2016-03-07  9:13 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2016-03-04 10:45 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx, Paulo Zanoni

On Thu, Mar 03, 2016 at 02:18:09PM -0800, Matt Roper wrote:
> As of commit d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> it's now possible to have non-zero values for watermark levels that are
> disabled (e.g., in the higher LP levels that can only be used when the sprite
> is disabled).  Stop WARN()'ing when we see these non-zero sprite values in LP2
> or LP3.
> 
> Fixes the SNB warning:
> 
>    WARNING: CPU: 1 PID: 25405 at /home/mattrope/work/kernel/gms/drivers/gpu/drm/i915/intel_pm.c:2580 ilk_program_watermarks+0x7b2/0x9d0 [i915]()
>    WARN_ON(wm_lp != 1)
>    Modules linked in: i915 drm_kms_helper drm bluetooth fuse iTCO_wdt iTCO_vendor_support syscopyarea sysfillrect sysimgblt fb_sys_fops tpm_tis mei_me e1000e snd_hda_codec_hdmi pcspkr tpm mei i2c_i801 lpc_ich snd_hda_codec snd_hda_core [last unloaded: drm]
>    CPU: 1 PID: 25405 Comm: kms_universal_p Tainted: G     U  W       4.5.0-rc6apollolake+ #462
>    Hardware name:                  /DH67GD, BIOS BLH6710H.86A.0160.2012.1204.1156 12/04/2012
>     0000000000000000 ffff88009d42b918 ffffffff8143cfab ffff88009d42b960
>     ffffffffa0363580 ffff88009d42b950 ffffffff81082746 ffff8800b9a24928
>     ffff88009d42ba00 ffff88009d4a0000 0000000000000000 ffff88009d42ba6c
>    Call Trace:
>     [<ffffffff8143cfab>] dump_stack+0x4d/0x72
>     [<ffffffff81082746>] warn_slowpath_common+0x86/0xc0
>     [<ffffffff810827cc>] warn_slowpath_fmt+0x4c/0x50
>     [<ffffffffa0292862>] ilk_program_watermarks+0x7b2/0x9d0 [i915]
>     [<ffffffffa0292cb7>] ilk_initial_watermarks+0x107/0x120 [i915]
>     [<ffffffffa02feffa>] intel_pre_plane_update+0x12a/0x190 [i915]
>     [<ffffffffa02ffb36>] intel_atomic_commit+0x546/0xd50 [i915]
>     [<ffffffffa012c9e7>] drm_atomic_commit+0x37/0x60 [drm]
>     [<ffffffffa0217361>] drm_atomic_helper_disable_plane+0xb1/0xf0 [drm_kms_helper]
>     [<ffffffffa011cdb4>] __setplane_internal+0x184/0x280 [drm]
>     [<ffffffffa012b57a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
>     [<ffffffffa012010f>] drm_mode_setplane+0x13f/0x1c0 [drm]
>     [<ffffffffa0111b52>] drm_ioctl+0x142/0x590 [drm]
>     [<ffffffffa011ffd0>] ? drm_plane_check_pixel_format+0x50/0x50 [drm]
>     [<ffffffff811f2744>] ? mntput+0x24/0x40
>     [<ffffffff811d28d4>] ? __fput+0x194/0x200
>     [<ffffffffa012dec3>] drm_compat_ioctl+0x33/0x40 [drm]
>     [<ffffffffa029e1c2>] i915_compat_ioctl+0x32/0x40 [i915]
>     [<ffffffff81228d72>] compat_SyS_ioctl+0xc2/0x330
>     [<ffffffff810021d5>] ? exit_to_usermode_loop+0x95/0xb0
>     [<ffffffff81002d2e>] do_fast_syscall_32+0x9e/0x210
>     [<ffffffff8197faf2>] entry_SYSENTER_compat+0x52/0x70
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Testcase: kms_universal_plane
> Fixes: d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> The comment above this block of code indicates that we need to turn on
> WM1S_LP_EN, even if the LP level is disabled, to avoid underruns.  It's not
> clear to me from the bspec why this is, so I'm trusting that the comment
> is still accurate, even though we wind up with non-zero values more often
> now than we previously did.
> 
>  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 823437f..9698360 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2576,10 +2576,9 @@ static void ilk_compute_wm_results(struct drm_device *dev,
>  		 * Always set WM1S_LP_EN when spr_val != 0, even if the
>  		 * level is disabled. Doing otherwise could cause underruns.
>  		 */
> -		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
> -			WARN_ON(wm_lp != 1);

NAK

There are no LP2+ sprite watermarks on ilk/snb, so if something is
computing it, then we have clearly a bug somewhere else.

> +		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val)
>  			results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
> -		} else
> +		else
>  			results->wm_lp_spr[wm_lp - 1] = r->spr_val;
>  	}
>  
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
  2016-03-04 10:45 ` Ville Syrjälä
@ 2016-03-04 17:37   ` Matt Roper
  2016-03-04 17:47     ` Ville Syrjälä
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Roper @ 2016-03-04 17:37 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Paulo Zanoni

On Fri, Mar 04, 2016 at 12:45:38PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 03, 2016 at 02:18:09PM -0800, Matt Roper wrote:
> > As of commit d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> > it's now possible to have non-zero values for watermark levels that are
> > disabled (e.g., in the higher LP levels that can only be used when the sprite
> > is disabled).  Stop WARN()'ing when we see these non-zero sprite values in LP2
> > or LP3.
> > 
> > Fixes the SNB warning:
> > 
> >    WARNING: CPU: 1 PID: 25405 at /home/mattrope/work/kernel/gms/drivers/gpu/drm/i915/intel_pm.c:2580 ilk_program_watermarks+0x7b2/0x9d0 [i915]()
> >    WARN_ON(wm_lp != 1)
> >    Modules linked in: i915 drm_kms_helper drm bluetooth fuse iTCO_wdt iTCO_vendor_support syscopyarea sysfillrect sysimgblt fb_sys_fops tpm_tis mei_me e1000e snd_hda_codec_hdmi pcspkr tpm mei i2c_i801 lpc_ich snd_hda_codec snd_hda_core [last unloaded: drm]
> >    CPU: 1 PID: 25405 Comm: kms_universal_p Tainted: G     U  W       4.5.0-rc6apollolake+ #462
> >    Hardware name:                  /DH67GD, BIOS BLH6710H.86A.0160.2012.1204.1156 12/04/2012
> >     0000000000000000 ffff88009d42b918 ffffffff8143cfab ffff88009d42b960
> >     ffffffffa0363580 ffff88009d42b950 ffffffff81082746 ffff8800b9a24928
> >     ffff88009d42ba00 ffff88009d4a0000 0000000000000000 ffff88009d42ba6c
> >    Call Trace:
> >     [<ffffffff8143cfab>] dump_stack+0x4d/0x72
> >     [<ffffffff81082746>] warn_slowpath_common+0x86/0xc0
> >     [<ffffffff810827cc>] warn_slowpath_fmt+0x4c/0x50
> >     [<ffffffffa0292862>] ilk_program_watermarks+0x7b2/0x9d0 [i915]
> >     [<ffffffffa0292cb7>] ilk_initial_watermarks+0x107/0x120 [i915]
> >     [<ffffffffa02feffa>] intel_pre_plane_update+0x12a/0x190 [i915]
> >     [<ffffffffa02ffb36>] intel_atomic_commit+0x546/0xd50 [i915]
> >     [<ffffffffa012c9e7>] drm_atomic_commit+0x37/0x60 [drm]
> >     [<ffffffffa0217361>] drm_atomic_helper_disable_plane+0xb1/0xf0 [drm_kms_helper]
> >     [<ffffffffa011cdb4>] __setplane_internal+0x184/0x280 [drm]
> >     [<ffffffffa012b57a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
> >     [<ffffffffa012010f>] drm_mode_setplane+0x13f/0x1c0 [drm]
> >     [<ffffffffa0111b52>] drm_ioctl+0x142/0x590 [drm]
> >     [<ffffffffa011ffd0>] ? drm_plane_check_pixel_format+0x50/0x50 [drm]
> >     [<ffffffff811f2744>] ? mntput+0x24/0x40
> >     [<ffffffff811d28d4>] ? __fput+0x194/0x200
> >     [<ffffffffa012dec3>] drm_compat_ioctl+0x33/0x40 [drm]
> >     [<ffffffffa029e1c2>] i915_compat_ioctl+0x32/0x40 [i915]
> >     [<ffffffff81228d72>] compat_SyS_ioctl+0xc2/0x330
> >     [<ffffffff810021d5>] ? exit_to_usermode_loop+0x95/0xb0
> >     [<ffffffff81002d2e>] do_fast_syscall_32+0x9e/0x210
> >     [<ffffffff8197faf2>] entry_SYSENTER_compat+0x52/0x70
> > 
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Testcase: kms_universal_plane
> > Fixes: d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > The comment above this block of code indicates that we need to turn on
> > WM1S_LP_EN, even if the LP level is disabled, to avoid underruns.  It's not
> > clear to me from the bspec why this is, so I'm trusting that the comment
> > is still accurate, even though we wind up with non-zero values more often
> > now than we previously did.
> > 
> >  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 823437f..9698360 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2576,10 +2576,9 @@ static void ilk_compute_wm_results(struct drm_device *dev,
> >  		 * Always set WM1S_LP_EN when spr_val != 0, even if the
> >  		 * level is disabled. Doing otherwise could cause underruns.
> >  		 */
> > -		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
> > -			WARN_ON(wm_lp != 1);
> 
> NAK
> 
> There are no LP2+ sprite watermarks on ilk/snb, so if something is
> computing it, then we have clearly a bug somewhere else.

That's the point of this patch...having these values computed is
intentional now, not a bug.  The patch from Maarten/Paaulo computes the
values for *all* levels, even the ones we know are invalid, but marks
them as disabled.

Given that change in behavior, it no longer makes sense to warn on
non-zero values since they're expected now (even though they'll never be
used).  Maybe instead of removing the WARN() I should just add an
"&& r->enable" to the warn condition?


Matt

> 
> > +		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val)
> >  			results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
> > -		} else
> > +		else
> >  			results->wm_lp_spr[wm_lp - 1] = r->spr_val;
> >  	}
> >  
> > -- 
> > 2.1.4
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
  2016-03-04 17:37   ` Matt Roper
@ 2016-03-04 17:47     ` Ville Syrjälä
  2016-03-04 18:09       ` Ville Syrjälä
  2016-03-07 10:52       ` Maarten Lankhorst
  0 siblings, 2 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-03-04 17:47 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx, Paulo Zanoni

On Fri, Mar 04, 2016 at 09:37:40AM -0800, Matt Roper wrote:
> On Fri, Mar 04, 2016 at 12:45:38PM +0200, Ville Syrjälä wrote:
> > On Thu, Mar 03, 2016 at 02:18:09PM -0800, Matt Roper wrote:
> > > As of commit d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> > > it's now possible to have non-zero values for watermark levels that are
> > > disabled (e.g., in the higher LP levels that can only be used when the sprite
> > > is disabled).  Stop WARN()'ing when we see these non-zero sprite values in LP2
> > > or LP3.
> > > 
> > > Fixes the SNB warning:
> > > 
> > >    WARNING: CPU: 1 PID: 25405 at /home/mattrope/work/kernel/gms/drivers/gpu/drm/i915/intel_pm.c:2580 ilk_program_watermarks+0x7b2/0x9d0 [i915]()
> > >    WARN_ON(wm_lp != 1)
> > >    Modules linked in: i915 drm_kms_helper drm bluetooth fuse iTCO_wdt iTCO_vendor_support syscopyarea sysfillrect sysimgblt fb_sys_fops tpm_tis mei_me e1000e snd_hda_codec_hdmi pcspkr tpm mei i2c_i801 lpc_ich snd_hda_codec snd_hda_core [last unloaded: drm]
> > >    CPU: 1 PID: 25405 Comm: kms_universal_p Tainted: G     U  W       4.5.0-rc6apollolake+ #462
> > >    Hardware name:                  /DH67GD, BIOS BLH6710H.86A.0160.2012.1204.1156 12/04/2012
> > >     0000000000000000 ffff88009d42b918 ffffffff8143cfab ffff88009d42b960
> > >     ffffffffa0363580 ffff88009d42b950 ffffffff81082746 ffff8800b9a24928
> > >     ffff88009d42ba00 ffff88009d4a0000 0000000000000000 ffff88009d42ba6c
> > >    Call Trace:
> > >     [<ffffffff8143cfab>] dump_stack+0x4d/0x72
> > >     [<ffffffff81082746>] warn_slowpath_common+0x86/0xc0
> > >     [<ffffffff810827cc>] warn_slowpath_fmt+0x4c/0x50
> > >     [<ffffffffa0292862>] ilk_program_watermarks+0x7b2/0x9d0 [i915]
> > >     [<ffffffffa0292cb7>] ilk_initial_watermarks+0x107/0x120 [i915]
> > >     [<ffffffffa02feffa>] intel_pre_plane_update+0x12a/0x190 [i915]
> > >     [<ffffffffa02ffb36>] intel_atomic_commit+0x546/0xd50 [i915]
> > >     [<ffffffffa012c9e7>] drm_atomic_commit+0x37/0x60 [drm]
> > >     [<ffffffffa0217361>] drm_atomic_helper_disable_plane+0xb1/0xf0 [drm_kms_helper]
> > >     [<ffffffffa011cdb4>] __setplane_internal+0x184/0x280 [drm]
> > >     [<ffffffffa012b57a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
> > >     [<ffffffffa012010f>] drm_mode_setplane+0x13f/0x1c0 [drm]
> > >     [<ffffffffa0111b52>] drm_ioctl+0x142/0x590 [drm]
> > >     [<ffffffffa011ffd0>] ? drm_plane_check_pixel_format+0x50/0x50 [drm]
> > >     [<ffffffff811f2744>] ? mntput+0x24/0x40
> > >     [<ffffffff811d28d4>] ? __fput+0x194/0x200
> > >     [<ffffffffa012dec3>] drm_compat_ioctl+0x33/0x40 [drm]
> > >     [<ffffffffa029e1c2>] i915_compat_ioctl+0x32/0x40 [i915]
> > >     [<ffffffff81228d72>] compat_SyS_ioctl+0xc2/0x330
> > >     [<ffffffff810021d5>] ? exit_to_usermode_loop+0x95/0xb0
> > >     [<ffffffff81002d2e>] do_fast_syscall_32+0x9e/0x210
> > >     [<ffffffff8197faf2>] entry_SYSENTER_compat+0x52/0x70
> > > 
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Testcase: kms_universal_plane
> > > Fixes: d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > > The comment above this block of code indicates that we need to turn on
> > > WM1S_LP_EN, even if the LP level is disabled, to avoid underruns.  It's not
> > > clear to me from the bspec why this is, so I'm trusting that the comment
> > > is still accurate, even though we wind up with non-zero values more often
> > > now than we previously did.
> > > 
> > >  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 823437f..9698360 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -2576,10 +2576,9 @@ static void ilk_compute_wm_results(struct drm_device *dev,
> > >  		 * Always set WM1S_LP_EN when spr_val != 0, even if the
> > >  		 * level is disabled. Doing otherwise could cause underruns.
> > >  		 */
> > > -		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
> > > -			WARN_ON(wm_lp != 1);
> > 
> > NAK
> > 
> > There are no LP2+ sprite watermarks on ilk/snb, so if something is
> > computing it, then we have clearly a bug somewhere else.
> 
> That's the point of this patch...having these values computed is
> intentional now, not a bug.  The patch from Maarten/Paaulo computes the
> values for *all* levels, even the ones we know are invalid, but marks
> them as disabled.

Then I think it should compute it as 0 since sprite LP2+ watermarks simply
don't exist.

> 
> Given that change in behavior, it no longer makes sense to warn on
> non-zero values since they're expected now (even though they'll never be
> used).  Maybe instead of removing the WARN() I should just add an
> "&& r->enable" to the warn condition?
> 
> 
> Matt
> 
> > 
> > > +		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val)
> > >  			results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
> > > -		} else
> > > +		else
> > >  			results->wm_lp_spr[wm_lp - 1] = r->spr_val;
> > >  	}
> > >  
> > > -- 
> > > 2.1.4
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
  2016-03-04 17:47     ` Ville Syrjälä
@ 2016-03-04 18:09       ` Ville Syrjälä
  2016-03-07 10:52       ` Maarten Lankhorst
  1 sibling, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-03-04 18:09 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx, Paulo Zanoni

On Fri, Mar 04, 2016 at 07:47:09PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 04, 2016 at 09:37:40AM -0800, Matt Roper wrote:
> > On Fri, Mar 04, 2016 at 12:45:38PM +0200, Ville Syrjälä wrote:
> > > On Thu, Mar 03, 2016 at 02:18:09PM -0800, Matt Roper wrote:
> > > > As of commit d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> > > > it's now possible to have non-zero values for watermark levels that are
> > > > disabled (e.g., in the higher LP levels that can only be used when the sprite
> > > > is disabled).  Stop WARN()'ing when we see these non-zero sprite values in LP2
> > > > or LP3.
> > > > 
> > > > Fixes the SNB warning:
> > > > 
> > > >    WARNING: CPU: 1 PID: 25405 at /home/mattrope/work/kernel/gms/drivers/gpu/drm/i915/intel_pm.c:2580 ilk_program_watermarks+0x7b2/0x9d0 [i915]()
> > > >    WARN_ON(wm_lp != 1)
> > > >    Modules linked in: i915 drm_kms_helper drm bluetooth fuse iTCO_wdt iTCO_vendor_support syscopyarea sysfillrect sysimgblt fb_sys_fops tpm_tis mei_me e1000e snd_hda_codec_hdmi pcspkr tpm mei i2c_i801 lpc_ich snd_hda_codec snd_hda_core [last unloaded: drm]
> > > >    CPU: 1 PID: 25405 Comm: kms_universal_p Tainted: G     U  W       4.5.0-rc6apollolake+ #462
> > > >    Hardware name:                  /DH67GD, BIOS BLH6710H.86A.0160.2012.1204.1156 12/04/2012
> > > >     0000000000000000 ffff88009d42b918 ffffffff8143cfab ffff88009d42b960
> > > >     ffffffffa0363580 ffff88009d42b950 ffffffff81082746 ffff8800b9a24928
> > > >     ffff88009d42ba00 ffff88009d4a0000 0000000000000000 ffff88009d42ba6c
> > > >    Call Trace:
> > > >     [<ffffffff8143cfab>] dump_stack+0x4d/0x72
> > > >     [<ffffffff81082746>] warn_slowpath_common+0x86/0xc0
> > > >     [<ffffffff810827cc>] warn_slowpath_fmt+0x4c/0x50
> > > >     [<ffffffffa0292862>] ilk_program_watermarks+0x7b2/0x9d0 [i915]
> > > >     [<ffffffffa0292cb7>] ilk_initial_watermarks+0x107/0x120 [i915]
> > > >     [<ffffffffa02feffa>] intel_pre_plane_update+0x12a/0x190 [i915]
> > > >     [<ffffffffa02ffb36>] intel_atomic_commit+0x546/0xd50 [i915]
> > > >     [<ffffffffa012c9e7>] drm_atomic_commit+0x37/0x60 [drm]
> > > >     [<ffffffffa0217361>] drm_atomic_helper_disable_plane+0xb1/0xf0 [drm_kms_helper]
> > > >     [<ffffffffa011cdb4>] __setplane_internal+0x184/0x280 [drm]
> > > >     [<ffffffffa012b57a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
> > > >     [<ffffffffa012010f>] drm_mode_setplane+0x13f/0x1c0 [drm]
> > > >     [<ffffffffa0111b52>] drm_ioctl+0x142/0x590 [drm]
> > > >     [<ffffffffa011ffd0>] ? drm_plane_check_pixel_format+0x50/0x50 [drm]
> > > >     [<ffffffff811f2744>] ? mntput+0x24/0x40
> > > >     [<ffffffff811d28d4>] ? __fput+0x194/0x200
> > > >     [<ffffffffa012dec3>] drm_compat_ioctl+0x33/0x40 [drm]
> > > >     [<ffffffffa029e1c2>] i915_compat_ioctl+0x32/0x40 [i915]
> > > >     [<ffffffff81228d72>] compat_SyS_ioctl+0xc2/0x330
> > > >     [<ffffffff810021d5>] ? exit_to_usermode_loop+0x95/0xb0
> > > >     [<ffffffff81002d2e>] do_fast_syscall_32+0x9e/0x210
> > > >     [<ffffffff8197faf2>] entry_SYSENTER_compat+0x52/0x70
> > > > 
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Testcase: kms_universal_plane
> > > > Fixes: d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
> > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > > ---
> > > > The comment above this block of code indicates that we need to turn on
> > > > WM1S_LP_EN, even if the LP level is disabled, to avoid underruns.  It's not
> > > > clear to me from the bspec why this is, so I'm trusting that the comment
> > > > is still accurate, even though we wind up with non-zero values more often
> > > > now than we previously did.
> > > > 
> > > >  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
> > > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > > index 823437f..9698360 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > @@ -2576,10 +2576,9 @@ static void ilk_compute_wm_results(struct drm_device *dev,
> > > >  		 * Always set WM1S_LP_EN when spr_val != 0, even if the
> > > >  		 * level is disabled. Doing otherwise could cause underruns.
> > > >  		 */
> > > > -		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
> > > > -			WARN_ON(wm_lp != 1);
> > > 
> > > NAK
> > > 
> > > There are no LP2+ sprite watermarks on ilk/snb, so if something is
> > > computing it, then we have clearly a bug somewhere else.
> > 
> > That's the point of this patch...having these values computed is
> > intentional now, not a bug.  The patch from Maarten/Paaulo computes the
> > values for *all* levels, even the ones we know are invalid, but marks
> > them as disabled.
> 
> Then I think it should compute it as 0 since sprite LP2+ watermarks simply
> don't exist.

Actually I think the whole approach of computing all the levels is sort
of wrong. Eg. we would now compute a sprite LP1 watermark for the
case where sprite scaling is enabled. But that means we'll then merge
that bogus watermark with something else for the intermediate watermark,
and thus we may actually get a different result than what we had before
this new scheme was implemented.

Even worse we don't actually limit the computed watermarks anymore so
they might even overflow the bits allotted.

So I think the whole approach needs more thought.

> 
> > 
> > Given that change in behavior, it no longer makes sense to warn on
> > non-zero values since they're expected now (even though they'll never be
> > used).  Maybe instead of removing the WARN() I should just add an
> > "&& r->enable" to the warn condition?
> > 
> > 
> > Matt
> > 
> > > 
> > > > +		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val)
> > > >  			results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
> > > > -		} else
> > > > +		else
> > > >  			results->wm_lp_spr[wm_lp - 1] = r->spr_val;
> > > >  	}
> > > >  
> > > > -- 
> > > > 2.1.4
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel OTC
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > IoTG Platform Enabling & Development
> > Intel Corporation
> > (916) 356-2795
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
  2016-03-03 22:18 [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels Matt Roper
  2016-03-04 10:45 ` Ville Syrjälä
@ 2016-03-07  9:13 ` Patchwork
  1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2016-03-07  9:13 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
URL   : https://patchwork.freedesktop.org/series/4078/
State : failure

== Summary ==

Series 4078v1 drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
http://patchwork.freedesktop.org/api/1.0/series/4078/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
        Subgroup basic-flip-vs-wf_vblank:
                dmesg-warn -> PASS       (hsw-gt2)
                dmesg-warn -> PASS       (hsw-brixbox)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                fail       -> PASS       (bsw-nuc-2)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                dmesg-warn -> PASS       (hsw-brixbox)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (hsw-gt2)
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> DMESG-WARN (snb-dellxps)
                pass       -> DMESG-WARN (hsw-brixbox)

bdw-nuci7        total:183  pass:172  dwarn:0   dfail:0   fail:0   skip:11 
bdw-ultra        total:183  pass:165  dwarn:0   dfail:0   fail:0   skip:18 
bsw-nuc-2        total:183  pass:149  dwarn:0   dfail:0   fail:0   skip:34 
byt-nuc          total:183  pass:152  dwarn:0   dfail:0   fail:0   skip:31 
hsw-brixbox      total:183  pass:163  dwarn:1   dfail:0   fail:0   skip:19 
hsw-gt2          total:183  pass:168  dwarn:1   dfail:0   fail:0   skip:14 
ilk-hp8440p      total:183  pass:124  dwarn:1   dfail:0   fail:0   skip:58 
ivb-t430s        total:183  pass:162  dwarn:0   dfail:0   fail:0   skip:21 
skl-i5k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
skl-i7k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
snb-dellxps      total:183  pass:152  dwarn:2   dfail:0   fail:0   skip:29 

Results at /archive/results/CI_IGT_test/Patchwork_1526/

91a38d589cb775a4486cec599413f581b2e8f437 drm-intel-nightly: 2016y-03m-03d-17h-31m-44s UTC integration manifest
9ee54500455eca9602ddd69f4d85e7f97015f2bb drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels
  2016-03-04 17:47     ` Ville Syrjälä
  2016-03-04 18:09       ` Ville Syrjälä
@ 2016-03-07 10:52       ` Maarten Lankhorst
  1 sibling, 0 replies; 7+ messages in thread
From: Maarten Lankhorst @ 2016-03-07 10:52 UTC (permalink / raw)
  To: Ville Syrjälä, Matt Roper; +Cc: intel-gfx, Paulo Zanoni

Op 04-03-16 om 18:47 schreef Ville Syrjälä:
> On Fri, Mar 04, 2016 at 09:37:40AM -0800, Matt Roper wrote:
>> On Fri, Mar 04, 2016 at 12:45:38PM +0200, Ville Syrjälä wrote:
>>> On Thu, Mar 03, 2016 at 02:18:09PM -0800, Matt Roper wrote:
>>>> As of commit d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
>>>> it's now possible to have non-zero values for watermark levels that are
>>>> disabled (e.g., in the higher LP levels that can only be used when the sprite
>>>> is disabled).  Stop WARN()'ing when we see these non-zero sprite values in LP2
>>>> or LP3.
>>>>
>>>> Fixes the SNB warning:
>>>>
>>>>    WARNING: CPU: 1 PID: 25405 at /home/mattrope/work/kernel/gms/drivers/gpu/drm/i915/intel_pm.c:2580 ilk_program_watermarks+0x7b2/0x9d0 [i915]()
>>>>    WARN_ON(wm_lp != 1)
>>>>    Modules linked in: i915 drm_kms_helper drm bluetooth fuse iTCO_wdt iTCO_vendor_support syscopyarea sysfillrect sysimgblt fb_sys_fops tpm_tis mei_me e1000e snd_hda_codec_hdmi pcspkr tpm mei i2c_i801 lpc_ich snd_hda_codec snd_hda_core [last unloaded: drm]
>>>>    CPU: 1 PID: 25405 Comm: kms_universal_p Tainted: G     U  W       4.5.0-rc6apollolake+ #462
>>>>    Hardware name:                  /DH67GD, BIOS BLH6710H.86A.0160.2012.1204.1156 12/04/2012
>>>>     0000000000000000 ffff88009d42b918 ffffffff8143cfab ffff88009d42b960
>>>>     ffffffffa0363580 ffff88009d42b950 ffffffff81082746 ffff8800b9a24928
>>>>     ffff88009d42ba00 ffff88009d4a0000 0000000000000000 ffff88009d42ba6c
>>>>    Call Trace:
>>>>     [<ffffffff8143cfab>] dump_stack+0x4d/0x72
>>>>     [<ffffffff81082746>] warn_slowpath_common+0x86/0xc0
>>>>     [<ffffffff810827cc>] warn_slowpath_fmt+0x4c/0x50
>>>>     [<ffffffffa0292862>] ilk_program_watermarks+0x7b2/0x9d0 [i915]
>>>>     [<ffffffffa0292cb7>] ilk_initial_watermarks+0x107/0x120 [i915]
>>>>     [<ffffffffa02feffa>] intel_pre_plane_update+0x12a/0x190 [i915]
>>>>     [<ffffffffa02ffb36>] intel_atomic_commit+0x546/0xd50 [i915]
>>>>     [<ffffffffa012c9e7>] drm_atomic_commit+0x37/0x60 [drm]
>>>>     [<ffffffffa0217361>] drm_atomic_helper_disable_plane+0xb1/0xf0 [drm_kms_helper]
>>>>     [<ffffffffa011cdb4>] __setplane_internal+0x184/0x280 [drm]
>>>>     [<ffffffffa012b57a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
>>>>     [<ffffffffa012010f>] drm_mode_setplane+0x13f/0x1c0 [drm]
>>>>     [<ffffffffa0111b52>] drm_ioctl+0x142/0x590 [drm]
>>>>     [<ffffffffa011ffd0>] ? drm_plane_check_pixel_format+0x50/0x50 [drm]
>>>>     [<ffffffff811f2744>] ? mntput+0x24/0x40
>>>>     [<ffffffff811d28d4>] ? __fput+0x194/0x200
>>>>     [<ffffffffa012dec3>] drm_compat_ioctl+0x33/0x40 [drm]
>>>>     [<ffffffffa029e1c2>] i915_compat_ioctl+0x32/0x40 [i915]
>>>>     [<ffffffff81228d72>] compat_SyS_ioctl+0xc2/0x330
>>>>     [<ffffffff810021d5>] ? exit_to_usermode_loop+0x95/0xb0
>>>>     [<ffffffff81002d2e>] do_fast_syscall_32+0x9e/0x210
>>>>     [<ffffffff8197faf2>] entry_SYSENTER_compat+0x52/0x70
>>>>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Testcase: kms_universal_plane
>>>> Fixes: d81f04c5ef ("drm/i915: Allow preservation of watermarks, v2.")
>>>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>>> ---
>>>> The comment above this block of code indicates that we need to turn on
>>>> WM1S_LP_EN, even if the LP level is disabled, to avoid underruns.  It's not
>>>> clear to me from the bspec why this is, so I'm trusting that the comment
>>>> is still accurate, even though we wind up with non-zero values more often
>>>> now than we previously did.
>>>>
>>>>  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
>>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>> index 823437f..9698360 100644
>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>> @@ -2576,10 +2576,9 @@ static void ilk_compute_wm_results(struct drm_device *dev,
>>>>  		 * Always set WM1S_LP_EN when spr_val != 0, even if the
>>>>  		 * level is disabled. Doing otherwise could cause underruns.
>>>>  		 */
>>>> -		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
>>>> -			WARN_ON(wm_lp != 1);
>>> NAK
>>>
>>> There are no LP2+ sprite watermarks on ilk/snb, so if something is
>>> computing it, then we have clearly a bug somewhere else.
>> That's the point of this patch...having these values computed is
>> intentional now, not a bug.  The patch from Maarten/Paaulo computes the
>> values for *all* levels, even the ones we know are invalid, but marks
>> them as disabled.
> Then I think it should compute it as 0 since sprite LP2+ watermarks simply
> don't exist.
>
>> Given that change in behavior, it no longer makes sense to warn on
>> non-zero values since they're expected now (even though they'll never be
>> used).  Maybe instead of removing the WARN() I should just add an
>> "&& r->enable" to the warn condition?
>>
>>
>> Matt
>>
>>>> +		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val)
>>>>  			results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
>>>> -		} else
>>>> +		else
>>>>  			results->wm_lp_spr[wm_lp - 1] = r->spr_val;
>>>>  	}
>>>>  
>>>> -- 
>>>> 2.1.4
>>> -- 
>>> Ville Syrjälä
>>> Intel OTC
>> -- 
>> Matt Roper
>> Graphics Software Engineer
>> IoTG Platform Enabling & Development
>> Intel Corporation
>> (916) 356-2795

Would the following work?

ilk_validate_wm_level checks for !wm->enable, so it does the right thing here..

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f65e84137060..8640199061cb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1976,8 +1976,24 @@ static bool ilk_validate_wm_level(int level,
 	return ret;
 }
 
+static bool ilk_valid_sprite_level(const struct drm_i915_private *dev_priv,
+				   const struct intel_pipe_wm *pipe_wm,
+				   int level)
+{
+	/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
+	if (level && pipe_wm->sprites_scaled)
+		return false;
+
+	/* ILK/SNB: LP2+ watermarks only w/o sprites */
+	if (level > 1 && INTEL_INFO(dev_priv)->gen <= 6 && pipe_wm->sprites_enabled)
+		return false;
+
+	return true;
+}
+
 static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
 				 const struct intel_crtc *intel_crtc,
+				 const struct intel_pipe_wm *pipe_wm,
 				 int level,
 				 struct intel_crtc_state *cstate,
 				 struct intel_plane_state *pristate,
@@ -1988,6 +2004,7 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
 	uint16_t pri_latency = dev_priv->wm.pri_latency[level];
 	uint16_t spr_latency = dev_priv->wm.spr_latency[level];
 	uint16_t cur_latency = dev_priv->wm.cur_latency[level];
+	bool spr_valid = ilk_valid_sprite_level(dev_priv, pipe_wm, level);
 
 	/* WM1+ latency values stored in 0.5us units */
 	if (level > 0) {
@@ -2002,13 +2019,19 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
 		result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
 	}
 
-	if (sprstate)
-		result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
+	if (sprstate && spr_valid)
+		result->spr_val = ilk_compute_spr_wm(cstate, sprstate,
+						     spr_latency);
+	else if (sprstate)
+		result->spr_val = 0;
 
 	if (curstate)
 		result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
 
-	result->enable = true;
+	if (level && pipe_wm->sprites_enabled && !spr_valid)
+		result->enable = false;
+	else
+		result->enable = true;
 }
 
 static uint32_t
@@ -2306,7 +2329,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate)
 	struct intel_plane_state *pristate = NULL;
 	struct intel_plane_state *sprstate = NULL;
 	struct intel_plane_state *curstate = NULL;
-	int level, max_level = ilk_wm_max_level(dev), usable_level;
+	int level, max_level = ilk_wm_max_level(dev), usable_level = max_level;
 	struct ilk_wm_maximums max;
 
 	pipe_wm = &cstate->wm.optimal.ilk;
@@ -2335,18 +2358,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate)
 			 drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16);
 	}
 
-
-	usable_level = max_level;
-
-	/* ILK/SNB: LP2+ watermarks only w/o sprites */
-	if (INTEL_INFO(dev)->gen <= 6 && pipe_wm->sprites_enabled)
-		usable_level = 1;
-
-	/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
-	if (pipe_wm->sprites_scaled)
-		usable_level = 0;
-
-	ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
+	ilk_compute_wm_level(dev_priv, intel_crtc, pipe_wm, 0, cstate,
 			     pristate, sprstate, curstate, &pipe_wm->wm[0]);
 
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
@@ -2360,8 +2372,9 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate)
 	for (level = 1; level <= max_level; level++) {
 		struct intel_wm_level *wm = &pipe_wm->wm[level];
 
-		ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate,
-				     pristate, sprstate, curstate, wm);
+		ilk_compute_wm_level(dev_priv, intel_crtc, pipe_wm,
+				     level, cstate, pristate, sprstate,
+				     curstate, wm);
 
 		/*
 		 * Disable any watermark level that exceeds the

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-03-07 10:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-03 22:18 [PATCH] drm/i915: Don't WARN() when sprite watermark > 0 for disabled LP2/LP3 levels Matt Roper
2016-03-04 10:45 ` Ville Syrjälä
2016-03-04 17:37   ` Matt Roper
2016-03-04 17:47     ` Ville Syrjälä
2016-03-04 18:09       ` Ville Syrjälä
2016-03-07 10:52       ` Maarten Lankhorst
2016-03-07  9:13 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox