Re: [PATCH] drm/atomic: refuse changing CRTC for planes directly

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 26-08-15 om 21:49 schreef Daniel Vetter:
> Very strictly speaking this is possible if you have special hw and
> genlocked CRTCs. In general switching a plane between two active CRTC
> just won't work so well and is probably not tested at all. Just forbid
> it.
>
> I've put this into the core since right now no helper or driver copes
> with it, no userspace has code for it and no one asks for it. Yes
> there's piles of corner-cases where this would be possible to do this
> like:
> - switch from inactive crtc to active crtc
> - swithc from active crtc to inactive crtc
> - genlocked display
> - invisible plane (to do whatever)
> - idle plane hw due to dsi cmd mode/psr
> - whatever
> but looking at details it's not that easy to implement this correctly.
> Hence just put it into the core and add a comment, since the only
> userspace we have right now for atomic (weston) doesn't want to use
> direct plane switching either.
>
> v2: don't bother with complexity and just outright disallow plane
> switching without the intermediate OFF state. Simplifies drivers, we
> don't have any hw that could do it anyway and current atomic userspace
> (weston) works like this already anyway.
>
> v3: Bikeshed function name (Ville) and add comment (Rob).
>
> v4: Also bikeshed commit message (Rob).
>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Acked-by: Daniel Stone <daniels@collabora.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> ---
>
> Imo this is bikeshedded enough, so either someone takes this or
> someone else can mangle this patch more.
> -Daniel
> ---
>  drivers/gpu/drm/drm_atomic.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 1066e4b658cf..40ddd6aa100f 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -663,6 +663,27 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  	return 0;
>  }
>  
> +static bool
> +plane_switching_crtc(struct drm_atomic_state *state,
> +		     struct drm_plane *plane,
> +		     struct drm_plane_state *plane_state)
> +{
> +	struct drm_crtc_state *crtc_state, *curr_crtc_state;
> +
> +	if (!plane->state->crtc || !plane_state->crtc)
> +		return false;
> +
> +	if (plane->state->crtc == plane_state->crtc)
> +		return false;
> +
> +	/* This could be refined, but currently there's no helper or driver code
> +	 * to implement direct switching of active planes nor userspace to take
> +	 * advantage of more direct plane switching without the intermediate
> +	 * full OFF state.
> +	 */
> +	return true;
> +}
> +
>  /**
>   * drm_atomic_plane_check - check plane state
>   * @plane: plane to check
> @@ -734,6 +755,12 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>  		return -ENOSPC;
>  	}
>  
> +	if (plane_switching_crtc(state->state, plane, state)) {
> +		DRM_DEBUG_ATOMIC("[PLANE:%d] switching CRTC directly\n",
> +				 plane->base.id);
> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel