All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Block enabling FBC until flips have been completed
@ 2018-04-12 16:07 Maarten Lankhorst
  2018-04-12 16:56 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2018-04-12 16:07 UTC (permalink / raw)
  To: intel-gfx

There is a small race window in which FBC can be enabled after
pre_plane_update is called, but before the page flip has been
queued or completed.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103167
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_fbc.c | 35 +++++++++++------------------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0b8db3db141..2e2f24c2db9e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -492,6 +492,7 @@ struct intel_fbc {
 
 	bool enabled;
 	bool active;
+	bool flip_pending;
 
 	bool underrun_detected;
 	struct work_struct underrun_work;
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index b431b6733cc1..4770dd7dad5c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -924,13 +924,6 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 						32 * fbc->threshold) * 8;
 }
 
-static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1,
-				       struct intel_fbc_reg_params *params2)
-{
-	/* We can use this since intel_fbc_get_reg_params() does a memset. */
-	return memcmp(params1, params2, sizeof(*params1)) == 0;
-}
-
 void intel_fbc_pre_update(struct intel_crtc *crtc,
 			  struct intel_crtc_state *crtc_state,
 			  struct intel_plane_state *plane_state)
@@ -952,6 +945,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc,
 	if (!fbc->enabled || fbc->crtc != crtc)
 		goto unlock;
 
+	fbc->flip_pending = true;
 	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
 
 deactivate:
@@ -988,13 +982,15 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_fbc *fbc = &dev_priv->fbc;
-	struct intel_fbc_reg_params old_params;
 
 	WARN_ON(!mutex_is_locked(&fbc->lock));
 
 	if (!fbc->enabled || fbc->crtc != crtc)
 		return;
 
+	fbc->flip_pending = false;
+	WARN_ON(fbc->active);
+
 	if (!i915_modparams.enable_fbc) {
 		intel_fbc_deactivate(dev_priv, "disabled at runtime per module param");
 		__intel_fbc_disable(dev_priv);
@@ -1002,25 +998,16 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
 		return;
 	}
 
-	if (!intel_fbc_can_activate(crtc)) {
-		WARN_ON(fbc->active);
+	if (!intel_fbc_can_activate(crtc))
 		return;
-	}
 
-	old_params = fbc->params;
 	intel_fbc_get_reg_params(crtc, &fbc->params);
 
-	/* If the scanout has not changed, don't modify the FBC settings.
-	 * Note that we make the fundamental assumption that the fb->obj
-	 * cannot be unpinned (and have its GTT offset and fence revoked)
-	 * without first being decoupled from the scanout and FBC disabled.
-	 */
-	if (fbc->active &&
-	    intel_fbc_reg_params_equal(&old_params, &fbc->params))
-		return;
-
-	intel_fbc_deactivate(dev_priv, "FBC enabled (active or scheduled)");
-	intel_fbc_schedule_activation(crtc);
+	if (!fbc->busy_bits) {
+		intel_fbc_deactivate(dev_priv, "FBC enabled (active or scheduled)");
+		intel_fbc_schedule_activation(crtc);
+	} else
+		intel_fbc_deactivate(dev_priv, "frontbuffer write");
 }
 
 void intel_fbc_post_update(struct intel_crtc *crtc)
@@ -1085,7 +1072,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
 	    (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) {
 		if (fbc->active)
 			intel_fbc_recompress(dev_priv);
-		else
+		else if (!fbc->flip_pending)
 			__intel_fbc_post_update(fbc->crtc);
 	}
 
-- 
2.16.3

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

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Block enabling FBC until flips have been completed
  2018-04-12 16:07 [PATCH] drm/i915: Block enabling FBC until flips have been completed Maarten Lankhorst
@ 2018-04-12 16:56 ` Patchwork
  2018-04-12 17:11 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-04-12 16:56 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Block enabling FBC until flips have been completed
URL   : https://patchwork.freedesktop.org/series/41634/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
aa6ceaa62430 drm/i915: Block enabling FBC until flips have been completed
-:92: CHECK:BRACES: braces {} should be used on all arms of this statement
#92: FILE: drivers/gpu/drm/i915/intel_fbc.c:1006:
+	if (!fbc->busy_bits) {
[...]
+	} else
[...]

-:95: CHECK:BRACES: Unbalanced braces around else statement
#95: FILE: drivers/gpu/drm/i915/intel_fbc.c:1009:
+	} else

total: 0 errors, 0 warnings, 2 checks, 82 lines checked

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

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

* ✓ Fi.CI.BAT: success for drm/i915: Block enabling FBC until flips have been completed
  2018-04-12 16:07 [PATCH] drm/i915: Block enabling FBC until flips have been completed Maarten Lankhorst
  2018-04-12 16:56 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-04-12 17:11 ` Patchwork
  2018-04-12 18:58 ` ✓ Fi.CI.IGT: " Patchwork
  2018-04-12 19:41 ` [PATCH] " Souza, Jose
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-04-12 17:11 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Block enabling FBC until flips have been completed
URL   : https://patchwork.freedesktop.org/series/41634/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4049 -> Patchwork_8683 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-cnl-y3:          NOTRUN -> DMESG-FAIL (fdo#103191)

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927


== Participating hosts (35 -> 33) ==

  Additional (1): fi-cnl-y3 
  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4049 -> Patchwork_8683

  CI_DRM_4049: 3dbfa04d62f1f1214a03c3b2d30f987dccf50ab4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4426: d502f055ac4500cada758876a512ac4f14b34851 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8683: aa6ceaa62430ef2df33a85a9be4266b3b514c138 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4426: 93b35926a150e318439d2505901288594b3548f5 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

aa6ceaa62430 drm/i915: Block enabling FBC until flips have been completed

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915: Block enabling FBC until flips have been completed
  2018-04-12 16:07 [PATCH] drm/i915: Block enabling FBC until flips have been completed Maarten Lankhorst
  2018-04-12 16:56 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2018-04-12 17:11 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-12 18:58 ` Patchwork
  2018-04-12 19:41 ` [PATCH] " Souza, Jose
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-04-12 18:58 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Block enabling FBC until flips have been completed
URL   : https://patchwork.freedesktop.org/series/41634/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4049_full -> Patchwork_8683_full =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-vebox:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@preemptive-hang-blt:
      shard-kbl:          PASS -> DMESG-WARN (fdo#103558) +13

    igt@kms_flip@flip-vs-expired-vblank:
      shard-hsw:          PASS -> FAIL (fdo#105189)

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
      shard-apl:          PASS -> FAIL (fdo#103167)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-dpms-vs-vblank-race-interruptible:
      shard-hsw:          FAIL (fdo#103060) -> PASS +1

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-hsw:          FAIL (fdo#100368) -> PASS +1

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

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Missing    (2): shard-glk shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4049 -> Patchwork_8683

  CI_DRM_4049: 3dbfa04d62f1f1214a03c3b2d30f987dccf50ab4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4426: d502f055ac4500cada758876a512ac4f14b34851 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8683: aa6ceaa62430ef2df33a85a9be4266b3b514c138 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4426: 93b35926a150e318439d2505901288594b3548f5 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [PATCH] drm/i915: Block enabling FBC until flips have been completed
  2018-04-12 16:07 [PATCH] drm/i915: Block enabling FBC until flips have been completed Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2018-04-12 18:58 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-04-12 19:41 ` Souza, Jose
  2018-04-13  7:19   ` Maarten Lankhorst
  3 siblings, 1 reply; 6+ messages in thread
From: Souza, Jose @ 2018-04-12 19:41 UTC (permalink / raw)
  To: intel-gfx, maarten.lankhorst

On Thu, 2018-04-12 at 18:07 +0200, Maarten Lankhorst wrote:
> There is a small race window in which FBC can be enabled after
> pre_plane_update is called, but before the page flip has been
> queued or completed.

I don't think there is such window, intel_fbc_deactivate() that is
called from intel_fbc_pre_update() will set fbc->work.scheduled =
false; so the FBC will not be enabled in intel_fbc_work_fn()

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103167
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c | 35 +++++++++++-------------------
> -----
>  2 files changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index a0b8db3db141..2e2f24c2db9e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -492,6 +492,7 @@ struct intel_fbc {
>  
>  	bool enabled;
>  	bool active;
> +	bool flip_pending;
>  
>  	bool underrun_detected;
>  	struct work_struct underrun_work;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index b431b6733cc1..4770dd7dad5c 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -924,13 +924,6 @@ static void intel_fbc_get_reg_params(struct
> intel_crtc *crtc,
>  						32 * fbc->threshold) 
> * 8;
>  }
>  
> -static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params
> *params1,
> -				       struct intel_fbc_reg_params
> *params2)
> -{
> -	/* We can use this since intel_fbc_get_reg_params() does a
> memset. */
> -	return memcmp(params1, params2, sizeof(*params1)) == 0;
> -}
> -
>  void intel_fbc_pre_update(struct intel_crtc *crtc,
>  			  struct intel_crtc_state *crtc_state,
>  			  struct intel_plane_state *plane_state)
> @@ -952,6 +945,7 @@ void intel_fbc_pre_update(struct intel_crtc
> *crtc,
>  	if (!fbc->enabled || fbc->crtc != crtc)
>  		goto unlock;
>  
> +	fbc->flip_pending = true;

Also this is not a good name, other actions can cause this function to
be executed other than a flip.

>  	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
>  
>  deactivate:
> @@ -988,13 +982,15 @@ static void __intel_fbc_post_update(struct
> intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct intel_fbc *fbc = &dev_priv->fbc;
> -	struct intel_fbc_reg_params old_params;
>  
>  	WARN_ON(!mutex_is_locked(&fbc->lock));
>  
>  	if (!fbc->enabled || fbc->crtc != crtc)
>  		return;
>  
> +	fbc->flip_pending = false;
> +	WARN_ON(fbc->active);
> +
>  	if (!i915_modparams.enable_fbc) {
>  		intel_fbc_deactivate(dev_priv, "disabled at runtime
> per module param");
>  		__intel_fbc_disable(dev_priv);
> @@ -1002,25 +998,16 @@ static void __intel_fbc_post_update(struct
> intel_crtc *crtc)
>  		return;
>  	}
>  
> -	if (!intel_fbc_can_activate(crtc)) {
> -		WARN_ON(fbc->active);
> +	if (!intel_fbc_can_activate(crtc))
>  		return;
> -	}
>  
> -	old_params = fbc->params;
>  	intel_fbc_get_reg_params(crtc, &fbc->params);
>  
> -	/* If the scanout has not changed, don't modify the FBC
> settings.
> -	 * Note that we make the fundamental assumption that the fb-
> >obj
> -	 * cannot be unpinned (and have its GTT offset and fence
> revoked)
> -	 * without first being decoupled from the scanout and FBC
> disabled.
> -	 */
> -	if (fbc->active &&
> -	    intel_fbc_reg_params_equal(&old_params, &fbc->params))
> -		return;
> -
> -	intel_fbc_deactivate(dev_priv, "FBC enabled (active or
> scheduled)");
> -	intel_fbc_schedule_activation(crtc);
> +	if (!fbc->busy_bits) {

I guess this 'if' the line that is fixing the issue.

> +		intel_fbc_deactivate(dev_priv, "FBC enabled (active
> or scheduled)");
> +		intel_fbc_schedule_activation(crtc);
> +	} else
> +		intel_fbc_deactivate(dev_priv, "frontbuffer write");
>  }
>  
>  void intel_fbc_post_update(struct intel_crtc *crtc)
> @@ -1085,7 +1072,7 @@ void intel_fbc_flush(struct drm_i915_private
> *dev_priv,
>  	    (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc)))
> {
>  		if (fbc->active)
>  			intel_fbc_recompress(dev_priv);
> -		else
> +		else if (!fbc->flip_pending)
>  			__intel_fbc_post_update(fbc->crtc);
>  	}
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Block enabling FBC until flips have been completed
  2018-04-12 19:41 ` [PATCH] " Souza, Jose
@ 2018-04-13  7:19   ` Maarten Lankhorst
  0 siblings, 0 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2018-04-13  7:19 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

Op 12-04-18 om 21:41 schreef Souza, Jose:
> On Thu, 2018-04-12 at 18:07 +0200, Maarten Lankhorst wrote:
>> There is a small race window in which FBC can be enabled after
>> pre_plane_update is called, but before the page flip has been
>> queued or completed.
> I don't think there is such window, intel_fbc_deactivate() that is
> called from intel_fbc_pre_update() will set fbc->work.scheduled =
> false; so the FBC will not be enabled in intel_fbc_work_fn()
Yeah, intel_fbc_pre_update deactivates it, but intel_fbc_flush() can re-enable it. :)
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>>  drivers/gpu/drm/i915/intel_fbc.c | 35 +++++++++++-------------------
>> -----
>>  2 files changed, 12 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index a0b8db3db141..2e2f24c2db9e 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -492,6 +492,7 @@ struct intel_fbc {
>>  
>>  	bool enabled;
>>  	bool active;
>> +	bool flip_pending;
>>  
>>  	bool underrun_detected;
>>  	struct work_struct underrun_work;
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
>> b/drivers/gpu/drm/i915/intel_fbc.c
>> index b431b6733cc1..4770dd7dad5c 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -924,13 +924,6 @@ static void intel_fbc_get_reg_params(struct
>> intel_crtc *crtc,
>>  						32 * fbc->threshold) 
>> * 8;
>>  }
>>  
>> -static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params
>> *params1,
>> -				       struct intel_fbc_reg_params
>> *params2)
>> -{
>> -	/* We can use this since intel_fbc_get_reg_params() does a
>> memset. */
>> -	return memcmp(params1, params2, sizeof(*params1)) == 0;
>> -}
>> -
>>  void intel_fbc_pre_update(struct intel_crtc *crtc,
>>  			  struct intel_crtc_state *crtc_state,
>>  			  struct intel_plane_state *plane_state)
>> @@ -952,6 +945,7 @@ void intel_fbc_pre_update(struct intel_crtc
>> *crtc,
>>  	if (!fbc->enabled || fbc->crtc != crtc)
>>  		goto unlock;
>>  
>> +	fbc->flip_pending = true;
> Also this is not a good name, other actions can cause this function to
> be executed other than a flip.
Well, for FBC purposes it's a flip, but I am also ok with renaming it to update_pending? :)
>>  	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
>>  
>>  deactivate:
>> @@ -988,13 +982,15 @@ static void __intel_fbc_post_update(struct
>> intel_crtc *crtc)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>  	struct intel_fbc *fbc = &dev_priv->fbc;
>> -	struct intel_fbc_reg_params old_params;
>>  
>>  	WARN_ON(!mutex_is_locked(&fbc->lock));
>>  
>>  	if (!fbc->enabled || fbc->crtc != crtc)
>>  		return;
>>  
>> +	fbc->flip_pending = false;
>> +	WARN_ON(fbc->active);
>> +
>>  	if (!i915_modparams.enable_fbc) {
>>  		intel_fbc_deactivate(dev_priv, "disabled at runtime
>> per module param");
>>  		__intel_fbc_disable(dev_priv);
>> @@ -1002,25 +998,16 @@ static void __intel_fbc_post_update(struct
>> intel_crtc *crtc)
>>  		return;
>>  	}
>>  
>> -	if (!intel_fbc_can_activate(crtc)) {
>> -		WARN_ON(fbc->active);
>> +	if (!intel_fbc_can_activate(crtc))
>>  		return;
>> -	}
>>  
>> -	old_params = fbc->params;
>>  	intel_fbc_get_reg_params(crtc, &fbc->params);
>>  
>> -	/* If the scanout has not changed, don't modify the FBC
>> settings.
>> -	 * Note that we make the fundamental assumption that the fb-
>>> obj
>> -	 * cannot be unpinned (and have its GTT offset and fence
>> revoked)
>> -	 * without first being decoupled from the scanout and FBC
>> disabled.
>> -	 */
>> -	if (fbc->active &&
>> -	    intel_fbc_reg_params_equal(&old_params, &fbc->params))
>> -		return;
>> -
>> -	intel_fbc_deactivate(dev_priv, "FBC enabled (active or
>> scheduled)");
>> -	intel_fbc_schedule_activation(crtc);
>> +	if (!fbc->busy_bits) {
> I guess this 'if' the line that is fixing the issue.

I think that's not necessarily the case for these tests. I don't know if this fixes
the bug, as the dirtyfb is called after the atomic update completed. I just noted that
after pre_plane_update, you could sneak in a dirtyfb and then fbc could be activated
too early.

That's the hole I've been trying to close. But I closed it the other way around too
just in case. :) 

>> +		intel_fbc_deactivate(dev_priv, "FBC enabled (active
>> or scheduled)");
>> +		intel_fbc_schedule_activation(crtc);
>> +	} else
>> +		intel_fbc_deactivate(dev_priv, "frontbuffer write");
>>  }
>>  
>>  void intel_fbc_post_update(struct intel_crtc *crtc)
>> @@ -1085,7 +1072,7 @@ void intel_fbc_flush(struct drm_i915_private
>> *dev_priv,
>>  	    (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc)))
>> {
>>  		if (fbc->active)
>>  			intel_fbc_recompress(dev_priv);
>> -		else
>> +		else if (!fbc->flip_pending)
>>  			__intel_fbc_post_update(fbc->crtc);
>>  	}
>>  


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

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

end of thread, other threads:[~2018-04-13  7:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12 16:07 [PATCH] drm/i915: Block enabling FBC until flips have been completed Maarten Lankhorst
2018-04-12 16:56 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-04-12 17:11 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-12 18:58 ` ✓ Fi.CI.IGT: " Patchwork
2018-04-12 19:41 ` [PATCH] " Souza, Jose
2018-04-13  7:19   ` Maarten Lankhorst

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.