On Thu, Feb 14, 2013 at 06:15:58PM -0200, Paulo Zanoni wrote:
> 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>
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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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
