[media] s5p-mfc: Move firmware allocation point to avoid allocation problems

Move firmware allocation from open to probe to avoid problems
when using CMA for allocation. In certain circumstances CMA may allocate
buffer that is not in the beginning of the MFC memory area.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 3930177db1256540ad940f03819f8ef4ba3cf86d..9a679fab73e235dcff390b3b70666ed126de01d7 100644 (file)
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -793,14 +793,16 @@ static int s5p_mfc_open(struct file *file)
                        goto err_pwr_enable;
                }
                s5p_mfc_clock_on();
-               ret = s5p_mfc_alloc_and_load_firmware(dev);
-               if (ret)
-                       goto err_alloc_fw;
+               ret = s5p_mfc_load_firmware(dev);
+               if (ret) {
+                       s5p_mfc_clock_off();
+                       goto err_load_fw;
+               }
                /* Init the FW */
                ret = s5p_mfc_init_hw(dev);
+               s5p_mfc_clock_off();
                if (ret)
                        goto err_init_hw;
-               s5p_mfc_clock_off();
        }
        /* Init videobuf2 queue for CAPTURE */
        q = &ctx->vq_dst;
@@ -849,17 +851,16 @@ static int s5p_mfc_open(struct file *file)
        return ret;
        /* Deinit when failure occured */
 err_queue_init:
+       if (dev->num_inst == 1)
+               s5p_mfc_deinit_hw(dev);
 err_init_hw:
-       s5p_mfc_release_firmware(dev);
-err_alloc_fw:
+err_load_fw:
        dev->ctx[ctx->num] = NULL;
        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);
@@ -917,11 +918,8 @@ static int s5p_mfc_release(struct file *file)
                clear_bit(0, &dev->hw_lock);
        dev->num_inst--;
        if (dev->num_inst == 0) {
-               mfc_debug(2, "Last instance - release firmware\n");
-               /* reset <-> F/W release */
-               s5p_mfc_reset(dev);
+               mfc_debug(2, "Last instance\n");
                s5p_mfc_deinit_hw(dev);
-               s5p_mfc_release_firmware(dev);
                del_timer_sync(&dev->watchdog_timer);
                if (s5p_mfc_power_off() < 0)
                        mfc_err("Power off failed\n");
@@ -1149,6 +1147,10 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 
        mutex_init(&dev->mfc_mutex);
 
+       ret = s5p_mfc_alloc_firmware(dev);
+       if (ret)
+               goto err_alloc_fw;
+
        ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
        if (ret)
                goto err_v4l2_dev_reg;
@@ -1230,6 +1232,8 @@ err_dec_reg:
 err_dec_alloc:
        v4l2_device_unregister(&dev->v4l2_dev);
 err_v4l2_dev_reg:
+       s5p_mfc_release_firmware(dev);
+err_alloc_fw:
        vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[1]);
 err_mem_init_ctx_1:
        vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
@@ -1255,6 +1259,7 @@ static int __devexit s5p_mfc_remove(struct platform_device *pdev)
        video_unregister_device(dev->vfd_enc);
        video_unregister_device(dev->vfd_dec);
        v4l2_device_unregister(&dev->v4l2_dev);
+       s5p_mfc_release_firmware(dev);
        vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
        vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[1]);
 

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 3b9b600a6182fb7c2f3e575d599dacc671a433cc..0df6454e407ee3616385de9423cfff4c6826cb99 100644 (file)
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -278,8 +278,9 @@ struct s5p_mfc_priv_buf {
  * @int_err:           error number for last interrupt
  * @queue:             waitqueue for waiting for completion of device commands
  * @fw_size:           size of firmware
- * @bank1:             address of the beggining of bank 1 memory
- * @bank2:             address of the beggining of bank 2 memory
+ * @fw_virt_addr:      virtual firmware address
+ * @bank1:             address of the beginning of bank 1 memory
+ * @bank2:             address of the beginning of bank 2 memory
  * @hw_lock:           used for hardware locking
  * @ctx:               array of driver contexts
  * @curr_ctx:          number of the currently running context
@@ -318,8 +319,9 @@ struct s5p_mfc_dev {
        unsigned int int_err;
        wait_queue_head_t queue;
        size_t fw_size;
-       size_t bank1;
-       size_t bank2;
+       void *fw_virt_addr;
+       dma_addr_t bank1;
+       dma_addr_t bank2;
        unsigned long hw_lock;
        struct s5p_mfc_ctx *ctx[MFC_NUM_CONTEXTS];
        int curr_ctx;

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index 585b7b0ed8ec6a4cd4422246e22c0170f29ac10d..1682271c24531096f05dc045c1df95a9f61f812a 100644 (file)
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -22,16 +22,64 @@
 #include "s5p_mfc_opr.h"
 #include "s5p_mfc_pm.h"
 
-static void *s5p_mfc_bitproc_buf;
-static size_t s5p_mfc_bitproc_phys;
-static unsigned char *s5p_mfc_bitproc_virt;
+/* Allocate memory for firmware */
+int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
+{
+       void *bank2_virt;
+       dma_addr_t bank2_dma_addr;
+
+       dev->fw_size = dev->variant->buf_size->fw;
+
+       if (dev->fw_virt_addr) {
+               mfc_err("Attempting to allocate firmware when it seems that it is already loaded\n");
+               return -ENOMEM;
+       }
+
+       dev->fw_virt_addr = dma_alloc_coherent(dev->mem_dev_l, dev->fw_size,
+                                       &dev->bank1, GFP_KERNEL);
+
+       if (IS_ERR(dev->fw_virt_addr)) {
+               dev->fw_virt_addr = NULL;
+               mfc_err("Allocating bitprocessor buffer failed\n");
+               return -ENOMEM;
+       }
+
+       dev->bank1 = dev->bank1;
+
+       if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) {
+               bank2_virt = dma_alloc_coherent(dev->mem_dev_r, 1 << MFC_BASE_ALIGN_ORDER,
+                                       &bank2_dma_addr, GFP_KERNEL);
+
+               if (IS_ERR(dev->fw_virt_addr)) {
+                       mfc_err("Allocating bank2 base failed\n");
+                       dma_free_coherent(dev->mem_dev_l, dev->fw_size,
+                               dev->fw_virt_addr, dev->bank1);
+                       dev->fw_virt_addr = NULL;
+                       return -ENOMEM;
+               }
+
+               /* Valid buffers passed to MFC encoder with LAST_FRAME command
+                * should not have address of bank2 - MFC will treat it as a null frame.
+                * To avoid such situation we set bank2 address below the pool address.
+                */
+               dev->bank2 = bank2_dma_addr - (1 << MFC_BASE_ALIGN_ORDER);
+
+               dma_free_coherent(dev->mem_dev_r, 1 << MFC_BASE_ALIGN_ORDER,
+                       bank2_virt, bank2_dma_addr);
+
+       } else {
+               /* In this case bank2 can point to the same address as bank1.
+                * Firmware will always occupy the beggining of this area so it is
+                * impossible having a video frame buffer with zero address. */
+               dev->bank2 = dev->bank1;
+       }
+       return 0;
+}
 
-/* Allocate and load firmware */
-int s5p_mfc_alloc_and_load_firmware(struct s5p_mfc_dev *dev)
+/* Load firmware */
+int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
 {
        struct firmware *fw_blob;
-       size_t bank2_base_phys;
-       void *b_base;
        int err;
 
        /* Firmare has to be present as a separate file or compiled
@@ -44,77 +92,17 @@ int s5p_mfc_alloc_and_load_firmware(struct s5p_mfc_dev *dev)
                mfc_err("Firmware is not present in the /lib/firmware directory nor compiled in kernel\n");
                return -EINVAL;
        }
-       dev->fw_size = dev->variant->buf_size->fw;
        if (fw_blob->size > dev->fw_size) {
                mfc_err("MFC firmware is too big to be loaded\n");
                release_firmware(fw_blob);
                return -ENOMEM;
        }
-       if (s5p_mfc_bitproc_buf) {
-               mfc_err("Attempting to allocate firmware when it seems that it is already loaded\n");
-               release_firmware(fw_blob);
-               return -ENOMEM;
-       }
-       s5p_mfc_bitproc_buf = vb2_dma_contig_memops.alloc(
-               dev->alloc_ctx[MFC_BANK1_ALLOC_CTX], dev->fw_size);
-       if (IS_ERR(s5p_mfc_bitproc_buf)) {
-               s5p_mfc_bitproc_buf = NULL;
-               mfc_err("Allocating bitprocessor buffer failed\n");
+       if (!dev->fw_virt_addr) {
+               mfc_err("MFC firmware is not allocated\n");
                release_firmware(fw_blob);
-               return -ENOMEM;
-       }
-       s5p_mfc_bitproc_phys = s5p_mfc_mem_cookie(
-               dev->alloc_ctx[MFC_BANK1_ALLOC_CTX], s5p_mfc_bitproc_buf);
-       if (s5p_mfc_bitproc_phys & ((1 << MFC_BASE_ALIGN_ORDER) - 1)) {
-               mfc_err("The base memory for bank 1 is not aligned to 128KB\n");
-               vb2_dma_contig_memops.put(s5p_mfc_bitproc_buf);
-               s5p_mfc_bitproc_phys = 0;
-               s5p_mfc_bitproc_buf = NULL;
-               release_firmware(fw_blob);
-               return -EIO;
-       }
-       s5p_mfc_bitproc_virt = vb2_dma_contig_memops.vaddr(s5p_mfc_bitproc_buf);
-       if (!s5p_mfc_bitproc_virt) {
-               mfc_err("Bitprocessor memory remap failed\n");
-               vb2_dma_contig_memops.put(s5p_mfc_bitproc_buf);
-               s5p_mfc_bitproc_phys = 0;
-               s5p_mfc_bitproc_buf = NULL;
-               release_firmware(fw_blob);
-               return -EIO;
-       }
-       dev->bank1 = s5p_mfc_bitproc_phys;
-       if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) {
-               b_base = vb2_dma_contig_memops.alloc(
-                       dev->alloc_ctx[MFC_BANK2_ALLOC_CTX],
-                       1 << MFC_BASE_ALIGN_ORDER);
-               if (IS_ERR(b_base)) {
-                       vb2_dma_contig_memops.put(s5p_mfc_bitproc_buf);
-                       s5p_mfc_bitproc_phys = 0;
-                       s5p_mfc_bitproc_buf = NULL;
-                       mfc_err("Allocating bank2 base failed\n");
-                       release_firmware(fw_blob);
-                       return -ENOMEM;
-               }
-               bank2_base_phys = s5p_mfc_mem_cookie(
-                       dev->alloc_ctx[MFC_BANK2_ALLOC_CTX], b_base);
-               vb2_dma_contig_memops.put(b_base);
-               if (bank2_base_phys & ((1 << MFC_BASE_ALIGN_ORDER) - 1)) {
-                       mfc_err("The base memory for bank 2 is not aligned to 128KB\n");
-                       vb2_dma_contig_memops.put(s5p_mfc_bitproc_buf);
-                       s5p_mfc_bitproc_phys = 0;
-                       s5p_mfc_bitproc_buf = NULL;
-                       release_firmware(fw_blob);
-                       return -EIO;
-               }
-               /* Valid buffers passed to MFC encoder with LAST_FRAME command
-                * should not have address of bank2 - MFC will treat it as a null frame.
-                * To avoid such situation we set bank2 address below the pool address.
-                */
-               dev->bank2 = bank2_base_phys - (1 << MFC_BASE_ALIGN_ORDER);
-       } else {
-               dev->bank2 = dev->bank1;
+               return -EINVAL;
        }
-       memcpy(s5p_mfc_bitproc_virt, fw_blob->data, fw_blob->size);
+       memcpy(dev->fw_virt_addr, fw_blob->data, fw_blob->size);
        wmb();
        release_firmware(fw_blob);
        mfc_debug_leave();
@@ -142,12 +130,12 @@ int s5p_mfc_reload_firmware(struct s5p_mfc_dev *dev)
                release_firmware(fw_blob);
                return -ENOMEM;
        }
-       if (s5p_mfc_bitproc_buf == NULL || s5p_mfc_bitproc_phys == 0) {
-               mfc_err("MFC firmware is not allocated or was not mapped correctly\n");
+       if (dev->fw_virt_addr) {
+               mfc_err("MFC firmware is not allocated\n");
                release_firmware(fw_blob);
                return -EINVAL;
        }
-       memcpy(s5p_mfc_bitproc_virt, fw_blob->data, fw_blob->size);
+       memcpy(dev->fw_virt_addr, fw_blob->data, fw_blob->size);
        wmb();
        release_firmware(fw_blob);
        mfc_debug_leave();
@@ -159,12 +147,11 @@ int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev)
 {
        /* Before calling this function one has to make sure
         * that MFC is no longer processing */
-       if (!s5p_mfc_bitproc_buf)
+       if (!dev->fw_virt_addr)
                return -EINVAL;
-       vb2_dma_contig_memops.put(s5p_mfc_bitproc_buf);
-       s5p_mfc_bitproc_virt = NULL;
-       s5p_mfc_bitproc_phys = 0;
-       s5p_mfc_bitproc_buf = NULL;
+       dma_free_coherent(dev->mem_dev_l, dev->fw_size, dev->fw_virt_addr,
+                                               dev->bank1);
+       dev->fw_virt_addr = NULL;
        return 0;
 }
 
@@ -257,8 +244,10 @@ int s5p_mfc_init_hw(struct s5p_mfc_dev *dev)
        int ret;
 
        mfc_debug_enter();
-       if (!s5p_mfc_bitproc_buf)
+       if (!dev->fw_virt_addr) {
+               mfc_err("Firmware memory is not allocated.\n");
                return -EINVAL;
+       }
 
        /* 0. MFC reset */
        mfc_debug(2, "MFC reset..\n");

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
index 90aa9b9886d56f1e3c30e204ad2ee61793d1b8d1..6a9b6f8606bb79bdc5c25ac915c0dc20e031b9f5 100644 (file)
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
@@ -16,7 +16,8 @@
 #include "s5p_mfc_common.h"
 
 int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev);
-int s5p_mfc_alloc_and_load_firmware(struct s5p_mfc_dev *dev);
+int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev);
+int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev);
 int s5p_mfc_reload_firmware(struct s5p_mfc_dev *dev);
 
 int s5p_mfc_init_hw(struct s5p_mfc_dev *dev);