[Intel-gfx] [PATCH v3 1/2] drm/i915/gtt: Do not initialize drm_mm twice.
Paulo Zanoni
przanoni at gmail.com
Wed Sep 23 11:55:20 PDT 2015
2015-09-23 13:48 GMT-03:00 Imre Deak <imre.deak at intel.com>:
> On ke, 2015-09-16 at 11:49 +0200, Michał Winiarski wrote:
>> It would be initialized just moments later by i915_init_vm.
>>
>> v2: Commit msg update,
>> s/i915_init_vm/i915_address_space_init, move to i915_gem_gtt.c,
>> init address_space during i915_gem_setup_global_gtt for ggtt.
>> v3: Do not init global_link - we are adding it to vm_list moments later,
>> make i915_address_space_init static, use OOP style parameter order.
>>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Michel Thierry <michel.thierry at intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
>> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
>
> With this patch I get the oops below when booting. Looking at it it's
> caused by dev_priv->gtt being initialized now too late for
> i915_gem_object_create_stolen_for_preallocated().
My BDW machine stopped booting, and git bisect tells me this patch is
the cause. I didn't setup anything to help me grab the logs before the
crash, so I can't confirm the backtrace below.
Something interesting is that if I boot with i915.modeset=0, then
later "rmmod i915; modprobe i915 modeset=1", the driver loads
successfully.
>
> [ 13.547091] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 13.555857] IP: [<ffffffffa0243f09>] i915_gem_object_create_stolen_for_preallocated+0x1e9/0x320 [i915]
> [ 13.566380] PGD 0
> [ 13.568634] Oops: 0002 [#1] PREEMPT SMP
> [ 13.573035] Modules linked in: snd_hda_codec_hdmi snd_hda_intel i915(O+) snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_seq_dummy snd_seq_oss kvm_intel i2c_algo_bit snd_seq_midi drm_kms_helper kvm snd_rawmidi syscopyarea sysfillrect sysimgblt snd_seq_midi_event crc32c_intel fb_sys_fops microcode snd_seq drm snd_seq_device snd_timer efivars intel_gtt snd agpgart fuse
> [ 13.609814] CPU: 1 PID: 329 Comm: systemd-udevd Tainted: G IO 4.3.0-rc2-bxt+ #177
> [ 13.619401] Hardware name: Intel Corp. BROXTON A1 PLATFORM/TABLET, BIOS BXTM_IFWI_X64_R_2015_36_4_00 09/02/2015
> [ 13.630681] task: ffff88007674cc80 ti: ffff880075c1c000 task.ti: ffff880075c1c000
> [ 13.639049] RIP: 0010:[<ffffffffa0243f09>] [<ffffffffa0243f09>] i915_gem_object_create_stolen_for_preallocated+0x1e9/0x320 [i915]
> [ 13.652219] RSP: 0018:ffff880075c1f810 EFLAGS: 00010286
> [ 13.658154] RAX: 0000000000000000 RBX: 00000000008ca000 RCX: ffff880072e61ca8
> [ 13.666129] RDX: ffff880072e61c00 RSI: ffff88006de4a0c8 RDI: ffff88007674d4c8
> [ 13.674103] RBP: ffff880075c1f848 R08: 00000000000196f0 R09: ffff880072e61c00
> [ 13.682078] R10: 0000000000000001 R11: 0000000000000001 R12: ffff88006de49f18
> [ 13.690053] R13: ffff88006df00000 R14: ffff8800754154e0 R15: 0000000000000000
> [ 13.698034] FS: 00007f4d61c0f880(0000) GS:ffff880079480000(0000) knlGS:0000000000000000
> [ 13.707089] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 13.713513] CR2: 0000000000000000 CR3: 0000000073304000 CR4: 00000000003406e0
> [ 13.721494] Stack:
> [ 13.723740] ffff88006de4a048 ffff88006de49e18 ffff880075c1f928 ffff880075c1f858
> [ 13.732044] ffff880072cba540 ffff880076061000 ffff880076061000 ffff880075c1f8e8
> [ 13.740345] ffffffffa027bdbc 0000000000000000 0000000000000000
> [ 13.745411] sdhci-pci 0000:00:1c.0: power state changed by ACPI to D3cold
> [ 13.754373] 0000000000000000
>
> [ 13.757903] Call Trace:
> [ 13.760709] [<ffffffffa027bdbc>] intel_alloc_initial_plane_obj.isra.56+0x7c/0x190 [i915]
> [ 13.769930] [<ffffffffa0287654>] intel_modeset_init+0xbb4/0x1a10 [i915]
> [ 13.777495] [<ffffffffa02c03fc>] i915_driver_load+0xeec/0x14e0 [i915]
> [ 13.784831] [<ffffffffa0069569>] drm_dev_register+0xa9/0xc0 [drm]
> [ 13.791762] [<ffffffffa006c37d>] drm_get_pci_dev+0x8d/0x1e0 [drm]
> [ 13.798682] [<ffffffff818c7e42>] ? _raw_spin_unlock_irqrestore+0x42/0x70
> [ 13.806321] [<ffffffffa0201234>] i915_pci_probe+0x34/0x50 [i915]
> [ 13.813144] [<ffffffff81452cbc>] pci_device_probe+0x8c/0x100
> [ 13.819577] [<ffffffff8152a5a9>] driver_probe_device+0x169/0x4a0
> [ 13.826393] [<ffffffff8152a968>] __driver_attach+0x88/0x90
> [ 13.832628] [<ffffffff8152a8e0>] ? driver_probe_device+0x4a0/0x4a0
> [ 13.839643] [<ffffffff81528126>] bus_for_each_dev+0x66/0xa0
> [ 13.845976] [<ffffffff81529f5e>] driver_attach+0x1e/0x20
> [ 13.852012] [<ffffffff815299b4>] bus_add_driver+0x1f4/0x290
> [ 13.858343] [<ffffffff8152b540>] driver_register+0x60/0xe0
> [ 13.864577] [<ffffffff814513b0>] __pci_register_driver+0x60/0x70
> [ 13.871410] [<ffffffffa006c5b0>] drm_pci_init+0xe0/0x110 [drm]
> [ 13.878034] [<ffffffff810d024d>] ? trace_hardirqs_on+0xd/0x10
> [ 13.884560] [<ffffffffa032b000>] ? 0xffffffffa032b000
> [ 13.890350] [<ffffffffa032b094>] i915_init+0x94/0x9b [i915]
> [ 13.896685] [<ffffffff810002f3>] do_one_initcall+0xb3/0x1d0
> [ 13.903016] [<ffffffff810ed246>] ? rcu_read_lock_sched_held+0x86/0x90
> [ 13.910323] [<ffffffff811e21f1>] ? kmem_cache_alloc_trace+0x1c1/0x270
> [ 13.917626] [<ffffffff81192893>] ? do_init_module+0x27/0x1ea
> [ 13.924052] [<ffffffff811928cc>] do_init_module+0x60/0x1ea
> [ 13.930288] [<ffffffff81114b02>] load_module+0x2012/0x2580
> [ 13.936521] [<ffffffff811109a0>] ? store_uevent+0x40/0x40
> [ 13.942659] [<ffffffff81111246>] ? copy_module_from_fd.isra.36+0xf6/0x140
> [ 13.950350] [<ffffffff81115270>] SyS_finit_module+0x80/0xb0
> [ 13.956681] [<ffffffff818c86db>] entry_SYSCALL_64_fastpath+0x16/0x73
>
>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 2 --
>> drivers/gpu/drm/i915/i915_gem.c | 14 --------------
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 26 ++++++++++++++++++--------
>> 3 files changed, 18 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 3bf8a9b..039227d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2804,8 +2804,6 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
>> size_t size);
>> struct drm_i915_gem_object *i915_gem_object_create_from_data(
>> struct drm_device *dev, const void *data, size_t size);
>> -void i915_init_vm(struct drm_i915_private *dev_priv,
>> - struct i915_address_space *vm);
>> void i915_gem_free_object(struct drm_gem_object *obj);
>> void i915_gem_vma_destroy(struct i915_vma *vma);
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index cb0df7e..4811f8a 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4844,18 +4844,6 @@ init_ring_lists(struct intel_engine_cs *ring)
>> INIT_LIST_HEAD(&ring->request_list);
>> }
>>
>> -void i915_init_vm(struct drm_i915_private *dev_priv,
>> - struct i915_address_space *vm)
>> -{
>> - if (!i915_is_ggtt(vm))
>> - drm_mm_init(&vm->mm, vm->start, vm->total);
>> - vm->dev = dev_priv->dev;
>> - INIT_LIST_HEAD(&vm->active_list);
>> - INIT_LIST_HEAD(&vm->inactive_list);
>> - INIT_LIST_HEAD(&vm->global_link);
>> - list_add_tail(&vm->global_link, &dev_priv->vm_list);
>> -}
>> -
>> void
>> i915_gem_load(struct drm_device *dev)
>> {
>> @@ -4879,8 +4867,6 @@ i915_gem_load(struct drm_device *dev)
>> NULL);
>>
>> INIT_LIST_HEAD(&dev_priv->vm_list);
>> - i915_init_vm(dev_priv, &dev_priv->gtt.base);
>> -
>> INIT_LIST_HEAD(&dev_priv->context_list);
>> INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
>> INIT_LIST_HEAD(&dev_priv->mm.bound_list);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 8786281..01f3521 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -2121,6 +2121,16 @@ static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>> return gen8_ppgtt_init(ppgtt);
>> }
>>
>> +static void i915_address_space_init(struct i915_address_space *vm,
>> + struct drm_i915_private *dev_priv)
>> +{
>> + drm_mm_init(&vm->mm, vm->start, vm->total);
>> + vm->dev = dev_priv->dev;
>> + INIT_LIST_HEAD(&vm->active_list);
>> + INIT_LIST_HEAD(&vm->inactive_list);
>> + list_add_tail(&vm->global_link, &dev_priv->vm_list);
>> +}
>> +
>> int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -2129,9 +2139,7 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>> ret = __hw_ppgtt_init(dev, ppgtt);
>> if (ret == 0) {
>> kref_init(&ppgtt->ref);
>> - drm_mm_init(&ppgtt->base.mm, ppgtt->base.start,
>> - ppgtt->base.total);
>> - i915_init_vm(dev_priv, &ppgtt->base);
>> + i915_address_space_init(&ppgtt->base, dev_priv);
>> }
>>
>> return ret;
>> @@ -2618,11 +2626,13 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>>
>> BUG_ON(mappable_end > end);
>>
>> - /* Subtract the guard page ... */
>> - drm_mm_init(&ggtt_vm->mm, start, end - start - PAGE_SIZE);
>> + ggtt_vm->start = start;
>>
>> - dev_priv->gtt.base.start = start;
>> - dev_priv->gtt.base.total = end - start;
>> + /* Subtract the guard page before address space initialization to
>> + * shrink the range used by drm_mm */
>> + ggtt_vm->total = end - start - PAGE_SIZE;
>> + i915_address_space_init(ggtt_vm, dev_priv);
>> + ggtt_vm->total += PAGE_SIZE;
>>
>> if (intel_vgpu_active(dev)) {
>> ret = intel_vgt_balloon(dev);
>> @@ -2631,7 +2641,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>> }
>>
>> if (!HAS_LLC(dev))
>> - dev_priv->gtt.base.mm.color_adjust = i915_gtt_color_adjust;
>> + ggtt_vm->mm.color_adjust = i915_gtt_color_adjust;
>>
>> /* Mark any preallocated objects as occupied */
>> list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
More information about the Intel-gfx
mailing list