On 13/06/2019 14:59, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-06-13 14:35:24)
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
More removal of implicit dev_priv from using old mmio accessors.
Actually the top level function remains but is split into a part which
writes to i915 and part which operates on intel_gt in order to initialize
the hardware.
GuC and engines are the only odd ones out remaining.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 66 ++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e54cd30534dc..b6f450e782e7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1234,28 +1234,32 @@ static void init_unused_rings(struct intel_gt *gt)
}
}
-int i915_gem_init_hw(struct drm_i915_private *dev_priv)
+static int init_hw(struct intel_gt *gt)
{
+ struct drm_i915_private *i915 = gt->i915;
+ struct intel_uncore *uncore = gt->uncore;
int ret;
- dev_priv->gt.last_init_time = ktime_get();
+ gt->last_init_time = ktime_get();
/* Double layer security blanket, see i915_gem_init() */
- intel_uncore_forcewake_get(&dev_priv->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
- if (HAS_EDRAM(dev_priv) && INTEL_GEN(dev_priv) < 9)
- I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
+ if (HAS_EDRAM(i915) && INTEL_GEN(i915) < 9)
+ intel_uncore_rmw(uncore, HSW_IDICR, 0, IDIHASHMSK(0xf));
- if (IS_HASWELL(dev_priv))
- I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ?
- LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
+ if (IS_HASWELL(i915))
+ intel_uncore_write(uncore,
+ MI_PREDICATE_RESULT_2,
+ IS_HSW_GT3(i915) ?
+ LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
/* Apply the GT workarounds... */
- intel_gt_apply_workarounds(&dev_priv->gt);
+ intel_gt_apply_workarounds(gt);
Would it be worth moving the above mmio into workarounds? Whilst you are
doing some spring cleaning :)
To GT workarounds? Are the above two workarounds? Do they have an
official designation?
/* ...and determine whether they are sticking. */
- intel_gt_verify_workarounds(&dev_priv->gt, "init");
+ intel_gt_verify_workarounds(gt, "init");
- intel_gt_init_swizzling(&dev_priv->gt);
+ intel_gt_init_swizzling(gt);
/*
* At least 830 can leave some of the unused rings
@@ -1263,48 +1267,58 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
* will prevent c3 entry. Makes sure all unused rings
* are totally idle.
*/
- init_unused_rings(&dev_priv->gt);
-
- BUG_ON(!dev_priv->kernel_context);
- ret = i915_terminally_wedged(dev_priv);
- if (ret)
- goto out;
+ init_unused_rings(gt);
- ret = i915_ppgtt_init_hw(&dev_priv->gt);
+ ret = i915_ppgtt_init_hw(gt);
if (ret) {
DRM_ERROR("Enabling PPGTT failed (%d)\n", ret);
goto out;
}
- ret = intel_wopcm_init_hw(&dev_priv->wopcm);
+ ret = intel_wopcm_init_hw(&i915->wopcm);
if (ret) {
DRM_ERROR("Enabling WOPCM failed (%d)\n", ret);
goto out;
}
/* We can't enable contexts until all firmware is loaded */
- ret = intel_uc_init_hw(dev_priv);
+ ret = intel_uc_init_hw(i915);
Sorting out the uc layering is an ongoing task. I think it probably
means our init_hw needs splitting.
I think guc and huc could be made children of intel_gt so this could be
changed to take gt. It's a lot of code which I am not sure has much yet
to live so I opted not to touch it.
if (ret) {
DRM_ERROR("Enabling uc failed (%d)\n", ret);
goto out;
}
- intel_mocs_init_l3cc_table(&dev_priv->gt);
+ intel_mocs_init_l3cc_table(gt);
/* Only when the HW is re-initialised, can we replay the requests */
- ret = intel_engines_resume(dev_priv);
+ ret = intel_engines_resume(i915);
if (ret)
goto cleanup_uc;
- intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
- intel_engines_set_scheduler_caps(dev_priv);
return 0;
cleanup_uc:
- intel_uc_fini_hw(dev_priv);
+ intel_uc_fini_hw(i915);
out:
- intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
+
+ return ret;
+}
+
+int i915_gem_init_hw(struct drm_i915_private *i915)
Do we also start to recognise this as i915_init_hw()? This is the driver
talking to the intel_gt and friends, not the driver setting up the GEM
api.
Not sure. There are some GEM bits inside like wedged status and
scheduler caps. So it sounds passable to leave it like it is for now.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
