Re: [PATCH 2/3] drm/i915: Preserve the DDI link reversal configuration

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Feb 14, 2013 at 06:15:58PM -0200, Paulo Zanoni wrote:
> Hi
> 
> 2012/12/11 Damien Lespiau <damien.lespiau@gmail.com>:
> > From: Damien Lespiau <damien.lespiau@intel.com>
> >
> > Similarly to:
> >
> >   commit 6a0d1df3d3a0d2370541164eb0595fe35dcd6de3
> >   Author: Damien Lespiau <damien.lespiau@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@intel.com>
> 
> Code looks correct, so:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Queued for -next, thanks for the patch.
-Daniel
> 
> 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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch