On Sat, 02 Jan 2016, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Thu, 31 Dec 2015, Vidya Srinivas <vidya.srinivas@xxxxxxxxx> 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@xxxxxxxxx>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
