Re: [Intel-gfx] [PATCH 2/2] drm/i915/ICL: Add pre_pll_enable hook for ICL and set DFLEXDPMLE in this hook

2018-10-22 Thread Manasi Navare
On Fri, Oct 19, 2018 at 05:52:14PM -0700, Souza, Jose wrote:
> On Thu, 2018-10-18 at 15:16 -0700, Manasi Navare wrote:
> > In case of Legacy DP connector on TypeC port, the
> > flex IO DPMLE register is set to number of lanes configured
> > by the display driver which will be programmed into DDI_BUF_CTL
> > PORT_WIDTH_SELECTION.
> > This needs to be programmed before enabling the shared PLLs hence
> > add a pre_pll_enable hook for ICL and add this programming in that
> > hook.
> > 
> > Cc: Lucas De Marchi 
> > Cc: Jose Roberto de Souza 
> > Cc: Animesh Manna 
> > Cc: Paulo Zanoni 
> > Signed-off-by: Manasi Navare 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 52
> > 
> >  1 file changed, 52 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 6b9742baa5f2..fd84f88753cb 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3311,6 +3311,56 @@ static void bxt_ddi_pre_pll_enable(struct
> > intel_encoder *encoder,
> > bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> >  }
> >  
> > +static void intel_ddi_set_fia_lane_count(struct intel_encoder
> > *encoder,
> > +const struct intel_crtc_state
> > *pipe_config,
> > +enum port port)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +   struct intel_digital_port *dig_port = enc_to_dig_port(
> > >base);
> > +   enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > +   u32 val = I915_READ(PORT_TX_DFLEXDPMLE1);
> > +   bool lane_reversal = (dig_port->tc_type == TC_PORT_TYPEC) ?
> > false :
> > +   dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> 
> I guess this is more easy to read:
> bool lane_reversal = dig_port->tc_type == TC_PORT_TYPEC && dig_port-
> >saved_port_bits & DDI_BUF_PORT_REVERSAL;

Actually this will not work for TC_PORT_LEGACY since the first condition 
dig_port->tc_type == TC_PORT_TYPEC will
be false and it will set lane_reversal to false irrespective of 
DDI_BUF_PORT_REVERSAL because of the &&

> 
> > +
> > +   val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> > +   switch (pipe_config->lane_count) {
> > +   case 1:
> > +   val |= (lane_reversal) ?
> > DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
> > +   DFLEXDPMLE1_DPMLETC_ML0(tc_port);
> 
> For me the DFLEXDPMLE1_DPMLETC_ML0(tc_port); should be align with '('
> but if bot said that it is okay...

Ok will check the alignment one more time.

> 
> > +   break;
> > +   case 2:
> > +   val |= (lane_reversal) ?
> > DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
> > +   DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
> > +   break;
> > +   case 4:
> > +   val |= DFLEXDPMLE1_DPMLETC_ML_3_0(tc_port);
> > +   break;
> > +   default:
> > +   MISSING_CASE(pipe_config->lane_count);
> > +   }
> > +   I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
> > +}
> > +
> > +static void icl_ddi_pre_pll_enable(struct intel_encoder *encoder,
> > +  const struct intel_crtc_state
> > *pipe_config,
> > +  const struct drm_connector_state
> > *conn_state)
> > +{
> > +   enum port port = encoder->port;
> > +   struct intel_digital_port *dig_port = enc_to_dig_port(
> > >base);
> > +
> > +   if (intel_port_is_combophy(to_i915(encoder->base.dev), port))
> > +   return;
> 
> Combophy ports will have dig_port->tc_type == TC_PORT_UNKNOWN so you
> can drop this.

Ok will do.

Manasi

> 
> With this change:
> Reviewed-by: José Roberto de Souza 
> 
> > +
> > +   /*
> > +* Program the lane count for static/dynamic connections on
> > Type-C ports.
> > +* Skip this step for TBT.
> > +*/
> > +   if (dig_port->tc_type == TC_PORT_TBT)
> > +   return;
> > +
> > +   intel_ddi_set_fia_lane_count(encoder, pipe_config, port);
> > +}
> > +
> >  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> >  {
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > @@ -3828,6 +3878,8 @@ void intel_ddi_init(struct drm_i915_private
> > *dev_priv, enum port port)
> > intel_encoder->enable = intel_enable_ddi;
> > if (IS_GEN9_LP(dev_priv))
> > intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> > +   if (IS_ICELAKE(dev_priv))
> > +   intel_encoder->pre_pll_enable = icl_ddi_pre_pll_enable;
> > intel_encoder->pre_enable = intel_ddi_pre_enable;
> > intel_encoder->disable = intel_disable_ddi;
> > intel_encoder->post_disable = intel_ddi_post_disable;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915/ICL: Add pre_pll_enable hook for ICL and set DFLEXDPMLE in this hook

2018-10-19 Thread Souza, Jose
On Thu, 2018-10-18 at 15:16 -0700, Manasi Navare wrote:
> In case of Legacy DP connector on TypeC port, the
> flex IO DPMLE register is set to number of lanes configured
> by the display driver which will be programmed into DDI_BUF_CTL
> PORT_WIDTH_SELECTION.
> This needs to be programmed before enabling the shared PLLs hence
> add a pre_pll_enable hook for ICL and add this programming in that
> hook.
> 
> Cc: Lucas De Marchi 
> Cc: Jose Roberto de Souza 
> Cc: Animesh Manna 
> Cc: Paulo Zanoni 
> Signed-off-by: Manasi Navare 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 52
> 
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 6b9742baa5f2..fd84f88753cb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3311,6 +3311,56 @@ static void bxt_ddi_pre_pll_enable(struct
> intel_encoder *encoder,
>   bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
>  }
>  
> +static void intel_ddi_set_fia_lane_count(struct intel_encoder
> *encoder,
> +  const struct intel_crtc_state
> *pipe_config,
> +  enum port port)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_digital_port *dig_port = enc_to_dig_port(
> >base);
> + enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> + u32 val = I915_READ(PORT_TX_DFLEXDPMLE1);
> + bool lane_reversal = (dig_port->tc_type == TC_PORT_TYPEC) ?
> false :
> + dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;

I guess this is more easy to read:
bool lane_reversal = dig_port->tc_type == TC_PORT_TYPEC && dig_port-
>saved_port_bits & DDI_BUF_PORT_REVERSAL;

> +
> + val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> + switch (pipe_config->lane_count) {
> + case 1:
> + val |= (lane_reversal) ?
> DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
> + DFLEXDPMLE1_DPMLETC_ML0(tc_port);

For me the DFLEXDPMLE1_DPMLETC_ML0(tc_port); should be align with '('
but if bot said that it is okay...

> + break;
> + case 2:
> + val |= (lane_reversal) ?
> DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
> + DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
> + break;
> + case 4:
> + val |= DFLEXDPMLE1_DPMLETC_ML_3_0(tc_port);
> + break;
> + default:
> + MISSING_CASE(pipe_config->lane_count);
> + }
> + I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
> +}
> +
> +static void icl_ddi_pre_pll_enable(struct intel_encoder *encoder,
> +const struct intel_crtc_state
> *pipe_config,
> +const struct drm_connector_state
> *conn_state)
> +{
> + enum port port = encoder->port;
> + struct intel_digital_port *dig_port = enc_to_dig_port(
> >base);
> +
> + if (intel_port_is_combophy(to_i915(encoder->base.dev), port))
> + return;

Combophy ports will have dig_port->tc_type == TC_PORT_UNKNOWN so you
can drop this.

With this change:
Reviewed-by: José Roberto de Souza 

> +
> + /*
> +  * Program the lane count for static/dynamic connections on
> Type-C ports.
> +  * Skip this step for TBT.
> +  */
> + if (dig_port->tc_type == TC_PORT_TBT)
> + return;
> +
> + intel_ddi_set_fia_lane_count(encoder, pipe_config, port);
> +}
> +
>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
>  {
>   struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> @@ -3828,6 +3878,8 @@ void intel_ddi_init(struct drm_i915_private
> *dev_priv, enum port port)
>   intel_encoder->enable = intel_enable_ddi;
>   if (IS_GEN9_LP(dev_priv))
>   intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> + if (IS_ICELAKE(dev_priv))
> + intel_encoder->pre_pll_enable = icl_ddi_pre_pll_enable;
>   intel_encoder->pre_enable = intel_ddi_pre_enable;
>   intel_encoder->disable = intel_disable_ddi;
>   intel_encoder->post_disable = intel_ddi_post_disable;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/2] drm/i915/ICL: Add pre_pll_enable hook for ICL and set DFLEXDPMLE in this hook

2018-10-18 Thread Manasi Navare
In case of Legacy DP connector on TypeC port, the
flex IO DPMLE register is set to number of lanes configured
by the display driver which will be programmed into DDI_BUF_CTL
PORT_WIDTH_SELECTION.
This needs to be programmed before enabling the shared PLLs hence
add a pre_pll_enable hook for ICL and add this programming in that hook.

Cc: Lucas De Marchi 
Cc: Jose Roberto de Souza 
Cc: Animesh Manna 
Cc: Paulo Zanoni 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_ddi.c | 52 
 1 file changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 6b9742baa5f2..fd84f88753cb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3311,6 +3311,56 @@ static void bxt_ddi_pre_pll_enable(struct intel_encoder 
*encoder,
bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
 }
 
+static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
+const struct intel_crtc_state 
*pipe_config,
+enum port port)
+{
+   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+   struct intel_digital_port *dig_port = enc_to_dig_port(>base);
+   enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+   u32 val = I915_READ(PORT_TX_DFLEXDPMLE1);
+   bool lane_reversal = (dig_port->tc_type == TC_PORT_TYPEC) ? false :
+   dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
+
+   val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
+   switch (pipe_config->lane_count) {
+   case 1:
+   val |= (lane_reversal) ? DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
+   DFLEXDPMLE1_DPMLETC_ML0(tc_port);
+   break;
+   case 2:
+   val |= (lane_reversal) ? DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
+   DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
+   break;
+   case 4:
+   val |= DFLEXDPMLE1_DPMLETC_ML_3_0(tc_port);
+   break;
+   default:
+   MISSING_CASE(pipe_config->lane_count);
+   }
+   I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
+}
+
+static void icl_ddi_pre_pll_enable(struct intel_encoder *encoder,
+  const struct intel_crtc_state *pipe_config,
+  const struct drm_connector_state *conn_state)
+{
+   enum port port = encoder->port;
+   struct intel_digital_port *dig_port = enc_to_dig_port(>base);
+
+   if (intel_port_is_combophy(to_i915(encoder->base.dev), port))
+   return;
+
+   /*
+* Program the lane count for static/dynamic connections on Type-C 
ports.
+* Skip this step for TBT.
+*/
+   if (dig_port->tc_type == TC_PORT_TBT)
+   return;
+
+   intel_ddi_set_fia_lane_count(encoder, pipe_config, port);
+}
+
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
 {
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -3828,6 +3878,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
enum port port)
intel_encoder->enable = intel_enable_ddi;
if (IS_GEN9_LP(dev_priv))
intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+   if (IS_ICELAKE(dev_priv))
+   intel_encoder->pre_pll_enable = icl_ddi_pre_pll_enable;
intel_encoder->pre_enable = intel_ddi_pre_enable;
intel_encoder->disable = intel_disable_ddi;
intel_encoder->post_disable = intel_ddi_post_disable;
-- 
2.18.0

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