RE: [RFC/PATCH v7 1/5] Changes in include/linux/videodev2.h for MFC 5.1
Hi, I commented as below.
-Original Message-
From: Kamil Debski [mailto:k.deb...@samsung.com]
Sent: Saturday, March 05, 2011 3:57 AM
To: 'Laurent Pinchart'
Cc: linux-me...@vger.kernel.org; linux-samsung-soc@vger.kernel.org; Marek
Szyprowski; kyungmin.p...@samsung.com; jaeryul...@samsung.com;
kgene@samsung.com
Subject: RE: [RFC/PATCH v7 1/5] Changes in include/linux/videodev2.h for
MFC 5.1
Hi,
-Original Message-
From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
Sent: 04 March 2011 17:39
To: Kamil Debski
Cc: linux-me...@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
m.szyprow...@samsung.com; kyungmin.p...@samsung.com;
jaeryul...@samsung.com; kgene@samsung.com
Subject: Re: [RFC/PATCH v7 1/5] Changes in include/linux/videodev2.h
for MFC 5.1
On Friday 04 March 2011 12:26:18 Kamil Debski wrote:
This patch adds fourcc values for compressed video stream formats and
V4L2_CTRL_CLASS_CODEC. Also adds controls used by MFC 5.1 driver.
Signed-off-by: Kamil Debski k.deb...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
include/linux/videodev2.h | 39
+++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index a94c4d5..a48a42e 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -369,6 +369,19 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_DV v4l2_fourcc('d', 'v', 's', 'd') /*
1394 */
#define V4L2_PIX_FMT_MPEG v4l2_fourcc('M', 'P', 'E', 'G') /*
MPEG-1/2/4 */
+#define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /*
H264 */
+#define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /*
H263 */
+#define V4L2_PIX_FMT_MPEG12 v4l2_fourcc('M', 'P', '1', '2') /*
MPEG-1/2 */
+#define V4L2_PIX_FMT_MPEG4v4l2_fourcc('M', 'P', 'G', '4') /*
MPEG-4 */
+#define V4L2_PIX_FMT_DIVX v4l2_fourcc('D', 'I', 'V', 'X') /*
DivX */
+#define V4L2_PIX_FMT_DIVX3v4l2_fourcc('D', 'I', 'V', '3') /*
DivX 3.11 */
+#define V4L2_PIX_FMT_DIVX4v4l2_fourcc('D', 'I', 'V', '4') /*
DivX 4.12 */
+#define V4L2_PIX_FMT_DIVX500 v4l2_fourcc('D', 'X', '5', '2') /*
DivX 5.00 - 5.02 */
+#define V4L2_PIX_FMT_DIVX503 v4l2_fourcc('D', 'X', '5', '3') /*
DivX 5.03 - x */
+#define V4L2_PIX_FMT_XVID v4l2_fourcc('X', 'V', 'I', 'D') /*
Xvid */
+#define V4L2_PIX_FMT_VC1 v4l2_fourcc('V', 'C', '1', 'A') /* VC-
1 */
+#define V4L2_PIX_FMT_VC1_RCV v4l2_fourcc('V', 'C', '1', 'R') /* VC-
1 RCV */
+
Hans, you mentioned some time ago that you were against ading H.264 or
MPEG4
fourccs, and that drivers should use the MPEG controls instead. Could
you
clarify your current position on this ?
If I remember correct there was no clear conclusion on this. I hope we can
discuss this
during the upcoming meeting.
Have you got an alternative suggestion to using fourccs?
The existing MPEG controls won't cover all the functions and parameters
that
are
used by video codecs. The controls that are in this patch are the ones
related to
decoding, there is even more for encoding.
Yesterday I have been talking with Hans on the IRC channel about the
control
for
quantization parameters and he has suggested to use different for MPEG4,
H263 and H264.
Personally I'd like to have a common one, as the QP meaning is the same in
those 3
cases, the difference is the range of the value.
So I think that this still is a subject that could use more discussion as
there are
a few ideas.
According to the fource.org, and if fourcc definition here has been made
from fourcc.org,
We need to have a discussion about new types that Kamil added not only h264
but divx_xxx.
QP might be a same topic. So let's discuss more at Poland meeting.
Best regards,
--
Kamil Debski
Linux Platform Group
Samsung Poland RD Center
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Kamil, see my comments.
-Original Message-
From: Kamil Debski [mailto:k.deb...@samsung.com]
Sent: Friday, February 18, 2011 8:23 PM
To: jaeryul...@samsung.com; linux-me...@vger.kernel.org; linux-samsung-
s...@vger.kernel.org
Cc: m.szyprow...@samsung.com; pa...@osciak.com; kyungmin.p...@samsung.com;
kgene@samsung.com
Subject: RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
From: Jaeryul Oh [mailto:jaeryul...@samsung.com]
Kamil, I have a question about MFC state FINISHING FINISHED as below.
Hi Peter,
I have answered your question below.
-Original Message-
From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Kamil Debski
Sent: Saturday, January 08, 2011 1:26 AM
To: linux-me...@vger.kernel.org; linux-samsung-soc@vger.kernel.org
Cc: m.szyprow...@samsung.com; pa...@osciak.com;
kyungmin.p...@samsung.com;
k.deb...@samsung.com; jaeryul...@samsung.com; kgene@samsung.com
Subject: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Multi Format Codec 5.1 is capable of handling a range of video codecs
and this driver provides V4L2 interface for video decoding.
Signed-off-by: Kamil Debski k.deb...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/Kconfig |8 +
drivers/media/video/Makefile |1 +
drivers/media/video/s5p-mfc/Makefile |3 +
drivers/media/video/s5p-mfc/regs-mfc5.h | 335 +
drivers/media/video/s5p-mfc/s5p_mfc.c| 2072
++
drivers/media/video/s5p-mfc/s5p_mfc_common.h | 224 +++
drivers/media/video/s5p-mfc/s5p_mfc_ctrls.h | 173 +++
drivers/media/video/s5p-mfc/s5p_mfc_debug.h | 47 +
drivers/media/video/s5p-mfc/s5p_mfc_intr.c | 92 ++
drivers/media/video/s5p-mfc/s5p_mfc_intr.h | 26 +
drivers/media/video/s5p-mfc/s5p_mfc_memory.h | 43 +
drivers/media/video/s5p-mfc/s5p_mfc_opr.c| 885 +++
drivers/media/video/s5p-mfc/s5p_mfc_opr.h| 160 ++
13 files changed, 4069 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/s5p-mfc/Makefile
create mode 100644 drivers/media/video/s5p-mfc/regs-mfc5.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc.c
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_common.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_ctrls.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_debug.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_intr.c
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_intr.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_memory.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr.c
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr.h
Snipped...
+/* Videobuf opts */
+static struct vb2_ops s5p_mfc_qops = {
+ .buf_queue = s5p_mfc_buf_queue,
+ .queue_setup = s5p_mfc_queue_setup,
+ .start_streaming = s5p_mfc_start_streaming,
+ .buf_init = s5p_mfc_buf_init,
+ .stop_streaming = s5p_mfc_stop_streaming,
+ .wait_prepare = s5p_mfc_unlock,
+ .wait_finish = s5p_mfc_lock,
+};
+
+static void s5p_mfc_handle_frame_all_extracted(struct s5p_mfc_ctx
*ctx)
+{
+ struct s5p_mfc_dev *dev = ctx-dev;
+ struct s5p_mfc_buf *dst_buf;
+
+ ctx-state = MFCINST_DEC_FINISHED;
+ mfc_debug(Decided to finish\n);
+ ctx-sequence++;
+ while (!list_empty(ctx-dst_queue)) {
+ dst_buf = list_entry(ctx-dst_queue.next,
+ struct s5p_mfc_buf, list);
+ mfc_debug(Cleaning up buffer: %d\n,
+ dst_buf-b-v4l2_buf.index);
+ vb2_set_plane_payload(dst_buf-b, 0, 0);
+ vb2_set_plane_payload(dst_buf-b, 1, 0);
+ list_del(dst_buf-list);
+ ctx-dst_queue_cnt--;
+ dst_buf-b-v4l2_buf.sequence = (ctx-sequence++);
+ if (s5p_mfc_get_pic_time_top(ctx) ==
+ s5p_mfc_get_pic_time_bottom(ctx))
+ dst_buf-b-v4l2_buf.field = V4L2_FIELD_NONE;
+ else
+ dst_buf-b-v4l2_buf.field =
+ V4L2_FIELD_INTERLACED;
+ ctx-dec_dst_flag = ~(1 dst_buf-b-v4l2_buf.index);
+ vb2_buffer_done(dst_buf-b, VB2_BUF_STATE_DONE);
+ mfc_debug(Cleaned up buffer: %d\n,
+ dst_buf-b-v4l2_buf.index);
+ }
+ mfc_debug(After cleanup\n);
+}
+
+static void s5p_mfc_handle_frame_new(struct s5p_mfc_ctx *ctx,
unsigned
int err)
+{
+ struct s5p_mfc_dev *dev = ctx-dev;
+ struct s5p_mfc_buf *dst_buf;
+ size_t dspl_y_addr = s5p_mfc_get_dspl_y_adr();
+
+ ctx-sequence++;
+ /* If frame is same as previous then skip and do not dequeue */
+ if (s5p_mfc_get_frame_type() == S5P_FIMV_DECODE_FRAME_SKIPPED
RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Kamil, I have a question about MFC state FINISHING FINISHED as below.
-Original Message-
From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Kamil Debski
Sent: Saturday, January 08, 2011 1:26 AM
To: linux-me...@vger.kernel.org; linux-samsung-soc@vger.kernel.org
Cc: m.szyprow...@samsung.com; pa...@osciak.com; kyungmin.p...@samsung.com;
k.deb...@samsung.com; jaeryul...@samsung.com; kgene@samsung.com
Subject: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Multi Format Codec 5.1 is capable of handling a range of video codecs
and this driver provides V4L2 interface for video decoding.
Signed-off-by: Kamil Debski k.deb...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/Kconfig |8 +
drivers/media/video/Makefile |1 +
drivers/media/video/s5p-mfc/Makefile |3 +
drivers/media/video/s5p-mfc/regs-mfc5.h | 335 +
drivers/media/video/s5p-mfc/s5p_mfc.c| 2072
++
drivers/media/video/s5p-mfc/s5p_mfc_common.h | 224 +++
drivers/media/video/s5p-mfc/s5p_mfc_ctrls.h | 173 +++
drivers/media/video/s5p-mfc/s5p_mfc_debug.h | 47 +
drivers/media/video/s5p-mfc/s5p_mfc_intr.c | 92 ++
drivers/media/video/s5p-mfc/s5p_mfc_intr.h | 26 +
drivers/media/video/s5p-mfc/s5p_mfc_memory.h | 43 +
drivers/media/video/s5p-mfc/s5p_mfc_opr.c| 885 +++
drivers/media/video/s5p-mfc/s5p_mfc_opr.h| 160 ++
13 files changed, 4069 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/s5p-mfc/Makefile
create mode 100644 drivers/media/video/s5p-mfc/regs-mfc5.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc.c
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_common.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_ctrls.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_debug.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_intr.c
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_intr.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_memory.h
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr.c
create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr.h
Snipped...
+/* Videobuf opts */
+static struct vb2_ops s5p_mfc_qops = {
+ .buf_queue = s5p_mfc_buf_queue,
+ .queue_setup = s5p_mfc_queue_setup,
+ .start_streaming = s5p_mfc_start_streaming,
+ .buf_init = s5p_mfc_buf_init,
+ .stop_streaming = s5p_mfc_stop_streaming,
+ .wait_prepare = s5p_mfc_unlock,
+ .wait_finish = s5p_mfc_lock,
+};
+
+static void s5p_mfc_handle_frame_all_extracted(struct s5p_mfc_ctx *ctx)
+{
+ struct s5p_mfc_dev *dev = ctx-dev;
+ struct s5p_mfc_buf *dst_buf;
+
+ ctx-state = MFCINST_DEC_FINISHED;
+ mfc_debug(Decided to finish\n);
+ ctx-sequence++;
+ while (!list_empty(ctx-dst_queue)) {
+ dst_buf = list_entry(ctx-dst_queue.next,
+ struct s5p_mfc_buf, list);
+ mfc_debug(Cleaning up buffer: %d\n,
+ dst_buf-b-v4l2_buf.index);
+ vb2_set_plane_payload(dst_buf-b, 0, 0);
+ vb2_set_plane_payload(dst_buf-b, 1, 0);
+ list_del(dst_buf-list);
+ ctx-dst_queue_cnt--;
+ dst_buf-b-v4l2_buf.sequence = (ctx-sequence++);
+ if (s5p_mfc_get_pic_time_top(ctx) ==
+ s5p_mfc_get_pic_time_bottom(ctx))
+ dst_buf-b-v4l2_buf.field = V4L2_FIELD_NONE;
+ else
+ dst_buf-b-v4l2_buf.field =
+ V4L2_FIELD_INTERLACED;
+ ctx-dec_dst_flag = ~(1 dst_buf-b-v4l2_buf.index);
+ vb2_buffer_done(dst_buf-b, VB2_BUF_STATE_DONE);
+ mfc_debug(Cleaned up buffer: %d\n,
+ dst_buf-b-v4l2_buf.index);
+ }
+ mfc_debug(After cleanup\n);
+}
+
+static void s5p_mfc_handle_frame_new(struct s5p_mfc_ctx *ctx, unsigned
int err)
+{
+ struct s5p_mfc_dev *dev = ctx-dev;
+ struct s5p_mfc_buf *dst_buf;
+ size_t dspl_y_addr = s5p_mfc_get_dspl_y_adr();
+
+ ctx-sequence++;
+ /* If frame is same as previous then skip and do not dequeue */
+ if (s5p_mfc_get_frame_type() == S5P_FIMV_DECODE_FRAME_SKIPPED)
+ return;
+ /* The MFC returns address of the buffer, now we have to
+ * check which videobuf does it correspond to */
+ list_for_each_entry(dst_buf, ctx-dst_queue, list) {
+ mfc_debug(Listing: %d\n, dst_buf-b-v4l2_buf.index);
+ /* Check if this is the buffer we're looking for */
+ if (vb2_cma_plane_paddr(dst_buf-b, 0) == dspl_y_addr) {
+ list_del(dst_buf-list);
+ ctx-dst_queue_cnt--;
+ dst_buf-b-v4l2_buf.sequence =
RE: [RFC/PATCH v6 1/4] Changes in include/linux/videodev2.h for MFC 5.1
for MPEG4 value. In addition - for MPEG1 and 2 MFC
accepts
elementary stream (ES) and I don't see it in the enum too.
It will accept only elementary stream. As to DivX one should select the
version and the hardware will support the features defined by the
standard.
I think Jaeryul Oh could provide more information about DivX support.
In H264 you can have different profiles supported by hardware,
still I imagine that the drivers would use V4L2_PIX_FMT_H264.
Basically, it was defined based on fourcc.org. but in the current fourcc.org
DIV3, DIV4, DIV5, DX50 have been described. But we need one more thing to
differentiate
DX53(DIV 5.03 ~) from DX50. which is required for MFC HW. Practically, to
decode higher
version of DivX5.03, we should set DX53 format. But
DX53(V4L2_PIX_FMT_DIVX503) is
NOT yet included in [RFC/PATCH v6 1/4] Changes in include/linux/videodev2.h
for MFC 5.1
Does the codec just go from compressed video to raw video? If it also
goes in
the other direction, how does one set bitrates, etc.?
This is the decoder only version of the driver. The hardware supports
also encoding and we are working at an updated driver. There will be
a set of controls for adjusting the encoding parameters.
Does it accept multiplexed streams containing audio as well? If so,
what does
it do with the audio?
Only video elementary streams are supported.
/* Vendor-specific formats */
#define V4L2_PIX_FMT_CPIA1v4l2_fourcc('C', 'P', 'I', 'A') /*
cpia1 YUV */
#define V4L2_PIX_FMT_WNVA v4l2_fourcc('W', 'N', 'V', 'A') /*
Winnov hw compress */
@@ -972,6 +992,7 @@ struct v4l2_output {
#define V4L2_OUTPUT_TYPE_ANALOG 2
#define V4L2_OUTPUT_TYPE_ANALOGVGAOVERLAY3
+
/* capabilities flags */
#define V4L2_OUT_CAP_PRESETS 0x0001 /* Supports
S_DV_PRESET */
#define V4L2_OUT_CAP_CUSTOM_TIMINGS 0x0002 /* Supports
S_DV_TIMINGS */
@@ -1009,6 +1030,7 @@ struct v4l2_ext_controls {
#define V4L2_CTRL_CLASS_MPEG 0x0099 /* MPEG-compression
controls */
#define V4L2_CTRL_CLASS_CAMERA 0x009a/* Camera class
controls */
#define V4L2_CTRL_CLASS_FM_TX 0x009b /* FM Modulator control
class */
+#define V4L2_CTRL_CLASS_CODEC 0x009c /* Codec control class
*/
#define V4L2_CTRL_ID_MASK (0x0fff)
#define V4L2_CTRL_ID2CLASS(id)((id) 0x0fffUL)
@@ -1342,6 +1364,29 @@ enum
v4l2_mpeg_cx2341x_video_median_filter_type {
#define V4L2_CID_MPEG_CX2341X_VIDEO_CHROMA_MEDIAN_FILTER_TOP
(V4L2_CID_MPEG_CX2341X_BASE+10)
#define V4L2_CID_MPEG_CX2341X_STREAM_INSERT_NAV_PACKETS
(V4L2_CID_MPEG_CX2341X_BASE+11)
+/* For codecs */
+
+#define V4L2_CID_CODEC_BASE
(V4L2_CTRL_CLASS_CODEC
| 0x900)
+#define V4L2_CID_CODEC_CLASS
(V4L2_CTRL_CLASS_CODEC
| 1)
+
+/* For both decoding and encoding */
+
+/* For encoding */
+#define V4L2_CID_CODEC_LOOP_FILTER_H264
(V4L2_CID_CODEC_BASE + 0)
+enum v4l2_cid_codec_loop_filter_h264 {
+ V4L2_CID_CODEC_LOOP_FILTER_H264_ENABLE = 0,
+ V4L2_CID_CODEC_LOOP_FILTER_H264_DISABLE = 1,
+ V4L2_CID_CODEC_LOOP_FILTER_H264_DISABLE_AT_BOUNDARY = 2,
+};
+
+/* For decoding */
+
+#define V4L2_CID_CODEC_LOOP_FILTER_MPEG4_ENABLE
(V4L2_CID_CODEC_BASE + 1)
+#define V4L2_CID_CODEC_DISPLAY_DELAY (V4L2_CID_CODEC_BASE
+
2)
+#define V4L2_CID_CODEC_REQ_NUM_BUFS (V4L2_CID_CODEC_BASE
+
3)
+#define V4L2_CID_CODEC_SLICE_INTERFACE (V4L2_CID_CODEC_BASE
+
4)
+#define V4L2_CID_CODEC_PACKED_PB (V4L2_CID_CODEC_BASE + 5)
+
This needs to be documented in the spec as well.
Ok.
It seems to me just looking at the names that these controls are highly
hardware specific. If so, then these controls would have to be in the
range
of (V4L2_CTRL_CLASS_CODEC | 0x1000) and up and need the name of the
chipset
as part of their ID. Similar to the cx2341x specific controls (see
V4L2_CID_MPEG_CX2341X_BASE in videodev2.h).
I think that DISPLAY_DELAY would be used by more than one codec. It may
be difficult to judge what is common until more chip vendors decide to
include drivers for their hardware codecs in the video4linux.
Creating a CODEC control class seems sensible to me, so I'm fine with
that.
That's good news.
/* Camera class control IDs */
#define V4L2_CID_CAMERA_CLASS_BASE (V4L2_CTRL_CLASS_CAMERA |
0x900)
#define V4L2_CID_CAMERA_CLASS(V4L2_CTRL_CLASS_CAMERA | 1)
Thank you for your comments.
Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland RD Center
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Commented as belows.
-Original Message-
From: Kamil Debski [mailto:k.deb...@samsung.com]
Sent: Monday, January 17, 2011 1:59 PM
To: jaeryul...@samsung.com; 'Jonghun Han'; linux-me...@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Cc: 'Marek Szyprowski'; pa...@osciak.com; kyungmin.p...@samsung.com;
kgene@samsung.com
Subject: RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Hi,
I don't see the need to do DQBUF on the source buffer that has the
I-frame with changed resolution. I think that one could notify the
application by setting size=0 on the CAPTURE buffer. So the OUTPUT
buffer would not be dequeued. It would be dequeued after it has
been decoded. Do you think anything is wrong with this approach?
I really think that copying the source buffer contents is unnecessary.
I just thought of this based on current driver sequence. Without copy for
I frame(resolution changed frame), it could be possible, but we need smart
driver
control for this situation(res. change).
So, one time queued, three times should be used
: recognition for res. changed / header parsing / I frame decoding
Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland RD Center
-Original Message-
From: Jaeryul Oh [mailto:jaeryul...@samsung.com]
Sent: 14 January 2011 19:21
To: 'Jonghun Han'; 'Kamil Debski'; linux-me...@vger.kernel.org; linux-
samsung-...@vger.kernel.org
Cc: 'Marek Szyprowski'; pa...@osciak.com; kyungmin.p...@samsung.com;
kgene@samsung.com
Subject: RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
I added my comments.
I guess that Kamil's way could be possible to run seq. of dynamic
resolution
change.
-Original Message-
From: Jonghun Han [mailto:jonghun@samsung.com]
Sent: Friday, January 14, 2011 6:45 PM
To: 'Kamil Debski'; linux-me...@vger.kernel.org; linux-samsung-
s...@vger.kernel.org
Cc: 'Marek Szyprowski'; pa...@osciak.com; kyungmin.p...@samsung.com;
jaeryul...@samsung.com; kgene@samsung.com
Subject: RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Hi,
I was confused the source and destination.
I agree with you mostly.
In my opinion, the way how to notify the resolution change is return
value
of the DQBUF.
But if we use DQBUF, there are some problem as below.
DQBUF means that the buffer has been operated already and application
will
have the buffer's right.
Although application new QBUF destination buffers after changing
destination(CAPTURE),
driver cannot re-decode the I-frame which has the resolution change
information
because the I-frame has been dequeued already.
If application re-QBUF the buffer, the buffer sequence will be out of
order
as below.
Original: I - B - B .
Out of order: missed - B - B - I .
How do you think about the following sequence.
1. getting the resolution change from the MFC H/W
2. copy the buffer to driver's internal memory.
3. send the result with DQBUF
4. changing destination buffers by application
5. QBUF for new destination buffers
6. in the first vb2_try_schedule
re-decode the driver's internal buffer instead of the B frame.
7. in the next vb2_try_schedule
decode the B frame in order.
I also welcome your comments.
In the blocking sequence, blocking will be done in DQBUF.
1. QBUF done for I1 frame with hdr(first buf, res. chd)
: Copy I1 frame in the driver
2. DQBUF done for I1 frame
3. QBUF P2,P3,P4 frame, but it should NOT be DEQUED which means
Qbufed p2,p3,p4 is not executed
: P4(third buf) - P3(second buf) - P2 frame(first buf)
4. While waiting for DEQUE about P2, copied I1 frame is executed for
INIT_CODEC
5. Copied I1 frame is runned FRAME_START after dst buffer is
reallocated
(5~9 in your seq.)
6. Queued P2 frame is executed, then DQBUF will be OK
Best regards,
-Original Message-
From: Kamil Debski [mailto:k.deb...@samsung.com]
Sent: Friday, January 14, 2011 4:24 PM
To: 'Jonghun Han'; linux-me...@vger.kernel.org; linux-samsung-
s...@vger.kernel.org
Cc: Marek Szyprowski; pa...@osciak.com; kyungmin.p...@samsung.com;
jaeryul...@samsung.com; kgene@samsung.com
Subject: RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Hi,
-Original Message-
From: Jonghun Han [mailto:jonghun@samsung.com]
Hi,
Kamil Debski wrote:
snip
+/* Reqeust buffers */
+static int vidioc_reqbufs(struct file *file, void *priv,
+ struct v4l2_requestbuffers
*reqbufs)
+{
+ struct s5p_mfc_dev *dev = video_drvdata(file);
+ struct s5p_mfc_ctx *ctx = priv;
+ int ret = 0;
+ unsigned long flags;
+
+ mfc_debug_enter();
+ mfc_debug(Memory type: %d
RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
I added my comments.
I guess that Kamil's way could be possible to run seq. of dynamic resolution
change.
-Original Message-
From: Jonghun Han [mailto:jonghun@samsung.com]
Sent: Friday, January 14, 2011 6:45 PM
To: 'Kamil Debski'; linux-me...@vger.kernel.org; linux-samsung-
s...@vger.kernel.org
Cc: 'Marek Szyprowski'; pa...@osciak.com; kyungmin.p...@samsung.com;
jaeryul...@samsung.com; kgene@samsung.com
Subject: RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Hi,
I was confused the source and destination.
I agree with you mostly.
In my opinion, the way how to notify the resolution change is return value
of the DQBUF.
But if we use DQBUF, there are some problem as below.
DQBUF means that the buffer has been operated already and application will
have the buffer's right.
Although application new QBUF destination buffers after changing
destination(CAPTURE),
driver cannot re-decode the I-frame which has the resolution change
information
because the I-frame has been dequeued already.
If application re-QBUF the buffer, the buffer sequence will be out of
order
as below.
Original: I - B - B .
Out of order: missed - B - B - I .
How do you think about the following sequence.
1. getting the resolution change from the MFC H/W
2. copy the buffer to driver's internal memory.
3. send the result with DQBUF
4. changing destination buffers by application
5. QBUF for new destination buffers
6. in the first vb2_try_schedule
re-decode the driver's internal buffer instead of the B frame.
7. in the next vb2_try_schedule
decode the B frame in order.
I also welcome your comments.
In the blocking sequence, blocking will be done in DQBUF.
1. QBUF done for I1 frame with hdr(first buf, res. chd)
: Copy I1 frame in the driver
2. DQBUF done for I1 frame
3. QBUF P2,P3,P4 frame, but it should NOT be DEQUED which means
Qbufed p2,p3,p4 is not executed
: P4(third buf) - P3(second buf) - P2 frame(first buf)
4. While waiting for DEQUE about P2, copied I1 frame is executed for
INIT_CODEC
5. Copied I1 frame is runned FRAME_START after dst buffer is reallocated
(5~9 in your seq.)
6. Queued P2 frame is executed, then DQBUF will be OK
Best regards,
-Original Message-
From: Kamil Debski [mailto:k.deb...@samsung.com]
Sent: Friday, January 14, 2011 4:24 PM
To: 'Jonghun Han'; linux-me...@vger.kernel.org; linux-samsung-
s...@vger.kernel.org
Cc: Marek Szyprowski; pa...@osciak.com; kyungmin.p...@samsung.com;
jaeryul...@samsung.com; kgene@samsung.com
Subject: RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Hi,
-Original Message-
From: Jonghun Han [mailto:jonghun@samsung.com]
Hi,
Kamil Debski wrote:
snip
+/* Reqeust buffers */
+static int vidioc_reqbufs(struct file *file, void *priv,
+ struct v4l2_requestbuffers
*reqbufs)
+{
+ struct s5p_mfc_dev *dev = video_drvdata(file);
+ struct s5p_mfc_ctx *ctx = priv;
+ int ret = 0;
+ unsigned long flags;
+
+ mfc_debug_enter();
+ mfc_debug(Memory type: %d\n, reqbufs-memory);
+ if (reqbufs-memory != V4L2_MEMORY_MMAP) {
+ mfc_err(Only V4L2_MEMORY_MAP is supported.\n);
+ return -EINVAL;
+ }
+ if (reqbufs-type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+ /* Can only request buffers after an instance has
been
opened.*/
+ if (ctx-state == MFCINST_DEC_GOT_INST) {
+ /* Decoding */
+ if (ctx-output_state != QUEUE_FREE) {
+ mfc_err(Bufs have already been
requested.\n);
+ return -EINVAL;
+ }
+ ret = vb2_reqbufs(ctx-vq_src, reqbufs);
+ if (ret) {
+ mfc_err(vb2_reqbufs on output
failed.\n);
+ return ret;
+ }
+ mfc_debug(vb2_reqbufs: %d\n, ret);
+ ctx-output_state = QUEUE_BUFS_REQUESTED;
+ }
+ } else if (reqbufs-type ==
V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+ if (ctx-capture_state != QUEUE_FREE) {
+ mfc_err(Bufs have already been
requested.\n);
+ return -EINVAL;
+ }
+ ctx-capture_state = QUEUE_BUFS_REQUESTED;
+ ret = vb2_reqbufs(ctx-vq_dst, reqbufs);
+ if (ret) {
+ mfc_err(vb2_reqbufs on capture failed.\n);
+ return ret;
+ }
+ if (reqbufs-count ctx-dpb_count) {
+
RE: [PATCH 1/9] media: Changes in include/linux/videodev2.h for MFC 5.1
Hi, Kamil
k.deb...@samsung.com wrote:
Hi Hans,
-Original Message-
From: Hans Verkuil [mailto:hverk...@xs4all.nl]
Sent: 22 December 2010 13:42
To: Jeongtae Park
Cc: linux-me...@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
k.deb...@samsung.com; jaeryul...@samsung.com; kgene@samsung.com;
ben-li...@fluff.org; jonghun@samsung.com
Subject: Re: [PATCH 1/9] media: Changes in include/linux/videodev2.h
for MFC 5.1
snip
#define V4L2_PIX_FMT_DV v4l2_fourcc('d', 'v', 's', 'd') /*
1394 */
#define V4L2_PIX_FMT_MPEG v4l2_fourcc('M', 'P', 'E', 'G') /*
MPEG-1/2/4*/
+
+#define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /*
H264*/
+#define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /*
H263*/
+#define V4L2_PIX_FMT_MPEG12 v4l2_fourcc('M', 'P', '1', '2') /*
MPEG-1/2 */
+#define V4L2_PIX_FMT_MPEG4v4l2_fourcc('M', 'P', 'G', '4') /*
MPEG-4 */
+#define V4L2_PIX_FMT_DIVX v4l2_fourcc('D', 'I', 'V', 'X') /*
DivX */
+#define V4L2_PIX_FMT_DIVX3v4l2_fourcc('D', 'I', 'V', '3') /*
DivX 3.11 */
+#define V4L2_PIX_FMT_DIVX4v4l2_fourcc('D', 'I', 'V', '4') /*
DivX 4.12 */
+#define V4L2_PIX_FMT_DIVX500v4l2_fourcc('D', 'X', '5', '2') /*
DivX 5.00 - 5.02 */
+#define V4L2_PIX_FMT_DIVX503v4l2_fourcc('D', 'X', '5', '3') /*
DivX 5.03 - x */
+#define V4L2_PIX_FMT_XVID v4l2_fourcc('X', 'V', 'I', 'D') /*
Xvid */
+#define V4L2_PIX_FMT_VC1 v4l2_fourcc('V', 'C', '1', 'A') /* VC-
1 */
+#define V4L2_PIX_FMT_VC1_RCV v4l2_fourcc('V', 'C', '1', 'R') /*
VC-1 RCV */
What do these formats describe? Are these container formats or the
actual
compressed video stream that is normally packaged inside a container?
Apart from VC-1 RCV those are elementary streams. If I understand
correctly
RCV is a simple semi-container that contains necessary information to play
the ES. I have asked a person from HW team if all those fourccs are
necessary.
I am waiting for reply.
The idea was to have a fourcc for each supported codec (by this I mean the
elementary stream).
I'm reviewing what we really should add for MFC except for previously
defined FOURCC type
based on http://www.fourcc.org/fourcc.php
FOURCC there seems that there is a little bit different from codec
supported by MFC(HW codec)
+
+
/* Vendor-specific formats */
#define V4L2_PIX_FMT_CPIA1v4l2_fourcc('C', 'P', 'I', 'A') /*
cpia1 YUV */
#define V4L2_PIX_FMT_WNVA v4l2_fourcc('W', 'N', 'V', 'A') /*
Winnov hw compress */
@@ -1009,6 +1034,7 @@ struct v4l2_ext_controls {
#define V4L2_CTRL_CLASS_MPEG 0x0099 /* MPEG-compression
controls */
#define V4L2_CTRL_CLASS_CAMERA 0x009a/* Camera class
controls */
#define V4L2_CTRL_CLASS_FM_TX 0x009b /* FM Modulator control
class */
+#define V4L2_CTRL_CLASS_CODEC 0x009c /* Codec control class
*/
#define V4L2_CTRL_ID_MASK (0x0fff)
#define V4L2_CTRL_ID2CLASS(id)((id) 0x0fffUL)
@@ -1342,6 +1368,150 @@ enum
v4l2_mpeg_cx2341x_video_median_filter_type {
#define V4L2_CID_MPEG_CX2341X_VIDEO_CHROMA_MEDIAN_FILTER_TOP
(V4L2_CID_MPEG_CX2341X_BASE+10)
#define V4L2_CID_MPEG_CX2341X_STREAM_INSERT_NAV_PACKETS
(V4L2_CID_MPEG_CX2341X_BASE+11)
+/* For codecs */
+#define V4L2_CID_CODEC_BASE
(V4L2_CTRL_CLASS_CODEC
| 0x900)
+#define V4L2_CID_CODEC_CLASS
(V4L2_CTRL_CLASS_CODEC
| 1)
+
+/* For decoding */
+#define V4L2_CID_CODEC_LOOP_FILTER_MPEG4_ENABLE
(V4L2_CID_CODEC_BASE + 110)
+#define V4L2_CID_CODEC_DISPLAY_DELAY (V4L2_CID_CODEC_BASE
+
137)
+#define V4L2_CID_CODEC_REQ_NUM_BUFS (V4L2_CID_CODEC_BASE
+
140)
+#define V4L2_CID_CODEC_SLICE_INTERFACE (V4L2_CID_CODEC_BASE
+
141)
+#define V4L2_CID_CODEC_PACKED_PB (V4L2_CID_CODEC_BASE + 142)
??? Weird CODEC_BASE offsets?
Are all these codec controls above general? I.e., applicable to any
codec? What
do they mean?
My mistake - I forgot to tidy up the offsets. It is difficult for me to
say which of those controls are MFC specific as I have little experience
with other codecs.
Currently PACKED_PB has been replaced with a simple mechanism that can
detect
if the stream has packed PB frames. You can read more about such streams
here:
http://itsjustonesandzeros.blogspot.com/2007/01/what-is-packed-
bitstream.htm
l
First approach required the application to set if the stream contained
packed-PB
Frames. Now the driver detects it the stream contains packed-PB frames.
Another
approach would require the stream parser to detect those frames and divide
them
into two buffers queued to MFC.
DISPLAY_DELAY is a number of frames that should be decoded before the
first
frame is
returned to the application. It is valid for H264 streams.
REQ_NUM_BUFS is the minimum
RE: [PATCH 3/4] MFC: Add MFC 5.1 V4L2 driver
Thank you for your reply about my comments. Refer to as below. k.deb...@samsung.com wrote: Hi, Peter. I have some first comments about this code. and Generally, Making patch set separately will be more helpful to everyone. Thank you for your comments. k.deb...@samsung.com wrote: Multi Format Codec 5.1 is a module available on S5PC110 and S5PC210 Samsung SoCs. Hardware is capable of handling a range of video codecs and this driver provides V4L2 interface for video decoding. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com [...] diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 6d0bd36..1d0b91e 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -1047,4 +1047,12 @@ config VIDEO_SAMSUNG_S5P_FIMC This is a v4l2 driver for the S5P camera interface (video postprocessor) +config VIDEO_SAMSUNG_S5P_MFC + tristate Samsung S5P MFC 5.0 Video Codec + depends on VIDEO_V4L2 CMA + select VIDEOBUF2_CMA + default n + help + MFC 5.0 driver for V4L2. + endif # V4L_MEM2MEM_DRIVERS What about unifying MFC version as a MFC 5.1, because we are using MFC HW ver.(MFC 5.1.x) in the C110/C210 chip. Right, will fix this. [...] diff --git a/drivers/media/video/s5p-mfc/regs-mfc5.h b/drivers/media/video/s5p-mfc/regs-mfc5.h new file mode 100644 index 000..8c628ad --- /dev/null +++ b/drivers/media/video/s5p-mfc/regs-mfc5.h @@ -0,0 +1,305 @@ +/* + * + * Register definition file for Samsung MFC V4.0 V5.0 Interface (FIMV) driver + * + * Kamil Debski, Copyright (c) 2010 Samsung Electronics + * http://www.samsung.com/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ Incorrect comment in the header, MFC V4.0 is not included Right again. + +#ifndef _REGS_FIMV_H +#define _REGS_FIMV_H + +#define S5P_FIMV_REG_SIZE(S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) +#define S5P_FIMV_REG_COUNT ((S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) / 4) + +#define S5P_FIMV_START_ADDR 0x +#define S5P_FIMV_END_ADDR0xe008 + +#define S5P_FIMV_SW_RESET0x +#define S5P_FIMV_RISC_HOST_INT 0x0008 +/* Command from HOST to RISC */ +#define S5P_FIMV_HOST2RISC_CMD 0x0030 +#define S5P_FIMV_HOST2RISC_ARG1 0x0034 +#define S5P_FIMV_HOST2RISC_ARG2 0x0038 +#define S5P_FIMV_HOST2RISC_ARG3 0x003c +#define S5P_FIMV_HOST2RISC_ARG4 0x0040 +/* Command from RISC to HOST */ +#define S5P_FIMV_RISC2HOST_CMD 0x0044 +#define S5P_FIMV_RISC2HOST_ARG1 0x0048 +#define S5P_FIMV_RISC2HOST_ARG2 0x004c +#define S5P_FIMV_RISC2HOST_ARG3 0x0050 +#define S5P_FIMV_RISC2HOST_ARG4 0x0054 + +#define S5P_FIMV_FW_VERSION 0x0058 +#define S5P_FIMV_SYS_MEM_SZ 0x005c +#define S5P_FIMV_FW_STATUS 0x0080 +/* Memory controller register */ +#define S5P_FIMV_MC_DRAMBASE_ADR_A 0x0508 +#define S5P_FIMV_MC_DRAMBASE_ADR_B 0x050c +#define S5P_FIMV_MC_STATUS 0x0510 + +/* In case of 2 master */ This comment(In case of 2 master) is meaningless. it was used at the beginning step of development. Will fix this too. +/* Common register */ +#define S5P_FIMV_SYS_MEM_ADR 0x0600 /* firmware buffer */ +#define S5P_FIMV_CPB_BUF_ADR 0x0604 /* stream buffer */ +#define S5P_FIMV_DESC_BUF_ADR0x0608 /* descriptor buffer */ +/* H264 decoding */ +#define S5P_FIMV_VERT_NB_MV_ADR 0x068c /* vertical neighbor motion vector */ +#define S5P_FIMV_VERT_NB_IP_ADR 0x0690 /* neighbor pixels for intra [...] displayed pic */ +#define S5P_FIMV_SI_DISPLAY_C_ADR 0x2014 /* chroma address of displayed pic */ +#define S5P_FIMV_SI_DEC_FRM_SIZE 0x2018 /* the number of frames decoded */ S5P_FIMV_SI_DEC_FRM_SIZE does actually means consumed number of bytes to decode a frame Ok, I've changed the name of the define to reflect the purpose of the register. +#define S5P_FIMV_SI_DISPLAY_STATUS 0x201c /* status of decoded picture */ +#define S5P_FIMV_SI_FRAME_TYPE 0x2020 /* frame type such as skip/I/P/B */ + +#define S5P_FIMV_SI_CH0_SB_ST_ADR0x2044 /* start addr of stream buf */ +#define S5P_FIMV_SI_CH0_SB_FRM_SIZE 0x2048 /* size of stream buf */ +#define S5P_FIMV_SI_CH0_DESC_ADR 0x204c /* addr of descriptor buf */ +/* Encoder for MPEG4 */ +#define S5P_FIMV_ENC_MPEG4_QUART_PXL 0xe008 /* qpel interpolation ctrl */ + +/* Additional */ +#define S5P_FIMV_SI_CH0_DPB_CONF_CTRL 0x2068 /* DPB Config Control Register */ +#define S5P_FIMV_SI_CH0_RELEASE_BUF 0x2060
