| Message ID | 1449218769-16577-23-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 04, 2015 at 09:46:03AM +0100, Daniel Vetter wrote: > This can happen when we run out of encoders for a multi-crtc modeset, > or also when userspace is silly and tries to clone multiple connectors > that need the same encoder on the same crtc. > > Reported-and-Tested-and-Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 2cf8ab7dbc8c..ab275499d2a3 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -86,6 +86,27 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, > } > } > > +static bool > +check_pending_encoder_assignment(struct drm_atomic_state *state, > + struct drm_encoder *new_encoder, > + struct drm_connector *new_connector) new_connector seems to be unused. > +{ > + struct drm_connector *connector; > + struct drm_connector_state *conn_state; > + int i; > + > + for_each_connector_in_state(state, connector, conn_state, i) { > + if (conn_state->best_encoder != new_encoder) > + continue; > + > + /* encoder already assigned and we're trying to re-steal it! */ > + if (connector->state->best_encoder != conn_state->best_encoder) Was this supposed to be new_connector->state->best_encoder? Thierry
On Mon, Dec 07, 2015 at 02:26:04PM +0100, Thierry Reding wrote: > On Fri, Dec 04, 2015 at 09:46:03AM +0100, Daniel Vetter wrote: > > This can happen when we run out of encoders for a multi-crtc modeset, > > or also when userspace is silly and tries to clone multiple connectors > > that need the same encoder on the same crtc. > > > > Reported-and-Tested-and-Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Oops that patch shouldn't have been in here. And it's already merged - somehow I botched the rebase before sending out this series and it didn't drop out. I'll address your feedback with a follow-up patch. > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index 2cf8ab7dbc8c..ab275499d2a3 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -86,6 +86,27 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, > > } > > } > > > > +static bool > > +check_pending_encoder_assignment(struct drm_atomic_state *state, > > + struct drm_encoder *new_encoder, > > + struct drm_connector *new_connector) > > new_connector seems to be unused. > > > +{ > > + struct drm_connector *connector; > > + struct drm_connector_state *conn_state; > > + int i; > > + > > + for_each_connector_in_state(state, connector, conn_state, i) { > > + if (conn_state->best_encoder != new_encoder) > > + continue; > > + > > + /* encoder already assigned and we're trying to re-steal it! */ > > + if (connector->state->best_encoder != conn_state->best_encoder) > > Was this supposed to be new_connector->state->best_encoder? Nah, new_connector was just leftovers from an earlier, broken version of this. The function really only checks whether the given encoder has alreaddy been reassigned in this update. That's enough to decide that there's a conflict and reject the modeset. I'll do a patch to drop the superflous argument. -Daniel
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2cf8ab7dbc8c..ab275499d2a3 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -86,6 +86,27 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, } } +static bool +check_pending_encoder_assignment(struct drm_atomic_state *state, + struct drm_encoder *new_encoder, + struct drm_connector *new_connector) +{ + struct drm_connector *connector; + struct drm_connector_state *conn_state; + int i; + + for_each_connector_in_state(state, connector, conn_state, i) { + if (conn_state->best_encoder != new_encoder) + continue; + + /* encoder already assigned and we're trying to re-steal it! */ + if (connector->state->best_encoder != conn_state->best_encoder) + return false; + } + + return true; +} + static struct drm_crtc * get_current_crtc_for_encoder(struct drm_device *dev, struct drm_encoder *encoder) @@ -235,6 +256,13 @@ update_connector_routing(struct drm_atomic_state *state, int conn_idx) return 0; } + if (!check_pending_encoder_assignment(state, new_encoder, connector)) { + DRM_DEBUG_ATOMIC("Encoder for [CONNECTOR:%d:%s] already assigned\n", + connector->base.id, + connector->name); + return -EINVAL; + } + encoder_crtc = get_current_crtc_for_encoder(state->dev, new_encoder);
This can happen when we run out of encoders for a multi-crtc modeset, or also when userspace is silly and tries to clone multiple connectors that need the same encoder on the same crtc. Reported-and-Tested-and-Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_atomic_helper.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)