Re: [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized.

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Fri, 2015-09-25 at 13:52 +0300, Jani Nikula wrote:
> On Wed, 23 Sep 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > In case something goes wrong with power well initialization we were
> > calling
> > intel_prepare_ddi during boot while encoder list isnt't initilized.
> 
> Broken record, is this a regression, what is the regressing commit,
> or
> if this was always broken, which commit introduced the broken
> feature?

I believe it is broken since this call was introduced, but when
everything goes as expected it isn't executed.
Only in rare cases where power well initialization didn't go well and
post call is called during init it will trigger this case.
I don't believe this is something that a regular user of stables
platforms should worrie though... but it is always good to protect to
be on the safe side.

> 
> BR,
> Jani.
> 
> 
> > 
> > [    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]
> > 
> > So let's protect this case.
> > 
> > My first attempt was to remove the intel_prepare_ddi, but Daniel
> > had pointed out
> > this is really needed to restore those registers values. And Imre
> > pointed out
> > that this case was without the flag protection and this was
> > actually where things
> > were going bad. So I've just checked and this indeed solves my
> > issue.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 85c35fd..d194492 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -246,7 +246,8 @@ 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);
> > +		if (!dev_priv->power_domains.initializing)
> > +			intel_prepare_ddi(dev);
> >  		gen8_irq_power_well_post_enable(dev_priv, 1 <<
> > PIPE_A);
> >  	}
> >  }
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx