Re: [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes.

From: "Zhang, Xiong Y" <xiong.y.zhang@intel.com>
To: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes.
Date: Wed, 12 Aug 2015 02:20:34 +0000	[thread overview]
Message-ID: <8082FF9BCB2B054996454E47167FF4EC029AC428@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <1439318369.3344.94.camel@intel.com>

> On Tue, 2015-08-11 at 07:05 +0000, Zhang, Xiong Y wrote:
> > > -----Original Message-----
> > > From: Vivi, Rodrigo
> > > Sent: Saturday, August 8, 2015 8:34 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Vivi, Rodrigo; Zhang, Xiong Y
> > > Subject: [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes.
> > >
> > > DDI-A and DDI-E shares the 4 lanes. So when DDI-E is present we need
> > > to configure lane count propperly for both.
> > >
> > > This was based on Sonika's
> > > [PATCH] drm/i915/skl: Select DDIA lane capability based upon vbt
> > >
> > > Credits-to: Sonika Jindal <sonika.jindal@intel.com>
> > > Cc: Xiong Zhang <xiong.y.zhang@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 12 ++++++++++--
> > > drivers/gpu/drm/i915/intel_dp.c  |  8 +++++---
> > >  2 files changed, 15 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 110d546..557cecf 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -3178,7 +3178,15 @@ void intel_ddi_init(struct drm_device *dev,
> > > enum port port)
> > >  	struct intel_digital_port *intel_dig_port;
> > >  	struct intel_encoder *intel_encoder;
> > >  	struct drm_encoder *encoder;
> > > -	bool init_hdmi, init_dp;
> > > +	bool init_hdmi, init_dp, ddi_e_present;
> > > +
> > > +	/*
> > > +	 * On SKL we don't have a way to detect DDI-E so we rely
> > > on VBT.
> > > +	 */
> > > +	ddie_present = IS_SKYLAKE(dev) &&
> > > +		(dev_priv->vbt.ddi_port_info[PORT_E].supports_dp
> > > ||
> > > +		 dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi
> > > ||
> > > +		 dev_priv
> > > ->vbt.ddi_port_info[PORT_E].supports_hdmi);
> > [Zhang, Xiong Y]  ddie_present should be ddi_e_present
> > >
> > >  	init_hdmi = (dev_priv
> > > ->vbt.ddi_port_info[port].supports_dvi ||
> > >  		     dev_priv
> > > ->vbt.ddi_port_info[port].supports_hdmi);
> > > @@ -3210,7 +3218,7 @@ void intel_ddi_init(struct drm_device *dev,
> > > enum port port)
> > >  	intel_dig_port->port = port;
> > >  	intel_dig_port->saved_port_bits =
> > > I915_READ(DDI_BUF_CTL(port)) &
> > >  					  (DDI_BUF_PORT_REVERSAL |
> > > -					   DDI_A_4_LANES);
> > > +					   ddi_e_present ? 0 :
> > > DDI_A_4_LANES);
> > [Zhang, Xiong Y] Sonika's patch will set DDI-A to 4 lanes when DDI-E
> > doesn't exist, I think your patch will do nothing.
> 
> Actually DDI_A_4_LANES is being already set unconditionally, so Sonika's
> patch has no effect.
[Zhang, Xiong Y] No. Sonika's patch set DDI_A_4_LANES under many conditions.
+	if (IS_SKYLAKE(dev) && port == PORT_A
+		&& !(val & DDI_BUF_CTL_ENABLE)
+		&& !dev_priv->vbt.ddi_e_used)
+		I915_WRITE(DDI_BUF_CTL(port), val | DDI_A_4_LANES)
> 
> saved_port_bits goes to intel_dp->DP that goes to DDI_BUF_CTL and also it is
> used to calculate the number of lanes.
> 
> With this patch we stop setting DDI_A_4_LANES when ddi_e is present so
> DDI-A keeps with 2 lanes and let other 2 lanes for DDI-E
[Zhang, Xiong Y] Yes, this patch will clear DDI_A_4_LANES when ddi_e is present.
But this patch won't set DDI_A_4_LANES under following conditions which is purpose for Sonika patch
1. Bios fail to driver eDP and doesn't enable DDI_A buffer
2. Bios clear DDI_A_4_LANES
3. DDI_E isn't present

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