[PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Until recently vlv_steal_power_sequencer() wasn't being called for
normal DP ports, and hence it could assert that it should only be
called for pipe A and B (since pipe C doesn't support eDP). However
that changed when we started to consider normal DP ports as well when
choosing a PPS. So we will now get spurious warnings when
vlv_steal_power_sequencer() does get called for pipe C. Avoid this by
moving the WARN down into vlv_detach_power_sequencer() where this
assertion should still hold.

Cc: Imre Deak <imre.deak@intel.com>
Cc: stable@vger.kernel.org
Fixes: 9f2bdb006a7e ("drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV")
References: https://bugs.freedesktop.org/show_bug.cgi?id=95287
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0f14e97e519b..2395046e19ce 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2905,6 +2905,9 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
 
 	WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
 
+	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
+		return;
+
 	edp_panel_vdd_off_sync(intel_dp);
 
 	/*
@@ -2932,9 +2935,6 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
-	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
-		return;
-
 	for_each_intel_encoder(dev, encoder) {
 		struct intel_dp *intel_dp;
 		enum port port;
-- 
2.10.2

[PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Until recently vlv_steal_power_sequencer() wasn't being called for
normal DP ports, and hence it could assert that it should only be
called for pipe A and B (since pipe C doesn't support eDP). However
that changed when we started to consider normal DP ports as well when
choosing a PPS. So we will now get spurious warnings when
vlv_steal_power_sequencer() does get called for pipe C. Avoid this by
moving the WARN down into vlv_detach_power_sequencer() where this
assertion should still hold.

Cc: Imre Deak <imre.deak@intel.com>
Cc: stable@vger.kernel.org
Fixes: 9f2bdb006a7e ("drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV")
References: https://bugs.freedesktop.org/show_bug.cgi?id=95287
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0f14e97e519b..2395046e19ce 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2905,6 +2905,9 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
 
 	WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
 
+	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
+		return;
+
 	edp_panel_vdd_off_sync(intel_dp);
 
 	/*
@@ -2932,9 +2935,6 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
-	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
-		return;
-
 	for_each_intel_encoder(dev, encoder) {
 		struct intel_dp *intel_dp;
 		enum port port;
-- 
2.10.2

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

✗ Fi.CI.BAT: failure for drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[Patchwork]

== Series Details ==

Series: drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code
URL   : https://patchwork.freedesktop.org/series/19329/
State : failure

== Summary ==

Series 19329v1 drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code
https://patchwork.freedesktop.org/api/1.0/series/19329/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-skl-6260u)

fi-bdw-5557u     total:252  pass:238  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:230  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:48   pass:41   dwarn:0   dfail:0   fail:0   skip:6  
fi-hsw-4770r     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:252  pass:199  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:252  pass:229  dwarn:0   dfail:0   fail:2   skip:21 
fi-skl-6260u     total:211  pass:200  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:232  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:252  pass:227  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:252  pass:239  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:252  pass:220  dwarn:0   dfail:0   fail:0   skip:32 

2699547830db3e698485f452e29278c09f084397 drm-tip: 2017y-02m-08d-18h-25m-45s UTC integration manifest
c703c77 drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3741/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[Imre Deak]

On Wed, Feb 08, 2017 at 07:52:54PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> 
> Until recently vlv_steal_power_sequencer() wasn't being called for
> normal DP ports, and hence it could assert that it should only be
> called for pipe A and B (since pipe C doesn't support eDP). However
> that changed when we started to consider normal DP ports as well when
> choosing a PPS. So we will now get spurious warnings when
> vlv_steal_power_sequencer() does get called for pipe C. Avoid this by
> moving the WARN down into vlv_detach_power_sequencer() where this
> assertion should still hold.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: stable@vger.kernel.org
> Fixes: 9f2bdb006a7e ("drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV")
> References: https://bugs.freedesktop.org/show_bug.cgi?id=95287
> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0f14e97e519b..2395046e19ce 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2905,6 +2905,9 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
>  
>  	WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
>  
> +	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> +		return;
> +

Yep, missed this:
Reviewed-by: Imre Deak <imre.deak@intel.com>

So the above is called only for eDP, checking for that too would make
things clearer imo.

>  	edp_panel_vdd_off_sync(intel_dp);
>  
>  	/*
> @@ -2932,9 +2935,6 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> -		return;
> -
>  	for_each_intel_encoder(dev, encoder) {
>  		struct intel_dp *intel_dp;
>  		enum port port;
> -- 
> 2.10.2
> 

Re: [PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[Imre Deak]

On Wed, Feb 08, 2017 at 07:52:54PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Until recently vlv_steal_power_sequencer() wasn't being called for
> normal DP ports, and hence it could assert that it should only be
> called for pipe A and B (since pipe C doesn't support eDP). However
> that changed when we started to consider normal DP ports as well when
> choosing a PPS. So we will now get spurious warnings when
> vlv_steal_power_sequencer() does get called for pipe C. Avoid this by
> moving the WARN down into vlv_detach_power_sequencer() where this
> assertion should still hold.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: stable@vger.kernel.org
> Fixes: 9f2bdb006a7e ("drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV")
> References: https://bugs.freedesktop.org/show_bug.cgi?id=95287
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0f14e97e519b..2395046e19ce 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2905,6 +2905,9 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
>  
>  	WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
>  
> +	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> +		return;
> +

Yep, missed this:
Reviewed-by: Imre Deak <imre.deak@intel.com>

So the above is called only for eDP, checking for that too would make
things clearer imo.

>  	edp_panel_vdd_off_sync(intel_dp);
>  
>  	/*
> @@ -2932,9 +2935,6 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> -		return;
> -
>  	for_each_intel_encoder(dev, encoder) {
>  		struct intel_dp *intel_dp;
>  		enum port port;
> -- 
> 2.10.2
> 

✓ Fi.CI.BAT: success for drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[Patchwork]

== Series Details ==

Series: drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code
URL   : https://patchwork.freedesktop.org/series/19329/
State : success

== Summary ==

Series 19329v1 drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code
https://patchwork.freedesktop.org/api/1.0/series/19329/revisions/1/mbox/


fi-bdw-5557u     total:252  pass:238  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:230  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:252  pass:199  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:252  pass:229  dwarn:0   dfail:0   fail:2   skip:21 
fi-skl-6260u     total:252  pass:239  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:252  pass:232  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:252  pass:227  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:252  pass:239  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:252  pass:220  dwarn:0   dfail:0   fail:0   skip:32 

8ca0c2e06c85344b8fee2bd5c48d6f3571fc870a drm-tip: 2017y-02m-09d-13h-35m-52s UTC integration manifest
cdecc32 drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3759/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[Ville Syrjälä]

On Thu, Feb 09, 2017 at 12:29:04PM +0200, Imre Deak wrote:
> On Wed, Feb 08, 2017 at 07:52:54PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > 
> > Until recently vlv_steal_power_sequencer() wasn't being called for
> > normal DP ports, and hence it could assert that it should only be
> > called for pipe A and B (since pipe C doesn't support eDP). However
> > that changed when we started to consider normal DP ports as well when
> > choosing a PPS. So we will now get spurious warnings when
> > vlv_steal_power_sequencer() does get called for pipe C. Avoid this by
> > moving the WARN down into vlv_detach_power_sequencer() where this
> > assertion should still hold.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: stable@vger.kernel.org
> > Fixes: 9f2bdb006a7e ("drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV")
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=95287
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 0f14e97e519b..2395046e19ce 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2905,6 +2905,9 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
> >  
> >  	WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >  
> > +	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > +		return;
> > +
> 
> Yep, missed this:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> So the above is called only for eDP, checking for that too would make
> things clearer imo.

We should have some is_edp() warns in the even lower level stuff,
I think. Not sure if we should have moved the pipe check even lower as
well. Oh well, at least this should shut up the spurious warns.

Pushed to dinq, thanks for the review.

> 
> >  	edp_panel_vdd_off_sync(intel_dp);
> >  
> >  	/*
> > @@ -2932,9 +2935,6 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
> >  
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> >  
> > -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > -		return;
> > -
> >  	for_each_intel_encoder(dev, encoder) {
> >  		struct intel_dp *intel_dp;
> >  		enum port port;
> > -- 
> > 2.10.2
> > 

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

[Ville Syrjälä]

On Thu, Feb 09, 2017 at 12:29:04PM +0200, Imre Deak wrote:
> On Wed, Feb 08, 2017 at 07:52:54PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Until recently vlv_steal_power_sequencer() wasn't being called for
> > normal DP ports, and hence it could assert that it should only be
> > called for pipe A and B (since pipe C doesn't support eDP). However
> > that changed when we started to consider normal DP ports as well when
> > choosing a PPS. So we will now get spurious warnings when
> > vlv_steal_power_sequencer() does get called for pipe C. Avoid this by
> > moving the WARN down into vlv_detach_power_sequencer() where this
> > assertion should still hold.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: stable@vger.kernel.org
> > Fixes: 9f2bdb006a7e ("drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV")
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=95287
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 0f14e97e519b..2395046e19ce 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2905,6 +2905,9 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
> >  
> >  	WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >  
> > +	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > +		return;
> > +
> 
> Yep, missed this:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> So the above is called only for eDP, checking for that too would make
> things clearer imo.

We should have some is_edp() warns in the even lower level stuff,
I think. Not sure if we should have moved the pipe check even lower as
well. Oh well, at least this should shut up the spurious warns.

Pushed to dinq, thanks for the review.

> 
> >  	edp_panel_vdd_off_sync(intel_dp);
> >  
> >  	/*
> > @@ -2932,9 +2935,6 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
> >  
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> >  
> > -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > -		return;
> > -
> >  	for_each_intel_encoder(dev, encoder) {
> >  		struct intel_dp *intel_dp;
> >  		enum port port;
> > -- 
> > 2.10.2
> > 

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