All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated
@ 2015-12-31 10:00 Vidya Srinivas
  2015-12-31 10:20 ` ✗ warning: Fi.CI.BAT Patchwork
  2016-01-02 13:45 ` [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated Jani Nikula
  0 siblings, 2 replies; 4+ messages in thread
From: Vidya Srinivas @ 2015-12-31 10:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Vidya Srinivas

Currently Backlight registers which are associated with Power Well 0
are not being saved before gating the power well for S0ix. Hence,
upon resume from S0ix, these registers are not being restored. Due to
this, the display has resumed and since there is no backlight, nothing is
seen. Patch fixes this issue by saving/restoring BLC registers for S0ix.

Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h   |  4 ++++
 drivers/gpu/drm/i915/intel_panel.c | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 50f83d2..752fb58 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -193,6 +193,10 @@ struct intel_panel {
 				      uint32_t hz);
 		void (*power)(struct intel_connector *, bool enable);
 	} backlight;
+
+	u32 blc_pwm_ctl;
+	u32 blc_pwm_freq;
+	u32 blc_pwm_duty;
 };
 
 struct intel_connector {
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index ae808b6..421cd3a 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -816,6 +816,16 @@ static void bxt_disable_backlight(struct intel_connector *connector)
 		val &= ~UTIL_PIN_ENABLE;
 		I915_WRITE(UTIL_PIN_CTL, val);
 	}
+
+	/* Saving BLC registers for PG0 gating */
+	panel->blc_pwm_ctl =
+		I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
+	panel->blc_pwm_freq =
+		I915_READ(BXT_BLC_PWM_FREQ(panel->backlight.controller));
+	panel->blc_pwm_duty =
+		I915_READ(BXT_BLC_PWM_DUTY(panel->backlight.controller));
+
+
 }
 
 static void pwm_disable_backlight(struct intel_connector *connector)
@@ -1050,6 +1060,14 @@ static void bxt_enable_backlight(struct intel_connector *connector)
 	enum pipe pipe = intel_get_pipe_from_connector(connector);
 	u32 pwm_ctl, val;
 
+	/* Restore BLC registers if PG0 was gated */
+	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
+				panel->blc_pwm_ctl);
+	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
+				panel->blc_pwm_freq);
+	I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller),
+				panel->blc_pwm_duty);
+
 	/* To use 2nd set of backlight registers, utility pin has to be
 	 * enabled with PWM mode.
 	 * The field should only be changed when the utility pin is disabled
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* ✗ warning: Fi.CI.BAT
  2015-12-31 10:00 [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated Vidya Srinivas
@ 2015-12-31 10:20 ` Patchwork
  2016-01-02 13:45 ` [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated Jani Nikula
  1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2015-12-31 10:20 UTC (permalink / raw)
  To: Vidya Srinivas; +Cc: intel-gfx

== Summary ==

Built on 79686f613b3955a4ed09cee936e7f70ec4e61b67 drm-intel-nightly: 2015y-12m-30d-11h-59m-54s UTC integration manifest

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p)
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (skl-i5k-2)
                dmesg-warn -> PASS       (bsw-nuc-2)
                dmesg-warn -> PASS       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a:
                dmesg-warn -> PASS       (snb-x220t)
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (snb-dellxps)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (snb-x220t)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                dmesg-warn -> PASS       (snb-dellxps)

bdw-nuci7        total:132  pass:121  dwarn:2   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:126  dwarn:2   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
hsw-xps12        total:132  pass:125  dwarn:3   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430s        total:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2        total:135  pass:124  dwarn:3   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:124  dwarn:3   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_975/

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated
  2015-12-31 10:00 [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated Vidya Srinivas
  2015-12-31 10:20 ` ✗ warning: Fi.CI.BAT Patchwork
@ 2016-01-02 13:45 ` Jani Nikula
  2016-01-04  7:15   ` Jani Nikula
  1 sibling, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2016-01-02 13:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Vidya Srinivas

On Thu, 31 Dec 2015, Vidya Srinivas <vidya.srinivas@intel.com> wrote:
> Currently Backlight registers which are associated with Power Well 0
> are not being saved before gating the power well for S0ix. Hence,
> upon resume from S0ix, these registers are not being restored. Due to
> this, the display has resumed and since there is no backlight, nothing is
> seen. Patch fixes this issue by saving/restoring BLC registers for S0ix.

No, this is not the approach.

The backlight disable/enable hooks should handle this by always writing
the full state of the registers on enable. Doing blind register copies
(and read-modify-writes) is what we've been trying to get rid of across
the driver.

BR,
Jani.


>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h   |  4 ++++
>  drivers/gpu/drm/i915/intel_panel.c | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 50f83d2..752fb58 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -193,6 +193,10 @@ struct intel_panel {
>  				      uint32_t hz);
>  		void (*power)(struct intel_connector *, bool enable);
>  	} backlight;
> +
> +	u32 blc_pwm_ctl;
> +	u32 blc_pwm_freq;
> +	u32 blc_pwm_duty;
>  };
>  
>  struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index ae808b6..421cd3a 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -816,6 +816,16 @@ static void bxt_disable_backlight(struct intel_connector *connector)
>  		val &= ~UTIL_PIN_ENABLE;
>  		I915_WRITE(UTIL_PIN_CTL, val);
>  	}
> +
> +	/* Saving BLC registers for PG0 gating */
> +	panel->blc_pwm_ctl =
> +		I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> +	panel->blc_pwm_freq =
> +		I915_READ(BXT_BLC_PWM_FREQ(panel->backlight.controller));
> +	panel->blc_pwm_duty =
> +		I915_READ(BXT_BLC_PWM_DUTY(panel->backlight.controller));
> +
> +
>  }
>  
>  static void pwm_disable_backlight(struct intel_connector *connector)
> @@ -1050,6 +1060,14 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>  	enum pipe pipe = intel_get_pipe_from_connector(connector);
>  	u32 pwm_ctl, val;
>  
> +	/* Restore BLC registers if PG0 was gated */
> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> +				panel->blc_pwm_ctl);
> +	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
> +				panel->blc_pwm_freq);
> +	I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller),
> +				panel->blc_pwm_duty);
> +
>  	/* To use 2nd set of backlight registers, utility pin has to be
>  	 * enabled with PWM mode.
>  	 * The field should only be changed when the utility pin is disabled

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated
  2016-01-02 13:45 ` [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated Jani Nikula
@ 2016-01-04  7:15   ` Jani Nikula
  0 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2016-01-04  7:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Vidya Srinivas

On Sat, 02 Jan 2016, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 31 Dec 2015, Vidya Srinivas <vidya.srinivas@intel.com> wrote:
>> Currently Backlight registers which are associated with Power Well 0
>> are not being saved before gating the power well for S0ix. Hence,
>> upon resume from S0ix, these registers are not being restored. Due to
>> this, the display has resumed and since there is no backlight, nothing is
>> seen. Patch fixes this issue by saving/restoring BLC registers for S0ix.
>
> No, this is not the approach.
>
> The backlight disable/enable hooks should handle this by always writing
> the full state of the registers on enable. Doing blind register copies
> (and read-modify-writes) is what we've been trying to get rid of across
> the driver.

To be more specific, we *already* write the full state to the three
registers in bxt_enable_backlight(). Your conclusions in the commit
message are wrong. If you're seeing issues, it must be about the
sequence in which the registers are written. Please look into that.

BR,
Jani.

>
> BR,
> Jani.
>
>
>>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_drv.h   |  4 ++++
>>  drivers/gpu/drm/i915/intel_panel.c | 18 ++++++++++++++++++
>>  2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 50f83d2..752fb58 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -193,6 +193,10 @@ struct intel_panel {
>>  				      uint32_t hz);
>>  		void (*power)(struct intel_connector *, bool enable);
>>  	} backlight;
>> +
>> +	u32 blc_pwm_ctl;
>> +	u32 blc_pwm_freq;
>> +	u32 blc_pwm_duty;
>>  };
>>  
>>  struct intel_connector {
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index ae808b6..421cd3a 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -816,6 +816,16 @@ static void bxt_disable_backlight(struct intel_connector *connector)
>>  		val &= ~UTIL_PIN_ENABLE;
>>  		I915_WRITE(UTIL_PIN_CTL, val);
>>  	}
>> +
>> +	/* Saving BLC registers for PG0 gating */
>> +	panel->blc_pwm_ctl =
>> +		I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> +	panel->blc_pwm_freq =
>> +		I915_READ(BXT_BLC_PWM_FREQ(panel->backlight.controller));
>> +	panel->blc_pwm_duty =
>> +		I915_READ(BXT_BLC_PWM_DUTY(panel->backlight.controller));
>> +
>> +
>>  }
>>  
>>  static void pwm_disable_backlight(struct intel_connector *connector)
>> @@ -1050,6 +1060,14 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>>  	enum pipe pipe = intel_get_pipe_from_connector(connector);
>>  	u32 pwm_ctl, val;
>>  
>> +	/* Restore BLC registers if PG0 was gated */
>> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>> +				panel->blc_pwm_ctl);
>> +	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
>> +				panel->blc_pwm_freq);
>> +	I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller),
>> +				panel->blc_pwm_duty);
>> +
>>  	/* To use 2nd set of backlight registers, utility pin has to be
>>  	 * enabled with PWM mode.
>>  	 * The field should only be changed when the utility pin is disabled

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-04  7:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-31 10:00 [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated Vidya Srinivas
2015-12-31 10:20 ` ✗ warning: Fi.CI.BAT Patchwork
2016-01-02 13:45 ` [PATCH] drm/i915/bxt: Save/Restore Backlight registers when PG0 is gated Jani Nikula
2016-01-04  7:15   ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.