On Wed, Feb 15, 2017 at 01:23:35PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 15, 2017 at 09:06:53AM +0000, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 6a9c16508ab5..bad4f14858e3 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -224,7 +224,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > }
> > }
> > }
> > - if (dev_priv->display.hpd_irq_setup)
> > + if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup)
>
> I don't think we're setting display_irqs_enabled on any other platform
> than VLV/CHV. So either we have to push this check down into
> hpd_irq_setup() or just set display_irqs_enable=true somewhere for all
> the other platforms.
Yup, only vlv/chv set display_irqs_enable. Would it not be safe to
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 99e2d659202c..7e205747f18d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4314,6 +4314,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
if (!IS_GEN2(dev_priv))
dev->vblank_disable_immediate = true;
+ dev_priv->display_irqs_enabled = true;
+
dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;
which may then cause us to enable the display irq mask before the
powerwell is powered, but as soon as we go through a display powerwell
cycle will be back in sync? That presumes we complete such a cycle
before hotplug init touches the register - and that we can touch the irq
registers outside of the powerwell.
Or we can do something a bit more complete:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c1b400f1ede4..681196987450 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1381,6 +1381,12 @@ struct i915_power_well;
struct i915_power_well_ops {
/*
+ * Initialize drm_i915_private or struct i915_power_well state,
+ * called on driver load.
+ */
+ void (*init)(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well);
+ /*
* Synchronize the well's hw state to match the current sw state, for
* example enable/disable it based on the current refcount. Called
* during driver init and resume time, possibly after first calling
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 879567987201..4cb2b099f6c1 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1126,7 +1126,7 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
}
-static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
+static void vlv_display_power_well_install(struct drm_i915_private *dev_priv)
{
struct intel_encoder *encoder;
enum pipe pipe;
@@ -1175,7 +1175,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
intel_pps_unlock_regs_wa(dev_priv);
}
-static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
+static void vlv_display_power_well_uninstall(struct drm_i915_private *dev_priv)
{
spin_lock_irq(&dev_priv->irq_lock);
valleyview_disable_display_irqs(dev_priv);
@@ -1191,6 +1191,12 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
intel_hpd_poll_init(dev_priv);
}
+static void vlv_display_power_well_init(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ dev_priv->display_irqs_enabled = false;
+}
+
static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
@@ -1198,7 +1204,7 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
vlv_set_power_well(dev_priv, power_well, true);
- vlv_display_power_well_init(dev_priv);
+ vlv_display_power_well_install(dev_priv);
}
static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
@@ -1206,7 +1212,7 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
{
WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DISP2D);
- vlv_display_power_well_deinit(dev_priv);
+ vlv_display_power_well_uninstall(dev_priv);
vlv_set_power_well(dev_priv, power_well, false);
}
@@ -1661,6 +1667,12 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
mutex_unlock(&dev_priv->rps.hw_lock);
}
+static void chv_pipe_power_well_init(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ dev_priv->display_irqs_enabled = false;
+}
+
static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
@@ -1676,7 +1688,7 @@ static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
chv_set_pipe_power_well(dev_priv, power_well, true);
- vlv_display_power_well_init(dev_priv);
+ vlv_display_power_well_install(dev_priv);
}
static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
@@ -1684,7 +1696,7 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
{
WARN_ON_ONCE(power_well->id != PIPE_A);
- vlv_display_power_well_deinit(dev_priv);
+ vlv_display_power_well_uninstall(dev_priv);
chv_set_pipe_power_well(dev_priv, power_well, false);
}
@@ -1921,6 +1933,7 @@ static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
};
static const struct i915_power_well_ops chv_pipe_power_well_ops = {
+ .init = chv_pipe_power_well_init,
.sync_hw = chv_pipe_power_well_sync_hw,
.enable = chv_pipe_power_well_enable,
.disable = chv_pipe_power_well_disable,
@@ -2000,6 +2013,7 @@ static struct i915_power_well bdw_power_wells[] = {
};
static const struct i915_power_well_ops vlv_display_power_well_ops = {
+ .init = vlv_display_power_well_init,
.sync_hw = vlv_power_well_sync_hw,
.enable = vlv_display_power_well_enable,
.disable = vlv_display_power_well_disable,
@@ -2367,6 +2381,19 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
return mask;
}
+static void __intel_power_domains_init(struct drm_i915_private *dev_priv)
+{
+ struct i915_power_domains *power_domains = &dev_priv->power_domains;
+ struct i915_power_well *power_well;
+ int i;
+
+ dev_priv->display_irqs_enabled = true;
+ for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
+ if (power_well->ops->init)
+ power_well->ops->init(dev_priv, power_well);
+ }
+}
+
#define set_power_wells(power_domains, __power_wells) ({ \
(power_domains)->power_wells = (__power_wells); \
(power_domains)->power_well_count = ARRAY_SIZE(__power_wells); \
@@ -2414,6 +2441,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
set_power_wells(power_domains, i9xx_always_on_power_well);
}
+ __intel_power_domains_init(dev_priv);
+
return 0;
}
--
Chris Wilson, Intel Open Source Technology Centre