| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d5425795-a581-65f6-ea19-f9dd3e07867f@xs4all.nl>
Date: Mon, 12 Jun 2017 12:03:06 +0200
From: Hans Verkuil <hverkuil@...all.nl>
To: Helen Koike <helen.koike@...labora.com>,
linux-media@...r.kernel.org,
Mauro Carvalho Chehab <mchehab@...nel.org>,
linux-kernel@...r.kernel.org
Cc: jgebben@...eaurora.org, mchehab@....samsung.com,
Sakari Ailus <sakari.ailus@....fi>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [RFC PATCH v3 08/11] [media] vimc: Optimize frame generation
through the pipe
On 06/03/2017 04:58 AM, Helen Koike wrote:
> Add a parameter called vsen_tpg, if true then vimc will work as before:
> frames will be generated in the sensor nodes then propagated through the
> pipe and processed by each node until a capture node is reached.
> If vsen_tpg is false, then the frame is not generated by the sensor, but
> directly from the capture node, thus saving intermediate memory buffers
> and process time, allowing a higher frame rate.
>
> Signed-off-by: Helen Koike <helen.koike@...labora.com>
>
> ---
>
> Changes in v3:
> [media] vimc: Optimize frame generation through the pipe
> - This is a new patch in the series
>
> Changes in v2: None
>
>
> ---
> drivers/media/platform/vimc/vimc-capture.c | 178 +++++++++++++++++++++--------
> drivers/media/platform/vimc/vimc-common.h | 2 +
> drivers/media/platform/vimc/vimc-sensor.c | 30 ++++-
> 3 files changed, 156 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> index e943267..b5da0ea 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -15,7 +15,10 @@
> *
> */
>
> +#include <linux/freezer.h>
> +#include <linux/kthread.h>
> #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-tpg.h>
> #include <media/videobuf2-core.h>
> #include <media/videobuf2-vmalloc.h>
>
> @@ -38,6 +41,8 @@ struct vimc_cap_device {
> struct mutex lock;
> u32 sequence;
> struct media_pipeline pipe;
> + struct tpg_data tpg;
> + struct task_struct *kthread_cap;
> };
>
> static const struct v4l2_pix_format fmt_default = {
> @@ -246,6 +251,91 @@ static void vimc_cap_return_all_buffers(struct vimc_cap_device *vcap,
> spin_unlock(&vcap->qlock);
> }
>
> +static void vimc_cap_process_frame(struct vimc_ent_device *ved,
> + struct media_pad *sink, const void *frame)
> +{
> + struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
> + ved);
> + struct vimc_cap_buffer *vimc_buf;
> + void *vbuf;
> +
> + spin_lock(&vcap->qlock);
> +
> + /* Get the first entry of the list */
> + vimc_buf = list_first_entry_or_null(&vcap->buf_list,
> + typeof(*vimc_buf), list);
> + if (!vimc_buf) {
> + spin_unlock(&vcap->qlock);
> + return;
> + }
> +
> + /* Remove this entry from the list */
> + list_del(&vimc_buf->list);
> +
> + spin_unlock(&vcap->qlock);
> +
> + /* Fill the buffer */
> + vimc_buf->vb2.vb2_buf.timestamp = ktime_get_ns();
> + vimc_buf->vb2.sequence = vcap->sequence++;
> + vimc_buf->vb2.field = vcap->format.field;
> +
> + vbuf = vb2_plane_vaddr(&vimc_buf->vb2.vb2_buf, 0);
> +
> + if (sink)
> + memcpy(vbuf, frame, vcap->format.sizeimage);
> + else
> + tpg_fill_plane_buffer(&vcap->tpg, V4L2_STD_PAL, 0, vbuf);
> +
> + /* Set it as ready */
> + vb2_set_plane_payload(&vimc_buf->vb2.vb2_buf, 0,
> + vcap->format.sizeimage);
> + vb2_buffer_done(&vimc_buf->vb2.vb2_buf, VB2_BUF_STATE_DONE);
> +}
> +
> +
> +static int vimc_cap_tpg_thread(void *data)
> +{
> + struct vimc_cap_device *vcap = data;
> +
> + set_freezable();
> + set_current_state(TASK_UNINTERRUPTIBLE);
> +
> + for (;;) {
> + try_to_freeze();
> + if (kthread_should_stop())
> + break;
> +
> + vimc_cap_process_frame(&vcap->ved, NULL, NULL);
> +
> + /* 60 frames per second */
> + schedule_timeout(HZ/60);
> + }
> +
> + return 0;
> +}
> +
> +static void vimc_cap_tpg_s_format(struct vimc_cap_device *vcap)
> +{
> + const struct vimc_pix_map *vpix =
> + vimc_pix_map_by_pixelformat(vcap->format.pixelformat);
> +
> + tpg_reset_source(&vcap->tpg, vcap->format.width, vcap->format.height,
> + vcap->format.field);
> + tpg_s_bytesperline(&vcap->tpg, 0, vcap->format.width * vpix->bpp);
> + tpg_s_buf_height(&vcap->tpg, vcap->format.height);
> + tpg_s_fourcc(&vcap->tpg, vpix->pixelformat);
> + /*
> + * TODO: check why the tpg_s_field need this third argument if
> + * it is already receiving the field
> + */
> + tpg_s_field(&vcap->tpg, vcap->format.field,
> + vcap->format.field == V4L2_FIELD_ALTERNATE);
I thought I explained that earlier? When in alternate field mode the format.field
value will be TOP or BOTTOM, but never ALTERNATE. So tpg_s_field needs to know if
it will create TOP or BOTTOM fields only, or if they will be alternating.
Since we don't support ALTERNATE anyway, just pass false as the third argument and
drop the comment.
> + tpg_s_colorspace(&vcap->tpg, vcap->format.colorspace);
> + tpg_s_ycbcr_enc(&vcap->tpg, vcap->format.ycbcr_enc);
> + tpg_s_quantization(&vcap->tpg, vcap->format.quantization);
> + tpg_s_xfer_func(&vcap->tpg, vcap->format.xfer_func);
> +}
> +
> static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
> {
> struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
> @@ -256,20 +346,43 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
>
> /* Start the media pipeline */
> ret = media_pipeline_start(entity, &vcap->pipe);
> - if (ret) {
> - vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
> - return ret;
> - }
> + if (ret)
> + goto err_ret_all_buffs;
>
> /* Enable streaming from the pipe */
> ret = vimc_pipeline_s_stream(&vcap->vdev.entity, 1);
> - if (ret) {
> - media_pipeline_stop(entity);
> - vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
> - return ret;
> + if (ret < 0)
> + goto err_mpipe_stop;
> +
> + if (ret == VIMC_PIPE_OPT) {
> + tpg_init(&vcap->tpg, vcap->format.width, vcap->format.height);
> + ret = tpg_alloc(&vcap->tpg, VIMC_FRAME_MAX_WIDTH);
> + if (ret)
> + /* We don't need to call vimc_pipeline_s_stream(e, 0) */
> + goto err_mpipe_stop;
> +
> + vimc_cap_tpg_s_format(vcap);
> + vcap->kthread_cap = kthread_run(vimc_cap_tpg_thread, vcap,
> + "%s-cap", vcap->vdev.v4l2_dev->name);
> + if (IS_ERR(vcap->kthread_cap)) {
> + dev_err(vcap->vdev.v4l2_dev->dev,
> + "%s: kernel_thread() failed\n",
> + vcap->vdev.name);
> + ret = PTR_ERR(vcap->kthread_cap);
> + goto err_tpg_free;
> + }
> }
>
> return 0;
> +
> +err_tpg_free:
> + tpg_free(&vcap->tpg);
> +err_mpipe_stop:
> + media_pipeline_stop(entity);
> +err_ret_all_buffs:
> + vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
> +
> + return ret;
> }
>
> /*
> @@ -280,8 +393,15 @@ static void vimc_cap_stop_streaming(struct vb2_queue *vq)
> {
> struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
>
> - /* Disable streaming from the pipe */
> - vimc_pipeline_s_stream(&vcap->vdev.entity, 0);
> + if (vcap->kthread_cap) {
> + /* Stop image generator */
> + kthread_stop(vcap->kthread_cap);
> + vcap->kthread_cap = NULL;
> + tpg_free(&vcap->tpg);
> + } else {
> + /* Disable streaming from the pipe */
> + vimc_pipeline_s_stream(&vcap->vdev.entity, 0);
> + }
>
> /* Stop the media pipeline */
> media_pipeline_stop(&vcap->vdev.entity);
> @@ -361,44 +481,6 @@ static void vimc_cap_destroy(struct vimc_ent_device *ved)
> kfree(vcap);
> }
>
> -static void vimc_cap_process_frame(struct vimc_ent_device *ved,
> - struct media_pad *sink, const void *frame)
> -{
> - struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
> - ved);
> - struct vimc_cap_buffer *vimc_buf;
> - void *vbuf;
> -
> - spin_lock(&vcap->qlock);
> -
> - /* Get the first entry of the list */
> - vimc_buf = list_first_entry_or_null(&vcap->buf_list,
> - typeof(*vimc_buf), list);
> - if (!vimc_buf) {
> - spin_unlock(&vcap->qlock);
> - return;
> - }
> -
> - /* Remove this entry from the list */
> - list_del(&vimc_buf->list);
> -
> - spin_unlock(&vcap->qlock);
> -
> - /* Fill the buffer */
> - vimc_buf->vb2.vb2_buf.timestamp = ktime_get_ns();
> - vimc_buf->vb2.sequence = vcap->sequence++;
> - vimc_buf->vb2.field = vcap->format.field;
> -
> - vbuf = vb2_plane_vaddr(&vimc_buf->vb2.vb2_buf, 0);
> -
> - memcpy(vbuf, frame, vcap->format.sizeimage);
> -
> - /* Set it as ready */
> - vb2_set_plane_payload(&vimc_buf->vb2.vb2_buf, 0,
> - vcap->format.sizeimage);
> - vb2_buffer_done(&vimc_buf->vb2.vb2_buf, VB2_BUF_STATE_DONE);
> -}
> -
> struct vimc_ent_device *vimc_cap_create(struct v4l2_device *v4l2_dev,
> const char *const name,
> u16 num_pads,
> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> index 2189fd6..5b2691e 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -27,6 +27,8 @@
> #define VIMC_FRAME_MIN_WIDTH 16
> #define VIMC_FRAME_MIN_HEIGHT 16
>
> +#define VIMC_PIPE_OPT 1
> +
> /**
> * struct vimc_pix_map - maps media bus code with v4l2 pixel format
> *
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 90c41c6..df89ee7 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -15,6 +15,7 @@
> *
> */
>
> +#include <linux/module.h>
> #include <linux/freezer.h>
> #include <linux/kthread.h>
> #include <linux/v4l2-mediabus.h>
> @@ -24,6 +25,13 @@
>
> #include "vimc-sensor.h"
>
> +static bool vsen_tpg;
> +module_param(vsen_tpg, bool, 0000);
> +MODULE_PARM_DESC(vsen_tpg, " generate image from sensor node\n"
> + "If set to false, then the pipe will be optimized and image will be "
> + "generated directly in the capture node instead of going through "
> + "the whole pipe");
> +
> struct vimc_sen_device {
> struct vimc_ent_device ved;
> struct v4l2_subdev sd;
> @@ -239,6 +247,13 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
> container_of(sd, struct vimc_sen_device, sd);
> int ret;
>
> + if (!vsen_tpg)
> + /*
> + * If we are not generating the frames, then inform the caller
> + * to generate the frame in shallow level of the pipe
> + */
> + return VIMC_PIPE_OPT;
> +
> if (enable) {
> const struct vimc_pix_map *vpix;
> unsigned int frame_size;
> @@ -306,7 +321,8 @@ static void vimc_sen_destroy(struct vimc_ent_device *ved)
> container_of(ved, struct vimc_sen_device, ved);
>
> vimc_ent_sd_unregister(ved, &vsen->sd);
> - tpg_free(&vsen->tpg);
> + if (vsen_tpg)
> + tpg_free(&vsen->tpg);
> kfree(vsen);
> }
>
> @@ -344,11 +360,13 @@ struct vimc_ent_device *vimc_sen_create(struct v4l2_device *v4l2_dev,
> vsen->mbus_format = fmt_default;
>
> /* Initialize the test pattern generator */
> - tpg_init(&vsen->tpg, vsen->mbus_format.width,
> - vsen->mbus_format.height);
> - ret = tpg_alloc(&vsen->tpg, VIMC_FRAME_MAX_WIDTH);
> - if (ret)
> - goto err_unregister_ent_sd;
> + if (vsen_tpg) {
> + tpg_init(&vsen->tpg, vsen->mbus_format.width,
> + vsen->mbus_format.height);
> + ret = tpg_alloc(&vsen->tpg, VIMC_FRAME_MAX_WIDTH);
> + if (ret)
> + goto err_unregister_ent_sd;
> + }
>
> return &vsen->ved;
>
>
Regards,
Hans
Powered by blists - more mailing lists