Re: [PATCH 1/8] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

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

 



On 28/03/18 00:39, Paulo Zanoni wrote:
Em Ter, 2018-03-27 às 15:42 -0700, Paulo Zanoni escreveu:
Em Sex, 2018-03-23 às 16:28 +0000, Lionel Landwerlin escreveu:
Hi Mika,

Even after this series, we're still missing support for reading
the
timestamp frequency (read_timestamp_frequency in
intel_device_info.c).
I'm pretty sure someone wrote a patch for it. Do you any idea?

If not, I can send something.
Yes, we have them. I'll see if I missed them while upstreaming and
resend in that case.
https://patchwork.freedesktop.org/patch/196710/

Hey Lionel, the Reviewed-by stamp you gave on the patch was before we
upstreamed it, so we need you (or someone else) to re-check the patch
and re-issue the reviewed-by tag. We do this because of the rebasing
that happened between the R-B tag and the upstreaming, since issues can
be introduced in between. If you can check the patch again and validate
the r-b tag (or point the issues) then we can move forward and
hopefully merge it.

Thanks,
Paulo

Thanks, just sent another Rb, please push it :)


Thanks,

-
Lionel

On 16/03/18 12:14, Mika Kuoppala wrote:
From: Oscar Mateo <oscar.mateo@xxxxxxxxx>

In Gen11, the Video Decode engines (aka VDBOX, aka VCS, aka BSD)
and the
Video Enhancement engines (aka VEBOX, aka VECS) could be fused
off.
Also,
each VDBOX and VEBOX has its own power well, which only exist if
the related
engine exists in the HW.

Unfortunately, we have a Catch-22 situation going on: we need the
blitter
forcewake to read the register with the fuse info, but we cannot
initialize
the forcewake domains without knowin about the engines present in
the HW.
We workaround this problem by allowing the initialization of all
forcewake
domains and then pruning the fused off ones, as per the fuse
information.

Bspec: 20680

v2: We were shifting incorrectly for vebox disable (Vinay)

v3: Assert mmio is ready and warn if we have attempted to
initialize
      forcewake for fused-off engines (Paulo)

v4:
    - Use INTEL_GEN in new code (Tvrtko)
    - Shorter local variable (Tvrtko, Michal)
    - Keep "if (!...) continue" style (Tvrtko)
    - No unnecessary BUG_ON (Tvrtko)
    - WARN_ON and cleanup if wrong mask (Tvrtko, Michal)
    - Use I915_READ_FW (Michal)
    - Use I915_MAX_VCS/VECS macros (Michal)

v5: Rebased by Rodrigo fixing conflicts on top of:
      commit 33def1ff7b0 ("drm/i915: Simplify intel_engines_init")

v6: Fix v5. Remove info->num_rings. (by Oscar)

v7: Rebase (Rodrigo).

v8:
    -
s/intel_device_info_fused_off_engines/intel_device_info_init_mmio
(Chris)
    - Make vdbox_disable & vebox_disable local variables (Chris)

v9:
    - Move function declaration to intel_device_info.h (Michal)
    - Missing indent in bit fields definitions (Michal)
    - When RC6 is enabled by BIOS, the fuse register cannot be
read
until
      the blitter powerwell is awake. Shuffle where the fuse is
read, prune
      the forcewake domains after the fact and change the commit
message
      accordingly (Vinay, Sagar, Chris).

v10:
    - Improved commit message (Sagar)
    - New line in header file (Sagar)
    - Specify the message in fw_domain_reset applies to ICL+
(Sagar)

Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
Cc: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
---
   drivers/gpu/drm/i915/i915_drv.c          |  4 +++
   drivers/gpu/drm/i915/i915_reg.h          |  5 +++
   drivers/gpu/drm/i915/intel_device_info.c | 47
+++++++++++++++++++++++++++
   drivers/gpu/drm/i915/intel_device_info.h |  2 ++
   drivers/gpu/drm/i915/intel_uncore.c      | 56
++++++++++++++++++++++++++++++++
   drivers/gpu/drm/i915/intel_uncore.h      |  1 +
   6 files changed, 115 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 3df5193487f3..83df8e21cec0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1033,6 +1033,10 @@ static int i915_driver_init_mmio(struct
drm_i915_private *dev_priv)
intel_uncore_init(dev_priv); + intel_device_info_init_mmio(dev_priv);
+
+	intel_uncore_prune(dev_priv);
+
   	intel_uc_init_mmio(dev_priv);
ret = intel_engines_init_mmio(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index cf7c837d6a09..982e72e73e99 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2545,6 +2545,11 @@ enum i915_power_well_id {
   #define GEN10_EU_DISABLE3		_MMIO(0x9140)
   #define   GEN10_EU_DIS_SS_MASK		0xff
+#define GEN11_GT_VEBOX_VDBOX_DISABLE _MMIO(0x9140)
+#define   GEN11_GT_VDBOX_DISABLE_MASK	0xff
+#define   GEN11_GT_VEBOX_DISABLE_SHIFT	16
+#define   GEN11_GT_VEBOX_DISABLE_MASK	(0xff <<
GEN11_GT_VEBOX_DISABLE_SHIFT)
+
   #define GEN6_BSD_SLEEP_PSMI_CONTROL	_MMIO(0x12050)
   #define   GEN6_BSD_SLEEP_MSG_DISABLE	(1 << 0)
   #define   GEN6_BSD_SLEEP_FLUSH_DISABLE	(1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c
b/drivers/gpu/drm/i915/intel_device_info.c
index 3dd350f7b8e6..4babfc6ee45b 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -780,3 +780,50 @@ void intel_driver_caps_print(const struct
intel_driver_caps *caps,
   {
   	drm_printf(p, "scheduler: %x\n", caps->scheduler);
   }
+
+/*
+ * Determine which engines are fused off in our particular
hardware. Since the
+ * fuse register is in the blitter powerwell, we need forcewake
to
be ready at
+ * this point (but later we need to prune the forcewake domains
for engines that
+ * are indeed fused off).
+ */
+void intel_device_info_init_mmio(struct drm_i915_private
*dev_priv)
+{
+	struct intel_device_info *info =
mkwrite_device_info(dev_priv);
+	u8 vdbox_disable, vebox_disable;
+	u32 media_fuse;
+	int i;
+
+	if (INTEL_GEN(dev_priv) < 11)
+		return;
+
+	media_fuse = I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE);
+
+	vdbox_disable = media_fuse &
GEN11_GT_VDBOX_DISABLE_MASK;
+	vebox_disable = (media_fuse &
GEN11_GT_VEBOX_DISABLE_MASK)
+			GEN11_GT_VEBOX_DISABLE_SHIFT;
+
+	DRM_DEBUG_DRIVER("vdbox disable: %04x\n",
vdbox_disable);
+	for (i = 0; i < I915_MAX_VCS; i++) {
+		if (!HAS_ENGINE(dev_priv, _VCS(i)))
+			continue;
+
+		if (!(BIT(i) & vdbox_disable))
+			continue;
+
+		info->ring_mask &= ~ENGINE_MASK(_VCS(i));
+		DRM_DEBUG_DRIVER("vcs%u fused off\n", i);
+	}
+
+	DRM_DEBUG_DRIVER("vebox disable: %04x\n",
vebox_disable);
+	for (i = 0; i < I915_MAX_VECS; i++) {
+		if (!HAS_ENGINE(dev_priv, _VECS(i)))
+			continue;
+
+		if (!(BIT(i) & vebox_disable))
+			continue;
+
+		info->ring_mask &= ~ENGINE_MASK(_VECS(i));
+		DRM_DEBUG_DRIVER("vecs%u fused off\n", i);
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index 0835752c8b22..0cbb92223013 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -247,6 +247,8 @@ void intel_device_info_dump_runtime(const
struct intel_device_info *info,
   void intel_device_info_dump_topology(const struct sseu_dev_info
*sseu,
   				     struct drm_printer *p);
+void intel_device_info_init_mmio(struct drm_i915_private
*dev_priv);
+
   void intel_driver_caps_print(const struct intel_driver_caps
*caps,
   			     struct drm_printer *p);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 4df7c2ef8576..4c616d074a97 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -62,6 +62,11 @@ static inline void
   fw_domain_reset(struct drm_i915_private *i915,
   		const struct intel_uncore_forcewake_domain *d)
   {
+	/*
+	 * We don't really know if the powerwell for the
forcewake
domain we are
+	 * trying to reset here does exist at this point
(engines
could be fused
+	 * off in ICL+), so no waiting for acks
+	 */
   	__raw_i915_write32(i915, d->reg_set, i915-
uncore.fw_reset);
   }
@@ -1353,6 +1358,23 @@ static void fw_domain_init(struct
drm_i915_private *dev_priv,
   	fw_domain_reset(dev_priv, d);
   }
+static void fw_domain_fini(struct drm_i915_private *dev_priv,
+			   enum forcewake_domain_id domain_id)
+{
+	struct intel_uncore_forcewake_domain *d;
+
+	if (WARN_ON(domain_id >= FW_DOMAIN_ID_COUNT))
+		return;
+
+	d = &dev_priv->uncore.fw_domain[domain_id];
+
+	WARN_ON(d->wake_count);
+	WARN_ON(hrtimer_cancel(&d->timer));
+	memset(d, 0, sizeof(*d));
+
+	dev_priv->uncore.fw_domains &= ~BIT(domain_id);
+}
+
   static void intel_uncore_fw_domains_init(struct
drm_i915_private
*dev_priv)
   {
   	if (INTEL_GEN(dev_priv) <= 5 ||
intel_vgpu_active(dev_priv))
@@ -1565,6 +1587,40 @@ void intel_uncore_init(struct
drm_i915_private *dev_priv)
   		&dev_priv->uncore.pmic_bus_access_nb);
   }
+/*
+ * We might have detected that some engines are fused off after
we
initialized
+ * the forcewake domains. Prune them, to make sure they only
reference existing
+ * engines.
+ */
+void intel_uncore_prune(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_GEN(dev_priv) >= 11) {
+		enum forcewake_domains fw_domains = dev_priv-
uncore.fw_domains;
+		enum forcewake_domain_id domain_id;
+		int i;
+
+		for (i = 0; i < I915_MAX_VCS; i++) {
+			domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 +
i;
+
+			if (HAS_ENGINE(dev_priv, _VCS(i)))
+				continue;
+
+			if (fw_domains & BIT(domain_id))
+				fw_domain_fini(dev_priv,
domain_id);
+		}
+
+		for (i = 0; i < I915_MAX_VECS; i++) {
+			domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 +
i;
+
+			if (HAS_ENGINE(dev_priv, _VECS(i)))
+				continue;
+
+			if (fw_domains & BIT(domain_id))
+				fw_domain_fini(dev_priv,
domain_id);
+		}
+	}
+}
+
   void intel_uncore_fini(struct drm_i915_private *dev_priv)
   {
   	/* Paranoia: make sure we have disabled everything
before
we exit. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.h
b/drivers/gpu/drm/i915/intel_uncore.h
index dfdf444e4bcc..47478d609630 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -140,6 +140,7 @@ struct intel_uncore {
void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
   void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_prune(struct drm_i915_private *dev_priv);
   bool intel_uncore_unclaimed_mmio(struct drm_i915_private
*dev_priv);
   bool intel_uncore_arm_unclaimed_mmio_detection(struct
drm_i915_private *dev_priv);
   void intel_uncore_fini(struct drm_i915_private *dev_priv);

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


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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux