Re: [Intel-gfx] [PATCHv2 1/5] drm: Introduce the blend-func property

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 29, 2016 at 02:59:13PM +0530, Vandita Kulkarni wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> We'd like to be able to program the blending modes of display planes.
> Ville suggested to use something similar to the GL blend states, which
> does seem like a good idea.
> 
> For now, we only consider blend factors, but room is left for
> extensions: blend equation, separate rgb/alpha blend factors, blend
> color.
> 
> V2: Added the belnd func property support in get property.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  Documentation/DocBook/gpu.tmpl | 11 +++++++++--
>  drivers/gpu/drm/drm_atomic.c   | 14 ++++++++++++++
>  drivers/gpu/drm/drm_crtc.c     |  5 +++++
>  include/drm/drm_crtc.h         | 20 ++++++++++++++++++++
>  4 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 1464fb2..f673989 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -1816,7 +1816,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >Description/Restrictions</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="42" valign="top" >DRM</td>
> +	<td rowspan="43" valign="top" >DRM</td>
>  	<td valign="top" >Generic</td>
>  	<td valign="top" >“rotation”</td>
>  	<td valign="top" >BITMASK</td>
> @@ -1868,7 +1868,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >CRTC that connector is attached to (atomic)</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="11" valign="top" >Plane</td>
> +	<td rowspan="12" valign="top" >Plane</td>
>  	<td valign="top" >“type”</td>
>  	<td valign="top" >ENUM | IMMUTABLE</td>
>  	<td valign="top" >{ "Overlay", "Primary", "Cursor" }</td>
> @@ -1946,6 +1946,13 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >CRTC that plane is attached to (atomic)</td>
>  	</tr>
>  	<tr>
> +	<td valign="top" >“blend_func”</td>
> +	<td valign="top" >None</td>
> +	<td valign="top" >DRM_BLEND_FUNC()</td>
> +	<td valign="top" >Plane</td>
> +	<td valign="top" >Source and destination blending factors</td>
> +	</tr>
> +	<tr>
>  	<td rowspan="2" valign="top" >DVI-I</td>
>  	<td valign="top" >“subconnector”</td>
>  	<td valign="top" >ENUM</td>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8ee1db8..c2ead2d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -701,6 +701,18 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->src_h = val;
>  	} else if (property == config->rotation_property) {
>  		state->rotation = val;
> +	} else if (property == config->prop_blend_func) {
> +		enum drm_blend_factor src_factor, dst_factor;
> +
> +		src_factor = DRM_BLEND_FUNC_SRC_FACTOR(val);
> +		dst_factor = DRM_BLEND_FUNC_DST_FACTOR(val);
> +
> +		if (src_factor != dst_factor &&
> +			(src_factor == DRM_BLEND_FACTOR_AUTO ||
> +			dst_factor == DRM_BLEND_FACTOR_AUTO))
> +			return -EINVAL;
> +
> +		state->blend_mode.func = val & GENMASK(31, 0);
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -757,6 +769,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->src_h;
>  	} else if (property == config->rotation_property) {
>  		*val = state->rotation;
> +	} else if (property == config->prop_blend_func) {
> +		*val = state->blend_mode.func;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f7fe9e1..2cac5e1 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1587,6 +1587,11 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.gamma_lut_size_property = prop;
>  
> +	prop = drm_property_create_range(dev, 0, "blend_func", 0, U32_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.prop_blend_func = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 6d46842..269f660 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -89,6 +89,23 @@ static inline uint64_t I642U64(int64_t val)
>  #define DRM_REFLECT_X	4
>  #define DRM_REFLECT_Y	5
>  
> +enum drm_blend_factor {
> +	DRM_BLEND_FACTOR_AUTO,
> +	DRM_BLEND_FACTOR_ZERO,
> +	DRM_BLEND_FACTOR_ONE,
> +	DRM_BLEND_FACTOR_SRC_ALPHA,
> +	DRM_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA,

Based on what I remember of the earlier discussion I don't think these
are quite enough. We could want some way to deal with the constant alpha
as well. Hmm. I think I need to refresh my brain a bit on this before I
can say anything really.

Also IIRC I was thinking that the premult vs. non-premult would be a
separate thing outside the blend equation. That's how we have it some
hw at least AFAIK, so be shoving it all into the blend func we may
lose certain combinations. But I suppose that's not a real problem,
at least for normal compositors and such.

Also I don't like the "auto" thing. What would be the problem in just
making the default for all blend capable planes '1, 1-Sa' and for
blend incapable planes '1, 0'?

> +};
> +
> +#define DRM_BLEND_FUNC(src_factor, dst_factor)		\
> +	(DRM_BLEND_FACTOR_##src_factor << 16 | DRM_BLEND_FACTOR_##dst_factor)
> +#define DRM_BLEND_FUNC_SRC_FACTOR(val)	(((val) >> 16) & 0xffff)
> +#define DRM_BLEND_FUNC_DST_FACTOR(val)	((val) & 0xffff)
> +
> +struct drm_blend_mode {
> +	uint64_t func;
> +};
> +
>  enum drm_connector_force {
>  	DRM_FORCE_UNSPECIFIED,
>  	DRM_FORCE_OFF,
> @@ -1273,6 +1290,8 @@ struct drm_plane_state {
>  	/* Plane rotation */
>  	unsigned int rotation;
>  
> +	struct drm_blend_mode blend_mode;
> +
>  	struct drm_atomic_state *state;
>  };
>  
> @@ -2125,6 +2144,7 @@ struct drm_mode_config {
>  	struct drm_property *prop_crtc_id;
>  	struct drm_property *prop_active;
>  	struct drm_property *prop_mode_id;
> +	struct drm_property *prop_blend_func;
>  
>  	/* DVI-I properties */
>  	struct drm_property *dvi_i_subconnector_property;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel