[PATCH 0/7] drm/i915: Pile of display related patches

[PATCH 0/7] drm/i915: Pile of display related patches

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

These are mostly prep work for some future stuff.

Rafael Barbalho (1):
  drm/i915: Fix framecount offset

Ville Syrjälä (6):
  drm/i915: Refactor gmch hpd irq handling
  drm/i915: Move DP M/N setup from update_pll to mode_set for gmch
    platforms
  drm/i915: Split dp post_disable hooks
  drm/i915: Warn when DPIO read returns 0xffffffff
  drm/i915: Provide a bit more info when pipestat bits are wrong
  drm/i915: Use enum plane instaad of numbers

 drivers/gpu/drm/i915/i915_irq.c       | 83 +++++++++++++++++------------------
 drivers/gpu/drm/i915/i915_reg.h       |  6 +--
 drivers/gpu/drm/i915/intel_display.c  | 23 +++++-----
 drivers/gpu/drm/i915/intel_dp.c       | 23 ++++++----
 drivers/gpu/drm/i915/intel_sideband.c |  4 ++
 5 files changed, 72 insertions(+), 67 deletions(-)

-- 
1.8.3.2

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

[PATCH 1/7] drm/i915: Refactor gmch hpd irq handling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pull all the gmch platform hotplug interrupt handling into one
function.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 71 +++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0858189..6d26719 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1647,6 +1647,34 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 		gmbus_irq_handler(dev);
 }
 
+static void i9xx_hpd_irq_handler(struct drm_device *dev, u32 iir)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	u32 hotplug_status;
+
+	if ((iir & I915_DISPLAY_PORT_INTERRUPT) == 0)
+		return;
+
+	hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+
+	if (IS_G4X(dev)) {
+		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
+
+		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x);
+	} else {
+		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
+
+		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
+	}
+
+	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
+	    hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
+		dp_aux_irq_handler(dev);
+
+	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
+	POSTING_READ(PORT_HOTPLUG_STAT);
+}
+
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
@@ -1669,19 +1697,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		valleyview_pipestat_irq_handler(dev, iir);
 
 		/* Consume port.  Then clear IIR or we'll miss events */
-		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
-			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
-
-			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
-
-			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
-				dp_aux_irq_handler(dev);
-
-			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			I915_READ(PORT_HOTPLUG_STAT);
-		}
-
+		i9xx_hpd_irq_handler(dev, iir);
 
 		if (pm_iir)
 			gen6_rps_irq_handler(dev_priv, pm_iir);
@@ -3713,16 +3729,8 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 			break;
 
 		/* Consume port.  Then clear IIR or we'll miss events */
-		if ((I915_HAS_HOTPLUG(dev)) &&
-		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
-			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
-
-			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
-
-			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			POSTING_READ(PORT_HOTPLUG_STAT);
-		}
+		if (I915_HAS_HOTPLUG(dev))
+			i9xx_hpd_irq_handler(dev, iir);
 
 		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
@@ -3956,22 +3964,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 		ret = IRQ_HANDLED;
 
 		/* Consume port.  Then clear IIR or we'll miss events */
-		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
-			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-			u32 hotplug_trigger = hotplug_status & (IS_G4X(dev) ?
-								  HOTPLUG_INT_STATUS_G4X :
-								  HOTPLUG_INT_STATUS_I915);
-
-			intel_hpd_irq_handler(dev, hotplug_trigger,
-					      IS_G4X(dev) ? hpd_status_g4x : hpd_status_i915);
-
-			if (IS_G4X(dev) &&
-			    (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X))
-				dp_aux_irq_handler(dev);
-
-			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			I915_READ(PORT_HOTPLUG_STAT);
-		}
+		i9xx_hpd_irq_handler(dev, iir);
 
 		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
-- 
1.8.3.2

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

[PATCH 2/7] drm/i915: Move DP M/N setup from update_pll to mode_set for gmch platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There's no point in hiding the DP M/N setup in the update_pll functions.
Just move it to the mode_set function.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ecc01f5..add940c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5245,9 +5245,6 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
 	crtc->config.dpll_hw_state.dpll_md = dpll_md;
 
-	if (crtc->config.has_dp_encoder)
-		intel_dp_set_m_n(crtc);
-
 	mutex_unlock(&dev_priv->dpio_lock);
 }
 
@@ -5325,9 +5322,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
 			<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
 		crtc->config.dpll_hw_state.dpll_md = dpll_md;
 	}
-
-	if (crtc->config.has_dp_encoder)
-		intel_dp_set_m_n(crtc);
 }
 
 static void i8xx_update_pll(struct intel_crtc *crtc,
@@ -5656,6 +5650,9 @@ skip_dpll:
 			dspcntr |= DISPPLANE_SEL_PIPE_B;
 	}
 
+	if (intel_crtc->config.has_dp_encoder)
+		intel_dp_set_m_n(intel_crtc);
+
 	intel_set_pipe_timings(intel_crtc);
 
 	/* pipesrc and dspsize control the size that is scaled from,
-- 
1.8.3.2

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

[PATCH 3/7] drm/i915: Split dp post_disable hooks

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split the post_disable hooks for DP to g4x and vlv variants. We'll
need another variant soon, so this should make it look a bit cleaner.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a2a0b01..c33971e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1782,17 +1782,23 @@ static void intel_disable_dp(struct intel_encoder *encoder)
 		intel_dp_link_down(intel_dp);
 }
 
-static void intel_post_disable_dp(struct intel_encoder *encoder)
+static void g4x_post_disable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	enum port port = dp_to_dig_port(intel_dp)->port;
-	struct drm_device *dev = encoder->base.dev;
 
-	if (port == PORT_A || IS_VALLEYVIEW(dev)) {
-		intel_dp_link_down(intel_dp);
-		if (!IS_VALLEYVIEW(dev))
-			ironlake_edp_pll_off(intel_dp);
-	}
+	if (port != PORT_A)
+		return;
+
+	intel_dp_link_down(intel_dp);
+	ironlake_edp_pll_off(intel_dp);
+}
+
+static void vlv_post_disable_dp(struct intel_encoder *encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	intel_dp_link_down(intel_dp);
 }
 
 static void intel_enable_dp(struct intel_encoder *encoder)
@@ -3836,16 +3842,17 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder->compute_config = intel_dp_compute_config;
 	intel_encoder->mode_set = intel_dp_mode_set;
 	intel_encoder->disable = intel_disable_dp;
-	intel_encoder->post_disable = intel_post_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
 	intel_encoder->get_config = intel_dp_get_config;
 	if (IS_VALLEYVIEW(dev)) {
 		intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = vlv_pre_enable_dp;
 		intel_encoder->enable = vlv_enable_dp;
+		intel_encoder->post_disable = vlv_post_disable_dp;
 	} else {
 		intel_encoder->pre_enable = g4x_pre_enable_dp;
 		intel_encoder->enable = g4x_enable_dp;
+		intel_encoder->post_disable = g4x_post_disable_dp;
 	}
 
 	intel_dig_port->port = port;
-- 
1.8.3.2

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

[PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

DPIO reads from groups/broadcast register offsets for PCS and
TX return all 1's. If that result gets used for something
we'll probably end up doing something wrong. So warn when that
happens.

FIXME there might be some registers where all 1's is a valid value,
so ideally we should check the register offset instead...

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sideband.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 0954f13..c1e56f5 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -182,6 +182,10 @@ u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg)
 
 	vlv_sideband_rw(dev_priv, DPIO_DEVFN, DPIO_PHY_IOSF_PORT(DPIO_PHY(pipe)),
 			DPIO_OPCODE_REG_READ, reg, &val);
+
+	WARN(val == 0xffffffff, "DPIO read pipe %c reg 0x%x == 0x%x\n",
+	     pipe_name(pipe), reg, val);
+
 	return val;
 }
 
-- 
1.8.3.2

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

[PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Print the enable_mask and status_mask from
__i915_{enable,disable}_pipestat() when the called has messed them up
somehow.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6d26719..407742f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -503,8 +503,10 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
 
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
-	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
+	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
+		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
+		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
+		      pipe, enable_mask, status_mask))
 		return;
 
 	if ((pipestat & enable_mask) == enable_mask)
@@ -527,8 +529,10 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
 
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
-	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
+	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
+		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
+		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
+		      pipe, enable_mask, status_mask))
 		return;
 
 	if ((pipestat & enable_mask) == 0)
-- 
1.8.3.2

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

[PATCH 6/7] drm/i915: Fix framecount offset

[ville.syrjala]

From: Rafael Barbalho <rafael.barbalho@intel.com>

The framecount register was still using the old PIPE macro instead
of the new PIPE2 macro

Signed-off-by: Rafael Barbalho <rafael.barbalho@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a47b4c3..b6441da 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3623,9 +3623,9 @@ enum punit_power_well {
 #define   PIPE_PIXEL_MASK         0x00ffffff
 #define   PIPE_PIXEL_SHIFT        0
 /* GM45+ just has to be different */
-#define _PIPEA_FRMCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70040)
-#define _PIPEA_FLIPCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70044)
-#define PIPE_FRMCOUNT_GM45(pipe) _PIPE(pipe, _PIPEA_FRMCOUNT_GM45, _PIPEB_FRMCOUNT_GM45)
+#define _PIPEA_FRMCOUNT_GM45	0x70040
+#define _PIPEA_FLIPCOUNT_GM45	0x70044
+#define PIPE_FRMCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FRMCOUNT_GM45)
 
 /* Cursor A & B regs */
 #define _CURACNTR		(dev_priv->info.display_mmio_offset + 0x70080)
-- 
1.8.3.2

[PATCH 7/7] drm/i915: Use enum plane instaad of numbers

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Change the update_plane() plane checks to use enum plane, and
also fix up the error message to say something that's not total
nonsense.

FIXME killing the checks entirely is probably a better idea

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index add940c..9a50b64 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2167,11 +2167,11 @@ static int i9xx_update_primary_plane(struct drm_crtc *crtc,
 	u32 reg;
 
 	switch (plane) {
-	case 0:
-	case 1:
+	case PLANE_A:
+	case PLANE_B:
 		break;
 	default:
-		DRM_ERROR("Can't update plane %c in SAREA\n", plane_name(plane));
+		DRM_ERROR("Can't update plane %c\n", plane_name(plane));
 		return -EINVAL;
 	}
 
@@ -2268,12 +2268,12 @@ static int ironlake_update_primary_plane(struct drm_crtc *crtc,
 	u32 reg;
 
 	switch (plane) {
-	case 0:
-	case 1:
-	case 2:
+	case PLANE_A:
+	case PLANE_B:
+	case PLANE_C:
 		break;
 	default:
-		DRM_ERROR("Can't update plane %c in SAREA\n", plane_name(plane));
+		DRM_ERROR("Can't update plane %c\n", plane_name(plane));
 		return -EINVAL;
 	}
 
-- 
1.8.3.2

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

Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling

[Chris Wilson]

On Mon, Mar 31, 2014 at 06:21:24PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull all the gmch platform hotplug interrupt handling into one
> function.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 71 +++++++++++++++++++----------------------
>  1 file changed, 32 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 0858189..6d26719 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1647,6 +1647,34 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
>  		gmbus_irq_handler(dev);
>  }
>  
> +static void i9xx_hpd_irq_handler(struct drm_device *dev, u32 iir)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	u32 hotplug_status;
> +
> +	if ((iir & I915_DISPLAY_PORT_INTERRUPT) == 0)
> +		return;
> +
> +	hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
> +
> +	if (IS_G4X(dev)) {
> +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> +
> +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x);
> +	} else {
> +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> +
> +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
> +	}
> +
> +	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
> +	    hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> +		dp_aux_irq_handler(dev);
> +
> +	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> +	POSTING_READ(PORT_HOTPLUG_STAT);
> +}

Hmm, after some thought I am in favour of the function as a readibility
improvement. However, I would prefer to have the iir check inlined
into the caller.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling

[Ville Syrjälä]

On Mon, Mar 31, 2014 at 04:49:28PM +0100, Chris Wilson wrote:
> On Mon, Mar 31, 2014 at 06:21:24PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Pull all the gmch platform hotplug interrupt handling into one
> > function.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 71 +++++++++++++++++++----------------------
> >  1 file changed, 32 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 0858189..6d26719 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1647,6 +1647,34 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
> >  		gmbus_irq_handler(dev);
> >  }
> >  
> > +static void i9xx_hpd_irq_handler(struct drm_device *dev, u32 iir)
> > +{
> > +	drm_i915_private_t *dev_priv = dev->dev_private;
> > +	u32 hotplug_status;
> > +
> > +	if ((iir & I915_DISPLAY_PORT_INTERRUPT) == 0)
> > +		return;
> > +
> > +	hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
> > +
> > +	if (IS_G4X(dev)) {
> > +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> > +
> > +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x);
> > +	} else {
> > +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> > +
> > +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
> > +	}
> > +
> > +	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
> > +	    hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> > +		dp_aux_irq_handler(dev);
> > +
> > +	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> > +	POSTING_READ(PORT_HOTPLUG_STAT);
> > +}
> 
> Hmm, after some thought I am in favour of the function as a readibility
> improvement. However, I would prefer to have the iir check inlined
> into the caller.

I had it like that originally, but I moved it into the function when I
realized all the callers have the exact same check, and it looked a bit
out of place next to the other "sub" irq handlers which do the IIR
checks themselves.

But I'm not really attached to the idea, so I can change it back if
that's the consensus.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling

[Daniel Vetter]

On Mon, Mar 31, 2014 at 6:35 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>> Hmm, after some thought I am in favour of the function as a readibility
>> improvement. However, I would prefer to have the iir check inlined
>> into the caller.
>
> I had it like that originally, but I moved it into the function when I
> realized all the callers have the exact same check, and it looked a bit
> out of place next to the other "sub" irq handlers which do the IIR
> checks themselves.
>
> But I'm not really attached to the idea, so I can change it back if
> that's the consensus.

In our shared interrupt handling code we have both cases, where either
the helper function itself of the caller checks for the bits. Or
sometimes both even. I'm ok with whatever approach makes sense most.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 7/7] drm/i915: Use enum plane instaad of numbers

[Daniel Vetter]

On Mon, Mar 31, 2014 at 06:21:30PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Change the update_plane() plane checks to use enum plane, and
> also fix up the error message to say something that's not total
> nonsense.
> 
> FIXME killing the checks entirely is probably a better idea

At least killing the totally outdated SAREA comment would be good ;-) But
yeah I really don't see much point in this, especially since we'll
happily frob plane C on ilk/snb despite that we're only supporting it on
ivb+ really.

Looking through git history with git blame it seems like this went defunct
somewhere in the large modesetting rewrite. Or even earlier ... in any
case very confusing history and ripe for the bin.
-Daniel

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index add940c..9a50b64 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2167,11 +2167,11 @@ static int i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	u32 reg;
>  
>  	switch (plane) {
> -	case 0:
> -	case 1:
> +	case PLANE_A:
> +	case PLANE_B:
>  		break;
>  	default:
> -		DRM_ERROR("Can't update plane %c in SAREA\n", plane_name(plane));
> +		DRM_ERROR("Can't update plane %c\n", plane_name(plane));
>  		return -EINVAL;
>  	}
>  
> @@ -2268,12 +2268,12 @@ static int ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	u32 reg;
>  
>  	switch (plane) {
> -	case 0:
> -	case 1:
> -	case 2:
> +	case PLANE_A:
> +	case PLANE_B:
> +	case PLANE_C:
>  		break;
>  	default:
> -		DRM_ERROR("Can't update plane %c in SAREA\n", plane_name(plane));
> +		DRM_ERROR("Can't update plane %c\n", plane_name(plane));
>  		return -EINVAL;
>  	}
>  
> -- 
> 1.8.3.2
> 
> _______________________________________________
> 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 7/7] drm/i915: Use enum plane instaad of numbers

[Daniel Vetter]

On Mon, Mar 31, 2014 at 07:31:04PM +0200, Daniel Vetter wrote:
> On Mon, Mar 31, 2014 at 06:21:30PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Change the update_plane() plane checks to use enum plane, and
> > also fix up the error message to say something that's not total
> > nonsense.
> > 
> > FIXME killing the checks entirely is probably a better idea
> 
> At least killing the totally outdated SAREA comment would be good ;-) But
> yeah I really don't see much point in this, especially since we'll
> happily frob plane C on ilk/snb despite that we're only supporting it on
> ivb+ really.
> 
> Looking through git history with git blame it seems like this went defunct
> somewhere in the large modesetting rewrite. Or even earlier ... in any
> case very confusing history and ripe for the bin.

Forgotten to add: The actually still working "can we update SAREA?" test
is in intel_crtc_update_sarea. That should be mentioned in the commit
message.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: Kill crtc->plane checks from the primary plane update hooks

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

These were apparently meant to protect the SAREA which only has
room for two pipes, but things clearly went a bit wonky when
first the .update_plane() hooks were split up and then pipe C
got introduced.

The checks actually protecting the SAREA live in
intel_crtc_update_sarea() these days, so the checks in the primary
plane update hooks are just historical leftovers which are to be
eliminated.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index add940c..f5d232f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2166,15 +2166,6 @@ static int i9xx_update_primary_plane(struct drm_crtc *crtc,
 	u32 dspcntr;
 	u32 reg;
 
-	switch (plane) {
-	case 0:
-	case 1:
-		break;
-	default:
-		DRM_ERROR("Can't update plane %c in SAREA\n", plane_name(plane));
-		return -EINVAL;
-	}
-
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2267,16 +2258,6 @@ static int ironlake_update_primary_plane(struct drm_crtc *crtc,
 	u32 dspcntr;
 	u32 reg;
 
-	switch (plane) {
-	case 0:
-	case 1:
-	case 2:
-		break;
-	default:
-		DRM_ERROR("Can't update plane %c in SAREA\n", plane_name(plane));
-		return -EINVAL;
-	}
-
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
-- 
1.8.3.2

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

Re: [PATCH] drm/i915: Kill crtc->plane checks from the primary plane update hooks

[Daniel Vetter]

On Mon, Mar 31, 2014 at 09:29:41PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> These were apparently meant to protect the SAREA which only has
> room for two pipes, but things clearly went a bit wonky when
> first the .update_plane() hooks were split up and then pipe C
> got introduced.
> 
> The checks actually protecting the SAREA live in
> intel_crtc_update_sarea() these days, so the checks in the primary
> plane update hooks are just historical leftovers which are to be
> eliminated.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling

[Chris Wilson]

On Mon, Mar 31, 2014 at 06:56:10PM +0200, Daniel Vetter wrote:
> On Mon, Mar 31, 2014 at 6:35 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >> Hmm, after some thought I am in favour of the function as a readibility
> >> improvement. However, I would prefer to have the iir check inlined
> >> into the caller.
> >
> > I had it like that originally, but I moved it into the function when I
> > realized all the callers have the exact same check, and it looked a bit
> > out of place next to the other "sub" irq handlers which do the IIR
> > checks themselves.
> >
> > But I'm not really attached to the idea, so I can change it back if
> > that's the consensus.
> 
> In our shared interrupt handling code we have both cases, where either
> the helper function itself of the caller checks for the bits. Or
> sometimes both even. I'm ok with whatever approach makes sense most.

I'm working on the theory that we get thousands more interrupts for
breadcrumbs and vblanks than we do hpd and so we should focus on
keeping those pathways short.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/7] drm/i915: Split dp post_disable hooks

[Jani Nikula]

On Mon, 31 Mar 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Split the post_disable hooks for DP to g4x and vlv variants. We'll
> need another variant soon, so this should make it look a bit cleaner.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a2a0b01..c33971e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1782,17 +1782,23 @@ static void intel_disable_dp(struct intel_encoder *encoder)
>  		intel_dp_link_down(intel_dp);
>  }
>  
> -static void intel_post_disable_dp(struct intel_encoder *encoder)
> +static void g4x_post_disable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	enum port port = dp_to_dig_port(intel_dp)->port;
> -	struct drm_device *dev = encoder->base.dev;
>  
> -	if (port == PORT_A || IS_VALLEYVIEW(dev)) {
> -		intel_dp_link_down(intel_dp);
> -		if (!IS_VALLEYVIEW(dev))
> -			ironlake_edp_pll_off(intel_dp);
> -	}
> +	if (port != PORT_A)
> +		return;
> +
> +	intel_dp_link_down(intel_dp);
> +	ironlake_edp_pll_off(intel_dp);
> +}
> +
> +static void vlv_post_disable_dp(struct intel_encoder *encoder)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +	intel_dp_link_down(intel_dp);
>  }
>  
>  static void intel_enable_dp(struct intel_encoder *encoder)
> @@ -3836,16 +3842,17 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	intel_encoder->compute_config = intel_dp_compute_config;
>  	intel_encoder->mode_set = intel_dp_mode_set;
>  	intel_encoder->disable = intel_disable_dp;
> -	intel_encoder->post_disable = intel_post_disable_dp;
>  	intel_encoder->get_hw_state = intel_dp_get_hw_state;
>  	intel_encoder->get_config = intel_dp_get_config;
>  	if (IS_VALLEYVIEW(dev)) {
>  		intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable;
>  		intel_encoder->pre_enable = vlv_pre_enable_dp;
>  		intel_encoder->enable = vlv_enable_dp;
> +		intel_encoder->post_disable = vlv_post_disable_dp;
>  	} else {
>  		intel_encoder->pre_enable = g4x_pre_enable_dp;
>  		intel_encoder->enable = g4x_enable_dp;
> +		intel_encoder->post_disable = g4x_post_disable_dp;
>  	}
>  
>  	intel_dig_port->port = port;
> -- 
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Split dp post_disable hooks

[Daniel Vetter]

On Tue, Apr 01, 2014 at 10:03:33AM +0300, Jani Nikula wrote:
> On Mon, 31 Mar 2014, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Split the post_disable hooks for DP to g4x and vlv variants. We'll
> > need another variant soon, so this should make it look a bit cleaner.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

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

[PATCH v2 1/7] drm/i915: Refactor gmch hpd irq handling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pull all the gmch platform hotplug interrupt handling into one
function.

v2: Move the IIR check to the caller
    s/drm_i915_private_t/struct drm_i915_private/

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0858189..361e9b3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1647,6 +1647,29 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 		gmbus_irq_handler(dev);
 }
 
+static void i9xx_hpd_irq_handler(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+
+	if (IS_G4X(dev)) {
+		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
+
+		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x);
+	} else {
+		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
+
+		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
+	}
+
+	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
+	    hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
+		dp_aux_irq_handler(dev);
+
+	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
+	POSTING_READ(PORT_HOTPLUG_STAT);
+}
+
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
@@ -1669,19 +1692,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		valleyview_pipestat_irq_handler(dev, iir);
 
 		/* Consume port.  Then clear IIR or we'll miss events */
-		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
-			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
-
-			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
-
-			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
-				dp_aux_irq_handler(dev);
-
-			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			I915_READ(PORT_HOTPLUG_STAT);
-		}
-
+		if (iir & I915_DISPLAY_PORT_INTERRUPT)
+			i9xx_hpd_irq_handler(dev);
 
 		if (pm_iir)
 			gen6_rps_irq_handler(dev_priv, pm_iir);
@@ -3713,16 +3725,9 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 			break;
 
 		/* Consume port.  Then clear IIR or we'll miss events */
-		if ((I915_HAS_HOTPLUG(dev)) &&
-		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
-			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
-
-			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
-
-			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			POSTING_READ(PORT_HOTPLUG_STAT);
-		}
+		if (I915_HAS_HOTPLUG(dev) &&
+		    iir & I915_DISPLAY_PORT_INTERRUPT)
+			i9xx_hpd_irq_handler(dev);
 
 		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
@@ -3956,22 +3961,8 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 		ret = IRQ_HANDLED;
 
 		/* Consume port.  Then clear IIR or we'll miss events */
-		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
-			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-			u32 hotplug_trigger = hotplug_status & (IS_G4X(dev) ?
-								  HOTPLUG_INT_STATUS_G4X :
-								  HOTPLUG_INT_STATUS_I915);
-
-			intel_hpd_irq_handler(dev, hotplug_trigger,
-					      IS_G4X(dev) ? hpd_status_g4x : hpd_status_i915);
-
-			if (IS_G4X(dev) &&
-			    (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X))
-				dp_aux_irq_handler(dev);
-
-			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			I915_READ(PORT_HOTPLUG_STAT);
-		}
+		if (iir & I915_DISPLAY_PORT_INTERRUPT)
+			i9xx_hpd_irq_handler(dev);
 
 		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
-- 
1.8.3.2

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

Re: [PATCH v2 1/7] drm/i915: Refactor gmch hpd irq handling

[Chris Wilson]

On Tue, Apr 01, 2014 at 10:54:36AM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull all the gmch platform hotplug interrupt handling into one
> function.
> 
> v2: Move the IIR check to the caller
>     s/drm_i915_private_t/struct drm_i915_private/
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

With the addition of the comment before the posting-read,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++++++++-----------------------
>  1 file changed, 30 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 0858189..361e9b3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1647,6 +1647,29 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
>  		gmbus_irq_handler(dev);
>  }
>  
> +static void i9xx_hpd_irq_handler(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
> +
> +	if (IS_G4X(dev)) {
> +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> +
> +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x);
> +	} else {
> +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> +
> +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
> +	}
> +
> +	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
> +	    hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> +		dp_aux_irq_handler(dev);
> +
> +	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);

/* Make sure hotplug status is cleared before we clear IIR,
 * or else we may miss hotplug events.
 */
> +	POSTING_READ(PORT_HOTPLUG_STAT);
> +}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v2 1/7] drm/i915: Refactor gmch hpd irq handling

[Daniel Vetter]

On Tue, Apr 01, 2014 at 09:11:56AM +0100, Chris Wilson wrote:
> On Tue, Apr 01, 2014 at 10:54:36AM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Pull all the gmch platform hotplug interrupt handling into one
> > function.
> > 
> > v2: Move the IIR check to the caller
> >     s/drm_i915_private_t/struct drm_i915_private/
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> With the addition of the comment before the posting-read,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++++++++-----------------------
> >  1 file changed, 30 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 0858189..361e9b3 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1647,6 +1647,29 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
> >  		gmbus_irq_handler(dev);
> >  }
> >  
> > +static void i9xx_hpd_irq_handler(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
> > +
> > +	if (IS_G4X(dev)) {
> > +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> > +
> > +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x);
> > +	} else {
> > +		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> > +
> > +		intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
> > +	}
> > +
> > +	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
> > +	    hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> > +		dp_aux_irq_handler(dev);
> > +
> > +	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> 
> /* Make sure hotplug status is cleared before we clear IIR,
>  * or else we may miss hotplug events.
>  */

Done&merged, thanks for the patch&review.
-Daniel

> > +	POSTING_READ(PORT_HOTPLUG_STAT);
> > +}
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> 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 2/7] drm/i915: Move DP M/N setup from update_pll to mode_set for gmch platforms

[Jesse Barnes]

On Mon, 31 Mar 2014 18:21:25 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There's no point in hiding the DP M/N setup in the update_pll functions.
> Just move it to the mode_set function.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ecc01f5..add940c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5245,9 +5245,6 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  	crtc->config.dpll_hw_state.dpll_md = dpll_md;
>  
> -	if (crtc->config.has_dp_encoder)
> -		intel_dp_set_m_n(crtc);
> -
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> @@ -5325,9 +5322,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>  			<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  		crtc->config.dpll_hw_state.dpll_md = dpll_md;
>  	}
> -
> -	if (crtc->config.has_dp_encoder)
> -		intel_dp_set_m_n(crtc);
>  }
>  
>  static void i8xx_update_pll(struct intel_crtc *crtc,
> @@ -5656,6 +5650,9 @@ skip_dpll:
>  			dspcntr |= DISPPLANE_SEL_PIPE_B;
>  	}
>  
> +	if (intel_crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(intel_crtc);
> +
>  	intel_set_pipe_timings(intel_crtc);
>  
>  	/* pipesrc and dspsize control the size that is scaled from,

Yeah I like it.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff

[Jesse Barnes]

On Mon, 31 Mar 2014 18:21:27 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> DPIO reads from groups/broadcast register offsets for PCS and
> TX return all 1's. If that result gets used for something
> we'll probably end up doing something wrong. So warn when that
> happens.
> 
> FIXME there might be some registers where all 1's is a valid value,
> so ideally we should check the register offset instead...
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sideband.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 0954f13..c1e56f5 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -182,6 +182,10 @@ u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg)
>  
>  	vlv_sideband_rw(dev_priv, DPIO_DEVFN, DPIO_PHY_IOSF_PORT(DPIO_PHY(pipe)),
>  			DPIO_OPCODE_REG_READ, reg, &val);
> +
> +	WARN(val == 0xffffffff, "DPIO read pipe %c reg 0x%x == 0x%x\n",
> +	     pipe_name(pipe), reg, val);
> +
>  	return val;
>  }
>  

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong

[Jesse Barnes]

On Mon, 31 Mar 2014 18:21:28 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Print the enable_mask and status_mask from
> __i915_{enable,disable}_pipestat() when the called has messed them up
> somehow.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 6d26719..407742f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -503,8 +503,10 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  
>  	assert_spin_locked(&dev_priv->irq_lock);
>  
> -	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> -	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
> +	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> +		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
> +		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
> +		      pipe, enable_mask, status_mask))
>  		return;
>  
>  	if ((pipestat & enable_mask) == enable_mask)
> @@ -527,8 +529,10 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  
>  	assert_spin_locked(&dev_priv->irq_lock);
>  
> -	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> -	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
> +	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> +		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
> +		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
> +		      pipe, enable_mask, status_mask))
>  		return;
>  
>  	if ((pipestat & enable_mask) == 0)

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915: Fix framecount offset

[Jesse Barnes]

On Mon, 31 Mar 2014 18:21:29 +0300
ville.syrjala@linux.intel.com wrote:

> From: Rafael Barbalho <rafael.barbalho@intel.com>
> 
> The framecount register was still using the old PIPE macro instead
> of the new PIPE2 macro
> 
> Signed-off-by: Rafael Barbalho <rafael.barbalho@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a47b4c3..b6441da 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3623,9 +3623,9 @@ enum punit_power_well {
>  #define   PIPE_PIXEL_MASK         0x00ffffff
>  #define   PIPE_PIXEL_SHIFT        0
>  /* GM45+ just has to be different */
> -#define _PIPEA_FRMCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70040)
> -#define _PIPEA_FLIPCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70044)
> -#define PIPE_FRMCOUNT_GM45(pipe) _PIPE(pipe, _PIPEA_FRMCOUNT_GM45, _PIPEB_FRMCOUNT_GM45)
> +#define _PIPEA_FRMCOUNT_GM45	0x70040
> +#define _PIPEA_FLIPCOUNT_GM45	0x70044
> +#define PIPE_FRMCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FRMCOUNT_GM45)
>  
>  /* Cursor A & B regs */
>  #define _CURACNTR		(dev_priv->info.display_mmio_offset + 0x70080)

Oh fun.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong

[Damien Lespiau]

On Mon, Mar 31, 2014 at 06:21:28PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Print the enable_mask and status_mask from
> __i915_{enable,disable}_pipestat() when the called has messed them up
> somehow.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 6d26719..407742f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -503,8 +503,10 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  
>  	assert_spin_locked(&dev_priv->irq_lock);
>  
> -	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> -	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
> +	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> +		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
> +		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
> +		      pipe, enable_mask, status_mask))

pipe_name(pipe)?

>  		return;
>  
>  	if ((pipestat & enable_mask) == enable_mask)
> @@ -527,8 +529,10 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  
>  	assert_spin_locked(&dev_priv->irq_lock);
>  
> -	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> -	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
> +	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> +		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
> +		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
> +		      pipe, enable_mask, status_mask))
>  		return;

Ditto.

>  
>  	if ((pipestat & enable_mask) == 0)
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff

[Daniel Vetter]

On Mon, Mar 31, 2014 at 06:21:27PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> DPIO reads from groups/broadcast register offsets for PCS and
> TX return all 1's. If that result gets used for something
> we'll probably end up doing something wrong. So warn when that
> happens.
> 
> FIXME there might be some registers where all 1's is a valid value,
> so ideally we should check the register offset instead...

FIXMEs like this look better in the code, I've pasted it there, too.
-Daniel

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sideband.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 0954f13..c1e56f5 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -182,6 +182,10 @@ u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg)
>  
>  	vlv_sideband_rw(dev_priv, DPIO_DEVFN, DPIO_PHY_IOSF_PORT(DPIO_PHY(pipe)),
>  			DPIO_OPCODE_REG_READ, reg, &val);
> +
> +	WARN(val == 0xffffffff, "DPIO read pipe %c reg 0x%x == 0x%x\n",
> +	     pipe_name(pipe), reg, val);
> +
>  	return val;
>  }
>  
> -- 
> 1.8.3.2
> 
> _______________________________________________
> 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 6/7] drm/i915: Fix framecount offset

[Daniel Vetter]

On Wed, Apr 02, 2014 at 10:29:08AM -0700, Jesse Barnes wrote:
> On Mon, 31 Mar 2014 18:21:29 +0300
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Rafael Barbalho <rafael.barbalho@intel.com>
> > 
> > The framecount register was still using the old PIPE macro instead
> > of the new PIPE2 macro
> > 
> > Signed-off-by: Rafael Barbalho <rafael.barbalho@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index a47b4c3..b6441da 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3623,9 +3623,9 @@ enum punit_power_well {
> >  #define   PIPE_PIXEL_MASK         0x00ffffff
> >  #define   PIPE_PIXEL_SHIFT        0
> >  /* GM45+ just has to be different */
> > -#define _PIPEA_FRMCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70040)
> > -#define _PIPEA_FLIPCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70044)
> > -#define PIPE_FRMCOUNT_GM45(pipe) _PIPE(pipe, _PIPEA_FRMCOUNT_GM45, _PIPEB_FRMCOUNT_GM45)
> > +#define _PIPEA_FRMCOUNT_GM45	0x70040
> > +#define _PIPEA_FLIPCOUNT_GM45	0x70044
> > +#define PIPE_FRMCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FRMCOUNT_GM45)
> >  
> >  /* Cursor A & B regs */
> >  #define _CURACNTR		(dev_priv->info.display_mmio_offset + 0x70080)
> 
> Oh fun.
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

I've merged all the patches Jesse reviewed from this series, expect the
one Damien has a bikeshed pending.

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

[PATCH v2 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Print the enable_mask and status_mask from
__i915_{enable,disable}_pipestat() when the caller has messed them up
somehow.

v2: Use pipe_name() (Damien)
    Fix a typo in the commit message

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 361e9b3..2343323 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -503,8 +503,10 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
 
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
-	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
+	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
+		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
+		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
+		      pipe_name(pipe), enable_mask, status_mask))
 		return;
 
 	if ((pipestat & enable_mask) == enable_mask)
@@ -527,8 +529,10 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
 
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
-	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
+	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
+		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
+		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
+		      pipe_name(pipe), enable_mask, status_mask))
 		return;
 
 	if ((pipestat & enable_mask) == 0)
-- 
1.8.3.2

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

Re: [PATCH v2 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong

[Damien Lespiau]

On Thu, Apr 03, 2014 at 01:28:33PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Print the enable_mask and status_mask from
> __i915_{enable,disable}_pipestat() when the caller has messed them up
> somehow.
> 
> v2: Use pipe_name() (Damien)
>     Fix a typo in the commit message
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 361e9b3..2343323 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -503,8 +503,10 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  
>  	assert_spin_locked(&dev_priv->irq_lock);
>  
> -	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> -	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
> +	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> +		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
> +		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
> +		      pipe_name(pipe), enable_mask, status_mask))
>  		return;
>  
>  	if ((pipestat & enable_mask) == enable_mask)
> @@ -527,8 +529,10 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  
>  	assert_spin_locked(&dev_priv->irq_lock);
>  
> -	if (WARN_ON_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> -	                 status_mask & ~PIPESTAT_INT_STATUS_MASK))
> +	if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
> +		      status_mask & ~PIPESTAT_INT_STATUS_MASK,
> +		      "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
> +		      pipe_name(pipe), enable_mask, status_mask))
>  		return;
>  
>  	if ((pipestat & enable_mask) == 0)
> -- 
> 1.8.3.2
> 

Re: [PATCH 6/7] drm/i915: Fix framecount offset

[Damien Lespiau]

On Thu, Apr 03, 2014 at 11:30:16AM +0200, Daniel Vetter wrote:
> I've merged all the patches Jesse reviewed from this series, expect the
> one Damien has a bikeshed pending.

It wasn't bikeshedding, we were printing "%c",0 instead of "%c",'A' :)

-- 
Damien

Re: [PATCH v2 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong

[Daniel Vetter]

On Thu, Apr 03, 2014 at 11:54:32AM +0100, Damien Lespiau wrote:
> On Thu, Apr 03, 2014 at 01:28:33PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Print the enable_mask and status_mask from
> > __i915_{enable,disable}_pipestat() when the caller has messed them up
> > somehow.
> > 
> > v2: Use pipe_name() (Damien)
> >     Fix a typo in the commit message
> > 
> > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

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

Re: [PATCH 6/7] drm/i915: Fix framecount offset

[Daniel Vetter]

On Thu, Apr 03, 2014 at 12:04:51PM +0100, Damien Lespiau wrote:
> On Thu, Apr 03, 2014 at 11:30:16AM +0200, Daniel Vetter wrote:
> > I've merged all the patches Jesse reviewed from this series, expect the
> > one Damien has a bikeshed pending.
> 
> It wasn't bikeshedding, we were printing "%c",0 instead of "%c",'A' :)

Oh, didn't spot that finesse, thought it was a %i, pipe. That's indeed not
a bikeshed, sorry for tacking the wrong label onto your review ;-)

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