RE: [RFC/PATCH v7 3/5] MFC: Add MFC 5.1 V4L2 driver
Hi, Kamil I found some mal-functional points. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Kamil Debski Sent: Friday, March 04, 2011 8:26 PM To: linux-media@vger.kernel.org; linux-samsung-...@vger.kernel.org Cc: m.szyprow...@samsung.com; kyungmin.p...@samsung.com; k.deb...@samsung.com; jaeryul...@samsung.com; kgene@samsung.com Subject: [RFC/PATCH v7 3/5] 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 | 346 drivers/media/video/s5p-mfc/s5p_mfc.c| 2253 ++ drivers/media/video/s5p-mfc/s5p_mfc_common.h | 240 +++ drivers/media/video/s5p-mfc/s5p_mfc_ctrls.h | 182 +++ 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| 913 +++ drivers/media/video/s5p-mfc/s5p_mfc_opr.h| 142 ++ 13 files changed, 4296 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 diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 12fb325..0bdc64d 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -1025,4 +1025,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.1 Video Codec + depends on VIDEO_V4L2 + select VIDEOBUF2_S5P_IOMMU + default n + help + MFC 5.1 driver for V4L2. + endif # V4L_MEM2MEM_DRIVERS diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index fd9488d..4b09ddb 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -164,6 +164,7 @@ obj-$(CONFIG_VIDEO_SH_MOBILE_CSI2)+= sh_mobile_csi2.o obj-$(CONFIG_VIDEO_SH_MOBILE_CEU)+= sh_mobile_ceu_camera.o obj-$(CONFIG_VIDEO_OMAP1)+= omap1_camera.o obj-$(CONFIG_VIDEO_SAMSUNG_S5P_FIMC) += s5p-fimc/ +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) += s5p-mfc/ obj-$(CONFIG_ARCH_DAVINCI) += davinci/ diff --git a/drivers/media/video/s5p-mfc/Makefile b/drivers/media/video/s5p-mfc/Makefile new file mode 100644 index 000..69b6294 --- /dev/null +++ b/drivers/media/video/s5p-mfc/Makefile @@ -0,0 +1,3 @@ +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) := s5p-mfc.o +s5p-mfc-y := s5p_mfc.o s5p_mfc_intr.o s5p_mfc_opr.o + 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..eeb6e2e --- /dev/null +++ b/drivers/media/video/s5p-mfc/regs-mfc5.h @@ -0,0 +1,346 @@ snipping +} + +static inline void s5p_mfc_run_res_change(struct s5p_mfc_ctx *ctx) +{ + struct s5p_mfc_dev *dev = ctx-dev; + + s5p_mfc_set_dec_stream_buffer(ctx, 0, 0, 0); + dev-curr_ctx = ctx-num; + s5p_mfc_clean_ctx_int_flags(ctx); + s5p_mfc_decode_one_frame(ctx, MFC_DEC_RES_CHANGE); +} + +static inline void s5p_mfc_run_dec_last_frames(struct s5p_mfc_ctx *ctx) +{ + struct s5p_mfc_dev *dev = ctx-dev; + + s5p_mfc_set_dec_stream_buffer(ctx, 0, 0, 0); + dev-curr_ctx = ctx-num; + s5p_mfc_clean_ctx_int_flags(ctx); + s5p_mfc_decode_one_frame(ctx, MFC_DEC_LAST_FRAME); +} + You used '0' as a second param.(= buf_addr) in s5p_mfc_set_dec_stream_buffer(ctx, 0, 0, 0) This must be a problem, 'cause OFFSETA(buf_addr) is calculated wrongly. And I propose that why don't you try to check addr condition(necessary cond. of MFC, it should be located at rear region from base_addr) +static inline int s5p_mfc_run_dec_frame(struct s5p_mfc_ctx *ctx) +{ + struct s5p_mfc_dev *dev = ctx-dev; + struct s5p_mfc_buf *temp_vb;
RE: [ANN] First draft of the agenda for the Warsaw meeting
Hi, Hans We want to confirm if it is possible to discuss our concerning points at each item of 1st draft that you made as below. Our concerning items that we want to discuss are : 1. Issues when using V4l2 control framework with codec : http://www.spinics.net/lists/linux-media/msg27975.html : Can we discuss 'V4l2 control framework to support codec' in your first agenda ? (Compressed format API for MPEG, H.264, etc. xxx) : We will post related contents in mailing list before joining Warsaw meeting. 2. Subdev architecture enhancements(second agenda) : http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/29442 /focus=29570 - We posted our issue(subdev related) as above link, and If it(Subdev architecture enhancements)is applied, it might resolve our issue. : So, we want to talk about some problem solving way in second agenda 3. Which interface is better for Mixer of Exynos, frame buffer or V4l2 ? : http://www.mail-archive.com/linux-media@vger.kernel.org/msg28549.html - Here at fifth agenda (HDMI receiver/transmitter API support), We want to discuss that frame buffer interface is more suitable for Mixer. Can we discuss that here ? 4. Method for sharing buffer b/w processes : http://www.spinics.net/lists/linux-media/msg29936.html : This is one of big topics, I know : In Warsaw meeting we would like to share the SDVMM concept for buffer sharing. I know it's a very huge topic. But if you arrange the priority of it in the agenda, one more person in Samsung(S.LSI) will attend the meeting even though it is just a concept sharing. As I know, ST-Ericsson also uses Mali(ARM GPU). I also want to hear the usage from Willy Poisson. : Can we discuss this items at 'Buffer Pool(HWMEM, GEM, CMA)' ? -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Hans Verkuil Sent: Sunday, March 06, 2011 12:21 AM To: linux-media Subject: [ANN] First draft of the agenda for the Warsaw meeting Hi all! Here is the first draft of the agenda for the Warsaw meeting. Next weekend I'll prepare the final version. Please let me know any changes to/errors in the attendee list! I tried to order the agenda so we start gently (I hope!) and mix large and small items. I tried to name the main 'proponents' or 'stakeholders' for each item. Let me know if I am missing someone. For each item I added links to mailinglist discussions. I'm sure I'm missing links, so please let me know and I'll add it to the final agenda. If there are items missing, let me know as well. Although I suspect there won't be time for anything complicated. Hans Draft Agenda for V4L2 brainstorm meeting in Warsaw, March 16-18 2011. = Purpose of the meeting: to brainstorm about current V4L2 API limitations with regards to required functionality. Ideally the results of the meeting are actual solutions to these problems, but at the very least we should have a concensus of what direction to take and who will continue working on each problem. The hope is that this meeting will save us endless email and irc discussions. It is *not* a summit meeting, so any conclusions need to be discussed and approved on the mailinglist. The basic outline is the same as during previous meetings: the first day we go through all the agenda points and make sure everyone understands the problem. Smaller issues will be discussed and decided, more complex issues are just discussed. The second day we go in depth into the complex issues and try to come up with ideas that might work. The last day we translate the all agenda items into actions. This approach worked well in the past and it ensures that we end up with something concrete. Those who have a vested interest in an agenda item should be prepared to explain their take on it and if necessary have a presentation ready. Besides the main agenda I also added a few items falling under the category 'if time permits'. Attendees: Samsung Poland RD Center: Kamil Debski Sylwester Nawrocki Tomasz Stanislawski Marek Szyprowski (Organizer) Cisco Systems Norway: Martin Bugge Hans Verkuil (Chair) Nokia: Sakari Ailus Ideas On Board: Laurent Pinchart ST-Ericsson: Willy Poisson Samsung Korea (is this correct?): Jonghun Han Jaeryul Oh Freelance: Guennadi Liakhovetski Agenda: 1) Compressed format API for MPEG, H.264, etc. Also need to discuss what to do with weird 'H.264 inside MJPEG' muxed formats. (Hans, Laurent, Samsung) 2) Subdev architecture enhancements: - Acquiring subdevs from other devices using subdev pool http://www.mail-archive.com/linux- me...@vger.kernel.org/msg21831.html - Introducing subdev hierarchy. Below
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-media@vger.kernel.org; linux-samsung-...@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-media@vger.kernel.org; linux-samsung-...@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-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: V4L2 'brainstorming' meeting in Warsaw, March 2011
Hi, Marek Jonghun Han I will participate in that meeting. We are preparing for some topic that we want to talk with participants. I'll let you know about the agenda soon. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Marek Szyprowski Sent: Wednesday, March 02, 2011 12:27 AM To: Marek Szyprowski; linux-media@vger.kernel.org Subject: RE: V4L2 'brainstorming' meeting in Warsaw, March 2011 Hello once more, After some discussion on #v4l irc channel, I would like to inform that the meeting date has been fixed to my initial proposition: 3 days from 16 to 18 March 2011. Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media 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-media@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-media@vger.kernel.org; linux-samsung-...@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-media@vger.kernel.org; linux-samsung-...@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-media 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-media@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 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-media@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-media@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-media@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-media@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-media@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-media@vger.kernel.org; linux-samsung-...@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: RFC: Problem of using v4l2 spec with codec function
Hi, Kamil Hello, From: Jonghun Han [mailto:jonghun@samsung.com] Sent: 06 December 2010 07:57 Hi, If MFC(encoder/decoder) has a single node, how to know application's object before VIDIOC_S_FMT calling ? You're right. No way to know that before. VIDIOC_S_CTRL can be called before VIDIOC_S_FMT. For example user wants to use MFC as an *encoder*, but user calls VIDIOC_S_CTRL with *decoder* command by mistake. In that case driver cannot return fail because it cannot know the purpose before VIDIOC_S_FMT calling. When VIDIOC_S_FMT is called to use MFC as an *encoder* after VIDIOC_S_CTRL, driver will be confused how to handle it. But in two nodes solution decoder command via VIDIOC_S_CTRL will be failed on encoder device node. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Pawel Osciak Sent: Sunday, December 05, 2010 7:55 AM Hi all, I would side with Laurent on this. Judging by formats seems to be enough for this driver and it has great, in my opinion, advantages of a) not overcomplicating things for applications b) not adding new pieces to the API... We've had a discussion here with Sylwester Nawrocki and Marek Szyprowski. I suppose the application has to detect which video node is MFC anyway, for example by looking through /sys/class/video4linux or adding rules to udev. Same way it can choose which of the two node to use, so I don't think it going to be such a problem. In case of FIMC functionality - camera capture and m2m transformations are divided between two video nodes. In case of differentiating by format, I agree that it is feasible, but in my opinion it will overcomplicate the driver. When two nodes are used, there are going to be separate videobuf2 ops. Those functions that can be reused, would be reused, otherwise there would be two versions - one for encoding and one for decoding. Without that most functions would have to check what the context is doing and act appropriately. This means more complex code. It will have similar camera capture and m2m style. But we are considering that functions are reused enough b/w encoder decoder. -- peter Oh, Senior Engineer SW Solution Development Team, System LSI Division, Samsung Electronics, Co., Ltd. Email: jaeryul...@samsung.com --- What API changes have you had in mind that would be required when using two video nodes? Best regards -- Kamil Debski Linux Platform Group Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: Problem of using v4l2 spec with codec function
Hello, everyone. When it comes to using v4l2 standard spec, I have a question about that. A month ago, Kamil Debski posted second version for the driver of a hw video codec. To be exact, it is decoding function of hw video codec which is called MFC(Multi Format Codec). For members not accustomed to this, refer to link as below. : http://www.spinics.net/lists/linux-media/msg24682.html I'm preparing for patch of encoding function of MFC. But simply I could say, According to the current version of v4l2 standard, User cannot call encoder or decoder differentially. I mean, Kamil designed MFC decoder driver using m2m style(not using by m2m framework, but V4L2_CAP_VIDEO_OUTPUT, V4L2_CAP_VIDEO_CAPTURE) And it would be almost same to implement encoder in terms of using same m2m style. But user should be able to call decoder for decoding, Due to the same reason, user should be able to call encoder. Of course, some different functions b/w encoder decoder are called. (Driver should know what function(enc or dec) is called) What do you think is the best way to let driver know what function user wants to execute ? 1. VIDIOC_QUERYCAP is mandatory, but it doesn't define about any encoder, decoder in the v4l2_capability : we need new capability for codec such as V4L2_BUF_TYPE_VIDEO_ENCODER, V4L2_BUF_TYPE_VIDEO_DECODER 2. What about using VIDIOC_S_CTRL, but it requires new defined decoder or encoder as a ctrl.id. Another problem is VIDIOC_S_CTRL is optional 3. What about VIDIOC_S_FMT ? it is also optional, which means default format will be used if not VIDIOC_S_FMT called. Let me know best way such as one among appropriate way that I mentioned as above, or any other idea ? Thanks all -- peter Oh, Senior Engineer SW Solution Development Team, System LSI Division, Samsung Electronics, Co., Ltd. Phone: +82-31-32-52287 Mobile:+82-10-3369-1989 Email: jaeryul...@samsung.com --- -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] MFC: Add MFC 5.1 V4L2 driver
I have some opinion about the usage of wait_event_interruptible_timeout() k.deb...@samsung.com wrote: [snip] + diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_intr.c b/drivers/media/video/s5p-mfc/s5p_mfc_intr.c new file mode 100644 index 000..543f3fb --- /dev/null +++ b/drivers/media/video/s5p-mfc/s5p_mfc_intr.c @@ -0,0 +1,77 @@ +/* + * drivers/media/video/samsung/mfc5/s5p_mfc_intr.c + * + * C file for Samsung MFC (Multi Function Codec - FIMV) driver + * This file contains functions used to wait for command completion. + * + * 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. + */ + +#include linux/delay.h +#include linux/errno.h +#include linux/wait.h +#include linux/sched.h +#include linux/io.h +#include regs-mfc5.h +#include s5p_mfc_intr.h +#include s5p_mfc_logmsg.h +#include s5p_mfc_common.h + +int s5p_mfc_wait_for_done_dev(struct s5p_mfc_dev *dev, int command) +{ + if (wait_event_interruptible_timeout(dev-queue, + (dev-int_cond (dev-int_type == command + || dev-int_type == S5P_FIMV_R2H_CMD_DECODE_ERR_RET)), + msecs_to_jiffies(MFC_INT_TIMEOUT)) == 0) { + mfc_err(Interrupt (%d dev) timed out.\n, dev-int_type); + return 1; + } + mfc_debug(Finished waiting (dev-queue, %d).\n, dev-int_type); + if (dev-int_type == S5P_FIMV_R2H_CMD_ERROR_RET) + return 1; + return 0; +} You used wait_event_interruptible_timeout() in the driver, but this function is considered not only int. by MFC but also int. by some signal. I'm wondering whether we have to consider interrupt by signal in the middle of hw operation. and also I cannot see some operation(handler) in case of wake-up by signal. So, why don’t you remove the interruptible function or add some operation in case of wake-up by signal. (refer to the other driver in the kernel) + +void s5p_mfc_clean_dev_int_flags(struct s5p_mfc_dev *dev) +{ + dev-int_cond = 0; + dev-int_type = 0; + dev-int_err = 0; +} + +int s5p_mfc_wait_for_done_ctx(struct s5p_mfc_ctx *ctx, + int command, int interrupt) +{ + int ret; + if (interrupt) { + ret = wait_event_interruptible_timeout(ctx-queue, + (ctx-int_cond (ctx-int_type == command + || ctx-int_type == S5P_FIMV_R2H_CMD_DECODE_ERR_RET)), + msecs_to_jiffies(MFC_INT_TIMEOUT)); + } else { + ret = wait_event_timeout(ctx-queue, + (ctx-int_cond (ctx-int_type == command + || ctx-int_type == S5P_FIMV_R2H_CMD_DECODE_ERR_RET)), + msecs_to_jiffies(MFC_INT_TIMEOUT)); + } + if (ret == 0) { + mfc_err(Interrupt (%d ctx) timed out.\n, ctx-int_type); + return 1; + } + mfc_debug(Finished waiting (ctx-queue, %d).\n, ctx-int_type); + if (ctx-int_type == S5P_FIMV_R2H_CMD_ERROR_RET) + return 1; + return 0; +} + +void s5p_mfc_clean_ctx_int_flags(struct s5p_mfc_ctx *ctx) +{ + ctx-int_cond = 0; + ctx-int_type = 0; + ctx-int_err = 0; +} [snip] -- 1.6.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] MFC: Add MFC 5.1 V4L2 driver
I commented as belows, And you missed one important things 'cause there were my comments in the very long email which is strongly fixed in the reset seq. [] +#define READL(offset)readl(dev-regs_base + (offset)) +#define WRITEL(data, offset) writel((data), dev-regs_base + (offset)) +#define OFFSETA(x) (((x) - dev-port_a) 11) +#define OFFSETB(x) (((x) - dev-port_b) 11) + +/* Reset the device */ +static int s5p_mfc_cmd_reset(struct s5p_mfc_dev *dev) +{ + unsigned int mc_status; + unsigned long timeout; + mfc_debug(s5p_mfc_cmd_reset++\n); + /* Stop procedure */ + WRITEL(0x3f7, S5P_FIMV_SW_RESET); /* reset VI */ Ahm, This (WRITEL(0x3f7, S5P_FIMV_SW_RESET)) might be a problem. In the reset seq. of MFC driver, we checked out That FW(s5pc110-mfc.fw in the s5p_mfc_load_firmware()) runned by RISC core at this point could access invalid address. It should be removed. Hi, Kamil This is second feedback about the HW op related code. (s5p_mfc_opr.c s5p_mfc.c) Hi, Peter Thanks for the comments. I have replied to them below. I would be grateful if you could cut out non relevant parts of the code in your replies. It was difficult to find your comments in such a big email. I hope I have found all of them. [...] + s5p_mfc_set_dec_stream_buffer(ctx, \ +vb2_plane_paddr(temp_vb, 0), 0, \ +temp_vb-v4l2_planes[0].bytesused); + dev-curr_ctx = ctx-num; + s5p_mfc_clean_ctx_int_flags(ctx); + s5p_mfc_decode_one_frame(ctx, + temp_vb-v4l2_planes[0].bytesused == 0); + } else if (ctx-state == MFCINST_DEC_INIT) { + /* Preparing decoding - getting instance number */ + mfc_debug(Getting instance number\n); + dev-curr_ctx = ctx-num; + s5p_mfc_clean_ctx_int_flags(ctx); +/*s5p_mfc_set_dec_temp_buffers(ctx); + *Removed per request by Peter, check if MFC works OK */ Yes, you don't have to set s5p_mfc_set_dec_temp_buffers(ctx)at this point 'cause this is only required before parsing header of the stream except for setting shared memory, so, I think, separating 'setting S5P_FIMV_SI_CH0_HOST_WR_ADR' from the s5p_mfc_set_dec_temp_buffers() is needed. So I mean remove 'setting S5P_FIMV_SI_CH0_HOST_WR_ADR' from s5p_mfc_set_dec_temp_buffers(ctx);, then add 'setting S5P_FIMV_SI_CH0_HOST_WR_ADR' in the 3 functions (s5p_mfc_set_dec_frame_buffer(),s5p_mfc_init_decode(), s5p_mfc_decode_one_frame()) I also commented suggested loc. of 'setting S5P_FIMV_SI_CH0_HOST_WR_ADR' Right, I've changed this. + ret = s5p_mfc_open_inst(ctx); + if (ret) { + mfc_err(Failed to create a new instance.\n); + ctx-state = MFCINST_DEC_ERROR; + } + } else if (ctx-state == MFCINST_DEC_RETURN_INST) { + /* Closing decoding instance */ + mfc_debug(Returning instance number\n); + dev-curr_ctx = ctx-num; + s5p_mfc_clean_ctx_int_flags(ctx); + ret = s5p_mfc_return_inst_no(ctx); + if (ret) { + mfc_err(Failed to return an instance.\n); + ctx-state = MFCINST_DEC_ERROR; + } [...] + } + /* Init videobuf2 queue for CAPTURE */ + ret = vb2_queue_init(ctx-vq_dst, s5p_mfc_qops, + dev-alloc_ctx[0], dev-irqlock, + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, ctx); + if (ret) { + mfc_err(Failed to initialize videobuf2 queue (capture)\n); + goto out_open_3; + } + /* Init videobuf2 queue for OUTPUT */ + ret = vb2_queue_init(ctx-vq_src, s5p_mfc_qops, + dev-alloc_ctx[1], dev-irqlock, + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, ctx); + if (ret) { + mfc_err(Failed to initialize videobuf2 queue (output)\n); + goto out_open_3; + } About using dev-irqlock, (spinlock_t *drv_lock) in the vb2_queue_init() is removed according to the Marek's patch (from the Hans Verkuil request) : http://www.spinics.net/lists/linux-media/msg23902.html Does it mean that we don't have to use drv_lock in the driver ? What actually purpose of drv_lock that you used in the MFC driver ? I'll remove this when videobuf2 will be merged. The version posted by Marek is still not final, so it is better to wait until a final version is worked out in the community. OK, I will wait until final version is released. + vb2_set_alloc_ctx(ctx-vq_dst, dev-alloc_ctx[1], 1); + init_waitqueue_head(ctx-queue); +
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