From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752782AbdFPPjF (ORCPT ); Fri, 16 Jun 2017 11:39:05 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:48742 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752451AbdFPPjC (ORCPT ); Fri, 16 Jun 2017 11:39:02 -0400 X-AuditID: cbfec7f2-f797e6d000004438-c1-5943fb93dd94 Subject: Re: [PATCH v2 5/6] [media] s5p-jpeg: Add support for resolution change event To: Thierry Escande , Jacek Anaszewski , Mauro Carvalho Chehab Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org From: Andrzej Pietrasiewicz Message-id: Date: Fri, 16 Jun 2017 17:38:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-version: 1.0 In-reply-to: <1497287605-20074-6-git-send-email-thierry.escande@collabora.com> Content-type: text/plain; charset=utf-8; format=flowed Content-language: en-US Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprDKsWRmVeSWpSXmKPExsWy7djPc7qTfztHGtzu0bW4vXUDi8XlXXPY LHo2bGW1WLbpD5NF/9VLbA6sHjvuLmH02DnrLrvHplWdbB6fN8kFsERx2aSk5mSWpRbp2yVw ZUxZ2cxccNyoYuPmt4wNjD/Uuxg5OSQETCR2PNvMCmGLSVy4t56ti5GLQ0hgKaPEmfkLWSCc z4wS/3ZvYIfpmNN1hxUisYxRYvf1+0wgCSGBF4wSbxclgdjCAuESS5u2MYMUiQhMYJQ4sG0n WDezgK1Ex6FOFhCbTcBYYu/BDkYQm1fATmLDtH6wQSwCqhKXv+1lA7FFBSIkFk2ayA5RIyjx Y/I9sF5OAT+J2Tf6WSBmWkk8+9fKCmGLSzS33oSKy0tsXvMW7AgJgXnsEh8aHgIN4gByZCU2 HWCG+MZFomt9NyOELSzx6vgWqC9lJDo7DjJB9E5mlNhw8wMLhLOdUWLtsRPQELOWOHz8ItRm PolJ26YzQyzglehoE4Io8ZD4euEZ1AJHiWvdG6EB/JRRYse930wTGBVmIXluFpKHZiF5aBaS hxYwsqxiFEktLc5NTy021itOzC0uzUvXS87P3cQITDOn/x3/tIPx6wmrQ4wCHIxKPLwMt50j hVgTy4orcw8xSnAwK4nwKr8HCvGmJFZWpRblxxeV5qQWH2KU5mBREuflOnUtQkggPbEkNTs1 tSC1CCbLxMEp1cAYueP6HU4LXrMzxzdr1m/xuNTZl8jg4LhmkViF6ewqU03ZYK3AH9q2Didf z2j3S27OVTv3KdiwQEvg2Z/FGVVHfy01v+EWzfi45mqm/6Piyi7vBGUbz8DOtH0qwftXsuyS a9xZ4X1TUSHkS/l+y0WNOre2vi6w8v2V0l7sZaK9mbtTZ5Y/mxJLcUaioRZzUXEiAA73tTMv AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrDIsWRmVeSWpSXmKPExsVy+t/xy7qTfztHGkxew29xe+sGFovLu+aw WfRs2MpqsWzTHyaL/quX2BxYPXbcXcLosXPWXXaPTas62Tw+b5ILYIlys8lITUxJLVJIzUvO T8nMS7dVCg1x07VQUshLzE21VYrQ9Q0JUlIoS8wpBfKMDNCAg3OAe7CSvl2CW8aUlc3MBceN KjZufsvYwPhDvYuRk0NCwERiTtcdVghbTOLCvfVsXYxcHEICSxgllj79xwThvGCUeDL5OCNI lbBAuMT1l51gCRGBSYwSK7f+ZQJJMAvYSnQc6mSB6HjKKHHm1VywuWwCxhJ7D3aAdfMK2Els mNYP1sAioCpx+dteNhBbVCBCYtf1A6wQNYISPybfYwGxOQX8JGbf6GeBWGAm8eXlYVYIW1yi ufUmVFxeYvOat8wTGAVnIWmfhaRlFpKWWUhaFjCyrGIUSS0tzk3PLTbUK07MLS7NS9dLzs/d xAiMuW3Hfm7ewXhpY/AhRgEORiUeXobbzpFCrIllxZW5hxglOJiVRHiV3wOFeFMSK6tSi/Lj i0pzUosPMZoCPTeRWUo0OR+YDvJK4g1NDM0tDY2MLSzMjYyUxHlLPlwJFxJITyxJzU5NLUgt gulj4uCUamDMNZZv/9Ese/FasMrMl9z1kxu+PVIOX3ZD3N/0b3ZpcFQgn5y0qKtm2jyX2BPv H+8JfMlpda2k+a3flycc7k07L6xe7OwkFzqlU/Tym5dS/7Rl7U8G3N62IPPByzuHd8lpM/z5 eznz/+xXapIvVn/6u8hmesrVjeGx3ZcLCh4uqI83jn+ewduoxFKckWioxVxUnAgAM2s1os8C AAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170616153859eucas1p2d66c344c2384ea1b827d2a1feb8870c8 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?QW5kcnplaiBQaWV0cmFzaWV3aWN6G1NSUE9MLUtlcm5lbCAo?= =?UTF-8?B?VFApG+yCvOyEseyghOyekBtTZW5pb3IgU29mdHdhcmUgRW5naW5lZXI=?= X-Global-Sender: =?UTF-8?B?QW5kcnplaiBUb21hc3ogUGlldHJhc2lld2ljehtTUlBPTC1L?= =?UTF-8?B?ZXJuZWwgKFRQKRtTYW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2Fy?= =?UTF-8?B?ZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170612171431epcas5p19a448035865da056440a819f17875601 X-RootMTR: 20170612171431epcas5p19a448035865da056440a819f17875601 References: <1497287605-20074-1-git-send-email-thierry.escande@collabora.com> <1497287605-20074-6-git-send-email-thierry.escande@collabora.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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 > Signed-off-by: Thierry Escande > --- > 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 > #include > #include > +#include > #include > #include > #include > @@ -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