* [PATCH 01/13] v4l: vsp1: Prevent pipelines from running when not streaming
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
@ 2016-09-13 23:16 ` Laurent Pinchart
2016-09-13 23:16 ` [PATCH 02/13] v4l: vsp1: Protect against race conditions between get and set format Laurent Pinchart
` (12 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:16 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
Pipelines can only be run if all their video nodes are streaming. Commit
b4dfb9b35a19 ("[media] v4l: vsp1: Stop the pipeline upon the first
STREAMOFF") fixed the pipeline stop sequence, but introduced a race
condition that makes it possible to run a pipeline after stopping the
stream on a video node by queuing a buffer on the other side of the
pipeline.
Fix this by clearing the buffers ready flag when stopping the stream,
which will prevent the QBUF handler from running the pipeline.
Fixes: b4dfb9b35a19 ("[media] v4l: vsp1: Stop the pipeline upon the first STREAMOFF")
Reported-by: Kieran Bingham <kieran@bingham.xyz>
Tested-by: Kieran Bingham <kieran@bingham.xyz>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_video.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 9fb4fc26a359..ed9759e8a6fc 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -675,6 +675,13 @@ static void vsp1_video_stop_streaming(struct vb2_queue *vq)
unsigned long flags;
int ret;
+ /* Clear the buffers ready flag to make sure the device won't be started
+ * by a QBUF on the video node on the other side of the pipeline.
+ */
+ spin_lock_irqsave(&video->irqlock, flags);
+ pipe->buffers_ready &= ~(1 << video->pipe_index);
+ spin_unlock_irqrestore(&video->irqlock, flags);
+
mutex_lock(&pipe->lock);
if (--pipe->stream_count == pipe->num_inputs) {
/* Stop the pipeline. */
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 02/13] v4l: vsp1: Protect against race conditions between get and set format
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
2016-09-13 23:16 ` [PATCH 01/13] v4l: vsp1: Prevent pipelines from running when not streaming Laurent Pinchart
@ 2016-09-13 23:16 ` Laurent Pinchart
2016-09-14 18:23 ` Niklas Söderlund
2016-09-13 23:16 ` [PATCH 03/13] v4l: vsp1: Ensure pipeline locking in resume path Laurent Pinchart
` (11 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:16 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
The subdev userspace API isn't serialized in the core, serialize access
to formats and selection rectangles in the driver.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_bru.c | 28 +++++++++++++++-----
drivers/media/platform/vsp1/vsp1_clu.c | 15 ++++++++---
drivers/media/platform/vsp1/vsp1_entity.c | 22 +++++++++++++---
drivers/media/platform/vsp1/vsp1_entity.h | 4 ++-
drivers/media/platform/vsp1/vsp1_hsit.c | 15 ++++++++---
drivers/media/platform/vsp1/vsp1_lif.c | 15 ++++++++---
drivers/media/platform/vsp1/vsp1_lut.c | 15 ++++++++---
drivers/media/platform/vsp1/vsp1_rwpf.c | 44 +++++++++++++++++++++++--------
drivers/media/platform/vsp1/vsp1_sru.c | 26 +++++++++++++-----
drivers/media/platform/vsp1/vsp1_uds.c | 26 +++++++++++++-----
10 files changed, 161 insertions(+), 49 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_bru.c b/drivers/media/platform/vsp1/vsp1_bru.c
index 8268b87727a7..26b9e2282a41 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -142,10 +142,15 @@ static int bru_set_format(struct v4l2_subdev *subdev,
struct vsp1_bru *bru = to_bru(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
+
+ mutex_lock(&bru->entity.lock);
config = vsp1_entity_get_pad_config(&bru->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ goto done;
+ ret = -EINVAL;
+ }
bru_try_format(bru, config, fmt->pad, &fmt->format);
@@ -174,7 +179,9 @@ static int bru_set_format(struct v4l2_subdev *subdev,
}
}
- return 0;
+done:
+ mutex_unlock(&bru->entity.lock);
+ return ret;
}
static int bru_get_selection(struct v4l2_subdev *subdev,
@@ -201,7 +208,9 @@ static int bru_get_selection(struct v4l2_subdev *subdev,
if (!config)
return -EINVAL;
+ mutex_lock(&bru->entity.lock);
sel->r = *bru_get_compose(bru, config, sel->pad);
+ mutex_unlock(&bru->entity.lock);
return 0;
default:
@@ -217,6 +226,7 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *compose;
+ int ret = 0;
if (sel->pad == bru->entity.source_pad)
return -EINVAL;
@@ -224,9 +234,13 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
if (sel->target != V4L2_SEL_TGT_COMPOSE)
return -EINVAL;
+ mutex_lock(&bru->entity.lock);
+
config = vsp1_entity_get_pad_config(&bru->entity, cfg, sel->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
/* The compose rectangle top left corner must be inside the output
* frame.
@@ -246,7 +260,9 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
compose = bru_get_compose(bru, config, sel->pad);
*compose = sel->r;
- return 0;
+done:
+ mutex_unlock(&bru->entity.lock);
+ return ret;
}
static const struct v4l2_subdev_pad_ops bru_pad_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c
index b63d2dbe5ea3..e1fd03811dda 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -148,10 +148,15 @@ static int clu_set_format(struct v4l2_subdev *subdev,
struct vsp1_clu *clu = to_clu(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
+
+ mutex_lock(&clu->entity.lock);
config = vsp1_entity_get_pad_config(&clu->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
/* Default to YUV if the requested format is not supported. */
if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
@@ -164,7 +169,7 @@ static int clu_set_format(struct v4l2_subdev *subdev,
if (fmt->pad == CLU_PAD_SOURCE) {
/* The CLU output format can't be modified. */
fmt->format = *format;
- return 0;
+ goto done;
}
format->code = fmt->format.code;
@@ -182,7 +187,9 @@ static int clu_set_format(struct v4l2_subdev *subdev,
CLU_PAD_SOURCE);
*format = fmt->format;
- return 0;
+done:
+ mutex_unlock(&clu->entity.lock);
+ return ret;
}
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/vsp1/vsp1_entity.c b/drivers/media/platform/vsp1/vsp1_entity.c
index 4cf6cc719c00..da673495c222 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/vsp1/vsp1_entity.c
@@ -51,6 +51,9 @@ void vsp1_entity_route_setup(struct vsp1_entity *source,
* @cfg: the TRY pad configuration
* @which: configuration selector (ACTIVE or TRY)
*
+ * When called with which set to V4L2_SUBDEV_FORMAT_ACTIVE the caller must hold
+ * the entity lock to access the returned configuration.
+ *
* Return the pad configuration requested by the which argument. The TRY
* configuration is passed explicitly to the function through the cfg argument
* and simply returned when requested. The ACTIVE configuration comes from the
@@ -160,7 +163,9 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
if (!config)
return -EINVAL;
+ mutex_lock(&entity->lock);
fmt->format = *vsp1_entity_get_pad_format(entity, config, fmt->pad);
+ mutex_unlock(&entity->lock);
return 0;
}
@@ -204,8 +209,10 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
if (!config)
return -EINVAL;
+ mutex_lock(&entity->lock);
format = vsp1_entity_get_pad_format(entity, config, 0);
code->code = format->code;
+ mutex_unlock(&entity->lock);
}
return 0;
@@ -235,6 +242,7 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
struct vsp1_entity *entity = to_vsp1_entity(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
config = vsp1_entity_get_pad_config(entity, cfg, fse->which);
if (!config)
@@ -242,8 +250,12 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
format = vsp1_entity_get_pad_format(entity, config, fse->pad);
- if (fse->index || fse->code != format->code)
- return -EINVAL;
+ mutex_lock(&entity->lock);
+
+ if (fse->index || fse->code != format->code) {
+ ret = -EINVAL;
+ goto done;
+ }
if (fse->pad == 0) {
fse->min_width = min_width;
@@ -260,7 +272,9 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
fse->max_height = format->height;
}
- return 0;
+done:
+ mutex_unlock(&entity->lock);
+ return ret;
}
/* -----------------------------------------------------------------------------
@@ -358,6 +372,8 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
if (i == ARRAY_SIZE(vsp1_routes))
return -EINVAL;
+ mutex_init(&entity->lock);
+
entity->vsp1 = vsp1;
entity->source_pad = num_pads - 1;
diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
index b43457fd2c43..b5e4dbb1f7d4 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/vsp1/vsp1_entity.h
@@ -14,7 +14,7 @@
#define __VSP1_ENTITY_H__
#include <linux/list.h>
-#include <linux/spinlock.h>
+#include <linux/mutex.h>
#include <media/v4l2-subdev.h>
@@ -96,6 +96,8 @@ struct vsp1_entity {
struct v4l2_subdev subdev;
struct v4l2_subdev_pad_config *config;
+
+ struct mutex lock; /* Protects the pad config */
};
static inline struct vsp1_entity *to_vsp1_entity(struct v4l2_subdev *subdev)
diff --git a/drivers/media/platform/vsp1/vsp1_hsit.c b/drivers/media/platform/vsp1/vsp1_hsit.c
index 6e5077beb38c..6ffbedb5c095 100644
--- a/drivers/media/platform/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/vsp1/vsp1_hsit.c
@@ -71,10 +71,15 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
struct vsp1_hsit *hsit = to_hsit(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
+
+ mutex_lock(&hsit->entity.lock);
config = vsp1_entity_get_pad_config(&hsit->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
format = vsp1_entity_get_pad_format(&hsit->entity, config, fmt->pad);
@@ -83,7 +88,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
* modified.
*/
fmt->format = *format;
- return 0;
+ goto done;
}
format->code = hsit->inverse ? MEDIA_BUS_FMT_AHSV8888_1X32
@@ -104,7 +109,9 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
format->code = hsit->inverse ? MEDIA_BUS_FMT_ARGB8888_1X32
: MEDIA_BUS_FMT_AHSV8888_1X32;
- return 0;
+done:
+ mutex_unlock(&hsit->entity.lock);
+ return ret;
}
static const struct v4l2_subdev_pad_ops hsit_pad_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_lif.c b/drivers/media/platform/vsp1/vsp1_lif.c
index a720063f38c5..702df863b13a 100644
--- a/drivers/media/platform/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/vsp1/vsp1_lif.c
@@ -66,10 +66,15 @@ static int lif_set_format(struct v4l2_subdev *subdev,
struct vsp1_lif *lif = to_lif(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
+
+ mutex_lock(&lif->entity.lock);
config = vsp1_entity_get_pad_config(&lif->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
/* Default to YUV if the requested format is not supported. */
if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
@@ -83,7 +88,7 @@ static int lif_set_format(struct v4l2_subdev *subdev,
* format.
*/
fmt->format = *format;
- return 0;
+ goto done;
}
format->code = fmt->format.code;
@@ -101,7 +106,9 @@ static int lif_set_format(struct v4l2_subdev *subdev,
LIF_PAD_SOURCE);
*format = fmt->format;
- return 0;
+done:
+ mutex_unlock(&lif->entity.lock);
+ return ret;
}
static const struct v4l2_subdev_pad_ops lif_pad_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c
index dc31de9602ba..e1c0bb7535e4 100644
--- a/drivers/media/platform/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/vsp1/vsp1_lut.c
@@ -124,10 +124,15 @@ static int lut_set_format(struct v4l2_subdev *subdev,
struct vsp1_lut *lut = to_lut(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
+
+ mutex_lock(&lut->entity.lock);
config = vsp1_entity_get_pad_config(&lut->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
/* Default to YUV if the requested format is not supported. */
if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
@@ -140,7 +145,7 @@ static int lut_set_format(struct v4l2_subdev *subdev,
if (fmt->pad == LUT_PAD_SOURCE) {
/* The LUT output format can't be modified. */
fmt->format = *format;
- return 0;
+ goto done;
}
format->code = fmt->format.code;
@@ -158,7 +163,9 @@ static int lut_set_format(struct v4l2_subdev *subdev,
LUT_PAD_SOURCE);
*format = fmt->format;
- return 0;
+done:
+ mutex_unlock(&lut->entity.lock);
+ return ret;
}
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c b/drivers/media/platform/vsp1/vsp1_rwpf.c
index 8d461b375e91..8cb87e96b78b 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
@@ -67,10 +67,15 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *crop;
+ int ret = 0;
+
+ mutex_lock(&rwpf->entity.lock);
config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
/* Default to YUV if the requested format is not supported. */
if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
@@ -85,7 +90,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
*/
format->code = fmt->format.code;
fmt->format = *format;
- return 0;
+ goto done;
}
format->code = fmt->format.code;
@@ -110,7 +115,9 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
RWPF_PAD_SOURCE);
*format = fmt->format;
- return 0;
+done:
+ mutex_unlock(&rwpf->entity.lock);
+ return ret;
}
static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
@@ -120,14 +127,19 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
struct vsp1_rwpf *rwpf = to_rwpf(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
/* Cropping is implemented on the sink pad. */
if (sel->pad != RWPF_PAD_SINK)
return -EINVAL;
+ mutex_lock(&rwpf->entity.lock);
+
config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, sel->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
switch (sel->target) {
case V4L2_SEL_TGT_CROP:
@@ -144,10 +156,13 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
break;
default:
- return -EINVAL;
+ ret = -EINVAL;
+ break;
}
- return 0;
+done:
+ mutex_unlock(&rwpf->entity.lock);
+ return ret;
}
static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
@@ -158,6 +173,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *crop;
+ int ret = 0;
/* Cropping is implemented on the sink pad. */
if (sel->pad != RWPF_PAD_SINK)
@@ -166,9 +182,13 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
if (sel->target != V4L2_SEL_TGT_CROP)
return -EINVAL;
+ mutex_lock(&rwpf->entity.lock);
+
config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, sel->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
/* Make sure the crop rectangle is entirely contained in the image. The
* WPF top and left offsets are limited to 255.
@@ -206,7 +226,9 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
format->width = crop->width;
format->height = crop->height;
- return 0;
+done:
+ mutex_unlock(&rwpf->entity.lock);
+ return ret;
}
const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
index 47f5e0cea2ce..6e13cdfa5ed4 100644
--- a/drivers/media/platform/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/vsp1/vsp1_sru.c
@@ -128,6 +128,7 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
struct vsp1_sru *sru = to_sru(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
config = vsp1_entity_get_pad_config(&sru->entity, cfg, fse->which);
if (!config)
@@ -135,8 +136,12 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
format = vsp1_entity_get_pad_format(&sru->entity, config, SRU_PAD_SINK);
- if (fse->index || fse->code != format->code)
- return -EINVAL;
+ mutex_lock(&sru->entity.lock);
+
+ if (fse->index || fse->code != format->code) {
+ ret = -EINVAL;
+ goto done;
+ }
if (fse->pad == SRU_PAD_SINK) {
fse->min_width = SRU_MIN_SIZE;
@@ -156,7 +161,9 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
}
}
- return 0;
+done:
+ mutex_unlock(&sru->entity.lock);
+ return ret;
}
static void sru_try_format(struct vsp1_sru *sru,
@@ -217,10 +224,15 @@ static int sru_set_format(struct v4l2_subdev *subdev,
struct vsp1_sru *sru = to_sru(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
+
+ mutex_lock(&sru->entity.lock);
config = vsp1_entity_get_pad_config(&sru->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
sru_try_format(sru, config, fmt->pad, &fmt->format);
@@ -236,7 +248,9 @@ static int sru_set_format(struct v4l2_subdev *subdev,
sru_try_format(sru, config, SRU_PAD_SOURCE, format);
}
- return 0;
+done:
+ mutex_unlock(&sru->entity.lock);
+ return ret;
}
static const struct v4l2_subdev_pad_ops sru_pad_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
index 652dcd895022..a8fc893a31ee 100644
--- a/drivers/media/platform/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/vsp1/vsp1_uds.c
@@ -133,6 +133,7 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
struct vsp1_uds *uds = to_uds(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
config = vsp1_entity_get_pad_config(&uds->entity, cfg, fse->which);
if (!config)
@@ -141,8 +142,12 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
format = vsp1_entity_get_pad_format(&uds->entity, config,
UDS_PAD_SINK);
- if (fse->index || fse->code != format->code)
- return -EINVAL;
+ mutex_lock(&uds->entity.lock);
+
+ if (fse->index || fse->code != format->code) {
+ ret = -EINVAL;
+ goto done;
+ }
if (fse->pad == UDS_PAD_SINK) {
fse->min_width = UDS_MIN_SIZE;
@@ -156,7 +161,9 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
&fse->max_height);
}
- return 0;
+done:
+ mutex_unlock(&uds->entity.lock);
+ return ret;
}
static void uds_try_format(struct vsp1_uds *uds,
@@ -202,10 +209,15 @@ static int uds_set_format(struct v4l2_subdev *subdev,
struct vsp1_uds *uds = to_uds(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
+ int ret = 0;
+
+ mutex_lock(&uds->entity.lock);
config = vsp1_entity_get_pad_config(&uds->entity, cfg, fmt->which);
- if (!config)
- return -EINVAL;
+ if (!config) {
+ ret = -EINVAL;
+ goto done;
+ }
uds_try_format(uds, config, fmt->pad, &fmt->format);
@@ -221,7 +233,9 @@ static int uds_set_format(struct v4l2_subdev *subdev,
uds_try_format(uds, config, UDS_PAD_SOURCE, format);
}
- return 0;
+done:
+ mutex_unlock(&uds->entity.lock);
+ return ret;
}
/* -----------------------------------------------------------------------------
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 02/13] v4l: vsp1: Protect against race conditions between get and set format
2016-09-13 23:16 ` [PATCH 02/13] v4l: vsp1: Protect against race conditions between get and set format Laurent Pinchart
@ 2016-09-14 18:23 ` Niklas Söderlund
2016-09-14 19:32 ` Niklas Söderlund
2016-09-14 19:50 ` Laurent Pinchart
0 siblings, 2 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 18:23 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
Hi Laurent,
Thanks for your patch.
On 2016-09-14 02:16:55 +0300, Laurent Pinchart wrote:
> The subdev userspace API isn't serialized in the core, serialize access
> to formats and selection rectangles in the driver.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/media/platform/vsp1/vsp1_bru.c | 28 +++++++++++++++-----
> drivers/media/platform/vsp1/vsp1_clu.c | 15 ++++++++---
> drivers/media/platform/vsp1/vsp1_entity.c | 22 +++++++++++++---
> drivers/media/platform/vsp1/vsp1_entity.h | 4 ++-
> drivers/media/platform/vsp1/vsp1_hsit.c | 15 ++++++++---
> drivers/media/platform/vsp1/vsp1_lif.c | 15 ++++++++---
> drivers/media/platform/vsp1/vsp1_lut.c | 15 ++++++++---
> drivers/media/platform/vsp1/vsp1_rwpf.c | 44 +++++++++++++++++++++++--------
> drivers/media/platform/vsp1/vsp1_sru.c | 26 +++++++++++++-----
> drivers/media/platform/vsp1/vsp1_uds.c | 26 +++++++++++++-----
> 10 files changed, 161 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_bru.c b/drivers/media/platform/vsp1/vsp1_bru.c
> index 8268b87727a7..26b9e2282a41 100644
> --- a/drivers/media/platform/vsp1/vsp1_bru.c
> +++ b/drivers/media/platform/vsp1/vsp1_bru.c
> @@ -142,10 +142,15 @@ static int bru_set_format(struct v4l2_subdev *subdev,
> struct vsp1_bru *bru = to_bru(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
> +
> + mutex_lock(&bru->entity.lock);
>
> config = vsp1_entity_get_pad_config(&bru->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + goto done;
> + ret = -EINVAL;
This looks funny to me, you probably intended to do that in the other
order right? If you fix this feel free to add my:
Acked-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> + }
>
> bru_try_format(bru, config, fmt->pad, &fmt->format);
>
> @@ -174,7 +179,9 @@ static int bru_set_format(struct v4l2_subdev *subdev,
> }
> }
>
> - return 0;
> +done:
> + mutex_unlock(&bru->entity.lock);
> + return ret;
> }
>
> static int bru_get_selection(struct v4l2_subdev *subdev,
> @@ -201,7 +208,9 @@ static int bru_get_selection(struct v4l2_subdev *subdev,
> if (!config)
> return -EINVAL;
>
> + mutex_lock(&bru->entity.lock);
> sel->r = *bru_get_compose(bru, config, sel->pad);
> + mutex_unlock(&bru->entity.lock);
> return 0;
>
> default:
> @@ -217,6 +226,7 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> struct v4l2_rect *compose;
> + int ret = 0;
>
> if (sel->pad == bru->entity.source_pad)
> return -EINVAL;
> @@ -224,9 +234,13 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> if (sel->target != V4L2_SEL_TGT_COMPOSE)
> return -EINVAL;
>
> + mutex_lock(&bru->entity.lock);
> +
> config = vsp1_entity_get_pad_config(&bru->entity, cfg, sel->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> /* The compose rectangle top left corner must be inside the output
> * frame.
> @@ -246,7 +260,9 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> compose = bru_get_compose(bru, config, sel->pad);
> *compose = sel->r;
>
> - return 0;
> +done:
> + mutex_unlock(&bru->entity.lock);
> + return ret;
> }
>
> static const struct v4l2_subdev_pad_ops bru_pad_ops = {
> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c
> index b63d2dbe5ea3..e1fd03811dda 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> @@ -148,10 +148,15 @@ static int clu_set_format(struct v4l2_subdev *subdev,
> struct vsp1_clu *clu = to_clu(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
> +
> + mutex_lock(&clu->entity.lock);
>
> config = vsp1_entity_get_pad_config(&clu->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> /* Default to YUV if the requested format is not supported. */
> if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> @@ -164,7 +169,7 @@ static int clu_set_format(struct v4l2_subdev *subdev,
> if (fmt->pad == CLU_PAD_SOURCE) {
> /* The CLU output format can't be modified. */
> fmt->format = *format;
> - return 0;
> + goto done;
> }
>
> format->code = fmt->format.code;
> @@ -182,7 +187,9 @@ static int clu_set_format(struct v4l2_subdev *subdev,
> CLU_PAD_SOURCE);
> *format = fmt->format;
>
> - return 0;
> +done:
> + mutex_unlock(&clu->entity.lock);
> + return ret;
> }
>
> /* -----------------------------------------------------------------------------
> diff --git a/drivers/media/platform/vsp1/vsp1_entity.c b/drivers/media/platform/vsp1/vsp1_entity.c
> index 4cf6cc719c00..da673495c222 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.c
> +++ b/drivers/media/platform/vsp1/vsp1_entity.c
> @@ -51,6 +51,9 @@ void vsp1_entity_route_setup(struct vsp1_entity *source,
> * @cfg: the TRY pad configuration
> * @which: configuration selector (ACTIVE or TRY)
> *
> + * When called with which set to V4L2_SUBDEV_FORMAT_ACTIVE the caller must hold
> + * the entity lock to access the returned configuration.
> + *
> * Return the pad configuration requested by the which argument. The TRY
> * configuration is passed explicitly to the function through the cfg argument
> * and simply returned when requested. The ACTIVE configuration comes from the
> @@ -160,7 +163,9 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
> if (!config)
> return -EINVAL;
>
> + mutex_lock(&entity->lock);
> fmt->format = *vsp1_entity_get_pad_format(entity, config, fmt->pad);
> + mutex_unlock(&entity->lock);
>
> return 0;
> }
> @@ -204,8 +209,10 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
> if (!config)
> return -EINVAL;
>
> + mutex_lock(&entity->lock);
> format = vsp1_entity_get_pad_format(entity, config, 0);
> code->code = format->code;
> + mutex_unlock(&entity->lock);
> }
>
> return 0;
> @@ -235,6 +242,7 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
> struct vsp1_entity *entity = to_vsp1_entity(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
>
> config = vsp1_entity_get_pad_config(entity, cfg, fse->which);
> if (!config)
> @@ -242,8 +250,12 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
>
> format = vsp1_entity_get_pad_format(entity, config, fse->pad);
>
> - if (fse->index || fse->code != format->code)
> - return -EINVAL;
> + mutex_lock(&entity->lock);
> +
> + if (fse->index || fse->code != format->code) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> if (fse->pad == 0) {
> fse->min_width = min_width;
> @@ -260,7 +272,9 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
> fse->max_height = format->height;
> }
>
> - return 0;
> +done:
> + mutex_unlock(&entity->lock);
> + return ret;
> }
>
> /* -----------------------------------------------------------------------------
> @@ -358,6 +372,8 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
> if (i == ARRAY_SIZE(vsp1_routes))
> return -EINVAL;
>
> + mutex_init(&entity->lock);
> +
> entity->vsp1 = vsp1;
> entity->source_pad = num_pads - 1;
>
> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
> index b43457fd2c43..b5e4dbb1f7d4 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> @@ -14,7 +14,7 @@
> #define __VSP1_ENTITY_H__
>
> #include <linux/list.h>
> -#include <linux/spinlock.h>
> +#include <linux/mutex.h>
>
> #include <media/v4l2-subdev.h>
>
> @@ -96,6 +96,8 @@ struct vsp1_entity {
>
> struct v4l2_subdev subdev;
> struct v4l2_subdev_pad_config *config;
> +
> + struct mutex lock; /* Protects the pad config */
> };
>
> static inline struct vsp1_entity *to_vsp1_entity(struct v4l2_subdev *subdev)
> diff --git a/drivers/media/platform/vsp1/vsp1_hsit.c b/drivers/media/platform/vsp1/vsp1_hsit.c
> index 6e5077beb38c..6ffbedb5c095 100644
> --- a/drivers/media/platform/vsp1/vsp1_hsit.c
> +++ b/drivers/media/platform/vsp1/vsp1_hsit.c
> @@ -71,10 +71,15 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
> struct vsp1_hsit *hsit = to_hsit(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
> +
> + mutex_lock(&hsit->entity.lock);
>
> config = vsp1_entity_get_pad_config(&hsit->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> format = vsp1_entity_get_pad_format(&hsit->entity, config, fmt->pad);
>
> @@ -83,7 +88,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
> * modified.
> */
> fmt->format = *format;
> - return 0;
> + goto done;
> }
>
> format->code = hsit->inverse ? MEDIA_BUS_FMT_AHSV8888_1X32
> @@ -104,7 +109,9 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
> format->code = hsit->inverse ? MEDIA_BUS_FMT_ARGB8888_1X32
> : MEDIA_BUS_FMT_AHSV8888_1X32;
>
> - return 0;
> +done:
> + mutex_unlock(&hsit->entity.lock);
> + return ret;
> }
>
> static const struct v4l2_subdev_pad_ops hsit_pad_ops = {
> diff --git a/drivers/media/platform/vsp1/vsp1_lif.c b/drivers/media/platform/vsp1/vsp1_lif.c
> index a720063f38c5..702df863b13a 100644
> --- a/drivers/media/platform/vsp1/vsp1_lif.c
> +++ b/drivers/media/platform/vsp1/vsp1_lif.c
> @@ -66,10 +66,15 @@ static int lif_set_format(struct v4l2_subdev *subdev,
> struct vsp1_lif *lif = to_lif(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
> +
> + mutex_lock(&lif->entity.lock);
>
> config = vsp1_entity_get_pad_config(&lif->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> /* Default to YUV if the requested format is not supported. */
> if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> @@ -83,7 +88,7 @@ static int lif_set_format(struct v4l2_subdev *subdev,
> * format.
> */
> fmt->format = *format;
> - return 0;
> + goto done;
> }
>
> format->code = fmt->format.code;
> @@ -101,7 +106,9 @@ static int lif_set_format(struct v4l2_subdev *subdev,
> LIF_PAD_SOURCE);
> *format = fmt->format;
>
> - return 0;
> +done:
> + mutex_unlock(&lif->entity.lock);
> + return ret;
> }
>
> static const struct v4l2_subdev_pad_ops lif_pad_ops = {
> diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c
> index dc31de9602ba..e1c0bb7535e4 100644
> --- a/drivers/media/platform/vsp1/vsp1_lut.c
> +++ b/drivers/media/platform/vsp1/vsp1_lut.c
> @@ -124,10 +124,15 @@ static int lut_set_format(struct v4l2_subdev *subdev,
> struct vsp1_lut *lut = to_lut(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
> +
> + mutex_lock(&lut->entity.lock);
>
> config = vsp1_entity_get_pad_config(&lut->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> /* Default to YUV if the requested format is not supported. */
> if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> @@ -140,7 +145,7 @@ static int lut_set_format(struct v4l2_subdev *subdev,
> if (fmt->pad == LUT_PAD_SOURCE) {
> /* The LUT output format can't be modified. */
> fmt->format = *format;
> - return 0;
> + goto done;
> }
>
> format->code = fmt->format.code;
> @@ -158,7 +163,9 @@ static int lut_set_format(struct v4l2_subdev *subdev,
> LUT_PAD_SOURCE);
> *format = fmt->format;
>
> - return 0;
> +done:
> + mutex_unlock(&lut->entity.lock);
> + return ret;
> }
>
> /* -----------------------------------------------------------------------------
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c b/drivers/media/platform/vsp1/vsp1_rwpf.c
> index 8d461b375e91..8cb87e96b78b 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -67,10 +67,15 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> struct v4l2_rect *crop;
> + int ret = 0;
> +
> + mutex_lock(&rwpf->entity.lock);
>
> config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> /* Default to YUV if the requested format is not supported. */
> if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> @@ -85,7 +90,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> */
> format->code = fmt->format.code;
> fmt->format = *format;
> - return 0;
> + goto done;
> }
>
> format->code = fmt->format.code;
> @@ -110,7 +115,9 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> RWPF_PAD_SOURCE);
> *format = fmt->format;
>
> - return 0;
> +done:
> + mutex_unlock(&rwpf->entity.lock);
> + return ret;
> }
>
> static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> @@ -120,14 +127,19 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
>
> /* Cropping is implemented on the sink pad. */
> if (sel->pad != RWPF_PAD_SINK)
> return -EINVAL;
>
> + mutex_lock(&rwpf->entity.lock);
> +
> config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, sel->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> switch (sel->target) {
> case V4L2_SEL_TGT_CROP:
> @@ -144,10 +156,13 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> break;
>
> default:
> - return -EINVAL;
> + ret = -EINVAL;
> + break;
> }
Nit-picking, maybe use goto here instead of break? I'm always scared of
constructs like this since code might get added after the switch
statement and this is a error path.
>
> - return 0;
> +done:
> + mutex_unlock(&rwpf->entity.lock);
> + return ret;
> }
>
> static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> @@ -158,6 +173,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> struct v4l2_rect *crop;
> + int ret = 0;
>
> /* Cropping is implemented on the sink pad. */
> if (sel->pad != RWPF_PAD_SINK)
> @@ -166,9 +182,13 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> if (sel->target != V4L2_SEL_TGT_CROP)
> return -EINVAL;
>
> + mutex_lock(&rwpf->entity.lock);
> +
> config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, sel->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> /* Make sure the crop rectangle is entirely contained in the image. The
> * WPF top and left offsets are limited to 255.
> @@ -206,7 +226,9 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> format->width = crop->width;
> format->height = crop->height;
>
> - return 0;
> +done:
> + mutex_unlock(&rwpf->entity.lock);
> + return ret;
> }
>
> const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = {
> diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
> index 47f5e0cea2ce..6e13cdfa5ed4 100644
> --- a/drivers/media/platform/vsp1/vsp1_sru.c
> +++ b/drivers/media/platform/vsp1/vsp1_sru.c
> @@ -128,6 +128,7 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
> struct vsp1_sru *sru = to_sru(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
>
> config = vsp1_entity_get_pad_config(&sru->entity, cfg, fse->which);
> if (!config)
> @@ -135,8 +136,12 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
>
> format = vsp1_entity_get_pad_format(&sru->entity, config, SRU_PAD_SINK);
>
> - if (fse->index || fse->code != format->code)
> - return -EINVAL;
> + mutex_lock(&sru->entity.lock);
> +
> + if (fse->index || fse->code != format->code) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> if (fse->pad == SRU_PAD_SINK) {
> fse->min_width = SRU_MIN_SIZE;
> @@ -156,7 +161,9 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
> }
> }
>
> - return 0;
> +done:
> + mutex_unlock(&sru->entity.lock);
> + return ret;
> }
>
> static void sru_try_format(struct vsp1_sru *sru,
> @@ -217,10 +224,15 @@ static int sru_set_format(struct v4l2_subdev *subdev,
> struct vsp1_sru *sru = to_sru(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
> +
> + mutex_lock(&sru->entity.lock);
>
> config = vsp1_entity_get_pad_config(&sru->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> sru_try_format(sru, config, fmt->pad, &fmt->format);
>
> @@ -236,7 +248,9 @@ static int sru_set_format(struct v4l2_subdev *subdev,
> sru_try_format(sru, config, SRU_PAD_SOURCE, format);
> }
>
> - return 0;
> +done:
> + mutex_unlock(&sru->entity.lock);
> + return ret;
> }
>
> static const struct v4l2_subdev_pad_ops sru_pad_ops = {
> diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
> index 652dcd895022..a8fc893a31ee 100644
> --- a/drivers/media/platform/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/vsp1/vsp1_uds.c
> @@ -133,6 +133,7 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
> struct vsp1_uds *uds = to_uds(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
>
> config = vsp1_entity_get_pad_config(&uds->entity, cfg, fse->which);
> if (!config)
> @@ -141,8 +142,12 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
> format = vsp1_entity_get_pad_format(&uds->entity, config,
> UDS_PAD_SINK);
>
> - if (fse->index || fse->code != format->code)
> - return -EINVAL;
> + mutex_lock(&uds->entity.lock);
> +
> + if (fse->index || fse->code != format->code) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> if (fse->pad == UDS_PAD_SINK) {
> fse->min_width = UDS_MIN_SIZE;
> @@ -156,7 +161,9 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
> &fse->max_height);
> }
>
> - return 0;
> +done:
> + mutex_unlock(&uds->entity.lock);
> + return ret;
> }
>
> static void uds_try_format(struct vsp1_uds *uds,
> @@ -202,10 +209,15 @@ static int uds_set_format(struct v4l2_subdev *subdev,
> struct vsp1_uds *uds = to_uds(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> + int ret = 0;
> +
> + mutex_lock(&uds->entity.lock);
>
> config = vsp1_entity_get_pad_config(&uds->entity, cfg, fmt->which);
> - if (!config)
> - return -EINVAL;
> + if (!config) {
> + ret = -EINVAL;
> + goto done;
> + }
>
> uds_try_format(uds, config, fmt->pad, &fmt->format);
>
> @@ -221,7 +233,9 @@ static int uds_set_format(struct v4l2_subdev *subdev,
> uds_try_format(uds, config, UDS_PAD_SOURCE, format);
> }
>
> - return 0;
> +done:
> + mutex_unlock(&uds->entity.lock);
> + return ret;
> }
>
> /* -----------------------------------------------------------------------------
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 02/13] v4l: vsp1: Protect against race conditions between get and set format
2016-09-14 18:23 ` Niklas Söderlund
@ 2016-09-14 19:32 ` Niklas Söderlund
2016-09-14 19:50 ` Laurent Pinchart
1 sibling, 0 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 19:32 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 20:23:17 +0200, Niklas Söderlund wrote:
> Hi Laurent,
>
> Thanks for your patch.
>
> On 2016-09-14 02:16:55 +0300, Laurent Pinchart wrote:
> > The subdev userspace API isn't serialized in the core, serialize access
> > to formats and selection rectangles in the driver.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > drivers/media/platform/vsp1/vsp1_bru.c | 28 +++++++++++++++-----
> > drivers/media/platform/vsp1/vsp1_clu.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_entity.c | 22 +++++++++++++---
> > drivers/media/platform/vsp1/vsp1_entity.h | 4 ++-
> > drivers/media/platform/vsp1/vsp1_hsit.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_lif.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_lut.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_rwpf.c | 44 +++++++++++++++++++++++--------
> > drivers/media/platform/vsp1/vsp1_sru.c | 26 +++++++++++++-----
> > drivers/media/platform/vsp1/vsp1_uds.c | 26 +++++++++++++-----
> > 10 files changed, 161 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_bru.c b/drivers/media/platform/vsp1/vsp1_bru.c
> > index 8268b87727a7..26b9e2282a41 100644
> > --- a/drivers/media/platform/vsp1/vsp1_bru.c
> > +++ b/drivers/media/platform/vsp1/vsp1_bru.c
> > @@ -142,10 +142,15 @@ static int bru_set_format(struct v4l2_subdev *subdev,
> > struct vsp1_bru *bru = to_bru(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> > +
> > + mutex_lock(&bru->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&bru->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + goto done;
> > + ret = -EINVAL;
>
> This looks funny to me, you probably intended to do that in the other
> order right? If you fix this feel free to add my:
>
> Acked-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Wops, forgot +renesas in the email. If you fix the issue and want to
collect my Ack, please use:
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Sorry for the noise.
>
> > + }
> >
> > bru_try_format(bru, config, fmt->pad, &fmt->format);
> >
> > @@ -174,7 +179,9 @@ static int bru_set_format(struct v4l2_subdev *subdev,
> > }
> > }
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&bru->entity.lock);
> > + return ret;
> > }
> >
> > static int bru_get_selection(struct v4l2_subdev *subdev,
> > @@ -201,7 +208,9 @@ static int bru_get_selection(struct v4l2_subdev *subdev,
> > if (!config)
> > return -EINVAL;
> >
> > + mutex_lock(&bru->entity.lock);
> > sel->r = *bru_get_compose(bru, config, sel->pad);
> > + mutex_unlock(&bru->entity.lock);
> > return 0;
> >
> > default:
> > @@ -217,6 +226,7 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > struct v4l2_rect *compose;
> > + int ret = 0;
> >
> > if (sel->pad == bru->entity.source_pad)
> > return -EINVAL;
> > @@ -224,9 +234,13 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> > if (sel->target != V4L2_SEL_TGT_COMPOSE)
> > return -EINVAL;
> >
> > + mutex_lock(&bru->entity.lock);
> > +
> > config = vsp1_entity_get_pad_config(&bru->entity, cfg, sel->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > /* The compose rectangle top left corner must be inside the output
> > * frame.
> > @@ -246,7 +260,9 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> > compose = bru_get_compose(bru, config, sel->pad);
> > *compose = sel->r;
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&bru->entity.lock);
> > + return ret;
> > }
> >
> > static const struct v4l2_subdev_pad_ops bru_pad_ops = {
> > diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c
> > index b63d2dbe5ea3..e1fd03811dda 100644
> > --- a/drivers/media/platform/vsp1/vsp1_clu.c
> > +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> > @@ -148,10 +148,15 @@ static int clu_set_format(struct v4l2_subdev *subdev,
> > struct vsp1_clu *clu = to_clu(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> > +
> > + mutex_lock(&clu->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&clu->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > /* Default to YUV if the requested format is not supported. */
> > if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> > @@ -164,7 +169,7 @@ static int clu_set_format(struct v4l2_subdev *subdev,
> > if (fmt->pad == CLU_PAD_SOURCE) {
> > /* The CLU output format can't be modified. */
> > fmt->format = *format;
> > - return 0;
> > + goto done;
> > }
> >
> > format->code = fmt->format.code;
> > @@ -182,7 +187,9 @@ static int clu_set_format(struct v4l2_subdev *subdev,
> > CLU_PAD_SOURCE);
> > *format = fmt->format;
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&clu->entity.lock);
> > + return ret;
> > }
> >
> > /* -----------------------------------------------------------------------------
> > diff --git a/drivers/media/platform/vsp1/vsp1_entity.c b/drivers/media/platform/vsp1/vsp1_entity.c
> > index 4cf6cc719c00..da673495c222 100644
> > --- a/drivers/media/platform/vsp1/vsp1_entity.c
> > +++ b/drivers/media/platform/vsp1/vsp1_entity.c
> > @@ -51,6 +51,9 @@ void vsp1_entity_route_setup(struct vsp1_entity *source,
> > * @cfg: the TRY pad configuration
> > * @which: configuration selector (ACTIVE or TRY)
> > *
> > + * When called with which set to V4L2_SUBDEV_FORMAT_ACTIVE the caller must hold
> > + * the entity lock to access the returned configuration.
> > + *
> > * Return the pad configuration requested by the which argument. The TRY
> > * configuration is passed explicitly to the function through the cfg argument
> > * and simply returned when requested. The ACTIVE configuration comes from the
> > @@ -160,7 +163,9 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
> > if (!config)
> > return -EINVAL;
> >
> > + mutex_lock(&entity->lock);
> > fmt->format = *vsp1_entity_get_pad_format(entity, config, fmt->pad);
> > + mutex_unlock(&entity->lock);
> >
> > return 0;
> > }
> > @@ -204,8 +209,10 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
> > if (!config)
> > return -EINVAL;
> >
> > + mutex_lock(&entity->lock);
> > format = vsp1_entity_get_pad_format(entity, config, 0);
> > code->code = format->code;
> > + mutex_unlock(&entity->lock);
> > }
> >
> > return 0;
> > @@ -235,6 +242,7 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
> > struct vsp1_entity *entity = to_vsp1_entity(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> >
> > config = vsp1_entity_get_pad_config(entity, cfg, fse->which);
> > if (!config)
> > @@ -242,8 +250,12 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
> >
> > format = vsp1_entity_get_pad_format(entity, config, fse->pad);
> >
> > - if (fse->index || fse->code != format->code)
> > - return -EINVAL;
> > + mutex_lock(&entity->lock);
> > +
> > + if (fse->index || fse->code != format->code) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > if (fse->pad == 0) {
> > fse->min_width = min_width;
> > @@ -260,7 +272,9 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
> > fse->max_height = format->height;
> > }
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&entity->lock);
> > + return ret;
> > }
> >
> > /* -----------------------------------------------------------------------------
> > @@ -358,6 +372,8 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
> > if (i == ARRAY_SIZE(vsp1_routes))
> > return -EINVAL;
> >
> > + mutex_init(&entity->lock);
> > +
> > entity->vsp1 = vsp1;
> > entity->source_pad = num_pads - 1;
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
> > index b43457fd2c43..b5e4dbb1f7d4 100644
> > --- a/drivers/media/platform/vsp1/vsp1_entity.h
> > +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> > @@ -14,7 +14,7 @@
> > #define __VSP1_ENTITY_H__
> >
> > #include <linux/list.h>
> > -#include <linux/spinlock.h>
> > +#include <linux/mutex.h>
> >
> > #include <media/v4l2-subdev.h>
> >
> > @@ -96,6 +96,8 @@ struct vsp1_entity {
> >
> > struct v4l2_subdev subdev;
> > struct v4l2_subdev_pad_config *config;
> > +
> > + struct mutex lock; /* Protects the pad config */
> > };
> >
> > static inline struct vsp1_entity *to_vsp1_entity(struct v4l2_subdev *subdev)
> > diff --git a/drivers/media/platform/vsp1/vsp1_hsit.c b/drivers/media/platform/vsp1/vsp1_hsit.c
> > index 6e5077beb38c..6ffbedb5c095 100644
> > --- a/drivers/media/platform/vsp1/vsp1_hsit.c
> > +++ b/drivers/media/platform/vsp1/vsp1_hsit.c
> > @@ -71,10 +71,15 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
> > struct vsp1_hsit *hsit = to_hsit(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> > +
> > + mutex_lock(&hsit->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&hsit->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > format = vsp1_entity_get_pad_format(&hsit->entity, config, fmt->pad);
> >
> > @@ -83,7 +88,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
> > * modified.
> > */
> > fmt->format = *format;
> > - return 0;
> > + goto done;
> > }
> >
> > format->code = hsit->inverse ? MEDIA_BUS_FMT_AHSV8888_1X32
> > @@ -104,7 +109,9 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
> > format->code = hsit->inverse ? MEDIA_BUS_FMT_ARGB8888_1X32
> > : MEDIA_BUS_FMT_AHSV8888_1X32;
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&hsit->entity.lock);
> > + return ret;
> > }
> >
> > static const struct v4l2_subdev_pad_ops hsit_pad_ops = {
> > diff --git a/drivers/media/platform/vsp1/vsp1_lif.c b/drivers/media/platform/vsp1/vsp1_lif.c
> > index a720063f38c5..702df863b13a 100644
> > --- a/drivers/media/platform/vsp1/vsp1_lif.c
> > +++ b/drivers/media/platform/vsp1/vsp1_lif.c
> > @@ -66,10 +66,15 @@ static int lif_set_format(struct v4l2_subdev *subdev,
> > struct vsp1_lif *lif = to_lif(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> > +
> > + mutex_lock(&lif->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&lif->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > /* Default to YUV if the requested format is not supported. */
> > if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> > @@ -83,7 +88,7 @@ static int lif_set_format(struct v4l2_subdev *subdev,
> > * format.
> > */
> > fmt->format = *format;
> > - return 0;
> > + goto done;
> > }
> >
> > format->code = fmt->format.code;
> > @@ -101,7 +106,9 @@ static int lif_set_format(struct v4l2_subdev *subdev,
> > LIF_PAD_SOURCE);
> > *format = fmt->format;
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&lif->entity.lock);
> > + return ret;
> > }
> >
> > static const struct v4l2_subdev_pad_ops lif_pad_ops = {
> > diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c
> > index dc31de9602ba..e1c0bb7535e4 100644
> > --- a/drivers/media/platform/vsp1/vsp1_lut.c
> > +++ b/drivers/media/platform/vsp1/vsp1_lut.c
> > @@ -124,10 +124,15 @@ static int lut_set_format(struct v4l2_subdev *subdev,
> > struct vsp1_lut *lut = to_lut(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> > +
> > + mutex_lock(&lut->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&lut->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > /* Default to YUV if the requested format is not supported. */
> > if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> > @@ -140,7 +145,7 @@ static int lut_set_format(struct v4l2_subdev *subdev,
> > if (fmt->pad == LUT_PAD_SOURCE) {
> > /* The LUT output format can't be modified. */
> > fmt->format = *format;
> > - return 0;
> > + goto done;
> > }
> >
> > format->code = fmt->format.code;
> > @@ -158,7 +163,9 @@ static int lut_set_format(struct v4l2_subdev *subdev,
> > LUT_PAD_SOURCE);
> > *format = fmt->format;
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&lut->entity.lock);
> > + return ret;
> > }
> >
> > /* -----------------------------------------------------------------------------
> > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > index 8d461b375e91..8cb87e96b78b 100644
> > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > @@ -67,10 +67,15 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > struct v4l2_rect *crop;
> > + int ret = 0;
> > +
> > + mutex_lock(&rwpf->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > /* Default to YUV if the requested format is not supported. */
> > if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
> > @@ -85,7 +90,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> > */
> > format->code = fmt->format.code;
> > fmt->format = *format;
> > - return 0;
> > + goto done;
> > }
> >
> > format->code = fmt->format.code;
> > @@ -110,7 +115,9 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> > RWPF_PAD_SOURCE);
> > *format = fmt->format;
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&rwpf->entity.lock);
> > + return ret;
> > }
> >
> > static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> > @@ -120,14 +127,19 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> > struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> >
> > /* Cropping is implemented on the sink pad. */
> > if (sel->pad != RWPF_PAD_SINK)
> > return -EINVAL;
> >
> > + mutex_lock(&rwpf->entity.lock);
> > +
> > config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, sel->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > switch (sel->target) {
> > case V4L2_SEL_TGT_CROP:
> > @@ -144,10 +156,13 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> > break;
> >
> > default:
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + break;
> > }
>
> Nit-picking, maybe use goto here instead of break? I'm always scared of
> constructs like this since code might get added after the switch
> statement and this is a error path.
>
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&rwpf->entity.lock);
> > + return ret;
> > }
> >
> > static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> > @@ -158,6 +173,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > struct v4l2_rect *crop;
> > + int ret = 0;
> >
> > /* Cropping is implemented on the sink pad. */
> > if (sel->pad != RWPF_PAD_SINK)
> > @@ -166,9 +182,13 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> > if (sel->target != V4L2_SEL_TGT_CROP)
> > return -EINVAL;
> >
> > + mutex_lock(&rwpf->entity.lock);
> > +
> > config = vsp1_entity_get_pad_config(&rwpf->entity, cfg, sel->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > /* Make sure the crop rectangle is entirely contained in the image. The
> > * WPF top and left offsets are limited to 255.
> > @@ -206,7 +226,9 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> > format->width = crop->width;
> > format->height = crop->height;
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&rwpf->entity.lock);
> > + return ret;
> > }
> >
> > const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = {
> > diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
> > index 47f5e0cea2ce..6e13cdfa5ed4 100644
> > --- a/drivers/media/platform/vsp1/vsp1_sru.c
> > +++ b/drivers/media/platform/vsp1/vsp1_sru.c
> > @@ -128,6 +128,7 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
> > struct vsp1_sru *sru = to_sru(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> >
> > config = vsp1_entity_get_pad_config(&sru->entity, cfg, fse->which);
> > if (!config)
> > @@ -135,8 +136,12 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
> >
> > format = vsp1_entity_get_pad_format(&sru->entity, config, SRU_PAD_SINK);
> >
> > - if (fse->index || fse->code != format->code)
> > - return -EINVAL;
> > + mutex_lock(&sru->entity.lock);
> > +
> > + if (fse->index || fse->code != format->code) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > if (fse->pad == SRU_PAD_SINK) {
> > fse->min_width = SRU_MIN_SIZE;
> > @@ -156,7 +161,9 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
> > }
> > }
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&sru->entity.lock);
> > + return ret;
> > }
> >
> > static void sru_try_format(struct vsp1_sru *sru,
> > @@ -217,10 +224,15 @@ static int sru_set_format(struct v4l2_subdev *subdev,
> > struct vsp1_sru *sru = to_sru(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> > +
> > + mutex_lock(&sru->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&sru->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > sru_try_format(sru, config, fmt->pad, &fmt->format);
> >
> > @@ -236,7 +248,9 @@ static int sru_set_format(struct v4l2_subdev *subdev,
> > sru_try_format(sru, config, SRU_PAD_SOURCE, format);
> > }
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&sru->entity.lock);
> > + return ret;
> > }
> >
> > static const struct v4l2_subdev_pad_ops sru_pad_ops = {
> > diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
> > index 652dcd895022..a8fc893a31ee 100644
> > --- a/drivers/media/platform/vsp1/vsp1_uds.c
> > +++ b/drivers/media/platform/vsp1/vsp1_uds.c
> > @@ -133,6 +133,7 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
> > struct vsp1_uds *uds = to_uds(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> >
> > config = vsp1_entity_get_pad_config(&uds->entity, cfg, fse->which);
> > if (!config)
> > @@ -141,8 +142,12 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
> > format = vsp1_entity_get_pad_format(&uds->entity, config,
> > UDS_PAD_SINK);
> >
> > - if (fse->index || fse->code != format->code)
> > - return -EINVAL;
> > + mutex_lock(&uds->entity.lock);
> > +
> > + if (fse->index || fse->code != format->code) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > if (fse->pad == UDS_PAD_SINK) {
> > fse->min_width = UDS_MIN_SIZE;
> > @@ -156,7 +161,9 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
> > &fse->max_height);
> > }
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&uds->entity.lock);
> > + return ret;
> > }
> >
> > static void uds_try_format(struct vsp1_uds *uds,
> > @@ -202,10 +209,15 @@ static int uds_set_format(struct v4l2_subdev *subdev,
> > struct vsp1_uds *uds = to_uds(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > + int ret = 0;
> > +
> > + mutex_lock(&uds->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&uds->entity, cfg, fmt->which);
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + ret = -EINVAL;
> > + goto done;
> > + }
> >
> > uds_try_format(uds, config, fmt->pad, &fmt->format);
> >
> > @@ -221,7 +233,9 @@ static int uds_set_format(struct v4l2_subdev *subdev,
> > uds_try_format(uds, config, UDS_PAD_SOURCE, format);
> > }
> >
> > - return 0;
> > +done:
> > + mutex_unlock(&uds->entity.lock);
> > + return ret;
> > }
> >
> > /* -----------------------------------------------------------------------------
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
>
> --
> Regards,
> Niklas Söderlund
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 02/13] v4l: vsp1: Protect against race conditions between get and set format
2016-09-14 18:23 ` Niklas Söderlund
2016-09-14 19:32 ` Niklas Söderlund
@ 2016-09-14 19:50 ` Laurent Pinchart
1 sibling, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-14 19:50 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, Kieran Bingham
Hi Niklas,
Thank you for the review.
On Wednesday 14 Sep 2016 20:23:18 Niklas Söderlund wrote:
> On 2016-09-14 02:16:55 +0300, Laurent Pinchart wrote:
> > The subdev userspace API isn't serialized in the core, serialize access
> > to formats and selection rectangles in the driver.
> >
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >
> > drivers/media/platform/vsp1/vsp1_bru.c | 28 +++++++++++++++-----
> > drivers/media/platform/vsp1/vsp1_clu.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_entity.c | 22 +++++++++++++---
> > drivers/media/platform/vsp1/vsp1_entity.h | 4 ++-
> > drivers/media/platform/vsp1/vsp1_hsit.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_lif.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_lut.c | 15 ++++++++---
> > drivers/media/platform/vsp1/vsp1_rwpf.c | 44 +++++++++++++++++++-------
> > drivers/media/platform/vsp1/vsp1_sru.c | 26 +++++++++++++-----
> > drivers/media/platform/vsp1/vsp1_uds.c | 26 +++++++++++++-----
> > 10 files changed, 161 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_bru.c
> > b/drivers/media/platform/vsp1/vsp1_bru.c index 8268b87727a7..26b9e2282a41
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_bru.c
> > +++ b/drivers/media/platform/vsp1/vsp1_bru.c
> > @@ -142,10 +142,15 @@ static int bru_set_format(struct v4l2_subdev
> > *subdev,
> >
> > struct vsp1_bru *bru = to_bru(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> >
> > + int ret = 0;
> > +
> > + mutex_lock(&bru->entity.lock);
> >
> > config = vsp1_entity_get_pad_config(&bru->entity, cfg, fmt->which);
> >
> > - if (!config)
> > - return -EINVAL;
> > + if (!config) {
> > + goto done;
> > + ret = -EINVAL;
>
> This looks funny to me, you probably intended to do that in the other
> order right?
Oops, good catch !
> If you fix this feel free to add my:
>
> Acked-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Fixed and applied your ack (with +renesas as mentioned in your other email).
> > + }
> >
> > bru_try_format(bru, config, fmt->pad, &fmt->format);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 03/13] v4l: vsp1: Ensure pipeline locking in resume path
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
2016-09-13 23:16 ` [PATCH 01/13] v4l: vsp1: Prevent pipelines from running when not streaming Laurent Pinchart
2016-09-13 23:16 ` [PATCH 02/13] v4l: vsp1: Protect against race conditions between get and set format Laurent Pinchart
@ 2016-09-13 23:16 ` Laurent Pinchart
2016-09-14 18:28 ` Niklas Söderlund
2016-09-13 23:16 ` [PATCH 04/13] v4l: vsp1: Repair race between frame end and qbuf handler Laurent Pinchart
` (10 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:16 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
From: Kieran Bingham <kieran+renesas@bingham.xyz>
The vsp1_pipeline_ready() and vsp1_pipeline_run() functions must be
called with the pipeline lock held, fix the resume code path.
Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_pipe.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c
index 3e75fb3fcace..474de82165d8 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -365,6 +365,7 @@ void vsp1_pipelines_suspend(struct vsp1_device *vsp1)
void vsp1_pipelines_resume(struct vsp1_device *vsp1)
{
+ unsigned long flags;
unsigned int i;
/* Resume all running pipelines. */
@@ -379,7 +380,9 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
if (pipe == NULL)
continue;
+ spin_lock_irqsave(&pipe->irqlock, flags);
if (vsp1_pipeline_ready(pipe))
vsp1_pipeline_run(pipe);
+ spin_unlock_irqrestore(&pipe->irqlock, flags);
}
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 03/13] v4l: vsp1: Ensure pipeline locking in resume path
2016-09-13 23:16 ` [PATCH 03/13] v4l: vsp1: Ensure pipeline locking in resume path Laurent Pinchart
@ 2016-09-14 18:28 ` Niklas Söderlund
0 siblings, 0 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 18:28 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 02:16:56 +0300, Laurent Pinchart wrote:
> From: Kieran Bingham <kieran+renesas@bingham.xyz>
>
> The vsp1_pipeline_ready() and vsp1_pipeline_run() functions must be
> called with the pipeline lock held, fix the resume code path.
>
> Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/media/platform/vsp1/vsp1_pipe.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c
> index 3e75fb3fcace..474de82165d8 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
> @@ -365,6 +365,7 @@ void vsp1_pipelines_suspend(struct vsp1_device *vsp1)
>
> void vsp1_pipelines_resume(struct vsp1_device *vsp1)
> {
> + unsigned long flags;
> unsigned int i;
>
> /* Resume all running pipelines. */
> @@ -379,7 +380,9 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
> if (pipe == NULL)
> continue;
>
> + spin_lock_irqsave(&pipe->irqlock, flags);
> if (vsp1_pipeline_ready(pipe))
> vsp1_pipeline_run(pipe);
> + spin_unlock_irqrestore(&pipe->irqlock, flags);
> }
> }
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 04/13] v4l: vsp1: Repair race between frame end and qbuf handler
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (2 preceding siblings ...)
2016-09-13 23:16 ` [PATCH 03/13] v4l: vsp1: Ensure pipeline locking in resume path Laurent Pinchart
@ 2016-09-13 23:16 ` Laurent Pinchart
2016-09-14 8:01 ` Kieran Bingham
2016-09-13 23:16 ` [PATCH 05/13] v4l: vsp1: Use DFE instead of FRE for frame end Laurent Pinchart
` (9 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:16 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
From: Kieran Bingham <kieran+renesas@bingham.xyz>
The frame-end function releases and completes the buffers on the input
and output entities of the pipe before marking the pipe->state as
'STOPPED'. This introduces a race whereby with the pipe->state still
'RUNNING', a QBUF handler can commence processing a frame before the
frame_end function has completed.
In the event that this happens, a frame queued by QBUF hangs due to the
incorrect pipe->state setting which prevents vsp1_pipeline_run from
issuing a CMD_STRCMD.
By locking the entire function we prevent this from occurring, but we
also change the locking state of the buffer release code. This has been
analysed visually as acceptable, but it must be considered that this now
causes the video->irqlock to be taken under the pipe->irqlock context.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_video.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index ed9759e8a6fc..cd7d215ed455 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -234,18 +234,13 @@ static void vsp1_video_frame_end(struct vsp1_pipeline *pipe,
{
struct vsp1_video *video = rwpf->video;
struct vsp1_vb2_buffer *buf;
- unsigned long flags;
buf = vsp1_video_complete_buffer(video);
if (buf == NULL)
return;
- spin_lock_irqsave(&pipe->irqlock, flags);
-
video->rwpf->mem = buf->mem;
pipe->buffers_ready |= 1 << video->pipe_index;
-
- spin_unlock_irqrestore(&pipe->irqlock, flags);
}
static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
@@ -285,6 +280,8 @@ static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe)
unsigned long flags;
unsigned int i;
+ spin_lock_irqsave(&pipe->irqlock, flags);
+
/* Complete buffers on all video nodes. */
for (i = 0; i < vsp1->info->rpf_count; ++i) {
if (!pipe->inputs[i])
@@ -295,8 +292,6 @@ static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe)
vsp1_video_frame_end(pipe, pipe->output);
- spin_lock_irqsave(&pipe->irqlock, flags);
-
state = pipe->state;
pipe->state = VSP1_PIPELINE_STOPPED;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 04/13] v4l: vsp1: Repair race between frame end and qbuf handler
2016-09-13 23:16 ` [PATCH 04/13] v4l: vsp1: Repair race between frame end and qbuf handler Laurent Pinchart
@ 2016-09-14 8:01 ` Kieran Bingham
0 siblings, 0 replies; 32+ messages in thread
From: Kieran Bingham @ 2016-09-14 8:01 UTC (permalink / raw)
To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Kieran Bingham
On 14/09/16 00:16, Laurent Pinchart wrote:
> From: Kieran Bingham <kieran+renesas@bingham.xyz>
>
> The frame-end function releases and completes the buffers on the input
> and output entities of the pipe before marking the pipe->state as
> 'STOPPED'. This introduces a race whereby with the pipe->state still
> 'RUNNING', a QBUF handler can commence processing a frame before the
> frame_end function has completed.
>
> In the event that this happens, a frame queued by QBUF hangs due to the
> incorrect pipe->state setting which prevents vsp1_pipeline_run from
> issuing a CMD_STRCMD.
>
> By locking the entire function we prevent this from occurring, but we
> also change the locking state of the buffer release code. This has been
> analysed visually as acceptable, but it must be considered that this now
> causes the video->irqlock to be taken under the pipe->irqlock context.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
This patch is of course perfectly welcome to my SoB:
Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
> ---
> drivers/media/platform/vsp1/vsp1_video.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
> index ed9759e8a6fc..cd7d215ed455 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -234,18 +234,13 @@ static void vsp1_video_frame_end(struct vsp1_pipeline *pipe,
> {
> struct vsp1_video *video = rwpf->video;
> struct vsp1_vb2_buffer *buf;
> - unsigned long flags;
>
> buf = vsp1_video_complete_buffer(video);
> if (buf == NULL)
> return;
>
> - spin_lock_irqsave(&pipe->irqlock, flags);
> -
> video->rwpf->mem = buf->mem;
> pipe->buffers_ready |= 1 << video->pipe_index;
> -
> - spin_unlock_irqrestore(&pipe->irqlock, flags);
> }
>
> static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
> @@ -285,6 +280,8 @@ static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe)
> unsigned long flags;
> unsigned int i;
>
> + spin_lock_irqsave(&pipe->irqlock, flags);
> +
> /* Complete buffers on all video nodes. */
> for (i = 0; i < vsp1->info->rpf_count; ++i) {
> if (!pipe->inputs[i])
> @@ -295,8 +292,6 @@ static void vsp1_video_pipeline_frame_end(struct vsp1_pipeline *pipe)
>
> vsp1_video_frame_end(pipe, pipe->output);
>
> - spin_lock_irqsave(&pipe->irqlock, flags);
> -
> state = pipe->state;
> pipe->state = VSP1_PIPELINE_STOPPED;
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 05/13] v4l: vsp1: Use DFE instead of FRE for frame end
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (3 preceding siblings ...)
2016-09-13 23:16 ` [PATCH 04/13] v4l: vsp1: Repair race between frame end and qbuf handler Laurent Pinchart
@ 2016-09-13 23:16 ` Laurent Pinchart
2016-09-14 18:39 ` Niklas Söderlund
2016-09-13 23:16 ` [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad Laurent Pinchart
` (8 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:16 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
From: Kieran Bingham <kieran+renesas@bingham.xyz>
The DFE and FRE interrupts are both fired at frame completion, as each
display list processes a single frame. This won't be true anymore when
using image partitioning, switch to DFE in preparation.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
---
drivers/media/platform/vsp1/vsp1_drv.c | 2 +-
drivers/media/platform/vsp1/vsp1_wpf.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index 92418fc09511..57c713a4e1df 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -60,7 +60,7 @@ static irqreturn_t vsp1_irq_handler(int irq, void *data)
status = vsp1_read(vsp1, VI6_WPF_IRQ_STA(i));
vsp1_write(vsp1, VI6_WPF_IRQ_STA(i), ~status & mask);
- if (status & VI6_WFP_IRQ_STA_FRE) {
+ if (status & VI6_WFP_IRQ_STA_DFE) {
vsp1_pipeline_frame_end(wpf->pipe);
ret = IRQ_HANDLED;
}
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index 31983169c24a..748f5af90b7e 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -318,7 +318,7 @@ static void wpf_configure(struct vsp1_entity *entity,
/* Enable interrupts */
vsp1_dl_list_write(dl, VI6_WPF_IRQ_STA(wpf->entity.index), 0);
vsp1_dl_list_write(dl, VI6_WPF_IRQ_ENB(wpf->entity.index),
- VI6_WFP_IRQ_ENB_FREE);
+ VI6_WFP_IRQ_ENB_DFEE);
}
static const struct vsp1_entity_operations wpf_entity_ops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 05/13] v4l: vsp1: Use DFE instead of FRE for frame end
2016-09-13 23:16 ` [PATCH 05/13] v4l: vsp1: Use DFE instead of FRE for frame end Laurent Pinchart
@ 2016-09-14 18:39 ` Niklas Söderlund
0 siblings, 0 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 18:39 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 02:16:58 +0300, Laurent Pinchart wrote:
> From: Kieran Bingham <kieran+renesas@bingham.xyz>
>
> The DFE and FRE interrupts are both fired at frame completion, as each
> display list processes a single frame. This won't be true anymore when
> using image partitioning, switch to DFE in preparation.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/media/platform/vsp1/vsp1_drv.c | 2 +-
> drivers/media/platform/vsp1/vsp1_wpf.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
> index 92418fc09511..57c713a4e1df 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -60,7 +60,7 @@ static irqreturn_t vsp1_irq_handler(int irq, void *data)
> status = vsp1_read(vsp1, VI6_WPF_IRQ_STA(i));
> vsp1_write(vsp1, VI6_WPF_IRQ_STA(i), ~status & mask);
>
> - if (status & VI6_WFP_IRQ_STA_FRE) {
> + if (status & VI6_WFP_IRQ_STA_DFE) {
> vsp1_pipeline_frame_end(wpf->pipe);
> ret = IRQ_HANDLED;
> }
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
> index 31983169c24a..748f5af90b7e 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -318,7 +318,7 @@ static void wpf_configure(struct vsp1_entity *entity,
> /* Enable interrupts */
> vsp1_dl_list_write(dl, VI6_WPF_IRQ_STA(wpf->entity.index), 0);
> vsp1_dl_list_write(dl, VI6_WPF_IRQ_ENB(wpf->entity.index),
> - VI6_WFP_IRQ_ENB_FREE);
> + VI6_WFP_IRQ_ENB_DFEE);
> }
>
> static const struct vsp1_entity_operations wpf_entity_ops = {
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (4 preceding siblings ...)
2016-09-13 23:16 ` [PATCH 05/13] v4l: vsp1: Use DFE instead of FRE for frame end Laurent Pinchart
@ 2016-09-13 23:16 ` Laurent Pinchart
2016-09-14 18:54 ` Niklas Söderlund
2016-09-19 17:55 ` Mauro Carvalho Chehab
2016-09-13 23:17 ` [PATCH 07/13] v4l: vsp1: Fix RPF cropping Laurent Pinchart
` (7 subsequent siblings)
13 siblings, 2 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:16 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
Cropping on the WPF sink pad restricts the left and top coordinates to
0-255. The same result can be obtained by cropping on the RPF without
any such restriction, this feature isn't useful. Disable it.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_rwpf.c | 37 +++++++++++++++++----------------
drivers/media/platform/vsp1/vsp1_wpf.c | 18 +++++++---------
2 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c b/drivers/media/platform/vsp1/vsp1_rwpf.c
index 8cb87e96b78b..a3ace8df7f4d 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
@@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
struct vsp1_rwpf *rwpf = to_rwpf(subdev);
struct v4l2_subdev_pad_config *config;
struct v4l2_mbus_framefmt *format;
- struct v4l2_rect *crop;
int ret = 0;
mutex_lock(&rwpf->entity.lock);
@@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
fmt->format = *format;
- /* Update the sink crop rectangle. */
- crop = vsp1_rwpf_get_crop(rwpf, config);
- crop->left = 0;
- crop->top = 0;
- crop->width = fmt->format.width;
- crop->height = fmt->format.height;
+ if (rwpf->entity.type == VSP1_ENTITY_RPF) {
+ struct v4l2_rect *crop;
+
+ /* Update the sink crop rectangle. */
+ crop = vsp1_rwpf_get_crop(rwpf, config);
+ crop->left = 0;
+ crop->top = 0;
+ crop->width = fmt->format.width;
+ crop->height = fmt->format.height;
+ }
/* Propagate the format to the source pad. */
format = vsp1_entity_get_pad_format(&rwpf->entity, config,
@@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
struct v4l2_mbus_framefmt *format;
int ret = 0;
- /* Cropping is implemented on the sink pad. */
- if (sel->pad != RWPF_PAD_SINK)
+ /* Cropping is only supported on the RPF and is implemented on the sink
+ * pad.
+ */
+ if (rwpf->entity.type == VSP1_ENTITY_WPF || sel->pad != RWPF_PAD_SINK)
return -EINVAL;
mutex_lock(&rwpf->entity.lock);
@@ -175,8 +180,10 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
struct v4l2_rect *crop;
int ret = 0;
- /* Cropping is implemented on the sink pad. */
- if (sel->pad != RWPF_PAD_SINK)
+ /* Cropping is only supported on the RPF and is implemented on the sink
+ * pad.
+ */
+ if (rwpf->entity.type == VSP1_ENTITY_WPF || sel->pad != RWPF_PAD_SINK)
return -EINVAL;
if (sel->target != V4L2_SEL_TGT_CROP)
@@ -190,9 +197,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
goto done;
}
- /* Make sure the crop rectangle is entirely contained in the image. The
- * WPF top and left offsets are limited to 255.
- */
+ /* Make sure the crop rectangle is entirely contained in the image. */
format = vsp1_entity_get_pad_format(&rwpf->entity, config,
RWPF_PAD_SINK);
@@ -208,10 +213,6 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
sel->r.left = min_t(unsigned int, sel->r.left, format->width - 2);
sel->r.top = min_t(unsigned int, sel->r.top, format->height - 2);
- if (rwpf->entity.type == VSP1_ENTITY_WPF) {
- sel->r.left = min_t(unsigned int, sel->r.left, 255);
- sel->r.top = min_t(unsigned int, sel->r.top, 255);
- }
sel->r.width = min_t(unsigned int, sel->r.width,
format->width - sel->r.left);
sel->r.height = min_t(unsigned int, sel->r.height,
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index 748f5af90b7e..f3a593196282 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -212,7 +212,6 @@ static void wpf_configure(struct vsp1_entity *entity,
struct vsp1_device *vsp1 = wpf->entity.vsp1;
const struct v4l2_mbus_framefmt *source_format;
const struct v4l2_mbus_framefmt *sink_format;
- const struct v4l2_rect *crop;
unsigned int i;
u32 outfmt = 0;
u32 srcrpf = 0;
@@ -237,16 +236,6 @@ static void wpf_configure(struct vsp1_entity *entity,
return;
}
- /* Cropping */
- crop = vsp1_rwpf_get_crop(wpf, wpf->entity.config);
-
- vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
- (crop->left << VI6_WPF_SZCLIP_OFST_SHIFT) |
- (crop->width << VI6_WPF_SZCLIP_SIZE_SHIFT));
- vsp1_wpf_write(wpf, dl, VI6_WPF_VSZCLIP, VI6_WPF_SZCLIP_EN |
- (crop->top << VI6_WPF_SZCLIP_OFST_SHIFT) |
- (crop->height << VI6_WPF_SZCLIP_SIZE_SHIFT));
-
/* Format */
sink_format = vsp1_entity_get_pad_format(&wpf->entity,
wpf->entity.config,
@@ -255,6 +244,13 @@ static void wpf_configure(struct vsp1_entity *entity,
wpf->entity.config,
RWPF_PAD_SOURCE);
+ vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
+ (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
+ (source_format->width << VI6_WPF_SZCLIP_SIZE_SHIFT));
+ vsp1_wpf_write(wpf, dl, VI6_WPF_VSZCLIP, VI6_WPF_SZCLIP_EN |
+ (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
+ (source_format->height << VI6_WPF_SZCLIP_SIZE_SHIFT));
+
if (!pipe->lif) {
const struct v4l2_pix_format_mplane *format = &wpf->format;
const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad
2016-09-13 23:16 ` [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad Laurent Pinchart
@ 2016-09-14 18:54 ` Niklas Söderlund
2016-09-19 17:55 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 18:54 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 02:16:59 +0300, Laurent Pinchart wrote:
> Cropping on the WPF sink pad restricts the left and top coordinates to
> 0-255. The same result can be obtained by cropping on the RPF without
> any such restriction, this feature isn't useful. Disable it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/media/platform/vsp1/vsp1_rwpf.c | 37 +++++++++++++++++----------------
> drivers/media/platform/vsp1/vsp1_wpf.c | 18 +++++++---------
> 2 files changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c b/drivers/media/platform/vsp1/vsp1_rwpf.c
> index 8cb87e96b78b..a3ace8df7f4d 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> - struct v4l2_rect *crop;
> int ret = 0;
>
> mutex_lock(&rwpf->entity.lock);
> @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
>
> fmt->format = *format;
>
> - /* Update the sink crop rectangle. */
> - crop = vsp1_rwpf_get_crop(rwpf, config);
> - crop->left = 0;
> - crop->top = 0;
> - crop->width = fmt->format.width;
> - crop->height = fmt->format.height;
> + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> + struct v4l2_rect *crop;
> +
> + /* Update the sink crop rectangle. */
> + crop = vsp1_rwpf_get_crop(rwpf, config);
> + crop->left = 0;
> + crop->top = 0;
> + crop->width = fmt->format.width;
> + crop->height = fmt->format.height;
> + }
>
> /* Propagate the format to the source pad. */
> format = vsp1_entity_get_pad_format(&rwpf->entity, config,
> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> struct v4l2_mbus_framefmt *format;
> int ret = 0;
>
> - /* Cropping is implemented on the sink pad. */
> - if (sel->pad != RWPF_PAD_SINK)
> + /* Cropping is only supported on the RPF and is implemented on the sink
> + * pad.
> + */
> + if (rwpf->entity.type == VSP1_ENTITY_WPF || sel->pad != RWPF_PAD_SINK)
> return -EINVAL;
>
> mutex_lock(&rwpf->entity.lock);
> @@ -175,8 +180,10 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> struct v4l2_rect *crop;
> int ret = 0;
>
> - /* Cropping is implemented on the sink pad. */
> - if (sel->pad != RWPF_PAD_SINK)
> + /* Cropping is only supported on the RPF and is implemented on the sink
> + * pad.
> + */
> + if (rwpf->entity.type == VSP1_ENTITY_WPF || sel->pad != RWPF_PAD_SINK)
> return -EINVAL;
>
> if (sel->target != V4L2_SEL_TGT_CROP)
> @@ -190,9 +197,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> goto done;
> }
>
> - /* Make sure the crop rectangle is entirely contained in the image. The
> - * WPF top and left offsets are limited to 255.
> - */
> + /* Make sure the crop rectangle is entirely contained in the image. */
> format = vsp1_entity_get_pad_format(&rwpf->entity, config,
> RWPF_PAD_SINK);
>
> @@ -208,10 +213,6 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
>
> sel->r.left = min_t(unsigned int, sel->r.left, format->width - 2);
> sel->r.top = min_t(unsigned int, sel->r.top, format->height - 2);
> - if (rwpf->entity.type == VSP1_ENTITY_WPF) {
> - sel->r.left = min_t(unsigned int, sel->r.left, 255);
> - sel->r.top = min_t(unsigned int, sel->r.top, 255);
> - }
> sel->r.width = min_t(unsigned int, sel->r.width,
> format->width - sel->r.left);
> sel->r.height = min_t(unsigned int, sel->r.height,
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
> index 748f5af90b7e..f3a593196282 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -212,7 +212,6 @@ static void wpf_configure(struct vsp1_entity *entity,
> struct vsp1_device *vsp1 = wpf->entity.vsp1;
> const struct v4l2_mbus_framefmt *source_format;
> const struct v4l2_mbus_framefmt *sink_format;
> - const struct v4l2_rect *crop;
> unsigned int i;
> u32 outfmt = 0;
> u32 srcrpf = 0;
> @@ -237,16 +236,6 @@ static void wpf_configure(struct vsp1_entity *entity,
> return;
> }
>
> - /* Cropping */
> - crop = vsp1_rwpf_get_crop(wpf, wpf->entity.config);
> -
> - vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
> - (crop->left << VI6_WPF_SZCLIP_OFST_SHIFT) |
> - (crop->width << VI6_WPF_SZCLIP_SIZE_SHIFT));
> - vsp1_wpf_write(wpf, dl, VI6_WPF_VSZCLIP, VI6_WPF_SZCLIP_EN |
> - (crop->top << VI6_WPF_SZCLIP_OFST_SHIFT) |
> - (crop->height << VI6_WPF_SZCLIP_SIZE_SHIFT));
> -
> /* Format */
> sink_format = vsp1_entity_get_pad_format(&wpf->entity,
> wpf->entity.config,
> @@ -255,6 +244,13 @@ static void wpf_configure(struct vsp1_entity *entity,
> wpf->entity.config,
> RWPF_PAD_SOURCE);
>
> + vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
> + (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
> + (source_format->width << VI6_WPF_SZCLIP_SIZE_SHIFT));
> + vsp1_wpf_write(wpf, dl, VI6_WPF_VSZCLIP, VI6_WPF_SZCLIP_EN |
> + (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
> + (source_format->height << VI6_WPF_SZCLIP_SIZE_SHIFT));
> +
> if (!pipe->lif) {
> const struct v4l2_pix_format_mplane *format = &wpf->format;
> const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad
2016-09-13 23:16 ` [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad Laurent Pinchart
2016-09-14 18:54 ` Niklas Söderlund
@ 2016-09-19 17:55 ` Mauro Carvalho Chehab
2016-09-19 17:59 ` Laurent Pinchart
1 sibling, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2016-09-19 17:55 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
Em Wed, 14 Sep 2016 02:16:59 +0300
Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> escreveu:
> Cropping on the WPF sink pad restricts the left and top coordinates to
> 0-255. The same result can be obtained by cropping on the RPF without
> any such restriction, this feature isn't useful. Disable it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/media/platform/vsp1/vsp1_rwpf.c | 37 +++++++++++++++++----------------
> drivers/media/platform/vsp1/vsp1_wpf.c | 18 +++++++---------
> 2 files changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c b/drivers/media/platform/vsp1/vsp1_rwpf.c
> index 8cb87e96b78b..a3ace8df7f4d 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> struct v4l2_subdev_pad_config *config;
> struct v4l2_mbus_framefmt *format;
> - struct v4l2_rect *crop;
> int ret = 0;
>
> mutex_lock(&rwpf->entity.lock);
> @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
>
> fmt->format = *format;
>
> - /* Update the sink crop rectangle. */
> - crop = vsp1_rwpf_get_crop(rwpf, config);
> - crop->left = 0;
> - crop->top = 0;
> - crop->width = fmt->format.width;
> - crop->height = fmt->format.height;
> + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> + struct v4l2_rect *crop;
> +
> + /* Update the sink crop rectangle. */
> + crop = vsp1_rwpf_get_crop(rwpf, config);
> + crop->left = 0;
> + crop->top = 0;
> + crop->width = fmt->format.width;
> + crop->height = fmt->format.height;
> + }
>
> /* Propagate the format to the source pad. */
> format = vsp1_entity_get_pad_format(&rwpf->entity, config,
> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
> struct v4l2_mbus_framefmt *format;
> int ret = 0;
>
> - /* Cropping is implemented on the sink pad. */
> - if (sel->pad != RWPF_PAD_SINK)
> + /* Cropping is only supported on the RPF and is implemented on the sink
> + * pad.
> + */
Please read CodingStyle and run checkpatch before sending stuff upstream.
This violates the CodingStyle: it should be, instead:
/*
* foo
* bar
*/
This time, I'll fix it, but next time I might not have enough time, and
need to reject the patch series.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad
2016-09-19 17:55 ` Mauro Carvalho Chehab
@ 2016-09-19 17:59 ` Laurent Pinchart
2016-09-19 18:26 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-19 17:59 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, Kieran Bingham
Hi Mauro,
On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> > Cropping on the WPF sink pad restricts the left and top coordinates to
> > 0-255. The same result can be obtained by cropping on the RPF without
> > any such restriction, this feature isn't useful. Disable it.
> >
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >
> > drivers/media/platform/vsp1/vsp1_rwpf.c | 37
> > +++++++++++++++++---------------- drivers/media/platform/vsp1/vsp1_wpf.c
> > | 18 +++++++---------
> > 2 files changed, 26 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > 8cb87e96b78b..a3ace8df7f4d 100644
> > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > *subdev,>
> > struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> >
> > - struct v4l2_rect *crop;
> >
> > int ret = 0;
> >
> > mutex_lock(&rwpf->entity.lock);
> >
> > @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > *subdev,>
> > fmt->format = *format;
> >
> > - /* Update the sink crop rectangle. */
> > - crop = vsp1_rwpf_get_crop(rwpf, config);
> > - crop->left = 0;
> > - crop->top = 0;
> > - crop->width = fmt->format.width;
> > - crop->height = fmt->format.height;
> > + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> > + struct v4l2_rect *crop;
> > +
> > + /* Update the sink crop rectangle. */
> > + crop = vsp1_rwpf_get_crop(rwpf, config);
> > + crop->left = 0;
> > + crop->top = 0;
> > + crop->width = fmt->format.width;
> > + crop->height = fmt->format.height;
> > + }
> >
> > /* Propagate the format to the source pad. */
> > format = vsp1_entity_get_pad_format(&rwpf->entity, config,
> >
> > @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev
> > *subdev,>
> > struct v4l2_mbus_framefmt *format;
> > int ret = 0;
> >
> > - /* Cropping is implemented on the sink pad. */
> > - if (sel->pad != RWPF_PAD_SINK)
> > + /* Cropping is only supported on the RPF and is implemented on the
sink
> > + * pad.
> > + */
>
> Please read CodingStyle and run checkpatch before sending stuff upstream.
>
> This violates the CodingStyle: it should be, instead:
> /*
> * foo
> * bar
> */
But it's consistent with the coding style of this driver. I'm OK fixing it,
but it should be done globally in that case.
> This time, I'll fix it, but next time I might not have enough time, and
> need to reject the patch series.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad
2016-09-19 17:59 ` Laurent Pinchart
@ 2016-09-19 18:26 ` Mauro Carvalho Chehab
2016-09-19 18:33 ` Laurent Pinchart
0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2016-09-19 18:26 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, Kieran Bingham
Em Mon, 19 Sep 2016 20:59:56 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:
> Hi Mauro,
>
> On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> > Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> > > Cropping on the WPF sink pad restricts the left and top coordinates to
> > > 0-255. The same result can be obtained by cropping on the RPF without
> > > any such restriction, this feature isn't useful. Disable it.
> > >
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >
> > > drivers/media/platform/vsp1/vsp1_rwpf.c | 37
> > > +++++++++++++++++---------------- drivers/media/platform/vsp1/vsp1_wpf.c
> > > | 18 +++++++---------
> > > 2 files changed, 26 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > > 8cb87e96b78b..a3ace8df7f4d 100644
> > > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > > *subdev,>
> > > struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> > > struct v4l2_subdev_pad_config *config;
> > > struct v4l2_mbus_framefmt *format;
> > >
> > > - struct v4l2_rect *crop;
> > >
> > > int ret = 0;
> > >
> > > mutex_lock(&rwpf->entity.lock);
> > >
> > > @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > > *subdev,>
> > > fmt->format = *format;
> > >
> > > - /* Update the sink crop rectangle. */
> > > - crop = vsp1_rwpf_get_crop(rwpf, config);
> > > - crop->left = 0;
> > > - crop->top = 0;
> > > - crop->width = fmt->format.width;
> > > - crop->height = fmt->format.height;
> > > + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> > > + struct v4l2_rect *crop;
> > > +
> > > + /* Update the sink crop rectangle. */
> > > + crop = vsp1_rwpf_get_crop(rwpf, config);
> > > + crop->left = 0;
> > > + crop->top = 0;
> > > + crop->width = fmt->format.width;
> > > + crop->height = fmt->format.height;
> > > + }
> > >
> > > /* Propagate the format to the source pad. */
> > > format = vsp1_entity_get_pad_format(&rwpf->entity, config,
> > >
> > > @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev
> > > *subdev,>
> > > struct v4l2_mbus_framefmt *format;
> > > int ret = 0;
> > >
> > > - /* Cropping is implemented on the sink pad. */
> > > - if (sel->pad != RWPF_PAD_SINK)
> > > + /* Cropping is only supported on the RPF and is implemented on the
> sink
> > > + * pad.
> > > + */
> >
> > Please read CodingStyle and run checkpatch before sending stuff upstream.
> >
> > This violates the CodingStyle: it should be, instead:
> > /*
> > * foo
> > * bar
> > */
>
> But it's consistent with the coding style of this driver. I'm OK fixing it,
> but it should be done globally in that case.
There are inconsistencies inside the driver too on multi-line
comments even without fixing the ones introduced on this series,
as, on several places, multi-line comments are correct:
drivers/media/platform/vsp1/vsp1_bru.c:/*
drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format conversion, all sink and source formats must be
drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on the first sink pad (pad 0) and propagate it
drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
drivers/media/platform/vsp1/vsp1_bru.c- */
drivers/media/platform/vsp1/vsp1_dl.c:/*
drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body object and allocate DMA memory for the body
drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object is expected to have been initialized to
drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
drivers/media/platform/vsp1/vsp1_dl.c- */
...
I'll address the ones only the CodingStyle violation introduced by this
series. I'll leave for the vsp1 maintainers/developers.
Btw, there are several kernel-doc tags that use just:
/*
...
*/
instead of:
/**
...
*/
I suggest you to add the files/headers with kernel-doc markups on
a Documentation/media/v4l-drivers/vsp1.rst file, to be created.
This way, you can validate that such documentation is correct,
and produce an auto-generated documentation for this driver.
Regards,
Mauro
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad
2016-09-19 18:26 ` Mauro Carvalho Chehab
@ 2016-09-19 18:33 ` Laurent Pinchart
2016-09-19 19:02 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-19 18:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, Kieran Bingham
Hi Mauro,
On Monday 19 Sep 2016 15:26:15 Mauro Carvalho Chehab wrote:
> Em Mon, 19 Sep 2016 20:59:56 +0300 Laurent Pinchart escreveu:
> > On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> >> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> >>> Cropping on the WPF sink pad restricts the left and top coordinates to
> >>> 0-255. The same result can be obtained by cropping on the RPF without
> >>> any such restriction, this feature isn't useful. Disable it.
> >>>
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>>
> >>> drivers/media/platform/vsp1/vsp1_rwpf.c | 37 ++++++++++++------------
> >>> drivers/media/platform/vsp1/vsp1_wpf.c | 18 +++++++---------
> >>> 2 files changed, 26 insertions(+), 29 deletions(-)
> >>>
> >>> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> >>> b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> >>> 8cb87e96b78b..a3ace8df7f4d 100644
> >>> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> >>> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
[snip]
> >>> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct
> >>> v4l2_subdev *subdev,
> >>> struct v4l2_mbus_framefmt *format;
> >>> int ret = 0;
> >>>
> >>> - /* Cropping is implemented on the sink pad. */
> >>> - if (sel->pad != RWPF_PAD_SINK)
> >>> + /* Cropping is only supported on the RPF and is implemented on
> >>> the sink
> >>> + * pad.
> >>> + */
> >>
> >> Please read CodingStyle and run checkpatch before sending stuff
> >> upstream.
> >>
> >> This violates the CodingStyle: it should be, instead:
> >> /*
> >> * foo
> >> * bar
> >> */
> >
> > But it's consistent with the coding style of this driver. I'm OK fixing
> > it, but it should be done globally in that case.
>
> There are inconsistencies inside the driver too on multi-line
> comments even without fixing the ones introduced on this series,
> as, on several places, multi-line comments are correct:
>
> drivers/media/platform/vsp1/vsp1_bru.c:/*
> drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format
> conversion, all sink and source formats must be
> drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on
> the first sink pad (pad 0) and propagate it
> drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
> drivers/media/platform/vsp1/vsp1_bru.c- */
>
> drivers/media/platform/vsp1/vsp1_dl.c:/*
> drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body
> object and allocate DMA memory for the body
> drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object
> is expected to have been initialized to
> drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
> drivers/media/platform/vsp1/vsp1_dl.c- */
>
> ...
>
> I'll address the ones only the CodingStyle violation introduced by this
> series. I'll leave for the vsp1 maintainers/developers.
OK, I'll address that.
> Btw, there are several kernel-doc tags that use just:
> /*
> ...
> */
>
> instead of:
>
> /**
> ...
> */
>
> I suggest you to add the files/headers with kernel-doc markups on
> a Documentation/media/v4l-drivers/vsp1.rst file, to be created.
>
> This way, you can validate that such documentation is correct,
> and produce an auto-generated documentation for this driver.
I've thought about it, but I don't think those comments should become part of
the kernel documentation. They're really about driver internals, and meant for
the driver developers. In particular only a subset of the driver is documented
that way, when I've considered that the code or structures were complex enough
to need proper documentation. A generated doc would then be quite incomplete
and not very useful, the comments are meant to be read while working on the
code.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad
2016-09-19 18:33 ` Laurent Pinchart
@ 2016-09-19 19:02 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2016-09-19 19:02 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, Kieran Bingham
Em Mon, 19 Sep 2016 21:33:13 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:
> Hi Mauro,
>
> On Monday 19 Sep 2016 15:26:15 Mauro Carvalho Chehab wrote:
> > Em Mon, 19 Sep 2016 20:59:56 +0300 Laurent Pinchart escreveu:
> > > On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> > >> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> > >>> Cropping on the WPF sink pad restricts the left and top coordinates to
> > >>> 0-255. The same result can be obtained by cropping on the RPF without
> > >>> any such restriction, this feature isn't useful. Disable it.
> > >>>
> > >>> Signed-off-by: Laurent Pinchart
> > >>> <laurent.pinchart+renesas@ideasonboard.com>
> > >>> ---
> > >>>
> > >>> drivers/media/platform/vsp1/vsp1_rwpf.c | 37 ++++++++++++------------
> > >>> drivers/media/platform/vsp1/vsp1_wpf.c | 18 +++++++---------
> > >>> 2 files changed, 26 insertions(+), 29 deletions(-)
> > >>>
> > >>> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > >>> b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > >>> 8cb87e96b78b..a3ace8df7f4d 100644
> > >>> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > >>> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
>
> [snip]
>
> > >>> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct
> > >>> v4l2_subdev *subdev,
> > >>> struct v4l2_mbus_framefmt *format;
> > >>> int ret = 0;
> > >>>
> > >>> - /* Cropping is implemented on the sink pad. */
> > >>> - if (sel->pad != RWPF_PAD_SINK)
> > >>> + /* Cropping is only supported on the RPF and is implemented on
> > >>> the sink
> > >>> + * pad.
> > >>> + */
> > >>
> > >> Please read CodingStyle and run checkpatch before sending stuff
> > >> upstream.
> > >>
> > >> This violates the CodingStyle: it should be, instead:
> > >> /*
> > >> * foo
> > >> * bar
> > >> */
> > >
> > > But it's consistent with the coding style of this driver. I'm OK fixing
> > > it, but it should be done globally in that case.
> >
> > There are inconsistencies inside the driver too on multi-line
> > comments even without fixing the ones introduced on this series,
> > as, on several places, multi-line comments are correct:
> >
> > drivers/media/platform/vsp1/vsp1_bru.c:/*
> > drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format
> > conversion, all sink and source formats must be
> > drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on
> > the first sink pad (pad 0) and propagate it
> > drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
> > drivers/media/platform/vsp1/vsp1_bru.c- */
> >
> > drivers/media/platform/vsp1/vsp1_dl.c:/*
> > drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body
> > object and allocate DMA memory for the body
> > drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object
> > is expected to have been initialized to
> > drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
> > drivers/media/platform/vsp1/vsp1_dl.c- */
> >
> > ...
> >
> > I'll address the ones only the CodingStyle violation introduced by this
> > series. I'll leave for the vsp1 maintainers/developers.
>
> OK, I'll address that.
>
> > Btw, there are several kernel-doc tags that use just:
> > /*
> > ...
> > */
> >
> > instead of:
> >
> > /**
> > ...
> > */
> >
> > I suggest you to add the files/headers with kernel-doc markups on
> > a Documentation/media/v4l-drivers/vsp1.rst file, to be created.
> >
> > This way, you can validate that such documentation is correct,
> > and produce an auto-generated documentation for this driver.
>
> I've thought about it, but I don't think those comments should become part of
> the kernel documentation. They're really about driver internals, and meant for
> the driver developers. In particular only a subset of the driver is documented
> that way, when I've considered that the code or structures were complex enough
> to need proper documentation. A generated doc would then be quite incomplete
> and not very useful, the comments are meant to be read while working on the
> code.
The v4l-drivers book is meant to have driver internals documentation,
and not the subsystem kAPI or uAPI.
I don't see any problems if you want to document just the more complex
functions/structs over the v4l-drivers/ book. Yet, as you already took
the time to write documentation for those functions, providing that the
kernel-doc markups are ok, a v4l-drivers/vsp1.rst file for it could be as
simple as (untested):
VSP1 driver
^^^^^^^^^^^
.. kernel-doc:: drivers/media/platform/vsp1/vsp1_dl.c
.. kernel-doc:: drivers/media/platform/vsp1/vsp1_drm.c
.. kernel-doc:: drivers/media/platform/vsp1/vsp1_drm.h
.. kernel-doc:: drivers/media/platform/vsp1/vsp1_entity.c
.. kernel-doc:: drivers/media/platform/vsp1/vsp1_entity.h
.. kernel-doc:: drivers/media/platform/vsp1/vsp1_pipe.c
.. kernel-doc:: drivers/media/platform/vsp1/vsp1_video.c
PS.: Eventually, you may need an extra attribute for the files with
EXPORT_SYMBOL*, in order to associate a *.c file with the
corresponding *.h file.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 07/13] v4l: vsp1: Fix RPF cropping
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (5 preceding siblings ...)
2016-09-13 23:16 ` [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad Laurent Pinchart
@ 2016-09-13 23:17 ` Laurent Pinchart
2016-09-13 23:17 ` [PATCH 08/13] v4l: vsp1: Pass parameter type to entity configuration operation Laurent Pinchart
` (6 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:17 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
The RPF cropping offset for the chroma planes is incorrectly computed,
it needs to be divided by the horizontal subsampling factor.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_rpf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 388838913205..3d6669dbeacf 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -105,7 +105,8 @@ static void rpf_configure(struct vsp1_entity *entity,
if (format->num_planes > 1) {
rpf->offsets[1] = crop->top * format->plane_fmt[1].bytesperline
- + crop->left * fmtinfo->bpp[1] / 8;
+ + crop->left / fmtinfo->hsub * fmtinfo->bpp[1]
+ / 8;
pstride |= format->plane_fmt[1].bytesperline
<< VI6_RPF_SRCM_PSTRIDE_C_SHIFT;
} else {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 08/13] v4l: vsp1: Pass parameter type to entity configuration operation
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (6 preceding siblings ...)
2016-09-13 23:17 ` [PATCH 07/13] v4l: vsp1: Fix RPF cropping Laurent Pinchart
@ 2016-09-13 23:17 ` Laurent Pinchart
2016-09-14 19:02 ` Niklas Söderlund
2016-09-13 23:17 ` [PATCH 09/13] v4l: vsp1: Replace .set_memory() with VSP1_ENTITY_PARAMS_PARTITION Laurent Pinchart
` (5 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:17 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
Replace the current boolean parameter (full / !full) with an explicit
enum.
- VSP1_ENTITY_PARAMS_INIT for parameters to be configured at pipeline
initialization time only (V4L2 stream on or DRM atomic update)
- VSP1_ENTITY_PARAMS_RUNTIME for all parameters that can be freely
modified at runtime (through V4L2 controls)
This will allow future extensions when implementing image partitioning
support.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_bru.c | 5 ++--
drivers/media/platform/vsp1/vsp1_clu.c | 43 +++++++++++++++++--------------
drivers/media/platform/vsp1/vsp1_drm.c | 6 +++--
drivers/media/platform/vsp1/vsp1_entity.h | 12 ++++++++-
drivers/media/platform/vsp1/vsp1_hsit.c | 5 ++--
drivers/media/platform/vsp1/vsp1_lif.c | 5 ++--
drivers/media/platform/vsp1/vsp1_lut.c | 24 ++++++++++-------
drivers/media/platform/vsp1/vsp1_rpf.c | 5 ++--
drivers/media/platform/vsp1/vsp1_sru.c | 5 ++--
drivers/media/platform/vsp1/vsp1_uds.c | 5 ++--
drivers/media/platform/vsp1/vsp1_video.c | 6 +++--
drivers/media/platform/vsp1/vsp1_wpf.c | 5 ++--
12 files changed, 78 insertions(+), 48 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_bru.c b/drivers/media/platform/vsp1/vsp1_bru.c
index 26b9e2282a41..80fb948860d5 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -285,14 +285,15 @@ static const struct v4l2_subdev_ops bru_ops = {
static void bru_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
struct vsp1_bru *bru = to_bru(&entity->subdev);
struct v4l2_mbus_framefmt *format;
unsigned int flags;
unsigned int i;
- if (!full)
+ if (params != VSP1_ENTITY_PARAMS_INIT)
return;
format = vsp1_entity_get_pad_format(&bru->entity, bru->entity.config,
diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c
index e1fd03811dda..a0a69dfc38fc 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -214,42 +214,47 @@ static const struct v4l2_subdev_ops clu_ops = {
static void clu_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
struct vsp1_clu *clu = to_clu(&entity->subdev);
struct vsp1_dl_body *dlb;
unsigned long flags;
u32 ctrl = VI6_CLU_CTRL_AAI | VI6_CLU_CTRL_MVS | VI6_CLU_CTRL_EN;
- /* The format can't be changed during streaming, only verify it at
- * stream start and store the information internally for future partial
- * reconfiguration calls.
- */
- if (full) {
+ switch (params) {
+ case VSP1_ENTITY_PARAMS_INIT: {
+ /* The format can't be changed during streaming, only verify it
+ * at setup time and store the information internally for future
+ * runtime configuration calls.
+ */
struct v4l2_mbus_framefmt *format;
format = vsp1_entity_get_pad_format(&clu->entity,
clu->entity.config,
CLU_PAD_SINK);
clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
- return;
+ break;
}
- /* 2D mode can only be used with the YCbCr pixel encoding. */
- if (clu->mode == V4L2_CID_VSP1_CLU_MODE_2D && clu->yuv_mode)
- ctrl |= VI6_CLU_CTRL_AX1I_2D | VI6_CLU_CTRL_AX2I_2D
- | VI6_CLU_CTRL_OS0_2D | VI6_CLU_CTRL_OS1_2D
- | VI6_CLU_CTRL_OS2_2D | VI6_CLU_CTRL_M2D;
+ case VSP1_ENTITY_PARAMS_RUNTIME:
+ /* 2D mode can only be used with the YCbCr pixel encoding. */
+ if (clu->mode == V4L2_CID_VSP1_CLU_MODE_2D && clu->yuv_mode)
+ ctrl |= VI6_CLU_CTRL_AX1I_2D | VI6_CLU_CTRL_AX2I_2D
+ | VI6_CLU_CTRL_OS0_2D | VI6_CLU_CTRL_OS1_2D
+ | VI6_CLU_CTRL_OS2_2D | VI6_CLU_CTRL_M2D;
- vsp1_clu_write(clu, dl, VI6_CLU_CTRL, ctrl);
+ vsp1_clu_write(clu, dl, VI6_CLU_CTRL, ctrl);
- spin_lock_irqsave(&clu->lock, flags);
- dlb = clu->clu;
- clu->clu = NULL;
- spin_unlock_irqrestore(&clu->lock, flags);
+ spin_lock_irqsave(&clu->lock, flags);
+ dlb = clu->clu;
+ clu->clu = NULL;
+ spin_unlock_irqrestore(&clu->lock, flags);
- if (dlb)
- vsp1_dl_list_add_fragment(dl, dlb);
+ if (dlb)
+ vsp1_dl_list_add_fragment(dl, dlb);
+ break;
+ }
}
static const struct vsp1_entity_operations clu_entity_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index 06972f612263..6cbd3aeedbe3 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -492,8 +492,10 @@ void vsp1_du_atomic_flush(struct device *dev)
vsp1_entity_route_setup(entity, pipe->dl);
if (entity->ops->configure) {
- entity->ops->configure(entity, pipe, pipe->dl, true);
- entity->ops->configure(entity, pipe, pipe->dl, false);
+ entity->ops->configure(entity, pipe, pipe->dl,
+ VSP1_ENTITY_PARAMS_INIT);
+ entity->ops->configure(entity, pipe, pipe->dl,
+ VSP1_ENTITY_PARAMS_RUNTIME);
}
/* The memory buffer address must be applied after configuring
diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
index b5e4dbb1f7d4..51835e73308d 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/vsp1/vsp1_entity.h
@@ -35,6 +35,16 @@ enum vsp1_entity_type {
VSP1_ENTITY_WPF,
};
+/*
+ * enum vsp1_entity_params - Entity configuration parameters class
+ * @VSP1_ENTITY_PARAMS_INIT - Initial parameters
+ * @VSP1_ENTITY_PARAMS_RUNTIME - Runtime-configurable parameters
+ */
+enum vsp1_entity_params {
+ VSP1_ENTITY_PARAMS_INIT,
+ VSP1_ENTITY_PARAMS_RUNTIME,
+};
+
#define VSP1_ENTITY_MAX_INPUTS 5 /* For the BRU */
/*
@@ -73,7 +83,7 @@ struct vsp1_entity_operations {
void (*destroy)(struct vsp1_entity *);
void (*set_memory)(struct vsp1_entity *, struct vsp1_dl_list *dl);
void (*configure)(struct vsp1_entity *, struct vsp1_pipeline *,
- struct vsp1_dl_list *, bool);
+ struct vsp1_dl_list *, enum vsp1_entity_params);
};
struct vsp1_entity {
diff --git a/drivers/media/platform/vsp1/vsp1_hsit.c b/drivers/media/platform/vsp1/vsp1_hsit.c
index 6ffbedb5c095..94316afc54ff 100644
--- a/drivers/media/platform/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/vsp1/vsp1_hsit.c
@@ -132,11 +132,12 @@ static const struct v4l2_subdev_ops hsit_ops = {
static void hsit_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
struct vsp1_hsit *hsit = to_hsit(&entity->subdev);
- if (!full)
+ if (params != VSP1_ENTITY_PARAMS_INIT)
return;
if (hsit->inverse)
diff --git a/drivers/media/platform/vsp1/vsp1_lif.c b/drivers/media/platform/vsp1/vsp1_lif.c
index 702df863b13a..e32acae1fc6e 100644
--- a/drivers/media/platform/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/vsp1/vsp1_lif.c
@@ -129,7 +129,8 @@ static const struct v4l2_subdev_ops lif_ops = {
static void lif_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
const struct v4l2_mbus_framefmt *format;
struct vsp1_lif *lif = to_lif(&entity->subdev);
@@ -137,7 +138,7 @@ static void lif_configure(struct vsp1_entity *entity,
unsigned int obth = 400;
unsigned int lbth = 200;
- if (!full)
+ if (params != VSP1_ENTITY_PARAMS_INIT)
return;
format = vsp1_entity_get_pad_format(&lif->entity, lif->entity.config,
diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c
index e1c0bb7535e4..ace8acce2076 100644
--- a/drivers/media/platform/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/vsp1/vsp1_lut.c
@@ -190,24 +190,28 @@ static const struct v4l2_subdev_ops lut_ops = {
static void lut_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
struct vsp1_lut *lut = to_lut(&entity->subdev);
struct vsp1_dl_body *dlb;
unsigned long flags;
- if (full) {
+ switch (params) {
+ case VSP1_ENTITY_PARAMS_INIT:
vsp1_lut_write(lut, dl, VI6_LUT_CTRL, VI6_LUT_CTRL_EN);
- return;
- }
+ break;
- spin_lock_irqsave(&lut->lock, flags);
- dlb = lut->lut;
- lut->lut = NULL;
- spin_unlock_irqrestore(&lut->lock, flags);
+ case VSP1_ENTITY_PARAMS_RUNTIME:
+ spin_lock_irqsave(&lut->lock, flags);
+ dlb = lut->lut;
+ lut->lut = NULL;
+ spin_unlock_irqrestore(&lut->lock, flags);
- if (dlb)
- vsp1_dl_list_add_fragment(dl, dlb);
+ if (dlb)
+ vsp1_dl_list_add_fragment(dl, dlb);
+ break;
+ }
}
static const struct vsp1_entity_operations lut_entity_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 3d6669dbeacf..795bf0fd1761 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -60,7 +60,8 @@ static void rpf_set_memory(struct vsp1_entity *entity, struct vsp1_dl_list *dl)
static void rpf_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
const struct vsp1_format_info *fmtinfo = rpf->fmtinfo;
@@ -73,7 +74,7 @@ static void rpf_configure(struct vsp1_entity *entity,
u32 pstride;
u32 infmt;
- if (!full) {
+ if (params == VSP1_ENTITY_PARAMS_RUNTIME) {
vsp1_rpf_write(rpf, dl, VI6_RPF_VRTCOL_SET,
rpf->alpha << VI6_RPF_VRTCOL_SET_LAYA_SHIFT);
vsp1_rpf_write(rpf, dl, VI6_RPF_MULT_ALPHA, rpf->mult_alpha |
diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
index 6e13cdfa5ed4..9d4a1afb6634 100644
--- a/drivers/media/platform/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/vsp1/vsp1_sru.c
@@ -271,7 +271,8 @@ static const struct v4l2_subdev_ops sru_ops = {
static void sru_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
const struct vsp1_sru_param *param;
struct vsp1_sru *sru = to_sru(&entity->subdev);
@@ -279,7 +280,7 @@ static void sru_configure(struct vsp1_entity *entity,
struct v4l2_mbus_framefmt *output;
u32 ctrl0;
- if (!full)
+ if (params != VSP1_ENTITY_PARAMS_INIT)
return;
input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
index a8fc893a31ee..62beae5d6944 100644
--- a/drivers/media/platform/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/vsp1/vsp1_uds.c
@@ -260,7 +260,8 @@ static const struct v4l2_subdev_ops uds_ops = {
static void uds_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
struct vsp1_uds *uds = to_uds(&entity->subdev);
const struct v4l2_mbus_framefmt *output;
@@ -269,7 +270,7 @@ static void uds_configure(struct vsp1_entity *entity,
unsigned int vscale;
bool multitap;
- if (!full)
+ if (params != VSP1_ENTITY_PARAMS_INIT)
return;
input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index cd7d215ed455..c66f0b480989 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -254,7 +254,8 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
list_for_each_entry(entity, &pipe->entities, list_pipe) {
if (entity->ops->configure)
- entity->ops->configure(entity, pipe, pipe->dl, false);
+ entity->ops->configure(entity, pipe, pipe->dl,
+ VSP1_ENTITY_PARAMS_RUNTIME);
}
for (i = 0; i < vsp1->info->rpf_count; ++i) {
@@ -629,7 +630,8 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
vsp1_entity_route_setup(entity, pipe->dl);
if (entity->ops->configure)
- entity->ops->configure(entity, pipe, pipe->dl, true);
+ entity->ops->configure(entity, pipe, pipe->dl,
+ VSP1_ENTITY_PARAMS_INIT);
}
return 0;
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index f3a593196282..adf348d08c64 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -206,7 +206,8 @@ static void wpf_set_memory(struct vsp1_entity *entity, struct vsp1_dl_list *dl)
static void wpf_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl, bool full)
+ struct vsp1_dl_list *dl,
+ enum vsp1_entity_params params)
{
struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
struct vsp1_device *vsp1 = wpf->entity.vsp1;
@@ -216,7 +217,7 @@ static void wpf_configure(struct vsp1_entity *entity,
u32 outfmt = 0;
u32 srcrpf = 0;
- if (!full) {
+ if (params == VSP1_ENTITY_PARAMS_RUNTIME) {
const unsigned int mask = BIT(WPF_CTRL_VFLIP)
| BIT(WPF_CTRL_HFLIP);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 08/13] v4l: vsp1: Pass parameter type to entity configuration operation
2016-09-13 23:17 ` [PATCH 08/13] v4l: vsp1: Pass parameter type to entity configuration operation Laurent Pinchart
@ 2016-09-14 19:02 ` Niklas Söderlund
0 siblings, 0 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 19:02 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 02:17:01 +0300, Laurent Pinchart wrote:
> Replace the current boolean parameter (full / !full) with an explicit
> enum.
>
> - VSP1_ENTITY_PARAMS_INIT for parameters to be configured at pipeline
> initialization time only (V4L2 stream on or DRM atomic update)
> - VSP1_ENTITY_PARAMS_RUNTIME for all parameters that can be freely
> modified at runtime (through V4L2 controls)
>
> This will allow future extensions when implementing image partitioning
> support.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/media/platform/vsp1/vsp1_bru.c | 5 ++--
> drivers/media/platform/vsp1/vsp1_clu.c | 43 +++++++++++++++++--------------
> drivers/media/platform/vsp1/vsp1_drm.c | 6 +++--
> drivers/media/platform/vsp1/vsp1_entity.h | 12 ++++++++-
> drivers/media/platform/vsp1/vsp1_hsit.c | 5 ++--
> drivers/media/platform/vsp1/vsp1_lif.c | 5 ++--
> drivers/media/platform/vsp1/vsp1_lut.c | 24 ++++++++++-------
> drivers/media/platform/vsp1/vsp1_rpf.c | 5 ++--
> drivers/media/platform/vsp1/vsp1_sru.c | 5 ++--
> drivers/media/platform/vsp1/vsp1_uds.c | 5 ++--
> drivers/media/platform/vsp1/vsp1_video.c | 6 +++--
> drivers/media/platform/vsp1/vsp1_wpf.c | 5 ++--
> 12 files changed, 78 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_bru.c b/drivers/media/platform/vsp1/vsp1_bru.c
> index 26b9e2282a41..80fb948860d5 100644
> --- a/drivers/media/platform/vsp1/vsp1_bru.c
> +++ b/drivers/media/platform/vsp1/vsp1_bru.c
> @@ -285,14 +285,15 @@ static const struct v4l2_subdev_ops bru_ops = {
>
> static void bru_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> struct vsp1_bru *bru = to_bru(&entity->subdev);
> struct v4l2_mbus_framefmt *format;
> unsigned int flags;
> unsigned int i;
>
> - if (!full)
> + if (params != VSP1_ENTITY_PARAMS_INIT)
> return;
>
> format = vsp1_entity_get_pad_format(&bru->entity, bru->entity.config,
> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c
> index e1fd03811dda..a0a69dfc38fc 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> @@ -214,42 +214,47 @@ static const struct v4l2_subdev_ops clu_ops = {
>
> static void clu_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> struct vsp1_clu *clu = to_clu(&entity->subdev);
> struct vsp1_dl_body *dlb;
> unsigned long flags;
> u32 ctrl = VI6_CLU_CTRL_AAI | VI6_CLU_CTRL_MVS | VI6_CLU_CTRL_EN;
>
> - /* The format can't be changed during streaming, only verify it at
> - * stream start and store the information internally for future partial
> - * reconfiguration calls.
> - */
> - if (full) {
> + switch (params) {
> + case VSP1_ENTITY_PARAMS_INIT: {
> + /* The format can't be changed during streaming, only verify it
> + * at setup time and store the information internally for future
> + * runtime configuration calls.
> + */
> struct v4l2_mbus_framefmt *format;
>
> format = vsp1_entity_get_pad_format(&clu->entity,
> clu->entity.config,
> CLU_PAD_SINK);
> clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
> - return;
> + break;
> }
>
> - /* 2D mode can only be used with the YCbCr pixel encoding. */
> - if (clu->mode == V4L2_CID_VSP1_CLU_MODE_2D && clu->yuv_mode)
> - ctrl |= VI6_CLU_CTRL_AX1I_2D | VI6_CLU_CTRL_AX2I_2D
> - | VI6_CLU_CTRL_OS0_2D | VI6_CLU_CTRL_OS1_2D
> - | VI6_CLU_CTRL_OS2_2D | VI6_CLU_CTRL_M2D;
> + case VSP1_ENTITY_PARAMS_RUNTIME:
> + /* 2D mode can only be used with the YCbCr pixel encoding. */
> + if (clu->mode == V4L2_CID_VSP1_CLU_MODE_2D && clu->yuv_mode)
> + ctrl |= VI6_CLU_CTRL_AX1I_2D | VI6_CLU_CTRL_AX2I_2D
> + | VI6_CLU_CTRL_OS0_2D | VI6_CLU_CTRL_OS1_2D
> + | VI6_CLU_CTRL_OS2_2D | VI6_CLU_CTRL_M2D;
>
> - vsp1_clu_write(clu, dl, VI6_CLU_CTRL, ctrl);
> + vsp1_clu_write(clu, dl, VI6_CLU_CTRL, ctrl);
>
> - spin_lock_irqsave(&clu->lock, flags);
> - dlb = clu->clu;
> - clu->clu = NULL;
> - spin_unlock_irqrestore(&clu->lock, flags);
> + spin_lock_irqsave(&clu->lock, flags);
> + dlb = clu->clu;
> + clu->clu = NULL;
> + spin_unlock_irqrestore(&clu->lock, flags);
>
> - if (dlb)
> - vsp1_dl_list_add_fragment(dl, dlb);
> + if (dlb)
> + vsp1_dl_list_add_fragment(dl, dlb);
> + break;
> + }
> }
>
> static const struct vsp1_entity_operations clu_entity_ops = {
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
> index 06972f612263..6cbd3aeedbe3 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -492,8 +492,10 @@ void vsp1_du_atomic_flush(struct device *dev)
> vsp1_entity_route_setup(entity, pipe->dl);
>
> if (entity->ops->configure) {
> - entity->ops->configure(entity, pipe, pipe->dl, true);
> - entity->ops->configure(entity, pipe, pipe->dl, false);
> + entity->ops->configure(entity, pipe, pipe->dl,
> + VSP1_ENTITY_PARAMS_INIT);
> + entity->ops->configure(entity, pipe, pipe->dl,
> + VSP1_ENTITY_PARAMS_RUNTIME);
> }
>
> /* The memory buffer address must be applied after configuring
> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
> index b5e4dbb1f7d4..51835e73308d 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> @@ -35,6 +35,16 @@ enum vsp1_entity_type {
> VSP1_ENTITY_WPF,
> };
>
> +/*
> + * enum vsp1_entity_params - Entity configuration parameters class
> + * @VSP1_ENTITY_PARAMS_INIT - Initial parameters
> + * @VSP1_ENTITY_PARAMS_RUNTIME - Runtime-configurable parameters
> + */
> +enum vsp1_entity_params {
> + VSP1_ENTITY_PARAMS_INIT,
> + VSP1_ENTITY_PARAMS_RUNTIME,
> +};
> +
> #define VSP1_ENTITY_MAX_INPUTS 5 /* For the BRU */
>
> /*
> @@ -73,7 +83,7 @@ struct vsp1_entity_operations {
> void (*destroy)(struct vsp1_entity *);
> void (*set_memory)(struct vsp1_entity *, struct vsp1_dl_list *dl);
> void (*configure)(struct vsp1_entity *, struct vsp1_pipeline *,
> - struct vsp1_dl_list *, bool);
> + struct vsp1_dl_list *, enum vsp1_entity_params);
> };
>
> struct vsp1_entity {
> diff --git a/drivers/media/platform/vsp1/vsp1_hsit.c b/drivers/media/platform/vsp1/vsp1_hsit.c
> index 6ffbedb5c095..94316afc54ff 100644
> --- a/drivers/media/platform/vsp1/vsp1_hsit.c
> +++ b/drivers/media/platform/vsp1/vsp1_hsit.c
> @@ -132,11 +132,12 @@ static const struct v4l2_subdev_ops hsit_ops = {
>
> static void hsit_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> struct vsp1_hsit *hsit = to_hsit(&entity->subdev);
>
> - if (!full)
> + if (params != VSP1_ENTITY_PARAMS_INIT)
> return;
>
> if (hsit->inverse)
> diff --git a/drivers/media/platform/vsp1/vsp1_lif.c b/drivers/media/platform/vsp1/vsp1_lif.c
> index 702df863b13a..e32acae1fc6e 100644
> --- a/drivers/media/platform/vsp1/vsp1_lif.c
> +++ b/drivers/media/platform/vsp1/vsp1_lif.c
> @@ -129,7 +129,8 @@ static const struct v4l2_subdev_ops lif_ops = {
>
> static void lif_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> const struct v4l2_mbus_framefmt *format;
> struct vsp1_lif *lif = to_lif(&entity->subdev);
> @@ -137,7 +138,7 @@ static void lif_configure(struct vsp1_entity *entity,
> unsigned int obth = 400;
> unsigned int lbth = 200;
>
> - if (!full)
> + if (params != VSP1_ENTITY_PARAMS_INIT)
> return;
>
> format = vsp1_entity_get_pad_format(&lif->entity, lif->entity.config,
> diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c
> index e1c0bb7535e4..ace8acce2076 100644
> --- a/drivers/media/platform/vsp1/vsp1_lut.c
> +++ b/drivers/media/platform/vsp1/vsp1_lut.c
> @@ -190,24 +190,28 @@ static const struct v4l2_subdev_ops lut_ops = {
>
> static void lut_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> struct vsp1_lut *lut = to_lut(&entity->subdev);
> struct vsp1_dl_body *dlb;
> unsigned long flags;
>
> - if (full) {
> + switch (params) {
> + case VSP1_ENTITY_PARAMS_INIT:
> vsp1_lut_write(lut, dl, VI6_LUT_CTRL, VI6_LUT_CTRL_EN);
> - return;
> - }
> + break;
>
> - spin_lock_irqsave(&lut->lock, flags);
> - dlb = lut->lut;
> - lut->lut = NULL;
> - spin_unlock_irqrestore(&lut->lock, flags);
> + case VSP1_ENTITY_PARAMS_RUNTIME:
> + spin_lock_irqsave(&lut->lock, flags);
> + dlb = lut->lut;
> + lut->lut = NULL;
> + spin_unlock_irqrestore(&lut->lock, flags);
>
> - if (dlb)
> - vsp1_dl_list_add_fragment(dl, dlb);
> + if (dlb)
> + vsp1_dl_list_add_fragment(dl, dlb);
> + break;
> + }
> }
>
> static const struct vsp1_entity_operations lut_entity_ops = {
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
> index 3d6669dbeacf..795bf0fd1761 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -60,7 +60,8 @@ static void rpf_set_memory(struct vsp1_entity *entity, struct vsp1_dl_list *dl)
>
> static void rpf_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> const struct vsp1_format_info *fmtinfo = rpf->fmtinfo;
> @@ -73,7 +74,7 @@ static void rpf_configure(struct vsp1_entity *entity,
> u32 pstride;
> u32 infmt;
>
> - if (!full) {
> + if (params == VSP1_ENTITY_PARAMS_RUNTIME) {
> vsp1_rpf_write(rpf, dl, VI6_RPF_VRTCOL_SET,
> rpf->alpha << VI6_RPF_VRTCOL_SET_LAYA_SHIFT);
> vsp1_rpf_write(rpf, dl, VI6_RPF_MULT_ALPHA, rpf->mult_alpha |
> diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
> index 6e13cdfa5ed4..9d4a1afb6634 100644
> --- a/drivers/media/platform/vsp1/vsp1_sru.c
> +++ b/drivers/media/platform/vsp1/vsp1_sru.c
> @@ -271,7 +271,8 @@ static const struct v4l2_subdev_ops sru_ops = {
>
> static void sru_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> const struct vsp1_sru_param *param;
> struct vsp1_sru *sru = to_sru(&entity->subdev);
> @@ -279,7 +280,7 @@ static void sru_configure(struct vsp1_entity *entity,
> struct v4l2_mbus_framefmt *output;
> u32 ctrl0;
>
> - if (!full)
> + if (params != VSP1_ENTITY_PARAMS_INIT)
> return;
>
> input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
> diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
> index a8fc893a31ee..62beae5d6944 100644
> --- a/drivers/media/platform/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/vsp1/vsp1_uds.c
> @@ -260,7 +260,8 @@ static const struct v4l2_subdev_ops uds_ops = {
>
> static void uds_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> struct vsp1_uds *uds = to_uds(&entity->subdev);
> const struct v4l2_mbus_framefmt *output;
> @@ -269,7 +270,7 @@ static void uds_configure(struct vsp1_entity *entity,
> unsigned int vscale;
> bool multitap;
>
> - if (!full)
> + if (params != VSP1_ENTITY_PARAMS_INIT)
> return;
>
> input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
> index cd7d215ed455..c66f0b480989 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -254,7 +254,8 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
>
> list_for_each_entry(entity, &pipe->entities, list_pipe) {
> if (entity->ops->configure)
> - entity->ops->configure(entity, pipe, pipe->dl, false);
> + entity->ops->configure(entity, pipe, pipe->dl,
> + VSP1_ENTITY_PARAMS_RUNTIME);
> }
>
> for (i = 0; i < vsp1->info->rpf_count; ++i) {
> @@ -629,7 +630,8 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
> vsp1_entity_route_setup(entity, pipe->dl);
>
> if (entity->ops->configure)
> - entity->ops->configure(entity, pipe, pipe->dl, true);
> + entity->ops->configure(entity, pipe, pipe->dl,
> + VSP1_ENTITY_PARAMS_INIT);
> }
>
> return 0;
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
> index f3a593196282..adf348d08c64 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -206,7 +206,8 @@ static void wpf_set_memory(struct vsp1_entity *entity, struct vsp1_dl_list *dl)
>
> static void wpf_configure(struct vsp1_entity *entity,
> struct vsp1_pipeline *pipe,
> - struct vsp1_dl_list *dl, bool full)
> + struct vsp1_dl_list *dl,
> + enum vsp1_entity_params params)
> {
> struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
> struct vsp1_device *vsp1 = wpf->entity.vsp1;
> @@ -216,7 +217,7 @@ static void wpf_configure(struct vsp1_entity *entity,
> u32 outfmt = 0;
> u32 srcrpf = 0;
>
> - if (!full) {
> + if (params == VSP1_ENTITY_PARAMS_RUNTIME) {
> const unsigned int mask = BIT(WPF_CTRL_VFLIP)
> | BIT(WPF_CTRL_HFLIP);
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 09/13] v4l: vsp1: Replace .set_memory() with VSP1_ENTITY_PARAMS_PARTITION
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (7 preceding siblings ...)
2016-09-13 23:17 ` [PATCH 08/13] v4l: vsp1: Pass parameter type to entity configuration operation Laurent Pinchart
@ 2016-09-13 23:17 ` Laurent Pinchart
2016-09-13 23:17 ` [PATCH 10/13] v4l: vsp1: Support chained display lists Laurent Pinchart
` (4 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:17 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
The new VSP1_ENTITY_PARAMS_PARTITION configuration parameters type
covers all registers that need to be configured for every partition.
This prepares for support of image partitioning, and replaces the
.set_memory() operation as the memory registers take different values
for every partition.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_clu.c | 3 ++
drivers/media/platform/vsp1/vsp1_drm.c | 9 +---
drivers/media/platform/vsp1/vsp1_entity.h | 6 +--
drivers/media/platform/vsp1/vsp1_lut.c | 3 ++
drivers/media/platform/vsp1/vsp1_rpf.c | 78 ++++++++++++++--------------
drivers/media/platform/vsp1/vsp1_rwpf.h | 13 -----
drivers/media/platform/vsp1/vsp1_video.c | 17 ++-----
drivers/media/platform/vsp1/vsp1_wpf.c | 85 +++++++++++++++++--------------
8 files changed, 100 insertions(+), 114 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c
index a0a69dfc38fc..f052abd05166 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -237,6 +237,9 @@ static void clu_configure(struct vsp1_entity *entity,
break;
}
+ case VSP1_ENTITY_PARAMS_PARTITION:
+ break;
+
case VSP1_ENTITY_PARAMS_RUNTIME:
/* 2D mode can only be used with the YCbCr pixel encoding. */
if (clu->mode == V4L2_CID_VSP1_CLU_MODE_2D && clu->yuv_mode)
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index 6cbd3aeedbe3..832286975e71 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -496,14 +496,9 @@ void vsp1_du_atomic_flush(struct device *dev)
VSP1_ENTITY_PARAMS_INIT);
entity->ops->configure(entity, pipe, pipe->dl,
VSP1_ENTITY_PARAMS_RUNTIME);
+ entity->ops->configure(entity, pipe, pipe->dl,
+ VSP1_ENTITY_PARAMS_PARTITION);
}
-
- /* The memory buffer address must be applied after configuring
- * the RPF to make sure the crop offset are computed.
- */
- if (entity->type == VSP1_ENTITY_RPF)
- vsp1_rwpf_set_memory(to_rwpf(&entity->subdev),
- pipe->dl);
}
vsp1_dl_list_commit(pipe->dl);
diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
index 51835e73308d..0e3e394c44cd 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/vsp1/vsp1_entity.h
@@ -38,10 +38,12 @@ enum vsp1_entity_type {
/*
* enum vsp1_entity_params - Entity configuration parameters class
* @VSP1_ENTITY_PARAMS_INIT - Initial parameters
+ * @VSP1_ENTITY_PARAMS_PARTITION - Per-image partition parameters
* @VSP1_ENTITY_PARAMS_RUNTIME - Runtime-configurable parameters
*/
enum vsp1_entity_params {
VSP1_ENTITY_PARAMS_INIT,
+ VSP1_ENTITY_PARAMS_PARTITION,
VSP1_ENTITY_PARAMS_RUNTIME,
};
@@ -73,15 +75,11 @@ struct vsp1_route {
/**
* struct vsp1_entity_operations - Entity operations
* @destroy: Destroy the entity.
- * @set_memory: Setup memory buffer access. This operation applies the settings
- * stored in the rwpf mem field to the display list. Valid for RPF
- * and WPF only.
* @configure: Setup the hardware based on the entity state (pipeline, formats,
* selection rectangles, ...)
*/
struct vsp1_entity_operations {
void (*destroy)(struct vsp1_entity *);
- void (*set_memory)(struct vsp1_entity *, struct vsp1_dl_list *dl);
void (*configure)(struct vsp1_entity *, struct vsp1_pipeline *,
struct vsp1_dl_list *, enum vsp1_entity_params);
};
diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c
index ace8acce2076..c67cc60db0db 100644
--- a/drivers/media/platform/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/vsp1/vsp1_lut.c
@@ -202,6 +202,9 @@ static void lut_configure(struct vsp1_entity *entity,
vsp1_lut_write(lut, dl, VI6_LUT_CTRL, VI6_LUT_CTRL_EN);
break;
+ case VSP1_ENTITY_PARAMS_PARTITION:
+ break;
+
case VSP1_ENTITY_PARAMS_RUNTIME:
spin_lock_irqsave(&lut->lock, flags);
dlb = lut->lut;
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 795bf0fd1761..de5ef76c5004 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -46,18 +46,6 @@ static const struct v4l2_subdev_ops rpf_ops = {
* VSP1 Entity Operations
*/
-static void rpf_set_memory(struct vsp1_entity *entity, struct vsp1_dl_list *dl)
-{
- struct vsp1_rwpf *rpf = entity_to_rwpf(entity);
-
- vsp1_rpf_write(rpf, dl, VI6_RPF_SRCM_ADDR_Y,
- rpf->mem.addr[0] + rpf->offsets[0]);
- vsp1_rpf_write(rpf, dl, VI6_RPF_SRCM_ADDR_C0,
- rpf->mem.addr[1] + rpf->offsets[1]);
- vsp1_rpf_write(rpf, dl, VI6_RPF_SRCM_ADDR_C1,
- rpf->mem.addr[2] + rpf->offsets[1]);
-}
-
static void rpf_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
@@ -68,7 +56,6 @@ static void rpf_configure(struct vsp1_entity *entity,
const struct v4l2_pix_format_mplane *format = &rpf->format;
const struct v4l2_mbus_framefmt *source_format;
const struct v4l2_mbus_framefmt *sink_format;
- const struct v4l2_rect *crop;
unsigned int left = 0;
unsigned int top = 0;
u32 pstride;
@@ -84,35 +71,51 @@ static void rpf_configure(struct vsp1_entity *entity,
return;
}
- /* Source size, stride and crop offsets.
- *
- * The crop offsets correspond to the location of the crop rectangle top
- * left corner in the plane buffer. Only two offsets are needed, as
- * planes 2 and 3 always have identical strides.
- */
- crop = vsp1_rwpf_get_crop(rpf, rpf->entity.config);
-
- vsp1_rpf_write(rpf, dl, VI6_RPF_SRC_BSIZE,
- (crop->width << VI6_RPF_SRC_BSIZE_BHSIZE_SHIFT) |
- (crop->height << VI6_RPF_SRC_BSIZE_BVSIZE_SHIFT));
- vsp1_rpf_write(rpf, dl, VI6_RPF_SRC_ESIZE,
- (crop->width << VI6_RPF_SRC_ESIZE_EHSIZE_SHIFT) |
- (crop->height << VI6_RPF_SRC_ESIZE_EVSIZE_SHIFT));
+ if (params == VSP1_ENTITY_PARAMS_PARTITION) {
+ const struct v4l2_rect *crop;
+ unsigned int offsets[2];
+
+ /* Source size and crop offsets.
+ *
+ * The crop offsets correspond to the location of the crop
+ * rectangle top left corner in the plane buffer. Only two
+ * offsets are needed, as planes 2 and 3 always have identical
+ * strides.
+ */
+ crop = vsp1_rwpf_get_crop(rpf, rpf->entity.config);
+
+ vsp1_rpf_write(rpf, dl, VI6_RPF_SRC_BSIZE,
+ (crop->width << VI6_RPF_SRC_BSIZE_BHSIZE_SHIFT) |
+ (crop->height << VI6_RPF_SRC_BSIZE_BVSIZE_SHIFT));
+ vsp1_rpf_write(rpf, dl, VI6_RPF_SRC_ESIZE,
+ (crop->width << VI6_RPF_SRC_ESIZE_EHSIZE_SHIFT) |
+ (crop->height << VI6_RPF_SRC_ESIZE_EVSIZE_SHIFT));
+
+ offsets[0] = crop->top * format->plane_fmt[0].bytesperline
+ + crop->left * fmtinfo->bpp[0] / 8;
+
+ if (format->num_planes > 1)
+ offsets[1] = crop->top * format->plane_fmt[1].bytesperline
+ + crop->left / fmtinfo->hsub
+ * fmtinfo->bpp[1] / 8;
+ else
+ offsets[1] = 0;
+
+ vsp1_rpf_write(rpf, dl, VI6_RPF_SRCM_ADDR_Y,
+ rpf->mem.addr[0] + offsets[0]);
+ vsp1_rpf_write(rpf, dl, VI6_RPF_SRCM_ADDR_C0,
+ rpf->mem.addr[1] + offsets[1]);
+ vsp1_rpf_write(rpf, dl, VI6_RPF_SRCM_ADDR_C1,
+ rpf->mem.addr[2] + offsets[1]);
+ return;
+ }
- rpf->offsets[0] = crop->top * format->plane_fmt[0].bytesperline
- + crop->left * fmtinfo->bpp[0] / 8;
+ /* Stride */
pstride = format->plane_fmt[0].bytesperline
<< VI6_RPF_SRCM_PSTRIDE_Y_SHIFT;
-
- if (format->num_planes > 1) {
- rpf->offsets[1] = crop->top * format->plane_fmt[1].bytesperline
- + crop->left / fmtinfo->hsub * fmtinfo->bpp[1]
- / 8;
+ if (format->num_planes > 1)
pstride |= format->plane_fmt[1].bytesperline
<< VI6_RPF_SRCM_PSTRIDE_C_SHIFT;
- } else {
- rpf->offsets[1] = 0;
- }
vsp1_rpf_write(rpf, dl, VI6_RPF_SRCM_PSTRIDE, pstride);
@@ -217,7 +220,6 @@ static void rpf_configure(struct vsp1_entity *entity,
}
static const struct vsp1_entity_operations rpf_entity_ops = {
- .set_memory = rpf_set_memory,
.configure = rpf_configure,
};
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h b/drivers/media/platform/vsp1/vsp1_rwpf.h
index cb20484e80da..1c98aff3da5d 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
@@ -61,7 +61,6 @@ struct vsp1_rwpf {
unsigned int active;
} flip;
- unsigned int offsets[2];
struct vsp1_rwpf_memory mem;
struct vsp1_dl_manager *dlm;
@@ -86,17 +85,5 @@ extern const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops;
struct v4l2_rect *vsp1_rwpf_get_crop(struct vsp1_rwpf *rwpf,
struct v4l2_subdev_pad_config *config);
-/**
- * vsp1_rwpf_set_memory - Configure DMA addresses for a [RW]PF
- * @rwpf: the [RW]PF instance
- * @dl: the display list
- *
- * This function applies the cached memory buffer address to the display list.
- */
-static inline void vsp1_rwpf_set_memory(struct vsp1_rwpf *rwpf,
- struct vsp1_dl_list *dl)
-{
- rwpf->entity.ops->set_memory(&rwpf->entity, dl);
-}
#endif /* __VSP1_RWPF_H__ */
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index c66f0b480989..b8339d874df4 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -245,29 +245,20 @@ static void vsp1_video_frame_end(struct vsp1_pipeline *pipe,
static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
{
- struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
struct vsp1_entity *entity;
- unsigned int i;
if (!pipe->dl)
pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
list_for_each_entry(entity, &pipe->entities, list_pipe) {
- if (entity->ops->configure)
+ if (entity->ops->configure) {
entity->ops->configure(entity, pipe, pipe->dl,
VSP1_ENTITY_PARAMS_RUNTIME);
+ entity->ops->configure(entity, pipe, pipe->dl,
+ VSP1_ENTITY_PARAMS_PARTITION);
+ }
}
- for (i = 0; i < vsp1->info->rpf_count; ++i) {
- struct vsp1_rwpf *rwpf = pipe->inputs[i];
-
- if (rwpf)
- vsp1_rwpf_set_memory(rwpf, pipe->dl);
- }
-
- if (!pipe->lif)
- vsp1_rwpf_set_memory(pipe->output, pipe->dl);
-
vsp1_dl_list_commit(pipe->dl);
pipe->dl = NULL;
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index adf348d08c64..717c0be58bfb 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -173,37 +173,6 @@ static void vsp1_wpf_destroy(struct vsp1_entity *entity)
vsp1_dlm_destroy(wpf->dlm);
}
-static void wpf_set_memory(struct vsp1_entity *entity, struct vsp1_dl_list *dl)
-{
- struct vsp1_rwpf *wpf = entity_to_rwpf(entity);
- const struct v4l2_pix_format_mplane *format = &wpf->format;
- struct vsp1_rwpf_memory mem = wpf->mem;
- unsigned int flip = wpf->flip.active;
- unsigned int offset;
-
- /* Update the memory offsets based on flipping configuration. The
- * destination addresses point to the locations where the VSP starts
- * writing to memory, which can be different corners of the image
- * depending on vertical flipping. Horizontal flipping is handled
- * through a line buffer and doesn't modify the start address.
- */
- if (flip & BIT(WPF_CTRL_VFLIP)) {
- mem.addr[0] += (format->height - 1)
- * format->plane_fmt[0].bytesperline;
-
- if (format->num_planes > 1) {
- offset = (format->height / wpf->fmtinfo->vsub - 1)
- * format->plane_fmt[1].bytesperline;
- mem.addr[1] += offset;
- mem.addr[2] += offset;
- }
- }
-
- vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_Y, mem.addr[0]);
- vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_C0, mem.addr[1]);
- vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_C1, mem.addr[2]);
-}
-
static void wpf_configure(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
@@ -237,7 +206,6 @@ static void wpf_configure(struct vsp1_entity *entity,
return;
}
- /* Format */
sink_format = vsp1_entity_get_pad_format(&wpf->entity,
wpf->entity.config,
RWPF_PAD_SINK);
@@ -245,13 +213,53 @@ static void wpf_configure(struct vsp1_entity *entity,
wpf->entity.config,
RWPF_PAD_SOURCE);
- vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
- (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
- (source_format->width << VI6_WPF_SZCLIP_SIZE_SHIFT));
- vsp1_wpf_write(wpf, dl, VI6_WPF_VSZCLIP, VI6_WPF_SZCLIP_EN |
- (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
- (source_format->height << VI6_WPF_SZCLIP_SIZE_SHIFT));
+ if (params == VSP1_ENTITY_PARAMS_PARTITION) {
+ const struct v4l2_pix_format_mplane *format = &wpf->format;
+ struct vsp1_rwpf_memory mem = wpf->mem;
+ unsigned int flip = wpf->flip.active;
+ unsigned int width = source_format->width;
+ unsigned int height = source_format->height;
+ unsigned int offset;
+
+ /* Cropping. The partition algorithm can split the image into
+ * multiple slices.
+ */
+ vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
+ (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
+ (width << VI6_WPF_SZCLIP_SIZE_SHIFT));
+ vsp1_wpf_write(wpf, dl, VI6_WPF_VSZCLIP, VI6_WPF_SZCLIP_EN |
+ (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
+ (height << VI6_WPF_SZCLIP_SIZE_SHIFT));
+
+ if (pipe->lif)
+ return;
+
+ /* Update the memory offsets based on flipping configuration.
+ * The destination addresses point to the locations where the
+ * VSP starts writing to memory, which can be different corners
+ * of the image depending on vertical flipping. Horizontal
+ * flipping is handled through a line buffer and doesn't modify
+ * the start address.
+ */
+ if (flip & BIT(WPF_CTRL_VFLIP)) {
+ mem.addr[0] += (format->height - 1)
+ * format->plane_fmt[0].bytesperline;
+
+ if (format->num_planes > 1) {
+ offset = (format->height / wpf->fmtinfo->vsub - 1)
+ * format->plane_fmt[1].bytesperline;
+ mem.addr[1] += offset;
+ mem.addr[2] += offset;
+ }
+ }
+ vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_Y, mem.addr[0]);
+ vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_C0, mem.addr[1]);
+ vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_C1, mem.addr[2]);
+ return;
+ }
+
+ /* Format */
if (!pipe->lif) {
const struct v4l2_pix_format_mplane *format = &wpf->format;
const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
@@ -320,7 +328,6 @@ static void wpf_configure(struct vsp1_entity *entity,
static const struct vsp1_entity_operations wpf_entity_ops = {
.destroy = vsp1_wpf_destroy,
- .set_memory = wpf_set_memory,
.configure = wpf_configure,
};
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 10/13] v4l: vsp1: Support chained display lists
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (8 preceding siblings ...)
2016-09-13 23:17 ` [PATCH 09/13] v4l: vsp1: Replace .set_memory() with VSP1_ENTITY_PARAMS_PARTITION Laurent Pinchart
@ 2016-09-13 23:17 ` Laurent Pinchart
2016-09-13 23:17 ` [PATCH 11/13] v4l: vsp1: Determine partition requirements for scaled images Laurent Pinchart
` (3 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:17 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
From: Kieran Bingham <kieran+renesas@bingham.xyz>
When display lists are linked in a chain, they will be processed
automatically by the hardware, with each list linking to the next. Only
on the last display list will the frame end interrupt be fired to mark
the completion event.
Upon frame-end, the chain will be iterated to release each display list
back to the free list.
The chained lists use case (image partitioning) can require up to 64
lists per frame in the worst case scenario, bump up the number of
preallocated lists.
Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_dl.c | 119 +++++++++++++++++++++++++++------
drivers/media/platform/vsp1/vsp1_dl.h | 1 +
drivers/media/platform/vsp1/vsp1_wpf.c | 2 +-
3 files changed, 102 insertions(+), 20 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 37c3518aa2a8..0af3e8fdc714 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -21,7 +21,6 @@
#include "vsp1_dl.h"
#define VSP1_DL_NUM_ENTRIES 256
-#define VSP1_DL_NUM_LISTS 3
#define VSP1_DLH_INT_ENABLE (1 << 1)
#define VSP1_DLH_AUTO_START (1 << 0)
@@ -71,6 +70,7 @@ struct vsp1_dl_body {
* @dma: DMA address for the header
* @body0: first display list body
* @fragments: list of extra display list bodies
+ * @chain: entry in the display list partition chain
*/
struct vsp1_dl_list {
struct list_head list;
@@ -81,6 +81,9 @@ struct vsp1_dl_list {
struct vsp1_dl_body body0;
struct list_head fragments;
+
+ bool has_chain;
+ struct list_head chain;
};
enum vsp1_dl_mode {
@@ -262,7 +265,6 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)
memset(dl->header, 0, sizeof(*dl->header));
dl->header->lists[0].addr = dl->body0.dma;
- dl->header->flags = VSP1_DLH_INT_ENABLE;
}
return dl;
@@ -293,6 +295,11 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm)
if (!list_empty(&dlm->free)) {
dl = list_first_entry(&dlm->free, struct vsp1_dl_list, list);
list_del(&dl->list);
+
+ /* The display list chain must be initialised to ensure every
+ * display list can assert list_empty() if it is not in a chain.
+ */
+ INIT_LIST_HEAD(&dl->chain);
}
spin_unlock_irqrestore(&dlm->lock, flags);
@@ -303,9 +310,21 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm)
/* This function must be called with the display list manager lock held.*/
static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
{
+ struct vsp1_dl_list *dl_child;
+
if (!dl)
return;
+ /* Release any linked display-lists which were chained for a single
+ * hardware operation.
+ */
+ if (dl->has_chain) {
+ list_for_each_entry(dl_child, &dl->chain, chain)
+ __vsp1_dl_list_put(dl_child);
+ }
+
+ dl->has_chain = false;
+
/* We can't free fragments here as DMA memory can only be freed in
* interruptible context. Move all fragments to the display list
* manager's list of fragments to be freed, they will be
@@ -383,6 +402,74 @@ int vsp1_dl_list_add_fragment(struct vsp1_dl_list *dl,
return 0;
}
+/**
+ * vsp1_dl_list_add_chain - Add a display list to a chain
+ * @head: The head display list
+ * @dl: The new display list
+ *
+ * Add a display list to an existing display list chain. The chained lists
+ * will be automatically processed by the hardware without intervention from
+ * the CPU. A display list end interrupt will only complete after the last
+ * display list in the chain has completed processing.
+ *
+ * Adding a display list to a chain passes ownership of the display list to
+ * the head display list item. The chain is released when the head dl item is
+ * put back with __vsp1_dl_list_put().
+ *
+ * Chained display lists are only usable in header mode. Attempts to add a
+ * display list to a chain in header-less mode will return an error.
+ */
+int vsp1_dl_list_add_chain(struct vsp1_dl_list *head,
+ struct vsp1_dl_list *dl)
+{
+ /* Chained lists are only available in header mode. */
+ if (head->dlm->mode != VSP1_DL_MODE_HEADER)
+ return -EINVAL;
+
+ head->has_chain = true;
+ list_add_tail(&dl->chain, &head->chain);
+ return 0;
+}
+
+static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last)
+{
+ struct vsp1_dl_header_list *hdr = dl->header->lists;
+ struct vsp1_dl_body *dlb;
+ unsigned int num_lists = 0;
+
+ /* Fill the header with the display list bodies addresses and sizes. The
+ * address of the first body has already been filled when the display
+ * list was allocated.
+ */
+
+ hdr->num_bytes = dl->body0.num_entries
+ * sizeof(*dl->header->lists);
+
+ list_for_each_entry(dlb, &dl->fragments, list) {
+ num_lists++;
+ hdr++;
+
+ hdr->addr = dlb->dma;
+ hdr->num_bytes = dlb->num_entries
+ * sizeof(*dl->header->lists);
+ }
+
+ dl->header->num_lists = num_lists;
+
+ /* If this display list's chain is not empty, we are on a list, where
+ * the next item in the list is the display list entity which should be
+ * automatically queued by the hardware.
+ */
+ if (!list_empty(&dl->chain) && !is_last) {
+ struct vsp1_dl_list *next = list_next_entry(dl, chain);
+
+ dl->header->next_header = next->dma;
+ dl->header->flags = VSP1_DLH_AUTO_START;
+ } else {
+ dl->header->flags = VSP1_DLH_INT_ENABLE;
+ }
+}
+
void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
{
struct vsp1_dl_manager *dlm = dl->dlm;
@@ -393,30 +480,24 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
spin_lock_irqsave(&dlm->lock, flags);
if (dl->dlm->mode == VSP1_DL_MODE_HEADER) {
- struct vsp1_dl_header_list *hdr = dl->header->lists;
- struct vsp1_dl_body *dlb;
- unsigned int num_lists = 0;
+ struct vsp1_dl_list *dl_child;
- /* Fill the header with the display list bodies addresses and
- * sizes. The address of the first body has already been filled
- * when the display list was allocated.
- *
- * In header mode the caller guarantees that the hardware is
+ /* In header mode the caller guarantees that the hardware is
* idle at this point.
*/
- hdr->num_bytes = dl->body0.num_entries
- * sizeof(*dl->header->lists);
- list_for_each_entry(dlb, &dl->fragments, list) {
- num_lists++;
- hdr++;
+ /* Fill the header for the head and chained display lists. */
+ vsp1_dl_list_fill_header(dl, list_empty(&dl->chain));
- hdr->addr = dlb->dma;
- hdr->num_bytes = dlb->num_entries
- * sizeof(*dl->header->lists);
+ list_for_each_entry(dl_child, &dl->chain, chain) {
+ bool last = list_is_last(&dl_child->chain, &dl->chain);
+
+ vsp1_dl_list_fill_header(dl_child, last);
}
- dl->header->num_lists = num_lists;
+ /* Commit the head display list to hardware. Chained headers
+ * will auto-start.
+ */
vsp1_write(vsp1, VI6_DL_HDR_ADDR(dlm->index), dl->dma);
dlm->active = dl;
diff --git a/drivers/media/platform/vsp1/vsp1_dl.h b/drivers/media/platform/vsp1/vsp1_dl.h
index de387cd4d745..7131aa3c5978 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.h
+++ b/drivers/media/platform/vsp1/vsp1_dl.h
@@ -41,5 +41,6 @@ void vsp1_dl_fragment_free(struct vsp1_dl_body *dlb);
void vsp1_dl_fragment_write(struct vsp1_dl_body *dlb, u32 reg, u32 data);
int vsp1_dl_list_add_fragment(struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb);
+int vsp1_dl_list_add_chain(struct vsp1_dl_list *head, struct vsp1_dl_list *dl);
#endif /* __VSP1_DL_H__ */
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index 717c0be58bfb..b757d2579d6c 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -364,7 +364,7 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
return ERR_PTR(ret);
/* Initialize the display list manager. */
- wpf->dlm = vsp1_dlm_create(vsp1, index, 4);
+ wpf->dlm = vsp1_dlm_create(vsp1, index, 64);
if (!wpf->dlm) {
ret = -ENOMEM;
goto error;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 11/13] v4l: vsp1: Determine partition requirements for scaled images
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (9 preceding siblings ...)
2016-09-13 23:17 ` [PATCH 10/13] v4l: vsp1: Support chained display lists Laurent Pinchart
@ 2016-09-13 23:17 ` Laurent Pinchart
2016-09-14 19:27 ` Niklas Söderlund
2016-09-13 23:17 ` [PATCH 12/13] v4l: vsp1: Support multiple partitions per frame Laurent Pinchart
` (2 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:17 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
From: Kieran Bingham <kieran+renesas@bingham.xyz>
The partition algorithm needs to determine the capabilities of each
entity in the pipeline to identify the correct maximum partition width.
Extend the vsp1 entity operations to provide a max_width operation and
use this call to calculate the number of partitions that will be
processed by the algorithm.
Gen 2 hardware does not require multiple partitioning, and as such
will always return a single partition.
Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_entity.h | 3 +++
drivers/media/platform/vsp1/vsp1_pipe.h | 5 ++++
drivers/media/platform/vsp1/vsp1_sru.c | 19 +++++++++++++++
drivers/media/platform/vsp1/vsp1_uds.c | 25 +++++++++++++++++++
drivers/media/platform/vsp1/vsp1_video.c | 40 +++++++++++++++++++++++++++++++
5 files changed, 92 insertions(+)
diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
index 0e3e394c44cd..90a4d95c0a50 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/vsp1/vsp1_entity.h
@@ -77,11 +77,14 @@ struct vsp1_route {
* @destroy: Destroy the entity.
* @configure: Setup the hardware based on the entity state (pipeline, formats,
* selection rectangles, ...)
+ * @max_width: Return the max supported width of data that the entity can
+ * process in a single operation.
*/
struct vsp1_entity_operations {
void (*destroy)(struct vsp1_entity *);
void (*configure)(struct vsp1_entity *, struct vsp1_pipeline *,
struct vsp1_dl_list *, enum vsp1_entity_params);
+ unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *);
};
struct vsp1_entity {
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index d20d997b1fda..af4cd23d399b 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -77,6 +77,8 @@ enum vsp1_pipeline_state {
* @uds_input: entity at the input of the UDS, if the UDS is present
* @entities: list of entities in the pipeline
* @dl: display list associated with the pipeline
+ * @div_size: The maximum allowed partition size for the pipeline
+ * @partitions: The number of partitions used to process one frame
*/
struct vsp1_pipeline {
struct media_pipeline pipe;
@@ -104,6 +106,9 @@ struct vsp1_pipeline {
struct list_head entities;
struct vsp1_dl_list *dl;
+
+ unsigned int div_size;
+ unsigned int partitions;
};
void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
index 9d4a1afb6634..b4e568a3b4ed 100644
--- a/drivers/media/platform/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/vsp1/vsp1_sru.c
@@ -306,8 +306,27 @@ static void sru_configure(struct vsp1_entity *entity,
vsp1_sru_write(sru, dl, VI6_SRU_CTRL2, param->ctrl2);
}
+static unsigned int sru_max_width(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe)
+{
+ struct vsp1_sru *sru = to_sru(&entity->subdev);
+ struct v4l2_mbus_framefmt *input;
+ struct v4l2_mbus_framefmt *output;
+
+ input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
+ SRU_PAD_SINK);
+ output = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
+ SRU_PAD_SOURCE);
+
+ if (input->width != output->width)
+ return 512;
+ else
+ return 256;
+}
+
static const struct vsp1_entity_operations sru_entity_ops = {
.configure = sru_configure,
+ .max_width = sru_max_width,
};
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
index 62beae5d6944..706b6e85f47d 100644
--- a/drivers/media/platform/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/vsp1/vsp1_uds.c
@@ -311,8 +311,33 @@ static void uds_configure(struct vsp1_entity *entity,
(output->height << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
}
+static unsigned int uds_max_width(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe)
+{
+ struct vsp1_uds *uds = to_uds(&entity->subdev);
+ const struct v4l2_mbus_framefmt *output;
+ const struct v4l2_mbus_framefmt *input;
+ unsigned int hscale;
+
+ input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
+ UDS_PAD_SINK);
+ output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
+ UDS_PAD_SOURCE);
+ hscale = output->width / input->width;
+
+ if (hscale <= 2)
+ return 256;
+ else if (hscale <= 4)
+ return 512;
+ else if (hscale <= 8)
+ return 1024;
+ else
+ return 2048;
+}
+
static const struct vsp1_entity_operations uds_entity_ops = {
.configure = uds_configure,
+ .max_width = uds_max_width,
};
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index b8339d874df4..b903cc5471e0 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -169,6 +169,43 @@ static int __vsp1_video_try_format(struct vsp1_video *video,
}
/* -----------------------------------------------------------------------------
+ * VSP1 Partition Algorithm support
+ */
+
+static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
+{
+ struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
+ const struct v4l2_mbus_framefmt *format;
+ struct vsp1_entity *entity;
+ unsigned int div_size;
+
+ format = vsp1_entity_get_pad_format(&pipe->output->entity,
+ pipe->output->entity.config,
+ RWPF_PAD_SOURCE);
+ div_size = format->width;
+
+ /* Gen2 hardware doesn't require image partitioning. */
+ if (vsp1->info->gen == 2) {
+ pipe->div_size = div_size;
+ pipe->partitions = 1;
+ return;
+ }
+
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ unsigned int entity_max = VSP1_VIDEO_MAX_WIDTH;
+
+ if (entity->ops->max_width) {
+ entity_max = entity->ops->max_width(entity, pipe);
+ if (entity_max)
+ div_size = min(div_size, entity_max);
+ }
+ }
+
+ pipe->div_size = div_size;
+ pipe->partitions = DIV_ROUND_UP(format->width, div_size);
+}
+
+/* -----------------------------------------------------------------------------
* Pipeline Management
*/
@@ -594,6 +631,9 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
{
struct vsp1_entity *entity;
+ /* Determine this pipelines sizes for image partitioning support. */
+ vsp1_video_pipeline_setup_partitions(pipe);
+
/* Prepare the display list. */
pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
if (!pipe->dl)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 11/13] v4l: vsp1: Determine partition requirements for scaled images
2016-09-13 23:17 ` [PATCH 11/13] v4l: vsp1: Determine partition requirements for scaled images Laurent Pinchart
@ 2016-09-14 19:27 ` Niklas Söderlund
2016-09-14 20:00 ` Laurent Pinchart
0 siblings, 1 reply; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 19:27 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 02:17:04 +0300, Laurent Pinchart wrote:
> From: Kieran Bingham <kieran+renesas@bingham.xyz>
>
> The partition algorithm needs to determine the capabilities of each
> entity in the pipeline to identify the correct maximum partition width.
>
> Extend the vsp1 entity operations to provide a max_width operation and
> use this call to calculate the number of partitions that will be
> processed by the algorithm.
>
> Gen 2 hardware does not require multiple partitioning, and as such
> will always return a single partition.
>
> Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
I can't find the information about the partition limitations for SRU or
UDS in any of the documents I have. But for the parts not relating to
the logic of figuring out the hscale from the input/output formats
width:
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/media/platform/vsp1/vsp1_entity.h | 3 +++
> drivers/media/platform/vsp1/vsp1_pipe.h | 5 ++++
> drivers/media/platform/vsp1/vsp1_sru.c | 19 +++++++++++++++
> drivers/media/platform/vsp1/vsp1_uds.c | 25 +++++++++++++++++++
> drivers/media/platform/vsp1/vsp1_video.c | 40 +++++++++++++++++++++++++++++++
> 5 files changed, 92 insertions(+)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
> index 0e3e394c44cd..90a4d95c0a50 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> @@ -77,11 +77,14 @@ struct vsp1_route {
> * @destroy: Destroy the entity.
> * @configure: Setup the hardware based on the entity state (pipeline, formats,
> * selection rectangles, ...)
> + * @max_width: Return the max supported width of data that the entity can
> + * process in a single operation.
> */
> struct vsp1_entity_operations {
> void (*destroy)(struct vsp1_entity *);
> void (*configure)(struct vsp1_entity *, struct vsp1_pipeline *,
> struct vsp1_dl_list *, enum vsp1_entity_params);
> + unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *);
> };
>
> struct vsp1_entity {
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
> index d20d997b1fda..af4cd23d399b 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
> @@ -77,6 +77,8 @@ enum vsp1_pipeline_state {
> * @uds_input: entity at the input of the UDS, if the UDS is present
> * @entities: list of entities in the pipeline
> * @dl: display list associated with the pipeline
> + * @div_size: The maximum allowed partition size for the pipeline
> + * @partitions: The number of partitions used to process one frame
> */
> struct vsp1_pipeline {
> struct media_pipeline pipe;
> @@ -104,6 +106,9 @@ struct vsp1_pipeline {
> struct list_head entities;
>
> struct vsp1_dl_list *dl;
> +
> + unsigned int div_size;
> + unsigned int partitions;
> };
>
> void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
> diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
> index 9d4a1afb6634..b4e568a3b4ed 100644
> --- a/drivers/media/platform/vsp1/vsp1_sru.c
> +++ b/drivers/media/platform/vsp1/vsp1_sru.c
> @@ -306,8 +306,27 @@ static void sru_configure(struct vsp1_entity *entity,
> vsp1_sru_write(sru, dl, VI6_SRU_CTRL2, param->ctrl2);
> }
>
> +static unsigned int sru_max_width(struct vsp1_entity *entity,
> + struct vsp1_pipeline *pipe)
> +{
> + struct vsp1_sru *sru = to_sru(&entity->subdev);
> + struct v4l2_mbus_framefmt *input;
> + struct v4l2_mbus_framefmt *output;
> +
> + input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
> + SRU_PAD_SINK);
> + output = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
> + SRU_PAD_SOURCE);
> +
> + if (input->width != output->width)
> + return 512;
> + else
> + return 256;
> +}
> +
> static const struct vsp1_entity_operations sru_entity_ops = {
> .configure = sru_configure,
> + .max_width = sru_max_width,
> };
>
> /* -----------------------------------------------------------------------------
> diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
> index 62beae5d6944..706b6e85f47d 100644
> --- a/drivers/media/platform/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/vsp1/vsp1_uds.c
> @@ -311,8 +311,33 @@ static void uds_configure(struct vsp1_entity *entity,
> (output->height << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
> }
>
> +static unsigned int uds_max_width(struct vsp1_entity *entity,
> + struct vsp1_pipeline *pipe)
> +{
> + struct vsp1_uds *uds = to_uds(&entity->subdev);
> + const struct v4l2_mbus_framefmt *output;
> + const struct v4l2_mbus_framefmt *input;
> + unsigned int hscale;
> +
> + input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
> + UDS_PAD_SINK);
> + output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
> + UDS_PAD_SOURCE);
> + hscale = output->width / input->width;
> +
> + if (hscale <= 2)
> + return 256;
> + else if (hscale <= 4)
> + return 512;
> + else if (hscale <= 8)
> + return 1024;
> + else
> + return 2048;
> +}
> +
> static const struct vsp1_entity_operations uds_entity_ops = {
> .configure = uds_configure,
> + .max_width = uds_max_width,
> };
>
> /* -----------------------------------------------------------------------------
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
> index b8339d874df4..b903cc5471e0 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -169,6 +169,43 @@ static int __vsp1_video_try_format(struct vsp1_video *video,
> }
>
> /* -----------------------------------------------------------------------------
> + * VSP1 Partition Algorithm support
> + */
> +
> +static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
> +{
> + struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
> + const struct v4l2_mbus_framefmt *format;
> + struct vsp1_entity *entity;
> + unsigned int div_size;
> +
> + format = vsp1_entity_get_pad_format(&pipe->output->entity,
> + pipe->output->entity.config,
> + RWPF_PAD_SOURCE);
> + div_size = format->width;
> +
> + /* Gen2 hardware doesn't require image partitioning. */
> + if (vsp1->info->gen == 2) {
> + pipe->div_size = div_size;
> + pipe->partitions = 1;
> + return;
> + }
> +
> + list_for_each_entry(entity, &pipe->entities, list_pipe) {
> + unsigned int entity_max = VSP1_VIDEO_MAX_WIDTH;
> +
> + if (entity->ops->max_width) {
> + entity_max = entity->ops->max_width(entity, pipe);
> + if (entity_max)
> + div_size = min(div_size, entity_max);
> + }
> + }
> +
> + pipe->div_size = div_size;
> + pipe->partitions = DIV_ROUND_UP(format->width, div_size);
> +}
> +
> +/* -----------------------------------------------------------------------------
> * Pipeline Management
> */
>
> @@ -594,6 +631,9 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
> {
> struct vsp1_entity *entity;
>
> + /* Determine this pipelines sizes for image partitioning support. */
> + vsp1_video_pipeline_setup_partitions(pipe);
> +
> /* Prepare the display list. */
> pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
> if (!pipe->dl)
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 11/13] v4l: vsp1: Determine partition requirements for scaled images
2016-09-14 19:27 ` Niklas Söderlund
@ 2016-09-14 20:00 ` Laurent Pinchart
2016-09-15 13:19 ` Niklas Söderlund
0 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-14 20:00 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, Kieran Bingham
Hi Niklas,
On Wednesday 14 Sep 2016 21:27:33 Niklas Söderlund wrote:
> On 2016-09-14 02:17:04 +0300, Laurent Pinchart wrote:
> > From: Kieran Bingham <kieran+renesas@bingham.xyz>
> >
> > The partition algorithm needs to determine the capabilities of each
> > entity in the pipeline to identify the correct maximum partition width.
> >
> > Extend the vsp1 entity operations to provide a max_width operation and
> > use this call to calculate the number of partitions that will be
> > processed by the algorithm.
> >
> > Gen 2 hardware does not require multiple partitioning, and as such
> > will always return a single partition.
> >
> > Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
>
> I can't find the information about the partition limitations for SRU or
> UDS in any of the documents I have.
That's because it's not documented in the datasheet :-(
> But for the parts not relating to the logic of figuring out the hscale from
> the input/output formats width:
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Thanks.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 11/13] v4l: vsp1: Determine partition requirements for scaled images
2016-09-14 20:00 ` Laurent Pinchart
@ 2016-09-15 13:19 ` Niklas Söderlund
0 siblings, 0 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-15 13:19 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 23:00:33 +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Wednesday 14 Sep 2016 21:27:33 Niklas Söderlund wrote:
> > On 2016-09-14 02:17:04 +0300, Laurent Pinchart wrote:
> > > From: Kieran Bingham <kieran+renesas@bingham.xyz>
> > >
> > > The partition algorithm needs to determine the capabilities of each
> > > entity in the pipeline to identify the correct maximum partition width.
> > >
> > > Extend the vsp1 entity operations to provide a max_width operation and
> > > use this call to calculate the number of partitions that will be
> > > processed by the algorithm.
> > >
> > > Gen 2 hardware does not require multiple partitioning, and as such
> > > will always return a single partition.
> > >
> > > Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> >
> > I can't find the information about the partition limitations for SRU or
> > UDS in any of the documents I have.
>
> That's because it's not documented in the datasheet :-(
Sometimes a kind soul provides you with the proper documentation :-)
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> > But for the parts not relating to the logic of figuring out the hscale from
> > the input/output formats width:
> >
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> Thanks.
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 12/13] v4l: vsp1: Support multiple partitions per frame
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (10 preceding siblings ...)
2016-09-13 23:17 ` [PATCH 11/13] v4l: vsp1: Determine partition requirements for scaled images Laurent Pinchart
@ 2016-09-13 23:17 ` Laurent Pinchart
2016-09-13 23:17 ` [PATCH 13/13] v4l: vsp1: wpf: Implement rotation support Laurent Pinchart
2016-09-13 23:29 ` [PATCH 14/13] v4l: vsp1: Fix spinlock in mixed IRQ context function Laurent Pinchart
13 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:17 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
From: Kieran Bingham <kieran+renesas@bingham.xyz>
Adapt vsp1_video_pipeline_run() such that it can iterate each partition
required for constructing this frame's display list chain in the event
that multiple display lists are required to process in hardware.
The first display list is held as the head list object, whilst any
following parition display lists are linked to the head by means of
vsp1_dl_list_add_chain().
Linking the chained display list headers to process using the auto start
mechanism of the hardware is performed during the vsp1_dl_list_commit().
Signed-off-by: Kieran Bingham <kieran+renesas@bingham.xyz>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_pipe.h | 3 +
drivers/media/platform/vsp1/vsp1_rpf.c | 46 +++++++++---
drivers/media/platform/vsp1/vsp1_uds.c | 15 +++-
drivers/media/platform/vsp1/vsp1_video.c | 121 ++++++++++++++++++++++++++++++-
drivers/media/platform/vsp1/vsp1_wpf.c | 30 +++++++-
5 files changed, 195 insertions(+), 20 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index af4cd23d399b..f15b697ad999 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -79,6 +79,7 @@ enum vsp1_pipeline_state {
* @dl: display list associated with the pipeline
* @div_size: The maximum allowed partition size for the pipeline
* @partitions: The number of partitions used to process one frame
+ * @current_partition: The partition number currently being configured
*/
struct vsp1_pipeline {
struct media_pipeline pipe;
@@ -109,6 +110,8 @@ struct vsp1_pipeline {
unsigned int div_size;
unsigned int partitions;
+ struct v4l2_rect partition;
+ unsigned int current_partition;
};
void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index de5ef76c5004..e6236ff2f74a 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -72,8 +72,8 @@ static void rpf_configure(struct vsp1_entity *entity,
}
if (params == VSP1_ENTITY_PARAMS_PARTITION) {
- const struct v4l2_rect *crop;
unsigned int offsets[2];
+ struct v4l2_rect crop;
/* Source size and crop offsets.
*
@@ -82,21 +82,47 @@ static void rpf_configure(struct vsp1_entity *entity,
* offsets are needed, as planes 2 and 3 always have identical
* strides.
*/
- crop = vsp1_rwpf_get_crop(rpf, rpf->entity.config);
+ crop = *vsp1_rwpf_get_crop(rpf, rpf->entity.config);
+
+ /* Partition Algorithm Control
+ *
+ * The partition algorithm can split this frame into multiple
+ * slices. We must scale our partition window based on the pipe
+ * configuration to match the destination partition window.
+ * To achieve this, we adjust our crop to provide a 'sub-crop'
+ * matching the expected partition window. Only 'left' and
+ * 'width' need to be adjusted.
+ */
+ if (pipe->partitions > 1) {
+ const struct v4l2_mbus_framefmt *output;
+ struct vsp1_entity *wpf = &pipe->output->entity;
+ unsigned int input_width = crop.width;
+
+ /* Scale the partition window based on the configuration
+ * of the pipeline.
+ */
+ output = vsp1_entity_get_pad_format(wpf, wpf->config,
+ RWPF_PAD_SOURCE);
+
+ crop.width = pipe->partition.width * input_width
+ / output->width;
+ crop.left += pipe->partition.left * input_width
+ / output->width;
+ }
vsp1_rpf_write(rpf, dl, VI6_RPF_SRC_BSIZE,
- (crop->width << VI6_RPF_SRC_BSIZE_BHSIZE_SHIFT) |
- (crop->height << VI6_RPF_SRC_BSIZE_BVSIZE_SHIFT));
+ (crop.width << VI6_RPF_SRC_BSIZE_BHSIZE_SHIFT) |
+ (crop.height << VI6_RPF_SRC_BSIZE_BVSIZE_SHIFT));
vsp1_rpf_write(rpf, dl, VI6_RPF_SRC_ESIZE,
- (crop->width << VI6_RPF_SRC_ESIZE_EHSIZE_SHIFT) |
- (crop->height << VI6_RPF_SRC_ESIZE_EVSIZE_SHIFT));
+ (crop.width << VI6_RPF_SRC_ESIZE_EHSIZE_SHIFT) |
+ (crop.height << VI6_RPF_SRC_ESIZE_EVSIZE_SHIFT));
- offsets[0] = crop->top * format->plane_fmt[0].bytesperline
- + crop->left * fmtinfo->bpp[0] / 8;
+ offsets[0] = crop.top * format->plane_fmt[0].bytesperline
+ + crop.left * fmtinfo->bpp[0] / 8;
if (format->num_planes > 1)
- offsets[1] = crop->top * format->plane_fmt[1].bytesperline
- + crop->left / fmtinfo->hsub
+ offsets[1] = crop.top * format->plane_fmt[1].bytesperline
+ + crop.left / fmtinfo->hsub
* fmtinfo->bpp[1] / 8;
else
offsets[1] = 0;
diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
index 706b6e85f47d..da8f89a31ea4 100644
--- a/drivers/media/platform/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/vsp1/vsp1_uds.c
@@ -18,6 +18,7 @@
#include "vsp1.h"
#include "vsp1_dl.h"
+#include "vsp1_pipe.h"
#include "vsp1_uds.h"
#define UDS_MIN_SIZE 4U
@@ -270,6 +271,15 @@ static void uds_configure(struct vsp1_entity *entity,
unsigned int vscale;
bool multitap;
+ if (params == VSP1_ENTITY_PARAMS_PARTITION) {
+ const struct v4l2_rect *clip = &pipe->partition;
+
+ vsp1_uds_write(uds, dl, VI6_UDS_CLIP_SIZE,
+ (clip->width << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
+ (clip->height << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
+ return;
+ }
+
if (params != VSP1_ENTITY_PARAMS_INIT)
return;
@@ -302,13 +312,10 @@ static void uds_configure(struct vsp1_entity *entity,
(uds_passband_width(vscale)
<< VI6_UDS_PASS_BWIDTH_V_SHIFT));
- /* Set the scaling ratios and the output size. */
+ /* Set the scaling ratios. */
vsp1_uds_write(uds, dl, VI6_UDS_SCALE,
(hscale << VI6_UDS_SCALE_HFRAC_SHIFT) |
(vscale << VI6_UDS_SCALE_VFRAC_SHIFT));
- vsp1_uds_write(uds, dl, VI6_UDS_CLIP_SIZE,
- (output->width << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
- (output->height << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
}
static unsigned int uds_max_width(struct vsp1_entity *entity,
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index b903cc5471e0..15d08cb50bd1 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -205,6 +205,74 @@ static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
pipe->partitions = DIV_ROUND_UP(format->width, div_size);
}
+/*
+ * vsp1_video_partition - Calculate the active partition output window
+ *
+ * @div_size: pre-determined maximum partition division size
+ * @index: partition index
+ *
+ * Returns a v4l2_rect describing the partition window.
+ */
+static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
+ unsigned int div_size,
+ unsigned int index)
+{
+ const struct v4l2_mbus_framefmt *format;
+ struct v4l2_rect partition;
+ unsigned int modulus;
+
+ format = vsp1_entity_get_pad_format(&pipe->output->entity,
+ pipe->output->entity.config,
+ RWPF_PAD_SOURCE);
+
+ /* A single partition simply processes the output size in full. */
+ if (pipe->partitions <= 1) {
+ partition.left = 0;
+ partition.top = 0;
+ partition.width = format->width;
+ partition.height = format->height;
+ return partition;
+ }
+
+ /* Initialise the partition with sane starting conditions. */
+ partition.left = index * div_size;
+ partition.top = 0;
+ partition.width = div_size;
+ partition.height = format->height;
+
+ modulus = format->width % div_size;
+
+ /* We need to prevent the last partition from being smaller than the
+ * *minimum* width of the hardware capabilities.
+ *
+ * If the modulus is less than half of the partition size,
+ * the penultimate partition is reduced to half, which is added
+ * to the final partition: |1234|1234|1234|12|341|
+ * to prevents this: |1234|1234|1234|1234|1|.
+ */
+ if (modulus) {
+ /* pipe->partitions is 1 based, whilst index is a 0 based index.
+ * Normalise this locally.
+ */
+ unsigned int partitions = pipe->partitions - 1;
+
+ if (modulus < div_size / 2) {
+ if (index == partitions - 1) {
+ /* Halve the penultimate partition. */
+ partition.width = div_size / 2;
+ } else if (index == partitions) {
+ /* Increase the final partition. */
+ partition.width = (div_size / 2) + modulus;
+ partition.left -= div_size / 2;
+ }
+ } else if (index == partitions) {
+ partition.width = modulus;
+ }
+ }
+
+ return partition;
+}
+
/* -----------------------------------------------------------------------------
* Pipeline Management
*/
@@ -280,22 +348,69 @@ static void vsp1_video_frame_end(struct vsp1_pipeline *pipe,
pipe->buffers_ready |= 1 << video->pipe_index;
}
+static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe,
+ struct vsp1_dl_list *dl)
+{
+ struct vsp1_entity *entity;
+
+ pipe->partition = vsp1_video_partition(pipe, pipe->div_size,
+ pipe->current_partition);
+
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ if (entity->ops->configure)
+ entity->ops->configure(entity, pipe, dl,
+ VSP1_ENTITY_PARAMS_PARTITION);
+ }
+}
+
static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
{
+ struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
struct vsp1_entity *entity;
if (!pipe->dl)
pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
+ /* Start with the runtime parameters as the configure operation can
+ * compute/cache information needed when configuring partitions. This
+ * is the case with flipping in the WPF.
+ */
list_for_each_entry(entity, &pipe->entities, list_pipe) {
- if (entity->ops->configure) {
+ if (entity->ops->configure)
entity->ops->configure(entity, pipe, pipe->dl,
VSP1_ENTITY_PARAMS_RUNTIME);
- entity->ops->configure(entity, pipe, pipe->dl,
- VSP1_ENTITY_PARAMS_PARTITION);
+ }
+
+ /* Run the first partition */
+ pipe->current_partition = 0;
+ vsp1_video_pipeline_run_partition(pipe, pipe->dl);
+
+ /* Process consecutive partitions as necessary */
+ for (pipe->current_partition = 1;
+ pipe->current_partition < pipe->partitions;
+ pipe->current_partition++) {
+ struct vsp1_dl_list *dl;
+
+ /* Partition configuration operations will utilise
+ * the pipe->current_partition variable to determine
+ * the work they should complete.
+ */
+ dl = vsp1_dl_list_get(pipe->output->dlm);
+
+ /* An incomplete chain will still function, but output only
+ * the partitions that had a dl available. The frame end
+ * interrupt will be marked on the last dl in the chain.
+ */
+ if (!dl) {
+ dev_err(vsp1->dev, "Failed to obtain a dl list. Frame will be incomplete\n");
+ break;
}
+
+ vsp1_video_pipeline_run_partition(pipe, dl);
+ vsp1_dl_list_add_chain(pipe->dl, dl);
}
+ /* Complete, and commit the head display list. */
vsp1_dl_list_commit(pipe->dl);
pipe->dl = NULL;
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index b757d2579d6c..fdee5a891e40 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -224,6 +224,9 @@ static void wpf_configure(struct vsp1_entity *entity,
/* Cropping. The partition algorithm can split the image into
* multiple slices.
*/
+ if (pipe->partitions > 1)
+ width = pipe->partition.width;
+
vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
(0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
(width << VI6_WPF_SZCLIP_SIZE_SHIFT));
@@ -237,10 +240,31 @@ static void wpf_configure(struct vsp1_entity *entity,
/* Update the memory offsets based on flipping configuration.
* The destination addresses point to the locations where the
* VSP starts writing to memory, which can be different corners
- * of the image depending on vertical flipping. Horizontal
- * flipping is handled through a line buffer and doesn't modify
- * the start address.
+ * of the image depending on vertical flipping.
*/
+ if (pipe->partitions > 1) {
+ const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
+
+ /* Horizontal flipping is handled through a line buffer
+ * and doesn't modify the start address, but still needs
+ * to be handled when image partitioning is in effect to
+ * order the partitions correctly.
+ */
+ if (flip & BIT(WPF_CTRL_HFLIP))
+ offset = format->width - pipe->partition.left
+ - pipe->partition.width;
+ else
+ offset = pipe->partition.left;
+
+ mem.addr[0] += offset * fmtinfo->bpp[0] / 8;
+ if (format->num_planes > 1) {
+ mem.addr[1] += offset / fmtinfo->hsub
+ * fmtinfo->bpp[1] / 8;
+ mem.addr[2] += offset / fmtinfo->hsub
+ * fmtinfo->bpp[2] / 8;
+ }
+ }
+
if (flip & BIT(WPF_CTRL_VFLIP)) {
mem.addr[0] += (format->height - 1)
* format->plane_fmt[0].bytesperline;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 13/13] v4l: vsp1: wpf: Implement rotation support
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (11 preceding siblings ...)
2016-09-13 23:17 ` [PATCH 12/13] v4l: vsp1: Support multiple partitions per frame Laurent Pinchart
@ 2016-09-13 23:17 ` Laurent Pinchart
2016-09-13 23:29 ` [PATCH 14/13] v4l: vsp1: Fix spinlock in mixed IRQ context function Laurent Pinchart
13 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:17 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
Some WPF instances, on Gen3 devices, can perform 90° rotation when
writing frames to memory. Implement support for this using the
V4L2_CID_ROTATE control.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_rpf.c | 2 +-
drivers/media/platform/vsp1/vsp1_rwpf.c | 5 +
drivers/media/platform/vsp1/vsp1_rwpf.h | 3 +-
drivers/media/platform/vsp1/vsp1_video.c | 10 +-
drivers/media/platform/vsp1/vsp1_wpf.c | 200 +++++++++++++++++++++++--------
5 files changed, 169 insertions(+), 51 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index e6236ff2f74a..b54f0513f4b0 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -102,7 +102,7 @@ static void rpf_configure(struct vsp1_entity *entity,
* of the pipeline.
*/
output = vsp1_entity_get_pad_format(wpf, wpf->config,
- RWPF_PAD_SOURCE);
+ RWPF_PAD_SINK);
crop.width = pipe->partition.width * input_width
/ output->width;
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c b/drivers/media/platform/vsp1/vsp1_rwpf.c
index a3ace8df7f4d..ca18c9cf1d6a 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
@@ -118,6 +118,11 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
RWPF_PAD_SOURCE);
*format = fmt->format;
+ if (rwpf->flip.rotate) {
+ format->width = fmt->format.height;
+ format->height = fmt->format.width;
+ }
+
done:
mutex_unlock(&rwpf->entity.lock);
return ret;
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h b/drivers/media/platform/vsp1/vsp1_rwpf.h
index 1c98aff3da5d..b4ffc38f48af 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
@@ -56,9 +56,10 @@ struct vsp1_rwpf {
struct {
spinlock_t lock;
- struct v4l2_ctrl *ctrls[2];
+ struct v4l2_ctrl *ctrls[3];
unsigned int pending;
unsigned int active;
+ bool rotate;
} flip;
struct vsp1_rwpf_memory mem;
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 15d08cb50bd1..ef65d2c1e76e 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -179,9 +179,12 @@ static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
struct vsp1_entity *entity;
unsigned int div_size;
+ /* Partitions are computed on the size before rotation, use the format
+ * at the WPF sink.
+ */
format = vsp1_entity_get_pad_format(&pipe->output->entity,
pipe->output->entity.config,
- RWPF_PAD_SOURCE);
+ RWPF_PAD_SINK);
div_size = format->width;
/* Gen2 hardware doesn't require image partitioning. */
@@ -221,9 +224,12 @@ static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
struct v4l2_rect partition;
unsigned int modulus;
+ /* Partitions are computed on the size before rotation, use the format
+ * at the WPF sink.
+ */
format = vsp1_entity_get_pad_format(&pipe->output->entity,
pipe->output->entity.config,
- RWPF_PAD_SOURCE);
+ RWPF_PAD_SINK);
/* A single partition simply processes the output size in full. */
if (pipe->partitions <= 1) {
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index fdee5a891e40..b4ecffbaa3e3 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -43,32 +43,92 @@ static inline void vsp1_wpf_write(struct vsp1_rwpf *wpf,
enum wpf_flip_ctrl {
WPF_CTRL_VFLIP = 0,
WPF_CTRL_HFLIP = 1,
- WPF_CTRL_MAX,
+ WPF_CTRL_ROTATE = 2,
};
+static int vsp1_wpf_set_rotation(struct vsp1_rwpf *wpf, unsigned int rotation)
+{
+ struct media_device *mdev = &wpf->entity.vsp1->media_dev;
+ struct v4l2_mbus_framefmt *sink_format;
+ struct v4l2_mbus_framefmt *source_format;
+ bool rotate;
+ int ret = 0;
+
+ /* Only consider the 0°/180° from/to 90°/270° modifications, the rest
+ * is taken care of by the flipping configuration.
+ */
+ rotate = rotation == 90 || rotation == 270;
+ if (rotate == wpf->flip.rotate)
+ return 0;
+
+ /* Changing rotation isn't allowed during streaming. */
+ mutex_lock(&mdev->graph_mutex);
+
+ if (wpf->entity.subdev.entity.stream_count) {
+ ret = -EBUSY;
+ goto done;
+ }
+
+ sink_format = vsp1_entity_get_pad_format(&wpf->entity,
+ wpf->entity.config,
+ RWPF_PAD_SINK);
+ source_format = vsp1_entity_get_pad_format(&wpf->entity,
+ wpf->entity.config,
+ RWPF_PAD_SOURCE);
+
+ mutex_lock(&wpf->entity.lock);
+
+ if (rotate) {
+ source_format->width = sink_format->height;
+ source_format->height = sink_format->width;
+ } else {
+ source_format->width = sink_format->width;
+ source_format->height = sink_format->height;
+ }
+
+ wpf->flip.rotate = rotate;
+
+ mutex_unlock(&wpf->entity.lock);
+
+done:
+ mutex_unlock(&mdev->graph_mutex);
+ return ret;
+}
+
static int vsp1_wpf_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct vsp1_rwpf *wpf =
container_of(ctrl->handler, struct vsp1_rwpf, ctrls);
- unsigned int i;
+ unsigned int rotation;
u32 flip = 0;
+ int ret;
- switch (ctrl->id) {
- case V4L2_CID_HFLIP:
- case V4L2_CID_VFLIP:
- for (i = 0; i < WPF_CTRL_MAX; ++i) {
- if (wpf->flip.ctrls[i])
- flip |= wpf->flip.ctrls[i]->val ? BIT(i) : 0;
- }
+ /* Update the rotation. */
+ rotation = wpf->flip.ctrls[WPF_CTRL_ROTATE]
+ ? wpf->flip.ctrls[WPF_CTRL_ROTATE]->val : 0;
- spin_lock_irq(&wpf->flip.lock);
- wpf->flip.pending = flip;
- spin_unlock_irq(&wpf->flip.lock);
- break;
+ ret = vsp1_wpf_set_rotation(wpf, rotation);
+ if (ret < 0)
+ return ret;
- default:
- return -EINVAL;
- }
+ /* Compute the flip value resulting from all three controls, with
+ * rotation by 180° flipping the image in both directions. Store the
+ * result in the pending flip field for the next frame that will be
+ * processed.
+ */
+ if (wpf->flip.ctrls[WPF_CTRL_VFLIP]->val)
+ flip |= BIT(WPF_CTRL_VFLIP);
+
+ if (wpf->flip.ctrls[WPF_CTRL_HFLIP] &&
+ wpf->flip.ctrls[WPF_CTRL_HFLIP]->val)
+ flip |= BIT(WPF_CTRL_HFLIP);
+
+ if (rotation == 180 || rotation == 270)
+ flip ^= BIT(WPF_CTRL_VFLIP) | BIT(WPF_CTRL_HFLIP);
+
+ spin_lock_irq(&wpf->flip.lock);
+ wpf->flip.pending = flip;
+ spin_unlock_irq(&wpf->flip.lock);
return 0;
}
@@ -88,10 +148,10 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
/* Only WPF0 supports flipping. */
num_flip_ctrls = 0;
} else if (vsp1->info->features & VSP1_HAS_WPF_HFLIP) {
- /* When horizontal flip is supported the WPF implements two
- * controls (horizontal flip and vertical flip).
+ /* When horizontal flip is supported the WPF implements three
+ * controls (horizontal flip, vertical flip and rotation).
*/
- num_flip_ctrls = 2;
+ num_flip_ctrls = 3;
} else if (vsp1->info->features & VSP1_HAS_WPF_VFLIP) {
/* When only vertical flip is supported the WPF implements a
* single control (vertical flip).
@@ -110,12 +170,14 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
V4L2_CID_VFLIP, 0, 1, 1, 0);
}
- if (num_flip_ctrls == 2) {
+ if (num_flip_ctrls == 3) {
wpf->flip.ctrls[WPF_CTRL_HFLIP] =
v4l2_ctrl_new_std(&wpf->ctrls, &vsp1_wpf_ctrl_ops,
V4L2_CID_HFLIP, 0, 1, 1, 0);
-
- v4l2_ctrl_cluster(2, wpf->flip.ctrls);
+ wpf->flip.ctrls[WPF_CTRL_ROTATE] =
+ v4l2_ctrl_new_std(&wpf->ctrls, &vsp1_wpf_ctrl_ops,
+ V4L2_CID_ROTATE, 0, 270, 90, 0);
+ v4l2_ctrl_cluster(3, wpf->flip.ctrls);
}
if (wpf->ctrls.error) {
@@ -215,10 +277,11 @@ static void wpf_configure(struct vsp1_entity *entity,
if (params == VSP1_ENTITY_PARAMS_PARTITION) {
const struct v4l2_pix_format_mplane *format = &wpf->format;
+ const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
struct vsp1_rwpf_memory mem = wpf->mem;
unsigned int flip = wpf->flip.active;
- unsigned int width = source_format->width;
- unsigned int height = source_format->height;
+ unsigned int width = sink_format->width;
+ unsigned int height = sink_format->height;
unsigned int offset;
/* Cropping. The partition algorithm can split the image into
@@ -239,44 +302,75 @@ static void wpf_configure(struct vsp1_entity *entity,
/* Update the memory offsets based on flipping configuration.
* The destination addresses point to the locations where the
- * VSP starts writing to memory, which can be different corners
- * of the image depending on vertical flipping.
+ * VSP starts writing to memory, which can be any corner of the
+ * image depending on the combination of flipping and rotation.
*/
- if (pipe->partitions > 1) {
- const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
- /* Horizontal flipping is handled through a line buffer
- * and doesn't modify the start address, but still needs
- * to be handled when image partitioning is in effect to
- * order the partitions correctly.
- */
- if (flip & BIT(WPF_CTRL_HFLIP))
- offset = format->width - pipe->partition.left
- - pipe->partition.width;
+ /* First take the partition left coordinate into account.
+ * Compute the offset to order the partitions correctly on the
+ * output based on whether flipping is enabled. Consider
+ * horizontal flipping when rotation is disabled but vertical
+ * flipping when rotation is enabled, as rotating the image
+ * switches the horizontal and vertical directions. The offset
+ * is applied horizontally or vertically accordingly.
+ */
+ if (flip & BIT(WPF_CTRL_HFLIP) && !wpf->flip.rotate)
+ offset = format->width - pipe->partition.left
+ - pipe->partition.width;
+ else if (flip & BIT(WPF_CTRL_VFLIP) && wpf->flip.rotate)
+ offset = format->height - pipe->partition.left
+ - pipe->partition.width;
+ else
+ offset = pipe->partition.left;
+
+ for (i = 0; i < format->num_planes; ++i) {
+ unsigned int hsub = i > 0 ? fmtinfo->hsub : 1;
+ unsigned int vsub = i > 0 ? fmtinfo->vsub : 1;
+
+ if (wpf->flip.rotate)
+ mem.addr[i] += offset / vsub
+ * format->plane_fmt[i].bytesperline;
else
- offset = pipe->partition.left;
-
- mem.addr[0] += offset * fmtinfo->bpp[0] / 8;
- if (format->num_planes > 1) {
- mem.addr[1] += offset / fmtinfo->hsub
- * fmtinfo->bpp[1] / 8;
- mem.addr[2] += offset / fmtinfo->hsub
- * fmtinfo->bpp[2] / 8;
- }
+ mem.addr[i] += offset / hsub
+ * fmtinfo->bpp[i] / 8;
}
if (flip & BIT(WPF_CTRL_VFLIP)) {
- mem.addr[0] += (format->height - 1)
+ /* When rotating the output (after rotation) image
+ * height is equal to the partition width (before
+ * rotation). Otherwise it is equal to the output
+ * image height.
+ */
+ if (wpf->flip.rotate)
+ height = pipe->partition.width;
+ else
+ height = format->height;
+
+ mem.addr[0] += (height - 1)
* format->plane_fmt[0].bytesperline;
if (format->num_planes > 1) {
- offset = (format->height / wpf->fmtinfo->vsub - 1)
+ offset = (height / fmtinfo->vsub - 1)
* format->plane_fmt[1].bytesperline;
mem.addr[1] += offset;
mem.addr[2] += offset;
}
}
+ if (wpf->flip.rotate && !(flip & BIT(WPF_CTRL_HFLIP))) {
+ unsigned int hoffset = max(0, (int)format->width - 16);
+
+ /* Compute the output coordinate. The partition
+ * horizontal (left) offset becomes a vertical offset.
+ */
+ for (i = 0; i < format->num_planes; ++i) {
+ unsigned int hsub = i > 0 ? fmtinfo->hsub : 1;
+
+ mem.addr[i] += hoffset / hsub
+ * fmtinfo->bpp[i] / 8;
+ }
+ }
+
vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_Y, mem.addr[0]);
vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_C0, mem.addr[1]);
vsp1_wpf_write(wpf, dl, VI6_WPF_DSTM_ADDR_C1, mem.addr[2]);
@@ -290,6 +384,9 @@ static void wpf_configure(struct vsp1_entity *entity,
outfmt = fmtinfo->hwfmt << VI6_WPF_OUTFMT_WRFMT_SHIFT;
+ if (wpf->flip.rotate)
+ outfmt |= VI6_WPF_OUTFMT_ROT;
+
if (fmtinfo->alpha)
outfmt |= VI6_WPF_OUTFMT_PXA;
if (fmtinfo->swap_yc)
@@ -350,9 +447,18 @@ static void wpf_configure(struct vsp1_entity *entity,
VI6_WFP_IRQ_ENB_DFEE);
}
+static unsigned int wpf_max_width(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe)
+{
+ struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
+
+ return wpf->flip.rotate ? 256 : wpf->max_width;
+}
+
static const struct vsp1_entity_operations wpf_entity_ops = {
.destroy = vsp1_wpf_destroy,
.configure = wpf_configure,
+ .max_width = wpf_max_width,
};
/* -----------------------------------------------------------------------------
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 14/13] v4l: vsp1: Fix spinlock in mixed IRQ context function
2016-09-13 23:16 [PATCH 00/13] Renesas R-Car VSP: Scaling and rotation support on Gen3 Laurent Pinchart
` (12 preceding siblings ...)
2016-09-13 23:17 ` [PATCH 13/13] v4l: vsp1: wpf: Implement rotation support Laurent Pinchart
@ 2016-09-13 23:29 ` Laurent Pinchart
2016-09-14 19:30 ` Niklas Söderlund
13 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2016-09-13 23:29 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Kieran Bingham
The wpf_configure() function can be called both from IRQ and non-IRQ
contexts, use spin_lock_irqsave().
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_wpf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index b4ecffbaa3e3..c483fead3e98 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -251,11 +251,12 @@ static void wpf_configure(struct vsp1_entity *entity,
if (params == VSP1_ENTITY_PARAMS_RUNTIME) {
const unsigned int mask = BIT(WPF_CTRL_VFLIP)
| BIT(WPF_CTRL_HFLIP);
+ unsigned long flags;
- spin_lock(&wpf->flip.lock);
+ spin_lock_irqsave(&wpf->flip.lock, flags);
wpf->flip.active = (wpf->flip.active & ~mask)
| (wpf->flip.pending & mask);
- spin_unlock(&wpf->flip.lock);
+ spin_unlock_irqrestore(&wpf->flip.lock, flags);
outfmt = (wpf->alpha << VI6_WPF_OUTFMT_PDV_SHIFT) | wpf->outfmt;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 14/13] v4l: vsp1: Fix spinlock in mixed IRQ context function
2016-09-13 23:29 ` [PATCH 14/13] v4l: vsp1: Fix spinlock in mixed IRQ context function Laurent Pinchart
@ 2016-09-14 19:30 ` Niklas Söderlund
0 siblings, 0 replies; 32+ messages in thread
From: Niklas Söderlund @ 2016-09-14 19:30 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, Kieran Bingham
On 2016-09-14 02:29:08 +0300, Laurent Pinchart wrote:
> The wpf_configure() function can be called both from IRQ and non-IRQ
> contexts, use spin_lock_irqsave().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/media/platform/vsp1/vsp1_wpf.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
> index b4ecffbaa3e3..c483fead3e98 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -251,11 +251,12 @@ static void wpf_configure(struct vsp1_entity *entity,
> if (params == VSP1_ENTITY_PARAMS_RUNTIME) {
> const unsigned int mask = BIT(WPF_CTRL_VFLIP)
> | BIT(WPF_CTRL_HFLIP);
> + unsigned long flags;
>
> - spin_lock(&wpf->flip.lock);
> + spin_lock_irqsave(&wpf->flip.lock, flags);
> wpf->flip.active = (wpf->flip.active & ~mask)
> | (wpf->flip.pending & mask);
> - spin_unlock(&wpf->flip.lock);
> + spin_unlock_irqrestore(&wpf->flip.lock, flags);
>
> outfmt = (wpf->alpha << VI6_WPF_OUTFMT_PDV_SHIFT) | wpf->outfmt;
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 32+ messages in thread