[RFC 0/3] Display uncore

[RFC 0/3] Display uncore

[Daniele Ceraolo Spurio]

I've been trying to identify MMIO ranges to clearly define what belongs
to display_uncore to do a check on access, but there are lots of
exceptions and differences across gens (with a few more coming with TGL),
so I don't think that's a viable way. The alternative option implemented
here is to differentiate the register by type, which should ensure we
never mix them up, but at the cost of a more complex transition.

Thoughts? I'm very open to (and I actually hope for) better ideas.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>

Daniele Ceraolo Spurio (3):
  drm/i915: split out uncore_mmio_debug
  drm/i915: introduce display_uncore
  drm/i915: convert a couple of registers to _DE_MMIO

 .../gpu/drm/i915/display/intel_display_reg.h  |  66 ++++++++++++
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  32 +++---
 drivers/gpu/drm/i915/i915_debugfs.c           |   2 +-
 drivers/gpu/drm/i915/i915_drv.c               |  20 +++-
 drivers/gpu/drm/i915/i915_drv.h               |  32 ++++++
 drivers/gpu/drm/i915/i915_reg.h               |  44 --------
 drivers/gpu/drm/i915/intel_uncore.c           | 100 +++++++++++++-----
 drivers/gpu/drm/i915/intel_uncore.h           |  18 ++--
 8 files changed, 215 insertions(+), 99 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_display_reg.h

-- 
2.22.0

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

[RFC 1/3] drm/i915: split out uncore_mmio_debug

[Daniele Ceraolo Spurio]

Multiple uncore structures will share the debug infrastructure, so
move it to a common place and add extra locking around it.
Also, since we now have a separate object, it is cleaner to have
dedicated functions working on the object to stop and restart the
mmio debug. Apart from the cosmetic changes, this patch introduces
2 functional updates:

- All calls to check_for_unclaimed_mmio will now return false when
  the debug is suspended, not just the ones that are active only when
  i915_modparams.mmio_debug is set. If we don't trust the result of the
  check while a user is doing mmio access then we shouldn't attempt the
  check anywhere.

- i915_modparams.mmio_debug is not save/restored anymore around user
  access. The value is now never touched by the kernel while debug is
  disabled so no need for save/restore.

v2: squash mmio_debug patches, restrict mmio_debug lock usage (Chris)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
 drivers/gpu/drm/i915/i915_drv.c     |  3 +-
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_uncore.c | 91 ++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_uncore.h | 18 +++---
 5 files changed, 79 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b15266c54fd..2310512111ab 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1129,7 +1129,7 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
 	unsigned int tmp;
 
 	seq_printf(m, "user.bypass_count = %u\n",
-		   uncore->user_forcewake.count);
+		   uncore->user_forcewake_count);
 
 	for_each_fw_domain(fw_domain, uncore, tmp)
 		seq_printf(m, "%s.wake_count = %u\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3480db36b63f..fbbff4a133ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -744,6 +744,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 
 	intel_device_info_subplatform_init(dev_priv);
 
+	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
 	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
 
 	spin_lock_init(&dev_priv->irq_lock);
@@ -2044,7 +2045,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 
 out:
 	enable_rpm_wakeref_asserts(rpm);
-	if (!dev_priv->uncore.user_forcewake.count)
+	if (!dev_priv->uncore.user_forcewake_count)
 		intel_runtime_pm_driver_release(rpm);
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9476f24f5c1..13c27a75dca8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1346,6 +1346,7 @@ struct drm_i915_private {
 	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
 
 	struct intel_uncore uncore;
+	struct intel_uncore_mmio_debug mmio_debug;
 
 	struct i915_virtual_gpu vgpu;
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 39e8710afdd9..9e583f13a9e4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -34,6 +34,32 @@
 
 #define __raw_posting_read(...) ((void)__raw_uncore_read32(__VA_ARGS__))
 
+void
+intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug)
+{
+	spin_lock_init(&mmio_debug->lock);
+	mmio_debug->unclaimed_mmio_check = 1;
+}
+
+static void mmio_debug_suspend(struct intel_uncore_mmio_debug *mmio_debug)
+{
+	lockdep_assert_held(&mmio_debug->lock);
+
+	/* Save and disable mmio debugging for the user bypass */
+	if (!mmio_debug->suspend_count++) {
+		mmio_debug->saved_mmio_check = mmio_debug->unclaimed_mmio_check;
+		mmio_debug->unclaimed_mmio_check = 0;
+	}
+}
+
+static void mmio_debug_resume(struct intel_uncore_mmio_debug *mmio_debug)
+{
+	lockdep_assert_held(&mmio_debug->lock);
+
+	if (!--mmio_debug->suspend_count)
+		mmio_debug->unclaimed_mmio_check = mmio_debug->saved_mmio_check;
+}
+
 static const char * const forcewake_domain_names[] = {
 	"render",
 	"blitter",
@@ -476,6 +502,11 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore)
 {
 	bool ret = false;
 
+	lockdep_assert_held(&uncore->debug->lock);
+
+	if (uncore->debug->suspend_count)
+		return false;
+
 	if (intel_uncore_has_fpga_dbg_unclaimed(uncore))
 		ret |= fpga_check_for_unclaimed_mmio(uncore);
 
@@ -608,17 +639,11 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
 void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
 {
 	spin_lock_irq(&uncore->lock);
-	if (!uncore->user_forcewake.count++) {
+	if (!uncore->user_forcewake_count++) {
 		intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
-
-		/* Save and disable mmio debugging for the user bypass */
-		uncore->user_forcewake.saved_mmio_check =
-			uncore->unclaimed_mmio_check;
-		uncore->user_forcewake.saved_mmio_debug =
-			i915_modparams.mmio_debug;
-
-		uncore->unclaimed_mmio_check = 0;
-		i915_modparams.mmio_debug = 0;
+		spin_lock(&uncore->debug->lock);
+		mmio_debug_suspend(uncore->debug);
+		spin_unlock(&uncore->debug->lock);
 	}
 	spin_unlock_irq(&uncore->lock);
 }
@@ -633,15 +658,14 @@ void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
 void intel_uncore_forcewake_user_put(struct intel_uncore *uncore)
 {
 	spin_lock_irq(&uncore->lock);
-	if (!--uncore->user_forcewake.count) {
-		if (intel_uncore_unclaimed_mmio(uncore))
+	if (!--uncore->user_forcewake_count) {
+		spin_lock(&uncore->debug->lock);
+		mmio_debug_resume(uncore->debug);
+
+		if (check_for_unclaimed_mmio(uncore))
 			dev_info(uncore->i915->drm.dev,
 				 "Invalid mmio detected during user access\n");
-
-		uncore->unclaimed_mmio_check =
-			uncore->user_forcewake.saved_mmio_check;
-		i915_modparams.mmio_debug =
-			uncore->user_forcewake.saved_mmio_debug;
+		spin_unlock(&uncore->debug->lock);
 
 		intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL);
 	}
@@ -1088,7 +1112,16 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
 	if (likely(!i915_modparams.mmio_debug))
 		return;
 
+	/* interrupts are disabled and re-enabled around uncore->lock usage */
+	lockdep_assert_held(&uncore->lock);
+
+	if (before)
+		spin_lock(&uncore->debug->lock);
+
 	__unclaimed_reg_debug(uncore, reg, read, before);
+
+	if (!before)
+		spin_unlock(&uncore->debug->lock);
 }
 
 #define GEN2_READ_HEADER(x) \
@@ -1607,6 +1640,7 @@ void intel_uncore_init_early(struct intel_uncore *uncore,
 	spin_lock_init(&uncore->lock);
 	uncore->i915 = i915;
 	uncore->rpm = &i915->runtime_pm;
+	uncore->debug = &i915->mmio_debug;
 }
 
 static void uncore_raw_init(struct intel_uncore *uncore)
@@ -1632,7 +1666,6 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
 	ret = intel_uncore_fw_domains_init(uncore);
 	if (ret)
 		return ret;
-
 	forcewake_early_sanitize(uncore, 0);
 
 	if (IS_GEN_RANGE(i915, 6, 7)) {
@@ -1681,8 +1714,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
 	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
 		uncore->flags |= UNCORE_HAS_FORCEWAKE;
 
-	uncore->unclaimed_mmio_check = 1;
-
 	if (!intel_uncore_has_forcewake(uncore)) {
 		uncore_raw_init(uncore);
 	} else {
@@ -1707,7 +1738,7 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
 		uncore->flags |= UNCORE_HAS_FIFO;
 
 	/* clear out unclaimed reg detection bit */
-	if (check_for_unclaimed_mmio(uncore))
+	if (intel_uncore_unclaimed_mmio(uncore))
 		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
 
 	return 0;
@@ -1952,7 +1983,13 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
 
 bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
 {
-	return check_for_unclaimed_mmio(uncore);
+	bool ret;
+
+	spin_lock_irq(&uncore->debug->lock);
+	ret = check_for_unclaimed_mmio(uncore);
+	spin_unlock_irq(&uncore->debug->lock);
+
+	return ret;
 }
 
 bool
@@ -1960,24 +1997,24 @@ intel_uncore_arm_unclaimed_mmio_detection(struct intel_uncore *uncore)
 {
 	bool ret = false;
 
-	spin_lock_irq(&uncore->lock);
+	spin_lock_irq(&uncore->debug->lock);
 
-	if (unlikely(uncore->unclaimed_mmio_check <= 0))
+	if (unlikely(uncore->debug->unclaimed_mmio_check <= 0))
 		goto out;
 
-	if (unlikely(intel_uncore_unclaimed_mmio(uncore))) {
+	if (unlikely(check_for_unclaimed_mmio(uncore))) {
 		if (!i915_modparams.mmio_debug) {
 			DRM_DEBUG("Unclaimed register detected, "
 				  "enabling oneshot unclaimed register reporting. "
 				  "Please use i915.mmio_debug=N for more information.\n");
 			i915_modparams.mmio_debug++;
 		}
-		uncore->unclaimed_mmio_check--;
+		uncore->debug->unclaimed_mmio_check--;
 		ret = true;
 	}
 
 out:
-	spin_unlock_irq(&uncore->lock);
+	spin_unlock_irq(&uncore->debug->lock);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index e603d210a34d..414fc2cb0459 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -36,6 +36,13 @@ struct drm_i915_private;
 struct intel_runtime_pm;
 struct intel_uncore;
 
+struct intel_uncore_mmio_debug {
+	spinlock_t lock; /** lock is also taken in irq contexts. */
+	int unclaimed_mmio_check;
+	int saved_mmio_check;
+	u32 suspend_count;
+};
+
 enum forcewake_domain_id {
 	FW_DOMAIN_ID_RENDER = 0,
 	FW_DOMAIN_ID_BLITTER,
@@ -137,14 +144,9 @@ struct intel_uncore {
 		u32 __iomem *reg_ack;
 	} *fw_domain[FW_DOMAIN_ID_COUNT];
 
-	struct {
-		unsigned int count;
-
-		int saved_mmio_check;
-		int saved_mmio_debug;
-	} user_forcewake;
+	unsigned int user_forcewake_count;
 
-	int unclaimed_mmio_check;
+	struct intel_uncore_mmio_debug *debug;
 };
 
 /* Iterate over initialised fw domains */
@@ -179,6 +181,8 @@ intel_uncore_has_fifo(const struct intel_uncore *uncore)
 	return uncore->flags & UNCORE_HAS_FIFO;
 }
 
+void
+intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug);
 void intel_uncore_init_early(struct intel_uncore *uncore,
 			     struct drm_i915_private *i915);
 int intel_uncore_init_mmio(struct intel_uncore *uncore);
-- 
2.22.0

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

Re: [RFC 1/3] drm/i915: split out uncore_mmio_debug

[Mika Kuoppala]

Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> writes:

> Multiple uncore structures will share the debug infrastructure, so
> move it to a common place and add extra locking around it.
> Also, since we now have a separate object, it is cleaner to have
> dedicated functions working on the object to stop and restart the
> mmio debug. Apart from the cosmetic changes, this patch introduces
> 2 functional updates:
>
> - All calls to check_for_unclaimed_mmio will now return false when
>   the debug is suspended, not just the ones that are active only when
>   i915_modparams.mmio_debug is set. If we don't trust the result of the
>   check while a user is doing mmio access then we shouldn't attempt the
>   check anywhere.
>
> - i915_modparams.mmio_debug is not save/restored anymore around user
>   access. The value is now never touched by the kernel while debug is
>   disabled so no need for save/restore.
>
> v2: squash mmio_debug patches, restrict mmio_debug lock usage (Chris)
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
>  drivers/gpu/drm/i915/i915_drv.c     |  3 +-
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_uncore.c | 91 ++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_uncore.h | 18 +++---
>  5 files changed, 79 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3b15266c54fd..2310512111ab 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1129,7 +1129,7 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
>  	unsigned int tmp;
>  
>  	seq_printf(m, "user.bypass_count = %u\n",
> -		   uncore->user_forcewake.count);
> +		   uncore->user_forcewake_count);
>  
>  	for_each_fw_domain(fw_domain, uncore, tmp)
>  		seq_printf(m, "%s.wake_count = %u\n",
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3480db36b63f..fbbff4a133ba 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -744,6 +744,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  
>  	intel_device_info_subplatform_init(dev_priv);
>  
> +	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
>  	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
>  
>  	spin_lock_init(&dev_priv->irq_lock);
> @@ -2044,7 +2045,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>  
>  out:
>  	enable_rpm_wakeref_asserts(rpm);
> -	if (!dev_priv->uncore.user_forcewake.count)
> +	if (!dev_priv->uncore.user_forcewake_count)
>  		intel_runtime_pm_driver_release(rpm);
>  
>  	return ret;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c9476f24f5c1..13c27a75dca8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1346,6 +1346,7 @@ struct drm_i915_private {
>  	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
>  
>  	struct intel_uncore uncore;
> +	struct intel_uncore_mmio_debug mmio_debug;
>  
>  	struct i915_virtual_gpu vgpu;
>  
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 39e8710afdd9..9e583f13a9e4 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -34,6 +34,32 @@
>  
>  #define __raw_posting_read(...) ((void)__raw_uncore_read32(__VA_ARGS__))
>  
> +void
> +intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug)
> +{
> +	spin_lock_init(&mmio_debug->lock);
> +	mmio_debug->unclaimed_mmio_check = 1;
> +}
> +
> +static void mmio_debug_suspend(struct intel_uncore_mmio_debug *mmio_debug)
> +{
> +	lockdep_assert_held(&mmio_debug->lock);
> +
> +	/* Save and disable mmio debugging for the user bypass */
> +	if (!mmio_debug->suspend_count++) {
> +		mmio_debug->saved_mmio_check = mmio_debug->unclaimed_mmio_check;
> +		mmio_debug->unclaimed_mmio_check = 0;
> +	}
> +}
> +
> +static void mmio_debug_resume(struct intel_uncore_mmio_debug *mmio_debug)
> +{
> +	lockdep_assert_held(&mmio_debug->lock);
> +
> +	if (!--mmio_debug->suspend_count)
> +		mmio_debug->unclaimed_mmio_check = mmio_debug->saved_mmio_check;
> +}
> +
>  static const char * const forcewake_domain_names[] = {
>  	"render",
>  	"blitter",
> @@ -476,6 +502,11 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore)
>  {
>  	bool ret = false;
>  
> +	lockdep_assert_held(&uncore->debug->lock);
> +
> +	if (uncore->debug->suspend_count)
> +		return false;
> +
>  	if (intel_uncore_has_fpga_dbg_unclaimed(uncore))
>  		ret |= fpga_check_for_unclaimed_mmio(uncore);
>  
> @@ -608,17 +639,11 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
>  void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>  {
>  	spin_lock_irq(&uncore->lock);
> -	if (!uncore->user_forcewake.count++) {
> +	if (!uncore->user_forcewake_count++) {
>  		intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
> -
> -		/* Save and disable mmio debugging for the user bypass */
> -		uncore->user_forcewake.saved_mmio_check =
> -			uncore->unclaimed_mmio_check;
> -		uncore->user_forcewake.saved_mmio_debug =
> -			i915_modparams.mmio_debug;
> -
> -		uncore->unclaimed_mmio_check = 0;
> -		i915_modparams.mmio_debug = 0;
> +		spin_lock(&uncore->debug->lock);

For me it looks like you need spin_lock_irq here also
like with the parent lock above.
-Mika

> +		mmio_debug_suspend(uncore->debug);
> +		spin_unlock(&uncore->debug->lock);
>  	}
>  	spin_unlock_irq(&uncore->lock);
>  }
> @@ -633,15 +658,14 @@ void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>  void intel_uncore_forcewake_user_put(struct intel_uncore *uncore)
>  {
>  	spin_lock_irq(&uncore->lock);
> -	if (!--uncore->user_forcewake.count) {
> -		if (intel_uncore_unclaimed_mmio(uncore))
> +	if (!--uncore->user_forcewake_count) {
> +		spin_lock(&uncore->debug->lock);
> +		mmio_debug_resume(uncore->debug);
> +
> +		if (check_for_unclaimed_mmio(uncore))
>  			dev_info(uncore->i915->drm.dev,
>  				 "Invalid mmio detected during user access\n");
> -
> -		uncore->unclaimed_mmio_check =
> -			uncore->user_forcewake.saved_mmio_check;
> -		i915_modparams.mmio_debug =
> -			uncore->user_forcewake.saved_mmio_debug;
> +		spin_unlock(&uncore->debug->lock);
>  
>  		intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL);
>  	}
> @@ -1088,7 +1112,16 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
>  	if (likely(!i915_modparams.mmio_debug))
>  		return;
>  
> +	/* interrupts are disabled and re-enabled around uncore->lock usage */
> +	lockdep_assert_held(&uncore->lock);
> +
> +	if (before)
> +		spin_lock(&uncore->debug->lock);
> +
>  	__unclaimed_reg_debug(uncore, reg, read, before);
> +
> +	if (!before)
> +		spin_unlock(&uncore->debug->lock);
>  }
>  
>  #define GEN2_READ_HEADER(x) \
> @@ -1607,6 +1640,7 @@ void intel_uncore_init_early(struct intel_uncore *uncore,
>  	spin_lock_init(&uncore->lock);
>  	uncore->i915 = i915;
>  	uncore->rpm = &i915->runtime_pm;
> +	uncore->debug = &i915->mmio_debug;
>  }
>  
>  static void uncore_raw_init(struct intel_uncore *uncore)
> @@ -1632,7 +1666,6 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
>  	ret = intel_uncore_fw_domains_init(uncore);
>  	if (ret)
>  		return ret;
> -
>  	forcewake_early_sanitize(uncore, 0);
>  
>  	if (IS_GEN_RANGE(i915, 6, 7)) {
> @@ -1681,8 +1714,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>  	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
>  		uncore->flags |= UNCORE_HAS_FORCEWAKE;
>  
> -	uncore->unclaimed_mmio_check = 1;
> -
>  	if (!intel_uncore_has_forcewake(uncore)) {
>  		uncore_raw_init(uncore);
>  	} else {
> @@ -1707,7 +1738,7 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>  		uncore->flags |= UNCORE_HAS_FIFO;
>  
>  	/* clear out unclaimed reg detection bit */
> -	if (check_for_unclaimed_mmio(uncore))
> +	if (intel_uncore_unclaimed_mmio(uncore))
>  		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
>  
>  	return 0;
> @@ -1952,7 +1983,13 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
>  
>  bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
>  {
> -	return check_for_unclaimed_mmio(uncore);
> +	bool ret;
> +
> +	spin_lock_irq(&uncore->debug->lock);
> +	ret = check_for_unclaimed_mmio(uncore);
> +	spin_unlock_irq(&uncore->debug->lock);
> +
> +	return ret;
>  }
>  
>  bool
> @@ -1960,24 +1997,24 @@ intel_uncore_arm_unclaimed_mmio_detection(struct intel_uncore *uncore)
>  {
>  	bool ret = false;
>  
> -	spin_lock_irq(&uncore->lock);
> +	spin_lock_irq(&uncore->debug->lock);
>  
> -	if (unlikely(uncore->unclaimed_mmio_check <= 0))
> +	if (unlikely(uncore->debug->unclaimed_mmio_check <= 0))
>  		goto out;
>  
> -	if (unlikely(intel_uncore_unclaimed_mmio(uncore))) {
> +	if (unlikely(check_for_unclaimed_mmio(uncore))) {
>  		if (!i915_modparams.mmio_debug) {
>  			DRM_DEBUG("Unclaimed register detected, "
>  				  "enabling oneshot unclaimed register reporting. "
>  				  "Please use i915.mmio_debug=N for more information.\n");
>  			i915_modparams.mmio_debug++;
>  		}
> -		uncore->unclaimed_mmio_check--;
> +		uncore->debug->unclaimed_mmio_check--;
>  		ret = true;
>  	}
>  
>  out:
> -	spin_unlock_irq(&uncore->lock);
> +	spin_unlock_irq(&uncore->debug->lock);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index e603d210a34d..414fc2cb0459 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -36,6 +36,13 @@ struct drm_i915_private;
>  struct intel_runtime_pm;
>  struct intel_uncore;
>  
> +struct intel_uncore_mmio_debug {
> +	spinlock_t lock; /** lock is also taken in irq contexts. */
> +	int unclaimed_mmio_check;
> +	int saved_mmio_check;
> +	u32 suspend_count;
> +};
> +
>  enum forcewake_domain_id {
>  	FW_DOMAIN_ID_RENDER = 0,
>  	FW_DOMAIN_ID_BLITTER,
> @@ -137,14 +144,9 @@ struct intel_uncore {
>  		u32 __iomem *reg_ack;
>  	} *fw_domain[FW_DOMAIN_ID_COUNT];
>  
> -	struct {
> -		unsigned int count;
> -
> -		int saved_mmio_check;
> -		int saved_mmio_debug;
> -	} user_forcewake;
> +	unsigned int user_forcewake_count;
>  
> -	int unclaimed_mmio_check;
> +	struct intel_uncore_mmio_debug *debug;
>  };
>  
>  /* Iterate over initialised fw domains */
> @@ -179,6 +181,8 @@ intel_uncore_has_fifo(const struct intel_uncore *uncore)
>  	return uncore->flags & UNCORE_HAS_FIFO;
>  }
>  
> +void
> +intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug);
>  void intel_uncore_init_early(struct intel_uncore *uncore,
>  			     struct drm_i915_private *i915);
>  int intel_uncore_init_mmio(struct intel_uncore *uncore);
> -- 
> 2.22.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 1/3] drm/i915: split out uncore_mmio_debug

[Daniele Ceraolo Spurio]

On 8/8/19 6:08 AM, Mika Kuoppala wrote:
> Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> writes:
> 
>> Multiple uncore structures will share the debug infrastructure, so
>> move it to a common place and add extra locking around it.
>> Also, since we now have a separate object, it is cleaner to have
>> dedicated functions working on the object to stop and restart the
>> mmio debug. Apart from the cosmetic changes, this patch introduces
>> 2 functional updates:
>>
>> - All calls to check_for_unclaimed_mmio will now return false when
>>    the debug is suspended, not just the ones that are active only when
>>    i915_modparams.mmio_debug is set. If we don't trust the result of the
>>    check while a user is doing mmio access then we shouldn't attempt the
>>    check anywhere.
>>
>> - i915_modparams.mmio_debug is not save/restored anymore around user
>>    access. The value is now never touched by the kernel while debug is
>>    disabled so no need for save/restore.
>>
>> v2: squash mmio_debug patches, restrict mmio_debug lock usage (Chris)
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
>>   drivers/gpu/drm/i915/i915_drv.c     |  3 +-
>>   drivers/gpu/drm/i915/i915_drv.h     |  1 +
>>   drivers/gpu/drm/i915/intel_uncore.c | 91 ++++++++++++++++++++---------
>>   drivers/gpu/drm/i915/intel_uncore.h | 18 +++---
>>   5 files changed, 79 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 3b15266c54fd..2310512111ab 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1129,7 +1129,7 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
>>   	unsigned int tmp;
>>   
>>   	seq_printf(m, "user.bypass_count = %u\n",
>> -		   uncore->user_forcewake.count);
>> +		   uncore->user_forcewake_count);
>>   
>>   	for_each_fw_domain(fw_domain, uncore, tmp)
>>   		seq_printf(m, "%s.wake_count = %u\n",
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 3480db36b63f..fbbff4a133ba 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -744,6 +744,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>>   
>>   	intel_device_info_subplatform_init(dev_priv);
>>   
>> +	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
>>   	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
>>   
>>   	spin_lock_init(&dev_priv->irq_lock);
>> @@ -2044,7 +2045,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>>   
>>   out:
>>   	enable_rpm_wakeref_asserts(rpm);
>> -	if (!dev_priv->uncore.user_forcewake.count)
>> +	if (!dev_priv->uncore.user_forcewake_count)
>>   		intel_runtime_pm_driver_release(rpm);
>>   
>>   	return ret;
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index c9476f24f5c1..13c27a75dca8 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1346,6 +1346,7 @@ struct drm_i915_private {
>>   	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
>>   
>>   	struct intel_uncore uncore;
>> +	struct intel_uncore_mmio_debug mmio_debug;
>>   
>>   	struct i915_virtual_gpu vgpu;
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 39e8710afdd9..9e583f13a9e4 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -34,6 +34,32 @@
>>   
>>   #define __raw_posting_read(...) ((void)__raw_uncore_read32(__VA_ARGS__))
>>   
>> +void
>> +intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug)
>> +{
>> +	spin_lock_init(&mmio_debug->lock);
>> +	mmio_debug->unclaimed_mmio_check = 1;
>> +}
>> +
>> +static void mmio_debug_suspend(struct intel_uncore_mmio_debug *mmio_debug)
>> +{
>> +	lockdep_assert_held(&mmio_debug->lock);
>> +
>> +	/* Save and disable mmio debugging for the user bypass */
>> +	if (!mmio_debug->suspend_count++) {
>> +		mmio_debug->saved_mmio_check = mmio_debug->unclaimed_mmio_check;
>> +		mmio_debug->unclaimed_mmio_check = 0;
>> +	}
>> +}
>> +
>> +static void mmio_debug_resume(struct intel_uncore_mmio_debug *mmio_debug)
>> +{
>> +	lockdep_assert_held(&mmio_debug->lock);
>> +
>> +	if (!--mmio_debug->suspend_count)
>> +		mmio_debug->unclaimed_mmio_check = mmio_debug->saved_mmio_check;
>> +}
>> +
>>   static const char * const forcewake_domain_names[] = {
>>   	"render",
>>   	"blitter",
>> @@ -476,6 +502,11 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore)
>>   {
>>   	bool ret = false;
>>   
>> +	lockdep_assert_held(&uncore->debug->lock);
>> +
>> +	if (uncore->debug->suspend_count)
>> +		return false;
>> +
>>   	if (intel_uncore_has_fpga_dbg_unclaimed(uncore))
>>   		ret |= fpga_check_for_unclaimed_mmio(uncore);
>>   
>> @@ -608,17 +639,11 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
>>   void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>>   {
>>   	spin_lock_irq(&uncore->lock);
>> -	if (!uncore->user_forcewake.count++) {
>> +	if (!uncore->user_forcewake_count++) {
>>   		intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
>> -
>> -		/* Save and disable mmio debugging for the user bypass */
>> -		uncore->user_forcewake.saved_mmio_check =
>> -			uncore->unclaimed_mmio_check;
>> -		uncore->user_forcewake.saved_mmio_debug =
>> -			i915_modparams.mmio_debug;
>> -
>> -		uncore->unclaimed_mmio_check = 0;
>> -		i915_modparams.mmio_debug = 0;
>> +		spin_lock(&uncore->debug->lock);
> 
> For me it looks like you need spin_lock_irq here also
> like with the parent lock above.

The parent lock should ensure that the irqs are already off at this 
point, so no need to disable them twice. Also, spin_unlock_irq can't be 
called twice because the first of the 2 calls would re-enable interrupts 
while the second lock is still taken. Irqflags would solve that, but 
still wouldn't give any benefit compared to a straight lock IMO.

Daniele

> -Mika
> 
>> +		mmio_debug_suspend(uncore->debug);
>> +		spin_unlock(&uncore->debug->lock);
>>   	}
>>   	spin_unlock_irq(&uncore->lock);
>>   }
>> @@ -633,15 +658,14 @@ void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>>   void intel_uncore_forcewake_user_put(struct intel_uncore *uncore)
>>   {
>>   	spin_lock_irq(&uncore->lock);
>> -	if (!--uncore->user_forcewake.count) {
>> -		if (intel_uncore_unclaimed_mmio(uncore))
>> +	if (!--uncore->user_forcewake_count) {
>> +		spin_lock(&uncore->debug->lock);
>> +		mmio_debug_resume(uncore->debug);
>> +
>> +		if (check_for_unclaimed_mmio(uncore))
>>   			dev_info(uncore->i915->drm.dev,
>>   				 "Invalid mmio detected during user access\n");
>> -
>> -		uncore->unclaimed_mmio_check =
>> -			uncore->user_forcewake.saved_mmio_check;
>> -		i915_modparams.mmio_debug =
>> -			uncore->user_forcewake.saved_mmio_debug;
>> +		spin_unlock(&uncore->debug->lock);
>>   
>>   		intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL);
>>   	}
>> @@ -1088,7 +1112,16 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
>>   	if (likely(!i915_modparams.mmio_debug))
>>   		return;
>>   
>> +	/* interrupts are disabled and re-enabled around uncore->lock usage */
>> +	lockdep_assert_held(&uncore->lock);
>> +
>> +	if (before)
>> +		spin_lock(&uncore->debug->lock);
>> +
>>   	__unclaimed_reg_debug(uncore, reg, read, before);
>> +
>> +	if (!before)
>> +		spin_unlock(&uncore->debug->lock);
>>   }
>>   
>>   #define GEN2_READ_HEADER(x) \
>> @@ -1607,6 +1640,7 @@ void intel_uncore_init_early(struct intel_uncore *uncore,
>>   	spin_lock_init(&uncore->lock);
>>   	uncore->i915 = i915;
>>   	uncore->rpm = &i915->runtime_pm;
>> +	uncore->debug = &i915->mmio_debug;
>>   }
>>   
>>   static void uncore_raw_init(struct intel_uncore *uncore)
>> @@ -1632,7 +1666,6 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
>>   	ret = intel_uncore_fw_domains_init(uncore);
>>   	if (ret)
>>   		return ret;
>> -
>>   	forcewake_early_sanitize(uncore, 0);
>>   
>>   	if (IS_GEN_RANGE(i915, 6, 7)) {
>> @@ -1681,8 +1714,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>>   	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
>>   		uncore->flags |= UNCORE_HAS_FORCEWAKE;
>>   
>> -	uncore->unclaimed_mmio_check = 1;
>> -
>>   	if (!intel_uncore_has_forcewake(uncore)) {
>>   		uncore_raw_init(uncore);
>>   	} else {
>> @@ -1707,7 +1738,7 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>>   		uncore->flags |= UNCORE_HAS_FIFO;
>>   
>>   	/* clear out unclaimed reg detection bit */
>> -	if (check_for_unclaimed_mmio(uncore))
>> +	if (intel_uncore_unclaimed_mmio(uncore))
>>   		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
>>   
>>   	return 0;
>> @@ -1952,7 +1983,13 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
>>   
>>   bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
>>   {
>> -	return check_for_unclaimed_mmio(uncore);
>> +	bool ret;
>> +
>> +	spin_lock_irq(&uncore->debug->lock);
>> +	ret = check_for_unclaimed_mmio(uncore);
>> +	spin_unlock_irq(&uncore->debug->lock);
>> +
>> +	return ret;
>>   }
>>   
>>   bool
>> @@ -1960,24 +1997,24 @@ intel_uncore_arm_unclaimed_mmio_detection(struct intel_uncore *uncore)
>>   {
>>   	bool ret = false;
>>   
>> -	spin_lock_irq(&uncore->lock);
>> +	spin_lock_irq(&uncore->debug->lock);
>>   
>> -	if (unlikely(uncore->unclaimed_mmio_check <= 0))
>> +	if (unlikely(uncore->debug->unclaimed_mmio_check <= 0))
>>   		goto out;
>>   
>> -	if (unlikely(intel_uncore_unclaimed_mmio(uncore))) {
>> +	if (unlikely(check_for_unclaimed_mmio(uncore))) {
>>   		if (!i915_modparams.mmio_debug) {
>>   			DRM_DEBUG("Unclaimed register detected, "
>>   				  "enabling oneshot unclaimed register reporting. "
>>   				  "Please use i915.mmio_debug=N for more information.\n");
>>   			i915_modparams.mmio_debug++;
>>   		}
>> -		uncore->unclaimed_mmio_check--;
>> +		uncore->debug->unclaimed_mmio_check--;
>>   		ret = true;
>>   	}
>>   
>>   out:
>> -	spin_unlock_irq(&uncore->lock);
>> +	spin_unlock_irq(&uncore->debug->lock);
>>   
>>   	return ret;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
>> index e603d210a34d..414fc2cb0459 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.h
>> +++ b/drivers/gpu/drm/i915/intel_uncore.h
>> @@ -36,6 +36,13 @@ struct drm_i915_private;
>>   struct intel_runtime_pm;
>>   struct intel_uncore;
>>   
>> +struct intel_uncore_mmio_debug {
>> +	spinlock_t lock; /** lock is also taken in irq contexts. */
>> +	int unclaimed_mmio_check;
>> +	int saved_mmio_check;
>> +	u32 suspend_count;
>> +};
>> +
>>   enum forcewake_domain_id {
>>   	FW_DOMAIN_ID_RENDER = 0,
>>   	FW_DOMAIN_ID_BLITTER,
>> @@ -137,14 +144,9 @@ struct intel_uncore {
>>   		u32 __iomem *reg_ack;
>>   	} *fw_domain[FW_DOMAIN_ID_COUNT];
>>   
>> -	struct {
>> -		unsigned int count;
>> -
>> -		int saved_mmio_check;
>> -		int saved_mmio_debug;
>> -	} user_forcewake;
>> +	unsigned int user_forcewake_count;
>>   
>> -	int unclaimed_mmio_check;
>> +	struct intel_uncore_mmio_debug *debug;
>>   };
>>   
>>   /* Iterate over initialised fw domains */
>> @@ -179,6 +181,8 @@ intel_uncore_has_fifo(const struct intel_uncore *uncore)
>>   	return uncore->flags & UNCORE_HAS_FIFO;
>>   }
>>   
>> +void
>> +intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug);
>>   void intel_uncore_init_early(struct intel_uncore *uncore,
>>   			     struct drm_i915_private *i915);
>>   int intel_uncore_init_mmio(struct intel_uncore *uncore);
>> -- 
>> 2.22.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 1/3] drm/i915: split out uncore_mmio_debug

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-08-08 02:44:21)
> Multiple uncore structures will share the debug infrastructure, so
> move it to a common place and add extra locking around it.
> Also, since we now have a separate object, it is cleaner to have
> dedicated functions working on the object to stop and restart the
> mmio debug. Apart from the cosmetic changes, this patch introduces
> 2 functional updates:
> 
> - All calls to check_for_unclaimed_mmio will now return false when
>   the debug is suspended, not just the ones that are active only when
>   i915_modparams.mmio_debug is set. If we don't trust the result of the
>   check while a user is doing mmio access then we shouldn't attempt the
>   check anywhere.
> 
> - i915_modparams.mmio_debug is not save/restored anymore around user
>   access. The value is now never touched by the kernel while debug is
>   disabled so no need for save/restore.

Yup. Early return from check_for_unclaimed_mmio() if suspended, with
user_forcewake calling mmio_suspend.

> v2: squash mmio_debug patches, restrict mmio_debug lock usage (Chris)
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 2/3] drm/i915: introduce display_uncore

[Daniele Ceraolo Spurio]

A forcewake-less uncore to be used to decouple GT accesses from display
ones to avoid serializing them when there is no need.

New accessors that implicitly use the new uncore have also been added.
To avoid accessing the same register from 2 different uncores (which
could cause hard hangs), the new accessors expect registers to be
defined with the new _DE_MMIO macro.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_display_reg.h  | 22 +++++++++++++
 drivers/gpu/drm/i915/i915_drv.c               | 17 ++++++++--
 drivers/gpu/drm/i915/i915_drv.h               | 31 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_uncore.c           |  9 +++++-
 4 files changed, 76 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_display_reg.h

diff --git a/drivers/gpu/drm/i915/display/intel_display_reg.h b/drivers/gpu/drm/i915/display/intel_display_reg.h
new file mode 100644
index 000000000000..ac0c6975271d
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_display_reg.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef _INTEL_DISPLAY_REG_H_
+#define _INTEL_DISPLAY_REG_H_
+
+#include "i915_reg.h"
+
+typedef struct {
+	const i915_reg_t reg;
+} intel_de_reg_t;
+
+#define _DE_MMIO(r) ((const intel_de_reg_t){ .reg = _MMIO(r) })
+
+static inline i915_reg_t intel_de_reg_to_mmio(intel_de_reg_t reg)
+{
+	return reg.reg;
+}
+
+#endif
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fbbff4a133ba..af015ecf3dcc 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -746,6 +746,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 
 	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
 	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
+	intel_uncore_init_early(&dev_priv->de_uncore, dev_priv);
 
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->gpu_error.lock);
@@ -841,6 +842,10 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		goto err_bridge;
 
+	ret = intel_uncore_init_mmio(&dev_priv->de_uncore);
+	if (ret < 0)
+		goto err_uncore;
+
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev_priv);
 
@@ -852,14 +857,16 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv)
 
 	ret = intel_engines_init_mmio(dev_priv);
 	if (ret)
-		goto err_uncore;
+		goto err_mchbar;
 
 	i915_gem_init_mmio(dev_priv);
 
 	return 0;
 
-err_uncore:
+err_mchbar:
 	intel_teardown_mchbar(dev_priv);
+	intel_uncore_fini_mmio(&dev_priv->de_uncore);
+err_uncore:
 	intel_uncore_fini_mmio(&dev_priv->uncore);
 err_bridge:
 	pci_dev_put(dev_priv->bridge_dev);
@@ -875,6 +882,7 @@ static void i915_driver_mmio_release(struct drm_i915_private *dev_priv)
 {
 	intel_engines_cleanup(dev_priv);
 	intel_teardown_mchbar(dev_priv);
+	intel_uncore_fini_mmio(&dev_priv->de_uncore);
 	intel_uncore_fini_mmio(&dev_priv->uncore);
 	pci_dev_put(dev_priv->bridge_dev);
 }
@@ -2010,6 +2018,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 
 	i915_gem_suspend_late(dev_priv);
 
+	intel_uncore_suspend(&dev_priv->de_uncore);
 	intel_uncore_suspend(&dev_priv->uncore);
 
 	intel_power_domains_suspend(dev_priv,
@@ -2199,6 +2208,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 			  ret);
 
 	intel_uncore_resume_early(&dev_priv->uncore);
+	intel_uncore_resume_early(&dev_priv->de_uncore);
 
 	intel_gt_check_and_clear_faults(&dev_priv->gt);
 
@@ -2764,6 +2774,7 @@ static int intel_runtime_suspend(struct device *kdev)
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
+	intel_uncore_suspend(&dev_priv->de_uncore);
 	intel_uncore_suspend(&dev_priv->uncore);
 
 	intel_display_power_suspend(dev_priv);
@@ -2774,6 +2785,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	if (ret) {
 		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
 		intel_uncore_runtime_resume(&dev_priv->uncore);
+		intel_uncore_runtime_resume(&dev_priv->de_uncore);
 
 		intel_runtime_pm_enable_interrupts(dev_priv);
 
@@ -2851,6 +2863,7 @@ static int intel_runtime_resume(struct device *kdev)
 		ret = vlv_resume_prepare(dev_priv, true);
 
 	intel_uncore_runtime_resume(&dev_priv->uncore);
+	intel_uncore_runtime_resume(&dev_priv->de_uncore);
 
 	intel_runtime_pm_enable_interrupts(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 13c27a75dca8..015b490c14d6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -66,6 +66,7 @@
 #include "display/intel_bios.h"
 #include "display/intel_display.h"
 #include "display/intel_display_power.h"
+#include "display/intel_display_reg.h"
 #include "display/intel_dpll_mgr.h"
 #include "display/intel_frontbuffer.h"
 #include "display/intel_opregion.h"
@@ -1346,6 +1347,7 @@ struct drm_i915_private {
 	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
 
 	struct intel_uncore uncore;
+	struct intel_uncore de_uncore;
 	struct intel_uncore_mmio_debug mmio_debug;
 
 	struct i915_virtual_gpu vgpu;
@@ -2687,6 +2689,35 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 #define I915_READ_FW(reg__) __I915_REG_OP(read_fw, dev_priv, (reg__))
 #define I915_WRITE_FW(reg__, val__) __I915_REG_OP(write_fw, dev_priv, (reg__), (val__))
 
+/*
+ * The following are mmio-accessors that use an independent lock and skip all
+ * the forcewake logic, to be used to access display registers, which are
+ * outside the GT forcewake wells.
+ */
+static inline u32
+intel_de_read(struct drm_i915_private *i915, intel_de_reg_t reg)
+{
+	return intel_uncore_read(&i915->de_uncore, intel_de_reg_to_mmio(reg));
+}
+
+static inline void
+intel_de_posting_read(struct drm_i915_private *i915, intel_de_reg_t reg)
+{
+	intel_uncore_posting_read(&i915->de_uncore, intel_de_reg_to_mmio(reg));
+}
+
+static inline void
+intel_de_write(struct drm_i915_private *i915, intel_de_reg_t reg, u32 val)
+{
+	intel_uncore_write(&i915->de_uncore, intel_de_reg_to_mmio(reg), val);
+}
+
+static inline void
+intel_de_rmw(struct drm_i915_private *i915, intel_de_reg_t reg, u32 clear, u32 set)
+{
+	intel_uncore_rmw(&i915->de_uncore, intel_de_reg_to_mmio(reg), clear, set);
+}
+
 void i915_memcpy_init_early(struct drm_i915_private *dev_priv);
 bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len);
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 9e583f13a9e4..fba5e2a63888 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1702,6 +1702,12 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
 	return 0;
 }
 
+static bool
+intel_uncore_is_display(const struct intel_uncore *uncore)
+{
+	return uncore == &uncore->i915->de_uncore;
+}
+
 int intel_uncore_init_mmio(struct intel_uncore *uncore)
 {
 	struct drm_i915_private *i915 = uncore->i915;
@@ -1711,7 +1717,8 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
 	if (ret)
 		return ret;
 
-	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
+	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915) &&
+	    !intel_uncore_is_display(uncore))
 		uncore->flags |= UNCORE_HAS_FORCEWAKE;
 
 	if (!intel_uncore_has_forcewake(uncore)) {
-- 
2.22.0

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

[RFC 3/3] drm/i915: convert a couple of registers to _DE_MMIO

[Daniele Ceraolo Spurio]

As an example of usage of the new accessors

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 .../gpu/drm/i915/display/intel_display_reg.h  | 44 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_hdmi.c     | 32 +++++++-------
 drivers/gpu/drm/i915/i915_reg.h               | 44 -------------------
 3 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_reg.h b/drivers/gpu/drm/i915/display/intel_display_reg.h
index ac0c6975271d..2b61d4fd12e3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_reg.h
+++ b/drivers/gpu/drm/i915/display/intel_display_reg.h
@@ -19,4 +19,48 @@ static inline i915_reg_t intel_de_reg_to_mmio(intel_de_reg_t reg)
 	return reg.reg;
 }
 
+/* Video Data Island Packet control */
+#define VIDEO_DIP_DATA		_DE_MMIO(0x61178)
+/* Read the description of VIDEO_DIP_DATA (before Haswell) or VIDEO_DIP_ECC
+ * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each byte
+ * of the infoframe structure specified by CEA-861. */
+#define   VIDEO_DIP_DATA_SIZE	32
+#define   VIDEO_DIP_VSC_DATA_SIZE	36
+#define   VIDEO_DIP_PPS_DATA_SIZE	132
+#define VIDEO_DIP_CTL		_DE_MMIO(0x61170)
+/* Pre HSW: */
+#define   VIDEO_DIP_ENABLE		(1 << 31)
+#define   VIDEO_DIP_PORT(port)		((port) << 29)
+#define   VIDEO_DIP_PORT_MASK		(3 << 29)
+#define   VIDEO_DIP_ENABLE_GCP		(1 << 25) /* ilk+ */
+#define   VIDEO_DIP_ENABLE_AVI		(1 << 21)
+#define   VIDEO_DIP_ENABLE_VENDOR	(2 << 21)
+#define   VIDEO_DIP_ENABLE_GAMUT	(4 << 21) /* ilk+ */
+#define   VIDEO_DIP_ENABLE_SPD		(8 << 21)
+#define   VIDEO_DIP_SELECT_AVI		(0 << 19)
+#define   VIDEO_DIP_SELECT_VENDOR	(1 << 19)
+#define   VIDEO_DIP_SELECT_GAMUT	(2 << 19)
+#define   VIDEO_DIP_SELECT_SPD		(3 << 19)
+#define   VIDEO_DIP_SELECT_MASK		(3 << 19)
+#define   VIDEO_DIP_FREQ_ONCE		(0 << 16)
+#define   VIDEO_DIP_FREQ_VSYNC		(1 << 16)
+#define   VIDEO_DIP_FREQ_2VSYNC		(2 << 16)
+#define   VIDEO_DIP_FREQ_MASK		(3 << 16)
+/* HSW and later: */
+#define   VIDEO_DIP_ENABLE_DRM_GLK	(1 << 28)
+#define   PSR_VSC_BIT_7_SET		(1 << 27)
+#define   VSC_SELECT_MASK		(0x3 << 25)
+#define   VSC_SELECT_SHIFT		25
+#define   VSC_DIP_HW_HEA_DATA		(0 << 25)
+#define   VSC_DIP_HW_HEA_SW_DATA	(1 << 25)
+#define   VSC_DIP_HW_DATA_SW_HEA	(2 << 25)
+#define   VSC_DIP_SW_HEA_DATA		(3 << 25)
+#define   VDIP_ENABLE_PPS		(1 << 24)
+#define   VIDEO_DIP_ENABLE_VSC_HSW	(1 << 20)
+#define   VIDEO_DIP_ENABLE_GCP_HSW	(1 << 16)
+#define   VIDEO_DIP_ENABLE_AVI_HSW	(1 << 12)
+#define   VIDEO_DIP_ENABLE_VS_HSW	(1 << 8)
+#define   VIDEO_DIP_ENABLE_GMP_HSW	(1 << 4)
+#define   VIDEO_DIP_ENABLE_SPD_HSW	(1 << 0)
+
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b1ca8e5bdb56..64666d9dfb49 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -208,7 +208,7 @@ static void g4x_write_infoframe(struct intel_encoder *encoder,
 {
 	const u32 *data = frame;
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	u32 val = I915_READ(VIDEO_DIP_CTL);
+	u32 val = intel_de_read(dev_priv, VIDEO_DIP_CTL);
 	int i;
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
@@ -218,22 +218,22 @@ static void g4x_write_infoframe(struct intel_encoder *encoder,
 
 	val &= ~g4x_infoframe_enable(type);
 
-	I915_WRITE(VIDEO_DIP_CTL, val);
+	intel_de_write(dev_priv, VIDEO_DIP_CTL, val);
 
 	for (i = 0; i < len; i += 4) {
-		I915_WRITE(VIDEO_DIP_DATA, *data);
+		intel_de_write(dev_priv, VIDEO_DIP_DATA, *data);
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
 	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
-		I915_WRITE(VIDEO_DIP_DATA, 0);
+		intel_de_write(dev_priv, VIDEO_DIP_DATA, 0);
 
 	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
-	I915_WRITE(VIDEO_DIP_CTL, val);
-	POSTING_READ(VIDEO_DIP_CTL);
+	intel_de_write(dev_priv, VIDEO_DIP_CTL, val);
+	intel_de_posting_read(dev_priv, VIDEO_DIP_CTL);
 }
 
 static void g4x_read_infoframe(struct intel_encoder *encoder,
@@ -245,22 +245,22 @@ static void g4x_read_infoframe(struct intel_encoder *encoder,
 	u32 val, *data = frame;
 	int i;
 
-	val = I915_READ(VIDEO_DIP_CTL);
+	val = intel_de_read(dev_priv, VIDEO_DIP_CTL);
 
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
 	val |= g4x_infoframe_index(type);
 
-	I915_WRITE(VIDEO_DIP_CTL, val);
+	intel_de_write(dev_priv, VIDEO_DIP_CTL, val);
 
 	for (i = 0; i < len; i += 4)
-		*data++ = I915_READ(VIDEO_DIP_DATA);
+		*data++ = intel_de_read(dev_priv, VIDEO_DIP_DATA);
 }
 
 static u32 g4x_infoframes_enabled(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	u32 val = I915_READ(VIDEO_DIP_CTL);
+	u32 val = intel_de_read(dev_priv, VIDEO_DIP_CTL);
 
 	if ((val & VIDEO_DIP_ENABLE) == 0)
 		return 0;
@@ -840,8 +840,8 @@ static void g4x_set_infoframes(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
-	i915_reg_t reg = VIDEO_DIP_CTL;
-	u32 val = I915_READ(reg);
+	intel_de_reg_t reg = VIDEO_DIP_CTL;
+	u32 val = intel_de_read(dev_priv, reg);
 	u32 port = VIDEO_DIP_PORT(encoder->port);
 
 	assert_hdmi_port_disabled(intel_hdmi);
@@ -867,8 +867,8 @@ static void g4x_set_infoframes(struct intel_encoder *encoder,
 		}
 		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
 			 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_SPD);
-		I915_WRITE(reg, val);
-		POSTING_READ(reg);
+		intel_de_write(dev_priv, reg, val);
+		intel_de_posting_read(dev_priv, reg);
 		return;
 	}
 
@@ -886,8 +886,8 @@ static void g4x_set_infoframes(struct intel_encoder *encoder,
 	val &= ~(VIDEO_DIP_ENABLE_AVI |
 		 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_SPD);
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_AVI,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d760830cfd7b..66f9a4e9c869 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4703,50 +4703,6 @@ enum {
 #define   LVDS_B0B3_POWER_DOWN		(0 << 2)
 #define   LVDS_B0B3_POWER_UP		(3 << 2)
 
-/* Video Data Island Packet control */
-#define VIDEO_DIP_DATA		_MMIO(0x61178)
-/* Read the description of VIDEO_DIP_DATA (before Haswell) or VIDEO_DIP_ECC
- * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each byte
- * of the infoframe structure specified by CEA-861. */
-#define   VIDEO_DIP_DATA_SIZE	32
-#define   VIDEO_DIP_VSC_DATA_SIZE	36
-#define   VIDEO_DIP_PPS_DATA_SIZE	132
-#define VIDEO_DIP_CTL		_MMIO(0x61170)
-/* Pre HSW: */
-#define   VIDEO_DIP_ENABLE		(1 << 31)
-#define   VIDEO_DIP_PORT(port)		((port) << 29)
-#define   VIDEO_DIP_PORT_MASK		(3 << 29)
-#define   VIDEO_DIP_ENABLE_GCP		(1 << 25) /* ilk+ */
-#define   VIDEO_DIP_ENABLE_AVI		(1 << 21)
-#define   VIDEO_DIP_ENABLE_VENDOR	(2 << 21)
-#define   VIDEO_DIP_ENABLE_GAMUT	(4 << 21) /* ilk+ */
-#define   VIDEO_DIP_ENABLE_SPD		(8 << 21)
-#define   VIDEO_DIP_SELECT_AVI		(0 << 19)
-#define   VIDEO_DIP_SELECT_VENDOR	(1 << 19)
-#define   VIDEO_DIP_SELECT_GAMUT	(2 << 19)
-#define   VIDEO_DIP_SELECT_SPD		(3 << 19)
-#define   VIDEO_DIP_SELECT_MASK		(3 << 19)
-#define   VIDEO_DIP_FREQ_ONCE		(0 << 16)
-#define   VIDEO_DIP_FREQ_VSYNC		(1 << 16)
-#define   VIDEO_DIP_FREQ_2VSYNC		(2 << 16)
-#define   VIDEO_DIP_FREQ_MASK		(3 << 16)
-/* HSW and later: */
-#define   VIDEO_DIP_ENABLE_DRM_GLK	(1 << 28)
-#define   PSR_VSC_BIT_7_SET		(1 << 27)
-#define   VSC_SELECT_MASK		(0x3 << 25)
-#define   VSC_SELECT_SHIFT		25
-#define   VSC_DIP_HW_HEA_DATA		(0 << 25)
-#define   VSC_DIP_HW_HEA_SW_DATA	(1 << 25)
-#define   VSC_DIP_HW_DATA_SW_HEA	(2 << 25)
-#define   VSC_DIP_SW_HEA_DATA		(3 << 25)
-#define   VDIP_ENABLE_PPS		(1 << 24)
-#define   VIDEO_DIP_ENABLE_VSC_HSW	(1 << 20)
-#define   VIDEO_DIP_ENABLE_GCP_HSW	(1 << 16)
-#define   VIDEO_DIP_ENABLE_AVI_HSW	(1 << 12)
-#define   VIDEO_DIP_ENABLE_VS_HSW	(1 << 8)
-#define   VIDEO_DIP_ENABLE_GMP_HSW	(1 << 4)
-#define   VIDEO_DIP_ENABLE_SPD_HSW	(1 << 0)
-
 /* Panel power sequencing */
 #define PPS_BASE			0x61200
 #define VLV_PPS_BASE			(VLV_DISPLAY_BASE + PPS_BASE)
-- 
2.22.0

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

✗ Fi.CI.CHECKPATCH: warning for Display uncore (rev3)

[Patchwork]

== Series Details ==

Series: Display uncore (rev3)
URL   : https://patchwork.freedesktop.org/series/61735/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a5d4c467f066 drm/i915: split out uncore_mmio_debug
b11140f433d2 drm/i915: introduce display_uncore
-:21: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#21: 
new file mode 100644

-:36: WARNING:NEW_TYPEDEFS: do not add new typedefs
#36: FILE: drivers/gpu/drm/i915/display/intel_display_reg.h:11:
+typedef struct {

total: 0 errors, 2 warnings, 0 checks, 169 lines checked
3dcca3c2b243 drm/i915: convert a couple of registers to _DE_MMIO
-:22: WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#22: FILE: drivers/gpu/drm/i915/display/intel_display_reg.h:26:
+ * of the infoframe structure specified by CEA-861. */

total: 0 errors, 1 warnings, 0 checks, 189 lines checked

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

✗ Fi.CI.SPARSE: warning for Display uncore (rev3)

[Patchwork]

== Series Details ==

Series: Display uncore (rev3)
URL   : https://patchwork.freedesktop.org/series/61735/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: split out uncore_mmio_debug
+drivers/gpu/drm/i915/intel_uncore.c:1231:1: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1231:1: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1231:1: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1231:1: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1232:1: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1232:1: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1232:1: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1232:1: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1233:1: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1233:1: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1233:1: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1233:1: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1296:1: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1297:1: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1298:1: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1322:1: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1322:1: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1322:1: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1323:1: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1323:1: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1323:1: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1324:1: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1324:1: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1324:1: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block

Commit: drm/i915: introduce display_uncore
Okay!

Commit: drm/i915: convert a couple of registers to _DE_MMIO
Okay!

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

✓ Fi.CI.BAT: success for Display uncore (rev3)

[Patchwork]

== Series Details ==

Series: Display uncore (rev3)
URL   : https://patchwork.freedesktop.org/series/61735/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6652 -> Patchwork_13910
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/

Known issues
------------

  Here are the changes found in Patchwork_13910 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload-no-display:
    - fi-bwr-2160:        [PASS][1] -> [INCOMPLETE][2] ([k.org#202361])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-bwr-2160/igt@i915_module_load@reload-no-display.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-bwr-2160/igt@i915_module_load@reload-no-display.html

  * igt@prime_vgem@basic-wait-default:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-icl-u3/igt@prime_vgem@basic-wait-default.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-icl-u3/igt@prime_vgem@basic-wait-default.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [INCOMPLETE][5] ([fdo#107718]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_module_load@reload-no-display:
    - fi-icl-u3:          [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8] +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-icl-u3/igt@i915_module_load@reload-no-display.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-icl-u3/igt@i915_module_load@reload-no-display.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6700k2:      [INCOMPLETE][9] ([fdo#107807]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_execlists:
    - {fi-icl-guc}:       [INCOMPLETE][11] ([fdo#107713]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-icl-guc/igt@i915_selftest@live_execlists.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-icl-guc/igt@i915_selftest@live_execlists.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       [WARN][13] ([fdo#109380]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [FAIL][15] ([fdo#109483]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - fi-kbl-7567u:       [SKIP][17] ([fdo#109271]) -> [PASS][18] +23 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#106350]: https://bugs.freedesktop.org/show_bug.cgi?id=106350
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [k.org#202361]: https://bugzilla.kernel.org/show_bug.cgi?id=202361


Participating hosts (55 -> 47)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6652 -> Patchwork_13910

  CI-20190529: 20190529
  CI_DRM_6652: 1df6c4c637347779c9175808f603b1574e2cddc9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5125: 35d81d01b1599b4bc4df0e09e25f6f531eed4f8a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13910: 3dcca3c2b2434a5119eea4c2a8eb1eff1e187fb1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3dcca3c2b243 drm/i915: convert a couple of registers to _DE_MMIO
b11140f433d2 drm/i915: introduce display_uncore
a5d4c467f066 drm/i915: split out uncore_mmio_debug

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Display uncore

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-08-08 02:44:20)
> I've been trying to identify MMIO ranges to clearly define what belongs
> to display_uncore to do a check on access, but there are lots of
> exceptions and differences across gens (with a few more coming with TGL),
> so I don't think that's a viable way. The alternative option implemented
> here is to differentiate the register by type, which should ensure we
> never mix them up, but at the cost of a more complex transition.

One thing we could try with this approach is to tag every _MMIO() as
either DE or GT and have the uncore accessors check the magic bits
before scrubbing them. (With enough magic macros to make it disappear

Huge task, but not insurmountable. The danger is that if we do this
piecemeal is that we may end up with two simultaneous accesses via the
separate uncore accessors. Hmm.

On thing though is that Jani may find the intel_de_write (or just
de_write, the frequency may be worth bending the naming rules) as being
palatable.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Display uncore

[Jani Nikula]

On Thu, 08 Aug 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Daniele Ceraolo Spurio (2019-08-08 02:44:20)
>> I've been trying to identify MMIO ranges to clearly define what belongs
>> to display_uncore to do a check on access, but there are lots of
>> exceptions and differences across gens (with a few more coming with TGL),
>> so I don't think that's a viable way. The alternative option implemented
>> here is to differentiate the register by type, which should ensure we
>> never mix them up, but at the cost of a more complex transition.
>
> One thing we could try with this approach is to tag every _MMIO() as
> either DE or GT and have the uncore accessors check the magic bits
> before scrubbing them. (With enough magic macros to make it disappear
>
> Huge task, but not insurmountable. The danger is that if we do this
> piecemeal is that we may end up with two simultaneous accesses via the
> separate uncore accessors. Hmm.

You mean something like this?

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b362ca0663a6..401490f79935 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -179,7 +179,8 @@
 #define REG_FIELD_GET(__mask, __val)	((u32)FIELD_GET(__mask, __val))
 
 typedef struct {
-	u32 reg;
+	u32 de:1;
+	u32 reg:31;
 } i915_reg_t;
 
 #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
---

bloat-o-meter tells me just that, with no other changes is +0.53%
increase. Perhaps still acceptable.

I think we could just add something like

#define _MMIO_DE(r) ((const i915_reg_t){ .de = 1, .reg = (r) })

and update i915_reg.h to use that as the first step, with no other
changes, and build on top of that. I don't think there should be large
scale changes outside of i915_reg.h required at all at first. The update
to move away from I915_READ and I915_WRITE could come afterwards and
piecemeal AFAICT.

> On thing though is that Jani may find the intel_de_write (or just
> de_write, the frequency may be worth bending the naming rules) as being
> palatable.

Indeed. Already intel_de_write(i915, ...) is so much better than
intel_uncore_write(&i915->uncore, ...).

Though... with de encoded in i915_reg_t, we could have intel_write(i915,
...) do the right thing based on .de. It could internally choose the
right uncore for intel_uncore_write(). Even if most non-de would
directly use the uncore versions.

BR,
Jani.


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

Re: [RFC 0/3] Display uncore

[Daniele Ceraolo Spurio]

On 8/8/19 6:58 AM, Jani Nikula wrote:
> On Thu, 08 Aug 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Quoting Daniele Ceraolo Spurio (2019-08-08 02:44:20)
>>> I've been trying to identify MMIO ranges to clearly define what belongs
>>> to display_uncore to do a check on access, but there are lots of
>>> exceptions and differences across gens (with a few more coming with TGL),
>>> so I don't think that's a viable way. The alternative option implemented
>>> here is to differentiate the register by type, which should ensure we
>>> never mix them up, but at the cost of a more complex transition.
>>
>> One thing we could try with this approach is to tag every _MMIO() as
>> either DE or GT and have the uncore accessors check the magic bits
>> before scrubbing them. (With enough magic macros to make it disappear
>>
>> Huge task, but not insurmountable. The danger is that if we do this
>> piecemeal is that we may end up with two simultaneous accesses via the
>> separate uncore accessors. Hmm.
> 
> You mean something like this?
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b362ca0663a6..401490f79935 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -179,7 +179,8 @@
>   #define REG_FIELD_GET(__mask, __val)	((u32)FIELD_GET(__mask, __val))
>   
>   typedef struct {
> -	u32 reg;
> +	u32 de:1;
> +	u32 reg:31;
>   } i915_reg_t;
>   
>   #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
> ---
> 
> bloat-o-meter tells me just that, with no other changes is +0.53%
> increase. Perhaps still acceptable.
> 
> I think we could just add something like
> 
> #define _MMIO_DE(r) ((const i915_reg_t){ .de = 1, .reg = (r) })
> 
> and update i915_reg.h to use that as the first step, with no other
> changes, and build on top of that. I don't think there should be large
> scale changes outside of i915_reg.h required at all at first. The update
> to move away from I915_READ and I915_WRITE could come afterwards and
> piecemeal AFAICT.
> 
>> On thing though is that Jani may find the intel_de_write (or just
>> de_write, the frequency may be worth bending the naming rules) as being
>> palatable.
> 
> Indeed. Already intel_de_write(i915, ...) is so much better than
> intel_uncore_write(&i915->uncore, ...).
> 
> Though... with de encoded in i915_reg_t, we could have intel_write(i915,
> ...) do the right thing based on .de. It could internally choose the
> right uncore for intel_uncore_write(). Even if most non-de would
> directly use the uncore versions.
> 

I'd prefer to avoid the implicit selection in the new functions, but, 
for compatibility during the transition, we could add the selection 
inside the old I915_READ/WRITE() calls. This way we'll be able to ensure 
that even the non yet converted accesses will go through the correct uncore.

Daniele

> BR,
> Jani.
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Display uncore

[Lucas De Marchi]

On Thu, Aug 08, 2019 at 09:47:56AM -0700, Daniele Ceraolo Spurio wrote:
>
>
>On 8/8/19 6:58 AM, Jani Nikula wrote:
>>On Thu, 08 Aug 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>>Quoting Daniele Ceraolo Spurio (2019-08-08 02:44:20)
>>>>I've been trying to identify MMIO ranges to clearly define what belongs
>>>>to display_uncore to do a check on access, but there are lots of
>>>>exceptions and differences across gens (with a few more coming with TGL),
>>>>so I don't think that's a viable way. The alternative option implemented
>>>>here is to differentiate the register by type, which should ensure we
>>>>never mix them up, but at the cost of a more complex transition.
>>>
>>>One thing we could try with this approach is to tag every _MMIO() as
>>>either DE or GT and have the uncore accessors check the magic bits
>>>before scrubbing them. (With enough magic macros to make it disappear
>>>
>>>Huge task, but not insurmountable. The danger is that if we do this
>>>piecemeal is that we may end up with two simultaneous accesses via the
>>>separate uncore accessors. Hmm.
>>
>>You mean something like this?
>>
>>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>index b362ca0663a6..401490f79935 100644
>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>@@ -179,7 +179,8 @@
>>  #define REG_FIELD_GET(__mask, __val)	((u32)FIELD_GET(__mask, __val))
>>  typedef struct {
>>-	u32 reg;
>>+	u32 de:1;
>>+	u32 reg:31;
>>  } i915_reg_t;
>>  #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
>>---
>>
>>bloat-o-meter tells me just that, with no other changes is +0.53%
>>increase. Perhaps still acceptable.
>>
>>I think we could just add something like
>>
>>#define _MMIO_DE(r) ((const i915_reg_t){ .de = 1, .reg = (r) })
>>
>>and update i915_reg.h to use that as the first step, with no other
>>changes, and build on top of that. I don't think there should be large
>>scale changes outside of i915_reg.h required at all at first. The update
>>to move away from I915_READ and I915_WRITE could come afterwards and
>>piecemeal AFAICT.
>>
>>>On thing though is that Jani may find the intel_de_write (or just
>>>de_write, the frequency may be worth bending the naming rules) as being
>>>palatable.
>>
>>Indeed. Already intel_de_write(i915, ...) is so much better than
>>intel_uncore_write(&i915->uncore, ...).
>>
>>Though... with de encoded in i915_reg_t, we could have intel_write(i915,
>>...) do the right thing based on .de. It could internally choose the
>>right uncore for intel_uncore_write(). Even if most non-de would
>>directly use the uncore versions.
>>
>
>I'd prefer to avoid the implicit selection in the new functions, but, 
>for compatibility during the transition, we could add the selection 
>inside the old I915_READ/WRITE() calls. This way we'll be able to 
>ensure that even the non yet converted accesses will go through the 
>correct uncore.

Yeah, I'm with you on this one. For new functions I think it's better to
be explicit.

Lucas De Marchi

>
>Daniele
>
>>BR,
>>Jani.
>>
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Display uncore

[Jani Nikula]

On Thu, 08 Aug 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Thu, Aug 08, 2019 at 09:47:56AM -0700, Daniele Ceraolo Spurio wrote:
>>
>>
>>On 8/8/19 6:58 AM, Jani Nikula wrote:
>>>On Thu, 08 Aug 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>>>Quoting Daniele Ceraolo Spurio (2019-08-08 02:44:20)
>>>>>I've been trying to identify MMIO ranges to clearly define what belongs
>>>>>to display_uncore to do a check on access, but there are lots of
>>>>>exceptions and differences across gens (with a few more coming with TGL),
>>>>>so I don't think that's a viable way. The alternative option implemented
>>>>>here is to differentiate the register by type, which should ensure we
>>>>>never mix them up, but at the cost of a more complex transition.
>>>>
>>>>One thing we could try with this approach is to tag every _MMIO() as
>>>>either DE or GT and have the uncore accessors check the magic bits
>>>>before scrubbing them. (With enough magic macros to make it disappear
>>>>
>>>>Huge task, but not insurmountable. The danger is that if we do this
>>>>piecemeal is that we may end up with two simultaneous accesses via the
>>>>separate uncore accessors. Hmm.
>>>
>>>You mean something like this?
>>>
>>>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>index b362ca0663a6..401490f79935 100644
>>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>>@@ -179,7 +179,8 @@
>>>  #define REG_FIELD_GET(__mask, __val)	((u32)FIELD_GET(__mask, __val))
>>>  typedef struct {
>>>-	u32 reg;
>>>+	u32 de:1;
>>>+	u32 reg:31;
>>>  } i915_reg_t;
>>>  #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
>>>---
>>>
>>>bloat-o-meter tells me just that, with no other changes is +0.53%
>>>increase. Perhaps still acceptable.
>>>
>>>I think we could just add something like
>>>
>>>#define _MMIO_DE(r) ((const i915_reg_t){ .de = 1, .reg = (r) })
>>>
>>>and update i915_reg.h to use that as the first step, with no other
>>>changes, and build on top of that. I don't think there should be large
>>>scale changes outside of i915_reg.h required at all at first. The update
>>>to move away from I915_READ and I915_WRITE could come afterwards and
>>>piecemeal AFAICT.
>>>
>>>>On thing though is that Jani may find the intel_de_write (or just
>>>>de_write, the frequency may be worth bending the naming rules) as being
>>>>palatable.
>>>
>>>Indeed. Already intel_de_write(i915, ...) is so much better than
>>>intel_uncore_write(&i915->uncore, ...).
>>>
>>>Though... with de encoded in i915_reg_t, we could have intel_write(i915,
>>>...) do the right thing based on .de. It could internally choose the
>>>right uncore for intel_uncore_write(). Even if most non-de would
>>>directly use the uncore versions.
>>>
>>
>>I'd prefer to avoid the implicit selection in the new functions, but, 
>>for compatibility during the transition, we could add the selection 
>>inside the old I915_READ/WRITE() calls. This way we'll be able to 
>>ensure that even the non yet converted accesses will go through the 
>>correct uncore.
>
> Yeah, I'm with you on this one. For new functions I think it's better to
> be explicit.

I suppose my question is, how is it implicit if it's explicitly
specified in the i915_reg_t?

BR,
Jani.


>
> Lucas De Marchi
>
>>
>>Daniele
>>
>>>BR,
>>>Jani.
>>>
>>>

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

✗ Fi.CI.IGT: failure for Display uncore (rev3)

[Patchwork]

== Series Details ==

Series: Display uncore (rev3)
URL   : https://patchwork.freedesktop.org/series/61735/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6652_full -> Patchwork_13910_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13910_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13910_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13910_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_schedule@preempt-queue-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb7/igt@gem_exec_schedule@preempt-queue-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb4/igt@gem_exec_schedule@preempt-queue-bsd.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [SKIP][3] ([fdo#109276]) -> [FAIL][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb7/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html

  
Known issues
------------

  Here are the changes found in Patchwork_13910_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#110841])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_schedule@fifo-bsd:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#111325])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb5/igt@gem_exec_schedule@fifo-bsd.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb4/igt@gem_exec_schedule@fifo-bsd.html

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#105363])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-glk8/igt@kms_flip@2x-flip-vs-expired-vblank.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([fdo#102887])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-glk2/igt@kms_flip@flip-vs-expired-vblank.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-glk5/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#105363])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-skl8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-snb:          [PASS][15] -> [INCOMPLETE][16] ([fdo#105411])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-snb6/igt@kms_flip@flip-vs-suspend.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-snb1/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103167]) +5 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][19] -> [DMESG-WARN][20] ([fdo#108566]) +3 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html
    - shard-apl:          [PASS][21] -> [DMESG-WARN][22] ([fdo#108566]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-apl6/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#108145] / [fdo#110403])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-c-tiling-y:
    - shard-kbl:          [PASS][25] -> [DMESG-WARN][26] ([fdo#105345])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-kbl4/igt@kms_plane_lowres@pipe-c-tiling-y.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-kbl2/igt@kms_plane_lowres@pipe-c-tiling-y.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109441]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb1/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109276]) +14 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb1/igt@prime_busy@hang-bsd2.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb6/igt@prime_busy@hang-bsd2.html

  
#### Possible fixes ####

  * igt@gem_exec_schedule@fifo-bsd1:
    - shard-iclb:         [SKIP][31] ([fdo#109276]) -> [PASS][32] +14 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb7/igt@gem_exec_schedule@fifo-bsd1.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb4/igt@gem_exec_schedule@fifo-bsd1.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][33] ([fdo#111325]) -> [PASS][34] +4 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb4/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb6/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_exec_schedule@semaphore-noskip:
    - shard-iclb:         [FAIL][35] ([fdo#110946]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb4/igt@gem_exec_schedule@semaphore-noskip.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb6/igt@gem_exec_schedule@semaphore-noskip.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][39] ([fdo#105363]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-skl10/igt@kms_flip@flip-vs-expired-vblank.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-skl5/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [FAIL][41] ([fdo#103167]) -> [PASS][42] +7 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][43] ([fdo#108145]) -> [PASS][44] +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [SKIP][45] ([fdo#109441]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb1/igt@kms_psr@psr2_primary_blt.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb2/igt@kms_psr@psr2_primary_blt.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][47] ([fdo#108566]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-apl7/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-apl4/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  * igt@perf@oa-exponents:
    - shard-glk:          [FAIL][49] ([fdo#105483]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-glk9/igt@perf@oa-exponents.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-glk4/igt@perf@oa-exponents.html

  
#### Warnings ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd2:
    - shard-iclb:         [FAIL][51] ([fdo#111327]) -> [SKIP][52] ([fdo#109276])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd2.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb3/igt@gem_ctx_shared@exec-single-timeline-bsd2.html

  * igt@gem_mocs_settings@mocs-isolation-bsd2:
    - shard-iclb:         [FAIL][53] ([fdo#111330]) -> [SKIP][54] ([fdo#109276])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6652/shard-iclb1/igt@gem_mocs_settings@mocs-isolation-bsd2.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/shard-iclb6/igt@gem_mocs_settings@mocs-isolation-bsd2.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#105345]: https://bugs.freedesktop.org/show_bug.cgi?id=105345
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#110946]: https://bugs.freedesktop.org/show_bug.cgi?id=110946
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111327]: https://bugs.freedesktop.org/show_bug.cgi?id=111327
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): pig-skl-6260u 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6652 -> Patchwork_13910

  CI-20190529: 20190529
  CI_DRM_6652: 1df6c4c637347779c9175808f603b1574e2cddc9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5125: 35d81d01b1599b4bc4df0e09e25f6f531eed4f8a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13910: 3dcca3c2b2434a5119eea4c2a8eb1eff1e187fb1 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13910/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Display uncore

[Jani Nikula]

On Wed, 07 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> I've been trying to identify MMIO ranges to clearly define what belongs
> to display_uncore to do a check on access, but there are lots of
> exceptions and differences across gens (with a few more coming with TGL),
> so I don't think that's a viable way. The alternative option implemented
> here is to differentiate the register by type, which should ensure we
> never mix them up, but at the cost of a more complex transition.
>
> Thoughts? I'm very open to (and I actually hope for) better ideas.

Has there been any progress in this front lately, or have I just missed
it?

BR,
Jani.

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

Re: [Intel-gfx] [RFC 0/3] Display uncore

[Jani Nikula]

On Wed, 07 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> I've been trying to identify MMIO ranges to clearly define what belongs
> to display_uncore to do a check on access, but there are lots of
> exceptions and differences across gens (with a few more coming with TGL),
> so I don't think that's a viable way. The alternative option implemented
> here is to differentiate the register by type, which should ensure we
> never mix them up, but at the cost of a more complex transition.
>
> Thoughts? I'm very open to (and I actually hope for) better ideas.

Has there been any progress in this front lately, or have I just missed
it?

BR,
Jani.

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

Re: [RFC 0/3] Display uncore

[Daniele Ceraolo Spurio]

On 10/29/19 2:23 AM, Jani Nikula wrote:
> On Wed, 07 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
>> I've been trying to identify MMIO ranges to clearly define what belongs
>> to display_uncore to do a check on access, but there are lots of
>> exceptions and differences across gens (with a few more coming with TGL),
>> so I don't think that's a viable way. The alternative option implemented
>> here is to differentiate the register by type, which should ensure we
>> never mix them up, but at the cost of a more complex transition.
>>
>> Thoughts? I'm very open to (and I actually hope for) better ideas.
> 
> Has there been any progress in this front lately, or have I just missed
> it?
> 

No progress on the ML. I've been locally trying on and off to break 
i915_reg.h in more manageable chunks to be able to more easily mark the 
display registers, but I keep finding special cases, especially around 
VLV/CHV. I'll try to prioritize this task more and get something out, at 
least as a RFC.

Daniele

> BR,
> Jani.
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 0/3] Display uncore

[Daniele Ceraolo Spurio]

On 10/29/19 2:23 AM, Jani Nikula wrote:
> On Wed, 07 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
>> I've been trying to identify MMIO ranges to clearly define what belongs
>> to display_uncore to do a check on access, but there are lots of
>> exceptions and differences across gens (with a few more coming with TGL),
>> so I don't think that's a viable way. The alternative option implemented
>> here is to differentiate the register by type, which should ensure we
>> never mix them up, but at the cost of a more complex transition.
>>
>> Thoughts? I'm very open to (and I actually hope for) better ideas.
> 
> Has there been any progress in this front lately, or have I just missed
> it?
> 

No progress on the ML. I've been locally trying on and off to break 
i915_reg.h in more manageable chunks to be able to more easily mark the 
display registers, but I keep finding special cases, especially around 
VLV/CHV. I'll try to prioritize this task more and get something out, at 
least as a RFC.

Daniele

> BR,
> Jani.
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] Display uncore

[Jani Nikula]

On Tue, 29 Oct 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> On 10/29/19 2:23 AM, Jani Nikula wrote:
>> On Wed, 07 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
>>> I've been trying to identify MMIO ranges to clearly define what belongs
>>> to display_uncore to do a check on access, but there are lots of
>>> exceptions and differences across gens (with a few more coming with TGL),
>>> so I don't think that's a viable way. The alternative option implemented
>>> here is to differentiate the register by type, which should ensure we
>>> never mix them up, but at the cost of a more complex transition.
>>>
>>> Thoughts? I'm very open to (and I actually hope for) better ideas.
>> 
>> Has there been any progress in this front lately, or have I just missed
>> it?
>> 
>
> No progress on the ML. I've been locally trying on and off to break 
> i915_reg.h in more manageable chunks to be able to more easily mark the 
> display registers, but I keep finding special cases, especially around 
> VLV/CHV. I'll try to prioritize this task more and get something out, at 
> least as a RFC.

Okay. As you saw, I decided to start moving forward with display uncore
helpers [1]. We'll need them anyway, and we can add the display checks
there afterwards. If anything, they should make your job easier, not
harder.

BR,
Jani.


[1] http://patchwork.freedesktop.org/patch/msgid/20191029105156.21658-1-jani.nikula@intel.com

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

Re: [Intel-gfx] [RFC 0/3] Display uncore

[Jani Nikula]

On Tue, 29 Oct 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> On 10/29/19 2:23 AM, Jani Nikula wrote:
>> On Wed, 07 Aug 2019, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
>>> I've been trying to identify MMIO ranges to clearly define what belongs
>>> to display_uncore to do a check on access, but there are lots of
>>> exceptions and differences across gens (with a few more coming with TGL),
>>> so I don't think that's a viable way. The alternative option implemented
>>> here is to differentiate the register by type, which should ensure we
>>> never mix them up, but at the cost of a more complex transition.
>>>
>>> Thoughts? I'm very open to (and I actually hope for) better ideas.
>> 
>> Has there been any progress in this front lately, or have I just missed
>> it?
>> 
>
> No progress on the ML. I've been locally trying on and off to break 
> i915_reg.h in more manageable chunks to be able to more easily mark the 
> display registers, but I keep finding special cases, especially around 
> VLV/CHV. I'll try to prioritize this task more and get something out, at 
> least as a RFC.

Okay. As you saw, I decided to start moving forward with display uncore
helpers [1]. We'll need them anyway, and we can add the display checks
there afterwards. If anything, they should make your job easier, not
harder.

BR,
Jani.


[1] http://patchwork.freedesktop.org/patch/msgid/20191029105156.21658-1-jani.nikula@intel.com

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