All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: add missing condition for committing planes on crtc
@ 2016-04-08 16:30 Lionel Landwerlin
  2016-04-09  6:26 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Lionel Landwerlin @ 2016-04-08 16:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter

We are currently missing the color management update condition to
commit planes on crtc.

Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index feb7028..670b2ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13480,6 +13480,13 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
 	return false;
 }
 
+static bool needs_commit_planes_on_crtc(struct drm_crtc_state *crtc_state)
+{
+	return (crtc_state->planes_changed ||
+		crtc_state->color_mgmt_changed ||
+		to_intel_crtc_state(crtc_state)->update_pipe);
+}
+
 /**
  * intel_atomic_commit - commit validated state object
  * @dev: DRM device
@@ -13583,7 +13590,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 		bool modeset = needs_modeset(crtc->state);
 		struct intel_crtc_state *pipe_config =
 			to_intel_crtc_state(crtc->state);
-		bool update_pipe = !modeset && pipe_config->update_pipe;
 
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
@@ -13597,8 +13603,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		    drm_atomic_get_existing_plane_state(state, crtc->primary))
 			intel_fbc_enable(intel_crtc);
 
-		if (crtc->state->active &&
-		    (crtc->state->planes_changed || update_pipe))
+		if (crtc->state->active && !modeset &&
+		    needs_commit_planes_on_crtc(crtc->state))
 			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
 
 		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
-- 
2.8.0.rc3

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: add missing condition for committing planes on crtc
  2016-04-08 16:30 [PATCH] drm/i915: add missing condition for committing planes on crtc Lionel Landwerlin
@ 2016-04-09  6:26 ` Patchwork
  2016-04-21 13:34   ` Maarten Lankhorst
  2016-04-18 11:05 ` [PATCH] " Lionel Landwerlin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: Patchwork @ 2016-04-09  6:26 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: add missing condition for committing planes on crtc
URL   : https://patchwork.freedesktop.org/series/5467/
State : failure

== Summary ==

Series 5467v1 drm/i915: add missing condition for committing planes on crtc
http://patchwork.freedesktop.org/api/1.0/series/5467/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
Test gem_exec_basic:
        Subgroup basic-bsd:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
Test gem_sync:
        Subgroup basic-bsd:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (skl-i7k-2)
                pass       -> DMESG-WARN (bdw-ultra)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> FAIL       (bsw-nuc-2)
                pass       -> FAIL       (ivb-t430s)
                pass       -> DMESG-FAIL (bdw-ultra)
                pass       -> FAIL       (skl-i7k-2)
                pass       -> FAIL       (byt-nuc)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> DMESG-FAIL (bdw-nuci7)
                skip       -> FAIL       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup read-crc-pipe-c:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                pass       -> DMESG-WARN (bdw-ultra)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)

bdw-nuci7        total:196  pass:157  dwarn:26  dfail:1   fail:0   skip:12 
bdw-ultra        total:196  pass:147  dwarn:27  dfail:1   fail:0   skip:21 
bsw-nuc-2        total:196  pass:158  dwarn:0   dfail:0   fail:1   skip:37 
byt-nuc          total:196  pass:160  dwarn:0   dfail:0   fail:1   skip:35 
hsw-brixbox      total:196  pass:173  dwarn:0   dfail:0   fail:1   skip:22 
ilk-hp8440p      total:196  pass:132  dwarn:0   dfail:0   fail:1   skip:63 
ivb-t430s        total:196  pass:170  dwarn:0   dfail:0   fail:1   skip:25 
skl-i7k-2        total:196  pass:171  dwarn:1   dfail:0   fail:1   skip:23 
snb-dellxps      total:196  pass:161  dwarn:0   dfail:0   fail:1   skip:34 
snb-x220t        total:196  pass:161  dwarn:0   dfail:0   fail:2   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1850/

949884a57b51aa158e3ae9afe1f08130cdb7a3ef drm-intel-nightly: 2016y-04m-08d-10h-45m-28s UTC integration manifest
198b973f5081944a3415d2786c9f42b59ca6285b drm/i915: add missing condition for committing planes on crtc

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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-04-08 16:30 [PATCH] drm/i915: add missing condition for committing planes on crtc Lionel Landwerlin
  2016-04-09  6:26 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-04-18 11:05 ` Lionel Landwerlin
  2016-04-21 13:30   ` Maarten Lankhorst
  2016-04-18 11:06 ` Maarten Lankhorst
  2016-04-18 11:28 ` Ville Syrjälä
  3 siblings, 1 reply; 24+ messages in thread
From: Lionel Landwerlin @ 2016-04-18 11:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter

Ping?

On 08/04/16 17:30, Lionel Landwerlin wrote:
> We are currently missing the color management update condition to
> commit planes on crtc.
>
> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index feb7028..670b2ad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13480,6 +13480,13 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
>   	return false;
>   }
>   
> +static bool needs_commit_planes_on_crtc(struct drm_crtc_state *crtc_state)
> +{
> +	return (crtc_state->planes_changed ||
> +		crtc_state->color_mgmt_changed ||
> +		to_intel_crtc_state(crtc_state)->update_pipe);
> +}
> +
>   /**
>    * intel_atomic_commit - commit validated state object
>    * @dev: DRM device
> @@ -13583,7 +13590,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>   		bool modeset = needs_modeset(crtc->state);
>   		struct intel_crtc_state *pipe_config =
>   			to_intel_crtc_state(crtc->state);
> -		bool update_pipe = !modeset && pipe_config->update_pipe;
>   
>   		if (modeset && crtc->state->active) {
>   			update_scanline_offset(to_intel_crtc(crtc));
> @@ -13597,8 +13603,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>   		    drm_atomic_get_existing_plane_state(state, crtc->primary))
>   			intel_fbc_enable(intel_crtc);
>   
> -		if (crtc->state->active &&
> -		    (crtc->state->planes_changed || update_pipe))
> +		if (crtc->state->active && !modeset &&
> +		    needs_commit_planes_on_crtc(crtc->state))
>   			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
>   
>   		if (pipe_config->base.active && needs_vblank_wait(pipe_config))

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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-04-08 16:30 [PATCH] drm/i915: add missing condition for committing planes on crtc Lionel Landwerlin
  2016-04-09  6:26 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2016-04-18 11:05 ` [PATCH] " Lionel Landwerlin
@ 2016-04-18 11:06 ` Maarten Lankhorst
  2016-04-18 11:28 ` Ville Syrjälä
  3 siblings, 0 replies; 24+ messages in thread
From: Maarten Lankhorst @ 2016-04-18 11:06 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx; +Cc: Jani Nikula, Daniel Vetter

Op 08-04-16 om 18:30 schreef Lionel Landwerlin:
> We are currently missing the color management update condition to
> commit planes on crtc.
>
> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-04-08 16:30 [PATCH] drm/i915: add missing condition for committing planes on crtc Lionel Landwerlin
                   ` (2 preceding siblings ...)
  2016-04-18 11:06 ` Maarten Lankhorst
@ 2016-04-18 11:28 ` Ville Syrjälä
  3 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2016-04-18 11:28 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Fri, Apr 08, 2016 at 05:30:40PM +0100, Lionel Landwerlin wrote:
> We are currently missing the color management update condition to
> commit planes on crtc.

On a related note, could you move the LUT stuff out from the critical
section? It has no business being there since the registers aren't
double buffered. I'm thinking this is the cause for the sporadic
atomic update fails that I've seen.

> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index feb7028..670b2ad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13480,6 +13480,13 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
>  	return false;
>  }
>  
> +static bool needs_commit_planes_on_crtc(struct drm_crtc_state *crtc_state)
> +{
> +	return (crtc_state->planes_changed ||
> +		crtc_state->color_mgmt_changed ||
> +		to_intel_crtc_state(crtc_state)->update_pipe);
> +}
> +
>  /**
>   * intel_atomic_commit - commit validated state object
>   * @dev: DRM device
> @@ -13583,7 +13590,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		bool modeset = needs_modeset(crtc->state);
>  		struct intel_crtc_state *pipe_config =
>  			to_intel_crtc_state(crtc->state);
> -		bool update_pipe = !modeset && pipe_config->update_pipe;
>  
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
> @@ -13597,8 +13603,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		    drm_atomic_get_existing_plane_state(state, crtc->primary))
>  			intel_fbc_enable(intel_crtc);
>  
> -		if (crtc->state->active &&
> -		    (crtc->state->planes_changed || update_pipe))
> +		if (crtc->state->active && !modeset &&
> +		    needs_commit_planes_on_crtc(crtc->state))
>  			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
>  
>  		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> -- 
> 2.8.0.rc3
> 
> _______________________________________________
> 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] 24+ messages in thread

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-04-18 11:05 ` [PATCH] " Lionel Landwerlin
@ 2016-04-21 13:30   ` Maarten Lankhorst
  2016-04-21 13:57     ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Maarten Lankhorst @ 2016-04-21 13:30 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx; +Cc: Jani Nikula, Daniel Vetter

Hey,

Op 18-04-16 om 13:05 schreef Lionel Landwerlin:
> Ping?
>
Will commit, but looks like Ville made a comment about double buffering.

Everything in intel_pipe_update_start depends on double buffering, so if the lut isn't then it probably has to be done either before or afterwards. Not sure which, either way would produce some flickering. :(

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

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

* Re: ✗ Fi.CI.BAT: failure for drm/i915: add missing condition for committing planes on crtc
  2016-04-09  6:26 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-04-21 13:34   ` Maarten Lankhorst
  0 siblings, 0 replies; 24+ messages in thread
From: Maarten Lankhorst @ 2016-04-21 13:34 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

Op 09-04-16 om 08:26 schreef Patchwork:
> == Series Details ==
>
> Series: drm/i915: add missing condition for committing planes on crtc
> URL   : https://patchwork.freedesktop.org/series/5467/
> State : failure
>
> == Summary ==
>
> Series 5467v1 drm/i915: add missing condition for committing planes on crtc
> http://patchwork.freedesktop.org/api/1.0/series/5467/revisions/1/mbox/
>
> Test drv_module_reload_basic:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
> Test gem_exec_basic:
>         Subgroup basic-bsd:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
> Test gem_sync:
>         Subgroup basic-bsd:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup basic-flip-vs-modeset:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup basic-plain-flip:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (skl-i7k-2)
>                 pass       -> DMESG-WARN (bdw-ultra)
> Test kms_frontbuffer_tracking:
>         Subgroup basic:
>                 pass       -> FAIL       (bsw-nuc-2)
>                 pass       -> FAIL       (ivb-t430s)
>                 pass       -> DMESG-FAIL (bdw-ultra)
>                 pass       -> FAIL       (skl-i7k-2)
>                 pass       -> FAIL       (byt-nuc)
>                 pass       -> FAIL       (snb-x220t)
>                 pass       -> FAIL       (snb-dellxps)
>                 pass       -> FAIL       (hsw-brixbox)
>                 pass       -> DMESG-FAIL (bdw-nuci7)
>                 skip       -> FAIL       (ilk-hp8440p)
> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup hang-read-crc-pipe-b:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup hang-read-crc-pipe-c:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup nonblocking-crc-pipe-a:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup nonblocking-crc-pipe-a-frame-sequence:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup nonblocking-crc-pipe-b:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup nonblocking-crc-pipe-b-frame-sequence:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup nonblocking-crc-pipe-c:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup nonblocking-crc-pipe-c-frame-sequence:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup read-crc-pipe-a:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup read-crc-pipe-a-frame-sequence:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup read-crc-pipe-b:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup read-crc-pipe-b-frame-sequence:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup read-crc-pipe-c:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup read-crc-pipe-c-frame-sequence:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup suspend-read-crc-pipe-b:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
> Test kms_psr_sink_crc:
>         Subgroup psr_basic:
>                 pass       -> DMESG-WARN (bdw-ultra)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup basic-rte:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
>                 pass       -> DMESG-WARN (bdw-nuci7)
>                 pass       -> DMESG-WARN (bdw-ultra)
>
Please look at the CI results.

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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-04-21 13:30   ` Maarten Lankhorst
@ 2016-04-21 13:57     ` Ville Syrjälä
  2016-05-04 10:25       ` Lionel Landwerlin
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2016-04-21 13:57 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Thu, Apr 21, 2016 at 03:30:09PM +0200, Maarten Lankhorst wrote:
> Hey,
> 
> Op 18-04-16 om 13:05 schreef Lionel Landwerlin:
> > Ping?
> >
> Will commit, but looks like Ville made a comment about double buffering.
> 
> Everything in intel_pipe_update_start depends on double buffering, so if the lut isn't then it probably has to be done either before or afterwards. Not sure which, either way would produce some flickering. :(

It should be done from a vblank worker. Unfortunately we still don't
have those, so either before or after is fine, and it'll result in
exactly the same amount of flickering as the current code.

-- 
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] 24+ messages in thread

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-04-21 13:57     ` Ville Syrjälä
@ 2016-05-04 10:25       ` Lionel Landwerlin
  2016-05-04 10:30         ` Maarten Lankhorst
  2016-05-04 11:41         ` Ville Syrjälä
  0 siblings, 2 replies; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-04 10:25 UTC (permalink / raw)
  To: Ville Syrjälä, Maarten Lankhorst
  Cc: Jani Nikula, Daniel Vetter, intel-gfx

On 21/04/16 14:57, Ville Syrjälä wrote:
> On Thu, Apr 21, 2016 at 03:30:09PM +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> Op 18-04-16 om 13:05 schreef Lionel Landwerlin:
>>> Ping?
>>>
>> Will commit, but looks like Ville made a comment about double buffering.
>>
>> Everything in intel_pipe_update_start depends on double buffering, so if the lut isn't then it probably has to be done either before or afterwards. Not sure which, either way would produce some flickering. :(
> It should be done from a vblank worker. Unfortunately we still don't
> have those, so either before or after is fine, and it'll result in
> exactly the same amount of flickering as the current code.
>
Should I take from this that the patch is fine for now as we don't have 
vblank workers yet?
I can resend another version with a TODO comment.

Thanks,

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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-04 10:25       ` Lionel Landwerlin
@ 2016-05-04 10:30         ` Maarten Lankhorst
  2016-05-04 11:41         ` Ville Syrjälä
  1 sibling, 0 replies; 24+ messages in thread
From: Maarten Lankhorst @ 2016-05-04 10:30 UTC (permalink / raw)
  To: Lionel Landwerlin, Ville Syrjälä
  Cc: Jani Nikula, Daniel Vetter, intel-gfx

Op 04-05-16 om 12:25 schreef Lionel Landwerlin:
> On 21/04/16 14:57, Ville Syrjälä wrote:
>> On Thu, Apr 21, 2016 at 03:30:09PM +0200, Maarten Lankhorst wrote:
>>> Hey,
>>>
>>> Op 18-04-16 om 13:05 schreef Lionel Landwerlin:
>>>> Ping?
>>>>
>>> Will commit, but looks like Ville made a comment about double buffering.
>>>
>>> Everything in intel_pipe_update_start depends on double buffering, so if the lut isn't then it probably has to be done either before or afterwards. Not sure which, either way would produce some flickering. :(
>> It should be done from a vblank worker. Unfortunately we still don't
>> have those, so either before or after is fine, and it'll result in
>> exactly the same amount of flickering as the current code.
>>
> Should I take from this that the patch is fine for now as we don't have vblank workers yet?
> I can resend another version with a TODO comment. 
Ok, but make sure all CI tests pass this time. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-04 10:25       ` Lionel Landwerlin
  2016-05-04 10:30         ` Maarten Lankhorst
@ 2016-05-04 11:41         ` Ville Syrjälä
  1 sibling, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2016-05-04 11:41 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Wed, May 04, 2016 at 11:25:12AM +0100, Lionel Landwerlin wrote:
> On 21/04/16 14:57, Ville Syrjälä wrote:
> > On Thu, Apr 21, 2016 at 03:30:09PM +0200, Maarten Lankhorst wrote:
> >> Hey,
> >>
> >> Op 18-04-16 om 13:05 schreef Lionel Landwerlin:
> >>> Ping?
> >>>
> >> Will commit, but looks like Ville made a comment about double buffering.
> >>
> >> Everything in intel_pipe_update_start depends on double buffering, so if the lut isn't then it probably has to be done either before or afterwards. Not sure which, either way would produce some flickering. :(
> > It should be done from a vblank worker. Unfortunately we still don't
> > have those, so either before or after is fine, and it'll result in
> > exactly the same amount of flickering as the current code.
> >
> Should I take from this that the patch is fine for now as we don't have 
> vblank workers yet?

It should be moved out regardless as having it in the critical section
has only downsides, no upsides. We do want to keep the CSC in the
critical section since it's double buffered (at least on PCH platforms,
not so sure about VLV/CHV).

-- 
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] 24+ messages in thread

* [PATCH] drm/i915: add missing condition for committing planes on crtc
@ 2016-05-25 13:30 Lionel Landwerlin
  0 siblings, 0 replies; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-25 13:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

The i915 driver checks for color management properties changes as part
of a plane update. Therefore a color management update must imply a
plane update, otherwise we never update the transformation matrixes
and degamma/gamma LUTs.

v2: add comment about moving the commit of color management registers
    to an async worker

v3: Commit color management register right after vblank

v4: Move back color management commit condition together with planes
    commit

v5: Trigger color management commit through the planes commit (Daniel)

v6: Make plane change update more readable

Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9ccd766..42348d4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12043,6 +12043,12 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 		ret = intel_color_check(crtc, crtc_state);
 		if (ret)
 			return ret;
+
+		/*
+		 * Changing color management on Intel hardware is
+		 * handled as part of planes update.
+		 */
+		crtc_state->planes_changed = true;
 	}
 
 	ret = 0;
-- 
2.8.1

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

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

* [PATCH] drm/i915: add missing condition for committing planes on crtc
@ 2016-05-25 13:20 Lionel Landwerlin
  0 siblings, 0 replies; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-25 13:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

The i915 driver checks for color management properties changes as part
of a plane update. Therefore a color management update must imply a
plane update, otherwise we never update the transformation matrixes
and degamma/gamma LUTs.

v2: add comment about moving the commit of color management registers
    to an async worker

v3: Commit color management register right after vblank

v4: Move back color management commit condition together with planes
    commit

v5: Trigger color management commit through the planes commit (Daniel)

Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 1b3f974..54a9a0d 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -509,7 +509,7 @@ int intel_color_check(struct drm_crtc *crtc,
 	     crtc_state->degamma_lut->length == degamma_length) &&
 	    (!crtc_state->gamma_lut ||
 	     crtc_state->gamma_lut->length == gamma_length))
-		return 0;
+		goto success;
 
 	/*
 	 * We also allow no degamma lut and a gamma lut at the legacy
@@ -518,9 +518,19 @@ int intel_color_check(struct drm_crtc *crtc,
 	if (!crtc_state->degamma_lut &&
 	    crtc_state->gamma_lut &&
 	    crtc_state->gamma_lut->length == LEGACY_LUT_LENGTH)
-		return 0;
+		goto success;
 
 	return -EINVAL;
+
+ success:
+
+	/*
+	 * Changing color management on Intel hardware is handled as part of
+	 * planes update.
+	 */
+	crtc_state->planes_changed = true;
+
+	return 0;
 }
 
 void intel_color_init(struct drm_crtc *crtc)
-- 
2.8.1

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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-12 10:31   ` Lionel Landwerlin
@ 2016-05-17  7:28     ` Daniel Vetter
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Vetter @ 2016-05-17  7:28 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Thu, May 12, 2016 at 11:31:16AM +0100, Lionel Landwerlin wrote:
> Hey Daniel,
> 
> Sorry to nag again. Could you cherry pick this?

In some earlier review (seems to have been lost, at least you didn't
reply) I suggested you just set planes_changed when computing color mgm
differences. That mail contained some lengthy explaination for why I think
that is the better idea.

Can you pls take a look and then decide?

Thanks, Daniel

> 
> Thanks!
> 
> -
> Lionel
> 
> On 11/05/16 11:51, Maarten Lankhorst wrote:
> >Op 09-05-16 om 16:40 schreef Lionel Landwerlin:
> >>We are currently missing the color management update condition to
> >>commit planes on crtc.
> >>
> >>v2: add comment about moving the commit of color management registers
> >>     to an async worker
> >>
> >>v3: Commit color management register right after vblank
> >>
> >>v4: Move back color management commit condition together with planes
> >>     commit
> >>
> >>Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> >>Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_display.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index 3c6b7b9..b63f33d 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -13669,7 +13669,9 @@ static int intel_atomic_commit(struct drm_device *dev,
> >>  			intel_fbc_enable(intel_crtc);
> >>  		if (crtc->state->active &&
> >>-		    (crtc->state->planes_changed || update_pipe))
> >>+		    (crtc->state->planes_changed ||
> >>+		     crtc->state->color_mgmt_changed ||
> >>+		     update_pipe))
> >>  			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
> >>  		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> >Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >
> >
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-11 10:51 ` Maarten Lankhorst
@ 2016-05-12 10:31   ` Lionel Landwerlin
  2016-05-17  7:28     ` Daniel Vetter
  0 siblings, 1 reply; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-12 10:31 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

Hey Daniel,

Sorry to nag again. Could you cherry pick this?

Thanks!

-
Lionel

On 11/05/16 11:51, Maarten Lankhorst wrote:
> Op 09-05-16 om 16:40 schreef Lionel Landwerlin:
>> We are currently missing the color management update condition to
>> commit planes on crtc.
>>
>> v2: add comment about moving the commit of color management registers
>>      to an async worker
>>
>> v3: Commit color management register right after vblank
>>
>> v4: Move back color management commit condition together with planes
>>      commit
>>
>> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3c6b7b9..b63f33d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13669,7 +13669,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>>   			intel_fbc_enable(intel_crtc);
>>   
>>   		if (crtc->state->active &&
>> -		    (crtc->state->planes_changed || update_pipe))
>> +		    (crtc->state->planes_changed ||
>> +		     crtc->state->color_mgmt_changed ||
>> +		     update_pipe))
>>   			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
>>   
>>   		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
>

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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-09 14:40 Lionel Landwerlin
@ 2016-05-11 10:51 ` Maarten Lankhorst
  2016-05-12 10:31   ` Lionel Landwerlin
  0 siblings, 1 reply; 24+ messages in thread
From: Maarten Lankhorst @ 2016-05-11 10:51 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Op 09-05-16 om 16:40 schreef Lionel Landwerlin:
> We are currently missing the color management update condition to
> commit planes on crtc.
>
> v2: add comment about moving the commit of color management registers
>     to an async worker
>
> v3: Commit color management register right after vblank
>
> v4: Move back color management commit condition together with planes
>     commit
>
> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3c6b7b9..b63f33d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13669,7 +13669,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			intel_fbc_enable(intel_crtc);
>  
>  		if (crtc->state->active &&
> -		    (crtc->state->planes_changed || update_pipe))
> +		    (crtc->state->planes_changed ||
> +		     crtc->state->color_mgmt_changed ||
> +		     update_pipe))
>  			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
>  
>  		if (pipe_config->base.active && needs_vblank_wait(pipe_config))

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

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

* [PATCH] drm/i915: add missing condition for committing planes on crtc
@ 2016-05-09 14:40 Lionel Landwerlin
  2016-05-11 10:51 ` Maarten Lankhorst
  0 siblings, 1 reply; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-09 14:40 UTC (permalink / raw)
  To: intel-gfx

We are currently missing the color management update condition to
commit planes on crtc.

v2: add comment about moving the commit of color management registers
    to an async worker

v3: Commit color management register right after vblank

v4: Move back color management commit condition together with planes
    commit

Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3c6b7b9..b63f33d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13669,7 +13669,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 			intel_fbc_enable(intel_crtc);
 
 		if (crtc->state->active &&
-		    (crtc->state->planes_changed || update_pipe))
+		    (crtc->state->planes_changed ||
+		     crtc->state->color_mgmt_changed ||
+		     update_pipe))
 			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
 
 		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
-- 
2.8.0.rc3.226.g39d4020

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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-06 10:52     ` Ville Syrjälä
@ 2016-05-09  7:41       ` Daniel Vetter
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Vetter @ 2016-05-09  7:41 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Fri, May 06, 2016 at 01:52:58PM +0300, Ville Syrjälä wrote:
> On Thu, May 05, 2016 at 03:04:54PM +0100, Lionel Landwerlin wrote:
> > On 04/05/16 15:30, Ville Syrjälä wrote:
> > > On Wed, May 04, 2016 at 02:40:34PM +0100, Lionel Landwerlin wrote:
> > >> We are currently missing the color management update condition to
> > >> commit planes on crtc.
> > >>
> > >> v2: add comment about moving the commit of color management registers
> > >>      to an async worker
> > >>
> > >> v3: Commit color management register right after vblank
> > >>
> > >> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> > >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > >> Cc: Jani Nikula <jani.nikula@intel.com>
> > >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/i915/intel_display.c | 5 +++++
> > >>   1 file changed, 5 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >> index 45c218d..c6acfe5 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -13688,6 +13688,11 @@ static int intel_atomic_commit(struct drm_device *dev,
> > >>   
> > >>   		if (dev_priv->display.optimize_watermarks)
> > >>   			dev_priv->display.optimize_watermarks(intel_cstate);
> > >> +
> > >> +		if (crtc->state->color_mgmt_changed) {
> > >> +			intel_color_set_csc(crtc->state);
> > > As I said earlier, csc shouldn't be here, at least on pch
> > > platforms. And someone should actually double check whether
> > > vlv/chv have double buffered csc registers or not. Oh and
> > > with these frankensocs the double buffering scheme used
> > > (if any) might be totally crazy, like it is for the pipe B
> > > primary plane scaler on chv. A fact which the spec fails
> > > to explain IIRC. So I'd recommend poking at the hardware
> > > to figure out how it actually works.
> > 
> > Where would you put this for pch platforms?
> 
> Where you tried to put it originally.
> 
> > If this patch is wrong the surely the content of intel_begin_crtc_commit 
> > is too right?
> 
> Nope, looks correct to me. Everything in there is double buffered
> AFAICS.
> 
> > 
> > I'm also struggling to understand why the double buffering of the CSC 
> > registers matters.
> > Most people will want to configure this at the same time they configure 
> > the gamma/degamma
> > LUTs to achieve color management and if the LUTs aren't double buffered 
> > then why is it
> > relevant for the CSC?
> 
> We want the update to be atomic, for every piece of hardware affecting
> the ouptut of the pipe. Otherwise it's going to look like crap.
> 
> For registers that are double buffered on vblank the atomicity can be
> achieved by arming the updates for everything right after evading
> the vblank.
> 
> For single buffered registers we'd need to write the registers during
> vblank. But since we suck and can't do that, simply writing them
> somewhere is the best we can do.

I really need to unlazy and type in my generic vblank workers ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-04 11:13 Lionel Landwerlin
@ 2016-05-09  7:37 ` Daniel Vetter
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Vetter @ 2016-05-09  7:37 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Wed, May 04, 2016 at 12:13:39PM +0100, Lionel Landwerlin wrote:
> We are currently missing the color management update condition to
> commit planes on crtc.
> 
> v2: add comment about moving the commit of color management registers
>     to an async worker
> 
> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45c218d..4936743 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13544,6 +13544,15 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
>  	return false;
>  }
>  
> +static bool needs_commit_planes_on_crtc(struct drm_crtc_state *crtc_state)
> +{
> +	/* TODO: drop the color management condition once committing
> +	 * those registers has been moved to an async worker. */
> +	return (crtc_state->planes_changed ||
> +		crtc_state->color_mgmt_changed ||
> +		to_intel_crtc_state(crtc_state)->update_pipe);
> +}

I think it'd be cleaner (as in related logic pulled together) and more in
spirit of the atomic framework if you set planes_changed when you notice
that color mgm state needs an update. That makes it clear that on intel hw
we do such changes with a plain flip (well later on maybe we need a bool
for the vblank worker). Other hw might need a full modeset.

Adding more conditions like this to the commit code makes it imo really
fragile and hard to understand. At least imo computing how to commit state
is best placed next to when we compute what changed.

All the *_changed variables computed by the helpers/core are meant to be
changeable by the driver (assuming it knows what it's doing), and in
general can be upgraded to true without causing any trouble (except maybe
running some code that doesn't necessarily need to be run).
-Daniel

> +
>  /**
>   * intel_atomic_commit - commit validated state object
>   * @dev: DRM device
> @@ -13649,7 +13658,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		bool modeset = needs_modeset(crtc->state);
>  		struct intel_crtc_state *pipe_config =
>  			to_intel_crtc_state(crtc->state);
> -		bool update_pipe = !modeset && pipe_config->update_pipe;
>  
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
> @@ -13663,8 +13671,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		    drm_atomic_get_existing_plane_state(state, crtc->primary))
>  			intel_fbc_enable(intel_crtc);
>  
> -		if (crtc->state->active &&
> -		    (crtc->state->planes_changed || update_pipe))
> +		if (crtc->state->active && !modeset &&
> +		    needs_commit_planes_on_crtc(crtc->state))
>  			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
>  
>  		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> @@ -13974,6 +13982,8 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
>  	if (modeset)
>  		return;
>  
> +	/* TODO: Color management registers commit should be moved to
> +	 * an async worker when we have them. */
>  	if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) {
>  		intel_color_set_csc(crtc->state);
>  		intel_color_load_luts(crtc->state);
> -- 
> 2.8.0.rc3.226.g39d4020
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-05 14:04   ` Lionel Landwerlin
@ 2016-05-06 10:52     ` Ville Syrjälä
  2016-05-09  7:41       ` Daniel Vetter
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2016-05-06 10:52 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Thu, May 05, 2016 at 03:04:54PM +0100, Lionel Landwerlin wrote:
> On 04/05/16 15:30, Ville Syrjälä wrote:
> > On Wed, May 04, 2016 at 02:40:34PM +0100, Lionel Landwerlin wrote:
> >> We are currently missing the color management update condition to
> >> commit planes on crtc.
> >>
> >> v2: add comment about moving the commit of color management registers
> >>      to an async worker
> >>
> >> v3: Commit color management register right after vblank
> >>
> >> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_display.c | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 45c218d..c6acfe5 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -13688,6 +13688,11 @@ static int intel_atomic_commit(struct drm_device *dev,
> >>   
> >>   		if (dev_priv->display.optimize_watermarks)
> >>   			dev_priv->display.optimize_watermarks(intel_cstate);
> >> +
> >> +		if (crtc->state->color_mgmt_changed) {
> >> +			intel_color_set_csc(crtc->state);
> > As I said earlier, csc shouldn't be here, at least on pch
> > platforms. And someone should actually double check whether
> > vlv/chv have double buffered csc registers or not. Oh and
> > with these frankensocs the double buffering scheme used
> > (if any) might be totally crazy, like it is for the pipe B
> > primary plane scaler on chv. A fact which the spec fails
> > to explain IIRC. So I'd recommend poking at the hardware
> > to figure out how it actually works.
> 
> Where would you put this for pch platforms?

Where you tried to put it originally.

> If this patch is wrong the surely the content of intel_begin_crtc_commit 
> is too right?

Nope, looks correct to me. Everything in there is double buffered
AFAICS.

> 
> I'm also struggling to understand why the double buffering of the CSC 
> registers matters.
> Most people will want to configure this at the same time they configure 
> the gamma/degamma
> LUTs to achieve color management and if the LUTs aren't double buffered 
> then why is it
> relevant for the CSC?

We want the update to be atomic, for every piece of hardware affecting
the ouptut of the pipe. Otherwise it's going to look like crap.

For registers that are double buffered on vblank the atomicity can be
achieved by arming the updates for everything right after evading
the vblank.

For single buffered registers we'd need to write the registers during
vblank. But since we suck and can't do that, simply writing them
somewhere is the best we can do.

> 
> Thanks for your time :)
> 
> >
> >> +			intel_color_load_luts(crtc->state);
> >> +		}
> >>   	}
> >>   
> >>   	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> >> -- 
> >> 2.8.0.rc3.226.g39d4020
> 

-- 
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] 24+ messages in thread

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-04 14:30 ` Ville Syrjälä
@ 2016-05-05 14:04   ` Lionel Landwerlin
  2016-05-06 10:52     ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-05 14:04 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On 04/05/16 15:30, Ville Syrjälä wrote:
> On Wed, May 04, 2016 at 02:40:34PM +0100, Lionel Landwerlin wrote:
>> We are currently missing the color management update condition to
>> commit planes on crtc.
>>
>> v2: add comment about moving the commit of color management registers
>>      to an async worker
>>
>> v3: Commit color management register right after vblank
>>
>> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 45c218d..c6acfe5 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13688,6 +13688,11 @@ static int intel_atomic_commit(struct drm_device *dev,
>>   
>>   		if (dev_priv->display.optimize_watermarks)
>>   			dev_priv->display.optimize_watermarks(intel_cstate);
>> +
>> +		if (crtc->state->color_mgmt_changed) {
>> +			intel_color_set_csc(crtc->state);
> As I said earlier, csc shouldn't be here, at least on pch
> platforms. And someone should actually double check whether
> vlv/chv have double buffered csc registers or not. Oh and
> with these frankensocs the double buffering scheme used
> (if any) might be totally crazy, like it is for the pipe B
> primary plane scaler on chv. A fact which the spec fails
> to explain IIRC. So I'd recommend poking at the hardware
> to figure out how it actually works.

Where would you put this for pch platforms?
If this patch is wrong the surely the content of intel_begin_crtc_commit 
is too right?

I'm also struggling to understand why the double buffering of the CSC 
registers matters.
Most people will want to configure this at the same time they configure 
the gamma/degamma
LUTs to achieve color management and if the LUTs aren't double buffered 
then why is it
relevant for the CSC?

Thanks for your time :)

>
>> +			intel_color_load_luts(crtc->state);
>> +		}
>>   	}
>>   
>>   	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
>> -- 
>> 2.8.0.rc3.226.g39d4020


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

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

* Re: [PATCH] drm/i915: add missing condition for committing planes on crtc
  2016-05-04 13:40 Lionel Landwerlin
@ 2016-05-04 14:30 ` Ville Syrjälä
  2016-05-05 14:04   ` Lionel Landwerlin
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2016-05-04 14:30 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Wed, May 04, 2016 at 02:40:34PM +0100, Lionel Landwerlin wrote:
> We are currently missing the color management update condition to
> commit planes on crtc.
> 
> v2: add comment about moving the commit of color management registers
>     to an async worker
> 
> v3: Commit color management register right after vblank
> 
> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45c218d..c6acfe5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13688,6 +13688,11 @@ static int intel_atomic_commit(struct drm_device *dev,
>  
>  		if (dev_priv->display.optimize_watermarks)
>  			dev_priv->display.optimize_watermarks(intel_cstate);
> +
> +		if (crtc->state->color_mgmt_changed) {
> +			intel_color_set_csc(crtc->state);

As I said earlier, csc shouldn't be here, at least on pch
platforms. And someone should actually double check whether
vlv/chv have double buffered csc registers or not. Oh and
with these frankensocs the double buffering scheme used 
(if any) might be totally crazy, like it is for the pipe B
primary plane scaler on chv. A fact which the spec fails
to explain IIRC. So I'd recommend poking at the hardware
to figure out how it actually works.

> +			intel_color_load_luts(crtc->state);
> +		}
>  	}
>  
>  	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> -- 
> 2.8.0.rc3.226.g39d4020

-- 
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] 24+ messages in thread

* [PATCH] drm/i915: add missing condition for committing planes on crtc
@ 2016-05-04 13:40 Lionel Landwerlin
  2016-05-04 14:30 ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-04 13:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter

We are currently missing the color management update condition to
commit planes on crtc.

v2: add comment about moving the commit of color management registers
    to an async worker

v3: Commit color management register right after vblank

Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45c218d..c6acfe5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13688,6 +13688,11 @@ static int intel_atomic_commit(struct drm_device *dev,
 
 		if (dev_priv->display.optimize_watermarks)
 			dev_priv->display.optimize_watermarks(intel_cstate);
+
+		if (crtc->state->color_mgmt_changed) {
+			intel_color_set_csc(crtc->state);
+			intel_color_load_luts(crtc->state);
+		}
 	}
 
 	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
-- 
2.8.0.rc3.226.g39d4020

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

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

* [PATCH] drm/i915: add missing condition for committing planes on crtc
@ 2016-05-04 11:13 Lionel Landwerlin
  2016-05-09  7:37 ` Daniel Vetter
  0 siblings, 1 reply; 24+ messages in thread
From: Lionel Landwerlin @ 2016-05-04 11:13 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter

We are currently missing the color management update condition to
commit planes on crtc.

v2: add comment about moving the commit of color management registers
    to an async worker

Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45c218d..4936743 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13544,6 +13544,15 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
 	return false;
 }
 
+static bool needs_commit_planes_on_crtc(struct drm_crtc_state *crtc_state)
+{
+	/* TODO: drop the color management condition once committing
+	 * those registers has been moved to an async worker. */
+	return (crtc_state->planes_changed ||
+		crtc_state->color_mgmt_changed ||
+		to_intel_crtc_state(crtc_state)->update_pipe);
+}
+
 /**
  * intel_atomic_commit - commit validated state object
  * @dev: DRM device
@@ -13649,7 +13658,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 		bool modeset = needs_modeset(crtc->state);
 		struct intel_crtc_state *pipe_config =
 			to_intel_crtc_state(crtc->state);
-		bool update_pipe = !modeset && pipe_config->update_pipe;
 
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
@@ -13663,8 +13671,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		    drm_atomic_get_existing_plane_state(state, crtc->primary))
 			intel_fbc_enable(intel_crtc);
 
-		if (crtc->state->active &&
-		    (crtc->state->planes_changed || update_pipe))
+		if (crtc->state->active && !modeset &&
+		    needs_commit_planes_on_crtc(crtc->state))
 			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
 
 		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
@@ -13974,6 +13982,8 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 	if (modeset)
 		return;
 
+	/* TODO: Color management registers commit should be moved to
+	 * an async worker when we have them. */
 	if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) {
 		intel_color_set_csc(crtc->state);
 		intel_color_load_luts(crtc->state);
-- 
2.8.0.rc3.226.g39d4020

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

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

end of thread, other threads:[~2016-05-25 13:31 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-08 16:30 [PATCH] drm/i915: add missing condition for committing planes on crtc Lionel Landwerlin
2016-04-09  6:26 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-04-21 13:34   ` Maarten Lankhorst
2016-04-18 11:05 ` [PATCH] " Lionel Landwerlin
2016-04-21 13:30   ` Maarten Lankhorst
2016-04-21 13:57     ` Ville Syrjälä
2016-05-04 10:25       ` Lionel Landwerlin
2016-05-04 10:30         ` Maarten Lankhorst
2016-05-04 11:41         ` Ville Syrjälä
2016-04-18 11:06 ` Maarten Lankhorst
2016-04-18 11:28 ` Ville Syrjälä
2016-05-04 11:13 Lionel Landwerlin
2016-05-09  7:37 ` Daniel Vetter
2016-05-04 13:40 Lionel Landwerlin
2016-05-04 14:30 ` Ville Syrjälä
2016-05-05 14:04   ` Lionel Landwerlin
2016-05-06 10:52     ` Ville Syrjälä
2016-05-09  7:41       ` Daniel Vetter
2016-05-09 14:40 Lionel Landwerlin
2016-05-11 10:51 ` Maarten Lankhorst
2016-05-12 10:31   ` Lionel Landwerlin
2016-05-17  7:28     ` Daniel Vetter
2016-05-25 13:20 Lionel Landwerlin
2016-05-25 13:30 Lionel Landwerlin

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.