On Mon, Dec 14, 2015 at 01:06:12PM +0100, Maarten Lankhorst wrote: > Op 04-12-15 om 09:12 schreef Daniel Vetter: > > On Thu, Dec 03, 2015 at 12:01:02PM +0100, Maarten Lankhorst wrote: > >> Op 03-12-15 om 10:53 schreef Daniel Vetter: > >>> On Tue, Nov 24, 2015 at 10:34:36AM +0100, Maarten Lankhorst wrote: > >>>> This allows iteration over encoders without requiring connection_mutex. > >>>> > >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > >>>> --- > >>>> drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > >>>> drivers/gpu/drm/i915/intel_display.c | 3 +++ > >>>> include/drm/drm_crtc.h | 2 ++ > >>>> 3 files changed, 9 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > >>>> index fb79c54b2aed..f3fd8f131f92 100644 > >>>> --- a/drivers/gpu/drm/drm_atomic_helper.c > >>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c > >>>> @@ -269,6 +269,8 @@ mode_fixup(struct drm_atomic_state *state) > >>>> continue; > >>>> > >>>> drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode); > >>>> + > >>>> + crtc_state->encoder_mask = 0; > >>> Hm, I think a small function to set the best_encoder (like we do to set > >>> the crtc for connector or planes) would be good. Otherwise we'll frob > >>> around the code and forget this, and much confusion will ensue. > >>> > >>> That helper should probably be in core drm_atomic.c, like the other > >>> set_foo_for_bar helpers. > >> As always only i915 assigns best_encoder outside drm_atomic_helper_check_modeset, and the i915 calls can't be fixed because of hw readout. :( > >> At the time of mode_fixup all encoders should have been updated, so I'm not sure adding a helper for best_encoder would help much.. > > I've meant just for the atomic helpers. i915 is a mess, yes, but that's > > not really an excuse to not make shared code pretty ;-) > > > It's not really possible to do it in a helper. The encoder might be > moved with the connector, or have a fixed mapping depending on crtc. > (i915 MST) > > So unfortunately there can be no generic helper, but it has to be dealt > with in this function, when assigning best_encoder per crtc. I meant a drm_atomic_set_best_encoder function, which sets both best_encoder and updates the encoder_mask for the crtc. Why would that not work? Of course actually figuring out what the best_encoder is would not be done by that function, it would only update the book-keeping. And then we could reuse it in our state reconstruction code too. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel