| Message ID | 1453130143-7228-2-git-send-email-vandita.kulkarni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 18, 2016 at 08:45:35PM +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> You'll want a Cc: dri-devel here as well for stuff like this that touches the DRM core. > --- > 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 351e801..2017162 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="37" valign="top" >DRM</td> > + <td rowspan="38" 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> Shouldn't these two be type 'range' with values 0x0-0xFFFFFFFF? Your description field below would then explain the actual meaning of the high/low 16-bits. > + <td valign="top" >Plane</td> > + <td valign="top" >Source and destination blending factors</td> I feel like a bit more detailed description here would be worthwhile. Actually, given that we're presumably only using this via atomic, it doesn't seem unreasonable to me to split this into two separate enum properties. That would be a little bit more self-explanatory and the atomic check code could still ensure that we don't wind up with invalid combinations of src/dest factors. Plus it will make it a little easier for us to add additional blending factors in the future. > + </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 3f74193..0c165c6 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -630,6 +630,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; Might be better to defer this until the atomic check phase since that's where we do most of our other tests and where we expect to get failures. This function is more about shoving values into state objects (and other tasks like refcounting where necessary) without doing anything else. > + > + state->blend_mode.func = val & GENMASK(31, 0); This check should also move to the atomic check phase, but raise a failure if we have any unwanted bits. If we let userspace be sloppy right now and put garbage there, then we can't really give those bits new meaning in the future without breaking existing userspace. > } else if (plane->funcs->atomic_set_property) { > return plane->funcs->atomic_set_property(plane, state, > property, val); > @@ -686,6 +698,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 62fa95f..b8dde06 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1542,6 +1542,11 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) > return -ENOMEM; > dev->mode_config.prop_mode_id = prop; > > + prop = drm_property_create_range(dev, 0, "blend_func", 0, U32_MAX); I'd add the DRM_MODE_PROP_ATOMIC flag to this. I think we have a direction that all new functionality only be exposed via the atomic interfaces. Matt > + 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 c65a212..9cfe601 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -99,6 +99,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, > +}; > + > +#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, > @@ -1267,6 +1284,8 @@ struct drm_plane_state { > /* Plane rotation */ > unsigned int rotation; > > + struct drm_blend_mode blend_mode; > + > struct drm_atomic_state *state; > }; > > @@ -2099,6 +2118,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 > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 351e801..2017162 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="37" valign="top" >DRM</td> + <td rowspan="38" 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 3f74193..0c165c6 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -630,6 +630,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); @@ -686,6 +698,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 62fa95f..b8dde06 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1542,6 +1542,11 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = 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 c65a212..9cfe601 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -99,6 +99,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, +}; + +#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, @@ -1267,6 +1284,8 @@ struct drm_plane_state { /* Plane rotation */ unsigned int rotation; + struct drm_blend_mode blend_mode; + struct drm_atomic_state *state; }; @@ -2099,6 +2118,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;