Re: [PATCH 9/9] drm/atomic: Add encoder_mask to crtc_state.

[Daniel Vetter <daniel@ffwll.ch>]

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@linux.intel.com>
> >>>> ---
> >>>>  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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx