Re: [Intel-gfx] [PATCHv2 1/5] drm: Introduce the blend-func property
On Fri, Apr 29, 2016 at 02:59:13PM +0530, Vandita Kulkarni wrote: > From: Damien Lespiau> > 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ä > Signed-off-by: Damien Lespiau > Signed-off-by: Vandita Kulkarni > --- > 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) > Description/Restrictions > > > - DRM > + DRM > Generic > “rotation” > BITMASK > @@ -1868,7 +1868,7 @@ void intel_crt_init(struct drm_device *dev) > CRTC that connector is attached to (atomic) > > > - Plane > + Plane > “type” > ENUM | IMMUTABLE > { "Overlay", "Primary", "Cursor" } > @@ -1946,6 +1946,13 @@ void intel_crt_init(struct drm_device *dev) > CRTC that plane is attached to (atomic) > > > + “blend_func” > + None > + DRM_BLEND_FUNC() > + Plane > + Source and destination blending factors > + > + > DVI-I > “subconnector” > ENUM > 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_X4 > #define DRM_REFLECT_Y5 > > +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
Re: [Intel-gfx] [PATCHv2 1/5] drm: Introduce the blend-func property
On Tue, 26 Apr 2016, Daniel Vetterwrote: > New props need to be cc'ed to dri-devel. And we need to coordinate with > other folks doing the same and have one unified set of blending ops. The more general answer is, check the maintainers for the files you're changing, in the MAINTAINERS file. scripts/get_maintainer.pl will help you with that. $ scripts/get_maintainer.pl 0001-drm-Introduce-the-blend-func-property.patch Jonathan Corbet (maintainer:DOCUMENTATION) David Airlie (maintainer:DRM DRIVERS) Daniel Vetter (commit_signer:45/42=100%,authored:19/42=45%) Lukas Wunner (commit_signer:8/42=19%,authored:7/42=17%) Thierry Reding (commit_signer:7/42=17%) Darren Hart (commit_signer:3/42=7%) Jani Nikula (commit_signer:3/42=7%,authored:3/42=7%) linux-...@vger.kernel.org (open list:DOCUMENTATION) linux-ker...@vger.kernel.org (open list) dri-de...@lists.freedesktop.org (open list:DRM DRIVERS) You can often ignore the "commit_signer" stats (and sometimes people get annyoed for being randomly Cc'd for things they did long ago), and here you need to take into account the following patches and include the drm/i915 list and maintainers. get_maintainer.pl is not the silver bullet, but it will give you an idea, and in this case a clear indication intel-gfx is not enough. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCHv2 1/5] drm: Introduce the blend-func property
On Tue, Apr 26, 2016 at 03:03:00PM +0530, Vandita Kulkarni wrote: > From: Damien Lespiau> > 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ä > Signed-off-by: Damien Lespiau > Signed-off-by: Vandita Kulkarni Also, new ABI = where is the open source userspace for this? -Daniel > --- > 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) > Description/Restrictions > > > - DRM > + DRM > Generic > “rotation” > BITMASK > @@ -1868,7 +1868,7 @@ void intel_crt_init(struct drm_device *dev) > CRTC that connector is attached to (atomic) > > > - Plane > + Plane > “type” > ENUM | IMMUTABLE > { "Overlay", "Primary", "Cursor" } > @@ -1946,6 +1946,13 @@ void intel_crt_init(struct drm_device *dev) > CRTC that plane is attached to (atomic) > > > + “blend_func” > + None > + DRM_BLEND_FUNC() > + Plane > + Source and destination blending factors > + > + > DVI-I > “subconnector” > ENUM > 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_X4 > #define DRM_REFLECT_Y5 > > +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, > +}; > + > +#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) & 0x) > +#define DRM_BLEND_FUNC_DST_FACTOR(val) ((val) & 0x) > + > +struct drm_blend_mode { > + uint64_t func; > +};
Re: [Intel-gfx] [PATCHv2 1/5] drm: Introduce the blend-func property
On Tue, Apr 26, 2016 at 03:03:00PM +0530, Vandita Kulkarni wrote: > From: Damien Lespiau> > 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ä > Signed-off-by: Damien Lespiau > Signed-off-by: Vandita Kulkarni New props need to be cc'ed to dri-devel. And we need to coordinate with other folks doing the same and have one unified set of blending ops. -Daniel > --- > 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) > Description/Restrictions > > > - DRM > + DRM > Generic > “rotation” > BITMASK > @@ -1868,7 +1868,7 @@ void intel_crt_init(struct drm_device *dev) > CRTC that connector is attached to (atomic) > > > - Plane > + Plane > “type” > ENUM | IMMUTABLE > { "Overlay", "Primary", "Cursor" } > @@ -1946,6 +1946,13 @@ void intel_crt_init(struct drm_device *dev) > CRTC that plane is attached to (atomic) > > > + “blend_func” > + None > + DRM_BLEND_FUNC() > + Plane > + Source and destination blending factors > + > + > DVI-I > “subconnector” > ENUM > 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_X4 > #define DRM_REFLECT_Y5 > > +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, > +}; > + > +#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) & 0x) > +#define DRM_BLEND_FUNC_DST_FACTOR(val)