Re: [Intel-gfx] [PATCH] drm/i915/icl: remove port A/E lane sharing limitation.

2018-02-05 Thread Jani Nikula 
On Mon, 05 Feb 2018, "Kumar, Mahesh"  wrote:
> Hi,
>
>
> On 2/2/2018 6:26 PM, Jani Nikula wrote:
>> On Fri, 02 Feb 2018, 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)
>>>
>>> Signed-off-by: Mahesh Kumar 
>>> Reviewed-by: Dhinakaran Pandiyan 
>>> ---
>>>   drivers/gpu/drm/i915/intel_ddi.c | 71 
>>> +++-
>>>   1 file changed, 33 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
>>> b/drivers/gpu/drm/i915/intel_ddi.c
>>> index cfcd9cb37d5d..ee9ba78d19c8 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -2842,39 +2842,44 @@ static bool intel_ddi_a_force_4_lanes(struct 
>>> intel_digital_port *dport)
>>> return false;
>>>   }
>>>   
>>> +static int
>>> +intel_ddi_max_lanes(struct drm_i915_private *dev_priv,
>>> +   struct intel_digital_port *intel_dig_port)
>>> +{
>> Please ditch the dev_priv parameter and add:
>>
>>  struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> ok, sure.
>>
>>> +   enum port port = intel_dig_port->base.port;
>>> +   int max_lanes = 4;
>> Unnecessary initialization.
> for ports other than PORT_A/E will have max_lanes=4 that's the reason 
> initializing it here, will fix the usages.
>>
>>> +
>>> +   if (INTEL_GEN(dev_priv) >= 11) {
>>> +   return 4;
>> Please either set max_lanes = 4 here, or remove the else. On early
>> returns, you don't need the else. Having both is confusing.
> yes, agree, will remove the else
>>
>>> +   } else 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_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;
>>> +   }
>>> +
>>> +   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);
>>> @@ -2954,19 +2959,7 @@ 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);
>>> @@ -2974,6 +2967,8 @@ void intel_ddi_init(struct drm_i915_private 
>>> *dev_priv, enum port port)
>>> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>>> intel_encoder->cloneable = 0;
>>>   
>>> +   intel_dig_port->max_lanes = intel_ddi_max_lanes(dev_priv,
>>> +

Re: [Intel-gfx] [PATCH] drm/i915/icl: remove port A/E lane sharing limitation.

2018-02-05 Thread Kumar, Mahesh 

Hi,


On 2/2/2018 6:26 PM, Jani Nikula wrote:

On Fri, 02 Feb 2018, 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)

Signed-off-by: Mahesh Kumar 
Reviewed-by: Dhinakaran Pandiyan 
---
  drivers/gpu/drm/i915/intel_ddi.c | 71 +++-
  1 file changed, 33 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cfcd9cb37d5d..ee9ba78d19c8 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2842,39 +2842,44 @@ static bool intel_ddi_a_force_4_lanes(struct 
intel_digital_port *dport)
return false;
  }
  
+static int

+intel_ddi_max_lanes(struct drm_i915_private *dev_priv,
+   struct intel_digital_port *intel_dig_port)
+{

Please ditch the dev_priv parameter and add:

struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);

ok, sure.



+   enum port port = intel_dig_port->base.port;
+   int max_lanes = 4;

Unnecessary initialization.
for ports other than PORT_A/E will have max_lanes=4 that's the reason 
initializing it here, will fix the usages.



+
+   if (INTEL_GEN(dev_priv) >= 11) {
+   return 4;

Please either set max_lanes = 4 here, or remove the else. On early
returns, you don't need the else. Having both is confusing.

yes, agree, will remove the else



+   } else 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_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;
+   }
+
+   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);
@@ -2954,19 +2959,7 @@ 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);
@@ -2974,6 +2967,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
enum port port)
intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
intel_encoder->cloneable = 0;
  
+	intel_dig_port->max_lanes = intel_ddi_max_lanes(dev_priv,

+   intel_dig_port);

Please keep this at the original location above.
I moved this here because intel_encoder->port was not initialized in 
original location, will pass port along with intel_dig_port & move it to 
original location

Re: [Intel-gfx] [PATCH] drm/i915/icl: remove port A/E lane sharing limitation.

2018-02-02 Thread Jani Nikula 
On Fri, 02 Feb 2018, 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)
>
> Signed-off-by: Mahesh Kumar 
> Reviewed-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 71 
> +++-
>  1 file changed, 33 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index cfcd9cb37d5d..ee9ba78d19c8 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2842,39 +2842,44 @@ static bool intel_ddi_a_force_4_lanes(struct 
> intel_digital_port *dport)
>   return false;
>  }
>  
> +static int
> +intel_ddi_max_lanes(struct drm_i915_private *dev_priv,
> + struct intel_digital_port *intel_dig_port)
> +{

Please ditch the dev_priv parameter and add:

struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);

> + enum port port = intel_dig_port->base.port;
> + int max_lanes = 4;

Unnecessary initialization.

> +
> + if (INTEL_GEN(dev_priv) >= 11) {
> + return 4;

Please either set max_lanes = 4 here, or remove the else. On early
returns, you don't need the else. Having both is confusing.

> + } else 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_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;
> + }
> +
> + 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);
> @@ -2954,19 +2959,7 @@ 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);
> @@ -2974,6 +2967,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
> enum port port)
>   intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>   intel_encoder->cloneable = 0;
>  
> + intel_dig_port->max_lanes = intel_ddi_max_lanes(dev_priv,
> + intel_dig_port);

Please keep this at the original location above.

BR,
Jani.

>   intel_infoframe_init(intel_dig_port);
>  
>   if (init_dp) {

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailma

[Intel-gfx] [PATCH] drm/i915/icl: remove port A/E lane sharing limitation.

2018-02-02 Thread Mahesh Kumar 
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)

Signed-off-by: Mahesh Kumar 
Reviewed-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/i915/intel_ddi.c | 71 +++-
 1 file changed, 33 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cfcd9cb37d5d..ee9ba78d19c8 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2842,39 +2842,44 @@ static bool intel_ddi_a_force_4_lanes(struct 
intel_digital_port *dport)
return false;
 }
 
+static int
+intel_ddi_max_lanes(struct drm_i915_private *dev_priv,
+   struct intel_digital_port *intel_dig_port)
+{
+   enum port port = intel_dig_port->base.port;
+   int max_lanes = 4;
+
+   if (INTEL_GEN(dev_priv) >= 11) {
+   return 4;
+   } else 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_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;
+   }
+
+   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);
@@ -2954,19 +2959,7 @@ 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);
@@ -2974,6 +2967,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
enum port port)
intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
intel_encoder->cloneable = 0;
 
+   intel_dig_port->max_lanes = intel_ddi_max_lanes(dev_priv,
+   intel_dig_port);
intel_infoframe_init(intel_dig_port);
 
if (init_dp) {
-- 
2.14.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/icl: remove port A/E lane sharing limitation.

2018-01-30 Thread Ville Syrjälä 
On Tue, Jan 30, 2018 at 07:52:14PM +, Pandiyan, Dhinakaran wrote:
> 
> On Tue, 2018-01-30 at 14:51 +0530, Mahesh Kumar wrote:
> > From: "Kumar, Mahesh" 
> > 
> > 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)
> > 
> > Signed-off-by: Mahesh Kumar 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 43 
> > +---
> >  1 file changed, 18 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index e51559be2e3b..4bde742a8ff4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2842,6 +2842,23 @@ static bool intel_ddi_a_force_4_lanes(struct 
> > intel_digital_port *dport)
> > return false;
> >  }
> >  
> > +static int
> > +intel_ddi_max_lanes(struct drm_i915_private *dev_priv, enum port port)
> > +{
> > +   if (INTEL_GEN(dev_priv) >= 11)
> > +   return 4;
> > +
> > +   if (port == PORT_A || port == PORT_E) {
> > +   if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
> 
> 
> Is the expectation that bios has already written the correct value
> depending on the board?

We have a fixup later on for BIOS fails. Might be nice to try and
pull that in as well so that we would have all the logic in one
clear place.

> 
> The patch itself looks correct 
> Reviewed-by: Dhinakaran Pandiyan 
> 
> 
> 
> > +   return port == PORT_A ? 4 : 0;
> > +   else
> > +   /* Both A and E share 2 lanes */
> > +   return 2;
> > +   }
> > +
> > +   return 4;
> > +}
> > +
> >  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  {
> > struct intel_digital_port *intel_dig_port;
> > @@ -2850,31 +2867,7 @@ void intel_ddi_init(struct drm_i915_private 
> > *dev_priv, enum port port)
> > 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;
> > -   }
> > -   }
> > +   max_lanes = intel_ddi_max_lanes(dev_priv, port);
> >  
> > init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> >  dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH] drm/i915/icl: remove port A/E lane sharing limitation.

2018-01-30 Thread Pandiyan, Dhinakaran 

On Tue, 2018-01-30 at 14:51 +0530, Mahesh Kumar wrote:
> From: "Kumar, Mahesh" 
> 
> 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)
> 
> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 43 
> +---
>  1 file changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index e51559be2e3b..4bde742a8ff4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2842,6 +2842,23 @@ static bool intel_ddi_a_force_4_lanes(struct 
> intel_digital_port *dport)
>   return false;
>  }
>  
> +static int
> +intel_ddi_max_lanes(struct drm_i915_private *dev_priv, enum port port)
> +{
> + if (INTEL_GEN(dev_priv) >= 11)
> + return 4;
> +
> + if (port == PORT_A || port == PORT_E) {
> + if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)


Is the expectation that bios has already written the correct value
depending on the board?

The patch itself looks correct 
Reviewed-by: Dhinakaran Pandiyan 



> + return port == PORT_A ? 4 : 0;
> + else
> + /* Both A and E share 2 lanes */
> + return 2;
> + }
> +
> + return 4;
> +}
> +
>  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  {
>   struct intel_digital_port *intel_dig_port;
> @@ -2850,31 +2867,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
> enum port port)
>   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;
> - }
> - }
> + max_lanes = intel_ddi_max_lanes(dev_priv, port);
>  
>   init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>dev_priv->vbt.ddi_port_info[port].supports_hdmi);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/icl: remove port A/E lane sharing limitation.

2018-01-30 Thread Mahesh Kumar 
From: "Kumar, Mahesh" 

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)

Signed-off-by: Mahesh Kumar 
---
 drivers/gpu/drm/i915/intel_ddi.c | 43 +---
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e51559be2e3b..4bde742a8ff4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2842,6 +2842,23 @@ static bool intel_ddi_a_force_4_lanes(struct 
intel_digital_port *dport)
return false;
 }
 
+static int
+intel_ddi_max_lanes(struct drm_i915_private *dev_priv, enum port port)
+{
+   if (INTEL_GEN(dev_priv) >= 11)
+   return 4;
+
+   if (port == PORT_A || port == PORT_E) {
+   if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
+   return port == PORT_A ? 4 : 0;
+   else
+   /* Both A and E share 2 lanes */
+   return 2;
+   }
+
+   return 4;
+}
+
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 {
struct intel_digital_port *intel_dig_port;
@@ -2850,31 +2867,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
enum port port)
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;
-   }
-   }
+   max_lanes = intel_ddi_max_lanes(dev_priv, port);
 
init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
 dev_priv->vbt.ddi_port_info[port].supports_hdmi);
-- 
2.14.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx