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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/13/2015 8:57 AM, Zhang, Xiong Y wrote:
On Wed, 2015-08-12 at 02:20 +0000, Zhang, Xiong Y wrote:
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@xxxxxxxxxxxxxxxxxxxxx
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@xxxxxxxxx>
Cc: Xiong Zhang <xiong.y.zhang@xxxxxxxxx>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
---
  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

If DDI_A isn't enabled we don't need to set DDI_A_4_LANES
[Zhang, Xiong Y] From commit message on Sonika patch, she want to
set DDI_A_4_LANES on such case. Maybe she met such fail case on one high
resolution eDP screen. Let's Sonikia explain it.

2. Bios clear DDI_A_4_LANES
3. DDI_E isn't present

I don't agree... This is already covered on current code. DDI_A_4_LANES is
already being set when enabling DDI_A.

As Zhang mentioned and as my commit message explains, my patch is needed when bios failed to drive edp (In my case it was an intermediate frequency supported panel which was set to 3.24 GHz and bios didn't have support for intermediate frequencies), it will not enable DDIA in which case, it will not set DDI_BUF_CTL and DDI Lane capability will remain 0 (which is DDIA with 2 lanes and DDIE with 2 lanes). So, since the native resolution of that panel was high and couldn't work with 2 lanes. So, ideally we should not rely on bios to set the initial value and set it based upon whether DDI_E is used or not.
So, my patch has some effect :)


thanks

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux