On 13/06/2019 17:30, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-06-13 17:11:43)
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?
To intel_gt_workarounds_apply, I'm sure you can find something ;)
Having looked up the docs for HSW_IDCR and MI_PREDICATE_RESULT_2,
neither of the programming looks like workarounds but normal device init
to me. As such I don't see how it would be appropriate to move them into
workarounds.
/* ...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.
I can go either way. There are a few hooks into e.g. i915_ggtt, but
mostly it is a different means of driving HW (by passing through to a
second driver of our HW, grumble) via a separate communication
channel. On the whole, I think it replaces gt/.
In passing, as we move i915_ggtt/i915_ppgtt beneath gt/, we should also
consider using the intel_ prefix. The goal is that these are the HW
interface for tracking the page tables with the i915_gem_context being
the API interface. (And if the GEM vm api grows more we should
introduce a i915_gem_vm/gtt wrapper around intel_ppgtt.)
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.
wedged is mostly our response to HW, and is controlled by
gt/intel_reset.c. But we also use to disable the GEM uAPI. However that
is just the API layer looking at the underlying HW state and saying "no can do".
Hopefully.
i915->gpu_error is definitely an interesting beast. I actually think it
doesn't belong inside i915->gt as it is a separate HW snapshot for
different API, but not actually a part of driving the HW.
The caps.sched are an interesting wart, we are summarising the gt
features and they change depending on how we mistreat gt (wedged).
I read this as leave as is for now.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
