[2/5] drm: sti: add atomic get/set properties for planes

Message ID 1452173441-30033-3-git-send-email-benjamin.gaignard@linaro.org
State New
Headers show

Commit Message

Benjamin Gaignard Jan. 7, 2016, 1:30 p.m.
Without those function zpos property isn't displayed in atomic mode.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/gpu/drm/sti/sti_plane.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Benjamin Gaignard Jan. 7, 2016, 2:05 p.m. | #1
zpos property isn't new in sti driver only the atomic set/get functions are
new.
Without those functions atomictest failed to discover any of the other
atomic planes properties.

I agree with you, zpos should be generic and I could work with Marek for
that
but if this patch isn't merge sti atomic modesetting support will be
limited to primary planes...

Benjamin


2016-01-07 14:50 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>:

> On Thu, Jan 07, 2016 at 02:30:38PM +0100, Benjamin Gaignard wrote:

> > Without those function zpos property isn't displayed in atomic mode.

> >

> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

>

> Oh, another iteration of a driver-private zpos property. This _really_

> should be generic, with some consistent cross-driver semantics. Marek from

> samsung has started with a patch series for that.

>

> Can you please work together with him, and rebase the sti zpos support on

> top of his work? And hold of this patch for now meanwhile?

>

> Also adding Marek.

>

> Thanks, Daniel

>

> > ---

> >  drivers/gpu/drm/sti/sti_plane.c | 40

> ++++++++++++++++++++++++++++++++++++++++

> >  1 file changed, 40 insertions(+)

> >

> > diff --git a/drivers/gpu/drm/sti/sti_plane.c

> b/drivers/gpu/drm/sti/sti_plane.c

> > index 2e5c751..e739c5a 100644

> > --- a/drivers/gpu/drm/sti/sti_plane.c

> > +++ b/drivers/gpu/drm/sti/sti_plane.c

> > @@ -69,6 +69,44 @@ static int sti_plane_set_property(struct drm_plane

> *drm_plane,

> >       return -EINVAL;

> >  }

> >

> > +static int sti_plane_atomic_set_property(struct drm_plane *drm_plane,

> > +                                      struct drm_plane_state *state,

> > +                                      struct drm_property *property,

> > +                                      uint64_t val)

> > +{

> > +     struct drm_device *dev = drm_plane->dev;

> > +     struct sti_private *private = dev->dev_private;

> > +     struct sti_plane *plane = to_sti_plane(drm_plane);

> > +

> > +     DRM_DEBUG_DRIVER("\n");

> > +

> > +     if (property == private->plane_zorder_property) {

> > +             plane->zorder = val;

> > +             return 0;

> > +     }

> > +

> > +     return -EINVAL;

> > +}

> > +

> > +static int sti_plane_atomic_get_property(struct drm_plane *drm_plane,

> > +                                      const struct drm_plane_state

> *state,

> > +                                      struct drm_property *property,

> > +                                      uint64_t *val)

> > +{

> > +     struct drm_device *dev = drm_plane->dev;

> > +     struct sti_private *private = dev->dev_private;

> > +     struct sti_plane *plane = to_sti_plane(drm_plane);

> > +

> > +     DRM_DEBUG_DRIVER("\n");

> > +

> > +     if (property == private->plane_zorder_property) {

> > +             *val = plane->zorder;

> > +             return 0;

> > +     }

> > +

> > +     return -EINVAL;

> > +}

> > +

> >  static void sti_plane_attach_zorder_property(struct drm_plane

> *drm_plane)

> >  {

> >       struct drm_device *dev = drm_plane->dev;

> > @@ -116,4 +154,6 @@ struct drm_plane_funcs sti_plane_helpers_funcs = {

> >       .reset = drm_atomic_helper_plane_reset,

> >       .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,

> >       .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,

> > +     .atomic_set_property = sti_plane_atomic_set_property,

> > +     .atomic_get_property = sti_plane_atomic_get_property,

> >  };

> > --

> > 1.9.1

> >

> > _______________________________________________

> > dri-devel mailing list

> > dri-devel@lists.freedesktop.org

> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

>

> --

> Daniel Vetter

> Software Engineer, Intel Corporation

> http://blog.ffwll.ch

>




-- 

Benjamin Gaignard

Graphic Working Group

Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs

Follow *Linaro: *Facebook <http://www.facebook.com/pages/Linaro> | Twitter
<http://twitter.com/#!/linaroorg> | Blog
<http://www.linaro.org/linaro-blog/>
Benjamin Gaignard Jan. 13, 2016, 8:53 a.m. | #2
Hi,

I have been able to implement zpos in generic way using Marek's patches (v3)
so I'm abandon this patch and wait to see Marek's patches merged to propose
a new one.

Regards,
Benjamin

2016-01-07 15:47 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>:

> On Thu, Jan 07, 2016 at 03:05:02PM +0100, Benjamin Gaignard wrote:

> > zpos property isn't new in sti driver only the atomic set/get functions

> are

> > new.

> > Without those functions atomictest failed to discover any of the other

> > atomic planes properties.

> >

> > I agree with you, zpos should be generic and I could work with Marek for

> > that

> > but if this patch isn't merge sti atomic modesetting support will be

> > limited to primary planes...

>

> These two hooks should be optional. The only downside of not adding them

> is that zpos property can't be read, everything else should still work ...

>

> And yeah everyone added zpos without a hole lot of thought about

> cross-driver consistency, which is not great. Now everyone gets to clean

> things up again I'd say.

> -Daniel

>

> >

> > Benjamin

> >

> >

> > 2016-01-07 14:50 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>:

> >

> > > On Thu, Jan 07, 2016 at 02:30:38PM +0100, Benjamin Gaignard wrote:

> > > > Without those function zpos property isn't displayed in atomic mode.

> > > >

> > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

> > >

> > > Oh, another iteration of a driver-private zpos property. This _really_

> > > should be generic, with some consistent cross-driver semantics. Marek

> from

> > > samsung has started with a patch series for that.

> > >

> > > Can you please work together with him, and rebase the sti zpos support

> on

> > > top of his work? And hold of this patch for now meanwhile?

> > >

> > > Also adding Marek.

> > >

> > > Thanks, Daniel

> > >

> > > > ---

> > > >  drivers/gpu/drm/sti/sti_plane.c | 40

> > > ++++++++++++++++++++++++++++++++++++++++

> > > >  1 file changed, 40 insertions(+)

> > > >

> > > > diff --git a/drivers/gpu/drm/sti/sti_plane.c

> > > b/drivers/gpu/drm/sti/sti_plane.c

> > > > index 2e5c751..e739c5a 100644

> > > > --- a/drivers/gpu/drm/sti/sti_plane.c

> > > > +++ b/drivers/gpu/drm/sti/sti_plane.c

> > > > @@ -69,6 +69,44 @@ static int sti_plane_set_property(struct drm_plane

> > > *drm_plane,

> > > >       return -EINVAL;

> > > >  }

> > > >

> > > > +static int sti_plane_atomic_set_property(struct drm_plane

> *drm_plane,

> > > > +                                      struct drm_plane_state *state,

> > > > +                                      struct drm_property *property,

> > > > +                                      uint64_t val)

> > > > +{

> > > > +     struct drm_device *dev = drm_plane->dev;

> > > > +     struct sti_private *private = dev->dev_private;

> > > > +     struct sti_plane *plane = to_sti_plane(drm_plane);

> > > > +

> > > > +     DRM_DEBUG_DRIVER("\n");

> > > > +

> > > > +     if (property == private->plane_zorder_property) {

> > > > +             plane->zorder = val;

> > > > +             return 0;

> > > > +     }

> > > > +

> > > > +     return -EINVAL;

> > > > +}

> > > > +

> > > > +static int sti_plane_atomic_get_property(struct drm_plane

> *drm_plane,

> > > > +                                      const struct drm_plane_state

> > > *state,

> > > > +                                      struct drm_property *property,

> > > > +                                      uint64_t *val)

> > > > +{

> > > > +     struct drm_device *dev = drm_plane->dev;

> > > > +     struct sti_private *private = dev->dev_private;

> > > > +     struct sti_plane *plane = to_sti_plane(drm_plane);

> > > > +

> > > > +     DRM_DEBUG_DRIVER("\n");

> > > > +

> > > > +     if (property == private->plane_zorder_property) {

> > > > +             *val = plane->zorder;

> > > > +             return 0;

> > > > +     }

> > > > +

> > > > +     return -EINVAL;

> > > > +}

> > > > +

> > > >  static void sti_plane_attach_zorder_property(struct drm_plane

> > > *drm_plane)

> > > >  {

> > > >       struct drm_device *dev = drm_plane->dev;

> > > > @@ -116,4 +154,6 @@ struct drm_plane_funcs sti_plane_helpers_funcs =

> {

> > > >       .reset = drm_atomic_helper_plane_reset,

> > > >       .atomic_duplicate_state =

> drm_atomic_helper_plane_duplicate_state,

> > > >       .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,

> > > > +     .atomic_set_property = sti_plane_atomic_set_property,

> > > > +     .atomic_get_property = sti_plane_atomic_get_property,

> > > >  };

> > > > --

> > > > 1.9.1

> > > >

> > > > _______________________________________________

> > > > dri-devel mailing list

> > > > dri-devel@lists.freedesktop.org

> > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel

> > >

> > > --

> > > Daniel Vetter

> > > Software Engineer, Intel Corporation

> > > http://blog.ffwll.ch

> > >

> >

> >

> >

> > --

> >

> > Benjamin Gaignard

> >

> > Graphic Working Group

> >

> > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM

> SoCs

> >

> > Follow *Linaro: *Facebook <http://www.facebook.com/pages/Linaro> |

> Twitter

> > <http://twitter.com/#!/linaroorg> | Blog

> > <http://www.linaro.org/linaro-blog/>

>

> --

> Daniel Vetter

> Software Engineer, Intel Corporation

> http://blog.ffwll.ch

>




-- 

Benjamin Gaignard

Graphic Working Group

Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs

Follow *Linaro: *Facebook <http://www.facebook.com/pages/Linaro> | Twitter
<http://twitter.com/#!/linaroorg> | Blog
<http://www.linaro.org/linaro-blog/>

Patch

diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index 2e5c751..e739c5a 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -69,6 +69,44 @@  static int sti_plane_set_property(struct drm_plane *drm_plane,
 	return -EINVAL;
 }
 
+static int sti_plane_atomic_set_property(struct drm_plane *drm_plane,
+					 struct drm_plane_state *state,
+					 struct drm_property *property,
+					 uint64_t val)
+{
+	struct drm_device *dev = drm_plane->dev;
+	struct sti_private *private = dev->dev_private;
+	struct sti_plane *plane = to_sti_plane(drm_plane);
+
+	DRM_DEBUG_DRIVER("\n");
+
+	if (property == private->plane_zorder_property) {
+		plane->zorder = val;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int sti_plane_atomic_get_property(struct drm_plane *drm_plane,
+					 const struct drm_plane_state *state,
+					 struct drm_property *property,
+					 uint64_t *val)
+{
+	struct drm_device *dev = drm_plane->dev;
+	struct sti_private *private = dev->dev_private;
+	struct sti_plane *plane = to_sti_plane(drm_plane);
+
+	DRM_DEBUG_DRIVER("\n");
+
+	if (property == private->plane_zorder_property) {
+		*val = plane->zorder;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane)
 {
 	struct drm_device *dev = drm_plane->dev;
@@ -116,4 +154,6 @@  struct drm_plane_funcs sti_plane_helpers_funcs = {
 	.reset = drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	.atomic_set_property = sti_plane_atomic_set_property,
+	.atomic_get_property = sti_plane_atomic_get_property,
 };