[PATCH 0/3] Fix cxsr warning spew.

[PATCH 0/3] Fix cxsr warning spew.

[Maarten Lankhorst]

First 2 patches from the remove intel_crtc->atomic series. They were already
reviewed when part of a different series which I had slightly adjusted.
I put them in the series because I need to pass old_crtc_state
to pre_plane_update, and those patches were reviewed anyway.

Maarten Lankhorst (3):
  drm/i915: Remove intel_crtc->atomic.disable_ips.
  drm/i915: Remove atomic.pre_disable_primary.
  drm/i915: Do not disable cxsr when crtc is disabled.

 drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 2 files changed, 21 insertions(+), 23 deletions(-)

-- 
2.1.0

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

[PATCH 1/3] drm/i915: Remove intel_crtc->atomic.disable_ips.

[Maarten Lankhorst]

This is a revert of commit 066cf55b9ce3 "drm/i915: Fix IPS related flicker".
intel_pre_disable_primary already handles this, and now everything
goes through the atomic path there's no need to try to disable ips twice.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 ---------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 2 files changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4d8c9f7857db..de94ef165fbe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4818,9 +4818,6 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
 	if (atomic->update_fbc)
 		intel_fbc_pre_update(crtc);
 
-	if (crtc->atomic.disable_ips)
-		hsw_disable_ips(crtc);
-
 	if (atomic->pre_disable_primary)
 		intel_pre_disable_primary(&crtc->base);
 
@@ -11890,18 +11887,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		intel_crtc->atomic.post_enable_primary = turn_on;
 		intel_crtc->atomic.update_fbc = true;
 
-		if (turn_off) {
-			/*
-			 * FIXME: Actually if we will still have any other
-			 * plane enabled on the pipe we could let IPS enabled
-			 * still, but for now lets consider that when we make
-			 * primary invisible by setting DSPCNTR to 0 on
-			 * update_primary_plane function IPS needs to be
-			 * disable.
-			 */
-			intel_crtc->atomic.disable_ips = true;
-		}
-
 		/*
 		 * BDW signals flip done immediately if the plane
 		 * is disabled, even if the plane enable is already
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 93ba14a3bb76..c9ae2be260d0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -544,7 +544,6 @@ struct intel_mmio_flip {
  */
 struct intel_crtc_atomic_commit {
 	/* Sleepable operations to perform before commit */
-	bool disable_ips;
 	bool pre_disable_primary;
 
 	/* Sleepable operations to perform after commit */
-- 
2.1.0

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

[PATCH 2/3] drm/i915: Remove atomic.pre_disable_primary.

[Maarten Lankhorst]

This can be derived from the atomic state in pre_plane_update,
which makes it more clear when it's supposed to be called.

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de94ef165fbe..c7f2a18ab34e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4807,19 +4807,33 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
 	memset(atomic, 0, sizeof(*atomic));
 }
 
-static void intel_pre_plane_update(struct intel_crtc *crtc)
+static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
 	struct intel_crtc_state *pipe_config =
 		to_intel_crtc_state(crtc->base.state);
+	struct drm_atomic_state *old_state = old_crtc_state->base.state;
+	struct drm_plane *primary = crtc->base.primary;
+	struct drm_plane_state *old_pri_state =
+		drm_atomic_get_existing_plane_state(old_state, primary);
+	bool modeset = needs_modeset(&pipe_config->base);
 
 	if (atomic->update_fbc)
 		intel_fbc_pre_update(crtc);
 
-	if (atomic->pre_disable_primary)
-		intel_pre_disable_primary(&crtc->base);
+	if (old_pri_state) {
+		struct intel_plane_state *primary_state =
+			to_intel_plane_state(primary->state);
+		struct intel_plane_state *old_primary_state =
+			to_intel_plane_state(old_pri_state);
+
+		if (old_primary_state->visible &&
+		    (modeset || !primary_state->visible))
+			intel_pre_disable_primary(&crtc->base);
+	}
 
 	if (pipe_config->disable_cxsr) {
 		crtc->wm.cxsr_allowed = false;
@@ -11883,7 +11897,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 
 	switch (plane->type) {
 	case DRM_PLANE_TYPE_PRIMARY:
-		intel_crtc->atomic.pre_disable_primary = turn_off;
 		intel_crtc->atomic.post_enable_primary = turn_on;
 		intel_crtc->atomic.update_fbc = true;
 
@@ -13495,7 +13508,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 		if (!needs_modeset(crtc->state))
 			continue;
 
-		intel_pre_plane_update(intel_crtc);
+		intel_pre_plane_update(to_intel_crtc_state(crtc_state));
 
 		if (crtc_state->active) {
 			intel_crtc_disable_planes(crtc, crtc_state->plane_mask);
@@ -13551,7 +13564,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 		}
 
 		if (!modeset)
-			intel_pre_plane_update(intel_crtc);
+			intel_pre_plane_update(to_intel_crtc_state(crtc_state));
 
 		if (crtc->state->active && intel_crtc->atomic.update_fbc)
 			intel_fbc_enable(intel_crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c9ae2be260d0..167f284ccb16 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -544,7 +544,6 @@ struct intel_mmio_flip {
  */
 struct intel_crtc_atomic_commit {
 	/* Sleepable operations to perform before commit */
-	bool pre_disable_primary;
 
 	/* Sleepable operations to perform after commit */
 	unsigned fb_bits;
-- 
2.1.0

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

Re: [PATCH 2/3] drm/i915: Remove atomic.pre_disable_primary.

[Ville Syrjälä]

On Wed, Feb 03, 2016 at 04:53:24PM +0100, Maarten Lankhorst wrote:
> This can be derived from the atomic state in pre_plane_update,
> which makes it more clear when it's supposed to be called.
> 
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  2 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index de94ef165fbe..c7f2a18ab34e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4807,19 +4807,33 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>  	memset(atomic, 0, sizeof(*atomic));
>  }
>  
> -static void intel_pre_plane_update(struct intel_crtc *crtc)
> +static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
>  	struct intel_crtc_state *pipe_config =
>  		to_intel_crtc_state(crtc->base.state);
> +	struct drm_atomic_state *old_state = old_crtc_state->base.state;
> +	struct drm_plane *primary = crtc->base.primary;
> +	struct drm_plane_state *old_pri_state =
> +		drm_atomic_get_existing_plane_state(old_state, primary);
> +	bool modeset = needs_modeset(&pipe_config->base);
>  
>  	if (atomic->update_fbc)
>  		intel_fbc_pre_update(crtc);
>  
> -	if (atomic->pre_disable_primary)
> -		intel_pre_disable_primary(&crtc->base);
> +	if (old_pri_state) {

When might we not have and old state for the primary plane?

> +		struct intel_plane_state *primary_state =
> +			to_intel_plane_state(primary->state);
> +		struct intel_plane_state *old_primary_state =
> +			to_intel_plane_state(old_pri_state);
> +
> +		if (old_primary_state->visible &&
> +		    (modeset || !primary_state->visible))
> +			intel_pre_disable_primary(&crtc->base);
> +	}
>  
>  	if (pipe_config->disable_cxsr) {
>  		crtc->wm.cxsr_allowed = false;
> @@ -11883,7 +11897,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  
>  	switch (plane->type) {
>  	case DRM_PLANE_TYPE_PRIMARY:
> -		intel_crtc->atomic.pre_disable_primary = turn_off;
>  		intel_crtc->atomic.post_enable_primary = turn_on;
>  		intel_crtc->atomic.update_fbc = true;
>  
> @@ -13495,7 +13508,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		if (!needs_modeset(crtc->state))
>  			continue;
>  
> -		intel_pre_plane_update(intel_crtc);
> +		intel_pre_plane_update(to_intel_crtc_state(crtc_state));
>  
>  		if (crtc_state->active) {
>  			intel_crtc_disable_planes(crtc, crtc_state->plane_mask);
> @@ -13551,7 +13564,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		}
>  
>  		if (!modeset)
> -			intel_pre_plane_update(intel_crtc);
> +			intel_pre_plane_update(to_intel_crtc_state(crtc_state));
>  
>  		if (crtc->state->active && intel_crtc->atomic.update_fbc)
>  			intel_fbc_enable(intel_crtc);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c9ae2be260d0..167f284ccb16 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -544,7 +544,6 @@ struct intel_mmio_flip {
>   */
>  struct intel_crtc_atomic_commit {
>  	/* Sleepable operations to perform before commit */
> -	bool pre_disable_primary;
>  
>  	/* Sleepable operations to perform after commit */
>  	unsigned fb_bits;
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Remove atomic.pre_disable_primary.

[Maarten Lankhorst]

Op 03-02-16 om 17:07 schreef Ville Syrjälä:
> On Wed, Feb 03, 2016 at 04:53:24PM +0100, Maarten Lankhorst wrote:
>> This can be derived from the atomic state in pre_plane_update,
>> which makes it more clear when it's supposed to be called.
>>
>> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++------
>>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>>  2 files changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index de94ef165fbe..c7f2a18ab34e 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4807,19 +4807,33 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>>  	memset(atomic, 0, sizeof(*atomic));
>>  }
>>  
>> -static void intel_pre_plane_update(struct intel_crtc *crtc)
>> +static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
>>  {
>> +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
>>  	struct drm_device *dev = crtc->base.dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
>>  	struct intel_crtc_state *pipe_config =
>>  		to_intel_crtc_state(crtc->base.state);
>> +	struct drm_atomic_state *old_state = old_crtc_state->base.state;
>> +	struct drm_plane *primary = crtc->base.primary;
>> +	struct drm_plane_state *old_pri_state =
>> +		drm_atomic_get_existing_plane_state(old_state, primary);
>> +	bool modeset = needs_modeset(&pipe_config->base);
>>  
>>  	if (atomic->update_fbc)
>>  		intel_fbc_pre_update(crtc);
>>  
>> -	if (atomic->pre_disable_primary)
>> -		intel_pre_disable_primary(&crtc->base);
>> +	if (old_pri_state) {
> When might we not have and old state for the primary plane?
>
When updating some state unrelated to the primary plane, for example changing cursor.

modeset and update_pipe should always add all planes.

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

Re: [PATCH 2/3] drm/i915: Remove atomic.pre_disable_primary.

[Ville Syrjälä]

On Wed, Feb 03, 2016 at 06:23:13PM +0100, Maarten Lankhorst wrote:
> Op 03-02-16 om 17:07 schreef Ville Syrjälä:
> > On Wed, Feb 03, 2016 at 04:53:24PM +0100, Maarten Lankhorst wrote:
> >> This can be derived from the atomic state in pre_plane_update,
> >> which makes it more clear when it's supposed to be called.
> >>
> >> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++------
> >>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
> >>  2 files changed, 19 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index de94ef165fbe..c7f2a18ab34e 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -4807,19 +4807,33 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> >>  	memset(atomic, 0, sizeof(*atomic));
> >>  }
> >>  
> >> -static void intel_pre_plane_update(struct intel_crtc *crtc)
> >> +static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
> >>  {
> >> +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> >>  	struct drm_device *dev = crtc->base.dev;
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  	struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
> >>  	struct intel_crtc_state *pipe_config =
> >>  		to_intel_crtc_state(crtc->base.state);
> >> +	struct drm_atomic_state *old_state = old_crtc_state->base.state;
> >> +	struct drm_plane *primary = crtc->base.primary;
> >> +	struct drm_plane_state *old_pri_state =
> >> +		drm_atomic_get_existing_plane_state(old_state, primary);
> >> +	bool modeset = needs_modeset(&pipe_config->base);
> >>  
> >>  	if (atomic->update_fbc)
> >>  		intel_fbc_pre_update(crtc);
> >>  
> >> -	if (atomic->pre_disable_primary)
> >> -		intel_pre_disable_primary(&crtc->base);
> >> +	if (old_pri_state) {
> > When might we not have and old state for the primary plane?
> >
> When updating some state unrelated to the primary plane, for example changing cursor.

Hmm. I thought we added all the planes to the state currently...
I guess that only happens due to watermark calculation on specific
platforms.

> 
> modeset and update_pipe should always add all planes.
> 
> ~Maarten

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Remove atomic.pre_disable_primary.

[Maarten Lankhorst]

Op 03-02-16 om 18:39 schreef Ville Syrjälä:
> On Wed, Feb 03, 2016 at 06:23:13PM +0100, Maarten Lankhorst wrote:
>> Op 03-02-16 om 17:07 schreef Ville Syrjälä:
>>> On Wed, Feb 03, 2016 at 04:53:24PM +0100, Maarten Lankhorst wrote:
>>>> This can be derived from the atomic state in pre_plane_update,
>>>> which makes it more clear when it's supposed to be called.
>>>>
>>>> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++------
>>>>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>>>>  2 files changed, 19 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index de94ef165fbe..c7f2a18ab34e 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -4807,19 +4807,33 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>>>>  	memset(atomic, 0, sizeof(*atomic));
>>>>  }
>>>>  
>>>> -static void intel_pre_plane_update(struct intel_crtc *crtc)
>>>> +static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
>>>>  {
>>>> +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
>>>>  	struct drm_device *dev = crtc->base.dev;
>>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>>>  	struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
>>>>  	struct intel_crtc_state *pipe_config =
>>>>  		to_intel_crtc_state(crtc->base.state);
>>>> +	struct drm_atomic_state *old_state = old_crtc_state->base.state;
>>>> +	struct drm_plane *primary = crtc->base.primary;
>>>> +	struct drm_plane_state *old_pri_state =
>>>> +		drm_atomic_get_existing_plane_state(old_state, primary);
>>>> +	bool modeset = needs_modeset(&pipe_config->base);
>>>>  
>>>>  	if (atomic->update_fbc)
>>>>  		intel_fbc_pre_update(crtc);
>>>>  
>>>> -	if (atomic->pre_disable_primary)
>>>> -		intel_pre_disable_primary(&crtc->base);
>>>> +	if (old_pri_state) {
>>> When might we not have and old state for the primary plane?
>>>
>> When updating some state unrelated to the primary plane, for example changing cursor.
> Hmm. I thought we added all the planes to the state currently...
> I guess that only happens due to watermark calculation on specific
> platforms.
>
Yes we do and that's a bug. I have a patch to reuse the old values for the planes not part of the state,
I just didn't send it upstream yet.

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

[PATCH 3/3] drm/i915: Do not disable cxsr when crtc is disabled.

[Maarten Lankhorst]

It's safe to assume cxsr is already disabled when the crtc is off.
This prevents an unclaimed register warning when the required power
wells are not enabled.

[  262.864984] ------------[ cut here ]------------
[  262.865025] WARNING: CPU: 1 PID: 6799 at drivers/gpu/drm/i915/intel_uncore.c:638 __unclaimed_reg_debug+0x68/0x80 [i915]()
[  262.865029] Unclaimed register detected before reading register 0x186500
[  262.865032] Modules linked in: i915 intel_powerclamp
[  262.865057] CPU: 1 PID: 6799 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-gfxbench+ #1
[  262.865060] Hardware name: DN2820FYK, BIOS FYBYT10H.86A.0038.2014.0717.1455 07/17/2014
[  262.865064]  ffffffffa0338cf8 ffff88007448ba78 ffffffff813df90c ffff88007448bac0
[  262.865071]  ffff88007448bab0 ffffffff810746e1 0000000000186500 0000000000000001
[  262.865077]  0000000000000001 ffff880074420000 0000000000000000 ffff88007448bb10
[  262.865083] Call Trace:
[  262.865092]  [<ffffffff813df90c>] dump_stack+0x4e/0x82
[  262.865098]  [<ffffffff810746e1>] warn_slowpath_common+0x81/0xc0
[  262.865102]  [<ffffffff81074767>] warn_slowpath_fmt+0x47/0x50
[  262.865128]  [<ffffffffa02a07e8>] __unclaimed_reg_debug+0x68/0x80 [i915]
[  262.865154]  [<ffffffffa02a0e4e>] vlv_read32+0x2de/0x370 [i915]
[  262.865173]  [<ffffffffa0256837>] intel_set_memory_cxsr+0x87/0x1a0 [i915]
[  262.865200]  [<ffffffffa02c4cb3>] intel_pre_plane_update+0xb3/0xf0 [i915]
[  262.865228]  [<ffffffffa02c54b5>] intel_atomic_commit+0x3b5/0x17c0 [i915]
[  262.865234]  [<ffffffff8150dc45>] ? drm_atomic_check_only+0x145/0x660
[  262.865239]  [<ffffffff8150d75a>] ? drm_atomic_set_crtc_for_connector+0x6a/0xe0
[  262.865243]  [<ffffffff8150e192>] drm_atomic_commit+0x32/0x50
[  262.865249]  [<ffffffff814eb155>] drm_atomic_helper_set_config+0x75/0xb0
[  262.865253]  [<ffffffff814fd090>] drm_mode_set_config_internal+0x60/0x110
[  262.865258]  [<ffffffff81501e26>] drm_mode_setcrtc+0x186/0x4f0
[  262.865263]  [<ffffffff814f3eed>] drm_ioctl+0x13d/0x590
[  262.865267]  [<ffffffff81501ca0>] ? drm_mode_setplane+0x1b0/0x1b0
[  262.865273]  [<ffffffff811d4c4c>] do_vfs_ioctl+0x2fc/0x550
[  262.865278]  [<ffffffff8118d5ea>] ? vm_munmap+0x4a/0x60
[  262.865283]  [<ffffffff811e06ba>] ? __fget_light+0x6a/0x90
[  262.865287]  [<ffffffff811d4edc>] SyS_ioctl+0x3c/0x70
[  262.865292]  [<ffffffff8179a75b>] entry_SYSCALL_64_fastpath+0x16/0x73
[  262.865296] ---[ end trace 6387a0ad001bb39f ]---

Testcase: kms_flip.basic-flip-vs-wf_vblank
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93698

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.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 c7f2a18ab34e..4fcbdd6037a4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4837,7 +4837,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
 
 	if (pipe_config->disable_cxsr) {
 		crtc->wm.cxsr_allowed = false;
-		intel_set_memory_cxsr(dev_priv, false);
+
+		if (old_crtc_state->base.active)
+			intel_set_memory_cxsr(dev_priv, false);
 	}
 
 	if (!needs_modeset(&pipe_config->base) && pipe_config->wm_changed)
-- 
2.1.0

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

Re: [PATCH 3/3] drm/i915: Do not disable cxsr when crtc is disabled.

[Ville Syrjälä]

On Wed, Feb 03, 2016 at 04:53:25PM +0100, Maarten Lankhorst wrote:
> It's safe to assume cxsr is already disabled when the crtc is off.

Hmm. Yeah, looks like that's how we set it up currently. I thought we
might leave it on with 0 active pipes, but I guess not.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> This prevents an unclaimed register warning when the required power
> wells are not enabled.
> 
> [  262.864984] ------------[ cut here ]------------
> [  262.865025] WARNING: CPU: 1 PID: 6799 at drivers/gpu/drm/i915/intel_uncore.c:638 __unclaimed_reg_debug+0x68/0x80 [i915]()
> [  262.865029] Unclaimed register detected before reading register 0x186500
> [  262.865032] Modules linked in: i915 intel_powerclamp
> [  262.865057] CPU: 1 PID: 6799 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-gfxbench+ #1
> [  262.865060] Hardware name: DN2820FYK, BIOS FYBYT10H.86A.0038.2014.0717.1455 07/17/2014
> [  262.865064]  ffffffffa0338cf8 ffff88007448ba78 ffffffff813df90c ffff88007448bac0
> [  262.865071]  ffff88007448bab0 ffffffff810746e1 0000000000186500 0000000000000001
> [  262.865077]  0000000000000001 ffff880074420000 0000000000000000 ffff88007448bb10
> [  262.865083] Call Trace:
> [  262.865092]  [<ffffffff813df90c>] dump_stack+0x4e/0x82
> [  262.865098]  [<ffffffff810746e1>] warn_slowpath_common+0x81/0xc0
> [  262.865102]  [<ffffffff81074767>] warn_slowpath_fmt+0x47/0x50
> [  262.865128]  [<ffffffffa02a07e8>] __unclaimed_reg_debug+0x68/0x80 [i915]
> [  262.865154]  [<ffffffffa02a0e4e>] vlv_read32+0x2de/0x370 [i915]
> [  262.865173]  [<ffffffffa0256837>] intel_set_memory_cxsr+0x87/0x1a0 [i915]
> [  262.865200]  [<ffffffffa02c4cb3>] intel_pre_plane_update+0xb3/0xf0 [i915]
> [  262.865228]  [<ffffffffa02c54b5>] intel_atomic_commit+0x3b5/0x17c0 [i915]
> [  262.865234]  [<ffffffff8150dc45>] ? drm_atomic_check_only+0x145/0x660
> [  262.865239]  [<ffffffff8150d75a>] ? drm_atomic_set_crtc_for_connector+0x6a/0xe0
> [  262.865243]  [<ffffffff8150e192>] drm_atomic_commit+0x32/0x50
> [  262.865249]  [<ffffffff814eb155>] drm_atomic_helper_set_config+0x75/0xb0
> [  262.865253]  [<ffffffff814fd090>] drm_mode_set_config_internal+0x60/0x110
> [  262.865258]  [<ffffffff81501e26>] drm_mode_setcrtc+0x186/0x4f0
> [  262.865263]  [<ffffffff814f3eed>] drm_ioctl+0x13d/0x590
> [  262.865267]  [<ffffffff81501ca0>] ? drm_mode_setplane+0x1b0/0x1b0
> [  262.865273]  [<ffffffff811d4c4c>] do_vfs_ioctl+0x2fc/0x550
> [  262.865278]  [<ffffffff8118d5ea>] ? vm_munmap+0x4a/0x60
> [  262.865283]  [<ffffffff811e06ba>] ? __fget_light+0x6a/0x90
> [  262.865287]  [<ffffffff811d4edc>] SyS_ioctl+0x3c/0x70
> [  262.865292]  [<ffffffff8179a75b>] entry_SYSCALL_64_fastpath+0x16/0x73
> [  262.865296] ---[ end trace 6387a0ad001bb39f ]---
> 
> Testcase: kms_flip.basic-flip-vs-wf_vblank
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93698
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.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 c7f2a18ab34e..4fcbdd6037a4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4837,7 +4837,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
>  
>  	if (pipe_config->disable_cxsr) {
>  		crtc->wm.cxsr_allowed = false;
> -		intel_set_memory_cxsr(dev_priv, false);
> +
> +		if (old_crtc_state->base.active)
> +			intel_set_memory_cxsr(dev_priv, false);
>  	}
>  
>  	if (!needs_modeset(&pipe_config->base) && pipe_config->wm_changed)
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Fix cxsr warning spew.

[Patchwork]

== Summary ==

Series 3053v1 Fix cxsr warning spew.
http://patchwork.freedesktop.org/api/1.0/series/3053/revisions/1/mbox/

Test gem_sync:
        Subgroup basic-blt:
                pass       -> INCOMPLETE (skl-i7k-2)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                dmesg-warn -> PASS       (skl-i7k-2)

bdw-nuci7        total:156  pass:147  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:159  pass:147  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:159  pass:131  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc          total:159  pass:136  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox      total:159  pass:146  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:159  pass:149  dwarn:0   dfail:0   fail:0   skip:10 
hsw-xps12        total:156  pass:146  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p      total:159  pass:111  dwarn:0   dfail:0   fail:0   skip:48 
ivb-t430s        total:159  pass:145  dwarn:0   dfail:0   fail:0   skip:14 
skl-i7k-2        total:55   pass:46   dwarn:0   dfail:0   fail:0   skip:8  
snb-dellxps      total:159  pass:137  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:159  pass:137  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1357/

64800473a902f6b9033ed9b519585426cf6affa9 drm-intel-nightly: 2016y-02m-03d-14h-02m-22s UTC integration manifest
3ec0d25c488201adf036b8324710f799970b4cf0 drm/i915: Do not disable cxsr when crtc is disabled.
d1ec8f2590c4119d0e7b4f615050eb47eaa34f7d drm/i915: Remove atomic.pre_disable_primary.
e2ae8b5ac932b8257bc3796d792d45779e1825e5 drm/i915: Remove intel_crtc->atomic.disable_ips.

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