Some DisplayPort clean-ups

Some DisplayPort clean-ups

[Damien Lespiau]

While cruising in intel_dp.c, a few things caught my eye.

-- 
Damien

[PATCH 1/4] drm/i915: Fix copy/paste DP vs eDP error in comment

[Damien Lespiau]

It's all about tiny details.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c8382f5..c8a5fb7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -73,7 +73,7 @@ static const u32 hsw_ddi_translations_hdmi[] = {
 };
 
 static const u32 bdw_ddi_translations_edp[] = {
-	0x00FFFFFF, 0x00000012,		/* DP parameters */
+	0x00FFFFFF, 0x00000012,		/* eDP parameters */
 	0x00EBAFFF, 0x00020011,
 	0x00C71FFF, 0x0006000F,
 	0x00FFFFFF, 0x00020011,
-- 
1.8.3.1

[PATCH 2/4] drm/i915: Remove the has_aux_irq = false code path in intel_dp_aux_ch()

[Damien Lespiau]

has_aux_irq is initialized to true and never touched again these days.
Just remove it along with the has_aux_irq = false code path.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7c54f62..7a825db 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -328,7 +328,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
 }
 
 static uint32_t
-intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
+intel_dp_aux_wait_done(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -338,14 +338,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	bool done;
 
 #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
-	if (has_aux_irq)
-		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
-					  msecs_to_jiffies_timeout(10));
-	else
-		done = wait_for_atomic(C, 10) == 0;
+	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
+				  msecs_to_jiffies_timeout(10));
 	if (!done)
-		DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
-			  has_aux_irq);
+		DRM_ERROR("dp aux hw did not signal timeout!\n");
 #undef C
 
 	return status;
@@ -404,7 +400,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	int i, ret, recv_bytes;
 	uint32_t status;
 	int try, precharge, clock = 0;
-	bool has_aux_irq = true;
 	uint32_t timeout;
 
 	/* dp aux is extremely sensitive to irq latency, hence request the
@@ -459,7 +454,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 			/* Send the command and wait for it to complete */
 			I915_WRITE(ch_ctl,
 				   DP_AUX_CH_CTL_SEND_BUSY |
-				   (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
+				   DP_AUX_CH_CTL_INTERRUPT |
 				   timeout |
 				   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
 				   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
@@ -468,7 +463,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
 				   DP_AUX_CH_CTL_RECEIVE_ERROR);
 
-			status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
+			status = intel_dp_aux_wait_done(intel_dp);
 
 			/* Clear done status and any errors */
 			I915_WRITE(ch_ctl,
-- 
1.8.3.1

[PATCH 3/4] drm/i915: Remove if 0'ed static arrays

[Damien Lespiau]

Sweeping some dead code away.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7a825db..07fcc9e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1936,18 +1936,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
 					      DP_LINK_STATUS_SIZE);
 }
 
-#if 0
-static char	*voltage_names[] = {
-	"0.4V", "0.6V", "0.8V", "1.2V"
-};
-static char	*pre_emph_names[] = {
-	"0dB", "3.5dB", "6dB", "9.5dB"
-};
-static char	*link_train_names[] = {
-	"pattern 1", "pattern 2", "idle", "off"
-};
-#endif
-
 /*
  * These are source-specific values; current Intel hardware supports
  * a maximum voltage of 800mV and a maximum pre-emphasis of 6dB
-- 
1.8.3.1

[PATCH 4/4] drm/i915: Cleanup the DP_AUX_CTL macros

[Damien Lespiau]

Let's use a less verbose version to fill the DP_AUX_CTL register.
Improves the readability a little bit. Also sort the fields by their
place in the register.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  8 ++++----
 drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++---------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3be449d..35ae2b3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3105,15 +3105,15 @@
 #define   DP_AUX_CH_CTL_RECEIVE_ERROR	    (1 << 25)
 #define   DP_AUX_CH_CTL_MESSAGE_SIZE_MASK    (0x1f << 20)
 #define   DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT   20
-#define   DP_AUX_CH_CTL_PRECHARGE_2US_MASK   (0xf << 16)
-#define   DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT  16
+#define   DP_AUX_CH_CTL_MESSAGE_SIZE(s) \
+	((s) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT)
+#define   DP_AUX_CH_CTL_PRECHARGE(p)	    ((p) << 16)
 #define   DP_AUX_CH_CTL_AUX_AKSV_SELECT	    (1 << 15)
 #define   DP_AUX_CH_CTL_MANCHESTER_TEST	    (1 << 14)
 #define   DP_AUX_CH_CTL_SYNC_TEST	    (1 << 13)
 #define   DP_AUX_CH_CTL_DEGLITCH_TEST	    (1 << 12)
 #define   DP_AUX_CH_CTL_PRECHARGE_TEST	    (1 << 11)
-#define   DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK    (0x7ff)
-#define   DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT   0
+#define   DP_AUX_CH_CTL_CLOCK_2X(c)	    (c)
 
 /*
  * Computing GMCH M and N values for the Display Port link
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 07fcc9e..a1113af 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -454,14 +454,14 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 			/* Send the command and wait for it to complete */
 			I915_WRITE(ch_ctl,
 				   DP_AUX_CH_CTL_SEND_BUSY |
-				   DP_AUX_CH_CTL_INTERRUPT |
-				   timeout |
-				   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
-				   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
-				   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
 				   DP_AUX_CH_CTL_DONE |
+				   DP_AUX_CH_CTL_INTERRUPT |
 				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
-				   DP_AUX_CH_CTL_RECEIVE_ERROR);
+				   timeout |
+				   DP_AUX_CH_CTL_RECEIVE_ERROR |
+				   DP_AUX_CH_CTL_MESSAGE_SIZE(send_bytes) |
+				   DP_AUX_CH_CTL_PRECHARGE(precharge) |
+				   DP_AUX_CH_CTL_CLOCK_2X(aux_clock_divider));
 
 			status = intel_dp_aux_wait_done(intel_dp);
 
@@ -1599,9 +1599,9 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 	I915_WRITE(EDP_PSR_AUX_DATA2(dev), EDP_PSR_DPCD_NORMAL_OPERATION);
 	I915_WRITE(EDP_PSR_AUX_CTL(dev),
 		   DP_AUX_CH_CTL_TIME_OUT_400us |
-		   (msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
-		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
-		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
+		   DP_AUX_CH_CTL_MESSAGE_SIZE(msg_size) |
+		   DP_AUX_CH_CTL_PRECHARGE(precharge) |
+		   DP_AUX_CH_CTL_CLOCK_2X(aux_clock_divider));
 }
 
 static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
-- 
1.8.3.1

Re: [PATCH 2/4] drm/i915: Remove the has_aux_irq = false code path in intel_dp_aux_ch()

[Paulo Zanoni]

2013/12/3 Damien Lespiau <damien.lespiau@intel.com>:
> has_aux_irq is initialized to true and never touched again these days.
> Just remove it along with the has_aux_irq = false code path.

I have set "has_aux_irq" to false a few times in the last months to
debug things, like the PC8 interrupts. I'm not sure how much we gain
just removing the code.

>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7c54f62..7a825db 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -328,7 +328,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
>  }
>
>  static uint32_t
> -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>  {
>         struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>         struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -338,14 +338,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>         bool done;
>
>  #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> -       if (has_aux_irq)
> -               done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> -                                         msecs_to_jiffies_timeout(10));
> -       else
> -               done = wait_for_atomic(C, 10) == 0;
> +       done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> +                                 msecs_to_jiffies_timeout(10));
>         if (!done)
> -               DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> -                         has_aux_irq);
> +               DRM_ERROR("dp aux hw did not signal timeout!\n");
>  #undef C
>
>         return status;
> @@ -404,7 +400,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>         int i, ret, recv_bytes;
>         uint32_t status;
>         int try, precharge, clock = 0;
> -       bool has_aux_irq = true;
>         uint32_t timeout;
>
>         /* dp aux is extremely sensitive to irq latency, hence request the
> @@ -459,7 +454,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>                         /* Send the command and wait for it to complete */
>                         I915_WRITE(ch_ctl,
>                                    DP_AUX_CH_CTL_SEND_BUSY |
> -                                  (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> +                                  DP_AUX_CH_CTL_INTERRUPT |
>                                    timeout |
>                                    (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
>                                    (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> @@ -468,7 +463,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>                                    DP_AUX_CH_CTL_TIME_OUT_ERROR |
>                                    DP_AUX_CH_CTL_RECEIVE_ERROR);
>
> -                       status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> +                       status = intel_dp_aux_wait_done(intel_dp);
>
>                         /* Clear done status and any errors */
>                         I915_WRITE(ch_ctl,
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 2/4] drm/i915: Remove the has_aux_irq = false code path in intel_dp_aux_ch()

[Daniel Vetter]

On Tue, Dec 03, 2013 at 01:07:46PM -0200, Paulo Zanoni wrote:
> 2013/12/3 Damien Lespiau <damien.lespiau@intel.com>:
> > has_aux_irq is initialized to true and never touched again these days.
> > Just remove it along with the has_aux_irq = false code path.
> 
> I have set "has_aux_irq" to false a few times in the last months to
> debug things, like the PC8 interrupts. I'm not sure how much we gain
> just removing the code.

I also expect this to be generally useful for bring-up - one less thing to
have working before edp works. Hence why I've originally left it in.
-Daniel

> 
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 17 ++++++-----------
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 7c54f62..7a825db 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -328,7 +328,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> >  }
> >
> >  static uint32_t
> > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> >  {
> >         struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >         struct drm_device *dev = intel_dig_port->base.base.dev;
> > @@ -338,14 +338,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> >         bool done;
> >
> >  #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > -       if (has_aux_irq)
> > -               done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > -                                         msecs_to_jiffies_timeout(10));
> > -       else
> > -               done = wait_for_atomic(C, 10) == 0;
> > +       done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > +                                 msecs_to_jiffies_timeout(10));
> >         if (!done)
> > -               DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> > -                         has_aux_irq);
> > +               DRM_ERROR("dp aux hw did not signal timeout!\n");
> >  #undef C
> >
> >         return status;
> > @@ -404,7 +400,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> >         int i, ret, recv_bytes;
> >         uint32_t status;
> >         int try, precharge, clock = 0;
> > -       bool has_aux_irq = true;
> >         uint32_t timeout;
> >
> >         /* dp aux is extremely sensitive to irq latency, hence request the
> > @@ -459,7 +454,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> >                         /* Send the command and wait for it to complete */
> >                         I915_WRITE(ch_ctl,
> >                                    DP_AUX_CH_CTL_SEND_BUSY |
> > -                                  (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > +                                  DP_AUX_CH_CTL_INTERRUPT |
> >                                    timeout |
> >                                    (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> >                                    (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> > @@ -468,7 +463,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> >                                    DP_AUX_CH_CTL_TIME_OUT_ERROR |
> >                                    DP_AUX_CH_CTL_RECEIVE_ERROR);
> >
> > -                       status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> > +                       status = intel_dp_aux_wait_done(intel_dp);
> >
> >                         /* Clear done status and any errors */
> >                         I915_WRITE(ch_ctl,
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > 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

Re: [PATCH 3/4] drm/i915: Remove if 0'ed static arrays

[Daniel Vetter]

On Tue, Dec 03, 2013 at 01:47:00PM +0000, Damien Lespiau wrote:
> Sweeping some dead code away.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

This one and patch 1 merged to dinq, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7a825db..07fcc9e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1936,18 +1936,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
>  					      DP_LINK_STATUS_SIZE);
>  }
>  
> -#if 0
> -static char	*voltage_names[] = {
> -	"0.4V", "0.6V", "0.8V", "1.2V"
> -};
> -static char	*pre_emph_names[] = {
> -	"0dB", "3.5dB", "6dB", "9.5dB"
> -};
> -static char	*link_train_names[] = {
> -	"pattern 1", "pattern 2", "idle", "off"
> -};
> -#endif
> -
>  /*
>   * These are source-specific values; current Intel hardware supports
>   * a maximum voltage of 800mV and a maximum pre-emphasis of 6dB
> -- 
> 1.8.3.1
> 
> _______________________________________________
> 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