Re: [PATCH v2 10/15] v4l: vsp1: Turn frame end completion status into a bitfield

[Kieran Bingham]

Hi Laurent,

Thanks for this enhancement.

On 05/04/18 10:18, Laurent Pinchart wrote:
> We will soon need to return more than a boolean completion status from
> the vsp1_dlm_irq_frame_end() IRQ handler. Turn the return value into a
> bitfield to prepare for that. No functional change is introduced here.

I think this is a good solution!

Reviewed-by: Kieran Bingham 


> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/platform/vsp1/vsp1_dl.c| 22 +-
>  drivers/media/platform/vsp1/vsp1_dl.h|  4 +++-
>  drivers/media/platform/vsp1/vsp1_drm.c   |  5 +++--
>  drivers/media/platform/vsp1/vsp1_pipe.c  |  8 
>  drivers/media/platform/vsp1/vsp1_pipe.h  |  2 +-
>  drivers/media/platform/vsp1/vsp1_video.c |  4 ++--
>  6 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
> b/drivers/media/platform/vsp1/vsp1_dl.c
> index 0b86ed01e85d..662fa2a347c9 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -616,14 +616,18 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>   * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt
>   * @dlm: the display list manager
>   *
> - * Return true if the previous display list has completed at frame end, or 
> false
> - * if it has been delayed by one frame because the display list commit raced
> - * with the frame end interrupt. The function always returns true in header 
> mode
> - * as display list processing is then not continuous and races never occur.
> + * Return a set of flags that indicates display list completion status.
> + *
> + * The VSP1_DL_FRAME_END_COMPLETED flag indicates that the previous display 
> list
> + * has completed at frame end. If the flag is not returned display list
> + * completion has been delayed by one frame because the display list commit
> + * raced with the frame end interrupt. The function always returns with the 
> flag
> + * set in header mode as display list processing is then not continuous and
> + * races never occur.
>   */
> -bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
> +unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>  {
> - bool completed = false;
> + unsigned int flags = 0;
>  
>   spin_lock(>lock);
>  
> @@ -634,7 +638,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>   if (dlm->singleshot) {
>   __vsp1_dl_list_put(dlm->active);
>   dlm->active = NULL;
> - completed = true;
> + flags |= VSP1_DL_FRAME_END_COMPLETED;
>   goto done;
>   }
>  
> @@ -655,7 +659,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>   __vsp1_dl_list_put(dlm->active);
>   dlm->active = dlm->queued;
>   dlm->queued = NULL;
> - completed = true;
> + flags |= VSP1_DL_FRAME_END_COMPLETED;
>   }
>  
>   /*
> @@ -672,7 +676,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>  done:
>   spin_unlock(>lock);
>  
> - return completed;
> + return flags;
>  }
>  
>  /* Hardware Setup */
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h 
> b/drivers/media/platform/vsp1/vsp1_dl.h
> index ee3508172f0a..cbc2fc53e10b 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.h
> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
> @@ -20,6 +20,8 @@ struct vsp1_dl_fragment;
>  struct vsp1_dl_list;
>  struct vsp1_dl_manager;
>  
> +#define VSP1_DL_FRAME_END_COMPLETED  BIT(0)
> +
>  void vsp1_dlm_setup(struct vsp1_device *vsp1);
>  
>  struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
> @@ -27,7 +29,7 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device 
> *vsp1,
>   unsigned int prealloc);
>  void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm);
>  void vsp1_dlm_reset(struct vsp1_dl_manager *dlm);
> -bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
> +unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
>  
>  struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
>  void vsp1_dl_list_put(struct vsp1_dl_list *dl);
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index a79b05ef..541473b1df67 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -34,12 +34,13 @@
>   */
>  
>  static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe,
> -bool completed)
> +unsigned int completion)
>  {
>   struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
>  
>   if (drm_pipe->du_complete)
> - drm_pipe->du_complete(drm_pipe->du_private, completed);
> + drm_pipe->du_complete(drm_pipe->du_private,
> +

[PATCH v2 10/15] v4l: vsp1: Turn frame end completion status into a bitfield

[Laurent Pinchart]

We will soon need to return more than a boolean completion status from
the vsp1_dlm_irq_frame_end() IRQ handler. Turn the return value into a
bitfield to prepare for that. No functional change is introduced here.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_dl.c| 22 +-
 drivers/media/platform/vsp1/vsp1_dl.h|  4 +++-
 drivers/media/platform/vsp1/vsp1_drm.c   |  5 +++--
 drivers/media/platform/vsp1/vsp1_pipe.c  |  8 
 drivers/media/platform/vsp1/vsp1_pipe.h  |  2 +-
 drivers/media/platform/vsp1/vsp1_video.c |  4 ++--
 6 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 0b86ed01e85d..662fa2a347c9 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -616,14 +616,18 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
  * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt
  * @dlm: the display list manager
  *
- * Return true if the previous display list has completed at frame end, or 
false
- * if it has been delayed by one frame because the display list commit raced
- * with the frame end interrupt. The function always returns true in header 
mode
- * as display list processing is then not continuous and races never occur.
+ * Return a set of flags that indicates display list completion status.
+ *
+ * The VSP1_DL_FRAME_END_COMPLETED flag indicates that the previous display 
list
+ * has completed at frame end. If the flag is not returned display list
+ * completion has been delayed by one frame because the display list commit
+ * raced with the frame end interrupt. The function always returns with the 
flag
+ * set in header mode as display list processing is then not continuous and
+ * races never occur.
  */
-bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
+unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 {
-   bool completed = false;
+   unsigned int flags = 0;
 
spin_lock(>lock);
 
@@ -634,7 +638,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
if (dlm->singleshot) {
__vsp1_dl_list_put(dlm->active);
dlm->active = NULL;
-   completed = true;
+   flags |= VSP1_DL_FRAME_END_COMPLETED;
goto done;
}
 
@@ -655,7 +659,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
__vsp1_dl_list_put(dlm->active);
dlm->active = dlm->queued;
dlm->queued = NULL;
-   completed = true;
+   flags |= VSP1_DL_FRAME_END_COMPLETED;
}
 
/*
@@ -672,7 +676,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 done:
spin_unlock(>lock);
 
-   return completed;
+   return flags;
 }
 
 /* Hardware Setup */
diff --git a/drivers/media/platform/vsp1/vsp1_dl.h 
b/drivers/media/platform/vsp1/vsp1_dl.h
index ee3508172f0a..cbc2fc53e10b 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.h
+++ b/drivers/media/platform/vsp1/vsp1_dl.h
@@ -20,6 +20,8 @@ struct vsp1_dl_fragment;
 struct vsp1_dl_list;
 struct vsp1_dl_manager;
 
+#define VSP1_DL_FRAME_END_COMPLETEDBIT(0)
+
 void vsp1_dlm_setup(struct vsp1_device *vsp1);
 
 struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
@@ -27,7 +29,7 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device 
*vsp1,
unsigned int prealloc);
 void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm);
 void vsp1_dlm_reset(struct vsp1_dl_manager *dlm);
-bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
+unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
 
 struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
 void vsp1_dl_list_put(struct vsp1_dl_list *dl);
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a79b05ef..541473b1df67 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -34,12 +34,13 @@
  */
 
 static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe,
-  bool completed)
+  unsigned int completion)
 {
struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
 
if (drm_pipe->du_complete)
-   drm_pipe->du_complete(drm_pipe->du_private, completed);
+   drm_pipe->du_complete(drm_pipe->du_private,
+ completion & VSP1_DL_FRAME_END_COMPLETED);
 }
 
 /* 
-
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
b/drivers/media/platform/vsp1/vsp1_pipe.c
index 99ccbac3256a..1134f14ed4aa 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++