[Intel-gfx] [PATCH 2/3] drm/i915: Preserve the DDI link reversal configuration

[Paulo Zanoni]

Hi

2012/12/11 Damien Lespiau <damien.lespiau at gmail.com>:
> From: Damien Lespiau <damien.lespiau at intel.com>
>
> Similarly to:
>
>   commit 6a0d1df3d3a0d2370541164eb0595fe35dcd6de3
>   Author: Damien Lespiau <damien.lespiau at intel.com>
>   Date:   Tue Dec 11 15:18:28 2012 +0000
>
>       drm/i915: Preserve the FDI line reversal override bit on CPT
>
> DDI port support lane reversal to easy the PCB layouting work. Let's
> preserve the bit configured by the BIOS (until we find how to correctly
> retrieve the information from the VBT, but this does sound more fragile
> then just relying on the BIOS that has, hopefully, been validated
> already.
>
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>

Code looks correct, so:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

My only optional bikeshedding would be about the fact that we're not
making optimal usage of the struct_to_another_struct() conversion
macros. Whenever you declare "struct intel_digital_port" you can
basically redefine all the other pointers with
intel_dig_port->something instead of using the macros that maybe call
container_of.

> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_ddi.c | 19 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  3 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ceeac97..3f2e6cf 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4519,6 +4519,7 @@
>  #define  DDI_BUF_EMP_800MV_0DB_HSW             (7<<24)   /* Sel7 */
>  #define  DDI_BUF_EMP_800MV_3_5DB_HSW           (8<<24)   /* Sel8 */
>  #define  DDI_BUF_EMP_MASK                      (0xf<<24)
> +#define  DDI_BUF_PORT_REVERSAL                 (1<<16)
>  #define  DDI_BUF_IS_IDLE                       (1<<7)
>  #define  DDI_A_4_LANES                         (1<<4)
>  #define  DDI_PORT_WIDTH_X1                     (0<<1)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cb2dcc6..59b778d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -201,7 +201,10 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>                                         DP_TP_CTL_LINK_TRAIN_PAT1 |
>                                         DP_TP_CTL_ENABLE);
>
> -               /* Configure and enable DDI_BUF_CTL for DDI E with next voltage */
> +               /* Configure and enable DDI_BUF_CTL for DDI E with next voltage.
> +                * DDI E does not support port reversal, the functionality is
> +                * achieved on the PCH side in FDI_RX_CTL, so no need to set the
> +                * port reversal bit */
>                 I915_WRITE(DDI_BUF_CTL(PORT_E),
>                            DDI_BUF_CTL_ENABLE |
>                            ((intel_crtc->fdi_lanes - 1) << 1) |
> @@ -675,8 +678,11 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
>
>         if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
>                 struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +               struct intel_digital_port *intel_dig_port =
> +                       enc_to_dig_port(encoder);
>
> -               intel_dp->DP = DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
> +               intel_dp->DP = intel_dig_port->port_reversal |
> +                              DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
>                 switch (intel_dp->lane_count) {
>                 case 1:
>                         intel_dp->DP |= DDI_PORT_WIDTH_X1;
> @@ -1289,11 +1295,15 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
>         int type = intel_encoder->type;
>
>         if (type == INTEL_OUTPUT_HDMI) {
> +               struct intel_digital_port *intel_dig_port =
> +                       enc_to_dig_port(encoder);
> +
>                 /* In HDMI/DVI mode, the port width, and swing/emphasis values
>                  * are ignored so nothing special needs to be done besides
>                  * enabling the port.
>                  */
> -               I915_WRITE(DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE);
> +               I915_WRITE(DDI_BUF_CTL(port),
> +                          intel_dig_port->port_reversal | DDI_BUF_CTL_ENABLE);
>         } else if (type == INTEL_OUTPUT_EDP) {
>                 struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>
> @@ -1455,6 +1465,7 @@ static const struct drm_encoder_helper_funcs intel_ddi_helper_funcs = {
>
>  void intel_ddi_init(struct drm_device *dev, enum port port)
>  {
> +       struct drm_i915_private *dev_priv = dev->dev_private;
>         struct intel_digital_port *intel_dig_port;
>         struct intel_encoder *intel_encoder;
>         struct drm_encoder *encoder;
> @@ -1495,6 +1506,8 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>         intel_encoder->get_hw_state = intel_ddi_get_hw_state;
>
>         intel_dig_port->port = port;
> +       intel_dig_port->port_reversal = I915_READ(DDI_BUF_CTL(port)) &
> +                                       DDI_BUF_PORT_REVERSAL;
>         if (hdmi_connector)
>                 intel_dig_port->hdmi.sdvox_reg = DDI_BUF_CTL(port);
>         else
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8a1bd4a..afa45a9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -377,6 +377,7 @@ struct intel_dp {
>  struct intel_digital_port {
>         struct intel_encoder base;
>         enum port port;
> +       u32 port_reversal;
>         struct intel_dp dp;
>         struct intel_hdmi hdmi;
>  };
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni