Hi Thierry,

Thank you for the patch.

Can you give a use case for resolution change event?

Also plase see inline.

W dniu 12.06.2017 o 19:13, Thierry Escande pisze:
From: henryhsu <henry...@chromium.org>

This patch adds support for resolution change event to notify clients so
they can prepare correct output buffer. When resolution change happened,
G_FMT for CAPTURE should return old resolution and format before CAPTURE
queues streamoff.

Signed-off-by: Henry-Ruey Hsu <henry...@chromium.org>
Signed-off-by: Thierry Escande <thierry.esca...@collabora.com>
---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 125 +++++++++++++++++++---------
  drivers/media/platform/s5p-jpeg/jpeg-core.h |   7 ++
  2 files changed, 91 insertions(+), 41 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 7ef7173..3d90a63 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -24,6 +24,7 @@
  #include <linux/slab.h>
  #include <linux/spinlock.h>
  #include <linux/string.h>
+#include <media/v4l2-event.h>
  #include <media/v4l2-mem2mem.h>
  #include <media/v4l2-ioctl.h>
  #include <media/videobuf2-v4l2.h>
@@ -1611,8 +1612,6 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct 
v4l2_format *f)
                        FMT_TYPE_OUTPUT : FMT_TYPE_CAPTURE;
q_data->fmt = s5p_jpeg_find_format(ct, pix->pixelformat, f_type);
-       q_data->w = pix->width;
-       q_data->h = pix->height;
        if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG) {
                /*
                 * During encoding Exynos4x12 SoCs access wider memory area
@@ -1620,6 +1619,8 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct 
v4l2_format *f)
                 * the JPEG_IMAGE_SIZE register. In order to avoid sysmmu
                 * page fault calculate proper buffer size in such a case.
                 */
+               q_data->w = pix->width;
+               q_data->h = pix->height;

Is this change related to what the patch is supposed to be doing?

                if (ct->jpeg->variant->hw_ex4_compat &&
                    f_type == FMT_TYPE_OUTPUT && ct->mode == S5P_JPEG_ENCODE)
                        q_data->size = exynos4_jpeg_get_output_buffer_size(ct,
@@ -1695,6 +1696,15 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, 
void *priv,
        return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
  }
+static int s5p_jpeg_subscribe_event(struct v4l2_fh *fh,
+                                   const struct v4l2_event_subscription *sub)
+{
+       if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
+               return v4l2_src_change_event_subscribe(fh, sub);
+
+       return -EINVAL;
+}
+
  static int exynos3250_jpeg_try_downscale(struct s5p_jpeg_ctx *ctx,
                                   struct v4l2_rect *r)
  {
@@ -2020,6 +2030,9 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = {
.vidioc_g_selection = s5p_jpeg_g_selection,
        .vidioc_s_selection             = s5p_jpeg_s_selection,
+
+       .vidioc_subscribe_event         = s5p_jpeg_subscribe_event,
+       .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,
  };
/*
@@ -2412,8 +2425,17 @@ static int s5p_jpeg_job_ready(void *priv)
  {
        struct s5p_jpeg_ctx *ctx = priv;
- if (ctx->mode == S5P_JPEG_DECODE)
+       if (ctx->mode == S5P_JPEG_DECODE) {
+               /*
+                * We have only one input buffer and one output buffer. If there
+                * is a resolution change event, no need to continue decoding.
+                */
+               if (ctx->state == JPEGCTX_RESOLUTION_CHANGE)
+                       return 0;
+
                return ctx->hdr_parsed;
+       }
+
        return 1;
  }
@@ -2492,6 +2514,30 @@ static int s5p_jpeg_buf_prepare(struct vb2_buffer *vb)
        return 0;
  }
+static void s5p_jpeg_set_capture_queue_data(struct s5p_jpeg_ctx *ctx)
+{
+       struct s5p_jpeg_q_data *q_data = &ctx->cap_q;
+
+       q_data->w = ctx->out_q.w;
+       q_data->h = ctx->out_q.h;
+
+       /*
+        * This call to jpeg_bound_align_image() takes care of width and
+        * height values alignment when user space calls the QBUF of
+        * OUTPUT buffer after the S_FMT of CAPTURE buffer.
+        * Please note that on Exynos4x12 SoCs, resigning from executing
+        * S_FMT on capture buffer for each JPEG image can result in a
+        * hardware hangup if subsampling is lower than the one of input
+        * JPEG.
+        */
+       jpeg_bound_align_image(ctx, &q_data->w, S5P_JPEG_MIN_WIDTH,
+                              S5P_JPEG_MAX_WIDTH, q_data->fmt->h_align,
+                              &q_data->h, S5P_JPEG_MIN_HEIGHT,
+                              S5P_JPEG_MAX_HEIGHT, q_data->fmt->v_align);
+
+       q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
+}
+
  static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
  {
        struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -2499,9 +2545,20 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
if (ctx->mode == S5P_JPEG_DECODE &&
            vb->vb2_queue->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
-               struct s5p_jpeg_q_data tmp, *q_data;
-
-               ctx->hdr_parsed = s5p_jpeg_parse_hdr(&tmp,
+               static const struct v4l2_event ev_src_ch = {
+                       .type = V4L2_EVENT_SOURCE_CHANGE,
+                       .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
+               };
+               struct vb2_queue *dst_vq;
+               u32 ori_w;
+               u32 ori_h;
+
+               dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+                                        V4L2_BUF_TYPE_VIDEO_CAPTURE);
+               ori_w = ctx->out_q.w;
+               ori_h = ctx->out_q.h;
+
+               ctx->hdr_parsed = s5p_jpeg_parse_hdr(&ctx->out_q,
                     (unsigned long)vb2_plane_vaddr(vb, 0),
                     min((unsigned long)ctx->out_q.size,
                         vb2_get_plane_payload(vb, 0)), ctx);
@@ -2510,43 +2567,18 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
                        return;
                }
- q_data = &ctx->out_q;
-               q_data->w = tmp.w;
-               q_data->h = tmp.h;
-               q_data->sos = tmp.sos;
-               memcpy(q_data->dht.marker, tmp.dht.marker,
-                      sizeof(tmp.dht.marker));
-               memcpy(q_data->dht.len, tmp.dht.len, sizeof(tmp.dht.len));
-               q_data->dht.n = tmp.dht.n;
-               memcpy(q_data->dqt.marker, tmp.dqt.marker,
-                      sizeof(tmp.dqt.marker));
-               memcpy(q_data->dqt.len, tmp.dqt.len, sizeof(tmp.dqt.len));
-               q_data->dqt.n = tmp.dqt.n;
-               q_data->sof = tmp.sof;
-               q_data->sof_len = tmp.sof_len;
-
-               q_data = &ctx->cap_q;
-               q_data->w = tmp.w;
-               q_data->h = tmp.h;


Why is this part removed?

Andrzej

Reply via email to