[PATCH] drm/msm/mdp5: expose "alpha" property

[PATCH] drm/msm/mdp5: expose "alpha" property

[Rob Clark]

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(+)

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

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: expose "alpha" property

[Daniel Vetter]

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: expose "alpha" property

[Rob Clark]

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
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: expose "alpha" property

[Daniel Vetter]

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: expose "alpha" property

[Ville Syrjälä]

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

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