| Message ID | 1363276516-10986-1-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 14, 2013 at 08:55:16AM -0700, Ben Widawsky wrote: > Interrupts, clock gating, and GMBUS are all within the, "this will hang > the CPU" range when we have PCH_NOP. > > There is a bit of a hack in init clock gating. We want to do most of the > clock gating, but the part we skip will hang the system. It could > probably be abstracted a bit better, but I don't feel it's too > unsightly. > > v2: Use inverse HAS_PCH_NOP check (Jani) > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ > drivers/gpu/drm/i915/intel_bios.c | 3 +++ > drivers/gpu/drm/i915/intel_i2c.c | 4 +++- > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 4 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index b860f0b..15b4668 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) > } > } > > - /* check event from PCH */ > - if (de_iir & DE_PCH_EVENT_IVB) { > - u32 pch_iir = I915_READ(SDEIIR); > + if (!HAS_PCH_NOP(dev)) { > + /* check event from PCH */ > + if (de_iir & DE_PCH_EVENT_IVB) { && the two conditions to not require another indent level (it starts to look a bit ugly). And the PCH_NOP looks here better than a num_pipes check, so I'll merge that patch (i.e. disregard my other suggestion to replace PCH_NOP with num_pipes checks from a previous patch). Also note that Paulo's SDEIIR race fix added more reads/writes to that register in -fixes, so please rebase these patches on top of -nightly so that I can correctly apply them (I'll do the backmerge before slurping in your series). > + u32 pch_iir = I915_READ(SDEIIR); > > - cpt_irq_handler(dev, pch_iir); > + cpt_irq_handler(dev, pch_iir); > > - /* clear PCH hotplug event before clear CPU irq */ > - I915_WRITE(SDEIIR, pch_iir); > + /* clear PCH hotplug event before CPU irq */ > + I915_WRITE(SDEIIR, pch_iir); > + } > } > > I915_WRITE(DEIIR, de_iir); > @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) > I915_WRITE(GTIER, 0x0); > POSTING_READ(GTIER); > > + if (HAS_PCH_NOP(dev)) > + return; > + > /* south display irq */ > I915_WRITE(SDEIMR, 0xffffffff); > I915_WRITE(SDEIER, 0x0); > @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) > SDE_GMBUS_CPT | > SDE_AUX_MASK_CPT; > > + if (HAS_PCH_NOP(dev)) > + return; > + > I915_WRITE(SDEIIR, I915_READ(SDEIIR)); > I915_WRITE(SDEIMR, ~mask); > I915_WRITE(SDEIER, mask); > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 55ffba1..194df27 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) > struct bdb_header *bdb = NULL; > u8 __iomem *bios = NULL; > > + if (HAS_PCH_NOP(dev)) > + return -ENODEV; > + > init_vbt_defaults(dev_priv); > > /* XXX Should this validation be moved to intel_opregion.c? */ > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c > index acf8aec..fc19e49 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > int ret, i; > > - if (HAS_PCH_SPLIT(dev)) > + if (HAS_PCH_NOP(dev)) > + return 0; > + else if (HAS_PCH_SPLIT(dev)) > dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; > else if (IS_VALLEYVIEW(dev)) > dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5479363..52203fd 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) > snpcr |= GEN6_MBC_SNPCR_MED; > I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); > > - cpt_init_clock_gating(dev); > + if (HAS_PCH_NOP(dev)) > + cpt_init_clock_gating(dev); Inverted check? Cheers, Daniel > > gen6_check_mch_setup(dev); > } > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sun, Mar 17, 2013 at 10:21:30PM +0100, Daniel Vetter wrote: > On Thu, Mar 14, 2013 at 08:55:16AM -0700, Ben Widawsky wrote: > > Interrupts, clock gating, and GMBUS are all within the, "this will hang > > the CPU" range when we have PCH_NOP. > > > > There is a bit of a hack in init clock gating. We want to do most of the > > clock gating, but the part we skip will hang the system. It could > > probably be abstracted a bit better, but I don't feel it's too > > unsightly. > > > > v2: Use inverse HAS_PCH_NOP check (Jani) > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ > > drivers/gpu/drm/i915/intel_bios.c | 3 +++ > > drivers/gpu/drm/i915/intel_i2c.c | 4 +++- > > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > > 4 files changed, 22 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index b860f0b..15b4668 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) > > } > > } > > > > - /* check event from PCH */ > > - if (de_iir & DE_PCH_EVENT_IVB) { > > - u32 pch_iir = I915_READ(SDEIIR); > > + if (!HAS_PCH_NOP(dev)) { > > + /* check event from PCH */ > > + if (de_iir & DE_PCH_EVENT_IVB) { > > && the two conditions to not require another indent level (it starts to > look a bit ugly). And the PCH_NOP looks here better than a num_pipes > check, so I'll merge that patch (i.e. disregard my other suggestion to > replace PCH_NOP with num_pipes checks from a previous patch). > > Also note that Paulo's SDEIIR race fix added more reads/writes to that > register in -fixes, so please rebase these patches on top of -nightly so > that I can correctly apply them (I'll do the backmerge before slurping in > your series). Okay to both. > > > + u32 pch_iir = I915_READ(SDEIIR); > > > > - cpt_irq_handler(dev, pch_iir); > > + cpt_irq_handler(dev, pch_iir); > > > > - /* clear PCH hotplug event before clear CPU irq */ > > - I915_WRITE(SDEIIR, pch_iir); > > + /* clear PCH hotplug event before CPU irq */ > > + I915_WRITE(SDEIIR, pch_iir); > > + } > > } > > > > I915_WRITE(DEIIR, de_iir); > > @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) > > I915_WRITE(GTIER, 0x0); > > POSTING_READ(GTIER); > > > > + if (HAS_PCH_NOP(dev)) > > + return; > > + > > /* south display irq */ > > I915_WRITE(SDEIMR, 0xffffffff); > > I915_WRITE(SDEIER, 0x0); > > @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) > > SDE_GMBUS_CPT | > > SDE_AUX_MASK_CPT; > > > > + if (HAS_PCH_NOP(dev)) > > + return; > > + > > I915_WRITE(SDEIIR, I915_READ(SDEIIR)); > > I915_WRITE(SDEIMR, ~mask); > > I915_WRITE(SDEIER, mask); > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > > index 55ffba1..194df27 100644 > > --- a/drivers/gpu/drm/i915/intel_bios.c > > +++ b/drivers/gpu/drm/i915/intel_bios.c > > @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) > > struct bdb_header *bdb = NULL; > > u8 __iomem *bios = NULL; > > > > + if (HAS_PCH_NOP(dev)) > > + return -ENODEV; > > + > > init_vbt_defaults(dev_priv); > > > > /* XXX Should this validation be moved to intel_opregion.c? */ > > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c > > index acf8aec..fc19e49 100644 > > --- a/drivers/gpu/drm/i915/intel_i2c.c > > +++ b/drivers/gpu/drm/i915/intel_i2c.c > > @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) > > struct drm_i915_private *dev_priv = dev->dev_private; > > int ret, i; > > > > - if (HAS_PCH_SPLIT(dev)) > > + if (HAS_PCH_NOP(dev)) > > + return 0; > > + else if (HAS_PCH_SPLIT(dev)) > > dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; > > else if (IS_VALLEYVIEW(dev)) > > dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 5479363..52203fd 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) > > snpcr |= GEN6_MBC_SNPCR_MED; > > I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); > > > > - cpt_init_clock_gating(dev); > > + if (HAS_PCH_NOP(dev)) > > + cpt_init_clock_gating(dev); > > Inverted check? Boy, how did I screw this up twice... Good thing you checked. > > Cheers, Daniel > > > > gen6_check_mch_setup(dev); > > } > > -- > > 1.8.1.5 > > > > _______________________________________________ > > 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
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b860f0b..15b4668 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) } } - /* check event from PCH */ - if (de_iir & DE_PCH_EVENT_IVB) { - u32 pch_iir = I915_READ(SDEIIR); + if (!HAS_PCH_NOP(dev)) { + /* check event from PCH */ + if (de_iir & DE_PCH_EVENT_IVB) { + u32 pch_iir = I915_READ(SDEIIR); - cpt_irq_handler(dev, pch_iir); + cpt_irq_handler(dev, pch_iir); - /* clear PCH hotplug event before clear CPU irq */ - I915_WRITE(SDEIIR, pch_iir); + /* clear PCH hotplug event before CPU irq */ + I915_WRITE(SDEIIR, pch_iir); + } } I915_WRITE(DEIIR, de_iir); @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) I915_WRITE(GTIER, 0x0); POSTING_READ(GTIER); + if (HAS_PCH_NOP(dev)) + return; + /* south display irq */ I915_WRITE(SDEIMR, 0xffffffff); I915_WRITE(SDEIER, 0x0); @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) SDE_GMBUS_CPT | SDE_AUX_MASK_CPT; + if (HAS_PCH_NOP(dev)) + return; + I915_WRITE(SDEIIR, I915_READ(SDEIIR)); I915_WRITE(SDEIMR, ~mask); I915_WRITE(SDEIER, mask); diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 55ffba1..194df27 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) struct bdb_header *bdb = NULL; u8 __iomem *bios = NULL; + if (HAS_PCH_NOP(dev)) + return -ENODEV; + init_vbt_defaults(dev_priv); /* XXX Should this validation be moved to intel_opregion.c? */ diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index acf8aec..fc19e49 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; int ret, i; - if (HAS_PCH_SPLIT(dev)) + if (HAS_PCH_NOP(dev)) + return 0; + else if (HAS_PCH_SPLIT(dev)) dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; else if (IS_VALLEYVIEW(dev)) dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5479363..52203fd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) snpcr |= GEN6_MBC_SNPCR_MED; I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); - cpt_init_clock_gating(dev); + if (HAS_PCH_NOP(dev)) + cpt_init_clock_gating(dev); gen6_check_mch_setup(dev); }
Interrupts, clock gating, and GMBUS are all within the, "this will hang the CPU" range when we have PCH_NOP. There is a bit of a hack in init clock gating. We want to do most of the clock gating, but the part we skip will hang the system. It could probably be abstracted a bit better, but I don't feel it's too unsightly. v2: Use inverse HAS_PCH_NOP check (Jani) Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ drivers/gpu/drm/i915/intel_bios.c | 3 +++ drivers/gpu/drm/i915/intel_i2c.c | 4 +++- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 4 files changed, 22 insertions(+), 8 deletions(-)