* [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling
2014-03-31 15:21 [PATCH 0/7] drm/i915: Pile of display related patches ville.syrjala
@ 2014-03-31 15:21 ` ville.syrjala
2014-03-31 15:49 ` Chris Wilson
2014-03-31 15:21 ` [PATCH 2/7] drm/i915: Move DP M/N setup from update_pll to mode_set for gmch platforms ville.syrjala
` (5 subsequent siblings)
6 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 15:21 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling
2014-03-31 15:21 ` [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling ville.syrjala
@ 2014-03-31 15:49 ` Chris Wilson
2014-03-31 16:35 ` Ville Syrjälä
2014-04-01 7:54 ` [PATCH v2 " ville.syrjala
0 siblings, 2 replies; 33+ messages in thread
From: Chris Wilson @ 2014-03-31 15:49 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling
2014-03-31 15:49 ` Chris Wilson
@ 2014-03-31 16:35 ` Ville Syrjälä
2014-03-31 16:56 ` Daniel Vetter
2014-04-01 7:54 ` [PATCH v2 " ville.syrjala
1 sibling, 1 reply; 33+ messages in thread
From: Ville Syrjälä @ 2014-03-31 16:35 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling
2014-03-31 16:35 ` Ville Syrjälä
@ 2014-03-31 16:56 ` Daniel Vetter
2014-03-31 20:57 ` Chris Wilson
0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2014-03-31 16:56 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling
2014-03-31 16:56 ` Daniel Vetter
@ 2014-03-31 20:57 ` Chris Wilson
0 siblings, 0 replies; 33+ messages in thread
From: Chris Wilson @ 2014-03-31 20:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 1/7] drm/i915: Refactor gmch hpd irq handling
2014-03-31 15:49 ` Chris Wilson
2014-03-31 16:35 ` Ville Syrjälä
@ 2014-04-01 7:54 ` ville.syrjala
2014-04-01 8:11 ` Chris Wilson
1 sibling, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-04-01 7:54 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/7] drm/i915: Refactor gmch hpd irq handling
2014-04-01 7:54 ` [PATCH v2 " ville.syrjala
@ 2014-04-01 8:11 ` Chris Wilson
2014-04-01 8:15 ` Daniel Vetter
0 siblings, 1 reply; 33+ messages in thread
From: Chris Wilson @ 2014-04-01 8:11 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/7] drm/i915: Refactor gmch hpd irq handling
2014-04-01 8:11 ` Chris Wilson
@ 2014-04-01 8:15 ` Daniel Vetter
0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2014-04-01 8:15 UTC (permalink / raw)
To: Chris Wilson, ville.syrjala, intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/7] drm/i915: Move DP M/N setup from update_pll to mode_set for gmch platforms
2014-03-31 15:21 [PATCH 0/7] drm/i915: Pile of display related patches ville.syrjala
2014-03-31 15:21 ` [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling ville.syrjala
@ 2014-03-31 15:21 ` ville.syrjala
2014-04-02 17:27 ` Jesse Barnes
2014-03-31 15:21 ` [PATCH 3/7] drm/i915: Split dp post_disable hooks ville.syrjala
` (4 subsequent siblings)
6 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 15:21 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 2/7] drm/i915: Move DP M/N setup from update_pll to mode_set for gmch platforms
2014-03-31 15:21 ` [PATCH 2/7] drm/i915: Move DP M/N setup from update_pll to mode_set for gmch platforms ville.syrjala
@ 2014-04-02 17:27 ` Jesse Barnes
0 siblings, 0 replies; 33+ messages in thread
From: Jesse Barnes @ 2014-04-02 17:27 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/7] drm/i915: Split dp post_disable hooks
2014-03-31 15:21 [PATCH 0/7] drm/i915: Pile of display related patches ville.syrjala
2014-03-31 15:21 ` [PATCH 1/7] drm/i915: Refactor gmch hpd irq handling ville.syrjala
2014-03-31 15:21 ` [PATCH 2/7] drm/i915: Move DP M/N setup from update_pll to mode_set for gmch platforms ville.syrjala
@ 2014-03-31 15:21 ` ville.syrjala
2014-04-01 7:03 ` Jani Nikula
2014-03-31 15:21 ` [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff ville.syrjala
` (3 subsequent siblings)
6 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 15:21 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 3/7] drm/i915: Split dp post_disable hooks
2014-03-31 15:21 ` [PATCH 3/7] drm/i915: Split dp post_disable hooks ville.syrjala
@ 2014-04-01 7:03 ` Jani Nikula
2014-04-01 7:37 ` Daniel Vetter
0 siblings, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2014-04-01 7:03 UTC (permalink / raw)
To: ville.syrjala, intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 3/7] drm/i915: Split dp post_disable hooks
2014-04-01 7:03 ` Jani Nikula
@ 2014-04-01 7:37 ` Daniel Vetter
0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2014-04-01 7:37 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff
2014-03-31 15:21 [PATCH 0/7] drm/i915: Pile of display related patches ville.syrjala
` (2 preceding siblings ...)
2014-03-31 15:21 ` [PATCH 3/7] drm/i915: Split dp post_disable hooks ville.syrjala
@ 2014-03-31 15:21 ` ville.syrjala
2014-04-02 17:28 ` Jesse Barnes
2014-04-03 9:29 ` Daniel Vetter
2014-03-31 15:21 ` [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong ville.syrjala
` (2 subsequent siblings)
6 siblings, 2 replies; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 15:21 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff
2014-03-31 15:21 ` [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff ville.syrjala
@ 2014-04-02 17:28 ` Jesse Barnes
2014-04-03 9:29 ` Daniel Vetter
1 sibling, 0 replies; 33+ messages in thread
From: Jesse Barnes @ 2014-04-02 17:28 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff
2014-03-31 15:21 ` [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff ville.syrjala
2014-04-02 17:28 ` Jesse Barnes
@ 2014-04-03 9:29 ` Daniel Vetter
1 sibling, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2014-04-03 9:29 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong
2014-03-31 15:21 [PATCH 0/7] drm/i915: Pile of display related patches ville.syrjala
` (3 preceding siblings ...)
2014-03-31 15:21 ` [PATCH 4/7] drm/i915: Warn when DPIO read returns 0xffffffff ville.syrjala
@ 2014-03-31 15:21 ` ville.syrjala
2014-04-02 17:28 ` Jesse Barnes
2014-04-02 21:35 ` Damien Lespiau
2014-03-31 15:21 ` [PATCH 6/7] drm/i915: Fix framecount offset ville.syrjala
2014-03-31 15:21 ` [PATCH 7/7] drm/i915: Use enum plane instaad of numbers ville.syrjala
6 siblings, 2 replies; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 15:21 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong
2014-03-31 15:21 ` [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong ville.syrjala
@ 2014-04-02 17:28 ` Jesse Barnes
2014-04-02 21:35 ` Damien Lespiau
1 sibling, 0 replies; 33+ messages in thread
From: Jesse Barnes @ 2014-04-02 17:28 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong
2014-03-31 15:21 ` [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong ville.syrjala
2014-04-02 17:28 ` Jesse Barnes
@ 2014-04-02 21:35 ` Damien Lespiau
2014-04-03 10:28 ` [PATCH v2 " ville.syrjala
1 sibling, 1 reply; 33+ messages in thread
From: Damien Lespiau @ 2014-04-02 21:35 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH v2 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong
2014-04-02 21:35 ` Damien Lespiau
@ 2014-04-03 10:28 ` ville.syrjala
2014-04-03 10:54 ` Damien Lespiau
0 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-04-03 10:28 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v2 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong
2014-04-03 10:28 ` [PATCH v2 " ville.syrjala
@ 2014-04-03 10:54 ` Damien Lespiau
2014-04-03 15:24 ` Daniel Vetter
0 siblings, 1 reply; 33+ messages in thread
From: Damien Lespiau @ 2014-04-03 10:54 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong
2014-04-03 10:54 ` Damien Lespiau
@ 2014-04-03 15:24 ` Daniel Vetter
0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2014-04-03 15:24 UTC (permalink / raw)
To: Damien Lespiau; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 6/7] drm/i915: Fix framecount offset
2014-03-31 15:21 [PATCH 0/7] drm/i915: Pile of display related patches ville.syrjala
` (4 preceding siblings ...)
2014-03-31 15:21 ` [PATCH 5/7] drm/i915: Provide a bit more info when pipestat bits are wrong ville.syrjala
@ 2014-03-31 15:21 ` ville.syrjala
2014-04-02 17:29 ` Jesse Barnes
2014-03-31 15:21 ` [PATCH 7/7] drm/i915: Use enum plane instaad of numbers ville.syrjala
6 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 15:21 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 6/7] drm/i915: Fix framecount offset
2014-03-31 15:21 ` [PATCH 6/7] drm/i915: Fix framecount offset ville.syrjala
@ 2014-04-02 17:29 ` Jesse Barnes
2014-04-03 9:30 ` Daniel Vetter
0 siblings, 1 reply; 33+ messages in thread
From: Jesse Barnes @ 2014-04-02 17:29 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 6/7] drm/i915: Fix framecount offset
2014-04-02 17:29 ` Jesse Barnes
@ 2014-04-03 9:30 ` Daniel Vetter
2014-04-03 11:04 ` Damien Lespiau
0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2014-04-03 9:30 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 6/7] drm/i915: Fix framecount offset
2014-04-03 9:30 ` Daniel Vetter
@ 2014-04-03 11:04 ` Damien Lespiau
2014-04-03 15:26 ` Daniel Vetter
0 siblings, 1 reply; 33+ messages in thread
From: Damien Lespiau @ 2014-04-03 11:04 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/7] drm/i915: Fix framecount offset
2014-04-03 11:04 ` Damien Lespiau
@ 2014-04-03 15:26 ` Daniel Vetter
0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2014-04-03 15:26 UTC (permalink / raw)
To: Damien Lespiau; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 7/7] drm/i915: Use enum plane instaad of numbers
2014-03-31 15:21 [PATCH 0/7] drm/i915: Pile of display related patches ville.syrjala
` (5 preceding siblings ...)
2014-03-31 15:21 ` [PATCH 6/7] drm/i915: Fix framecount offset ville.syrjala
@ 2014-03-31 15:21 ` ville.syrjala
2014-03-31 17:31 ` Daniel Vetter
6 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 15:21 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 7/7] drm/i915: Use enum plane instaad of numbers
2014-03-31 15:21 ` [PATCH 7/7] drm/i915: Use enum plane instaad of numbers ville.syrjala
@ 2014-03-31 17:31 ` Daniel Vetter
2014-03-31 17:33 ` Daniel Vetter
0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2014-03-31 17:31 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 7/7] drm/i915: Use enum plane instaad of numbers
2014-03-31 17:31 ` Daniel Vetter
@ 2014-03-31 17:33 ` Daniel Vetter
2014-03-31 18:29 ` [PATCH] drm/i915: Kill crtc->plane checks from the primary plane update hooks ville.syrjala
0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2014-03-31 17:33 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] drm/i915: Kill crtc->plane checks from the primary plane update hooks
2014-03-31 17:33 ` Daniel Vetter
@ 2014-03-31 18:29 ` ville.syrjala
2014-03-31 20:37 ` Daniel Vetter
0 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2014-03-31 18:29 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH] drm/i915: Kill crtc->plane checks from the primary plane update hooks
2014-03-31 18:29 ` [PATCH] drm/i915: Kill crtc->plane checks from the primary plane update hooks ville.syrjala
@ 2014-03-31 20:37 ` Daniel Vetter
0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2014-03-31 20:37 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 33+ messages in thread