Re: [PATCH v2 06/14] v4l: vsp1: Add pipe index argument to the VSP-DU API

[Mauro Carvalho Chehab]

Em Fri, 14 Jul 2017 02:04:06 +0300
Laurent Pinchart  escreveu:

> On Thursday 13 Jul 2017 14:16:03 Kieran Bingham wrote:
> > On 26/06/17 19:12, Laurent Pinchart wrote:  
> > > In the H3 ES2.0 SoC the VSP2-DL instance has two connections to DU
> > > channels that need to be configured independently. Extend the VSP-DU API
> > > with a pipeline index to identify which pipeline the caller wants to
> > > operate on.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > >   
> > 
> > A bit of comment merge between this and the next patch but it's minor and
> > not worth the effort to change that ... so I'll happily ignore it if you do
> > :)
> > 
> > Reviewed-by: Kieran Bingham 
> >   
> > > ---
> > > 
> > >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 12 ++--
> > >  drivers/media/platform/vsp1/vsp1_drm.c | 32 +++
> > >  include/media/vsp1.h   | 10 ++
> > >  3 files changed, 34 insertions(+), 20 deletions(-)  
> 
> [snip]
> 
> > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> > > b/drivers/media/platform/vsp1/vsp1_drm.c index c72d021ff820..daaafe7885fa
> > > 100644
> > > --- a/drivers/media/platform/vsp1/vsp1_drm.c
> > > +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> > > @@ -58,21 +58,26 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
> > >  /**
> > >   * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
> > >   * @dev: the VSP device
> > > + * @pipe_index: the DRM pipeline index
> > >   * @cfg: the LIF configuration
> > >   *
> > >   * Configure the output part of VSP DRM pipeline for the given frame
> > >   @cfg.width
> > > - * and @cfg.height. This sets up formats on the BRU source pad, the WPF0
> > > sink
> > > - * and source pads, and the LIF sink pad.
> > > + * and @cfg.height. This sets up formats on the blend unit (BRU or BRS)
> > > source
> > > + * pad, the WPF sink and source pads, and the LIF sink pad.
> > >   *
> > > - * As the media bus code on the BRU source pad is conditioned by the
> > > - * configuration of the BRU sink 0 pad, we also set up the formats on all
> > > BRU
> > > + * The @pipe_index argument selects which DRM pipeline to setup. The
> > > number of
> > > + * available pipelines depend on the VSP instance.
> > > + *
> > > + * As the media bus code on the blend unit source pad is conditioned by
> > > the
> > > + * configuration of its sink 0 pad, we also set up the formats on all
> > > blend unit
> > >   * sinks, even if the configuration will be overwritten later by
> > > - * vsp1_du_setup_rpf(). This ensures that the BRU configuration is set to
> > > a well
> > > - * defined state.
> > > + * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is
> > > set to
> > > + * a well defined state.  
> > 
> > I presume those comment updates for the BRU/ blend-unit configuration are
> > actually for the next patch - but I don't think it matters here - and isn't
> > worth the effort to move the hunks.  
> 
> Too late, I've fixed it already :-) Thanks for pointing it out.

Acked-by: Mauro Carvalho Chehab 


Thanks,
Mauro

Re: [PATCH v2 06/14] v4l: vsp1: Add pipe index argument to the VSP-DU API

[Laurent Pinchart]

Hi Kieran,

On Thursday 13 Jul 2017 14:16:03 Kieran Bingham wrote:
> On 26/06/17 19:12, Laurent Pinchart wrote:
> > In the H3 ES2.0 SoC the VSP2-DL instance has two connections to DU
> > channels that need to be configured independently. Extend the VSP-DU API
> > with a pipeline index to identify which pipeline the caller wants to
> > operate on.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> A bit of comment merge between this and the next patch but it's minor and
> not worth the effort to change that ... so I'll happily ignore it if you do
> :)
> 
> Reviewed-by: Kieran Bingham 
> 
> > ---
> > 
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 12 ++--
> >  drivers/media/platform/vsp1/vsp1_drm.c | 32 +++
> >  include/media/vsp1.h   | 10 ++
> >  3 files changed, 34 insertions(+), 20 deletions(-)

[snip]

> > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> > b/drivers/media/platform/vsp1/vsp1_drm.c index c72d021ff820..daaafe7885fa
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_drm.c
> > +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> > @@ -58,21 +58,26 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
> >  /**
> >   * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
> >   * @dev: the VSP device
> > + * @pipe_index: the DRM pipeline index
> >   * @cfg: the LIF configuration
> >   *
> >   * Configure the output part of VSP DRM pipeline for the given frame
> >   @cfg.width
> > - * and @cfg.height. This sets up formats on the BRU source pad, the WPF0
> > sink
> > - * and source pads, and the LIF sink pad.
> > + * and @cfg.height. This sets up formats on the blend unit (BRU or BRS)
> > source
> > + * pad, the WPF sink and source pads, and the LIF sink pad.
> >   *
> > - * As the media bus code on the BRU source pad is conditioned by the
> > - * configuration of the BRU sink 0 pad, we also set up the formats on all
> > BRU
> > + * The @pipe_index argument selects which DRM pipeline to setup. The
> > number of
> > + * available pipelines depend on the VSP instance.
> > + *
> > + * As the media bus code on the blend unit source pad is conditioned by
> > the
> > + * configuration of its sink 0 pad, we also set up the formats on all
> > blend unit
> >   * sinks, even if the configuration will be overwritten later by
> > - * vsp1_du_setup_rpf(). This ensures that the BRU configuration is set to
> > a well
> > - * defined state.
> > + * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is
> > set to
> > + * a well defined state.
> 
> I presume those comment updates for the BRU/ blend-unit configuration are
> actually for the next patch - but I don't think it matters here - and isn't
> worth the effort to move the hunks.

Too late, I've fixed it already :-) Thanks for pointing it out.

> It all reads OK.
> 
> >   *
> >   * Return 0 on success or a negative error code on failure.
> >   */
> > -int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config
> > *cfg)
> > +int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> > + const struct vsp1_du_lif_config *cfg)
> >  {
> > struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > struct vsp1_pipeline *pipe = >drm->pipe;

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 06/14] v4l: vsp1: Add pipe index argument to the VSP-DU API

[Kieran Bingham]

On 26/06/17 19:12, Laurent Pinchart wrote:
> In the H3 ES2.0 SoC the VSP2-DL instance has two connections to DU
> channels that need to be configured independently. Extend the VSP-DU API
> with a pipeline index to identify which pipeline the caller wants to
> operate on.
> 
> Signed-off-by: Laurent Pinchart 

A bit of comment merge between this and the next patch but it's minor and not
worth the effort to change that ... so I'll happily ignore it if you do :)

Reviewed-by: Kieran Bingham 

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 12 ++--
>  drivers/media/platform/vsp1/vsp1_drm.c | 32 ++--
>  include/media/vsp1.h   | 10 ++
>  3 files changed, 34 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index f870445ebc8d..d46dce054442 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -81,22 +81,22 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
>*/
>   crtc->group->need_restart = true;
>  
> - vsp1_du_setup_lif(crtc->vsp->vsp, );
> + vsp1_du_setup_lif(crtc->vsp->vsp, 0, );
>  }
>  
>  void rcar_du_vsp_disable(struct rcar_du_crtc *crtc)
>  {
> - vsp1_du_setup_lif(crtc->vsp->vsp, NULL);
> + vsp1_du_setup_lif(crtc->vsp->vsp, 0, NULL);
>  }
>  
>  void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
>  {
> - vsp1_du_atomic_begin(crtc->vsp->vsp);
> + vsp1_du_atomic_begin(crtc->vsp->vsp, 0);
>  }
>  
>  void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc)
>  {
> - vsp1_du_atomic_flush(crtc->vsp->vsp);
> + vsp1_du_atomic_flush(crtc->vsp->vsp, 0);
>  }
>  
>  /* Keep the two tables in sync. */
> @@ -192,7 +192,7 @@ static void rcar_du_vsp_plane_setup(struct 
> rcar_du_vsp_plane *plane)
>   }
>   }
>  
> - vsp1_du_atomic_update(plane->vsp->vsp, plane->index, );
> + vsp1_du_atomic_update(plane->vsp->vsp, 0, plane->index, );
>  }
>  
>  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
> @@ -292,7 +292,7 @@ static void rcar_du_vsp_plane_atomic_update(struct 
> drm_plane *plane,
>   if (plane->state->crtc)
>   rcar_du_vsp_plane_setup(rplane);
>   else
> - vsp1_du_atomic_update(rplane->vsp->vsp, rplane->index, NULL);
> + vsp1_du_atomic_update(rplane->vsp->vsp, 0, rplane->index, NULL);
>  }
>  
>  static const struct drm_plane_helper_funcs rcar_du_vsp_plane_helper_funcs = {
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index c72d021ff820..daaafe7885fa 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -58,21 +58,26 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
>  /**
>   * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
>   * @dev: the VSP device
> + * @pipe_index: the DRM pipeline index
>   * @cfg: the LIF configuration
>   *
>   * Configure the output part of VSP DRM pipeline for the given frame 
> @cfg.width
> - * and @cfg.height. This sets up formats on the BRU source pad, the WPF0 sink
> - * and source pads, and the LIF sink pad.
> + * and @cfg.height. This sets up formats on the blend unit (BRU or BRS) 
> source
> + * pad, the WPF sink and source pads, and the LIF sink pad.
>   *
> - * As the media bus code on the BRU source pad is conditioned by the
> - * configuration of the BRU sink 0 pad, we also set up the formats on all BRU
> + * The @pipe_index argument selects which DRM pipeline to setup. The number 
> of
> + * available pipelines depend on the VSP instance.
> + *
> + * As the media bus code on the blend unit source pad is conditioned by the
> + * configuration of its sink 0 pad, we also set up the formats on all blend 
> unit
>   * sinks, even if the configuration will be overwritten later by
> - * vsp1_du_setup_rpf(). This ensures that the BRU configuration is set to a 
> well
> - * defined state.
> + * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is 
> set to
> + * a well defined state.

I presume those comment updates for the BRU/ blend-unit configuration are
actually for the next patch - but I don't think it matters here - and isn't
worth the effort to move the hunks.

It all reads OK.


>   *
>   * Return 0 on success or a negative error code on failure.
>   */
> -int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config 
> *cfg)
> +int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> +   const struct vsp1_du_lif_config *cfg)
>  {
>   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
>   struct vsp1_pipeline *pipe = >drm->pipe;
> @@ -81,6 +86,9 @@ int vsp1_du_setup_lif(struct device *dev, const struct 
> vsp1_du_lif_config *cfg)
>   unsigned int i;
>   int ret;
>  
> + if (pipe_index > 

[PATCH v2 06/14] v4l: vsp1: Add pipe index argument to the VSP-DU API

[Laurent Pinchart]

In the H3 ES2.0 SoC the VSP2-DL instance has two connections to DU
channels that need to be configured independently. Extend the VSP-DU API
with a pipeline index to identify which pipeline the caller wants to
operate on.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 12 ++--
 drivers/media/platform/vsp1/vsp1_drm.c | 32 ++--
 include/media/vsp1.h   | 10 ++
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index f870445ebc8d..d46dce054442 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -81,22 +81,22 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 */
crtc->group->need_restart = true;
 
-   vsp1_du_setup_lif(crtc->vsp->vsp, );
+   vsp1_du_setup_lif(crtc->vsp->vsp, 0, );
 }
 
 void rcar_du_vsp_disable(struct rcar_du_crtc *crtc)
 {
-   vsp1_du_setup_lif(crtc->vsp->vsp, NULL);
+   vsp1_du_setup_lif(crtc->vsp->vsp, 0, NULL);
 }
 
 void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
 {
-   vsp1_du_atomic_begin(crtc->vsp->vsp);
+   vsp1_du_atomic_begin(crtc->vsp->vsp, 0);
 }
 
 void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc)
 {
-   vsp1_du_atomic_flush(crtc->vsp->vsp);
+   vsp1_du_atomic_flush(crtc->vsp->vsp, 0);
 }
 
 /* Keep the two tables in sync. */
@@ -192,7 +192,7 @@ static void rcar_du_vsp_plane_setup(struct 
rcar_du_vsp_plane *plane)
}
}
 
-   vsp1_du_atomic_update(plane->vsp->vsp, plane->index, );
+   vsp1_du_atomic_update(plane->vsp->vsp, 0, plane->index, );
 }
 
 static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
@@ -292,7 +292,7 @@ static void rcar_du_vsp_plane_atomic_update(struct 
drm_plane *plane,
if (plane->state->crtc)
rcar_du_vsp_plane_setup(rplane);
else
-   vsp1_du_atomic_update(rplane->vsp->vsp, rplane->index, NULL);
+   vsp1_du_atomic_update(rplane->vsp->vsp, 0, rplane->index, NULL);
 }
 
 static const struct drm_plane_helper_funcs rcar_du_vsp_plane_helper_funcs = {
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index c72d021ff820..daaafe7885fa 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -58,21 +58,26 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
 /**
  * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
  * @dev: the VSP device
+ * @pipe_index: the DRM pipeline index
  * @cfg: the LIF configuration
  *
  * Configure the output part of VSP DRM pipeline for the given frame @cfg.width
- * and @cfg.height. This sets up formats on the BRU source pad, the WPF0 sink
- * and source pads, and the LIF sink pad.
+ * and @cfg.height. This sets up formats on the blend unit (BRU or BRS) source
+ * pad, the WPF sink and source pads, and the LIF sink pad.
  *
- * As the media bus code on the BRU source pad is conditioned by the
- * configuration of the BRU sink 0 pad, we also set up the formats on all BRU
+ * The @pipe_index argument selects which DRM pipeline to setup. The number of
+ * available pipelines depend on the VSP instance.
+ *
+ * As the media bus code on the blend unit source pad is conditioned by the
+ * configuration of its sink 0 pad, we also set up the formats on all blend 
unit
  * sinks, even if the configuration will be overwritten later by
- * vsp1_du_setup_rpf(). This ensures that the BRU configuration is set to a 
well
- * defined state.
+ * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is set 
to
+ * a well defined state.
  *
  * Return 0 on success or a negative error code on failure.
  */
-int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config *cfg)
+int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
+ const struct vsp1_du_lif_config *cfg)
 {
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
struct vsp1_pipeline *pipe = >drm->pipe;
@@ -81,6 +86,9 @@ int vsp1_du_setup_lif(struct device *dev, const struct 
vsp1_du_lif_config *cfg)
unsigned int i;
int ret;
 
+   if (pipe_index > 0)
+   return -EINVAL;
+
if (!cfg) {
/*
 * NULL configuration means the CRTC is being disabled, stop
@@ -232,8 +240,9 @@ EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
 /**
  * vsp1_du_atomic_begin - Prepare for an atomic update
  * @dev: the VSP device
+ * @pipe_index: the DRM pipeline index
  */
-void vsp1_du_atomic_begin(struct device *dev)
+void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index)
 {
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
struct vsp1_pipeline *pipe = >drm->pipe;
@@ -245,6 +254,7 @@ EXPORT_SYMBOL_GPL(vsp1_du_atomic_begin);
 /**
  * vsp1_du_atomic_update -