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.
If that is the case, I think this could also benefit from the !crtc checks the plane equivalent has.
Yup, otherwise helpers need to check for that. I'll fix both and all the nits you've spotted, too. -Daniel