| Message ID | 1367323306-13605-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>: > If we ever leak a non-DP compliant port width through here, we have a > pretty serious issue. So just rip out all these WARNs - if we need > them it's probably better to have them at a central place where we > compute the dp lane count. > > Also use the new DDI width macro for FDI mode. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Nice one. Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 11 ++--------- > drivers/gpu/drm/i915/intel_ddi.c | 34 ++-------------------------------- > drivers/gpu/drm/i915/intel_dp.c | 12 +----------- > 3 files changed, 5 insertions(+), 52 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 95ae5cf..d84d694 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2664,9 +2664,7 @@ > #define DP_PRE_EMPHASIS_SHIFT 22 > > /* How many wires to use. I guess 3 was too hard */ > -#define DP_PORT_WIDTH_1 (0 << 19) > -#define DP_PORT_WIDTH_2 (1 << 19) > -#define DP_PORT_WIDTH_4 (3 << 19) > +#define DP_PORT_WIDTH(width) (((width) - 1) << 19) > #define DP_PORT_WIDTH_MASK (7 << 19) > > /* Mystic DPCD version 1.1 special mode */ > @@ -4751,9 +4749,6 @@ > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) > #define TRANS_DDI_BFI_ENABLE (1<<4) > -#define TRANS_DDI_PORT_WIDTH_X1 (0<<1) > -#define TRANS_DDI_PORT_WIDTH_X2 (1<<1) > -#define TRANS_DDI_PORT_WIDTH_X4 (3<<1) > > /* DisplayPort Transport Control */ > #define DP_TP_CTL_A 0x64040 > @@ -4797,9 +4792,7 @@ > #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) > -#define DDI_PORT_WIDTH_X2 (1<<1) > -#define DDI_PORT_WIDTH_X4 (3<<1) > +#define DDI_PORT_WIDTH(width) (((width) - 1) << 1) > #define DDI_INIT_DISPLAY_DETECTED (1<<0) > > /* DDI Buffer Translations */ > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 72941f9..841c9a9 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -687,22 +687,7 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder, > > 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; > - break; > - case 2: > - intel_dp->DP |= DDI_PORT_WIDTH_X2; > - break; > - case 4: > - intel_dp->DP |= DDI_PORT_WIDTH_X4; > - break; > - default: > - intel_dp->DP |= DDI_PORT_WIDTH_X4; > - WARN(1, "Unexpected DP lane count %d\n", > - intel_dp->lane_count); > - break; > - } > + intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count); > > if (intel_dp->has_audio) { > DRM_DEBUG_DRIVER("DP audio on pipe %c on DDI\n", > @@ -1031,22 +1016,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) > > temp |= TRANS_DDI_MODE_SELECT_DP_SST; > > - switch (intel_dp->lane_count) { > - case 1: > - temp |= TRANS_DDI_PORT_WIDTH_X1; > - break; > - case 2: > - temp |= TRANS_DDI_PORT_WIDTH_X2; > - break; > - case 4: > - temp |= TRANS_DDI_PORT_WIDTH_X4; > - break; > - default: > - temp |= TRANS_DDI_PORT_WIDTH_X4; > - WARN(1, "Unsupported lane count %d\n", > - intel_dp->lane_count); > - } > - > + intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count); > } else { > WARN(1, "Invalid encoder type %d for pipe %c\n", > intel_encoder->type, pipe_name(pipe)); > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 8759fb1..99f798a 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -891,18 +891,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, > > /* Handle DP bits in common between all three register formats */ > intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0; > + intel_dp->DP |= DP_PORT_WIDTH(intel_dp->lane_count); > > - switch (intel_dp->lane_count) { > - case 1: > - intel_dp->DP |= DP_PORT_WIDTH_1; > - break; > - case 2: > - intel_dp->DP |= DP_PORT_WIDTH_2; > - break; > - case 4: > - intel_dp->DP |= DP_PORT_WIDTH_4; > - break; > - } > if (intel_dp->has_audio) { > DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n", > pipe_name(intel_crtc->pipe)); > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote: > 2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>: > > If we ever leak a non-DP compliant port width through here, we have a > > pretty serious issue. So just rip out all these WARNs - if we need > > them it's probably better to have them at a central place where we > > compute the dp lane count. > > > > Also use the new DDI width macro for FDI mode. > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Nice one. > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Queued for -next, thanks for the review. -Daniel
2013/5/2 Daniel Vetter <daniel@ffwll.ch>: > On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote: >> 2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>: >> > If we ever leak a non-DP compliant port width through here, we have a >> > pretty serious issue. So just rip out all these WARNs - if we need >> > them it's probably better to have them at a central place where we >> > compute the dp lane count. >> > >> > Also use the new DDI width macro for FDI mode. >> > >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> Nice one. >> >> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > Queued for -next, thanks for the review. And now that I've actually booted a Kernel with the patch I see that the chunk inside intel_ddi_enable_transcoder_func is wrong. We should assign to the "temp" variable, not "intel_dp->DP". This gives me a nice black screen on DP. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Thu, May 02, 2013 at 02:38:09PM -0300, Paulo Zanoni wrote: > 2013/5/2 Daniel Vetter <daniel@ffwll.ch>: > > On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote: > >> 2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>: > >> > If we ever leak a non-DP compliant port width through here, we have a > >> > pretty serious issue. So just rip out all these WARNs - if we need > >> > them it's probably better to have them at a central place where we > >> > compute the dp lane count. > >> > > >> > Also use the new DDI width macro for FDI mode. > >> > > >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> > >> Nice one. > >> > >> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Queued for -next, thanks for the review. > > And now that I've actually booted a Kernel with the patch I see that > the chunk inside intel_ddi_enable_transcoder_func is wrong. We should > assign to the "temp" variable, not "intel_dp->DP". This gives me a > nice black screen on DP. Oops. Ok, I've pushed an updated version, that one better? -Daniel
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 95ae5cf..d84d694 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2664,9 +2664,7 @@ #define DP_PRE_EMPHASIS_SHIFT 22 /* How many wires to use. I guess 3 was too hard */ -#define DP_PORT_WIDTH_1 (0 << 19) -#define DP_PORT_WIDTH_2 (1 << 19) -#define DP_PORT_WIDTH_4 (3 << 19) +#define DP_PORT_WIDTH(width) (((width) - 1) << 19) #define DP_PORT_WIDTH_MASK (7 << 19) /* Mystic DPCD version 1.1 special mode */ @@ -4751,9 +4749,6 @@ #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) #define TRANS_DDI_BFI_ENABLE (1<<4) -#define TRANS_DDI_PORT_WIDTH_X1 (0<<1) -#define TRANS_DDI_PORT_WIDTH_X2 (1<<1) -#define TRANS_DDI_PORT_WIDTH_X4 (3<<1) /* DisplayPort Transport Control */ #define DP_TP_CTL_A 0x64040 @@ -4797,9 +4792,7 @@ #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) -#define DDI_PORT_WIDTH_X2 (1<<1) -#define DDI_PORT_WIDTH_X4 (3<<1) +#define DDI_PORT_WIDTH(width) (((width) - 1) << 1) #define DDI_INIT_DISPLAY_DETECTED (1<<0) /* DDI Buffer Translations */ diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 72941f9..841c9a9 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -687,22 +687,7 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder, 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; - break; - case 2: - intel_dp->DP |= DDI_PORT_WIDTH_X2; - break; - case 4: - intel_dp->DP |= DDI_PORT_WIDTH_X4; - break; - default: - intel_dp->DP |= DDI_PORT_WIDTH_X4; - WARN(1, "Unexpected DP lane count %d\n", - intel_dp->lane_count); - break; - } + intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count); if (intel_dp->has_audio) { DRM_DEBUG_DRIVER("DP audio on pipe %c on DDI\n", @@ -1031,22 +1016,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) temp |= TRANS_DDI_MODE_SELECT_DP_SST; - switch (intel_dp->lane_count) { - case 1: - temp |= TRANS_DDI_PORT_WIDTH_X1; - break; - case 2: - temp |= TRANS_DDI_PORT_WIDTH_X2; - break; - case 4: - temp |= TRANS_DDI_PORT_WIDTH_X4; - break; - default: - temp |= TRANS_DDI_PORT_WIDTH_X4; - WARN(1, "Unsupported lane count %d\n", - intel_dp->lane_count); - } - + intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count); } else { WARN(1, "Invalid encoder type %d for pipe %c\n", intel_encoder->type, pipe_name(pipe)); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8759fb1..99f798a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -891,18 +891,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, /* Handle DP bits in common between all three register formats */ intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0; + intel_dp->DP |= DP_PORT_WIDTH(intel_dp->lane_count); - switch (intel_dp->lane_count) { - case 1: - intel_dp->DP |= DP_PORT_WIDTH_1; - break; - case 2: - intel_dp->DP |= DP_PORT_WIDTH_2; - break; - case 4: - intel_dp->DP |= DP_PORT_WIDTH_4; - break; - } if (intel_dp->has_audio) { DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n", pipe_name(intel_crtc->pipe));
If we ever leak a non-DP compliant port width through here, we have a pretty serious issue. So just rip out all these WARNs - if we need them it's probably better to have them at a central place where we compute the dp lane count. Also use the new DDI width macro for FDI mode. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_reg.h | 11 ++--------- drivers/gpu/drm/i915/intel_ddi.c | 34 ++-------------------------------- drivers/gpu/drm/i915/intel_dp.c | 12 +----------- 3 files changed, 5 insertions(+), 52 deletions(-)