[RFC PATCH 0/3] VSP1/DU atomic interface changes

[RFC PATCH 0/3] VSP1/DU atomic interface changes

[Kieran Bingham]

As part of the ongoing DU group refactoring it became apparent that we need to
split the configuration of the VSP to allow fine grain control of setting the
VSP1 mode configuration and enabling/disabling of the pipeline.

To split the mode configuration and the pipeline enablement, we add three new
calls:

 - vsp1_du_atomic_modeset()
 - vsp1_du_atomic_enable()
 - vsp1_du_atomic_disable()

To support the cross-component API, the new interface is added in [patch 1/3],
including an implementation of vsp1_du_setup_lif() to support the transition.

The DRM usage is adapted in [patch 2/3], before the call is removed entirely in
[patch 3/3]

Whilst these patches are independent and could be reviewed separately, they are
not expected to be integrated until the associated group rework is completed.

Kieran Bingham (3):
  media: vsp1: drm: Split vsp1_du_setup_lif()
  drm: rcar-du: Convert to the new VSP atomic API
  media: vsp1: drm: Remove vsp1_du_setup_lif()

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c |   4 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c  |  21 ++-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h  |   2 +
 drivers/media/platform/vsp1/vsp1_drm.c | 188 ++++++++++++++++---------
 include/media/vsp1.h                   |  26 ++--
 5 files changed, 159 insertions(+), 82 deletions(-)

-- 
2.20.1

[PATCH 1/3] media: vsp1: drm: Split vsp1_du_setup_lif()

[Kieran Bingham]

Break vsp1_du_setup_lif() into components more suited to the DRM Atomic
API. The existing vsp1_du_setup_lif() API call is maintained as it is
still used from the DU.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_drm.c | 233 ++++++++++++++++++-------
 include/media/vsp1.h                   |  32 +++-
 2 files changed, 199 insertions(+), 66 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index a4a45d68a6ef..ce5c0780680f 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -616,18 +616,15 @@ int vsp1_du_init(struct device *dev)
 EXPORT_SYMBOL_GPL(vsp1_du_init);
 
 /**
- * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
+ * vsp1_du_atomic_modeset - Configure the mode as part of an atomic update
  * @dev: the VSP device
  * @pipe_index: the DRM pipeline index
- * @cfg: the LIF configuration
+ * @cfg: the mode configuration
  *
  * Configure the output part of VSP DRM pipeline for the given frame @cfg.width
  * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink and
  * source pads, and the LIF sink pad.
  *
- * 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
@@ -635,15 +632,14 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
  * a well defined state.
  *
  * Return 0 on success or a negative error code on failure.
+ *
  */
-int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
-		      const struct vsp1_du_lif_config *cfg)
+int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index,
+			   const struct vsp1_du_modeset_config *cfg)
 {
 	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
 	struct vsp1_drm_pipeline *drm_pipe;
 	struct vsp1_pipeline *pipe;
-	unsigned long flags;
-	unsigned int i;
 	int ret;
 
 	if (pipe_index >= vsp1->info->lif_count)
@@ -652,60 +648,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
 	drm_pipe = &vsp1->drm->pipe[pipe_index];
 	pipe = &drm_pipe->pipe;
 
-	if (!cfg) {
-		struct vsp1_brx *brx;
-
-		mutex_lock(&vsp1->drm->lock);
-
-		brx = to_brx(&pipe->brx->subdev);
-
-		/*
-		 * NULL configuration means the CRTC is being disabled, stop
-		 * the pipeline and turn the light off.
-		 */
-		ret = vsp1_pipeline_stop(pipe);
-		if (ret == -ETIMEDOUT)
-			dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
-
-		for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
-			struct vsp1_rwpf *rpf = pipe->inputs[i];
-
-			if (!rpf)
-				continue;
-
-			/*
-			 * Remove the RPF from the pipe and the list of BRx
-			 * inputs.
-			 */
-			WARN_ON(!rpf->entity.pipe);
-			rpf->entity.pipe = NULL;
-			list_del(&rpf->entity.list_pipe);
-			pipe->inputs[i] = NULL;
-
-			brx->inputs[rpf->brx_input].rpf = NULL;
-		}
-
-		drm_pipe->du_complete = NULL;
-		pipe->num_inputs = 0;
-
-		dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
-			__func__, pipe->lif->index,
-			BRX_NAME(pipe->brx));
-
-		list_del(&pipe->brx->list_pipe);
-		pipe->brx->pipe = NULL;
-		pipe->brx = NULL;
-
-		mutex_unlock(&vsp1->drm->lock);
-
-		vsp1_dlm_reset(pipe->output->dlm);
-		vsp1_device_put(vsp1);
-
-		dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__);
-
-		return 0;
-	}
-
 	drm_pipe->width = cfg->width;
 	drm_pipe->height = cfg->height;
 	pipe->interlaced = cfg->interlaced;
@@ -722,8 +664,43 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
 		goto unlock;
 
 	ret = vsp1_du_pipeline_setup_output(vsp1, pipe);
-	if (ret < 0)
-		goto unlock;
+
+unlock:
+	mutex_unlock(&vsp1->drm->lock);
+
+	return ret;
+}
+
+/**
+ * vsp1_du_atomic_enable - Enable and start a DU pipeline
+ * @dev: the VSP device
+ * @pipe_index: the DRM pipeline index
+ * @cfg: the enablement configuration
+ *
+ * The @pipe_index argument selects which DRM pipeline to enable. The number of
+ * available pipelines depend on the VSP instance.
+ *
+ * The configuration passes a callback function to register notification of
+ * frame completion events.
+ *
+ * Return 0 on success or a negative error code on failure.
+ */
+int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index,
+			  const struct vsp1_du_enable_config *cfg)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct vsp1_drm_pipeline *drm_pipe;
+	struct vsp1_pipeline *pipe;
+	unsigned long flags;
+	int ret;
+
+	if (pipe_index >= vsp1->info->lif_count)
+		return -EINVAL;
+
+	drm_pipe = &vsp1->drm->pipe[pipe_index];
+	pipe = &drm_pipe->pipe;
+
+	mutex_lock(&vsp1->drm->lock);
 
 	/* Enable the VSP1. */
 	ret = vsp1_device_get(vsp1);
@@ -758,6 +735,132 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
 	dev_dbg(vsp1->dev, "%s: pipeline enabled\n", __func__);
 
 	return 0;
+
+}
+EXPORT_SYMBOL_GPL(vsp1_du_atomic_enable);
+
+
+/**
+ * vsp1_du_atomic_disable - Disable and stop a DU pipeline
+ * @dev: the VSP device
+ * @pipe_index: the DRM pipeline index
+ *
+ * The @pipe_index argument selects which DRM pipeline to disable. The number
+ * of available pipelines depend on the VSP instance.
+ *
+ * Return 0 on success or a negative error code on failure.
+ */
+int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct vsp1_drm_pipeline *drm_pipe;
+	struct vsp1_pipeline *pipe;
+	struct vsp1_brx *brx;
+	unsigned int i;
+	int ret;
+
+	if (pipe_index >= vsp1->info->lif_count)
+		return -EINVAL;
+
+	drm_pipe = &vsp1->drm->pipe[pipe_index];
+	pipe = &drm_pipe->pipe;
+
+	mutex_lock(&vsp1->drm->lock);
+
+	brx = to_brx(&pipe->brx->subdev);
+
+	/*
+	 * NULL configuration means the CRTC is being disabled, stop
+	 * the pipeline and turn the light off.
+	 */
+	ret = vsp1_pipeline_stop(pipe);
+	if (ret == -ETIMEDOUT)
+		dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
+
+	for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
+		struct vsp1_rwpf *rpf = pipe->inputs[i];
+
+		if (!rpf)
+			continue;
+
+		/*
+		 * Remove the RPF from the pipe and the list of BRx
+		 * inputs.
+		 */
+		WARN_ON(!rpf->entity.pipe);
+		rpf->entity.pipe = NULL;
+		list_del(&rpf->entity.list_pipe);
+		pipe->inputs[i] = NULL;
+
+		brx->inputs[rpf->brx_input].rpf = NULL;
+	}
+
+	drm_pipe->du_complete = NULL;
+	pipe->num_inputs = 0;
+
+	dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
+		__func__, pipe->lif->index,
+		BRX_NAME(pipe->brx));
+
+	list_del(&pipe->brx->list_pipe);
+	pipe->brx->pipe = NULL;
+	pipe->brx = NULL;
+
+	mutex_unlock(&vsp1->drm->lock);
+
+	vsp1_dlm_reset(pipe->output->dlm);
+	vsp1_device_put(vsp1);
+
+	dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable);
+
+/**
+ * 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 BRx source pad, the WPF sink and
+ * source pads, and the LIF sink pad.
+ *
+ * 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 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, unsigned int pipe_index,
+		      const struct vsp1_du_lif_config *cfg)
+{
+	int ret;
+
+	struct vsp1_du_modeset_config modes = {
+		.width = cfg->width,
+		.height = cfg->height,
+		.interlaced = cfg->interlaced,
+	};
+	struct vsp1_du_enable_config enable = {
+		.callback = cfg->callback,
+		.callback_data = cfg->callback_data,
+	};
+
+	if (!cfg)
+		return vsp1_du_atomic_disable(dev, pipe_index);
+
+	ret = vsp1_du_atomic_modeset(dev, pipe_index, &modes);
+	if (ret)
+		return ret;
+
+	return vsp1_du_atomic_enable(dev, pipe_index, &enable);
 }
 EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
 
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index cc1b0d42ce95..13f5a1c4d45a 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -21,7 +21,7 @@ int vsp1_du_init(struct device *dev);
 #define VSP1_DU_STATUS_WRITEBACK	BIT(1)
 
 /**
- * struct vsp1_du_lif_config - VSP LIF configuration
+ * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated
  * @width: output frame width
  * @height: output frame height
  * @interlaced: true for interlaced pipelines
@@ -42,6 +42,30 @@ struct vsp1_du_lif_config {
 int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
 		      const struct vsp1_du_lif_config *cfg);
 
+/**
+ * struct vsp1_du_modeset_config - VSP LIF Mode configuration
+ * @width: output frame width
+ * @height: output frame height
+ * @interlaced: true for interlaced pipelines
+ */
+struct vsp1_du_modeset_config {
+	unsigned int width;
+	unsigned int height;
+	bool interlaced;
+};
+
+/**
+ * struct vsp1_du_enable_config - VSP enable configuration
+ * @callback: frame completion callback function (optional). When a callback
+ *	      is provided, the VSP driver guarantees that it will be called once
+ *	      and only once for each vsp1_du_atomic_flush() call.
+ * @callback_data: data to be passed to the frame completion callback
+ */
+struct vsp1_du_enable_config {
+	void (*callback)(void *data, unsigned int status, u32 crc);
+	void *callback_data;
+};
+
 /**
  * struct vsp1_du_atomic_config - VSP atomic configuration parameters
  * @pixelformat: plane pixel format (V4L2 4CC)
@@ -106,6 +130,12 @@ struct vsp1_du_atomic_pipe_config {
 	struct vsp1_du_writeback_config writeback;
 };
 
+
+int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index,
+		    const struct vsp1_du_modeset_config *cfg);
+int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index,
+		   const struct vsp1_du_enable_config *cfg);
+int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index);
 void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index);
 int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index,
 			  unsigned int rpf,
-- 
2.20.1

[PATCH 2/3] drm: rcar-du: Convert to the new VSP atomic API

[Kieran Bingham]

The configuration API between the VSP and the DU has been updated to
provide finer grain control over modesetting, and enablement.

Split rcar_du_vsp_enable() into rcar_du_vsp_modeset() and
rcar_du_vsp_enable() accordingly, and update each function to use the
new VSP API.

There are no further users of the deprecated vsp1_du_setup_lif() which
can now be removed.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  4 +++-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 21 +++++++++++++++------
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h  |  2 ++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 2da46e3dc4ae..cccd6fe85749 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -492,8 +492,10 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc)
 	rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
 
 	/* Enable the VSP compositor. */
-	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
+	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
+		rcar_du_vsp_modeset(rcrtc);
 		rcar_du_vsp_enable(rcrtc);
+	}
 
 	/* Turn vertical blanking interrupt reporting on. */
 	drm_crtc_vblank_on(&rcrtc->crtc);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 5e4faf258c31..c170427fcad9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -44,16 +44,14 @@ static void rcar_du_vsp_complete(void *private, unsigned int status, u32 crc)
 	drm_crtc_add_crc_entry(&crtc->crtc, false, 0, &crc);
 }
 
-void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
+void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc)
 {
 	const struct drm_display_mode *mode = &crtc->crtc.state->adjusted_mode;
 	struct rcar_du_device *rcdu = crtc->dev;
-	struct vsp1_du_lif_config cfg = {
+	struct vsp1_du_modeset_config cfg = {
 		.width = mode->hdisplay,
 		.height = mode->vdisplay,
 		.interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE,
-		.callback = rcar_du_vsp_complete,
-		.callback_data = crtc,
 	};
 	struct rcar_du_plane_state state = {
 		.state = {
@@ -90,12 +88,23 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 	 */
 	crtc->group->need_restart = true;
 
-	vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);
+
+	vsp1_du_atomic_modeset(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);
+}
+
+void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
+{
+	struct vsp1_du_enable_config cfg = {
+		.callback = rcar_du_vsp_complete,
+		.callback_data = crtc,
+	};
+
+	vsp1_du_atomic_enable(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);
 }
 
 void rcar_du_vsp_disable(struct rcar_du_crtc *crtc)
 {
-	vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, NULL);
+	vsp1_du_atomic_disable(crtc->vsp->vsp, crtc->vsp_pipe);
 }
 
 void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index 9b4724159378..a6f6bb4690f2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -58,6 +58,7 @@ to_rcar_vsp_plane_state(struct drm_plane_state *state)
 #ifdef CONFIG_DRM_RCAR_VSP
 int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
 		     unsigned int crtcs);
+void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_enable(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_disable(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
@@ -73,6 +74,7 @@ static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp,
 {
 	return -ENXIO;
 }
+static inlinc void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
-- 
2.20.1

[PATCH 3/3] media: vsp1: drm: Remove vsp1_du_setup_lif()

[Kieran Bingham]

The vsp1_du_setup_lif() function is deprecated, and the users have been
removed. Remove the implementation and the associated configuration
structure.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_drm.c | 47 --------------------------
 include/media/vsp1.h                   | 22 ------------
 2 files changed, 69 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index ce5c0780680f..12f76344bdec 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -817,53 +817,6 @@ int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index)
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable);
 
-/**
- * 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 BRx source pad, the WPF sink and
- * source pads, and the LIF sink pad.
- *
- * 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 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, unsigned int pipe_index,
-		      const struct vsp1_du_lif_config *cfg)
-{
-	int ret;
-
-	struct vsp1_du_modeset_config modes = {
-		.width = cfg->width,
-		.height = cfg->height,
-		.interlaced = cfg->interlaced,
-	};
-	struct vsp1_du_enable_config enable = {
-		.callback = cfg->callback,
-		.callback_data = cfg->callback_data,
-	};
-
-	if (!cfg)
-		return vsp1_du_atomic_disable(dev, pipe_index);
-
-	ret = vsp1_du_atomic_modeset(dev, pipe_index, &modes);
-	if (ret)
-		return ret;
-
-	return vsp1_du_atomic_enable(dev, pipe_index, &enable);
-}
-EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
-
 /**
  * vsp1_du_atomic_begin - Prepare for an atomic update
  * @dev: the VSP device
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 13f5a1c4d45a..bc0a26d33d9a 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -20,28 +20,6 @@ int vsp1_du_init(struct device *dev);
 #define VSP1_DU_STATUS_COMPLETE		BIT(0)
 #define VSP1_DU_STATUS_WRITEBACK	BIT(1)
 
-/**
- * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated
- * @width: output frame width
- * @height: output frame height
- * @interlaced: true for interlaced pipelines
- * @callback: frame completion callback function (optional). When a callback
- *	      is provided, the VSP driver guarantees that it will be called once
- *	      and only once for each vsp1_du_atomic_flush() call.
- * @callback_data: data to be passed to the frame completion callback
- */
-struct vsp1_du_lif_config {
-	unsigned int width;
-	unsigned int height;
-	bool interlaced;
-
-	void (*callback)(void *data, unsigned int status, u32 crc);
-	void *callback_data;
-};
-
-int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
-		      const struct vsp1_du_lif_config *cfg);
-
 /**
  * struct vsp1_du_modeset_config - VSP LIF Mode configuration
  * @width: output frame width
-- 
2.20.1

Re: [PATCH 1/3] media: vsp1: drm: Split vsp1_du_setup_lif()

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, May 17, 2019 at 11:31:41PM +0100, Kieran Bingham wrote:
> Break vsp1_du_setup_lif() into components more suited to the DRM Atomic
> API. The existing vsp1_du_setup_lif() API call is maintained as it is
> still used from the DU.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 233 ++++++++++++++++++-------
>  include/media/vsp1.h                   |  32 +++-
>  2 files changed, 199 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
> index a4a45d68a6ef..ce5c0780680f 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -616,18 +616,15 @@ int vsp1_du_init(struct device *dev)
>  EXPORT_SYMBOL_GPL(vsp1_du_init);
>  
>  /**
> - * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
> + * vsp1_du_atomic_modeset - Configure the mode as part of an atomic update
>   * @dev: the VSP device
>   * @pipe_index: the DRM pipeline index
> - * @cfg: the LIF configuration
> + * @cfg: the mode configuration
>   *
>   * Configure the output part of VSP DRM pipeline for the given frame @cfg.width
>   * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink and
>   * source pads, and the LIF sink pad.
>   *
> - * The @pipe_index argument selects which DRM pipeline to setup. The number of
> - * available pipelines depend on the VSP instance.
> - *

Shouldn't this paragraph be preserved, as the function keeps its
pipe_index argument ?

>   * 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
> @@ -635,15 +632,14 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
>   * a well defined state.
>   *
>   * Return 0 on success or a negative error code on failure.
> + *

Not needed.

>   */
> -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> -		      const struct vsp1_du_lif_config *cfg)
> +int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index,
> +			   const struct vsp1_du_modeset_config *cfg)
>  {
>  	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
>  	struct vsp1_drm_pipeline *drm_pipe;
>  	struct vsp1_pipeline *pipe;
> -	unsigned long flags;
> -	unsigned int i;
>  	int ret;
>  
>  	if (pipe_index >= vsp1->info->lif_count)
> @@ -652,60 +648,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
>  	drm_pipe = &vsp1->drm->pipe[pipe_index];
>  	pipe = &drm_pipe->pipe;
>  
> -	if (!cfg) {
> -		struct vsp1_brx *brx;
> -
> -		mutex_lock(&vsp1->drm->lock);
> -
> -		brx = to_brx(&pipe->brx->subdev);
> -
> -		/*
> -		 * NULL configuration means the CRTC is being disabled, stop
> -		 * the pipeline and turn the light off.
> -		 */
> -		ret = vsp1_pipeline_stop(pipe);
> -		if (ret == -ETIMEDOUT)
> -			dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
> -
> -		for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
> -			struct vsp1_rwpf *rpf = pipe->inputs[i];
> -
> -			if (!rpf)
> -				continue;
> -
> -			/*
> -			 * Remove the RPF from the pipe and the list of BRx
> -			 * inputs.
> -			 */
> -			WARN_ON(!rpf->entity.pipe);
> -			rpf->entity.pipe = NULL;
> -			list_del(&rpf->entity.list_pipe);
> -			pipe->inputs[i] = NULL;
> -
> -			brx->inputs[rpf->brx_input].rpf = NULL;
> -		}
> -
> -		drm_pipe->du_complete = NULL;
> -		pipe->num_inputs = 0;
> -
> -		dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
> -			__func__, pipe->lif->index,
> -			BRX_NAME(pipe->brx));
> -
> -		list_del(&pipe->brx->list_pipe);
> -		pipe->brx->pipe = NULL;
> -		pipe->brx = NULL;
> -
> -		mutex_unlock(&vsp1->drm->lock);
> -
> -		vsp1_dlm_reset(pipe->output->dlm);
> -		vsp1_device_put(vsp1);
> -
> -		dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__);
> -
> -		return 0;
> -	}
> -
>  	drm_pipe->width = cfg->width;
>  	drm_pipe->height = cfg->height;
>  	pipe->interlaced = cfg->interlaced;
> @@ -722,8 +664,43 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
>  		goto unlock;
>  
>  	ret = vsp1_du_pipeline_setup_output(vsp1, pipe);
> -	if (ret < 0)
> -		goto unlock;
> +
> +unlock:
> +	mutex_unlock(&vsp1->drm->lock);
> +
> +	return ret;
> +}
> +
> +/**
> + * vsp1_du_atomic_enable - Enable and start a DU pipeline
> + * @dev: the VSP device
> + * @pipe_index: the DRM pipeline index
> + * @cfg: the enablement configuration

Not a big fan of this description or of the vsp1_du_enable_config name,
but I can't really think of a better alternative. vsp1_du_callback would
be an option, but it would prevent us from adding new fields not related
to the callback.

> + *
> + * The @pipe_index argument selects which DRM pipeline to enable. The number of
> + * available pipelines depend on the VSP instance.
> + *
> + * The configuration passes a callback function to register notification of
> + * frame completion events.
> + *
> + * Return 0 on success or a negative error code on failure.
> + */
> +int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index,
> +			  const struct vsp1_du_enable_config *cfg)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_drm_pipeline *drm_pipe;
> +	struct vsp1_pipeline *pipe;
> +	unsigned long flags;
> +	int ret;
> +
> +	if (pipe_index >= vsp1->info->lif_count)
> +		return -EINVAL;
> +
> +	drm_pipe = &vsp1->drm->pipe[pipe_index];
> +	pipe = &drm_pipe->pipe;
> +
> +	mutex_lock(&vsp1->drm->lock);
>  
>  	/* Enable the VSP1. */
>  	ret = vsp1_device_get(vsp1);
> @@ -758,6 +735,132 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
>  	dev_dbg(vsp1->dev, "%s: pipeline enabled\n", __func__);
>  
>  	return 0;
> +
> +}
> +EXPORT_SYMBOL_GPL(vsp1_du_atomic_enable);
> +
> +

Extra blank line.

> +/**
> + * vsp1_du_atomic_disable - Disable and stop a DU pipeline
> + * @dev: the VSP device
> + * @pipe_index: the DRM pipeline index
> + *
> + * The @pipe_index argument selects which DRM pipeline to disable. The number
> + * of available pipelines depend on the VSP instance.
> + *
> + * Return 0 on success or a negative error code on failure.
> + */
> +int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_drm_pipeline *drm_pipe;
> +	struct vsp1_pipeline *pipe;
> +	struct vsp1_brx *brx;
> +	unsigned int i;
> +	int ret;
> +
> +	if (pipe_index >= vsp1->info->lif_count)
> +		return -EINVAL;
> +
> +	drm_pipe = &vsp1->drm->pipe[pipe_index];
> +	pipe = &drm_pipe->pipe;
> +
> +	mutex_lock(&vsp1->drm->lock);
> +
> +	brx = to_brx(&pipe->brx->subdev);
> +
> +	/*
> +	 * NULL configuration means the CRTC is being disabled, stop
> +	 * the pipeline and turn the light off.
> +	 */

I think you can now drop this comment, or at least the first part of the
sentence.

> +	ret = vsp1_pipeline_stop(pipe);
> +	if (ret == -ETIMEDOUT)
> +		dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
> +
> +	for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
> +		struct vsp1_rwpf *rpf = pipe->inputs[i];
> +
> +		if (!rpf)
> +			continue;
> +
> +		/*
> +		 * Remove the RPF from the pipe and the list of BRx
> +		 * inputs.
> +		 */

This now holds on a single line.

> +		WARN_ON(!rpf->entity.pipe);
> +		rpf->entity.pipe = NULL;
> +		list_del(&rpf->entity.list_pipe);
> +		pipe->inputs[i] = NULL;
> +
> +		brx->inputs[rpf->brx_input].rpf = NULL;
> +	}
> +
> +	drm_pipe->du_complete = NULL;
> +	pipe->num_inputs = 0;
> +
> +	dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
> +		__func__, pipe->lif->index,
> +		BRX_NAME(pipe->brx));

And this can hold on two lines.

> +
> +	list_del(&pipe->brx->list_pipe);
> +	pipe->brx->pipe = NULL;
> +	pipe->brx = NULL;
> +
> +	mutex_unlock(&vsp1->drm->lock);
> +
> +	vsp1_dlm_reset(pipe->output->dlm);
> +	vsp1_device_put(vsp1);
> +
> +	dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable);
> +
> +/**
> + * 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 BRx source pad, the WPF sink and
> + * source pads, and the LIF sink pad.
> + *
> + * 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 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, unsigned int pipe_index,
> +		      const struct vsp1_du_lif_config *cfg)
> +{
> +	int ret;
> +
> +	struct vsp1_du_modeset_config modes = {
> +		.width = cfg->width,
> +		.height = cfg->height,
> +		.interlaced = cfg->interlaced,
> +	};
> +	struct vsp1_du_enable_config enable = {
> +		.callback = cfg->callback,
> +		.callback_data = cfg->callback_data,
> +	};
> +
> +	if (!cfg)
> +		return vsp1_du_atomic_disable(dev, pipe_index);
> +
> +	ret = vsp1_du_atomic_modeset(dev, pipe_index, &modes);
> +	if (ret)
> +		return ret;
> +
> +	return vsp1_du_atomic_enable(dev, pipe_index, &enable);
>  }
>  EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
>  
> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> index cc1b0d42ce95..13f5a1c4d45a 100644
> --- a/include/media/vsp1.h
> +++ b/include/media/vsp1.h
> @@ -21,7 +21,7 @@ int vsp1_du_init(struct device *dev);
>  #define VSP1_DU_STATUS_WRITEBACK	BIT(1)
>  
>  /**
> - * struct vsp1_du_lif_config - VSP LIF configuration
> + * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated
>   * @width: output frame width
>   * @height: output frame height
>   * @interlaced: true for interlaced pipelines
> @@ -42,6 +42,30 @@ struct vsp1_du_lif_config {
>  int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
>  		      const struct vsp1_du_lif_config *cfg);
>  
> +/**
> + * struct vsp1_du_modeset_config - VSP LIF Mode configuration

Maybe "VSP display mode configuration" as it's not restricted to the LIF
?

> + * @width: output frame width
> + * @height: output frame height
> + * @interlaced: true for interlaced pipelines
> + */
> +struct vsp1_du_modeset_config {
> +	unsigned int width;
> +	unsigned int height;
> +	bool interlaced;
> +};
> +
> +/**
> + * struct vsp1_du_enable_config - VSP enable configuration
> + * @callback: frame completion callback function (optional). When a callback
> + *	      is provided, the VSP driver guarantees that it will be called once
> + *	      and only once for each vsp1_du_atomic_flush() call.
> + * @callback_data: data to be passed to the frame completion callback
> + */
> +struct vsp1_du_enable_config {
> +	void (*callback)(void *data, unsigned int status, u32 crc);
> +	void *callback_data;
> +};
> +
>  /**
>   * struct vsp1_du_atomic_config - VSP atomic configuration parameters
>   * @pixelformat: plane pixel format (V4L2 4CC)
> @@ -106,6 +130,12 @@ struct vsp1_du_atomic_pipe_config {
>  	struct vsp1_du_writeback_config writeback;
>  };
>  
> +
> +int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index,
> +		    const struct vsp1_du_modeset_config *cfg);
> +int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index,
> +		   const struct vsp1_du_enable_config *cfg);
> +int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index);
>  void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index);
>  int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index,
>  			  unsigned int rpf,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] drm: rcar-du: Convert to the new VSP atomic API

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, May 17, 2019 at 11:31:42PM +0100, Kieran Bingham wrote:
> The configuration API between the VSP and the DU has been updated to
> provide finer grain control over modesetting, and enablement.
> 
> Split rcar_du_vsp_enable() into rcar_du_vsp_modeset() and
> rcar_du_vsp_enable() accordingly, and update each function to use the
> new VSP API.
> 
> There are no further users of the deprecated vsp1_du_setup_lif() which
> can now be removed.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  4 +++-
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 21 +++++++++++++++------
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.h  |  2 ++
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 2da46e3dc4ae..cccd6fe85749 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -492,8 +492,10 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc)
>  	rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>  
>  	/* Enable the VSP compositor. */
> -	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
> +	if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
> +		rcar_du_vsp_modeset(rcrtc);
>  		rcar_du_vsp_enable(rcrtc);
> +	}
>  
>  	/* Turn vertical blanking interrupt reporting on. */
>  	drm_crtc_vblank_on(&rcrtc->crtc);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index 5e4faf258c31..c170427fcad9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -44,16 +44,14 @@ static void rcar_du_vsp_complete(void *private, unsigned int status, u32 crc)
>  	drm_crtc_add_crc_entry(&crtc->crtc, false, 0, &crc);
>  }
>  
> -void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
> +void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc)
>  {
>  	const struct drm_display_mode *mode = &crtc->crtc.state->adjusted_mode;
>  	struct rcar_du_device *rcdu = crtc->dev;
> -	struct vsp1_du_lif_config cfg = {
> +	struct vsp1_du_modeset_config cfg = {
>  		.width = mode->hdisplay,
>  		.height = mode->vdisplay,
>  		.interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE,
> -		.callback = rcar_du_vsp_complete,
> -		.callback_data = crtc,
>  	};
>  	struct rcar_du_plane_state state = {
>  		.state = {
> @@ -90,12 +88,23 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
>  	 */
>  	crtc->group->need_restart = true;
>  
> -	vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);
> +

Extra blank line.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	vsp1_du_atomic_modeset(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);
> +}
> +
> +void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
> +{
> +	struct vsp1_du_enable_config cfg = {
> +		.callback = rcar_du_vsp_complete,
> +		.callback_data = crtc,
> +	};
> +
> +	vsp1_du_atomic_enable(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);
>  }
>  
>  void rcar_du_vsp_disable(struct rcar_du_crtc *crtc)
>  {
> -	vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, NULL);
> +	vsp1_du_atomic_disable(crtc->vsp->vsp, crtc->vsp_pipe);
>  }
>  
>  void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> index 9b4724159378..a6f6bb4690f2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> @@ -58,6 +58,7 @@ to_rcar_vsp_plane_state(struct drm_plane_state *state)
>  #ifdef CONFIG_DRM_RCAR_VSP
>  int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
>  		     unsigned int crtcs);
> +void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc);
>  void rcar_du_vsp_enable(struct rcar_du_crtc *crtc);
>  void rcar_du_vsp_disable(struct rcar_du_crtc *crtc);
>  void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
> @@ -73,6 +74,7 @@ static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp,
>  {
>  	return -ENXIO;
>  }
> +static inlinc void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc) { };
>  static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
>  static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
>  static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] media: vsp1: drm: Remove vsp1_du_setup_lif()

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, May 17, 2019 at 11:31:43PM +0100, Kieran Bingham wrote:
> The vsp1_du_setup_lif() function is deprecated, and the users have been
> removed. Remove the implementation and the associated configuration
> structure.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 47 --------------------------
>  include/media/vsp1.h                   | 22 ------------
>  2 files changed, 69 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
> index ce5c0780680f..12f76344bdec 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -817,53 +817,6 @@ int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index)
>  }
>  EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable);
>  
> -/**
> - * 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 BRx source pad, the WPF sink and
> - * source pads, and the LIF sink pad.
> - *
> - * 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 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, unsigned int pipe_index,
> -		      const struct vsp1_du_lif_config *cfg)
> -{
> -	int ret;
> -
> -	struct vsp1_du_modeset_config modes = {
> -		.width = cfg->width,
> -		.height = cfg->height,
> -		.interlaced = cfg->interlaced,
> -	};
> -	struct vsp1_du_enable_config enable = {
> -		.callback = cfg->callback,
> -		.callback_data = cfg->callback_data,
> -	};
> -
> -	if (!cfg)
> -		return vsp1_du_atomic_disable(dev, pipe_index);
> -
> -	ret = vsp1_du_atomic_modeset(dev, pipe_index, &modes);
> -	if (ret)
> -		return ret;
> -
> -	return vsp1_du_atomic_enable(dev, pipe_index, &enable);
> -}
> -EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
> -
>  /**
>   * vsp1_du_atomic_begin - Prepare for an atomic update
>   * @dev: the VSP device
> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> index 13f5a1c4d45a..bc0a26d33d9a 100644
> --- a/include/media/vsp1.h
> +++ b/include/media/vsp1.h
> @@ -20,28 +20,6 @@ int vsp1_du_init(struct device *dev);
>  #define VSP1_DU_STATUS_COMPLETE		BIT(0)
>  #define VSP1_DU_STATUS_WRITEBACK	BIT(1)
>  
> -/**
> - * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated
> - * @width: output frame width
> - * @height: output frame height
> - * @interlaced: true for interlaced pipelines
> - * @callback: frame completion callback function (optional). When a callback
> - *	      is provided, the VSP driver guarantees that it will be called once
> - *	      and only once for each vsp1_du_atomic_flush() call.
> - * @callback_data: data to be passed to the frame completion callback
> - */
> -struct vsp1_du_lif_config {
> -	unsigned int width;
> -	unsigned int height;
> -	bool interlaced;
> -
> -	void (*callback)(void *data, unsigned int status, u32 crc);
> -	void *callback_data;
> -};
> -
> -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> -		      const struct vsp1_du_lif_config *cfg);
> -
>  /**
>   * struct vsp1_du_modeset_config - VSP LIF Mode configuration
>   * @width: output frame width

-- 
Regards,

Laurent Pinchart