Re: [Intel-gfx] [PATCH 1/7] drm/i915: simplify DP/DDI port width macros

2013-05-02 Thread Daniel Vetter
On Thu, May 02, 2013 at 02:38:09PM -0300, Paulo Zanoni wrote:
> 2013/5/2 Daniel Vetter :
> > On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote:
> >> 2013/4/30 Daniel Vetter :
> >> > 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 
> >> > Signed-off-by: Daniel Vetter 
> >>
> >> Nice one.
> >>
> >> Reviewed-by: Paulo Zanoni 
> > 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/7] drm/i915: simplify DP/DDI port width macros

2013-05-02 Thread Paulo Zanoni
2013/5/2 Daniel Vetter :
> On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote:
>> 2013/4/30 Daniel Vetter :
>> > 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 
>> > Signed-off-by: Daniel Vetter 
>>
>> Nice one.
>>
>> Reviewed-by: Paulo Zanoni 
> 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



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


Re: [Intel-gfx] [PATCH 1/7] drm/i915: simplify DP/DDI port width macros

2013-05-02 Thread Daniel Vetter
On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote:
> 2013/4/30 Daniel Vetter :
> > 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 
> > Signed-off-by: Daniel Vetter 
> 
> Nice one.
> 
> Reviewed-by: Paulo Zanoni 
Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/7] drm/i915: simplify DP/DDI port width macros

2013-05-02 Thread Paulo Zanoni
2013/4/30 Daniel Vetter :
> 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 
> Signed-off-by: Daniel Vetter 

Nice one.

Reviewed-by: Paulo Zanoni 

> ---
>  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_SHIFT22
>
>  /* 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_A0x64040
> @@ -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