diff mbox

drm/i915: Wait for power cycle delay after turning off DSI panel power

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

Commit Message

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

The power cycle delay starts _after_ turning off the panel power. Do the
msleep after frobbing the pmic panel power gpio.

Also toss in a FIXME about optimizing away needless waits.

Cc: Shobhit Kumar <shobhit.kumar@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Fixes: fc45e8219907 ("drm/i915: Use the CRC gpio for panel enable/disable")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Jani Nikula April 18, 2016, 5:19 p.m. UTC | #1 
On Mon, 18 Apr 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The power cycle delay starts _after_ turning off the panel power. Do the
> msleep after frobbing the pmic panel power gpio.
>
> Also toss in a FIXME about optimizing away needless waits.
>
> Cc: Shobhit Kumar <shobhit.kumar@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Fixes: fc45e8219907 ("drm/i915: Use the CRC gpio for panel enable/disable")
> 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 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 34328ddaaab5..2b22bb9bb86f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -688,11 +688,16 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>  	drm_panel_unprepare(intel_dsi->panel);
>  
>  	msleep(intel_dsi->panel_off_delay);
> -	msleep(intel_dsi->panel_pwr_cycle_delay);
>  
>  	/* Panel Disable over CRC PMIC */
>  	if (intel_dsi->gpio_panel)
>  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
> +
> +	/*
> +	 * FIXME As we do with eDP, just make a note of the time here
> +	 * and perform the wait before the next panel power on.
> +	 */
> +	msleep(intel_dsi->panel_pwr_cycle_delay);
>  }
>  
>  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
Shobhit Kumar April 19, 2016, 2:30 a.m. UTC | #2 
On Monday 18 April 2016 09:47 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The power cycle delay starts _after_ turning off the panel power. Do the
> msleep after frobbing the pmic panel power gpio.
>
> Also toss in a FIXME about optimizing away needless waits.
>
> Cc: Shobhit Kumar <shobhit.kumar@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Fixes: fc45e8219907 ("drm/i915: Use the CRC gpio for panel enable/disable")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Shobhit Kumar <shobhit.kumar@intel.com>

> ---
>   drivers/gpu/drm/i915/intel_dsi.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 34328ddaaab5..2b22bb9bb86f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -688,11 +688,16 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>   	drm_panel_unprepare(intel_dsi->panel);
>
>   	msleep(intel_dsi->panel_off_delay);
> -	msleep(intel_dsi->panel_pwr_cycle_delay);
>
>   	/* Panel Disable over CRC PMIC */
>   	if (intel_dsi->gpio_panel)
>   		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
> +
> +	/*
> +	 * FIXME As we do with eDP, just make a note of the time here
> +	 * and perform the wait before the next panel power on.
> +	 */
> +	msleep(intel_dsi->panel_pwr_cycle_delay);
>   }
>
>   static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>
Ville Syrjälä April 19, 2016, 11:56 a.m. UTC | #3 
On Mon, Apr 18, 2016 at 05:24:47PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Wait for power cycle delay after turning off DSI panel power
> URL   : https://patchwork.freedesktop.org/series/5885/
> State : warning
> 
> == Summary ==
> 
> Series 5885v1 drm/i915: Wait for power cycle delay after turning off DSI panel power
> http://patchwork.freedesktop.org/api/1.0/series/5885/revisions/1/mbox/
> 
> Test gem_busy:
>         Subgroup basic-vebox:
>                 skip       -> PASS       (bsw-nuc-2)
> Test gem_sync:
>         Subgroup basic-each:
>                 dmesg-fail -> PASS       (hsw-brixbox)
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 pass       -> DMESG-WARN (hsw-gt2)

[  212.413095] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 109
[  212.413118] Raw EDID:
[  212.413126]  	00 ff ff ff ff ff ff 00 04 72 5b 03 63 15 90 34
[  212.413138]  	31 17 01 03 80 35 1e 78 ee a0 a5 a6 56 ff ff ff
[  212.413151]  	ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[  212.413164]  	ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[  212.413177]  	ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[  212.413190]  	ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[  212.413203]  	ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[  212.413216]  	ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

Flaky cable perhaps?

> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-b:
>                 dmesg-warn -> PASS       (snb-dellxps)
>         Subgroup suspend-read-crc-pipe-b:
>                 skip       -> PASS       (hsw-brixbox)
> 
> bdw-nuci7        total:203  pass:191  dwarn:0   dfail:0   fail:0   skip:12 
> bdw-ultra        total:203  pass:180  dwarn:0   dfail:0   fail:0   skip:23 
> bsw-nuc-2        total:202  pass:163  dwarn:0   dfail:0   fail:0   skip:39 
> byt-nuc          total:202  pass:164  dwarn:0   dfail:0   fail:0   skip:38 
> hsw-brixbox      total:203  pass:179  dwarn:0   dfail:0   fail:0   skip:24 
> hsw-gt2          total:203  pass:183  dwarn:1   dfail:0   fail:0   skip:19 
> ivb-t430s        total:203  pass:175  dwarn:0   dfail:0   fail:0   skip:28 
> skl-i7k-2        total:203  pass:178  dwarn:0   dfail:0   fail:0   skip:25 
> skl-nuci5        total:203  pass:192  dwarn:0   dfail:0   fail:0   skip:11 
> snb-dellxps      total:203  pass:165  dwarn:0   dfail:0   fail:0   skip:38 
> snb-x220t        total:203  pass:165  dwarn:0   dfail:0   fail:1   skip:37 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1932/
> 
> b1acc63c4301f12ef2ca31872d937641aaf3c9dc drm-intel-nightly: 2016y-04m-18d-15h-55m-43s UTC integration manifest
> 9e979d6 drm/i915: Wait for power cycle delay after turning off DSI panel power
Ville Syrjälä April 19, 2016, 11:59 a.m. UTC | #4 
On Tue, Apr 19, 2016 at 08:00:10AM +0530, Kumar, Shobhit wrote:
> On Monday 18 April 2016 09:47 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The power cycle delay starts _after_ turning off the panel power. Do the
> > msleep after frobbing the pmic panel power gpio.
> >
> > Also toss in a FIXME about optimizing away needless waits.
> >
> > Cc: Shobhit Kumar <shobhit.kumar@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Fixes: fc45e8219907 ("drm/i915: Use the CRC gpio for panel enable/disable")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Shobhit Kumar <shobhit.kumar@intel.com>

Pushed to dinq. Thanks for the reviews.

> 
> > ---
> >   drivers/gpu/drm/i915/intel_dsi.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 34328ddaaab5..2b22bb9bb86f 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -688,11 +688,16 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
> >   	drm_panel_unprepare(intel_dsi->panel);
> >
> >   	msleep(intel_dsi->panel_off_delay);
> > -	msleep(intel_dsi->panel_pwr_cycle_delay);
> >
> >   	/* Panel Disable over CRC PMIC */
> >   	if (intel_dsi->gpio_panel)
> >   		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
> > +
> > +	/*
> > +	 * FIXME As we do with eDP, just make a note of the time here
> > +	 * and perform the wait before the next panel power on.
> > +	 */
> > +	msleep(intel_dsi->panel_pwr_cycle_delay);
> >   }
> >
> >   static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> >
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 34328ddaaab5..2b22bb9bb86f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -688,11 +688,16 @@  static void intel_dsi_post_disable(struct intel_encoder *encoder)
 	drm_panel_unprepare(intel_dsi->panel);
 
 	msleep(intel_dsi->panel_off_delay);
-	msleep(intel_dsi->panel_pwr_cycle_delay);
 
 	/* Panel Disable over CRC PMIC */
 	if (intel_dsi->gpio_panel)
 		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
+
+	/*
+	 * FIXME As we do with eDP, just make a note of the time here
+	 * and perform the wait before the next panel power on.
+	 */
+	msleep(intel_dsi->panel_pwr_cycle_delay);
 }
 
 static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,