On Tue, Nov 04, 2014 at 10:30:37PM +0100, Daniel Vetter wrote:
On Tue, Nov 04, 2014 at 03:31:07PM -0500, Sean Paul wrote:
On Sun, Nov 02, 2014 at 02:19:19PM +0100, Daniel Vetter wrote:
+drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
- struct drm_crtc *crtc)
+{
- struct drm_crtc_state *crtc_state;
- crtc_state = drm_atomic_get_crtc_state(conn_state->state, crtc);
- if (IS_ERR(crtc_state))
- return PTR_ERR(crtc_state);
- conn_state->crtc = crtc;
- DRM_DEBUG_KMS("Link connector state %p to [CRTC:%d]\n",
conn_state, crtc->base.id);- return 0;
+} +EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
In drm_atomic_helper.c, you're assigning directly to conn_state->crtc. I think you should probably be using this helper (as it doesn't seem to be used anywhere). I realize this happens in a different patch, but I want to make sure I don't miss it later :-)
Indeed this is a bug, and I didn't catch it because I couldn't test connector stealing. Scenario:
Connector A is active on CRTC 0.
Userspace does a legacy setCrtc with connector A and CRTC 1.
-> We should steal the connector and disable CRTC 0 (since it doesn't have any other connectors) completely. But because the helpers didn't pull in the state for CRTC this doesn't happen.
Actually there's a very good chance we'll get away: If we steal the connector we'll likely also steal its old encoder, and the encoder stealing already grabs the crtc on its own. Still, better safe than sorry. -Daniel