| Message ID | 1476489013-16019-1-git-send-email-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 14, 2016 at 07:50:13PM -0400, Rob Clark wrote: > We had this wired up already internally but initially did not expose the > property pending bikeshed about alpha and color management properties. > I noted that drm-hwc2 was looking for this property, and a couple other > drivers already support it in the same way. So time to expose it! > > Signed-off-by: Rob Clark <robdclark@gmail.com> Can we please have a bit of shared property setup in drm_blend.c and some documentation how it is supposed to be used? Adding props is nice, but greating an undocumented and ill-defined mess of them, not so much ;-) I'd prefer the full specced blending equation in the docs, including how this interacts with fb formats which already have their own alpha value. Thanks, Daniel > --- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > index 432c098..b6f1fc66 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > @@ -120,6 +120,7 @@ static void mdp5_plane_install_properties(struct drm_plane *plane, > ARRAY_SIZE(name##_prop_enum_list)) > > INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1); > + INSTALL_RANGE_PROPERTY(alpha, ALPHA, 0, 255, 255); > > mdp5_plane_install_rotation_property(dev, plane); > > @@ -148,6 +149,7 @@ static int mdp5_plane_atomic_set_property(struct drm_plane *plane, > } while (0) > > SET_PROPERTY(zpos, ZPOS, uint8_t); > + SET_PROPERTY(alpha, ALPHA, uint8_t); > > dev_err(dev->dev, "Invalid property\n"); > ret = -EINVAL; > @@ -176,6 +178,7 @@ static int mdp5_plane_atomic_get_property(struct drm_plane *plane, > } while (0) > > GET_PROPERTY(zpos, ZPOS, uint8_t); > + GET_PROPERTY(alpha, ALPHA, uint8_t); > > dev_err(dev->dev, "Invalid property\n"); > ret = -EINVAL; > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Oct 17, 2016 at 2:24 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Oct 14, 2016 at 07:50:13PM -0400, Rob Clark wrote: >> We had this wired up already internally but initially did not expose the >> property pending bikeshed about alpha and color management properties. >> I noted that drm-hwc2 was looking for this property, and a couple other >> drivers already support it in the same way. So time to expose it! >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> > > Can we please have a bit of shared property setup in drm_blend.c and some > documentation how it is supposed to be used? Adding props is nice, but > greating an undocumented and ill-defined mess of them, not so much ;-) > > I'd prefer the full specced blending equation in the docs, including how > this interacts with fb formats which already have their own alpha value. It is used same way as rcar-du and atmel-hlcdc (and already is in kms-properties.csv couldn't really tell you the blend equation.. but I guess it is "whatever android wants" BR, -R > Thanks, Daniel > >> --- >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> index 432c098..b6f1fc66 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> @@ -120,6 +120,7 @@ static void mdp5_plane_install_properties(struct drm_plane *plane, >> ARRAY_SIZE(name##_prop_enum_list)) >> >> INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1); >> + INSTALL_RANGE_PROPERTY(alpha, ALPHA, 0, 255, 255); >> >> mdp5_plane_install_rotation_property(dev, plane); >> >> @@ -148,6 +149,7 @@ static int mdp5_plane_atomic_set_property(struct drm_plane *plane, >> } while (0) >> >> SET_PROPERTY(zpos, ZPOS, uint8_t); >> + SET_PROPERTY(alpha, ALPHA, uint8_t); >> >> dev_err(dev->dev, "Invalid property\n"); >> ret = -EINVAL; >> @@ -176,6 +178,7 @@ static int mdp5_plane_atomic_get_property(struct drm_plane *plane, >> } while (0) >> >> GET_PROPERTY(zpos, ZPOS, uint8_t); >> + GET_PROPERTY(alpha, ALPHA, uint8_t); >> >> dev_err(dev->dev, "Invalid property\n"); >> ret = -EINVAL; >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Mon, Oct 17, 2016 at 10:08:28AM -0400, Rob Clark wrote: > On Mon, Oct 17, 2016 at 2:24 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Fri, Oct 14, 2016 at 07:50:13PM -0400, Rob Clark wrote: > >> We had this wired up already internally but initially did not expose the > >> property pending bikeshed about alpha and color management properties. > >> I noted that drm-hwc2 was looking for this property, and a couple other > >> drivers already support it in the same way. So time to expose it! > >> > >> Signed-off-by: Rob Clark <robdclark@gmail.com> > > > > Can we please have a bit of shared property setup in drm_blend.c and some > > documentation how it is supposed to be used? Adding props is nice, but > > greating an undocumented and ill-defined mess of them, not so much ;-) > > > > I'd prefer the full specced blending equation in the docs, including how > > this interacts with fb formats which already have their own alpha value. > > > It is used same way as rcar-du and atmel-hlcdc (and already is in > kms-properties.csv > > couldn't really tell you the blend equation.. but I guess it is > "whatever android wants" This is the kind of answer that uapi disasters are made of. Can you pls figure this one out? Worst case just make it to fit msm ;-) Also would be great to put the alpha value into drm_plane_state, like with the other standardized properties. Imo we really should be doing this right, and just because we didn't do it right in the past isn't a good excuse. -Daniel > > BR, > -R > > > Thanks, Daniel > > > >> --- > >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > >> index 432c098..b6f1fc66 100644 > >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > >> @@ -120,6 +120,7 @@ static void mdp5_plane_install_properties(struct drm_plane *plane, > >> ARRAY_SIZE(name##_prop_enum_list)) > >> > >> INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1); > >> + INSTALL_RANGE_PROPERTY(alpha, ALPHA, 0, 255, 255); > >> > >> mdp5_plane_install_rotation_property(dev, plane); > >> > >> @@ -148,6 +149,7 @@ static int mdp5_plane_atomic_set_property(struct drm_plane *plane, > >> } while (0) > >> > >> SET_PROPERTY(zpos, ZPOS, uint8_t); > >> + SET_PROPERTY(alpha, ALPHA, uint8_t); > >> > >> dev_err(dev->dev, "Invalid property\n"); > >> ret = -EINVAL; > >> @@ -176,6 +178,7 @@ static int mdp5_plane_atomic_get_property(struct drm_plane *plane, > >> } while (0) > >> > >> GET_PROPERTY(zpos, ZPOS, uint8_t); > >> + GET_PROPERTY(alpha, ALPHA, uint8_t); > >> > >> dev_err(dev->dev, "Invalid property\n"); > >> ret = -EINVAL; > >> -- > >> 2.7.4 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
On Mon, Oct 17, 2016 at 04:34:27PM +0200, Daniel Vetter wrote: > On Mon, Oct 17, 2016 at 10:08:28AM -0400, Rob Clark wrote: > > On Mon, Oct 17, 2016 at 2:24 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Fri, Oct 14, 2016 at 07:50:13PM -0400, Rob Clark wrote: > > >> We had this wired up already internally but initially did not expose the > > >> property pending bikeshed about alpha and color management properties. > > >> I noted that drm-hwc2 was looking for this property, and a couple other > > >> drivers already support it in the same way. So time to expose it! > > >> > > >> Signed-off-by: Rob Clark <robdclark@gmail.com> > > > > > > Can we please have a bit of shared property setup in drm_blend.c and some > > > documentation how it is supposed to be used? Adding props is nice, but > > > greating an undocumented and ill-defined mess of them, not so much ;-) > > > > > > I'd prefer the full specced blending equation in the docs, including how > > > this interacts with fb formats which already have their own alpha value. > > > > > > It is used same way as rcar-du and atmel-hlcdc (and already is in > > kms-properties.csv > > > > couldn't really tell you the blend equation.. but I guess it is > > "whatever android wants" > > This is the kind of answer that uapi disasters are made of. Can you pls > figure this one out? Worst case just make it to fit msm ;-) Someone could also try to pick up the full blend equation property stuff. By now I've forgotten how we ended with that one the last time around. > > Also would be great to put the alpha value into drm_plane_state, like with > the other standardized properties. > > Imo we really should be doing this right, and just because we didn't do it > right in the past isn't a good excuse. > -Daniel > > > > > BR, > > -R > > > > > Thanks, Daniel > > > > > >> --- > > >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > >> index 432c098..b6f1fc66 100644 > > >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > >> @@ -120,6 +120,7 @@ static void mdp5_plane_install_properties(struct drm_plane *plane, > > >> ARRAY_SIZE(name##_prop_enum_list)) > > >> > > >> INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1); > > >> + INSTALL_RANGE_PROPERTY(alpha, ALPHA, 0, 255, 255); > > >> > > >> mdp5_plane_install_rotation_property(dev, plane); > > >> > > >> @@ -148,6 +149,7 @@ static int mdp5_plane_atomic_set_property(struct drm_plane *plane, > > >> } while (0) > > >> > > >> SET_PROPERTY(zpos, ZPOS, uint8_t); > > >> + SET_PROPERTY(alpha, ALPHA, uint8_t); > > >> > > >> dev_err(dev->dev, "Invalid property\n"); > > >> ret = -EINVAL; > > >> @@ -176,6 +178,7 @@ static int mdp5_plane_atomic_get_property(struct drm_plane *plane, > > >> } while (0) > > >> > > >> GET_PROPERTY(zpos, ZPOS, uint8_t); > > >> + GET_PROPERTY(alpha, ALPHA, uint8_t); > > >> > > >> dev_err(dev->dev, "Invalid property\n"); > > >> ret = -EINVAL; > > >> -- > > >> 2.7.4 > > >> > > >> _______________________________________________ > > >> dri-devel mailing list > > >> dri-devel@lists.freedesktop.org > > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index 432c098..b6f1fc66 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -120,6 +120,7 @@ static void mdp5_plane_install_properties(struct drm_plane *plane, ARRAY_SIZE(name##_prop_enum_list)) INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1); + INSTALL_RANGE_PROPERTY(alpha, ALPHA, 0, 255, 255); mdp5_plane_install_rotation_property(dev, plane); @@ -148,6 +149,7 @@ static int mdp5_plane_atomic_set_property(struct drm_plane *plane, } while (0) SET_PROPERTY(zpos, ZPOS, uint8_t); + SET_PROPERTY(alpha, ALPHA, uint8_t); dev_err(dev->dev, "Invalid property\n"); ret = -EINVAL; @@ -176,6 +178,7 @@ static int mdp5_plane_atomic_get_property(struct drm_plane *plane, } while (0) GET_PROPERTY(zpos, ZPOS, uint8_t); + GET_PROPERTY(alpha, ALPHA, uint8_t); dev_err(dev->dev, "Invalid property\n"); ret = -EINVAL;
We had this wired up already internally but initially did not expose the property pending bikeshed about alpha and color management properties. I noted that drm-hwc2 was looking for this property, and a couple other drivers already support it in the same way. So time to expose it! Signed-off-by: Rob Clark <robdclark@gmail.com> --- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 3 +++ 1 file changed, 3 insertions(+)