diff mbox

drm/i915: Unify VLV/CHV DPOunit clock gating disable/enable

Message ID 1460996305-30453-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjala April 18, 2016, 4:18 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Check for VLV/CHV instead if !BXT when re-enabling DPOunit clock gating
after DSI disable. That's what we checked when disabling the clock
gating when enabling DSI.

Also use the same temporary variable name in both cases, and toss in a
bit of dev vs. dev_priv cleanup while at it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Jani Nikula April 18, 2016, 5:26 p.m. UTC | #1 
On Mon, 18 Apr 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Check for VLV/CHV instead if !BXT when re-enabling DPOunit clock gating
> after DSI disable. That's what we checked when disabling the clock
> gating when enabling DSI.
>
> Also use the same temporary variable name in both cases, and toss in a
> bit of dev vs. dev_priv cleanup while at it.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 34328ddaaab5..599045359538 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -516,7 +516,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>  	enum port port;
> -	u32 tmp;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> @@ -535,11 +534,13 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  
>  	msleep(intel_dsi->panel_on_delay);
>  
> -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +		u32 val;
> +
>  		/* Disable DPOunit clock gating, can stall pipe */
> -		tmp = I915_READ(DSPCLK_GATE_D);
> -		tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> -		I915_WRITE(DSPCLK_GATE_D, tmp);
> +		val = I915_READ(DSPCLK_GATE_D);
> +		val |= DPOUNIT_CLOCK_GATE_DISABLE;
> +		I915_WRITE(DSPCLK_GATE_D, val);
>  	}
>  
>  	/* put device in ready state */
> @@ -677,7 +678,7 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>  
>  	intel_dsi_clear_device_ready(encoder);
>  
> -	if (!IS_BROXTON(dev_priv)) {
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		u32 val;
>  
>  		val = I915_READ(DSPCLK_GATE_D);
Ville Syrjala April 27, 2016, 2:49 p.m. UTC | #2 
On Mon, Apr 18, 2016 at 07:18:25PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check for VLV/CHV instead if !BXT when re-enabling DPOunit clock gating
> after DSI disable. That's what we checked when disabling the clock
> gating when enabling DSI.
> 
> Also use the same temporary variable name in both cases, and toss in a
> bit of dev vs. dev_priv cleanup while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Anyone know why this wasn't picked up by the .fi CI?
Tomi?

> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 34328ddaaab5..599045359538 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -516,7 +516,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>  	enum port port;
> -	u32 tmp;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> @@ -535,11 +534,13 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  
>  	msleep(intel_dsi->panel_on_delay);
>  
> -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +		u32 val;
> +
>  		/* Disable DPOunit clock gating, can stall pipe */
> -		tmp = I915_READ(DSPCLK_GATE_D);
> -		tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> -		I915_WRITE(DSPCLK_GATE_D, tmp);
> +		val = I915_READ(DSPCLK_GATE_D);
> +		val |= DPOUNIT_CLOCK_GATE_DISABLE;
> +		I915_WRITE(DSPCLK_GATE_D, val);
>  	}
>  
>  	/* put device in ready state */
> @@ -677,7 +678,7 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>  
>  	intel_dsi_clear_device_ready(encoder);
>  
> -	if (!IS_BROXTON(dev_priv)) {
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		u32 val;
>  
>  		val = I915_READ(DSPCLK_GATE_D);
> -- 
> 2.7.4
Sarvela, Tomi P April 27, 2016, 3:06 p.m. UTC | #3 
On Wednesday 27 April 2016 17:49:26 Ville Syrjälä wrote:
> On Mon, Apr 18, 2016 at 07:18:25PM +0300, ville.syrjala@linux.intel.com 
wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Check for VLV/CHV instead if !BXT when re-enabling DPOunit clock gating
> > after DSI disable. That's what we checked when disabling the clock
> > gating when enabling DSI.
> > 
> > Also use the same temporary variable name in both cases, and toss in a
> > bit of dev vs. dev_priv cleanup while at it.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Anyone know why this wasn't picked up by the .fi CI?

Was tested, but posting had failed. Retried.

> Tomi?

Tomi
Ville Syrjala April 27, 2016, 3:25 p.m. UTC | #4 
On Wed, Apr 27, 2016 at 03:02:01PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Unify VLV/CHV DPOunit clock gating disable/enable
> URL   : https://patchwork.freedesktop.org/series/5886/
> State : warning
> 
> == Summary ==
> 
> Series 5886v1 drm/i915: Unify VLV/CHV DPOunit clock gating disable/enable
> http://patchwork.freedesktop.org/api/1.0/series/5886/revisions/1/mbox/
> 
> Test kms_force_connector_basic:
>         Subgroup force-load-detect:
>                 skip       -> PASS       (snb-x220t)
> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (snb-dellxps)

[  345.841921] ------------[ cut here ]------------
[  345.841947] WARNING: CPU: 1 PID: 6167 at drivers/gpu/drm/i915/intel_display.c:13529 intel_atomic_commit+0x1271/0x1400 [i915]
[  345.841948] pipe A vblank wait timed out
[  345.841950] Modules linked in: smsc75xx usbnet mii snd_hda_codec_hdmi snd_hda_codec_realtek x86_pkg_temp_thermal snd_hda_codec_generic intel_powerclamp coretemp crct10dif_pclmul i915 crc32_pclmul ghash_clmulni_intel snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core mei_me snd_pcm mei lpc_ich
[  345.841964] CPU: 1 PID: 6167 Comm: kms_pipe_crc_ba Tainted: G     U          4.6.0-rc4-CI-Patchwork_1933+ #1
[  345.841965] Hardware name: Dell Inc. XPS 8300  /0Y2MRG, BIOS A06 10/17/2011
[  345.841967]  0000000000000000 ffff8800bfe77b28 ffffffff81405e35 ffff8800bfe77b78
[  345.841970]  0000000000000000 ffff8800bfe77b68 ffffffff81079c7c 000034d9c0c84520
[  345.841972]  0000000000000001 ffff8801299722a8 0000000000000000 0000000000000000
[  345.841974] Call Trace:
[  345.841978]  [<ffffffff81405e35>] dump_stack+0x67/0x92
[  345.841981]  [<ffffffff81079c7c>] __warn+0xcc/0xf0
[  345.841983]  [<ffffffff81079cea>] warn_slowpath_fmt+0x4a/0x50
[  345.841986]  [<ffffffff810c2df9>] ? finish_wait+0x59/0x70
[  345.842001]  [<ffffffffa014c241>] intel_atomic_commit+0x1271/0x1400 [i915]
[  345.842003]  [<ffffffff810c2f60>] ? wait_woken+0x90/0x90
[  345.842005]  [<ffffffff81538b52>] drm_atomic_commit+0x32/0x50
[  345.842007]  [<ffffffff8151565d>] drm_atomic_helper_set_config+0x7d/0xb0
[  345.842010]  [<ffffffff81528e40>] drm_mode_set_config_internal+0x60/0x110
[  345.842011]  [<ffffffff8152c616>] drm_mode_setcrtc+0x186/0x4f0
[  345.842013]  [<ffffffff8151e64d>] drm_ioctl+0x13d/0x590
[  345.842014]  [<ffffffff8152c490>] ? drm_mode_setplane+0x1b0/0x1b0
[  345.842016]  [<ffffffff811ee0aa>] do_vfs_ioctl+0x8a/0x670
[  345.842018]  [<ffffffff811f9f3a>] ? __fget_light+0x6a/0x90
[  345.842020]  [<ffffffff811ee6cc>] SyS_ioctl+0x3c/0x70
[  345.842022]  [<ffffffff817d24a9>] entry_SYSCALL_64_fastpath+0x1c/0xac
[  345.842023] ---[ end trace caa7340649a55fcb ]---
[  354.764853] drm/i915: Resetting chip after gpu hang

https://bugs.freedesktop.org/show_bug.cgi?id=95125

> 
> bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
> bdw-ultra        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
> bsw-nuc-2        total:191  pass:152  dwarn:0   dfail:0   fail:0   skip:39 
> hsw-brixbox      total:192  pass:168  dwarn:0   dfail:0   fail:0   skip:24 
> ivb-t430s        total:192  pass:164  dwarn:0   dfail:0   fail:0   skip:28 
> skl-i7k-2        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
> skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
> snb-dellxps      total:192  pass:153  dwarn:1   dfail:0   fail:0   skip:38 
> snb-x220t        total:192  pass:154  dwarn:0   dfail:0   fail:1   skip:37 
> byt-nuc failed to collect. IGT log at Patchwork_1933/byt-nuc/igt.log
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1933/
> 
> b1b2678f7bf90de8cecba84acb8f8c967a5f5d80 drm-intel-nightly: 2016y-04m-18d-17h-17m-00s UTC integration manifest
> ce15fac drm/i915: Unify VLV/CHV DPOunit clock gating disable/enable
Ville Syrjala April 27, 2016, 7:37 p.m. UTC | #5 
On Mon, Apr 18, 2016 at 08:26:46PM +0300, Jani Nikula wrote:
> On Mon, 18 Apr 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Check for VLV/CHV instead if !BXT when re-enabling DPOunit clock gating
> > after DSI disable. That's what we checked when disabling the clock
> > gating when enabling DSI.
> >
> > Also use the same temporary variable name in both cases, and toss in a
> > bit of dev vs. dev_priv cleanup while at it.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Pushed to dinq. Thanks for the review.

> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 34328ddaaab5..599045359538 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -516,7 +516,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> >  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> >  	enum port port;
> > -	u32 tmp;
> >  
> >  	DRM_DEBUG_KMS("\n");
> >  
> > @@ -535,11 +534,13 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >  
> >  	msleep(intel_dsi->panel_on_delay);
> >  
> > -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > +		u32 val;
> > +
> >  		/* Disable DPOunit clock gating, can stall pipe */
> > -		tmp = I915_READ(DSPCLK_GATE_D);
> > -		tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> > -		I915_WRITE(DSPCLK_GATE_D, tmp);
> > +		val = I915_READ(DSPCLK_GATE_D);
> > +		val |= DPOUNIT_CLOCK_GATE_DISABLE;
> > +		I915_WRITE(DSPCLK_GATE_D, val);
> >  	}
> >  
> >  	/* put device in ready state */
> > @@ -677,7 +678,7 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
> >  
> >  	intel_dsi_clear_device_ready(encoder);
> >  
> > -	if (!IS_BROXTON(dev_priv)) {
> > +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> >  		u32 val;
> >  
> >  		val = I915_READ(DSPCLK_GATE_D);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 34328ddaaab5..599045359538 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -516,7 +516,6 @@  static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	enum port port;
-	u32 tmp;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -535,11 +534,13 @@  static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 
 	msleep(intel_dsi->panel_on_delay);
 
-	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		u32 val;
+
 		/* Disable DPOunit clock gating, can stall pipe */
-		tmp = I915_READ(DSPCLK_GATE_D);
-		tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
-		I915_WRITE(DSPCLK_GATE_D, tmp);
+		val = I915_READ(DSPCLK_GATE_D);
+		val |= DPOUNIT_CLOCK_GATE_DISABLE;
+		I915_WRITE(DSPCLK_GATE_D, val);
 	}
 
 	/* put device in ready state */
@@ -677,7 +678,7 @@  static void intel_dsi_post_disable(struct intel_encoder *encoder)
 
 	intel_dsi_clear_device_ready(encoder);
 
-	if (!IS_BROXTON(dev_priv)) {
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		u32 val;
 
 		val = I915_READ(DSPCLK_GATE_D);