Re: [PATCH 22/28] drm/atomic-helper: Reject attempts at re-stealing encoders

[Daniel Vetter <daniel@ffwll.ch>]

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx