Message ID | 1456141948-7172-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hey, Op 22-02-16 om 12:52 schreef Tvrtko Ursulin: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Not sure if intel_wm_config->num_pipes_active is supposed to > ever be zero when intel_update_watermarks gets called. But > since it can be triggered in early platform bringup perhaps > we want to guard against it rather than divide by zero. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index feb57598727a..2b7998889617 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2831,8 +2831,13 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev, > nth_active_pipe++; > } > > - pipe_size = ddb_size / config->num_pipes_active; > - alloc->start = nth_active_pipe * ddb_size / config->num_pipes_active; > + if (WARN_ON(!config->num_pipes_active)) { > + pipe_size = 0; > + alloc->start = 0; > + } else { > + pipe_size = ddb_size / config->num_pipes_active; > + alloc->start = nth_active_pipe * ddb_size / config->num_pipes_active; > + } > alloc->end = alloc->start + pipe_size; > } > How can this happen? It seems in that case cstate->base.active would be false for the current pipe, and the code should bail early already..
On 22/02/16 13:09, Maarten Lankhorst wrote: > Hey, > > Op 22-02-16 om 12:52 schreef Tvrtko Ursulin: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Not sure if intel_wm_config->num_pipes_active is supposed to >> ever be zero when intel_update_watermarks gets called. But >> since it can be triggered in early platform bringup perhaps >> we want to guard against it rather than divide by zero. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Matt Roper <matthew.d.roper@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Daniel Vetter <daniel@ffwll.ch> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index feb57598727a..2b7998889617 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -2831,8 +2831,13 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev, >> nth_active_pipe++; >> } >> >> - pipe_size = ddb_size / config->num_pipes_active; >> - alloc->start = nth_active_pipe * ddb_size / config->num_pipes_active; >> + if (WARN_ON(!config->num_pipes_active)) { >> + pipe_size = 0; >> + alloc->start = 0; >> + } else { >> + pipe_size = ddb_size / config->num_pipes_active; >> + alloc->start = nth_active_pipe * ddb_size / config->num_pipes_active; >> + } >> alloc->end = alloc->start + pipe_size; >> } >> > How can this happen? It seems in that case cstate->base.active would be false for the current pipe, > and the code should bail early already.. Don't know, but does it harm to guard against it? Helps early platform bringup a bit. [drm:intel_modeset_readout_hw_state] [CRTC:21] hw state readout: enabled [drm:intel_modeset_readout_hw_state] [CRTC:26] hw state readout: disabled [drm:intel_modeset_readout_hw_state] [CRTC:31] hw state readout: disabled [drm:intel_modeset_readout_hw_state] PORT PLL A hw state readout: crtc_mask 0x00000001, on 0 [drm:intel_modeset_readout_hw_state] PORT PLL B hw state readout: crtc_mask 0x00000000, on 0 [drm:intel_modeset_readout_hw_state] PORT PLL C hw state readout: crtc_mask 0x00000000, on 0 [drm:intel_modeset_readout_hw_state] [ENCODER:33:TMDS-33] hw state readout: disabled, pipe A [drm:intel_modeset_readout_hw_state] [ENCODER:35:DP MST-35] hw state readout: disabled, pipe A [drm:intel_modeset_readout_hw_state] [ENCODER:36:DP MST-36] hw state readout: disabled, pipe B [drm:intel_modeset_readout_hw_state] [ENCODER:37:DP MST-37] hw state readout: disabled, pipe C [drm:intel_modeset_readout_hw_state] [ENCODER:42:TMDS-42] hw state readout: disabled, pipe A [drm:intel_modeset_readout_hw_state] [ENCODER:44:DP MST-44] hw state readout: disabled, pipe A [drm:intel_modeset_readout_hw_state] [ENCODER:45:DP MST-45] hw state readout: disabled, pipe B [drm:intel_modeset_readout_hw_state] [ENCODER:46:DP MST-46] hw state readout: disabled, pipe C [drm:intel_modeset_readout_hw_state] [CONNECTOR:34:DP-1] hw state readout: disabled [drm:intel_modeset_readout_hw_state] [CONNECTOR:40:HDMI-A-1] hw state readout: disabled [drm:intel_modeset_readout_hw_state] [CONNECTOR:43:DP-2] hw state readout: disabled [drm:intel_modeset_readout_hw_state] [CONNECTOR:47:HDMI-A-2] hw state readout: disabled [drm:drm_calc_timestamping_constants [drm]] *ERROR* crtc 21: Can't calculate constants, dotclock = 0! [drm:i915_get_vblank_timestamp] crtc 0 is disabled [drm:i915_get_vblank_timestamp] crtc 0 is disabled [drm:i915_get_vblank_timestamp] crtc 0 is disabled [drm:i915_get_vblank_timestamp] crtc 0 is disabled [drm:i915_get_vblank_timestamp] crtc 0 is disabled [drm:i915_get_vblank_timestamp] crtc 0 is disabled [drm:intel_disable_pipe] disabling pipe A ... [ 10.062881] [<ffffffffa016add2>] skl_update_pipe_wm+0x102/0x8c0 [i915] [ 10.062942] [<ffffffffa016b96f>] skl_update_wm+0xff/0x5f0 [i915] [ 10.063027] [<ffffffffa016ce6e>] intel_update_watermarks+0x1e/0x30 [i915] [ 10.063087] [<ffffffffa01d3ee2>] intel_crtc_disable_noatomic+0xd2/0x150 [i915] [ 10.063148] [<ffffffffa01dd3d2>] intel_modeset_setup_hw_state+0xdd2/0xde0 [i915] [ 10.063181] [<ffffffffa01dfd83>] intel_modeset_init+0x15a3/0x1950 [i915] [ 10.063181] [<ffffffffa02160b6>] i915_driver_load+0x13c6/0x1720 [i915] Regards, Tvrtko
Op 22-02-16 om 15:27 schreef Tvrtko Ursulin: > [ 10.062881] [<ffffffffa016add2>] skl_update_pipe_wm+0x102/0x8c0 [i915] > [ 10.062942] [<ffffffffa016b96f>] skl_update_wm+0xff/0x5f0 [i915] > [ 10.063027] [<ffffffffa016ce6e>] intel_update_watermarks+0x1e/0x30 [i915] > [ 10.063087] [<ffffffffa01d3ee2>] intel_crtc_disable_noatomic+0xd2/0x150 [i915] > [ 10.063148] [<ffffffffa01dd3d2>] intel_modeset_setup_hw_state+0xdd2/0xde0 [i915] > [ 10.063181] [<ffffffffa01dfd83>] intel_modeset_init+0x15a3/0x1950 [i915] > [ 10.063181] [<ffffffffa02160b6>] i915_driver_load+0x13c6/0x1720 [i915] Do you have a full oops I can use for a commit description?
On 23/02/16 08:38, Maarten Lankhorst wrote: > Op 22-02-16 om 15:27 schreef Tvrtko Ursulin: >> [ 10.062881] [<ffffffffa016add2>] skl_update_pipe_wm+0x102/0x8c0 [i915] >> [ 10.062942] [<ffffffffa016b96f>] skl_update_wm+0xff/0x5f0 [i915] >> [ 10.063027] [<ffffffffa016ce6e>] intel_update_watermarks+0x1e/0x30 [i915] >> [ 10.063087] [<ffffffffa01d3ee2>] intel_crtc_disable_noatomic+0xd2/0x150 [i915] >> [ 10.063148] [<ffffffffa01dd3d2>] intel_modeset_setup_hw_state+0xdd2/0xde0 [i915] >> [ 10.063181] [<ffffffffa01dfd83>] intel_modeset_init+0x15a3/0x1950 [i915] >> [ 10.063181] [<ffffffffa02160b6>] i915_driver_load+0x13c6/0x1720 [i915] > Do you have a full oops I can use for a commit description? Sure: ------------[ cut here ]------------ WARNING: CPU: 1 PID: 295 at drivers/gpu/drm/i915/intel_pm.c:2834 skl_update_pipe_wm+0x102/0x8c0 [i915]() WARN_ON(!config->num_pipes_active) Modules linked in: coretemp i915(+) xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx CPU: 1 PID: 295 Comm: systemd-udevd Tainted: G U W 4.5.0-rc4-xxxxxx #25 Hardware name: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx 0000000000000000 ffff88003777f5a8 ffffffff813485c2 ffff88003777f5f0 ffffffffa0236240 ffff88003777f5e0 ffffffff81050fce ffff8800aa420000 ffff8800aba18000 ffff8800aba18000 ffff880037304c00 ffff8800aa420000 Call Trace: [<ffffffff813485c2>] dump_stack+0x67/0x95 [<ffffffff81050fce>] warn_slowpath_common+0x9e/0xc0 [<ffffffff8105103c>] warn_slowpath_fmt+0x4c/0x50 [<ffffffff8106945e>] ? flush_work+0x8e/0x280 [<ffffffff810693d5>] ? flush_work+0x5/0x280 [<ffffffffa016add2>] skl_update_pipe_wm+0x102/0x8c0 [i915] [<ffffffffa016b96f>] skl_update_wm+0xff/0x5f0 [i915] [<ffffffff810928ee>] ? trace_hardirqs_on_caller+0x15e/0x1d0 [<ffffffff8109296d>] ? trace_hardirqs_on+0xd/0x10 [<ffffffffa016ce6e>] intel_update_watermarks+0x1e/0x30 [i915] [<ffffffffa01d3ee2>] intel_crtc_disable_noatomic+0xd2/0x150 [i915] [<ffffffffa01dd3d2>] intel_modeset_setup_hw_state+0xdd2/0xde0 [i915] [<ffffffffa01dfd83>] intel_modeset_init+0x15a3/0x1950 [i915] [<ffffffffa02160b6>] i915_driver_load+0x13c6/0x1720 [i915] [<ffffffff81522160>] ? add_sysfs_fw_map_entry+0x9b/0x9b [<ffffffffa00b15ef>] drm_dev_register+0x6f/0xb0 [drm] [<ffffffffa00b3b3a>] drm_get_pci_dev+0x10a/0x1d0 [drm] [<ffffffffa01582d9>] i915_pci_probe+0x49/0x50 [i915] [<ffffffff8138ae30>] pci_device_probe+0x80/0xf0 [<ffffffff8143e2ac>] driver_probe_device+0x1bc/0x3d0 [<ffffffff8143e526>] __driver_attach+0x66/0x90 [<ffffffff8143e4c0>] ? driver_probe_device+0x3d0/0x3d0 [<ffffffff8143be3b>] bus_for_each_dev+0x5b/0xa0 [<ffffffff8143db3e>] driver_attach+0x1e/0x20 [<ffffffff8143d461>] bus_add_driver+0x151/0x270 [<ffffffff8143eabc>] driver_register+0x8c/0xd0 [<ffffffff8138a2ed>] __pci_register_driver+0x5d/0x60 [<ffffffffa00b3c58>] drm_pci_init+0x58/0xf0 [drm] [<ffffffff8109296d>] ? trace_hardirqs_on+0xd/0x10 [<ffffffffa02aa000>] ? 0xffffffffa02aa000 [<ffffffffa02aa094>] i915_init+0x94/0x9b [i915] [<ffffffff81000423>] do_one_initcall+0x113/0x1f0 [<ffffffff810a4b21>] ? rcu_read_lock_sched_held+0x61/0x90 [<ffffffff811601dc>] ? kmem_cache_alloc_trace+0x1cc/0x280 [<ffffffff8111110a>] do_init_module+0x60/0x1c8 [<ffffffff810c731b>] load_module+0x1ceb/0x2410 [<ffffffff810c3a60>] ? store_uevent+0x40/0x40 [<ffffffff811763d1>] ? kernel_read+0x41/0x60 [<ffffffff810c7c1d>] SYSC_finit_module+0x8d/0xa0 [<ffffffff810c7c4e>] SyS_finit_module+0xe/0x10 [<ffffffff815f1e97>] entry_SYSCALL_64_fastpath+0x12/0x6f ---[ end trace 1149e9ab3695a423 ]--- ------------[ cut here ]------------ Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index feb57598727a..2b7998889617 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2831,8 +2831,13 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev, nth_active_pipe++; } - pipe_size = ddb_size / config->num_pipes_active; - alloc->start = nth_active_pipe * ddb_size / config->num_pipes_active; + if (WARN_ON(!config->num_pipes_active)) { + pipe_size = 0; + alloc->start = 0; + } else { + pipe_size = ddb_size / config->num_pipes_active; + alloc->start = nth_active_pipe * ddb_size / config->num_pipes_active; + } alloc->end = alloc->start + pipe_size; }