* [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization. @ 2015-09-22 15:03 Rodrigo Vivi 2015-09-23 15:58 ` Daniel Vetter 0 siblings, 1 reply; 10+ messages in thread From: Rodrigo Vivi @ 2015-09-22 15:03 UTC (permalink / raw) To: intel-gfx; +Cc: Suketu Shah, Rodrigo Vivi 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> --- 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization. 2015-09-22 15:03 [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization Rodrigo Vivi @ 2015-09-23 15:58 ` Daniel Vetter 2015-09-23 17:34 ` Vivi, Rodrigo 0 siblings, 1 reply; 10+ messages in thread From: Daniel Vetter @ 2015-09-23 15:58 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: Suketu Shah, intel-gfx 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 http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization. 2015-09-23 15:58 ` Daniel Vetter @ 2015-09-23 17:34 ` Vivi, Rodrigo 2015-09-23 17:48 ` Imre Deak 0 siblings, 1 reply; 10+ messages in thread From: Vivi, Rodrigo @ 2015-09-23 17:34 UTC (permalink / raw) To: Daniel Vetter; +Cc: Shah, Suketu J, intel-gfx@lists.freedesktop.org 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 http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization. 2015-09-23 17:34 ` Vivi, Rodrigo @ 2015-09-23 17:48 ` Imre Deak 2015-09-23 18:32 ` [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized Rodrigo Vivi 0 siblings, 1 reply; 10+ messages in thread From: Imre Deak @ 2015-09-23 17:48 UTC (permalink / raw) To: Vivi, Rodrigo; +Cc: Shah, Suketu J, intel-gfx@lists.freedesktop.org On ke, 2015-09-23 at 17:34 +0000, Vivi, Rodrigo wrote: > 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... It is the one in skl_power_well_post_enable(), since that doesn't check for the flag. > > 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 > http://blog.ffwll.ch > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized. 2015-09-23 17:48 ` Imre Deak @ 2015-09-23 18:32 ` Rodrigo Vivi 2015-09-24 13:08 ` Imre Deak 2015-09-25 10:52 ` Jani Nikula 0 siblings, 2 replies; 10+ messages in thread From: Rodrigo Vivi @ 2015-09-23 18:32 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi In case something goes wrong with power well initialization we were calling intel_prepare_ddi during boot while encoder list isnt't initilized. [ 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized. 2015-09-23 18:32 ` [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized Rodrigo Vivi @ 2015-09-24 13:08 ` Imre Deak 2015-09-25 10:52 ` Jani Nikula 1 sibling, 0 replies; 10+ messages in thread From: Imre Deak @ 2015-09-24 13:08 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: Daniel Vetter, intel-gfx On ke, 2015-09-23 at 11:32 -0700, Rodrigo Vivi wrote: > In case something goes wrong with power well initialization we were calling > intel_prepare_ddi during boot while encoder list isnt't initilized. > > [ 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> I think this is a good solution for now, so: Reviewed-by: Imre Deak <imre.deak@intel.com> About removing the DDI initialization from here: At the moment DDI is initialized on all DDI platforms whenever we toggle on/off some power well, or exit some power saving state. It's also reinitialized for (e)DP where we have to change the settings during link training. I'm not sure if we really need to initialize it before modesetting at all though, I can't see why DDI functionality would be needed before that. If it's not needed then the ideal would be to remove it from every power well/power state handling path and to make sure we call it during modesetting in all cases. > --- > 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); > } > } _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized. 2015-09-23 18:32 ` [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized Rodrigo Vivi 2015-09-24 13:08 ` Imre Deak @ 2015-09-25 10:52 ` Jani Nikula 2015-09-25 16:09 ` Vivi, Rodrigo 1 sibling, 1 reply; 10+ messages in thread From: Jani Nikula @ 2015-09-25 10:52 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi 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? 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 -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized. 2015-09-25 10:52 ` Jani Nikula @ 2015-09-25 16:09 ` Vivi, Rodrigo 2015-09-28 8:11 ` Jani Nikula 2015-09-28 8:40 ` Daniel Vetter 0 siblings, 2 replies; 10+ messages in thread From: Vivi, Rodrigo @ 2015-09-25 16:09 UTC (permalink / raw) To: intel-gfx@lists.freedesktop.org, jani.nikula@linux.intel.com Cc: daniel.vetter@ffwll.ch 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized. 2015-09-25 16:09 ` Vivi, Rodrigo @ 2015-09-28 8:11 ` Jani Nikula 2015-09-28 8:40 ` Daniel Vetter 1 sibling, 0 replies; 10+ messages in thread From: Jani Nikula @ 2015-09-28 8:11 UTC (permalink / raw) To: Vivi, Rodrigo, intel-gfx@lists.freedesktop.org; +Cc: daniel.vetter@ffwll.ch On Fri, 25 Sep 2015, "Vivi, Rodrigo" <rodrigo.vivi@intel.com> wrote: > 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. Rodrigo, both Daniel and I asked which commit broke things. We have to keep asking this again and again, like a broken record. It doesn't scale that we have to dig the logs for every patch that looks like a fix, just to decide where to queueu the patch. This was the one: commit 1d2b9526a790d55b7ae870934a74937081f62de2 Author: Damien Lespiau <damien.lespiau@intel.com> Date: Fri Mar 6 18:50:53 2015 +0000 drm/i915/skl: Restore the DDI translation tables when enabling PW1 And the patch, amended with the reference, is now pushed to drm-intel-fixes. Thanks for the patch and review. BR, Jani. > >> >> 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 >> -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized. 2015-09-25 16:09 ` Vivi, Rodrigo 2015-09-28 8:11 ` Jani Nikula @ 2015-09-28 8:40 ` Daniel Vetter 1 sibling, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2015-09-28 8:40 UTC (permalink / raw) To: Vivi, Rodrigo; +Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org On Fri, Sep 25, 2015 at 04:09:56PM +0000, Vivi, Rodrigo wrote: > 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. You posted a backtrace of the kernel dying in an oops. That looks pretty serious ;-) But even if this has been broken since forever in skl we only need to apply it to 4.3-fixes. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-09-28 8:37 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-22 15:03 [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization Rodrigo Vivi 2015-09-23 15:58 ` Daniel Vetter 2015-09-23 17:34 ` Vivi, Rodrigo 2015-09-23 17:48 ` Imre Deak 2015-09-23 18:32 ` [PATCH] drm/i915: Don't call intel_prepare_ddi when encoder list isn't yet initialized Rodrigo Vivi 2015-09-24 13:08 ` Imre Deak 2015-09-25 10:52 ` Jani Nikula 2015-09-25 16:09 ` Vivi, Rodrigo 2015-09-28 8:11 ` Jani Nikula 2015-09-28 8:40 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox