Re: [PATCH] drm/i915: vlv: fix DP PHY lockup due to invalid PP sequencer setup

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3123 bytes --]

On Thu, 2014-01-30 at 16:52 +0100, Daniel Vetter wrote:
> On Thu, Jan 30, 2014 at 04:50:42PM +0200, Imre Deak wrote:
> > Atm we setup the HW panel power sequencer logic both for eDP and DP
> > ports. On eDP we then go on and start the power on sequence and commence
> > with link training when it's ready. On DP we don't do the power on
> > sequencing but do the link training immediately. At this point the DP
> > PHY block gets stuck, since - supposedly - it is waiting for the power
> > on sequence to finish. The actual register write that seems to hold off
> > the PHY is PIPEX_PP_ON_DELAYS[Panel Control Port Select]. Writing here
> > a non-0 value eventually sets PIPEX_PP_STATUS[Require Asset Status] to
> > 1 and blocks the PHY until the panel power on is ready.
> > 
> > Fix this by not doing any PP sequencing setup for DP ports.
> > 
> > Thanks to Ville Syrjälä, Jesse Barnes and Todd Previte for the help in
> > tracking this down.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Ah, the infamous ABCD hack we're using all over the place in intel_lvds.c.
> On edp we didn't have a need for it thus far since the "require asset
> status" checks have all been fused of, with the PP being on the PCH and
> the edp port on the north display block. If this is really all we need to
> appease the hardware then I'm heavily in favour of it as opposed to
> resurrect the ABCD hack for intel_dp.c.

Yea, it seems it's not needed on BYT, since even with the PP_CONTROL
being all 0 (and not doing any further PP sequencing setup) things work
fine.

> One thing though: Should we add a check for the "Required Asset Status"
> bit somewhere? I don't really have a good idea for a spot to put this
> into, hence the question.

We could add it for eDP, but I guess based on the above for DP we don't
need to check it.

--Imre

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index ffac7e8..b744073 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1933,10 +1933,12 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder)
> >  
> >  	mutex_unlock(&dev_priv->dpio_lock);
> >  
> > -	/* init power sequencer on this pipe and port */
> > -	intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
> > -	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
> > -						      &power_seq);
> > +	if (is_edp(intel_dp)) {
> > +		/* init power sequencer on this pipe and port */
> > +		intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
> > +		intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
> > +							      &power_seq);
> > +	}
> >  
> >  	intel_enable_dp(encoder);
> >  
> > -- 
> > 1.8.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx