Re: pull request: linux-firmware: Update Mediatek MT8173 VPU firmware

2017-07-28 Thread andrew-ct chen
Hi linux-firmware maintainers,

Sorry, problems with this release firmware.
We will release v1.0.6 in a few days. Thanks a lot.

Andrew

On Thu, 2017-07-20 at 15:59 +0800, Andrew-CT Chen wrote:
> Hi linux-firmware maintainers,
> 
> The following changes since commit 7d2c913dcd1be083350d97a8cb1eba24cfacbc8a:
> 
>   ath10k: update year in license (2017-06-22 12:06:02 -0700)
> 
> are available in the git repository at:
> 
>   https://github.com/andrewct-chen/linux_fw_vpu_v1.0.5.git v1.0.5
> 
> for you to fetch changes up to 0d4ab52ba9be6f619024d7a57b1af05a03daa099:
> 
>   mediatek: update MT8173 VPU firmware to 1.0.5 (2017-07-20 15:27:05 +0800)
> 
> --------
> Andrew-CT Chen (1):
>   mediatek: update MT8173 VPU firmware to 1.0.5
> 
>  vpu_d.bin | Bin 4082928 -> 2977008 bytes
>  vpu_p.bin | Bin 131160 -> 131324 bytes
>  2 files changed, 0 insertions(+), 0 deletions(-)
> 
> 
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek




pull request: linux-firmware: Update Mediatek MT8173 VPU firmware

2017-07-20 Thread Andrew-CT Chen
Hi linux-firmware maintainers,

The following changes since commit 7d2c913dcd1be083350d97a8cb1eba24cfacbc8a:

  ath10k: update year in license (2017-06-22 12:06:02 -0700)

are available in the git repository at:

  https://github.com/andrewct-chen/linux_fw_vpu_v1.0.5.git v1.0.5

for you to fetch changes up to 0d4ab52ba9be6f619024d7a57b1af05a03daa099:

  mediatek: update MT8173 VPU firmware to 1.0.5 (2017-07-20 15:27:05 +0800)


Andrew-CT Chen (1):
  mediatek: update MT8173 VPU firmware to 1.0.5

 vpu_d.bin | Bin 4082928 -> 2977008 bytes
 vpu_p.bin | Bin 131160 -> 131324 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)



pull request: linux-firmware: Update Mediatek MT8173 VPU firmware

2017-03-28 Thread Andrew-CT Chen
Hi linux-firmware maintainers,

The following changes since commit 44d8e8d4fdcdff56e9e21208d4b4aa7cd6962656:

  Merge branch 'master' of https://github.com/Netronome/linux-firmware 
(2017-03-24 09:43:35 -0700)

are available in the git repository at:


  https://github.com/andrewct-chen/linux_fw_vpu_v1.0.4.git vp9_profile

for you to fetch changes up to 936f3e98847e89f119b24e0fa50f7028d667c744:

  mediatek: update MT8173 VPU firmware for unsupported VP9 profiles (2017-03-28 
15:21:24 +0800)


Andrew-CT Chen (1):
  mediatek: update MT8173 VPU firmware for unsupported VP9 profiles

 vpu_d.bin | Bin 4082928 -> 4082928 bytes
 vpu_p.bin | Bin 131160 -> 131160 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)



Re: [PATCH] [media] VPU: mediatek: fix dereference of pdev before checking it is null

2016-11-21 Thread andrew-ct chen
On Wed, 2016-11-16 at 19:16 +, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> pdev is dereferenced using platform_get_drvdata before a check to
> see if it is null, hence there could be a potential null pointer
> dereference issue. Instead, first check if pdev is null and only then
> deference pdev when initializing vpu.
> 
> Found with static analysis by CoverityScan, CID 1357797
> 
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> ---

Reviewed-by: Andrew-CT Chen <andrew-ct.c...@mediatek.com>

>  drivers/media/platform/mtk-vpu/mtk_vpu.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu.c 
> b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> index c9bf58c..41f31b2 100644
> --- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> @@ -523,9 +523,9 @@ static int load_requested_vpu(struct mtk_vpu *vpu,
>  
>  int vpu_load_firmware(struct platform_device *pdev)
>  {
> - struct mtk_vpu *vpu = platform_get_drvdata(pdev);
> + struct mtk_vpu *vpu;
>   struct device *dev = >dev;
> - struct vpu_run *run = >run;
> + struct vpu_run *run;
>   const struct firmware *vpu_fw = NULL;
>   int ret;
>  
> @@ -533,6 +533,8 @@ int vpu_load_firmware(struct platform_device *pdev)
>   dev_err(dev, "VPU platform device is invalid\n");
>   return -EINVAL;
>   }
> + vpu = platform_get_drvdata(pdev);
> + run = >run;
>  
>   mutex_lock(>vpu_mutex);
>   if (vpu->fw_loaded) {


--
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


pull request: linux-firmware: Update Mediatek MT8173 VPU firmware

2016-11-11 Thread Andrew-CT Chen
Hi linux-firmware maintainers,

The following changes since commit a179db97914da5e650c21ba8f9b0bae04a0f8a41:

  amdgpu: update SMC firmware for VI parts (2016-10-05 10:30:11 -0700)

are available in the git repository at:

  https://github.com/andrewct-chen/linux_fw_vpu_v1.0.3.git v1.0.3

for you to fetch changes up to f52fd5b4f156bd1eef672d29abbdc57cf310ac1b:

  mediatek: update MT8173 VPU firmware for idle state (2016-11-11 19:34:56 
+0800)


Andrew-CT Chen (1):
  mediatek: update MT8173 VPU firmware for idle state

 vpu_d.bin | Bin 4082928 -> 4082928 bytes
 vpu_p.bin | Bin 131160 -> 131160 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)
--
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] [media] VPU: mediatek: fix null pointer dereference on pdev

2016-09-08 Thread andrew-ct chen
On Wed, 2016-09-07 at 18:10 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> pdev is being null checked, however, prior to that it is being
> dereferenced by platform_get_drvdata.  Move the assignments of
> vpu and run to after the pdev null check to avoid a potential
> null pointer dereference.
> 

Reviewed-by:Andrew-CT Chen 

> Signed-off-by: Colin Ian King 
> ---
>  drivers/media/platform/mtk-vpu/mtk_vpu.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu.c 
> b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> index c9bf58c..43907a3 100644
> --- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> @@ -523,9 +523,9 @@ static int load_requested_vpu(struct mtk_vpu *vpu,
>  
>  int vpu_load_firmware(struct platform_device *pdev)
>  {
> - struct mtk_vpu *vpu = platform_get_drvdata(pdev);
> + struct mtk_vpu *vpu;
>   struct device *dev = >dev;
> - struct vpu_run *run = >run;
> + struct vpu_run *run;
>   const struct firmware *vpu_fw = NULL;
>   int ret;
>  
> @@ -534,6 +534,9 @@ int vpu_load_firmware(struct platform_device *pdev)
>   return -EINVAL;
>   }
>  
> + vpu = platform_get_drvdata(pdev);
> + run = >run;
> +
>   mutex_lock(>vpu_mutex);
>   if (vpu->fw_loaded) {
>   mutex_unlock(>vpu_mutex);


--
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


pull request: linux-firmware: Update Mediatek MT8173 VPU firmware

2016-09-08 Thread Andrew-CT Chen
Hi linux-firmware maintainers,

The following changes since commit c883a6b6186bb2415761d287cbac773062911212:

  rtl_bt: Add firmware and config file for RTL8822BE (2016-08-31 02:38:31 +0100)

are available in the git repository at:

  https://github.com/andrewct-chen/linux_fw_vpu_v1.0.2.git decoder_mdp

for you to fetch changes up to 5b0873b6e750f906fe4244fd173df919c66e1b73:

  mediatek: Update firmware for decoder and MDP (2016-09-08 18:58:37 +0800)


Andrew-CT Chen (1):
  mediatek: Update firmware for decoder and MDP

 vpu_d.bin |  Bin 4084848 -> 4082928 bytes
 vpu_p.bin |  Bin 131036 -> 131160 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)

--
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 v3 0/9] Add MT8173 Video Decoder Driver

2016-07-04 Thread andrew-ct chen
On Fri, 2016-07-01 at 14:00 +0200, Hans Verkuil wrote:
> On 07/01/2016 01:53 PM, andrew-ct chen wrote:
> > On Fri, 2016-07-01 at 12:11 +0200, Hans Verkuil wrote:
> >> On 06/16/2016 12:54 PM, Mauro Carvalho Chehab wrote:
> >>> Em Tue, 14 Jun 2016 19:08:08 +0800
> >>> tiffany lin <tiffany@mediatek.com> escreveu:
> >>>
> >>>> Hi Mauro,
> >>>>
> >>>>
> >>>> On Wed, 2016-06-08 at 07:13 +0900, Hans Verkuil wrote:
> >>>>>
> >>>>> On 06/07/2016 11:22 PM, Mauro Carvalho Chehab wrote:  
> >>>>>> Em Mon, 30 May 2016 20:29:14 +0800
> >>>>>> Tiffany Lin <tiffany@mediatek.com> escreveu:
> >>>>>>  
> >>>>>>> ==
> >>>>>>>   Introduction
> >>>>>>> ==
> >>>>>>>
> >>>>>>> The purpose of this series is to add the driver for video codec hw 
> >>>>>>> embedded in the Mediatek's MT8173 SoCs.
> >>>>>>> Mediatek Video Codec is able to handle video decoding of in a range 
> >>>>>>> of formats.
> >>>>>>>
> >>>>>>> This patch series add Mediatek block format V4L2_PIX_FMT_MT21, the 
> >>>>>>> decoder driver will decoded bitstream to
> >>>>>>> V4L2_PIX_FMT_MT21 format.
> >>>>>>>
> >>>>>>> This patch series rely on MTK VPU driver in patch series "Add MT8173 
> >>>>>>> Video Encoder Driver and VPU Driver"[1]
> >>>>>>> and patch "CHROMIUM: v4l: Add V4L2_PIX_FMT_VP9 definition"[2] for VP9 
> >>>>>>> support.
> >>>>>>> Mediatek Video Decoder driver rely on VPU driver to load, communicate 
> >>>>>>> with VPU.
> >>>>>>>
> >>>>>>> Internally the driver uses videobuf2 framework and MTK IOMMU and MTK 
> >>>>>>> SMI both have been merged in v4.6-rc1.
> >>>>>>>
> >>>>>>> [1]https://patchwork.linuxtv.org/patch/33734/
> >>>>>>> [2]https://chromium-review.googlesource.com/#/c/245241/  
> >>>>>>
> >>>>>> Hmm... I'm not seeing the firmware for this driver at the
> >>>>>> linux-firmware tree:
> >>>>>>
> >>>>>> https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/log/
> >>>>>>
> >>>>>> Nor I'm seeing any pull request for them. Did you send it?
> >>>>>> I'll only merge the driver upstream after seeing such pull request.  
> >>>>>   
> >>>> Sorry, I am not familiar with how to upstream firmware.
> >>>> Do you mean we need to upstream vpu firmware first before merge encoder
> >>>> driver upstream?
> >>>
> >>> Please look at this page:
> >>>   
> >>> https://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches#Firmware_submission
> >>>
> >>> The information here can also be useful:
> >>>   https://www.kernel.org/doc/readme/firmware-README.AddingFirmware
> >>>
> >>> In summary, you need to provide redistribution rights for the
> >>> firmware blob. You can either submit it to me or directly to
> >>> linux-firmware. In the latter, please c/c me on such patch.
> >>
> >> Tiffany, what is the status of the firmware submission?
> >>
> >> Regards,


> >>
> >>Hans
> > 
> > Hi Hans,
> > We are working on firmware test to make sure that both decoder and
> > encoder work well. Hopes it can be ready (firmware submission) on July 4
> > or July 5.
> 
> OK, great! Just wanted to make sure that this work was progressing and not 
> stalled.
> 
> Thanks!
> 
>   Hans

Hi Hans,

We have submitted VPU firmware for encoder as the following link.
http://lists.infradead.org/pipermail/linux-mediatek/2016-July/006082.html
If the series of decoder patches apply, we will update the VPU firmware
for decoder support.

Thanks!.

Andrew

--
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


pull request: lunux-firmware: Add Mediatek MT8173 VPU firmware

2016-07-04 Thread Andrew-CT Chen
Hi linux-firmware maintainers,

The following changes since commit 3ef7857d511ce6a91c5ce609da76c4702651cfa5:

  amdgpu: update polaris ucode (2016-06-28 14:31:11 -0400)

are available in the git repository at:

  https://github.com/andrewct-chen/vpu-linux-firmware.git vpu_encode

for you to fetch changes up to 40876d7b3c911161ab71bc84a6e90f257a13cdc4:

  mediatek: Add mt8173 VPU firmware (2016-07-04 16:26:54 +0800)


Andrew-CT Chen (1):
  mediatek: Add mt8173 VPU firmware

 WHENCE|   19 +++
 vpu_d.bin |  Bin 0 -> 4084848 bytes
 vpu_p.bin |  Bin 0 -> 131036 bytes
 3 files changed, 19 insertions(+)
 create mode 100644 vpu_d.bin
 create mode 100644 vpu_p.bin
--
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 v3 0/9] Add MT8173 Video Decoder Driver

2016-07-01 Thread andrew-ct chen
On Fri, 2016-07-01 at 12:11 +0200, Hans Verkuil wrote:
> On 06/16/2016 12:54 PM, Mauro Carvalho Chehab wrote:
> > Em Tue, 14 Jun 2016 19:08:08 +0800
> > tiffany lin  escreveu:
> > 
> >> Hi Mauro,
> >>
> >>
> >> On Wed, 2016-06-08 at 07:13 +0900, Hans Verkuil wrote:
> >>>
> >>> On 06/07/2016 11:22 PM, Mauro Carvalho Chehab wrote:  
>  Em Mon, 30 May 2016 20:29:14 +0800
>  Tiffany Lin  escreveu:
>   
> > ==
> >   Introduction
> > ==
> >
> > The purpose of this series is to add the driver for video codec hw 
> > embedded in the Mediatek's MT8173 SoCs.
> > Mediatek Video Codec is able to handle video decoding of in a range of 
> > formats.
> >
> > This patch series add Mediatek block format V4L2_PIX_FMT_MT21, the 
> > decoder driver will decoded bitstream to
> > V4L2_PIX_FMT_MT21 format.
> >
> > This patch series rely on MTK VPU driver in patch series "Add MT8173 
> > Video Encoder Driver and VPU Driver"[1]
> > and patch "CHROMIUM: v4l: Add V4L2_PIX_FMT_VP9 definition"[2] for VP9 
> > support.
> > Mediatek Video Decoder driver rely on VPU driver to load, communicate 
> > with VPU.
> >
> > Internally the driver uses videobuf2 framework and MTK IOMMU and MTK 
> > SMI both have been merged in v4.6-rc1.
> >
> > [1]https://patchwork.linuxtv.org/patch/33734/
> > [2]https://chromium-review.googlesource.com/#/c/245241/  
> 
>  Hmm... I'm not seeing the firmware for this driver at the
>  linux-firmware tree:
>   
>  https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/log/
> 
>  Nor I'm seeing any pull request for them. Did you send it?
>  I'll only merge the driver upstream after seeing such pull request.  
> >>>   
> >> Sorry, I am not familiar with how to upstream firmware.
> >> Do you mean we need to upstream vpu firmware first before merge encoder
> >> driver upstream?
> > 
> > Please look at this page:
> > 
> > https://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches#Firmware_submission
> > 
> > The information here can also be useful:
> > https://www.kernel.org/doc/readme/firmware-README.AddingFirmware
> > 
> > In summary, you need to provide redistribution rights for the
> > firmware blob. You can either submit it to me or directly to
> > linux-firmware. In the latter, please c/c me on such patch.
> 
> Tiffany, what is the status of the firmware submission?
> 
> Regards,
> 
>   Hans

Hi Hans,
We are working on firmware test to make sure that both decoder and
encoder work well. Hopes it can be ready (firmware submission) on July 4
or July 5.

Thanks.
Andrew

> 
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek


--
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] VPU: mediatek: fix platform_no_drv_owner.cocci warnings

2016-04-25 Thread andrew-ct chen
On Fri, 2016-04-22 at 15:08 +0200, Julia Lawall wrote:
> Remove .owner field if calls are used which set it automatically
> 
> Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
> 
> CC: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> Signed-off-by: Fengguang Wu <fengguang...@intel.com>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
> ---

Acked-by: Andrew-CT Chen <andrew-ct.c...@mediatek.com>

> 
> base:   git://linuxtv.org/media_tree.git master
> 
>  mtk_vpu.c |1 -
>  1 file changed, 1 deletion(-)
> 
> --- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> @@ -939,7 +939,6 @@ static struct platform_driver mtk_vpu_dr
>   .remove = mtk_vpu_remove,
>   .driver = {
>   .name   = "mtk_vpu",
> - .owner  = THIS_MODULE,
>   .of_match_table = mtk_vpu_match,
>   },
>  };


--
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] VPU: mediatek: fix simple_open.cocci warnings

2016-04-25 Thread andrew-ct chen
On Fri, 2016-04-22 at 15:12 +0200, Julia Lawall wrote:
> Remove an open coded simple_open() function
> and replace file operations references to the function
> with simple_open() instead.
> 
> Generated by: scripts/coccinelle/api/simple_open.cocci
> 
> CC: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> Signed-off-by: Fengguang Wu <fengguang...@intel.com>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
> ---

Acked-by: Andrew-CT Chen <andrew-ct.c...@mediatek.com>

> 
> I'm just passing this along.  Simple_open additionally has a check that
> inode->i_private is not NULL, before doing the assignment.  I don't know
> if that difference is important in this case.
> 
> base:   git://linuxtv.org/media_tree.git master
> 
>  mtk_vpu.c |7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> --- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> @@ -599,11 +599,6 @@ static void vpu_init_ipi_handler(void *d
>  }
> 
>  #ifdef CONFIG_DEBUG_FS
> -static int vpu_debug_open(struct inode *inode, struct file *file)
> -{
> - file->private_data = inode->i_private;
> - return 0;
> -}
> 
>  static ssize_t vpu_debug_read(struct file *file, char __user *user_buf,
> size_t count, loff_t *ppos)
> @@ -646,7 +641,7 @@ static ssize_t vpu_debug_read(struct fil
>  }
> 
>  static const struct file_operations vpu_debug_fops = {
> - .open = vpu_debug_open,
> + .open = simple_open,
>   .read = vpu_debug_read,
>  };
>  #endif /* CONFIG_DEBUG_FS */


--
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 v7 2/8] [media] VPU: mediatek: support Mediatek VPU

2016-04-25 Thread andrew-ct chen
On Mon, 2016-04-25 at 09:40 +0200, Hans Verkuil wrote:
> On 04/22/2016 06:25 AM, Tiffany Lin wrote:
> > From: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> > 
> > The VPU driver for hw video codec embedded in Mediatek's MT8173 SOCs.
> > It is able to handle video decoding/encoding of in a range of formats.
> > The driver provides with VPU firmware download, memory management and
> > the communication interface between CPU and VPU.
> > For VPU initialization, it will create virtual memory for CPU access and
> > IOMMU address for vcodec hw device access. When a decode/encode instance
> > opens a device node, vpu driver will download vpu firmware to the device.
> > A decode/encode instant will decode/encode a frame using VPU
> > interface to interrupt vpu to handle decoding/encoding jobs.
> > 
> > Signed-off-by: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> > Signed-off-by: Tiffany Lin <tiffany@mediatek.com>
> > 
> > ---
> >  drivers/media/platform/Kconfig   |   13 +
> >  drivers/media/platform/Makefile  |2 +
> >  drivers/media/platform/mtk-vpu/Makefile  |3 +
> >  drivers/media/platform/mtk-vpu/mtk_vpu.c |  950 
> > ++
> >  drivers/media/platform/mtk-vpu/mtk_vpu.h |  162 +
> >  5 files changed, 1130 insertions(+)
> >  create mode 100644 drivers/media/platform/mtk-vpu/Makefile
> >  create mode 100755 drivers/media/platform/mtk-vpu/mtk_vpu.c
> >  create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu.h
> > 
> 
> 
> > +int vpu_load_firmware(struct platform_device *pdev)
> > +{
> > +   struct mtk_vpu *vpu = platform_get_drvdata(pdev);
> > +   struct device *dev = >dev;
> > +   struct vpu_run *run = >run;
> > +   const struct firmware *vpu_fw;
> > +   int ret;
> > +
> > +   if (!pdev) {
> > +   dev_err(dev, "VPU platform device is invalid\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   ret = vpu_clock_enable(vpu);
> > +   if (ret) {
> > +   dev_err(dev, "enable clock failed %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   mutex_lock(>vpu_mutex);
> > +
> > +   if (vpu_running(vpu)) {
> > +   mutex_unlock(>vpu_mutex);
> > +   vpu_clock_disable(vpu);
> > +   dev_warn(dev, "vpu is running already\n");
> 
> This warning should be dropped. Currently vpu_load_firmware is called
> every time the video device is opened and no one else has the video device
> open. So calling this multiple times is perfectly normal and the log shouldn't
> be spammed with warnings.
> 
> I would recommend adding a fw_loaded bool to struct mtk_vpu and just
> check that at the beginning of this function and just return 0 if it is true.
> 
> Then you don't need to enable the vpu clock either.
> 
> I hope I understand the hw correctly, though.
> 
> Assuming you can do this, then this code from the v4l driver needs an
> additional comment:

We will change this in next version.

> 
> >>> + if (v4l2_fh_is_singular(>fh)) {
> 
> Add a comment here that says that vpu_load_firmware checks if it was
> loaded already and does nothing in that case.

We will change this in next version. Thanks

> 
> >>> + ret = vpu_load_firmware(dev->vpu_plat_dev);
> >>> + if (ret < 0) {
> >>> + /*
> >>> +   * Return 0 if downloading firmware successfully,
> >>> +   * otherwise it is failed
> >>> +   */
> >>> + mtk_v4l2_err("vpu_load_firmware failed!");
> >>> + goto err_load_fw;
> >>> + }
> 
> That makes it clear to the reader (i.e. me :-) ) that you can safely call
> vpu_load_firmware multiple times.
> 
> Regards,
> 
>   Hans
> 
> > +   return 0;
> > +   }
> > +
> > +   run->signaled = false;
> > +   dev_dbg(vpu->dev, "firmware request\n");
> > +   /* Downloading program firmware to device*/
> > +   ret = load_requested_vpu(vpu, vpu_fw, P_FW);
> > +   if (ret < 0) {
> > +   dev_err(dev, "Failed to request %s, %d\n", VPU_P_FW, ret);
> > +   goto OUT_LOAD_FW;
> > +   }
> > +
> > +   /* Downloading data firmware to device */
> > +   ret = load_requested_vpu(vpu, vpu_fw, D_FW);
> > +   if (ret < 0) {
> > +   dev_err(dev, "Failed to request %s, %d\n", VPU_D_FW,

Re: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU

2015-12-01 Thread andrew-ct chen
On Mon, 2015-11-30 at 15:36 +, Daniel Thompson wrote:
> On 30 November 2015 at 11:43, andrew-ct chen
> <andrew-ct.c...@mediatek.com> wrote:
> > On Fri, 2015-11-27 at 12:21 +, Daniel Thompson wrote:
> >> On 27/11/15 12:10, andrew-ct chen wrote:
> >> >>> +
> >> >>> > >+  memcpy((void *)send_obj->share_buf, buf, len);
> >> >>> > >+  send_obj->len = len;
> >> >>> > >+  send_obj->id = id;
> >> >>> > >+  vpu_cfg_writel(vpu, 0x1, HOST_TO_VPU);
> >> >>> > >+
> >> >>> > >+  /* Wait until VPU receives the command */
> >> >>> > >+  timeout = jiffies + msecs_to_jiffies(IPI_TIMEOUT_MS);
> >> >>> > >+  do {
> >> >>> > >+  if (time_after(jiffies, timeout)) {
> >> >>> > >+  dev_err(vpu->dev, "vpu_ipi_send: IPI 
> >> >>> > >timeout!\n");
> >> >>> > >+  return -EIO;
> >> >>> > >+  }
> >> >>> > >+  } while (vpu_cfg_readl(vpu, HOST_TO_VPU));
> >> >> >
> >> >> >Do we need to busy wait every time we communicate with the 
> >> >> >co-processor?
> >> >> >Couldn't we put this wait*before*  we write to HOST_TO_VPU above.
> >> >> >
> >> >> >That way we only spin when there is a need to.
> >> >> >
> >> > Since the hardware VPU only allows that one client sends the command to
> >> > it each time.
> >> > We need the wait to make sure VPU accepted the command and cleared the
> >> > interrupt and then the next command would be served.
> >>
> >> I understand that the VPU  can only have on message outstanding at once.
> >>
> >> I just wonder why we busy wait *after* sending the first command rather
> >> than *before* sending the second one.
> >
> > No other special reasons. Just send one command and wait until VPU gets
> > the command. Then, I think this wait also can be put before we write to
> > HOST_TO_VPU.Is this better than former? May I know the reason?
> 
> Busy waiting is bad; it is a waste of host CPU processor time and/or power.
> 
> When the busy wait occurs after queuing then we will busy wait for
> every command we send.
> 
> If busy wait occurs before next queuing then we will wait for a
> shorter time in total because we have the chance to do something
> useful on the host before we have to wait.
> 

Got it. Thanks a lot for the explanation.
I'll put the busy wait before next queuing in next version. 

> 
> >> Streamed decode/encode typically ends up being rate controlled by
> >> capture or display meaning that in these cases we don't need to busy
> >> wait at all (because by the time we send the next frame the VPU has
> >> already accepted the previous message).
> >
> > For now, only one device "encoder" exists, it is true.
> > But, we'll have encoder and decoder devices, the decode and encode
> > requested to VPU are simultaneous.
> 
> Sure, I accept that lock and busy-wait is an appropriate way to ensure
> safety (assuming the VPU is fairly quick clearing the HOST_TO_VPU
> flag).
> 

The busy wait time is less than 1ms in average.

> 
> > Is this supposed to be removed for this patches and we can add it back
> > if the another device(decoder) is ready for review?
> 
> No I'm not suggesting that.
> 
> I only recommend moving the busy wait *before* end sending of command
> (for efficiency reasons mentioned above).

Ok. Thanks.

> 
> 
> Daniel.


--
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: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU

2015-11-30 Thread andrew-ct chen
On Fri, 2015-11-27 at 12:21 +, Daniel Thompson wrote:
> On 27/11/15 12:10, andrew-ct chen wrote:
> >>> +
> >>> > >+  memcpy((void *)send_obj->share_buf, buf, len);
> >>> > >+  send_obj->len = len;
> >>> > >+  send_obj->id = id;
> >>> > >+  vpu_cfg_writel(vpu, 0x1, HOST_TO_VPU);
> >>> > >+
> >>> > >+  /* Wait until VPU receives the command */
> >>> > >+  timeout = jiffies + msecs_to_jiffies(IPI_TIMEOUT_MS);
> >>> > >+  do {
> >>> > >+  if (time_after(jiffies, timeout)) {
> >>> > >+  dev_err(vpu->dev, "vpu_ipi_send: IPI 
> >>> > >timeout!\n");
> >>> > >+  return -EIO;
> >>> > >+  }
> >>> > >+  } while (vpu_cfg_readl(vpu, HOST_TO_VPU));
> >> >
> >> >Do we need to busy wait every time we communicate with the co-processor?
> >> >Couldn't we put this wait*before*  we write to HOST_TO_VPU above.
> >> >
> >> >That way we only spin when there is a need to.
> >> >
> > Since the hardware VPU only allows that one client sends the command to
> > it each time.
> > We need the wait to make sure VPU accepted the command and cleared the
> > interrupt and then the next command would be served.
> 
> I understand that the VPU  can only have on message outstanding at once.
> 
> I just wonder why we busy wait *after* sending the first command rather 
> than *before* sending the second one.

No other special reasons. Just send one command and wait until VPU gets
the command. Then, I think this wait also can be put before we write to
HOST_TO_VPU.Is this better than former? May I know the reason?


> 
> Streamed decode/encode typically ends up being rate controlled by 
> capture or display meaning that in these cases we don't need to busy 
> wait at all (because by the time we send the next frame the VPU has 
> already accepted the previous message).

For now, only one device "encoder" exists, it is true.
But, we'll have encoder and decoder devices, the decode and encode
requested to VPU are simultaneous.
Is this supposed to be removed for this patches and we can add it back
if the another device(decoder) is ready for review?


Andrew


> 
> 
> Daniel.
> 
> 
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek


--
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: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU

2015-11-27 Thread andrew-ct chen
Thanks a lot for your review comments.

On Wed, 2015-11-25 at 16:11 +, Daniel Thompson wrote:
> On 17/11/15 12:54, Tiffany Lin wrote:
> > From: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> >
> > The VPU driver for hw video codec embedded in Mediatek's MT8173 SOCs.
> > It is able to handle video decoding/encoding of in a range of formats.
> > The driver provides with VPU firmware download, memory management and
> > the communication interface between CPU and VPU.
> > For VPU initialization, it will create virtual memory for CPU access and
> > IOMMU address for vcodec hw device access. When a decode/encode instance
> > opens a device node, vpu driver will download vpu firmware to the device.
> > A decode/encode instant will decode/encode a frame using VPU
> > interface to interrupt vpu to handle decoding/encoding jobs.
> >
> > Signed-off-by: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> > ---
> >   drivers/media/platform/Kconfig |6 +
> >   drivers/media/platform/Makefile|2 +
> >   drivers/media/platform/mtk-vpu/Makefile|1 +
> >   .../platform/mtk-vpu/h264_enc/venc_h264_vpu.h  |  127 +++
> >   .../media/platform/mtk-vpu/include/venc_ipi_msg.h  |  212 +
> >   drivers/media/platform/mtk-vpu/mtk_vpu_core.c  |  823 
> > 
> >   drivers/media/platform/mtk-vpu/mtk_vpu_core.h  |  161 
> >   .../media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h  |  119 +++
> >   8 files changed, 1451 insertions(+)
> >   create mode 100644 drivers/media/platform/mtk-vpu/Makefile
> >   create mode 100644 drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h
> >   create mode 100644 drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h
> >   create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.c
> >   create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.h
> >   create mode 100644 drivers/media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h
> >
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index ccbc974..f98eb47 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -148,6 +148,12 @@ config VIDEO_CODA
> >Coda is a range of video codec IPs that supports
> >H.264, MPEG-4, and other video formats.
> >
> > +config MEDIATEK_VPU
> > +   bool "Mediatek Video Processor Unit"
> > +   ---help---
> > +   This driver provides downloading firmware vpu and
> > +   communicating with vpu.
> > +
> 
> This looks pretty broken.
> 
> Shouldn't this option be tristate? Why are there no depends-on or selects?
> 

Yes, this should be tristate and depends on VIDEO_DEV && VIDEO_V4L2 &&
ARCH_MEDIATEK

> Also I think the help text could be more descriptive here (and so does 
> checkpatch ;-) ).
> 

I'll put more descriptive. Thanks.

> 
> > diff --git a/drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h 
> > b/drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h
> > new file mode 100644
> > index 000..9c8ebdd
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h
> 
> Why is this file included in *this* patch? It is not included by 
> anything in the patch and defines functions that do not exist yet.

I'll move this to the h264 patch.Thanks

> 
> 
> > diff --git a/drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h 
> > b/drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h
> > new file mode 100644
> > index 000..a345b98
> 
> This file also is not included by anything and should, perhaps be 
> included in a different patch.
> 

I'll move this to the v4l2 encoder patch.

> 
> > diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu_core.c 
> > b/drivers/media/platform/mtk-vpu/mtk_vpu_core.c
> > new file mode 100644
> > index 000..b524dbc
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vpu/mtk_vpu_core.c
> > @@ -0,0 +1,823 @@
> > +/*
> > +* Copyright (c) 2015 MediaTek Inc.
> > +* Author: Andrew-CT Chen <andrew-ct.c...@mediatek.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.
> > +*
> > +* This program is distributed in the hope that it will be useful,
> > +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
&g

Re: [RESEND RFC/PATCH 1/8] dt-bindings: Add a binding for Mediatek Video Processor Unit

2015-11-18 Thread andrew-ct chen
On Tue, 2015-11-17 at 14:13 +, Mark Rutland wrote:
> On Tue, Nov 17, 2015 at 08:54:38PM +0800, Tiffany Lin wrote:
> > From: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> > 
> > Add a DT binding documentation of Video Processor Unit for the
> > MT8173 SoC from Mediatek.
> > 
> > Signed-off-by: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> > ---
> >  .../devicetree/bindings/media/mediatek-vpu.txt |   27 
> > 
> >  1 file changed, 27 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/mediatek-vpu.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-vpu.txt 
> > b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
> > new file mode 100644
> > index 000..99a4e5e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
> > @@ -0,0 +1,27 @@
> > +* Mediatek Video Processor Unit
> > +
> > +Video Processor Unit is a HW video controller. It controls HW Codec 
> > including
> > +H.264/VP8/VP9 Decode, H.264/VP8 Encode and Image Processor 
> > (scale/rotate/color convert).
> > +
> > +Required properties:
> > +  - compatible: "mediatek,mt8173-vpu"
> > +  - reg: Must contain an entry for each entry in reg-names.
> > +  - reg-names: Must include the following entries:
> > +"sram": SRAM base
> > +"cfg_reg": Main configuration registers base
> > +  - interrupts: interrupt number to the cpu.
> > +  - clocks : clock name from clock manager
> > +  - clock-names: the clocks of the VPU H/W
> 
> You need to explicitly define the set of clock-names you expect here.
> 
> Mark.

Sorry, only one clock to enable VPU hardware.
We will modify to
- clocks : must contain one entry for each clock-names.
- clock-names : must be "main", It is the main clock of VPU.

Thanks..

> 
> > +  - iommus : phandle and IOMMU spcifier for the IOMMU that serves the VPU.
> > +
> > +Example:
> > +   vpu: vpu@1002 {
> > +   compatible = "mediatek,mt8173-vpu";
> > +   reg = <0 0x1002 0 0x3>,
> > + <0 0x1005 0 0x100>;
> > +   reg-names = "sram", "cfg_reg";
> > +   interrupts = ;
> > +   clocks = < TOP_SCP_SEL>;
> > +   clock-names = "main";
> > +   iommus = < M4U_PORT_VENC_RCPU>;
> > +   };
> > -- 
> > 1.7.9.5
> > 


--
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