Re: [PATCH 07/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function

Hi Kieran,

On Wednesday, 28 March 2018 17:43:13 EEST Kieran Bingham wrote:

On 26/02/18 21:45, Laurent Pinchart wrote:

...

The DRM pipeline setup code used at atomic commit time is similar to the setup code used when enabling the pipeline. Move it to a separate function in order to share it.

Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com

Assuming no hidden secret code addition in this code move that I haven't seen..

Only a minor nit below asking if the function should be pluralised (_inputs, rather than _input)

I'll fix that in v2, thanks.

Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com

...

---

drivers/media/platform/vsp1/vsp1_drm.c | 347 +++++++++++++++++---------------- 1 file changed, 180 insertions(+), 167 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 9a043a915c0b..7bf697ba7969 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -46,6 +46,185 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe,>

• Pipeline Configuration

*/

+/* Setup one RPF and the connected BRU sink pad. */ +static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,

• 		      struct vsp1_pipeline *pipe,
  

• 		      struct vsp1_rwpf *rpf,
  

• 		      unsigned int bru_input)
  

+{

• struct v4l2_subdev_selection sel;
• struct v4l2_subdev_format format;
• const struct v4l2_rect *crop;
• int ret;
• /*

• * Configure the format on the RPF sink pad and propagate it up to the
  

• * BRU sink pad.
  

• */
  

• crop = &vsp1->drm->inputs[rpf->entity.index].crop;
• memset(&format, 0, sizeof(format));
• format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
• format.pad = RWPF_PAD_SINK;
• format.format.width = crop->width + crop->left;
• format.format.height = crop->height + crop->top;
• format.format.code = rpf->fmtinfo->mbus;
• format.format.field = V4L2_FIELD_NONE;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, set_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev,

• "%s: set format %ux%u (%x) on RPF%u sink\n",
  

• __func__, format.format.width, format.format.height,
  

• format.format.code, rpf->entity.index);
  

• memset(&sel, 0, sizeof(sel));
• sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
• sel.pad = RWPF_PAD_SINK;
• sel.target = V4L2_SEL_TGT_CROP;
• sel.r = *crop;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, set_selection, NULL,

• 	       &sel);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev,

• "%s: set selection (%u,%u)/%ux%u on RPF%u sink\n",
  

• __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
  

• rpf->entity.index);
  

• /*

• * RPF source, hardcode the format to ARGB8888 to turn on format
  

• * conversion if needed.
  

• */
  

• format.pad = RWPF_PAD_SOURCE;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, get_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev,

• "%s: got format %ux%u (%x) on RPF%u source\n",
  

• __func__, format.format.width, format.format.height,
  

• format.format.code, rpf->entity.index);
  

• format.format.code = MEDIA_BUS_FMT_ARGB8888_1X32;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, set_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• /* BRU sink, propagate the format from the RPF source. */
• format.pad = bru_input;
• ret = v4l2_subdev_call(&pipe->bru->subdev, pad, set_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",

• __func__, format.format.width, format.format.height,
  

• format.format.code, BRU_NAME(pipe->bru), format.pad);
  

• sel.pad = bru_input;
• sel.target = V4L2_SEL_TGT_COMPOSE;
• sel.r = vsp1->drm->inputs[rpf->entity.index].compose;
• ret = v4l2_subdev_call(&pipe->bru->subdev, pad, set_selection, NULL,

• 	       &sel);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n",

• __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
  

• BRU_NAME(pipe->bru), sel.pad);
  

• return 0;

+}

+static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf) +{

• return vsp1->drm->inputs[rpf->entity.index].zpos;

+}

+/* Setup the input side of the pipeline (RPFs and BRU sink pads). */ +static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,

Minor nit - shouldn't this be _setup_inputs(..) as we could have multiple inputs, and it configures them all.

...

• 			struct vsp1_pipeline *pipe)
  

