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

Daniel Vetter
2015-12-03T09:53:39Z

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.
-Daniel

> }
>
> for_each_connector_in_state(state, connector, conn_state, i) {
> @@ -288,6 +290,8 @@ mode_fixup(struct drm_atomic_state *state)
> * it away), so we won't call ->mode_fixup twice.
> */
> encoder = conn_state->best_encoder;
> + crtc_state->encoder_mask |= 1 << drm_encoder_index(encoder);
> +
> funcs = encoder->helper_private;
> if (!funcs)
> continue;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 21b1984e828b..3317db3806a8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15310,6 +15310,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
> crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
> crtc->base.state->connector_mask = 0;
> + crtc->base.state->encoder_mask = 0;
>
> /* Because we only establish the connector -> encoder ->
> * crtc links if something is active, this means the
> @@ -15363,6 +15364,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
>
> crtc->state->connector_mask |=
> 1 << drm_connector_index(&connector->base);
> + crtc->state->encoder_mask |=
> + 1 << drm_encoder_index(&encoder->base);
> }
>
> if (active && !has_active_crtc) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 4999fe530f37..7ea83b6a2864 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -265,6 +265,7 @@ struct drm_atomic_state;
> * @connectors_changed: connectors to this crtc have been updated
> * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
> * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
> + * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders
> * @last_vblank_count: for helpers and drivers to capture the vblank of the
> * update to ensure framebuffer cleanup isn't done too early
> * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
> @@ -299,6 +300,7 @@ struct drm_crtc_state {
> u32 plane_mask;
>
> u32 connector_mask;
> + u32 encoder_mask;
>
> /* last_vblank_count: for vblank waits before cleanup */
> u32 last_vblank_count;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation