* [PATCH] drm/i915: Avoid divbyzero in modesetting
@ 2016-02-22 11:52 Tvrtko Ursulin
2016-02-22 12:17 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-02-22 13:09 ` [PATCH] " Maarten Lankhorst
0 siblings, 2 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2016-02-22 11:52 UTC (permalink / raw)
To: Intel-gfx
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;
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Avoid divbyzero in modesetting
2016-02-22 11:52 [PATCH] drm/i915: Avoid divbyzero in modesetting Tvrtko Ursulin
@ 2016-02-22 12:17 ` Patchwork
2016-02-22 13:09 ` [PATCH] " Maarten Lankhorst
1 sibling, 0 replies; 8+ messages in thread
From: Patchwork @ 2016-02-22 12:17 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Summary ==
Series 3677v1 drm/i915: Avoid divbyzero in modesetting
http://patchwork.freedesktop.org/api/1.0/series/3677/revisions/1/mbox/
Test gem_cs_prefetch:
Subgroup basic-default:
incomplete -> PASS (ilk-hp8440p)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> INCOMPLETE (snb-x220t)
Subgroup basic-flip-vs-modeset:
dmesg-warn -> INCOMPLETE (ilk-hp8440p) UNSTABLE
Test kms_force_connector_basic:
Subgroup force-load-detect:
dmesg-fail -> FAIL (snb-x220t)
fail -> DMESG-FAIL (snb-dellxps)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (skl-i5k-2) UNSTABLE
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (skl-i5k-2) UNSTABLE
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> DMESG-WARN (byt-nuc)
Subgroup basic-rte:
dmesg-warn -> PASS (bsw-nuc-2)
bdw-nuci7 total:165 pass:154 dwarn:0 dfail:0 fail:0 skip:11
bdw-ultra total:168 pass:154 dwarn:0 dfail:0 fail:0 skip:14
bsw-nuc-2 total:168 pass:137 dwarn:0 dfail:0 fail:1 skip:30
byt-nuc total:168 pass:142 dwarn:1 dfail:0 fail:0 skip:25
hsw-gt2 total:168 pass:157 dwarn:0 dfail:1 fail:0 skip:10
ilk-hp8440p total:80 pass:53 dwarn:0 dfail:0 fail:0 skip:26
ivb-t430s total:168 pass:153 dwarn:0 dfail:0 fail:1 skip:14
skl-i5k-2 total:168 pass:151 dwarn:1 dfail:0 fail:0 skip:16
snb-dellxps total:168 pass:145 dwarn:0 dfail:1 fail:0 skip:22
snb-x220t total:151 pass:129 dwarn:0 dfail:0 fail:2 skip:19
Results at /archive/results/CI_IGT_test/Patchwork_1453/
c278ff791cc73f90079c86f343be16214a0038b7 drm-intel-nightly: 2016y-02m-22d-09h-17m-11s UTC integration manifest
8f2f0140ab09500682dfd5afce0c81bd466b2ea9 drm/i915: Avoid divbyzero in modesetting
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Avoid divbyzero in modesetting
2016-02-22 11:52 [PATCH] drm/i915: Avoid divbyzero in modesetting Tvrtko Ursulin
2016-02-22 12:17 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-02-22 13:09 ` Maarten Lankhorst
2016-02-22 14:27 ` Tvrtko Ursulin
1 sibling, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2016-02-22 13:09 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
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..
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Avoid divbyzero in modesetting
2016-02-22 13:09 ` [PATCH] " Maarten Lankhorst
@ 2016-02-22 14:27 ` Tvrtko Ursulin
2016-02-22 14:56 ` Maarten Lankhorst
2016-02-23 8:38 ` Maarten Lankhorst
0 siblings, 2 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2016-02-22 14:27 UTC (permalink / raw)
To: Maarten Lankhorst, Intel-gfx
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Avoid divbyzero in modesetting
2016-02-22 14:27 ` Tvrtko Ursulin
@ 2016-02-22 14:56 ` Maarten Lankhorst
2016-02-22 15:22 ` Tvrtko Ursulin
2016-02-23 8:38 ` Maarten Lankhorst
1 sibling, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2016-02-22 14:56 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
Op 22-02-16 om 15:27 schreef Tvrtko Ursulin:
> 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
>
Uh oh, we didn't update the atomic state which confused the wm stuff.
Does the below work?
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index deee56010eee..d9e0470419a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6358,6 +6358,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
{
+ struct intel_encoder *encoder;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
enum intel_display_power_domain domain;
@@ -6378,6 +6379,19 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
dev_priv->display.crtc_disable(crtc);
intel_crtc->active = false;
intel_fbc_disable(intel_crtc);
+
+ DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was enabled, now disabled\n",
+ crtc->base.id);
+
+ WARN_ON(drm_atomic_set_mode_for_crtc(crtc->state, NULL) < 0);
+ crtc->state->active = false;
+ crtc->enabled = false;
+ crtc->state->connector_mask = 0;
+ crtc->state->encoder_mask = 0;
+
+ for_each_encoder_on_crtc(crtc->dev, crtc, encoder)
+ encoder->base.crtc = NULL;
+
intel_update_watermarks(crtc);
intel_disable_shared_dpll(intel_crtc);
@@ -15526,38 +15540,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
/* Adjust the state of the output pipe according to whether we
* have active connectors/encoders. */
- if (!intel_crtc_has_encoders(crtc))
+ if (crtc->active && !intel_crtc_has_encoders(crtc))
intel_crtc_disable_noatomic(&crtc->base);
- if (crtc->active != crtc->base.state->active) {
- struct intel_encoder *encoder;
-
- /* This can happen either due to bugs in the get_hw_state
- * functions or because of calls to intel_crtc_disable_noatomic,
- * or because the pipe is force-enabled due to the
- * pipe A quirk. */
- DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was %s, now %s\n",
- crtc->base.base.id,
- crtc->base.state->enable ? "enabled" : "disabled",
- crtc->active ? "enabled" : "disabled");
-
- WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) < 0);
- crtc->base.state->active = crtc->active;
- crtc->base.enabled = crtc->active;
- crtc->base.state->connector_mask = 0;
- crtc->base.state->encoder_mask = 0;
-
- /* Because we only establish the connector -> encoder ->
- * crtc links if something is active, this means the
- * crtc is now deactivated. Break the links. connector
- * -> encoder links are only establish when things are
- * actually up, hence no need to break them. */
- WARN_ON(crtc->active);
-
- for_each_encoder_on_crtc(dev, &crtc->base, encoder)
- encoder->base.crtc = NULL;
- }
-
if (crtc->active || HAS_GMCH_DISPLAY(dev)) {
/*
* We start out with underrun reporting disabled to avoid races.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Avoid divbyzero in modesetting
2016-02-22 14:56 ` Maarten Lankhorst
@ 2016-02-22 15:22 ` Tvrtko Ursulin
0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2016-02-22 15:22 UTC (permalink / raw)
To: Maarten Lankhorst, Intel-gfx
On 22/02/16 14:56, Maarten Lankhorst wrote:
> Op 22-02-16 om 15:27 schreef Tvrtko Ursulin:
>> 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
>>
> Uh oh, we didn't update the atomic state which confused the wm stuff.
>
> Does the below work?
Yep, with your patch it doesn't trigger my would-be-div-by-zero warning
any more.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Avoid divbyzero in modesetting
2016-02-22 14:27 ` Tvrtko Ursulin
2016-02-22 14:56 ` Maarten Lankhorst
@ 2016-02-23 8:38 ` Maarten Lankhorst
2016-02-23 9:49 ` Tvrtko Ursulin
1 sibling, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2016-02-23 8:38 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
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?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Avoid divbyzero in modesetting
2016-02-23 8:38 ` Maarten Lankhorst
@ 2016-02-23 9:49 ` Tvrtko Ursulin
0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2016-02-23 9:49 UTC (permalink / raw)
To: Maarten Lankhorst, Intel-gfx
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-23 9:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-22 11:52 [PATCH] drm/i915: Avoid divbyzero in modesetting Tvrtko Ursulin
2016-02-22 12:17 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-02-22 13:09 ` [PATCH] " Maarten Lankhorst
2016-02-22 14:27 ` Tvrtko Ursulin
2016-02-22 14:56 ` Maarten Lankhorst
2016-02-22 15:22 ` Tvrtko Ursulin
2016-02-23 8:38 ` Maarten Lankhorst
2016-02-23 9:49 ` Tvrtko Ursulin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).