+{

• struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
• struct vsp1_bru *bru = to_bru(&pipe->bru->subdev);
• unsigned int i;
• int ret;
• /* Count the number of enabled inputs and sort them by Z-order. */
• pipe->num_inputs = 0;
• for (i = 0; i < vsp1->info->rpf_count; ++i) {

• struct vsp1_rwpf *rpf = vsp1->rpf[i];
  

• unsigned int j;
  

• /*
  

•  * Make sure we don't accept more inputs than the hardware can
  

•  * handle. This is a temporary fix to avoid display stall, we
  

•  * need to instead allocate the BRU or BRS to display pipelines
  

•  * dynamically based on the number of planes they each use.
  

•  */
  

• if (pipe->num_inputs >= pipe->bru->source_pad)
  

• 	pipe->inputs[i] = NULL;
  

• if (!pipe->inputs[i])
  

• 	continue;
  

• /* Insert the RPF in the sorted RPFs array. */
  

• for (j = pipe->num_inputs++; j > 0; --j) {
  

• 	if (rpf_zpos(vsp1, inputs[j-1]) <= rpf_zpos(vsp1, rpf))
  

• 		break;
  

• 	inputs[j] = inputs[j-1];
  

• }
  

• inputs[j] = rpf;
  

• }
• /* Setup the RPF input pipeline for every enabled input. */
• for (i = 0; i < pipe->bru->source_pad; ++i) {

• struct vsp1_rwpf *rpf = inputs[i];
  

• if (!rpf) {
  

• 	bru->inputs[i].rpf = NULL;
  

• 	continue;
  

• }
  

• if (!rpf->entity.pipe) {
  

• 	rpf->entity.pipe = pipe;
  

• 	list_add_tail(&rpf->entity.list_pipe, &pipe->entities);
  

• }
  

• bru->inputs[i].rpf = rpf;
  

• rpf->bru_input = i;
  

• rpf->entity.sink = pipe->bru;
  

• rpf->entity.sink_pad = i;
  

• dev_dbg(vsp1->dev, "%s: connecting RPF.%u to %s:%u\n",
  

• 	__func__, rpf->entity.index, BRU_NAME(pipe->bru), i);
  

• ret = vsp1_du_pipeline_setup_rpf(vsp1, pipe, rpf, i);
  

• if (ret < 0) {
  

• 	dev_err(vsp1->dev,
  

• 		"%s: failed to setup RPF.%u\n",
  

• 		__func__, rpf->entity.index);
  

• 	return ret;
  

• }
  

• }
• return 0;

+}

/* Configure all entities in the pipeline. */ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) {

@@ -396,111 +575,6 @@ int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index,> } EXPORT_SYMBOL_GPL(vsp1_du_atomic_update);

-static int vsp1_du_setup_rpf_pipe(struct vsp1_device *vsp1,

• 		  struct vsp1_pipeline *pipe,
  

• 		  struct vsp1_rwpf *rpf, unsigned int bru_input)
  

-{

• struct v4l2_subdev_selection sel;
• struct v4l2_subdev_format format;
• const struct v4l2_rect *crop;
• int ret;
• /*

• * Configure the format on the RPF sink pad and propagate it up to the
  

• * BRU sink pad.
  

• */
  

• crop = &vsp1->drm->inputs[rpf->entity.index].crop;
• memset(&format, 0, sizeof(format));
• format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
• format.pad = RWPF_PAD_SINK;
• format.format.width = crop->width + crop->left;
• format.format.height = crop->height + crop->top;
• format.format.code = rpf->fmtinfo->mbus;
• format.format.field = V4L2_FIELD_NONE;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, set_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev,

• "%s: set format %ux%u (%x) on RPF%u sink\n",
  

• __func__, format.format.width, format.format.height,
  

• format.format.code, rpf->entity.index);
  

• memset(&sel, 0, sizeof(sel));
• sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
• sel.pad = RWPF_PAD_SINK;
• sel.target = V4L2_SEL_TGT_CROP;
• sel.r = *crop;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, set_selection, NULL,

• 	       &sel);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev,

• "%s: set selection (%u,%u)/%ux%u on RPF%u sink\n",
  

• __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
  

• rpf->entity.index);
  

• /*

• * RPF source, hardcode the format to ARGB8888 to turn on format
  

• * conversion if needed.
  

• */
  

• format.pad = RWPF_PAD_SOURCE;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, get_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev,

• "%s: got format %ux%u (%x) on RPF%u source\n",
  

• __func__, format.format.width, format.format.height,
  

• format.format.code, rpf->entity.index);
  

• format.format.code = MEDIA_BUS_FMT_ARGB8888_1X32;
• ret = v4l2_subdev_call(&rpf->entity.subdev, pad, set_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• /* BRU sink, propagate the format from the RPF source. */
• format.pad = bru_input;
• ret = v4l2_subdev_call(&pipe->bru->subdev, pad, set_fmt, NULL,

• 	       &format);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",

• __func__, format.format.width, format.format.height,
  

• format.format.code, BRU_NAME(pipe->bru), format.pad);
  

• sel.pad = bru_input;
• sel.target = V4L2_SEL_TGT_COMPOSE;
• sel.r = vsp1->drm->inputs[rpf->entity.index].compose;
• ret = v4l2_subdev_call(&pipe->bru->subdev, pad, set_selection, NULL,

• 	       &sel);
  

• if (ret < 0)

• return ret;
  

• dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n",

• __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
  

• BRU_NAME(pipe->bru), sel.pad);
  

• return 0;

-}

-static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf) -{

• return vsp1->drm->inputs[rpf->entity.index].zpos;

-}

/**

• vsp1_du_atomic_flush - Commit an atomic update
• @dev: the VSP device

@@ -511,69 +585,8 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index)> struct vsp1_device *vsp1 = dev_get_drvdata(dev); struct vsp1_drm_pipeline *drm_pipe = &vsp1->drm->pipe[pipe_index]; struct vsp1_pipeline *pipe = &drm_pipe->pipe;

• struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
• struct vsp1_bru *bru = to_bru(&pipe->bru->subdev);
• unsigned int i;
• int ret;
• /* Count the number of enabled inputs and sort them by Z-order. */
• pipe->num_inputs = 0;
• for (i = 0; i < vsp1->info->rpf_count; ++i) {

• struct vsp1_rwpf *rpf = vsp1->rpf[i];
  

• unsigned int j;
  

• /*
  

•  * Make sure we don't accept more inputs than the hardware can
  

•  * handle. This is a temporary fix to avoid display stall, we
  

•  * need to instead allocate the BRU or BRS to display pipelines
  

•  * dynamically based on the number of planes they each use.
  

•  */
  

• if (pipe->num_inputs >= pipe->bru->source_pad)
  

• 	pipe->inputs[i] = NULL;
  

• if (!pipe->inputs[i])
  

• 	continue;
  

• /* Insert the RPF in the sorted RPFs array. */
  

• for (j = pipe->num_inputs++; j > 0; --j) {
  

• 	if (rpf_zpos(vsp1, inputs[j-1]) <= rpf_zpos(vsp1, rpf))
  

• 		break;
  

• 	inputs[j] = inputs[j-1];
  

• }
  

• inputs[j] = rpf;
  

• }
• /* Setup the RPF input pipeline for every enabled input. */
• for (i = 0; i < pipe->bru->source_pad; ++i) {

• struct vsp1_rwpf *rpf = inputs[i];
  

• if (!rpf) {
  

• 	bru->inputs[i].rpf = NULL;
  

• 	continue;
  

• }
  

• if (!rpf->entity.pipe) {
  

• 	rpf->entity.pipe = pipe;
  

• 	list_add_tail(&rpf->entity.list_pipe, &pipe->entities);
  

• }
  

• bru->inputs[i].rpf = rpf;
  

• rpf->bru_input = i;
  

• rpf->entity.sink = pipe->bru;
  

• rpf->entity.sink_pad = i;
  

• dev_dbg(vsp1->dev, "%s: connecting RPF.%u to %s:%u\n",
  

• 	__func__, rpf->entity.index, BRU_NAME(pipe->bru), i);
  

• ret = vsp1_du_setup_rpf_pipe(vsp1, pipe, rpf, i);
  

• if (ret < 0)
  

• 	dev_err(vsp1->dev,
  

• 		"%s: failed to setup RPF.%u\n",
  

• 		__func__, rpf->entity.index);
  

• }

• vsp1_du_pipeline_setup_input(vsp1, pipe);

  vsp1_du_pipeline_configure(pipe);

} EXPORT_SYMBOL_GPL(vsp1_du_atomic_flush);