Re: [PATCH] drm/i915: reboot notifier delay for eDP panels

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jan 12, 2016 at 09:06:24AM -0800, Clint Taylor wrote:
> On 01/12/2016 05:21 AM, Ville Syrjälä wrote:
> > On Mon, Jan 11, 2016 at 01:52:17PM -0800, clinton.a.taylor@intel.com wrote:
> >> From: Clint Taylor <clinton.a.taylor@intel.com>
> >>
> >> Add reboot notifier for all platforms. This guarantees T12 delay
> >> compliance during reboot cycles when pre-os enables the panel within
> >> 500ms.
> >>
> >> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_dp.c |   11 ++++++++---
> >>   1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 796e3d3..dbbd27a 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -126,6 +126,7 @@ static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
> >>   static void intel_dp_link_down(struct intel_dp *intel_dp);
> >>   static bool edp_panel_vdd_on(struct intel_dp *intel_dp);
> >>   static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
> >> +static void edp_panel_off(struct intel_dp *intel_dp);
> >>   static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp);
> >>   static void vlv_steal_power_sequencer(struct drm_device *dev,
> >>   				      enum pipe pipe);
> >> @@ -596,6 +597,10 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
> >>   		I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
> >>   		msleep(intel_dp->panel_power_cycle_delay);
> >>   	}
> >> +	else
> >> +	{
> >> +		edp_panel_off(intel_dp);
> >> +	}
> >
> > I don't think that actually waits for the power cycle delay. Also
> > you can't call it without having vdd already enabled unless you
> > specifically want to see a WARN.
> >
> edp_panel_off() does wait for panel off time. This has also been 
> confirmed with a scope watching vdd.

Panel off delay yes, but not power cycle delay. Unless you wait for that
as well, then the firmware/whoever could re-enable vdd too soon AFAICS.

> 
> > I suppose you could just do something along these lines:
> >
> > if (have_panel_power || have_panel_vdd) {
> Good catch. I will add the check to make sure panel power is on before 
> calling edp_panel_off(). If the panel power was off I will still need to 
> check to make sure it has been off for the required time.

Which is why my sample code had the wait_panel_power_cycle() call
uncnditionally.

> 
> > 	edp_panel_vdd_on
> > 	edp_panel_off
> > }
> > wait_panel_power_cycle
> >
> > Also should change VLV/CHV to do it the same way.
> 
> VLV/CHV PPS handles things a little different and the existing code was 
> required to prevent VDD from asserting during the reboot cycle. We 
> increased the delay to MAX the VLV handler to prevent the VDD from 
> asserting once the PPS cycle completed.

That doesn't sound sane at all. Are you saying the hardware re-enabled
vdd on its own somehow unless you frobbed with the delay?

> I will test on my CHV to confirm 
> the old handler is still required.
> 
> >
> >>
> >>   	pps_unlock(intel_dp);
> >>
> >> @@ -5796,10 +5801,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >>   	}
> >>   	mutex_unlock(&dev->mode_config.mutex);
> >>
> >> -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> >> -		intel_dp->edp_notifier.notifier_call = edp_notify_handler;
> >> -		register_reboot_notifier(&intel_dp->edp_notifier);
> >> +	intel_dp->edp_notifier.notifier_call = edp_notify_handler;
> >> +	register_reboot_notifier(&intel_dp->edp_notifier);
> >>
> >> +	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> >>   		/*
> >>   		 * Figure out the current pipe for the initial backlight setup.
> >>   		 * If the current pipe isn't valid, try the PPS pipe, and if that
> >> --
> >> 1.7.9.5
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx