Re: [PATCH v4 4/4] v4l: vsp1: Remove redundant pipe->dl usage from drm

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday 06 Jan 2017 12:15:31 Kieran Bingham wrote:
> The pipe->dl is used only inside vsp1_du_atomic_flush(), and can be
> obtained and stored locally to simplify the code.
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

As I still have issues with patch 3/4, I propose moving this one earlier in 
the series.

> ---
>  drivers/media/platform/vsp1/vsp1_drm.c  | 20 ++--
>  drivers/media/platform/vsp1/vsp1_pipe.h |  2 --
>  2 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c index b4b583f7137a..d7ec980300dd
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -220,9 +220,6 @@ void vsp1_du_atomic_begin(struct device *dev)
>   struct vsp1_pipeline *pipe = >drm->pipe;
> 
>   vsp1->drm->num_inputs = pipe->num_inputs;
> -
> - /* Prepare the display list. */
> - pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
>  }
>  EXPORT_SYMBOL_GPL(vsp1_du_atomic_begin);
> 
> @@ -426,10 +423,14 @@ void vsp1_du_atomic_flush(struct device *dev)
>   struct vsp1_pipeline *pipe = >drm->pipe;
>   struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
>   struct vsp1_entity *entity;
> + struct vsp1_dl_list *dl;
>   unsigned long flags;
>   unsigned int i;
>   int ret;
> 
> + /* Prepare the display list. */
> + dl = vsp1_dl_list_get(pipe->output->dlm);
> +
>   /* Count the number of enabled inputs and sort them by Z-order. */
>   pipe->num_inputs = 0;
> 
> @@ -484,26 +485,25 @@ void vsp1_du_atomic_flush(struct device *dev)
>   struct vsp1_rwpf *rpf = to_rwpf(>subdev);
> 
>   if (!pipe->inputs[rpf->entity.index]) {
> - vsp1_dl_list_write(pipe->dl, entity->route-
>reg,
> + vsp1_dl_list_write(dl, entity->route->reg,
>  VI6_DPR_NODE_UNUSED);
>   continue;
>   }
>   }
> 
> - vsp1_entity_route_setup(entity, pipe->dl);
> + vsp1_entity_route_setup(entity, dl);
> 
>   if (entity->ops->configure) {
> - entity->ops->configure(entity, pipe, pipe->dl,
> + entity->ops->configure(entity, pipe, dl,
>  VSP1_ENTITY_PARAMS_INIT);
> - entity->ops->configure(entity, pipe, pipe->dl,
> + entity->ops->configure(entity, pipe, dl,
>  VSP1_ENTITY_PARAMS_RUNTIME);
> - entity->ops->configure(entity, pipe, pipe->dl,
> + entity->ops->configure(entity, pipe, dl,
>  VSP1_ENTITY_PARAMS_PARTITION);
>   }
>   }
> 
> - vsp1_dl_list_commit(pipe->dl);
> - pipe->dl = NULL;
> + vsp1_dl_list_commit(dl);
> 
>   /* Start or stop the pipeline if needed. */
>   if (!vsp1->drm->num_inputs && pipe->num_inputs) {
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
> b/drivers/media/platform/vsp1/vsp1_pipe.h index fff122b4874d..e59bef2653f6
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
> @@ -108,8 +108,6 @@ struct vsp1_pipeline {
> 
>   struct list_head entities;
> 
> - struct vsp1_dl_list *dl;
> -
>   unsigned int div_size;
>   unsigned int partitions;
>   struct v4l2_rect partition;

-- 
Regards,

Laurent Pinchart

[PATCH v4 4/4] v4l: vsp1: Remove redundant pipe->dl usage from drm

[Kieran Bingham]

The pipe->dl is used only inside vsp1_du_atomic_flush(), and can be
obtained and stored locally to simplify the code.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c  | 20 ++--
 drivers/media/platform/vsp1/vsp1_pipe.h |  2 --
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index b4b583f7137a..d7ec980300dd 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -220,9 +220,6 @@ void vsp1_du_atomic_begin(struct device *dev)
struct vsp1_pipeline *pipe = >drm->pipe;
 
vsp1->drm->num_inputs = pipe->num_inputs;
-
-   /* Prepare the display list. */
-   pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_begin);
 
@@ -426,10 +423,14 @@ void vsp1_du_atomic_flush(struct device *dev)
struct vsp1_pipeline *pipe = >drm->pipe;
struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
struct vsp1_entity *entity;
+   struct vsp1_dl_list *dl;
unsigned long flags;
unsigned int i;
int ret;
 
+   /* Prepare the display list. */
+   dl = vsp1_dl_list_get(pipe->output->dlm);
+
/* Count the number of enabled inputs and sort them by Z-order. */
pipe->num_inputs = 0;
 
@@ -484,26 +485,25 @@ void vsp1_du_atomic_flush(struct device *dev)
struct vsp1_rwpf *rpf = to_rwpf(>subdev);
 
if (!pipe->inputs[rpf->entity.index]) {
-   vsp1_dl_list_write(pipe->dl, entity->route->reg,
+   vsp1_dl_list_write(dl, entity->route->reg,
   VI6_DPR_NODE_UNUSED);
continue;
}
}
 
-   vsp1_entity_route_setup(entity, pipe->dl);
+   vsp1_entity_route_setup(entity, dl);
 
if (entity->ops->configure) {
-   entity->ops->configure(entity, pipe, pipe->dl,
+   entity->ops->configure(entity, pipe, dl,
   VSP1_ENTITY_PARAMS_INIT);
-   entity->ops->configure(entity, pipe, pipe->dl,
+   entity->ops->configure(entity, pipe, dl,
   VSP1_ENTITY_PARAMS_RUNTIME);
-   entity->ops->configure(entity, pipe, pipe->dl,
+   entity->ops->configure(entity, pipe, dl,
   VSP1_ENTITY_PARAMS_PARTITION);
}
}
 
-   vsp1_dl_list_commit(pipe->dl);
-   pipe->dl = NULL;
+   vsp1_dl_list_commit(dl);
 
/* Start or stop the pipeline if needed. */
if (!vsp1->drm->num_inputs && pipe->num_inputs) {
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h 
b/drivers/media/platform/vsp1/vsp1_pipe.h
index fff122b4874d..e59bef2653f6 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -108,8 +108,6 @@ struct vsp1_pipeline {
 
struct list_head entities;
 
-   struct vsp1_dl_list *dl;
-
unsigned int div_size;
unsigned int partitions;
struct v4l2_rect partition;
-- 
git-series 0.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html