On Fri, Aug 22, 2014 at 05:12:25PM +0000, Runyan, Arthur J wrote: > I'll check it out. I haven't heard any complaint about this before, but > it may be one of those things that started back on i745 and never got > documented. Only i855 and later started to have native lvds with all the surrounding stuff, before there's only bridge chips that did all the magic. So won't be quite _that_ old ;-) Cheers, Daniel > > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > Sent: Friday, August 22, 2014 6:07 AM > To: Taylor, Clinton A; Ville Syrjälä; Runyan, Arthur J > Cc: Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert > > > +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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
