Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]