Hi,

On 2/2/2018 6:26 PM, Jani Nikula wrote:
On Fri, 02 Feb 2018, Mahesh Kumar <mahesh1.ku...@intel.com> 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 <mahesh1.ku...@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
---
  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.
Thanks for review.

-Mahesh

BR,
Jani.

        intel_infoframe_init(intel_dig_port);
if (init_dp) {

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

Reply via email to