[Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the .shutdown() hook

[Ville Syrjälä]

On Tue, Oct 06, 2020 at 09:13:32PM +0300, Jani Nikula wrote:
> On Tue, 06 Oct 2020, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > On Tue, Oct 06, 2020 at 12:29:22PM +0300, Jani Nikula wrote:
> >> On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala at linux.intel.com> wrote:
> >> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> >
> >> > Currently VLV/CHV use a reboot notifier to make sure the panel
> >> > power cycle delay isn't violated across a system reboot. Replace
> >> > that with the new encoder .shutdown() hook.
> >> >
> >> > And let's also stop overriding the power cycle delay with the
> >> > max value. No idea why the current code does that. The already
> >> > programmed delay should be correct.
> >> 
> >> I kind of have a little uneasy feeling about conflating these two
> >> changes together. I think both are objectively good changes, just not
> >> necessarily at once.
> >> 
> >> ISTR setting the max delay was, perhaps, somehow related to the hardware
> >> losing its marbles after power is cut, effectively not ensuring any of
> >> the delays at power-on. So it's possible we set the max here to account
> >> for that. Maybe. ;)
> >> 
> >> Anyway,
> >> 
> >> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
> >> 
> >> on the whole.
> >> 
> >> I'm leaving it up to you, but personally I'd lean towards switching
> >> edp_notify_handler() to use wait_panel_power_cycle(intel_dp) first in a
> >> separate patch, to help with potential bisect results, and then doing
> >> the rest.
> >
> > I don't think it would be quite that simple. We'd have to also toss
> > in some combination of panel_off() and vdd_off_sync() in there,
> > depending on whether the panel power is currently enabled or not.
> > Otherwise the bookkeeping needed by wait_panel_power_cycle() isn't
> > going to be up to date.
> 
> Oh? So the calls via encoder hooks actually ensure that?

1. drm_atomic_helper_shutdown()
   -> panel_off() as needed via the normal crtc disable sequence
2. mst suspend + interrupts off + cancel hpd stuff
   -> hopefully nothing can re-enable vdd after this point
3. encoder.suspend()
   -> vdd_off_sync() to really turn vdd off if it's still lingering
4. encoder.shutdown()
   -> wait_panel_power_cycle()

Hopefully it even works like that :P

-- 
Ville Syrjälä
Intel