Re: [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization.

Vivi, Rodrigo
2015-09-23T17:34:22Z

here goes one trace...
To be honest I'm confused as well: looking to the code that initializing boolean should be protecting this case...
And if I remove this first one it will end up on that other at post enable...

But apparently this only happens when something didn't go well with power well initialization...

so, besides the initialization and resume from s3 this path here happens on every wake up from DC5/DC6?

[ 9.618747] i915 0000:00:02.0: Invalid ROM contents
[ 9.631446] [drm] failed to find VBIOS tables
[ 9.720036] BUG: unable to handle kernel NULL pointer dereference at 00000000
00000058
[ 9.721986] IP: [<ffffffffa014eb72>] ddi_get_encoder_port+0x82/0x190 [i915]
[ 9.723736] PGD 0
[ 9.724286] Oops: 0000 [#1] PREEMPT SMP
[ 9.725386] Modules linked in: intel_powerclamp snd_hda_intel(+) coretemp crc
32c_intel snd_hda_codec snd_hda_core serio_raw snd_pcm snd_timer i915(+) parport
_pc parport pinctrl_sunrisepoint pinctrl_intel nfsd nfs_acl
[ 9.730635] CPU: 0 PID: 497 Comm: systemd-udevd Not tainted 4.3.0-rc2-eywa-10
967-g72de2cfd-dirty #2
[ 9.732785] Hardware name: Intel Corporation Cannonlake Client platform/Skyla
ke DT DDR4 RVP8, BIOS CNLSE2R1.R00.X021.B00.1508040310 08/04/2015
[ 9.735785] task: ffff88008a704700 ti: ffff88016a1ac000 task.ti: ffff88016a1a
c000
[ 9.737584] RIP: 0010:[<ffffffffa014eb72>] [<ffffffffa014eb72>] ddi_get_enco
der_port+0x82/0x190 [i915]
[ 9.739934] RSP: 0000:ffff88016a1af710 EFLAGS: 00010296
[ 9.741184] RAX: 000000000000004e RBX: ffff88008a9edc98 RCX: 0000000000000001
[ 9.742934] RDX: 000000000000004e RSI: ffffffff81fc1e82 RDI: 00000000ffffffff
[ 9.744634] RBP: ffff88016a1af730 R08: 0000000000000000 R09: 0000000000000578
[ 9.746333] R10: 0000000000001065 R11: 0000000000000578 R12: fffffffffffffff8
[ 9.748033] R13: ffff88016a1af7a8 R14: ffff88016a1af794 R15: 0000000000000000
[ 9.749733] FS: 00007eff2e1e07c0(0000) GS:ffff88016fc00000(0000) knlGS:00000
00000000000
[ 9.751683] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 9.753083] CR2: 0000000000000058 CR3: 000000016922b000 CR4: 00000000003406f0
[ 9.754782] Stack:
[ 9.755332] ffff88008a9edc98 ffff88008a9ed800 ffffffffa01d07b0 00000000fffb9
09e
[ 9.757232] ffff88016a1af7d8 ffffffffa0154ea7 0000000000000246 ffff88016a370
080
[ 9.759182] ffff88016a370080 ffff88008a9ed800 0000000000000246 ffff88008a9ed
c98
[ 9.761132] Call Trace:
[ 9.761782] [<ffffffffa0154ea7>] intel_prepare_ddi+0x67/0x860 [i915]
[ 9.763332] [<ffffffff81a56996>] ? _raw_spin_unlock_irqrestore+0x26/0x40
[ 9.765031] [<ffffffffa00fad01>] ? gen9_read32+0x141/0x360 [i915]
[ 9.766531] [<ffffffffa00b43e1>] skl_set_power_well+0x431/0xa80 [i915]
[ 9.768181] [<ffffffffa00b4a63>] skl_power_well_enable+0x13/0x20 [i915]
[ 9.769781] [<ffffffffa00b2188>] intel_power_well_enable+0x28/0x50 [i915]
[ 9.771481] [<ffffffffa00b4d52>] intel_display_power_get+0x92/0xc0 [i915]
[ 9.773180] [<ffffffffa00b4fcb>] intel_display_set_init_power+0x3b/0x40 [i91
5]
[ 9.774980] [<ffffffffa00b5170>] intel_power_domains_init_hw+0x120/0x520 [i9
15]
[ 9.776780] [<ffffffffa0194c61>] i915_driver_load+0xb21/0xf40 [i915]

________________________________________
From: Daniel Vetter [daniel.vetter@ffwll.ch] on behalf of Daniel Vetter [daniel@ffwll.ch]
Sent: Wednesday, September 23, 2015 8:58 AM
To: Vivi, Rodrigo
Cc: intel-gfx@lists.freedesktop.org; Shah, Suketu J
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization.

On Tue, Sep 22, 2015 at 08:03:59AM -0700, Rodrigo Vivi wrote:
> The comment removed along with the calls explains why they shouldn't be here:
>
> /* DDI buffer programming unnecessary during driver-load/resume
> * as it's already done during modeset initialization then.
> * It's also invalid here as encoder list is still uninitialized.
> */
>
> And the protection is not working well causing issues during the boot where
> power well initialization doesn't go as expected.
>
> So this call during modeset initialization where the encoder list is yet
> unitilized causes a NULL dereference breaking i915.
>
> While we don't find the right protection and we don't understand why this is
> actually needed I believe it is safe to just remove these calls.
>
> Cc: Suketu Shah <suketu.j.shah@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

But if we shut down the power well the hw will forget these values. Where
are they programmed now? Which commit broke this, how does this blow up?
Please add more details, atm I have no idea what to do with this patch but
be confused about it ...
-Daniel

> ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 85c35fd..f7027ea 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -246,7 +246,6 @@ static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
> }
>
> if (power_well->data == SKL_DISP_PW_1) {
> - intel_prepare_ddi(dev);
> gen8_irq_power_well_post_enable(dev_priv, 1 << PIPE_A);
> }
> }
> @@ -632,13 +631,6 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
> power_well->data == SKL_DISP_PW_2) {
> if (SKL_ENABLE_DC6(dev)) {
> skl_disable_dc6(dev_priv);
> - /*
> - * DDI buffer programming unnecessary during driver-load/resume
> - * as it's already done during modeset initialization then.
> - * It's also invalid here as encoder list is still uninitialized.
> - */
> - if (!dev_priv->power_domains.initializing)
> - intel_prepare_ddi(dev);
> } else {
> gen9_disable_dc5(dev_priv);
> }
> --
> 2.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation