[PATCH] drm/i915: Fix big cursors on snb

[PATCH] drm/i915: Fix big cursors on snb

[Daniel Vetter]

My snb seemed somewhat unhappy with 256x256 cursors and failed all the
relevant kms_cursor_crc subtests sporadically, including logging cpu
fifo underruns. Smaller cursor work perfectly with a failure rate at
least 1000x less (got bored after running tests for days).

After some playing around with impressive hammers Ville suggested to
increase the watermarks to at least cover a full cursor line. It seems
to work thus far. This means that for lp wms we now required 1/4th of
the fifo for 256 wide cursors, and much less on gen7+. Hence fetches
should still be nice&big. Therefore I expect very little power impact
and decided to just do this everywhere, even though I've only seen
this on my snb laptop. Especially since Ville is thinking about
similar changes for the gmch wm code. skl separate wm code and also
completely new plane hw, so hopefully isn't affected.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Paulo Zanoni <przanoni@gmail.com>
Testcase: igt/kms_cursor_crc/*-256x256-*
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7dcb5b60600b..7e97a30bd6c0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1449,14 +1449,21 @@ static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
 static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
 				   uint32_t mem_value)
 {
+	uint32_t line_wm, method2;
+
 	if (!params->active || !params->cur.enabled)
 		return 0;
 
-	return ilk_wm_method2(params->pixel_rate,
-			      params->pipe_htotal,
-			      params->cur.horiz_pixels,
-			      params->cur.bytes_per_pixel,
-			      mem_value);
+	/* HACK: Big cursors tend to underrun, load at least one line. */
+	line_wm = params->cur.horiz_pixels * params->cur.bytes_per_pixel / 64;
+
+	method2 = ilk_wm_method2(params->pixel_rate,
+				 params->pipe_htotal,
+				 params->cur.horiz_pixels,
+				 params->cur.bytes_per_pixel,
+				 mem_value);
+
+	return max(line_wm, method2);
 }
 
 /* Only for WM_LP. */
-- 
1.9.3

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

Re: [PATCH] drm/i915: Fix big cursors on snb

[Daniel Vetter]

Forgotten to cc Art as fyi.
-Daniel

On Mon, Mar 2, 2015 at 5:35 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> My snb seemed somewhat unhappy with 256x256 cursors and failed all the
> relevant kms_cursor_crc subtests sporadically, including logging cpu
> fifo underruns. Smaller cursor work perfectly with a failure rate at
> least 1000x less (got bored after running tests for days).
>
> After some playing around with impressive hammers Ville suggested to
> increase the watermarks to at least cover a full cursor line. It seems
> to work thus far. This means that for lp wms we now required 1/4th of
> the fifo for 256 wide cursors, and much less on gen7+. Hence fetches
> should still be nice&big. Therefore I expect very little power impact
> and decided to just do this everywhere, even though I've only seen
> this on my snb laptop. Especially since Ville is thinking about
> similar changes for the gmch wm code. skl separate wm code and also
> completely new plane hw, so hopefully isn't affected.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Testcase: igt/kms_cursor_crc/*-256x256-*
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7dcb5b60600b..7e97a30bd6c0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1449,14 +1449,21 @@ static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
>  static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
>                                    uint32_t mem_value)
>  {
> +       uint32_t line_wm, method2;
> +
>         if (!params->active || !params->cur.enabled)
>                 return 0;
>
> -       return ilk_wm_method2(params->pixel_rate,
> -                             params->pipe_htotal,
> -                             params->cur.horiz_pixels,
> -                             params->cur.bytes_per_pixel,
> -                             mem_value);
> +       /* HACK: Big cursors tend to underrun, load at least one line. */
> +       line_wm = params->cur.horiz_pixels * params->cur.bytes_per_pixel / 64;
> +
> +       method2 = ilk_wm_method2(params->pixel_rate,
> +                                params->pipe_htotal,
> +                                params->cur.horiz_pixels,
> +                                params->cur.bytes_per_pixel,
> +                                mem_value);
> +
> +       return max(line_wm, method2);
>  }
>
>  /* Only for WM_LP. */
> --
> 1.9.3
>



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

Re: [PATCH] drm/i915: Fix big cursors on snb

[Runyan, Arthur J]

I think your ilk_wm_method2 is busted.  Method 2 should always give more than one full line, making this 1 line redundant.


-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] 
Sent: Monday, March 02, 2015 9:09 AM
To: Intel Graphics Development; Runyan, Arthur J
Cc: Daniel Vetter; Ville Syrjälä; Roper, Matthew D; Paulo Zanoni; Vetter, Daniel
Subject: Re: [PATCH] drm/i915: Fix big cursors on snb

Forgotten to cc Art as fyi.
-Daniel

On Mon, Mar 2, 2015 at 5:35 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> My snb seemed somewhat unhappy with 256x256 cursors and failed all the
> relevant kms_cursor_crc subtests sporadically, including logging cpu
> fifo underruns. Smaller cursor work perfectly with a failure rate at
> least 1000x less (got bored after running tests for days).
>
> After some playing around with impressive hammers Ville suggested to
> increase the watermarks to at least cover a full cursor line. It seems
> to work thus far. This means that for lp wms we now required 1/4th of
> the fifo for 256 wide cursors, and much less on gen7+. Hence fetches
> should still be nice&big. Therefore I expect very little power impact
> and decided to just do this everywhere, even though I've only seen
> this on my snb laptop. Especially since Ville is thinking about
> similar changes for the gmch wm code. skl separate wm code and also
> completely new plane hw, so hopefully isn't affected.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Testcase: igt/kms_cursor_crc/*-256x256-*
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7dcb5b60600b..7e97a30bd6c0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm[Art]  /i915/intel_pm.c
> @@ -1449,14 +1449,21 @@ static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
>  static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
>                                    uint32_t mem_value)
>  {
> +       uint32_t line_wm, method2;
> +
>         if (!params->active || !params->cur.enabled)
>                 return 0;
>
> -       return ilk_wm_method2(params->pixel_rate,
> -                             params->pipe_htotal,
> -                             params->cur.horiz_pixels,
> -                             params->cur.bytes_per_pixel,
> -                             mem_value);
> +       /* HACK: Big cursors tend to underrun, load at least one line. */
> +       line_wm = params->cur.horiz_pixels * params->cur.bytes_per_pixel / 64;
> +
> +       method2 = ilk_wm_method2(params->pixel_rate,
> +                                params->pipe_htotal,
> +                                params->cur.horiz_pixels,
> +                                params->cur.bytes_per_pixel,
> +                                mem_value);
> +
> +       return max(line_wm, method2);
>  }
>
>  /* Only for WM_LP. */
> --
> 1.9.3
>



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

Re: [PATCH] drm/i915: Fix big cursors on snb

[Daniel Vetter]

Hi Art,

Indeed my patch looks fishy and it's because this isn't the code I've
tested the entire night :(

The one that works simply disables lp1+ wm levels. Which is a bit an
oversized hammer really. Do you have any ideas for w/a or smaller
tricks I could try? I did try just blantantly multiplying wm levels a
bit but that didn't help. At least not the few things I've tried.

Still only seen this on snb thus far.

Thanks, Daniel


On Mon, Mar 2, 2015 at 8:39 PM, Runyan, Arthur J
<arthur.j.runyan@intel.com> wrote:
> I think your ilk_wm_method2 is busted.  Method 2 should always give more than one full line, making this 1 line redundant.
>
>
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch]
> Sent: Monday, March 02, 2015 9:09 AM
> To: Intel Graphics Development; Runyan, Arthur J
> Cc: Daniel Vetter; Ville Syrjälä; Roper, Matthew D; Paulo Zanoni; Vetter, Daniel
> Subject: Re: [PATCH] drm/i915: Fix big cursors on snb
>
> Forgotten to cc Art as fyi.
> -Daniel
>
> On Mon, Mar 2, 2015 at 5:35 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> My snb seemed somewhat unhappy with 256x256 cursors and failed all the
>> relevant kms_cursor_crc subtests sporadically, including logging cpu
>> fifo underruns. Smaller cursor work perfectly with a failure rate at
>> least 1000x less (got bored after running tests for days).
>>
>> After some playing around with impressive hammers Ville suggested to
>> increase the watermarks to at least cover a full cursor line. It seems
>> to work thus far. This means that for lp wms we now required 1/4th of
>> the fifo for 256 wide cursors, and much less on gen7+. Hence fetches
>> should still be nice&big. Therefore I expect very little power impact
>> and decided to just do this everywhere, even though I've only seen
>> this on my snb laptop. Especially since Ville is thinking about
>> similar changes for the gmch wm code. skl separate wm code and also
>> completely new plane hw, so hopefully isn't affected.
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: Paulo Zanoni <przanoni@gmail.com>
>> Testcase: igt/kms_cursor_crc/*-256x256-*
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 7dcb5b60600b..7e97a30bd6c0 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm[Art]  /i915/intel_pm.c
>> @@ -1449,14 +1449,21 @@ static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
>>  static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
>>                                    uint32_t mem_value)
>>  {
>> +       uint32_t line_wm, method2;
>> +
>>         if (!params->active || !params->cur.enabled)
>>                 return 0;
>>
>> -       return ilk_wm_method2(params->pixel_rate,
>> -                             params->pipe_htotal,
>> -                             params->cur.horiz_pixels,
>> -                             params->cur.bytes_per_pixel,
>> -                             mem_value);
>> +       /* HACK: Big cursors tend to underrun, load at least one line. */
>> +       line_wm = params->cur.horiz_pixels * params->cur.bytes_per_pixel / 64;
>> +
>> +       method2 = ilk_wm_method2(params->pixel_rate,
>> +                                params->pipe_htotal,
>> +                                params->cur.horiz_pixels,
>> +                                params->cur.bytes_per_pixel,
>> +                                mem_value);
>> +
>> +       return max(line_wm, method2);
>>  }
>>
>>  /* Only for WM_LP. */
>> --
>> 1.9.3
>>
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



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

Re: [PATCH] drm/i915: Fix big cursors on snb

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5871
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -5              278/278              273/278
ILK                                  308/308              308/308
SNB                                  284/284              284/284
IVB                                  380/380              380/380
BYT                                  294/294              294/294
HSW                                  387/387              387/387
BDW                 -1              316/316              315/316
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_fence_thrash_bo-write-verify-none      PASS(5)      FAIL(1)NRUN(1)
*PNV  igt_gem_fence_thrash_bo-write-verify-x      PASS(5)      FAIL(1)NO_RESULT(1)
*PNV  igt_gem_fence_thrash_bo-write-verify-y      PASS(5)      FAIL(1)PASS(1)
 PNV  igt_gem_userptr_blits_coherency-sync      CRASH(5)PASS(7)      CRASH(2)
*PNV  igt_gem_userptr_blits_minor-unsync-interruptible      PASS(4)      DMESG_WARN(1)PASS(1)
*BDW  igt_gem_gtt_hog      PASS(15)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx