[Intel-gfx] [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+

[Lisovskiy, Stanislav]

On Tue, 2018-05-29 at 21:28 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> On SKL+ the dst colorkey must be configured on the lower
> plane that contains the colorkey. This is in contrast to
> most earlier platforms where the dst colorkey is configured
> on the plane above.
> 
> The hardware will peform dst keying only between two immediately
> adjacent (in zorder) planes. Plane 1 will be keyed against plane 0,
> plane 2 againts plane 1, and so on. There is no way to key arbitrary
> planes against plane 0. Thus offering dst color keying on plane 2+
> is pointless. In fact it can be harmful since enabling dst keying on
> more than one plane on the same pipe leads to only the top-most of
> the planes performing the keying. For any plane lower in zorder the
> dst key enable is simply ignored.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 63
> +++++++++++++++++++++++++++++++++++--
>  1 file changed, 60 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index ee23613f9fd4..6164c2ca20c3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1071,6 +1071,36 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
>  	return 0;
>  }
>  
> +static bool has_dst_key_in_primary_plane(struct drm_i915_private
> *dev_priv)
> +{
> +	return INTEL_GEN(dev_priv) >= 9;
> +}
> +
> +static void intel_plane_set_ckey(struct intel_plane_state
> *plane_state,
> +				 const struct
> drm_intel_sprite_colorkey *set)
> +{
> +	struct intel_plane *plane = to_intel_plane(plane_state-
> >base.plane);
> +	struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> +
> +	*key = *set;
> +
> +	/*
> +	 * We want src key enabled on the
> +	 * sprite and not on the primary.
> +	 */
> +	if (plane->id == PLANE_PRIMARY &&
> +	    set->flags & I915_SET_COLORKEY_SOURCE)
> +		key->flags = 0;
> +
> +	/*
> +	 * On SKL+ we want dst key enabled on
> +	 * the primary and not on the sprite.
> +	 */
> +	if (plane->id != PLANE_PRIMARY &&
> +	    set->flags & I915_SET_COLORKEY_DESTINATION)
> +		key->flags = 0;
> +}
> +
>  int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void
> *data,
>  				    struct drm_file *file_priv)
>  {
> @@ -1100,6 +1130,16 @@ int intel_sprite_set_colorkey_ioctl(struct
> drm_device *dev, void *data,
>  	if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY)
>  		return -ENOENT;
>  
> +	/*
> +	 * SKL+ only plane 1 can do destination keying against plane
> 0.
> +	 * Also multiple planes can't do destination keying on the
> same
> +	 * pipe simultaneously.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 9 &&
> +	    to_intel_plane(plane)->id >= PLANE_SPRITE1 &&
> +	    set->flags & I915_SET_COLORKEY_DESTINATION)
> +		return -EINVAL;
> +
>  	drm_modeset_acquire_init(&ctx, 0);
>  
>  	state = drm_atomic_state_alloc(plane->dev);
> @@ -1112,11 +1152,28 @@ int intel_sprite_set_colorkey_ioctl(struct
> drm_device *dev, void *data,
>  	while (1) {
>  		plane_state = drm_atomic_get_plane_state(state,
> plane);
>  		ret = PTR_ERR_OR_ZERO(plane_state);
> -		if (!ret) {
> -			to_intel_plane_state(plane_state)->ckey =
> *set;
> -			ret = drm_atomic_commit(state);
> +		if (!ret)
> +			intel_plane_set_ckey(to_intel_plane_state(pl
> ane_state), set);
> +
> +		/*
> +		 * On some platforms we have to configure
> +		 * the dst colorkey on the primary plane.
> +		 */
> +		if (!ret && has_dst_key_in_primary_plane(dev_priv))
> {
> +			struct intel_crtc *crtc =
> +				intel_get_crtc_for_pipe(dev_priv,
> +							to_intel_pla
> ne(plane)->pipe);
> +
> +			plane_state =
> drm_atomic_get_plane_state(state,
> +								 crt
> c->base.primary);
> +			ret = PTR_ERR_OR_ZERO(plane_state);
> +			if (!ret)
> +				intel_plane_set_ckey(to_intel_plane_
> state(plane_state), set);
>  		}
>  
> +		if (!ret)
> +			ret = drm_atomic_commit(state);
> +
>  		if (ret != -EDEADLK)
>  			break;
>  
-- 
Best Regards,

Lisovskiy Stanislav