[Intel-gfx] [PATCH 02/14] drm/i915: Reorganize vlv eDP reboot notifier

[Ville Syrjälä]

On Tue, Aug 26, 2014 at 03:36:07PM +0200, Daniel Vetter wrote:
> On Tue, Aug 26, 2014 at 04:21:00PM +0300, Jani Nikula wrote:
> > On Tue, 26 Aug 2014, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > > On Tue, Aug 19, 2014 at 10:00:55AM +0300, Jani Nikula wrote:
> > >> On Mon, 18 Aug 2014, ville.syrjala at linux.intel.com wrote:
> > >> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >> >
> > >> > Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check
> > >> > and flatten the rest of the function.
> > >> 
> > >> Please imagine adding another platform there, and realize this just adds
> > >> unnecessary churn.
> > >
> > > I'd just add another reboot notifier then.
> > 
> > Fair enough; it should be vlv_edp_notify_handler then. (No, don't send a
> > patch to change that! ;)
> > 
> > > Frankly I don't understand the current one either. Why does it need to
> > > set the delay to max for instance? And does this mean that the
> > > PANEL_POWER_RESET bit doesn't actually work as advertised in the docs?
> > 
> > *shrug* experimental evidence?
> > 
> > commit 01527b3127997ef6370d5ad4fa25d96847fbf12a
> > Author: Clint Taylor <clinton.a.taylor at intel.com>
> > Date:   Mon Jul 7 13:01:46 2014 -0700
> > 
> >     drm/i915/vlv: T12 eDP panel timing enforcement during reboot
> >     
> >     The panel power sequencer on vlv doesn't appear to accept changes to its
> >     T12 power down duration during warm reboots. This change forces a delay
> >     for warm reboots to the T12 panel timing as defined in the VBT table for
> >     the connected panel.
> 
> So if I remember this piece of lore correctly in the past the pp was
> pessimistic, and enforced this delay on resume/boot-up, assuming you've
> shut down _right_ before the machine was lit up again. Apparently people
> where unhappy with that enforced delay and it was ditched on vlv, but then
> it broke panels if you actually managed to reboot quickly enough.

IIRC the way the reset bit is documented the hardware itself is supposed
to initiate the power off cycle when it gets some reset notification and
it should enforce the timing before allowing the panel power to be
re-enabled. Although it does seem that it would also reset the
"power cycle delay" so it would maybe only enforce some default delay in
that case (300ms based on the documented default value of 0x4). So if the
panel requires more than the 300ms then I understand the msleep() here.
I guess use of the VDD force bit just after reset might also require that
we do the power down + wait before reset. So that part does make sense
to me, but I still don't understand the "power cycle delay"=0x1f part.

-- 
Ville Syrjälä
Intel OTC