diff mbox

[v3] drm/i915/icl: remove port A/E lane sharing limitation.

Message ID 20180206060855.30026-1-mahesh1.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Mahesh Feb. 6, 2018, 6:08 a.m. UTC 
Platforms before Gen11 were sharing lanes between port-A & port-E.
This limitation is no more there.

Changes since V1:
 - optimize the code (Shashank/Jani)
 - create helper function to get max lanes (ville)
Changes since V2:
 - Include BIOS fail fix-up in same helper function (ville)
Changes since V3:
 - remove confusing if/else (jani)
 - group intel_encoder initialization

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 85 ++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 46 deletions(-)

Comments

Kumar, Mahesh March 5, 2018, 6:53 a.m. UTC | #1 
Please review.

thanks,

-Mahesh

On 2/6/2018 11:38 AM, Mahesh Kumar wrote:
> Platforms before Gen11 were sharing lanes between port-A & port-E.
> This limitation is no more there.
>
> Changes since V1:
>   - optimize the code (Shashank/Jani)
>   - create helper function to get max lanes (ville)
> Changes since V2:
>   - Include BIOS fail fix-up in same helper function (ville)
> Changes since V3:
>   - remove confusing if/else (jani)
>   - group intel_encoder initialization
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c | 85 ++++++++++++++++++----------------------
>   1 file changed, 39 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cfcd9cb37d5d..60fe2ba4b29c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2842,39 +2842,45 @@ static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport)
>   	return false;
>   }
>   
> +static int
> +intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(intel_dport->base.base.dev);
> +	enum port port = intel_dport->base.port;
> +	int max_lanes = 4;
> +
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		return max_lanes;
> +
> +	if (port == PORT_A || port == PORT_E) {
> +		if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
> +			max_lanes = port == PORT_A ? 4 : 0;
> +		else
> +			/* Both A and E share 2 lanes */
> +			max_lanes = 2;
> +	}
> +
> +	/*
> +	 * Some BIOS might fail to set this bit on port A if eDP
> +	 * wasn't lit up at boot.  Force this bit set when needed
> +	 * so we use the proper lane count for our calculations.
> +	 */
> +	if (intel_ddi_a_force_4_lanes(intel_dport)) {
> +		DRM_DEBUG_KMS("Forcing DDI_A_4_LANES for port A\n");
> +		intel_dport->saved_port_bits |= DDI_A_4_LANES;
> +		max_lanes = 4;
> +	}
> +
> +	return max_lanes;
> +}
> +
>   void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>   {
>   	struct intel_digital_port *intel_dig_port;
>   	struct intel_encoder *intel_encoder;
>   	struct drm_encoder *encoder;
>   	bool init_hdmi, init_dp, init_lspcon = false;
> -	int max_lanes;
>   
> -	if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) {
> -		switch (port) {
> -		case PORT_A:
> -			max_lanes = 4;
> -			break;
> -		case PORT_E:
> -			max_lanes = 0;
> -			break;
> -		default:
> -			max_lanes = 4;
> -			break;
> -		}
> -	} else {
> -		switch (port) {
> -		case PORT_A:
> -			max_lanes = 2;
> -			break;
> -		case PORT_E:
> -			max_lanes = 2;
> -			break;
> -		default:
> -			max_lanes = 4;
> -			break;
> -		}
> -	}
>   
>   	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>   		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> @@ -2920,10 +2926,17 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>   	intel_encoder->get_config = intel_ddi_get_config;
>   	intel_encoder->suspend = intel_dp_encoder_suspend;
>   	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
> +	intel_encoder->type = INTEL_OUTPUT_DDI;
> +	intel_encoder->power_domain = intel_port_to_power_domain(port);
> +	intel_encoder->port = port;
> +	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> +	intel_encoder->cloneable = 0;
>   
>   	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>   					  (DDI_BUF_PORT_REVERSAL |
>   					   DDI_A_4_LANES);
> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
> +	intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
>   
>   	switch (port) {
>   	case PORT_A:
> @@ -2954,26 +2967,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>   		MISSING_CASE(port);
>   	}
>   
> -	/*
> -	 * Some BIOS might fail to set this bit on port A if eDP
> -	 * wasn't lit up at boot.  Force this bit set when needed
> -	 * so we use the proper lane count for our calculations.
> -	 */
> -	if (intel_ddi_a_force_4_lanes(intel_dig_port)) {
> -		DRM_DEBUG_KMS("Forcing DDI_A_4_LANES for port A\n");
> -		intel_dig_port->saved_port_bits |= DDI_A_4_LANES;
> -		max_lanes = 4;
> -	}
> -
> -	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
> -	intel_dig_port->max_lanes = max_lanes;
> -
> -	intel_encoder->type = INTEL_OUTPUT_DDI;
> -	intel_encoder->power_domain = intel_port_to_power_domain(port);
> -	intel_encoder->port = port;
> -	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> -	intel_encoder->cloneable = 0;
> -
>   	intel_infoframe_init(intel_dig_port);
>   
>   	if (init_dp) {
Jani Nikula March 5, 2018, 10:58 a.m. UTC | #2 
On Mon, 05 Mar 2018, "Kumar, Mahesh" <mahesh1.kumar@intel.com> wrote:
> Please review.

Pushed to drm-intel-next-queued, thanks for the patch.

Personally, I would have split this into 2-3 patches: 1) code movement
to allow 2) abstraction of the function and 3) functional change of the
limit on icl. It would have been faster and easier to review, and easier
to figure out what went wrong in case a bisect ever lands on the commit.

BR,
Jani.


>
> thanks,
>
> -Mahesh
>
> On 2/6/2018 11:38 AM, Mahesh Kumar wrote:
>> Platforms before Gen11 were sharing lanes between port-A & port-E.
>> This limitation is no more there.
>>
>> Changes since V1:
>>   - optimize the code (Shashank/Jani)
>>   - create helper function to get max lanes (ville)
>> Changes since V2:
>>   - Include BIOS fail fix-up in same helper function (ville)
>> Changes since V3:
>>   - remove confusing if/else (jani)
>>   - group intel_encoder initialization
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_ddi.c | 85 ++++++++++++++++++----------------------
>>   1 file changed, 39 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index cfcd9cb37d5d..60fe2ba4b29c 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2842,39 +2842,45 @@ static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport)
>>   	return false;
>>   }
>>   
>> +static int
>> +intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(intel_dport->base.base.dev);
>> +	enum port port = intel_dport->base.port;
>> +	int max_lanes = 4;
>> +
>> +	if (INTEL_GEN(dev_priv) >= 11)
>> +		return max_lanes;
>> +
>> +	if (port == PORT_A || port == PORT_E) {
>> +		if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
>> +			max_lanes = port == PORT_A ? 4 : 0;
>> +		else
>> +			/* Both A and E share 2 lanes */
>> +			max_lanes = 2;
>> +	}
>> +
>> +	/*
>> +	 * Some BIOS might fail to set this bit on port A if eDP
>> +	 * wasn't lit up at boot.  Force this bit set when needed
>> +	 * so we use the proper lane count for our calculations.
>> +	 */
>> +	if (intel_ddi_a_force_4_lanes(intel_dport)) {
>> +		DRM_DEBUG_KMS("Forcing DDI_A_4_LANES for port A\n");
>> +		intel_dport->saved_port_bits |= DDI_A_4_LANES;
>> +		max_lanes = 4;
>> +	}
>> +
>> +	return max_lanes;
>> +}
>> +
>>   void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   {
>>   	struct intel_digital_port *intel_dig_port;
>>   	struct intel_encoder *intel_encoder;
>>   	struct drm_encoder *encoder;
>>   	bool init_hdmi, init_dp, init_lspcon = false;
>> -	int max_lanes;
>>   
>> -	if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) {
>> -		switch (port) {
>> -		case PORT_A:
>> -			max_lanes = 4;
>> -			break;
>> -		case PORT_E:
>> -			max_lanes = 0;
>> -			break;
>> -		default:
>> -			max_lanes = 4;
>> -			break;
>> -		}
>> -	} else {
>> -		switch (port) {
>> -		case PORT_A:
>> -			max_lanes = 2;
>> -			break;
>> -		case PORT_E:
>> -			max_lanes = 2;
>> -			break;
>> -		default:
>> -			max_lanes = 4;
>> -			break;
>> -		}
>> -	}
>>   
>>   	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>>   		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
>> @@ -2920,10 +2926,17 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   	intel_encoder->get_config = intel_ddi_get_config;
>>   	intel_encoder->suspend = intel_dp_encoder_suspend;
>>   	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
>> +	intel_encoder->type = INTEL_OUTPUT_DDI;
>> +	intel_encoder->power_domain = intel_port_to_power_domain(port);
>> +	intel_encoder->port = port;
>> +	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>> +	intel_encoder->cloneable = 0;
>>   
>>   	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>>   					  (DDI_BUF_PORT_REVERSAL |
>>   					   DDI_A_4_LANES);
>> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>> +	intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
>>   
>>   	switch (port) {
>>   	case PORT_A:
>> @@ -2954,26 +2967,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   		MISSING_CASE(port);
>>   	}
>>   
>> -	/*
>> -	 * Some BIOS might fail to set this bit on port A if eDP
>> -	 * wasn't lit up at boot.  Force this bit set when needed
>> -	 * so we use the proper lane count for our calculations.
>> -	 */
>> -	if (intel_ddi_a_force_4_lanes(intel_dig_port)) {
>> -		DRM_DEBUG_KMS("Forcing DDI_A_4_LANES for port A\n");
>> -		intel_dig_port->saved_port_bits |= DDI_A_4_LANES;
>> -		max_lanes = 4;
>> -	}
>> -
>> -	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>> -	intel_dig_port->max_lanes = max_lanes;
>> -
>> -	intel_encoder->type = INTEL_OUTPUT_DDI;
>> -	intel_encoder->power_domain = intel_port_to_power_domain(port);
>> -	intel_encoder->port = port;
>> -	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>> -	intel_encoder->cloneable = 0;
>> -
>>   	intel_infoframe_init(intel_dig_port);
>>   
>>   	if (init_dp) {
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cfcd9cb37d5d..60fe2ba4b29c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2842,39 +2842,45 @@  static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport)
 	return false;
 }
 
+static int
+intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_dport->base.base.dev);
+	enum port port = intel_dport->base.port;
+	int max_lanes = 4;
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		return max_lanes;
+
+	if (port == PORT_A || port == PORT_E) {
+		if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
+			max_lanes = port == PORT_A ? 4 : 0;
+		else
+			/* Both A and E share 2 lanes */
+			max_lanes = 2;
+	}
+
+	/*
+	 * Some BIOS might fail to set this bit on port A if eDP
+	 * wasn't lit up at boot.  Force this bit set when needed
+	 * so we use the proper lane count for our calculations.
+	 */
+	if (intel_ddi_a_force_4_lanes(intel_dport)) {
+		DRM_DEBUG_KMS("Forcing DDI_A_4_LANES for port A\n");
+		intel_dport->saved_port_bits |= DDI_A_4_LANES;
+		max_lanes = 4;
+	}
+
+	return max_lanes;
+}
+
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 {
 	struct intel_digital_port *intel_dig_port;
 	struct intel_encoder *intel_encoder;
 	struct drm_encoder *encoder;
 	bool init_hdmi, init_dp, init_lspcon = false;
-	int max_lanes;
 
-	if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) {
-		switch (port) {
-		case PORT_A:
-			max_lanes = 4;
-			break;
-		case PORT_E:
-			max_lanes = 0;
-			break;
-		default:
-			max_lanes = 4;
-			break;
-		}
-	} else {
-		switch (port) {
-		case PORT_A:
-			max_lanes = 2;
-			break;
-		case PORT_E:
-			max_lanes = 2;
-			break;
-		default:
-			max_lanes = 4;
-			break;
-		}
-	}
 
 	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
 		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
@@ -2920,10 +2926,17 @@  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->get_config = intel_ddi_get_config;
 	intel_encoder->suspend = intel_dp_encoder_suspend;
 	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
+	intel_encoder->type = INTEL_OUTPUT_DDI;
+	intel_encoder->power_domain = intel_port_to_power_domain(port);
+	intel_encoder->port = port;
+	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
+	intel_encoder->cloneable = 0;
 
 	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
 					  (DDI_BUF_PORT_REVERSAL |
 					   DDI_A_4_LANES);
+	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
+	intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
 
 	switch (port) {
 	case PORT_A:
@@ -2954,26 +2967,6 @@  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 		MISSING_CASE(port);
 	}
 
-	/*
-	 * Some BIOS might fail to set this bit on port A if eDP
-	 * wasn't lit up at boot.  Force this bit set when needed
-	 * so we use the proper lane count for our calculations.
-	 */
-	if (intel_ddi_a_force_4_lanes(intel_dig_port)) {
-		DRM_DEBUG_KMS("Forcing DDI_A_4_LANES for port A\n");
-		intel_dig_port->saved_port_bits |= DDI_A_4_LANES;
-		max_lanes = 4;
-	}
-
-	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
-	intel_dig_port->max_lanes = max_lanes;
-
-	intel_encoder->type = INTEL_OUTPUT_DDI;
-	intel_encoder->power_domain = intel_port_to_power_domain(port);
-	intel_encoder->port = port;
-	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
-	intel_encoder->cloneable = 0;
-
 	intel_infoframe_init(intel_dig_port);
 
 	if (init_dp) {