[PATCH 03/11] drm/exynos: add prepare and cleanup phases for planes

[Inki Dae]

On 2015년 08월 27일 00:45, Gustavo Padovan wrote:
> Hi Inki,
> 
> 2015-08-24 Inki Dae :
> 
>> On 2015년 08월 16일 01:26, Gustavo Padovan wrote:
>>> From: Gustavo Padovan 
>>>
>>> .prepare_plane() and .cleanup_plane() allows to perform extra operations
>>> before and after the update of planes. For FIMD for example this will
>>> be used to enable disable the shadow protection bit.
>>>
>>> Signed-off-by: Gustavo Padovan 
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 19 +++
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  6 ++
>>>  2 files changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
>>> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> index 5a19e16..3a89fc9 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> @@ -72,15 +72,34 @@ exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>>  static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
>>>  {
>>> struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
>>> +   struct drm_plane *plane;
>>>
>>> if (crtc->state->event) {
>>> WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>>> exynos_crtc->event = crtc->state->event;
>>> }
>>> +
>>> +   drm_atomic_crtc_for_each_plane(plane, crtc) {
>>> +   struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
>>> +
>>> +   if (exynos_crtc->ops->prepare_plane)
>>> +   exynos_crtc->ops->prepare_plane(exynos_crtc,
>>> +   exynos_plane);
>>
>> There is no any reason to use prepare_plane/cleanup_plane callback
>> names. How about using atomic_begin/atomic_flush callback names instead
>> for consistency between framework and device drivers?
> 
> Either names are fine for me. So let's go with atomic_begin/flush. I'll
> send an updated patchset.

Merged.

Thanks,
Inki Dae

> 
>   Gustavo
> 

[PATCH 03/11] drm/exynos: add prepare and cleanup phases for planes

[Gustavo Padovan]

Hi Inki,

2015-08-24 Inki Dae :

> On 2015년 08월 16일 01:26, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> >
> > .prepare_plane() and .cleanup_plane() allows to perform extra operations
> > before and after the update of planes. For FIMD for example this will
> > be used to enable disable the shadow protection bit.
> >
> > Signed-off-by: Gustavo Padovan 
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 19 +++
> >  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  6 ++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
> > b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> > index 5a19e16..3a89fc9 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> > @@ -72,15 +72,34 @@ exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
> >  {
> > struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> > +   struct drm_plane *plane;
> >
> > if (crtc->state->event) {
> > WARN_ON(drm_crtc_vblank_get(crtc) != 0);
> > exynos_crtc->event = crtc->state->event;
> > }
> > +
> > +   drm_atomic_crtc_for_each_plane(plane, crtc) {
> > +   struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> > +
> > +   if (exynos_crtc->ops->prepare_plane)
> > +   exynos_crtc->ops->prepare_plane(exynos_crtc,
> > +   exynos_plane);
> 
> There is no any reason to use prepare_plane/cleanup_plane callback
> names. How about using atomic_begin/atomic_flush callback names instead
> for consistency between framework and device drivers?

Either names are fine for me. So let's go with atomic_begin/flush. I'll
send an updated patchset.

Gustavo

[PATCH 03/11] drm/exynos: add prepare and cleanup phases for planes

[Inki Dae]

On 2015년 08월 16일 01:26, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> .prepare_plane() and .cleanup_plane() allows to perform extra operations
> before and after the update of planes. For FIMD for example this will
> be used to enable disable the shadow protection bit.
> 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 19 +++
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  6 ++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 5a19e16..3a89fc9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -72,15 +72,34 @@ exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
>  {
>   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> + struct drm_plane *plane;
>  
>   if (crtc->state->event) {
>   WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>   exynos_crtc->event = crtc->state->event;
>   }
> +
> + drm_atomic_crtc_for_each_plane(plane, crtc) {
> + struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> +
> + if (exynos_crtc->ops->prepare_plane)
> + exynos_crtc->ops->prepare_plane(exynos_crtc,
> + exynos_plane);

There is no any reason to use prepare_plane/cleanup_plane callback
names. How about using atomic_begin/atomic_flush callback names instead
for consistency between framework and device drivers?

Thanks,
Inki Dae

> + }
>  }
>  
>  static void exynos_crtc_atomic_flush(struct drm_crtc *crtc)
>  {
> + struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> + struct drm_plane *plane;
> +
> + drm_atomic_crtc_for_each_plane(plane, crtc) {
> + struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> +
> + if (exynos_crtc->ops->cleanup_plane)
> + exynos_crtc->ops->cleanup_plane(exynos_crtc,
> + exynos_plane);
> + }
>  }
>  
>  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
> b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index a993aac..9f2b5c9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -87,6 +87,8 @@ struct exynos_drm_plane {
>   * @disable_vblank: specific driver callback for disabling vblank interrupt.
>   * @wait_for_vblank: wait for vblank interrupt to make sure that
>   *   hardware overlay is updated.
> + * @prepare_plane: prepare a window to receive a update
> + * @cleanup_plane: mark the end of a window update
>   * @update_plane: apply hardware specific overlay data to registers.
>   * @disable_plane: disable hardware specific overlay.
>   * @te_handler: trigger to transfer video image at the tearing effect
> @@ -107,10 +109,14 @@ struct exynos_drm_crtc_ops {
>   int (*enable_vblank)(struct exynos_drm_crtc *crtc);
>   void (*disable_vblank)(struct exynos_drm_crtc *crtc);
>   void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
> + void (*prepare_plane)(struct exynos_drm_crtc *crtc,
> +   struct exynos_drm_plane *plane);
>   void (*update_plane)(struct exynos_drm_crtc *crtc,
>struct exynos_drm_plane *plane);
>   void (*disable_plane)(struct exynos_drm_crtc *crtc,
> struct exynos_drm_plane *plane);
> + void (*cleanup_plane)(struct exynos_drm_crtc *crtc,
> +   struct exynos_drm_plane *plane);
>   void (*te_handler)(struct exynos_drm_crtc *crtc);
>   void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable);
>  };
> 

[PATCH 03/11] drm/exynos: add prepare and cleanup phases for planes

[Gustavo Padovan]

From: Gustavo Padovan 

.prepare_plane() and .cleanup_plane() allows to perform extra operations
before and after the update of planes. For FIMD for example this will
be used to enable disable the shadow protection bit.

Signed-off-by: Gustavo Padovan 
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 19 +++
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |  6 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 5a19e16..3a89fc9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -72,15 +72,34 @@ exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
 {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+   struct drm_plane *plane;

if (crtc->state->event) {
WARN_ON(drm_crtc_vblank_get(crtc) != 0);
exynos_crtc->event = crtc->state->event;
}
+
+   drm_atomic_crtc_for_each_plane(plane, crtc) {
+   struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
+
+   if (exynos_crtc->ops->prepare_plane)
+   exynos_crtc->ops->prepare_plane(exynos_crtc,
+   exynos_plane);
+   }
 }

 static void exynos_crtc_atomic_flush(struct drm_crtc *crtc)
 {
+   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+   struct drm_plane *plane;
+
+   drm_atomic_crtc_for_each_plane(plane, crtc) {
+   struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
+
+   if (exynos_crtc->ops->cleanup_plane)
+   exynos_crtc->ops->cleanup_plane(exynos_crtc,
+   exynos_plane);
+   }
 }

 static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index a993aac..9f2b5c9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -87,6 +87,8 @@ struct exynos_drm_plane {
  * @disable_vblank: specific driver callback for disabling vblank interrupt.
  * @wait_for_vblank: wait for vblank interrupt to make sure that
  * hardware overlay is updated.
+ * @prepare_plane: prepare a window to receive a update
+ * @cleanup_plane: mark the end of a window update
  * @update_plane: apply hardware specific overlay data to registers.
  * @disable_plane: disable hardware specific overlay.
  * @te_handler: trigger to transfer video image at the tearing effect
@@ -107,10 +109,14 @@ struct exynos_drm_crtc_ops {
int (*enable_vblank)(struct exynos_drm_crtc *crtc);
void (*disable_vblank)(struct exynos_drm_crtc *crtc);
void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
+   void (*prepare_plane)(struct exynos_drm_crtc *crtc,
+ struct exynos_drm_plane *plane);
void (*update_plane)(struct exynos_drm_crtc *crtc,
 struct exynos_drm_plane *plane);
void (*disable_plane)(struct exynos_drm_crtc *crtc,
  struct exynos_drm_plane *plane);
+   void (*cleanup_plane)(struct exynos_drm_crtc *crtc,
+ struct exynos_drm_plane *plane);
void (*te_handler)(struct exynos_drm_crtc *crtc);
void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable);
 };
-- 
2.1.0