All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] Fix the possible watermark miswriting
@ 2018-11-13 14:27 Stanislav Lisovskiy
  2018-11-13 14:47 ` [Intel-gfx] " Ville Syrjälä
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stanislav Lisovskiy @ 2018-11-13 14:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: ville.syrjala, martin.peres, dri-devel

Currently whenever we attempt to recalculate
watermarks, we assign dirty_pipes to zero,
then compare current wm results to the recalculated
one and if they changed we set correspondent dirty_pipes
bit again.
This can lead to situation, when we same clearing dirty_pipes,
same wm results twice and not setting dirty_pipes => so that
watermarks are not actually updated, which then might
lead to fifo underruns, crc mismatch and other issues.

Instead, whenever we detect that wm results are changed,
need to set correspondent dirty_pipes bit and clear it
only once the change is written, but not clear it everytime
we attempt to recalculate those in skl_compute_wm.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dc034617febb..f7fbc4bc0d43 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5441,9 +5441,6 @@ skl_compute_wm(struct drm_atomic_state *state)
 	bool changed = false;
 	int ret, i;
 
-	/* Clear all dirty flags */
-	results->dirty_pipes = 0;
-
 	ret = skl_ddb_add_affected_pipes(state, &changed);
 	if (ret || !changed)
 		return ret;
@@ -5496,6 +5493,7 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal;
 	const struct skl_ddb_allocation *ddb = &state->wm_results.ddb;
+	struct skl_ddb_values *results = &state->wm_results;
 	enum pipe pipe = crtc->pipe;
 	enum plane_id plane_id;
 
@@ -5512,6 +5510,10 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 			skl_write_cursor_wm(crtc, &pipe_wm->planes[plane_id],
 					    ddb);
 	}
+
+	/* Clear correspondent dirty bit */
+	results->dirty_pipes &= ~drm_crtc_mask(&crtc->base);
+
 }
 
 static void skl_initial_wm(struct intel_atomic_state *state,
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Intel-gfx] [PATCH v1] Fix the possible watermark miswriting
  2018-11-13 14:27 [PATCH v1] Fix the possible watermark miswriting Stanislav Lisovskiy
@ 2018-11-13 14:47 ` Ville Syrjälä
  2018-11-13 14:55 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-11-13 18:50 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2018-11-13 14:47 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx, ville.syrjala, martin.peres, dri-devel

On Tue, Nov 13, 2018 at 04:27:28PM +0200, Stanislav Lisovskiy wrote:
> Currently whenever we attempt to recalculate
> watermarks, we assign dirty_pipes to zero,
> then compare current wm results to the recalculated
> one and if they changed we set correspondent dirty_pipes
> bit again.
> This can lead to situation, when we same clearing dirty_pipes,
> same wm results twice and not setting dirty_pipes => so that
> watermarks are not actually updated, which then might
> lead to fifo underruns, crc mismatch and other issues.
> 
> Instead, whenever we detect that wm results are changed,
> need to set correspondent dirty_pipes bit and clear it
> only once the change is written, but not clear it everytime
> we attempt to recalculate those in skl_compute_wm.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index dc034617febb..f7fbc4bc0d43 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5441,9 +5441,6 @@ skl_compute_wm(struct drm_atomic_state *state)
>  	bool changed = false;
>  	int ret, i;
>  
> -	/* Clear all dirty flags */
> -	results->dirty_pipes = 0;
> -
>  	ret = skl_ddb_add_affected_pipes(state, &changed);
>  	if (ret || !changed)
>  		return ret;
> @@ -5496,6 +5493,7 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal;
>  	const struct skl_ddb_allocation *ddb = &state->wm_results.ddb;
> +	struct skl_ddb_values *results = &state->wm_results;
>  	enum pipe pipe = crtc->pipe;
>  	enum plane_id plane_id;
>  
> @@ -5512,6 +5510,10 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
>  			skl_write_cursor_wm(crtc, &pipe_wm->planes[plane_id],
>  					    ddb);
>  	}
> +
> +	/* Clear correspondent dirty bit */
> +	results->dirty_pipes &= ~drm_crtc_mask(&crtc->base);
> +
>  }

Hmm. I can't figure out what the problem really is here. Yes, it does
look like we'd end up writing the watermarks twice for pipes that
are getting enabled. But that should be safeish (apart from the whole
"need PLANE_SURF write to atually arm the update" mess).

Note that this whole code is getting nuked soon (I hope):
https://patchwork.freedesktop.org/series/51878/
but in the meantime if there is a clear bug we probably want the fix in
first so that it can be backported. So I think we need a clear analysis
of the actual problem.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* ✓ Fi.CI.BAT: success for Fix the possible watermark miswriting
  2018-11-13 14:27 [PATCH v1] Fix the possible watermark miswriting Stanislav Lisovskiy
  2018-11-13 14:47 ` [Intel-gfx] " Ville Syrjälä
@ 2018-11-13 14:55 ` Patchwork
  2018-11-13 18:50 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 4+ messages in thread
From: Patchwork @ 2018-11-13 14:55 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

== Series Details ==

Series: Fix the possible watermark miswriting
URL   : https://patchwork.freedesktop.org/series/52423/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5132 -> Patchwork_10813 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52423/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10813 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


== Participating hosts (49 -> 44) ==

  Additional (3): fi-byt-j1900 fi-glk-j4005 fi-kbl-7567u 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-elk-e7500 fi-icl-u 


== Build changes ==

    * Linux: CI_DRM_5132 -> Patchwork_10813

  CI_DRM_5132: 7fa49786320fddb8a4f89318e7eeb65ce6134f8b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10813: 4bd7c3cebd073fd0205cfa012293c97b2e9c5a51 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4bd7c3cebd07 Fix the possible watermark miswriting

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10813/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

* ✓ Fi.CI.IGT: success for Fix the possible watermark miswriting
  2018-11-13 14:27 [PATCH v1] Fix the possible watermark miswriting Stanislav Lisovskiy
  2018-11-13 14:47 ` [Intel-gfx] " Ville Syrjälä
  2018-11-13 14:55 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-11-13 18:50 ` Patchwork
  2 siblings, 0 replies; 4+ messages in thread
From: Patchwork @ 2018-11-13 18:50 UTC (permalink / raw)
  To: Stanislav Lisovskiy; +Cc: intel-gfx

== Series Details ==

Series: Fix the possible watermark miswriting
URL   : https://patchwork.freedesktop.org/series/52423/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5132_full -> Patchwork_10813_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10813_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10813_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10813_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    igt@tools_test@tools_test:
      shard-apl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10813_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_color@pipe-c-legacy-gamma:
      shard-apl:          PASS -> FAIL (fdo#104782)

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-glk:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
      shard-apl:          PASS -> FAIL (fdo#103167) +2

    igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#107815)

    igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
      shard-glk:          NOTRUN -> FAIL (fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@kms_vblank@pipe-b-ts-continuation-suspend:
      shard-skl:          PASS -> INCOMPLETE (fdo#104108, fdo#107773)

    igt@perf@polling:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    igt@perf_pmu@busy-start-vcs0:
      shard-apl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558) +1

    igt@pm_rpm@pc8-residency:
      shard-skl:          SKIP -> INCOMPLETE (fdo#107807) +1

    igt@pm_rpm@universal-planes-dpms:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-skl:          INCOMPLETE (fdo#106886) -> PASS

    igt@gem_exec_suspend@basic-s3:
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@gem_exec_whisper@normal:
      shard-skl:          TIMEOUT (fdo#108592) -> PASS

    igt@gem_tiled_blits@normal:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_color@pipe-a-ctm-0-75:
      shard-skl:          FAIL (fdo#108682) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
      shard-glk:          FAIL (fdo#103167) -> PASS +2

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-apl:          FAIL (fdo#103166) -> PASS

    igt@kms_properties@plane-properties-atomic:
      shard-kbl:          DMESG-WARN (fdo#103313, fdo#105345) -> PASS

    igt@kms_setmode@basic:
      shard-hsw:          FAIL (fdo#99912) -> PASS
      shard-kbl:          FAIL (fdo#99912) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105345 https://bugs.freedesktop.org/show_bug.cgi?id=105345
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108592 https://bugs.freedesktop.org/show_bug.cgi?id=108592
  fdo#108682 https://bugs.freedesktop.org/show_bug.cgi?id=108682
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5132 -> Patchwork_10813

  CI_DRM_5132: 7fa49786320fddb8a4f89318e7eeb65ce6134f8b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10813: 4bd7c3cebd073fd0205cfa012293c97b2e9c5a51 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10813/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-11-13 18:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-13 14:27 [PATCH v1] Fix the possible watermark miswriting Stanislav Lisovskiy
2018-11-13 14:47 ` [Intel-gfx] " Ville Syrjälä
2018-11-13 14:55 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-11-13 18:50 ` ✓ Fi.CI.IGT: " Patchwork

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.