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

[Linus Torvalds]

On Wed, Jul 29, 2015 at 6:39 PM, Theodore Ts'o <tytso@xxxxxxx> 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
 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,