Hi Kamil,
I went through the decoder flow (not codec specific yet) of your MFC
driver. Since I havent acquired/produced a v4l2 based test case, I didnt
check the functionality yet. Below are some of minor comments I found
during my review.
Regards,
Subash
SISO-SLG
On 06/14/2011 10:06 PM, Kamil Debski wrote:
...
diff --git a/drivers/media/video/s5p-mfc/s5p_mfc.c b/drivers/media/video/s5p-mfc/s5p_mfc.c
new file mode 100644
...
+/* Open an MFC node */
+static int s5p_mfc_open(struct file *file)
+{
+ struct s5p_mfc_ctx *ctx = NULL;
+ struct s5p_mfc_dev *dev = video_drvdata(file);
+ struct vb2_queue *q;
+ unsigned long flags;
+ int ret = 0;
+
+ mfc_debug_enter();
+
+ dev->num_inst++; /* It is guarded by mfc_mutex in vfd */
+
+ /* Allocate memory for context */
+ ctx = kzalloc(sizeof *ctx, GFP_KERNEL);
+ if (!ctx) {
+ mfc_err("Not enough memory.\n");
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ ret = v4l2_fh_init(&ctx->fh, video_devdata(file));
+ if (ret) {
+ mfc_err("Failed to init v4l2_fh\n");
+ goto err_fh;
+ }
+
+ file->private_data =&ctx->fh;
+ v4l2_fh_add(&ctx->fh);
+
+ ctx->dev = dev;
+ INIT_LIST_HEAD(&ctx->src_queue);
+ INIT_LIST_HEAD(&ctx->dst_queue);
+ ctx->src_queue_cnt = 0;
+ ctx->dst_queue_cnt = 0;
+ /* Get context number */
+ ctx->num = 0;
+ while (dev->ctx[ctx->num]) {
+ ctx->num++;
+ if (ctx->num>= MFC_NUM_CONTEXTS) {
+ mfc_err("Too many open contexts.\n");
+ ret = -EBUSY;
+ goto err_no_ctx;
+ }
+ }
+ /* Mark context as idle */
+ spin_lock_irqsave(&dev->condlock, flags);
+ clear_bit(ctx->num,&dev->ctx_work_bits);
+ spin_unlock_irqrestore(&dev->condlock, flags);
+ dev->ctx[ctx->num] = ctx;
+ if (s5p_mfc_get_node_type(file) == MFCNODE_DECODER) {
+ ctx->type = MFCINST_DECODER;
+ ctx->c_ops = get_dec_codec_ops();
+ /* Setup ctrl handler */
+ ret = s5p_mfc_dec_ctrls_setup(ctx);
+ if (ret) {
+ mfc_err("Failed to setup mfc controls\n");
+ goto err_ctrls_setup;
+ }
+
+
+ } else if (s5p_mfc_get_node_type(file) == MFCNODE_ENCODER) {
+ ctx->type = MFCINST_ENCODER;
+ ctx->c_ops = get_enc_codec_ops();
+ /* only for encoder */
+ INIT_LIST_HEAD(&ctx->ref_queue);
+ ctx->ref_queue_cnt = 0;
+
+ /* Setup ctrl handler */
+ ret = s5p_mfc_enc_ctrls_setup(ctx);
+ if (ret) {
+ mfc_err("Failed to setup mfc controls\n");
+ goto err_ctrls_setup;
+ }
+ } else {
+ ret = -ENOENT;
+ goto err_bad_node;
+ }
+
+ ctx->fh.ctrl_handler =&ctx->ctrl_handler;
+ ctx->inst_no = -1;
+ /* Load firmware if this is the first instance */
+ if (dev->num_inst == 1) {
+ dev->watchdog_timer.expires = jiffies +
+ msecs_to_jiffies(MFC_WATCHDOG_INTERVAL);
+ add_timer(&dev->watchdog_timer);
+
+ mfc_debug(2, "power on\n");
+ ret = s5p_mfc_power_on();
+ if (ret< 0) {
+ mfc_err("power on failed\n");
+ goto err_pwr_enable;
+ }
+
+ s5p_mfc_clock_on();
+
+ /* Load the FW */
+ ret = s5p_mfc_alloc_firmware(dev);
+ if (ret != 0)
+ goto err_alloc_fw;
+ ret = s5p_mfc_load_firmware(dev);
+ if (ret != 0)
+ goto err_load_fw;
+
+ /* Init the FW */
+ ret = s5p_mfc_init_hw(dev);
+ if (ret != 0)
+ goto err_init_hw;
+
+ s5p_mfc_clock_off();
+ }
+
+ /* Init videobuf2 queue for CAPTURE */
+ q =&ctx->vq_dst;
+ q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+ q->drv_priv =&ctx->fh;
+ if (s5p_mfc_get_node_type(file) == MFCNODE_DECODER) {
+ q->io_modes = VB2_MMAP;
+ q->ops = get_dec_queue_ops();
+ } else {
+ q->io_modes = VB2_MMAP | VB2_USERPTR;
+ q->ops = get_enc_queue_ops();
+ }
Node type is declared as one of MFCNODE_INVALID = -1,MFCNODE_DECODER =
0, MFCNODE_ENCODER = 1. Hence it would be good to check even for encoder
and return error for the invalid case.
+
+ q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
+ ret = vb2_queue_init(q);
+ if (ret) {
+ mfc_err("Failed to initialize videobuf2 queue(capture)\n");
+ goto err_queue_init;
+ }
+
+ /* Init videobuf2 queue for OUTPUT */
+ q =&ctx->vq_src;
+ q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+ q->io_modes = VB2_MMAP;
+ q->drv_priv =&ctx->fh;
+ if (s5p_mfc_get_node_type(file) == MFCNODE_DECODER) {
+ q->io_modes = VB2_MMAP;
+ q->ops = get_dec_queue_ops();
+ } else {
+ q->io_modes = VB2_MMAP | VB2_USERPTR;
+ q->ops = get_enc_queue_ops();
+ }
+
Same as above.
+ q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
+ ret = vb2_queue_init(q);
+ if (ret) {
+ mfc_err("Failed to initialize videobuf2 queue(output)\n");
+ goto err_queue_init;
+ }
+
+ init_waitqueue_head(&ctx->queue);
+ mfc_debug(2, "%s-- (via irq_cleanup_hw)\n", __func__);
+ return ret;
+
+ /* Deinit when failure occured */
+err_queue_init:
+err_init_hw:
+err_load_fw:
+ s5p_mfc_release_firmware(dev);
+err_alloc_fw:
+ dev->ctx[ctx->num] = 0;
+ del_timer_sync(&dev->watchdog_timer);
+ s5p_mfc_clock_off();
+err_pwr_enable:
+ if (dev->num_inst == 1) {
+ if (s5p_mfc_power_off()< 0)
+ mfc_err("power off failed\n");
+ s5p_mfc_release_firmware(dev);
+ }
+err_ctrls_setup:
+ s5p_mfc_dec_ctrls_delete(ctx);
+err_bad_node:
+err_no_ctx:
+ v4l2_fh_del(&ctx->fh);
+ v4l2_fh_exit(&ctx->fh);
+err_fh:
+ kfree(ctx);
+err_alloc:
+ dev->num_inst--;
+ mfc_debug_leave();
+ return ret;
+}
+
diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
new file mode 100644
index 0000000..a3d7378
...
+/* Get format */
+static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
+{
+ struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
+ struct v4l2_pix_format_mplane *pix_mp;
+
+ mfc_debug_enter();
+ pix_mp =&f->fmt.pix_mp;
+ mfc_debug(2, "f->type = %d ctx->state = %d\n", f->type, ctx->state);
+ if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE&&
+ (ctx->state == MFCINST_GOT_INST || ctx->state ==
+ MFCINST_RES_CHANGE_END)) {
+ /* If the MFC is parsing the header,
+ * so wait until it is finished */
+ s5p_mfc_clean_ctx_int_flags(ctx);
+ s5p_mfc_wait_for_done_ctx(ctx, S5P_FIMV_R2H_CMD_SEQ_DONE_RET,
+ 0);
+ }
+ if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE&&
+ ctx->state>= MFCINST_HEAD_PARSED&&
+ ctx->state< MFCINST_ABORT) {
+ /* This is run on CAPTURE (deocde output) */
decode
+ /* Width and height are set to the dimensions
+ of the movie, the buffer is bigger and
+ further processing stages should crop to this
+ rectangle. */
+ pix_mp->width = ctx->buf_width;
+ pix_mp->height = ctx->buf_height;
+ pix_mp->field = V4L2_FIELD_NONE;
+ pix_mp->num_planes = 2;
+ /* Set pixelformat to the format in which MFC
+ outputs the decoded frame */
+ pix_mp->pixelformat = V4L2_PIX_FMT_NV12MT;
+ pix_mp->plane_fmt[0].bytesperline = ctx->buf_width;
+ pix_mp->plane_fmt[0].sizeimage = ctx->luma_size;
+ pix_mp->plane_fmt[1].bytesperline = ctx->buf_width;
+ pix_mp->plane_fmt[1].sizeimage = ctx->chroma_size;
+ } else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+ /* This is run on OUTPUT
+ The buffer contains compressed image
+ so width and height have no meaning */
+ pix_mp->width = 0;
+ pix_mp->height = 0;
+ pix_mp->field = V4L2_FIELD_NONE;
+ pix_mp->plane_fmt[0].bytesperline = ctx->dec_src_buf_size;
+ pix_mp->plane_fmt[0].sizeimage = ctx->dec_src_buf_size;
+ pix_mp->pixelformat = ctx->src_fmt->fourcc;
+ pix_mp->num_planes = ctx->src_fmt->num_planes;
+ } else {
+ mfc_err("Format could not be read\n");
+ mfc_debug(2, "%s-- with error\n", __func__);
+ return -EINVAL;
+ }
+ mfc_debug_leave();
+ return 0;
+}
+
...
diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_mem.h b/drivers/media/video/s5p-mfc/s5p_mfc_mem.h
new file mode 100644
index 0000000..0ffa931
--- /dev/null
+++ b/drivers/media/video/s5p-mfc/s5p_mfc_mem.h
@@ -0,0 +1,55 @@
+/*
+ * linux/drivers/media/video/s5p-mfc/s5p_mfc_mem.h
+ *
+ * Copyright (c) 2010 Samsung Electronics Co., Ltd.
+ * 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 as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __S5P_MFC_MEM_H_
+#define __S5P_MFC_MEM_H_ __FILE__
+
+#include<linux/platform_device.h>
+#include "s5p_mfc_common.h"
+
+/* Offset base used to differentiate between CAPTURE and OUTPUT
+* while mmaping */
+
+#define MFC_OFFSET_SHIFT 11
+
+#define DST_QUEUE_OFF_BASE (TASK_SIZE / 2)
+
+#define FIRMWARE_CODE_SIZE 0x60000 /* 384KB */
Instead of hard-coding firware size value, cant we pick it up from
.config file? I hope with more codecs being supported, this size will
change in future.
+#define MFC_H264_CTX_BUF_SIZE 0x96000 /* 600KB per H264 instance */
+#define MFC_CTX_BUF_SIZE 0x2800 /* 10KB per instance */
+#define DESC_BUF_SIZE 0x20000 /* 128KB for DESC buffer */
+#define SHARED_BUF_SIZE 0x1000 /* 4KB for shared buffer */
+
+#define DEF_CPB_SIZE 0x40000 /* 512KB */
+
+#define MFC_BANK_A_ALLOC_CTX 0
+#define MFC_BANK_B_ALLOC_CTX 1
+#define MFC_FW_ALLOC_CTX 0
Since we are loading the firmware into bank-0, cant this be
#define MFC_FW_ALLOC_CTX MFC_BANK_A_ALLOC_CTX
But what in future, if this needs to be configurable?
+
+#define MFC_BANK1_ALIGN_ORDER 13
+#define MFC_BANK2_ALIGN_ORDER 13
+#define MFC_FW_ALIGN_ORDER 17
+
+#define MFC_BASE_ALIGN_ORDER MFC_FW_ALIGN_ORDER
+
+#include<media/videobuf2-dma-contig.h>
+
+static inline size_t s5p_mfc_mem_cookie(void *a, void *b)
+{
+ /* Same functionality as the vb2_dma_contig_plane_paddr */
+ dma_addr_t *paddr = vb2_dma_contig_memops.cookie(b);
+
+ return *paddr;
+}
+
+
+#endif /* __S5P_MFC_MEM_H_ */
Regards,
Subash
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
