+Art
On Thu, 21 Aug 2014, Clint Taylor <clinton.a.taylor@xxxxxxxxx> wrote:
> There is also a need to add this delay when turning off the PWM enable
> bit through intel_panel_disable_backlight(). If the PWM enable bit is
> turned off while the PWM signal is high then the signal remains high. If
> the bit is turned off when the signal is low the PWM will remain low.
> Since we don't know the current state of the PWM signal we must set the
> duty_cycle to 0, wait PWM_CYCLE_DELAY, and then turn off the enable bit.
[citation needed]
Really, I want this in the specs if this is true.
> Actually it might be better if we never turn off the PWM enable bit in
> intel_panel_disable_backlight() and we just use the duty cycle register
> to control the PWM.
Art, any feedback on these two?
BR,
Jani.
>
>> So I guess your approach is the simplest option here.
>>
>>> _intel_edp_backlight_on(intel_dp);
>>> }
>>>
>>> @@ -4256,9 +4257,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>> assign_final(t11_t12);
>>> #undef assign_final
>>>
>>> +#define PWM_CYCLE_DELAY 5
>>
>> Shoduln't this be calculated from the PWM frequqncy? Not sure what a PWM
>> cycle here is exactly. Just one full period of the signal?
>>
>
> The PWM cycle in this case turns out to be 1 full cycle of the PWM
> waveform though it depends on which display core clock (128 or
> 25Mhz(S0ix) is being used. Since we only care about the minimum elapsed
> time to meet the panel specification a value of 5ms will work even
> though more or less PWM cycles would take place before the BL_ENABLE is
> asserted. I would prefer not to add complexity to switch between panel
> timings for normal and S0ix display clock modes. How often does the
> backlight get enabled while in S0ix?
>
>> Also would be nice to have a comment in the code explaining what this is
>> and why we need to add it to the delay.
>
> Sure, As you may have noticed in other patches I really like comments.
>
>>
>>> #define get_delay(field) (DIV_ROUND_UP(final.field, 10))
>>> intel_dp->panel_power_up_delay = get_delay(t1_t3);
>>> - intel_dp->backlight_on_delay = get_delay(t8);
>>> + intel_dp->backlight_on_delay = get_delay(t8) + PWM_CYCLE_DELAY;
>>> intel_dp->backlight_off_delay = get_delay(t9);
>>> intel_dp->panel_power_down_delay = get_delay(t10);
>>> intel_dp->panel_power_cycle_delay = get_delay(t11_t12);
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index 3abc915..ad6fcc1 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -556,6 +556,7 @@ struct intel_dp {
>>> bool want_panel_vdd;
>>> unsigned long last_power_cycle;
>>> unsigned long last_power_on;
>>> + unsigned long last_backlight_on;
>>> unsigned long last_backlight_off;
>>>
>>> struct notifier_block edp_notifier;
>>> --
>>> 1.8.3.2
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
