Path: csiph.com!news.redatomik.org!aioe.org!bofh.it!news.nic.it!robomod From: Jacek Anaszewski Newsgroups: linux.kernel Subject: Re: [PATCH 9/9] [media] s5p-jpeg: Add support for multi-planar APIs Date: Sat, 03 Jun 2017 00:10:01 +0200 Message-ID: References: Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=pToEQnfq3kkNwo/RDZ/DUZDwq3IW/S7EuOMiQ51m6XA=; b=I/NXZjIa15lz16nVcn1B8FKhQHxAdijjXkv9eVt5cUtbep9+/97ZMZg0Ahs6RHwfbd 8Zyv4JzKkVL4GkIe+SW1kUgpU4u12ap2E05E3QnxiKqUPsrY2WrhqTar88PC2aQFnxFH WCzDuFwA89G+hViGta3kZb1uYtSpyn/YruDURu54yqS6UhO2NggEZiSk5mKd+GHrhMFS JS5119pJhyiVzIFwdzPdAU/RYGeYkh4DuQ0uIuUeiCSgFfd+NXHpLF6mFAFLdEnI2RoB PgxbFyJZMfaUAQs5AfhwYYzyMLzXBNlZtf7IziI8B3i3uIGh+fEvaWpwgTCnOZQn/aiy 3y+A== X-Google-Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=pToEQnfq3kkNwo/RDZ/DUZDwq3IW/S7EuOMiQ51m6XA=; b=RwZV4xKnt3gytkZfJhsbS0rhN+g2UMuG1lNnDtl2NxRXZVMma2mk/1Aa2TZbJhztOt mG9N8SJDmjismn8UavjJs0T89KBspT+cnlcPjEq4GcAJjDcqGEYZ7eNEl4fft0I9slp4 ixWX21Nt0IYW2CyIuqO3658cH/EAYPwgYi6VMW7DwPzhPY2nON2dHWRLZk1nmD5M0D9g UrmKQuWnPxNP0sOr9KBM2ZLg8igbrZu9OcRD3O8hPXxIW51vqEFk8eVkUK8MhAxusA9F NLT+dr5wA1zYANc2XRIecQCJ4horqXJ1Wafn3pwfEOFprRuE5QGbDSfRYXUdHlZTlEsM Jb/w== X-Gm-Message-State: AODbwcAZQbGAa+doy1uEAmA+j7NIpgLjYvzesCfag6I+5ciGcjleTAG3 a6IUOsO/sNOupxEqeH8= X-Received: by 10.28.197.129 with SMTP id v123mr775906wmf.101.1496441103205; Fri, 02 Jun 2017 15:05:03 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: robomod@news.nic.it List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Approved: robomod@news.nic.it Lines: 286 Organization: linux.* mail to news gateway X-Original-Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org X-Original-Date: Sat, 3 Jun 2017 00:04:20 +0200 X-Original-Message-ID: <6ad28dbf-a031-29a2-11a8-cdd688be3c3d@gmail.com> X-Original-References: <1496419376-17099-1-git-send-email-thierry.escande@collabora.com> <1496419376-17099-10-git-send-email-thierry.escande@collabora.com> X-Original-Sender: linux-kernel-owner@vger.kernel.org Xref: csiph.com linux.kernel:1656600 Hi Thierry, What is the gain of introducing multiplanar API for this hardware? AFAIR all the HW implementations store the data in a single contiguous memory region and use suitable padding between planes. On 06/02/2017 06:02 PM, Thierry Escande wrote: > From: Ricky Liang > > This patch adds multi-planar APIs to the s5p-jpeg driver. The multi-planar > APIs are identical to the exisiting single-planar APIs except the plane > format info is stored in the v4l2_pixel_format_mplan struct instead > of the v4l2_pixel_format struct. > > Signed-off-by: Ricky Liang > Signed-off-by: Thierry Escande > --- > drivers/media/platform/s5p-jpeg/jpeg-core.c | 152 +++++++++++++++++++++++++--- > drivers/media/platform/s5p-jpeg/jpeg-core.h | 2 + > 2 files changed, 139 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c > index db56135..a8fd7ed 100644 > --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c > +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c > @@ -1371,6 +1371,15 @@ static int s5p_jpeg_querycap(struct file *file, void *priv, > dev_name(ctx->jpeg->dev)); > cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M; > cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; > + /* > + * Advertise multi-planar capabilities. The driver supports only > + * single-planar pixel format at this moment so all the buffers will > + * have only one plane. > + */ > + cap->capabilities |= V4L2_CAP_VIDEO_M2M_MPLANE | > + V4L2_CAP_VIDEO_CAPTURE_MPLANE | > + V4L2_CAP_VIDEO_OUTPUT_MPLANE; > + > return 0; > } > > @@ -1430,12 +1439,10 @@ static int s5p_jpeg_enum_fmt_vid_out(struct file *file, void *priv, > static struct s5p_jpeg_q_data *get_q_data(struct s5p_jpeg_ctx *ctx, > enum v4l2_buf_type type) > { > - if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT) > + if (V4L2_TYPE_IS_OUTPUT(type)) > return &ctx->out_q; > - if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > - return &ctx->cap_q; > > - return NULL; > + return &ctx->cap_q; > } > > static int s5p_jpeg_g_fmt(struct file *file, void *priv, struct v4l2_format *f) > @@ -1449,16 +1456,14 @@ static int s5p_jpeg_g_fmt(struct file *file, void *priv, struct v4l2_format *f) > if (!vq) > return -EINVAL; > > - if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && > + if (!V4L2_TYPE_IS_OUTPUT(f->type) && > ct->mode == S5P_JPEG_DECODE && !ct->hdr_parsed) > return -EINVAL; > q_data = get_q_data(ct, f->type); > BUG_ON(q_data == NULL); > > - if ((f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && > - ct->mode == S5P_JPEG_ENCODE) || > - (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT && > - ct->mode == S5P_JPEG_DECODE)) { > + if ((!V4L2_TYPE_IS_OUTPUT(f->type) && ct->mode == S5P_JPEG_ENCODE) || > + (V4L2_TYPE_IS_OUTPUT(f->type) && ct->mode == S5P_JPEG_DECODE)) { > pix->width = 0; > pix->height = 0; > } else { > @@ -1715,6 +1720,8 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct v4l2_format *f) > > q_data = get_q_data(ct, f->type); > BUG_ON(q_data == NULL); > + vq->type = f->type; > + q_data->type = f->type; > > if (vb2_is_busy(vq)) { > v4l2_err(&ct->jpeg->v4l2_dev, "%s queue busy\n", __func__); > @@ -1919,7 +1926,9 @@ static int s5p_jpeg_g_selection(struct file *file, void *priv, > struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv); > > if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT && > - s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > + s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE && > + s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE && > + s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) > return -EINVAL; > > /* For JPEG blob active == default == bounds */ > @@ -1957,7 +1966,8 @@ static int s5p_jpeg_s_selection(struct file *file, void *fh, > struct v4l2_rect *rect = &s->r; > int ret = -EINVAL; > > - if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE && > + s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) > return -EINVAL; > > if (s->target == V4L2_SEL_TGT_COMPOSE) { > @@ -2118,6 +2128,107 @@ static int s5p_jpeg_controls_create(struct s5p_jpeg_ctx *ctx) > return ret; > } > > +static void v4l2_format_pixmp_to_pix(struct v4l2_format *fmt_pix_mp, > + struct v4l2_format *fmt_pix) { > + struct v4l2_pix_format *pix = &fmt_pix->fmt.pix; > + struct v4l2_pix_format_mplane *pix_mp = &fmt_pix_mp->fmt.pix_mp; > + > + fmt_pix->type = fmt_pix_mp->type; > + pix->width = pix_mp->width; > + pix->height = pix_mp->height; > + pix->pixelformat = pix_mp->pixelformat; > + pix->field = pix_mp->field; > + pix->colorspace = pix_mp->colorspace; > + pix->bytesperline = pix_mp->plane_fmt[0].bytesperline; > + pix->sizeimage = pix_mp->plane_fmt[0].sizeimage; > +} > + > +static void v4l2_format_pixmp_from_pix(struct v4l2_format *fmt_pix_mp, > + struct v4l2_format *fmt_pix) { > + struct v4l2_pix_format *pix = &fmt_pix->fmt.pix; > + struct v4l2_pix_format_mplane *pix_mp = &fmt_pix_mp->fmt.pix_mp; > + > + fmt_pix_mp->type = fmt_pix->type; > + pix_mp->width = pix->width; > + pix_mp->height = pix->height; > + pix_mp->pixelformat = pix->pixelformat; > + pix_mp->field = pix->field; > + pix_mp->colorspace = pix->colorspace; > + pix_mp->plane_fmt[0].bytesperline = pix->bytesperline; > + pix_mp->plane_fmt[0].sizeimage = pix->sizeimage; > + pix_mp->num_planes = 1; > +} > + > +static int s5p_jpeg_g_fmt_mplane(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + struct v4l2_format tmp; > + int ret; > + > + memset(&tmp, 0, sizeof(tmp)); > + v4l2_format_pixmp_to_pix(f, &tmp); > + ret = s5p_jpeg_g_fmt(file, priv, &tmp); > + v4l2_format_pixmp_from_pix(f, &tmp); > + > + return ret; > +} > + > +static int s5p_jpeg_try_fmt_vid_cap_mplane(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + struct v4l2_format tmp; > + int ret; > + > + memset(&tmp, 0, sizeof(tmp)); > + v4l2_format_pixmp_to_pix(f, &tmp); > + ret = s5p_jpeg_try_fmt_vid_cap(file, priv, &tmp); > + v4l2_format_pixmp_from_pix(f, &tmp); > + > + return ret; > +} > + > +static int s5p_jpeg_try_fmt_vid_out_mplane(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + struct v4l2_format tmp; > + int ret; > + > + memset(&tmp, 0, sizeof(tmp)); > + v4l2_format_pixmp_to_pix(f, &tmp); > + ret = s5p_jpeg_try_fmt_vid_out(file, priv, &tmp); > + v4l2_format_pixmp_from_pix(f, &tmp); > + > + return ret; > +} > + > +static int s5p_jpeg_s_fmt_vid_cap_mplane(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + struct v4l2_format tmp; > + int ret; > + > + memset(&tmp, 0, sizeof(tmp)); > + v4l2_format_pixmp_to_pix(f, &tmp); > + ret = s5p_jpeg_s_fmt_vid_cap(file, priv, &tmp); > + v4l2_format_pixmp_from_pix(f, &tmp); > + > + return ret; > +} > + > +static int s5p_jpeg_s_fmt_vid_out_mplane(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + struct v4l2_format tmp; > + int ret; > + > + memset(&tmp, 0, sizeof(tmp)); > + v4l2_format_pixmp_to_pix(f, &tmp); > + ret = s5p_jpeg_s_fmt_vid_out(file, priv, &tmp); > + v4l2_format_pixmp_from_pix(f, &tmp); > + > + return ret; > +} > + > static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = { > .vidioc_querycap = s5p_jpeg_querycap, > > @@ -2133,6 +2244,18 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = { > .vidioc_s_fmt_vid_cap = s5p_jpeg_s_fmt_vid_cap, > .vidioc_s_fmt_vid_out = s5p_jpeg_s_fmt_vid_out, > > + .vidioc_enum_fmt_vid_cap_mplane = s5p_jpeg_enum_fmt_vid_cap, > + .vidioc_enum_fmt_vid_out_mplane = s5p_jpeg_enum_fmt_vid_out, > + > + .vidioc_g_fmt_vid_cap_mplane = s5p_jpeg_g_fmt_mplane, > + .vidioc_g_fmt_vid_out_mplane = s5p_jpeg_g_fmt_mplane, > + > + .vidioc_try_fmt_vid_cap_mplane = s5p_jpeg_try_fmt_vid_cap_mplane, > + .vidioc_try_fmt_vid_out_mplane = s5p_jpeg_try_fmt_vid_out_mplane, > + > + .vidioc_s_fmt_vid_cap_mplane = s5p_jpeg_s_fmt_vid_cap_mplane, > + .vidioc_s_fmt_vid_out_mplane = s5p_jpeg_s_fmt_vid_out_mplane, > + > .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, > .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, > .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, > @@ -2648,7 +2771,7 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb) > struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > > if (ctx->mode == S5P_JPEG_DECODE && > - vb->vb2_queue->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { > + vb->vb2_queue->type == ctx->out_q.type) { > static const struct v4l2_event ev_src_ch = { > .type = V4L2_EVENT_SOURCE_CHANGE, > .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION, > @@ -2657,8 +2780,7 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb) > u32 ori_w; > u32 ori_h; > > - dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, > - V4L2_BUF_TYPE_VIDEO_CAPTURE); > + dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, ctx->cap_q.type); > ori_w = ctx->out_q.w; > ori_h = ctx->out_q.h; > > @@ -2708,7 +2830,7 @@ static void s5p_jpeg_stop_streaming(struct vb2_queue *q) > * subsampling. Update capture queue when the stream is off. > */ > if (ctx->state == JPEGCTX_RESOLUTION_CHANGE && > - q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { > + !V4L2_TYPE_IS_OUTPUT(q->type)) { > s5p_jpeg_set_capture_queue_data(ctx); > ctx->state = JPEGCTX_RUNNING; > } > diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.h b/drivers/media/platform/s5p-jpeg/jpeg-core.h > index 9aa26bd..302a297 100644 > --- a/drivers/media/platform/s5p-jpeg/jpeg-core.h > +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.h > @@ -196,6 +196,7 @@ struct s5p_jpeg_marker { > * @sof_len: SOF0 marker's payload length (without length field itself) > * @components: number of image components > * @size: image buffer size in bytes > + * @type: buffer type of the queue (enum v4l2_buf_type) > */ > struct s5p_jpeg_q_data { > struct s5p_jpeg_fmt *fmt; > @@ -208,6 +209,7 @@ struct s5p_jpeg_q_data { > u32 sof_len; > u32 components; > u32 size; > + u32 type; > }; > > /** > -- Best regards, Jacek Anaszewski