Re: [REGRESSION] Re: i915 driver crashes on T540p if docking station attached

Linus Torvalds
2015-07-30T05:18:18Z

[-- Attachment #1: Type: text/plain, Size: 1635 bytes --]

On Wed, Jul 29, 2015 at 6:39 PM, Theodore Ts'o <tytso@mit.edu> wrote:
>
> It's here: https://goo.gl/photos/xHjn2Z97JQEw6k2C9

You didn't catch enough of the code line to decode the code, but it's
early enough in drm_crtc_index() (just five bytes in) that it's almost
certainly the very first dereference, so it's almost guaranteed to be
that

crtc->dev

access as part of list_for_each_entry(), with crtc being NULL. And
yes, "->dev" is the very first field, so the offset is zero too (while
the "->mode_config" list access would not be at offset zero).

And it looks like it is called from drm_atomic_helper_check_modeset():
the reason it has a question mark in the backtrace is because the
fault happens before the stack frame has even been set up.

There are multiple calls to "drm_crtc_index()" from that function, I
can't tell which one it is. Looking at the code generation I get, I
think it's because update_connector_routing() gets inlined, and that
one does several calls. Most of them look like this:

if (connector->state->crtc) {
idx = drm_crtc_index(connector->state->crtc);

ie they check that the crtc is non-NULL, but that last one does not:

connector_state->best_encoder = new_encoder;
idx = drm_crtc_index(connector_state->crtc);

crtc_state = state->crtc_states[idx];
crtc_state->mode_changed = true;

and I suspect the fix might be something like the attached. Totally
untested. Ted?

This whole "atomic modeset" series has been one royal fuck-up, guys.
We've had too many of these kinds of crap issues.

Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 845 bytes --]

drivers/gpu/drm/drm_atomic_helper.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 5b59d5ad7d1c..aac212297b49 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -230,10 +230,12 @@ update_connector_routing(struct drm_atomic_state *state, int conn_idx)
}

connector_state->best_encoder = new_encoder;
- idx = drm_crtc_index(connector_state->crtc);
+ if (connector_state->crtc) {
+ idx = drm_crtc_index(connector_state->crtc);

- crtc_state = state->crtc_states[idx];
- crtc_state->mode_changed = true;
+ crtc_state = state->crtc_states[idx];
+ crtc_state->mode_changed = true;
+ }

DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d]\n",
connector->base.id,