[PATCH] drm/i915: add missing condition for committing planes on crtc

[PATCH] drm/i915: add missing condition for committing planes on crtc

[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))
-- 
2.8.0.rc3.226.g39d4020

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Maarten Lankhorst]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Lionel Landwerlin]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Daniel Vetter]

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

[PATCH] drm/i915: add missing condition for committing planes on crtc

[Lionel Landwerlin]

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

[PATCH] drm/i915: add missing condition for committing planes on crtc

[Lionel Landwerlin]

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

[PATCH] drm/i915: add missing condition for committing planes on crtc

[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

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Ville Syrjälä]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Lionel Landwerlin]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Ville Syrjälä]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Daniel Vetter]

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

[PATCH] drm/i915: add missing condition for committing planes on crtc

[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

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Daniel Vetter]

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

[PATCH] drm/i915: add missing condition for committing planes on crtc

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Lionel Landwerlin]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Maarten Lankhorst]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Ville Syrjälä]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

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

Thanks,

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Maarten Lankhorst]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Ville Syrjälä]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Maarten Lankhorst]

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

Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

[Ville Syrjälä]

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