Hi Andrzej,
[...]
-int intel_gt_probe_lmem(struct intel_gt *gt) +static int intel_gt_probe_lmem(struct intel_gt *gt) { struct drm_i915_private *i915 = gt->i915;
- unsigned int instance = gt->info.id; struct intel_memory_region *mem; int id; int err;
- id = INTEL_REGION_LMEM_0 + instance;
- if (drm_WARN_ON(&i915->drm, id >= INTEL_REGION_STOLEN_SMEM))
Do we need to check id correctness? wouldn't be enough to check it on initialization of gt->info.id. If yes, maybe (id > INTEL_REGION_LMEM_3) would be more readable, or (info.id < MAX_GT), up to you.
yes, it's indeed redundant. Also because if that 'if' was true it would be a bit more catastrophic than a simple warning. I will remove it completely.
[...]
- if (id) {
struct intel_uncore_mmio_debug *mmio_debug;struct intel_uncore *uncore;uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);if (!gt->uncore)return -ENOMEM;s/gt->uncore/uncore/
thanks!
[...]
+static void +intel_gt_tile_cleanup(struct intel_gt *gt) +{
- intel_uncore_cleanup_mmio(gt->uncore);
- if (gt->info.id) {
kfree(gt->uncore);kfree(gt);What about gt->uncore->debug ?
you don't want to leak anything? :)
will add it, nice catch! Thanks!
[...]
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 1c67ff735f18..144f989e4fef 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -320,9 +320,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv) intel_device_info_subplatform_init(dev_priv); intel_step_init(dev_priv);
- intel_gt_init_early(to_gt(dev_priv), dev_priv);
- /* All tiles share a single mmio_debug */
So why are we allocating mmio_debug in intel_gt_tile_setup ?
yes... this is a leftover from previous development cycles... I will remove the comment. Indeed this goes only to tile 0.
[...]
void intel_uncore_cleanup_mmio(struct intel_uncore *uncore) {
- struct pci_dev *pdev = to_pci_dev(uncore->i915->drm.dev);
- pci_iounmap(pdev, uncore->regs);
- if (uncore->regs)
iounmap(uncore->regs);'if' is not necessary, up to you.
will remove, thanks!
[...]
Thank you for the review! Andi