Re: [PATCH 4/4] venus: firmware: register separate platform_device for firmware loader

[Vikash Garodia]

Hi Stanimir,

On 2018-08-22 18:07, Stanimir Varbanov wrote:

Hi Vikash,

Could you give a try below patch on your environment? You should keep
your 1/4 to 3/4 patches and replace your 4/4 with the below one.

You have to drop the compatible string in firmware DT subnode (keep 
only

iommus).


I have successfully tested this patch on my environment without tz. I 
have

post a new series by including below patch. Thank you Stanimir for your
review and below patch.


On 08/22/2018 03:34 PM, Stanimir Varbanov wrote:

This registers a firmware platform_device and associate it with
video-firmware DT subnode. Then calls dma configure to initialize
dma and iommu.

Signed-off-by: Stanimir Varbanov 
---
 .../devicetree/bindings/media/qcom,venus.txt   | 13 +-
 drivers/media/platform/qcom/venus/core.c   | 14 +--
 drivers/media/platform/qcom/venus/firmware.c   | 49 
++

 drivers/media/platform/qcom/venus/firmware.h   |  2 +
 4 files changed, 73 insertions(+), 5 deletions(-)

qcom: update firmware file for Venus on SDM845

[Vikash Garodia]

hi,

This pull request updates firmware files for Venus h/w codec found on the 
Qualcomm SDM845 chipset.

The following changes since commit 7b5835fd37630d18ac0c755329172f6a17c1af29:

  linux-firmware: add firmware for mt76x2u (2018-07-30 07:20:31 -0400)

are available in the git repository at:

  https://github.com/vgarodia/venus_firmware_23 master

for you to fetch changes up to 6ae7a5bf57f035aecc7613943528e52ada7e1e03:

  qcom: update venus firmware files for v5.2 (2018-08-10 12:57:47 +0530)


Vikash Garodia (1):
  qcom: update venus firmware files for v5.2

 WHENCE   |   2 +-
 qcom/venus-5.2/venus.b00 | Bin 212 -> 212 bytes
 qcom/venus-5.2/venus.b01 | Bin 6600 -> 6600 bytes
 qcom/venus-5.2/venus.b02 | Bin 819552 -> 837304 bytes
 qcom/venus-5.2/venus.b03 | Bin 33536 -> 33640 bytes
 qcom/venus-5.2/venus.mbn | Bin 865408 -> 883264 bytes
 qcom/venus-5.2/venus.mdt | Bin 6812 -> 6812 bytes
 7 files changed, 1 insertion(+), 1 deletion(-)

Re: qcom: add firmware file for Venus on SDM845

[Vikash Garodia]

Hi Josh,

On 2018-05-25 17:34, Josh Boyer wrote:
On Fri, May 25, 2018 at 7:03 AM Vikash Garodia 
<vgaro...@codeaurora.org>

wrote:


This pull request adds firmware files for Venus h/w codec found on the

Qualcomm SDM845 chipset.


The following changes since commit

2a9b2cf50fb32e36e4fc1586c2f6f1421913b553:


   Merge branch 'for-upstreaming-v1.7.2' of
https://github.com/felix-cavium/linux-firmware (2018-05-18 08:35:22 
-0400)



are available in the git repository at:




   https://github.com/vgarodia/linux-firmware master


for you to fetch changes up to 
d6088b9c9d7f49d3c6c43681190889eca0abdcce:



   qcom: add venus firmware files for v5.2 (2018-05-25 15:16:43 +0530)



----
Vikash Garodia (1):
   qcom: add venus firmware files for v5.2



  WHENCE   |   9 +
  qcom/venus-5.2/venus.b00 | Bin 0 -> 212 bytes
  qcom/venus-5.2/venus.b01 | Bin 0 -> 6600 bytes
  qcom/venus-5.2/venus.b02 | Bin 0 -> 819552 bytes
  qcom/venus-5.2/venus.b03 | Bin 0 -> 33536 bytes
  qcom/venus-5.2/venus.b04 |   1 +
  qcom/venus-5.2/venus.mbn | Bin 0 -> 865408 bytes
  qcom/venus-5.2/venus.mdt | Bin 0 -> 6812 bytes
  8 files changed, 10 insertions(+)
  create mode 100644 qcom/venus-5.2/venus.b00
  create mode 100644 qcom/venus-5.2/venus.b01
  create mode 100644 qcom/venus-5.2/venus.b02
  create mode 100644 qcom/venus-5.2/venus.b03
  create mode 100644 qcom/venus-5.2/venus.b04
  create mode 100644 qcom/venus-5.2/venus.mbn
  create mode 100644 qcom/venus-5.2/venus.mdt


The venus.mbn file isn't mentioned in WHENCE:

[jwboyer@vader linux-firmware]$ ./check_whence.py
E: qcom/venus-5.2/venus.mbn not listed in WHENCE
[jwboyer@vader linux-firmware]$

Can you fix that up and let me know when to re-pull?
I have fixed the error and is ready to be re-pulled from the same git 
repository.
I have noted the process to run check_whence.py before uploading the 
firmwares.


Do i need to resend the pull request as the end commit is now changed to
7602644358157e4b89960472b6d59ffcc040ca52 ?

-Vikash

qcom: add firmware file for Venus on SDM845

[Vikash Garodia]

This pull request adds firmware files for Venus h/w codec found on the Qualcomm 
SDM845 chipset.

The following changes since commit 2a9b2cf50fb32e36e4fc1586c2f6f1421913b553:

  Merge branch 'for-upstreaming-v1.7.2' of 
https://github.com/felix-cavium/linux-firmware (2018-05-18 08:35:22 -0400)

are available in the git repository at:


  https://github.com/vgarodia/linux-firmware master

for you to fetch changes up to d6088b9c9d7f49d3c6c43681190889eca0abdcce:

  qcom: add venus firmware files for v5.2 (2018-05-25 15:16:43 +0530)


Vikash Garodia (1):
  qcom: add venus firmware files for v5.2

 WHENCE   |   9 +
 qcom/venus-5.2/venus.b00 | Bin 0 -> 212 bytes
 qcom/venus-5.2/venus.b01 | Bin 0 -> 6600 bytes
 qcom/venus-5.2/venus.b02 | Bin 0 -> 819552 bytes
 qcom/venus-5.2/venus.b03 | Bin 0 -> 33536 bytes
 qcom/venus-5.2/venus.b04 |   1 +
 qcom/venus-5.2/venus.mbn | Bin 0 -> 865408 bytes
 qcom/venus-5.2/venus.mdt | Bin 0 -> 6812 bytes
 8 files changed, 10 insertions(+)
 create mode 100644 qcom/venus-5.2/venus.b00
 create mode 100644 qcom/venus-5.2/venus.b01
 create mode 100644 qcom/venus-5.2/venus.b02
 create mode 100644 qcom/venus-5.2/venus.b03
 create mode 100644 qcom/venus-5.2/venus.b04
 create mode 100644 qcom/venus-5.2/venus.mbn
 create mode 100644 qcom/venus-5.2/venus.mdt

qcom: add firmware file for Venus on SDM845

[Vikash Garodia]

hi,

This pull request adds firmware files for Venus h/w codec found on the Qualcomm 
SDM845 chipset.

The following changes since commit 2a9b2cf50fb32e36e4fc1586c2f6f1421913b553:

  Merge branch 'for-upstreaming-v1.7.2' of 
https://github.com/felix-cavium/linux-firmware (2018-05-18 08:35:22 -0400)

are available in the git repository at:


  https://github.com/vgarodia/linux-firmware master

for you to fetch changes up to d6088b9c9d7f49d3c6c43681190889eca0abdcce:

  qcom: add venus firmware files for v5.2 (2018-05-25 15:16:43 +0530)


Vikash Garodia (1):
  qcom: add venus firmware files for v5.2

 WHENCE   |   9 +
 qcom/venus-5.2/venus.b00 | Bin 0 -> 212 bytes
 qcom/venus-5.2/venus.b01 | Bin 0 -> 6600 bytes
 qcom/venus-5.2/venus.b02 | Bin 0 -> 819552 bytes
 qcom/venus-5.2/venus.b03 | Bin 0 -> 33536 bytes
 qcom/venus-5.2/venus.b04 |   1 +
 qcom/venus-5.2/venus.mbn | Bin 0 -> 865408 bytes
 qcom/venus-5.2/venus.mdt | Bin 0 -> 6812 bytes
 8 files changed, 10 insertions(+)
 create mode 100644 qcom/venus-5.2/venus.b00
 create mode 100644 qcom/venus-5.2/venus.b01
 create mode 100644 qcom/venus-5.2/venus.b02
 create mode 100644 qcom/venus-5.2/venus.b03
 create mode 100644 qcom/venus-5.2/venus.b04
 create mode 100644 qcom/venus-5.2/venus.mbn
 create mode 100644 qcom/venus-5.2/venus.mdt

Re: [PATCH 3/4] venus: add check to make scm calls

[Vikash Garodia]

Hi Stan,

On 2018-05-23 02:27, Stanimir Varbanov wrote:

Hi Jordan,

On 22.05.2018 22:50, Jordan Crouse wrote:

On Tue, May 22, 2018 at 04:04:51PM +0300, Stanimir Varbanov wrote:

Hi Vikash,

On 05/17/2018 02:32 PM, Vikash Garodia wrote:

In order to invoke scm calls, ensure that the platform
has the required support to invoke the scm calls in
secure world. This code is in preparation to add PIL
functionality in venus driver.

Signed-off-by: Vikash Garodia <vgaro...@codeaurora.org>
---
  drivers/media/platform/qcom/venus/hfi_venus.c | 26 
+++---

  1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c 
b/drivers/media/platform/qcom/venus/hfi_venus.c

index f61d34b..9bcce94 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -27,6 +27,7 @@
  #include "hfi_msgs.h"
  #include "hfi_venus.h"
  #include "hfi_venus_io.h"
+#include "firmware.h"
#define HFI_MASK_QHDR_TX_TYPE   0xff00
  #define HFI_MASK_QHDR_RX_TYPE 0x00ff
@@ -570,13 +571,19 @@ static int venus_halt_axi(struct 
venus_hfi_device *hdev)

  static int venus_power_off(struct venus_hfi_device *hdev)
  {
int ret;
+   void __iomem *reg_base;
if (!hdev->power_enabled)
return 0;
  - ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
-   if (ret)
-   return ret;
+   if (qcom_scm_is_available()) {
+   ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);


I think it will be clearer if we abstract qcom_scm_set_remote_state 
to
something like venus_set_state(SUSPEND|RESUME) in firmware.c and 
export

the functions to be used here.


This specific function is a little odd because the SCM function got 
overloaded

and used as a hardware workaround for the adreno a5xx zap shader.

When we added it for the GPU we knew the day would come that we would 
need it
for Venus so we kept the name purposely generic. You can wrap if if 
you want
but just know that there are other non video entities out there using 
it.


Sorry I wasn't clear, by abstract it I meant to introduce a new
venus_set_state function in venus/firmware.c where we'll select
tz/non-tz functions for suspend / resume depending on the
configuration.


Yes, that's a good idea to abstract the decision to use tz or non-tz way 
as much

as possible to firmware.c. Will add this in my next patch.


regards,
Stan

Re: [PATCH 4/4] media: venus: add PIL support

[Vikash Garodia]

Hi Trilok,

On 2018-05-18 06:10, Trilok Soni wrote:

Hi Vikash,

On 5/17/2018 4:32 AM, Vikash Garodia wrote:

This adds support to load the video firmware
and bring ARM9 out of reset. This is useful
for platforms which does not have trustzone
to reset the ARM9.


ARM9 = video core here? May be commit text needs little bit more 
detail.

Yes, ARM9 here refers to the CPU running the firmware inside video core.
I can add some more detail on the same.


  +static int store_firmware_dev(struct device *dev, void *data)
+{
+   struct venus_core *core;
+
+   core = (struct venus_core *)data;


No need of casting.


Ok. Will remove the casting.




+   if (!core)
+   return -EINVAL;
+
+   if (of_device_is_compatible(dev->of_node, "qcom,venus-pil-no-tz"))
+   core->fw.dev = dev;
+
+   return 0;
+}
+




  - ret = venus_boot(dev, core->res->fwname);
+   ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+   if (ret)
+   goto err_runtime_disable;
+
+   /* Attempt to register child devices */
+   ret = device_for_each_child(dev, core, store_firmware_dev);
+


and not ret check needed?


Not needed. The fn (store_firmware_dev) just stores the child device 
pointer.
Later in the driver, if the child device pointer is not populated, probe 
is
deferred. Again, child device for which this populate is added, is an 
optional

child node.


+   ret = venus_boot(core);
if (ret)
goto err_runtime_disable;

Re: [PATCH 1/4] soc: qcom: mdt_loader: Add check to make scm calls

[Vikash Garodia]

Hi Bjorn,

On 2018-05-18 10:58, Bjorn Andersson wrote:

On Thu 17 May 04:32 PDT 2018, Vikash Garodia wrote:


In order to invoke scm calls, ensure that the platform
has the required support to invoke the scm calls in
secure world.

Signed-off-by: Vikash Garodia <vgaro...@codeaurora.org>
---
 drivers/soc/qcom/mdt_loader.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/qcom/mdt_loader.c 
b/drivers/soc/qcom/mdt_loader.c

index 17b314d..db55d53 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -121,10 +121,12 @@ int qcom_mdt_load(struct device *dev, const 
struct firmware *fw,

if (!fw_name)
return -ENOMEM;

-   ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
-   if (ret) {
-   dev_err(dev, "invalid firmware metadata\n");
-   goto out;
+   if (qcom_scm_is_available()) {


qcom_scm_is_available() tells you if the qcom_scm driver has been
probed, not if your platform implements PAS.

Please add a DT property to tell the driver if it should require PAS or
not (the absence of such property should indicate PAS is required, for
backwards compatibility purposes). For the MDT loader we need to merge
the following patch to make this work:

https://patchwork.kernel.org/patch/10397889/


Thanks for your inputs. I have already added a child node in video DT 
node

to tell the driver if PAS is not needed.
I will drop this patch as use 
https://patchwork.kernel.org/patch/10397889

and update the driver to call new api qcom_mdt_load_no_init.


Regards,
Bjorn

[PATCH 4/4] media: venus: add PIL support

[Vikash Garodia]

This adds support to load the video firmware
and bring ARM9 out of reset. This is useful
for platforms which does not have trustzone
to reset the ARM9.

Signed-off-by: Vikash Garodia <vgaro...@codeaurora.org>
---
 .../devicetree/bindings/media/qcom,venus.txt   |   8 +-
 drivers/media/platform/qcom/venus/core.c   |  67 +++--
 drivers/media/platform/qcom/venus/core.h   |   6 +
 drivers/media/platform/qcom/venus/firmware.c   | 163 +
 drivers/media/platform/qcom/venus/firmware.h   |  10 +-
 5 files changed, 217 insertions(+), 37 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/qcom,venus.txt 
b/Documentation/devicetree/bindings/media/qcom,venus.txt
index 00d0d1b..0ff0f2d 100644
--- a/Documentation/devicetree/bindings/media/qcom,venus.txt
+++ b/Documentation/devicetree/bindings/media/qcom,venus.txt
@@ -53,7 +53,7 @@
 
 * Subnodes
 The Venus video-codec node must contain two subnodes representing
-video-decoder and video-encoder.
+video-decoder and video-encoder, one optional firmware subnode.
 
 Every of video-encoder or video-decoder subnode should have:
 
@@ -79,6 +79,8 @@ Every of video-encoder or video-decoder subnode should have:
power domain which is responsible for collapsing
and restoring power to the subcore.
 
+The firmware sub node must contain the iommus specifiers for ARM9.
+
 * An Example
video-codec@1d0 {
compatible = "qcom,msm8916-venus";
@@ -105,4 +107,8 @@ Every of video-encoder or video-decoder subnode should have:
clock-names = "core";
power-domains = < VENUS_CORE1_GDSC>;
};
+   firmware {
+   compatible = "qcom,venus-pil-no-tz";
+   iommus = <_smmu 0x10b2 0x0>;
+   }
};
diff --git a/drivers/media/platform/qcom/venus/core.c 
b/drivers/media/platform/qcom/venus/core.c
index 1308488..16910558 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -30,6 +31,7 @@
 #include "vdec.h"
 #include "venc.h"
 #include "firmware.h"
+#include "hfi_venus.h"
 
 static void venus_event_notify(struct venus_core *core, u32 event)
 {
@@ -76,7 +78,7 @@ static void venus_sys_error_handler(struct work_struct *work)
hfi_core_deinit(core, true);
hfi_destroy(core);
mutex_lock(>lock);
-   venus_shutdown(core->dev);
+   venus_shutdown(core);
 
pm_runtime_put_sync(core->dev);
 
@@ -84,7 +86,7 @@ static void venus_sys_error_handler(struct work_struct *work)
 
pm_runtime_get_sync(core->dev);
 
-   ret |= venus_boot(core->dev, core->res->fwname);
+   ret |= venus_boot(core);
 
ret |= hfi_core_resume(core, true);
 
@@ -179,6 +181,20 @@ static u32 to_v4l2_codec_type(u32 codec)
}
 }
 
+static int store_firmware_dev(struct device *dev, void *data)
+{
+   struct venus_core *core;
+
+   core = (struct venus_core *)data;
+   if (!core)
+   return -EINVAL;
+
+   if (of_device_is_compatible(dev->of_node, "qcom,venus-pil-no-tz"))
+   core->fw.dev = dev;
+
+   return 0;
+}
+
 static int venus_enumerate_codecs(struct venus_core *core, u32 type)
 {
const struct hfi_inst_ops dummy_ops = {};
@@ -229,6 +245,7 @@ static int venus_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct venus_core *core;
struct resource *r;
+   struct iommu_domain *iommu_domain;
int ret;
 
core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
@@ -279,7 +296,14 @@ static int venus_probe(struct platform_device *pdev)
if (ret < 0)
goto err_runtime_disable;
 
-   ret = venus_boot(dev, core->res->fwname);
+   ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+   if (ret)
+   goto err_runtime_disable;
+
+   /* Attempt to register child devices */
+   ret = device_for_each_child(dev, core, store_firmware_dev);
+
+   ret = venus_boot(core);
if (ret)
goto err_runtime_disable;
 
@@ -303,14 +327,17 @@ static int venus_probe(struct platform_device *pdev)
if (ret)
goto err_core_deinit;
 
-   ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+   ret = pm_runtime_put_sync(dev);
if (ret)
goto err_dev_unregister;
 
-   ret = pm_runtime_put_sync(dev);
-   if (ret)
+   iommu_domain = iommu_get_domain_for_dev(dev);
+   if (!iommu_domain)
goto err_dev_unregister;
 
+   iommu_domain->geometry.aperture_start = VENUS_FW_MEM_SIZE;
+   iommu_domain->geome

[PATCH 1/4] soc: qcom: mdt_loader: Add check to make scm calls

[Vikash Garodia]

In order to invoke scm calls, ensure that the platform
has the required support to invoke the scm calls in
secure world.

Signed-off-by: Vikash Garodia <vgaro...@codeaurora.org>
---
 drivers/soc/qcom/mdt_loader.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index 17b314d..db55d53 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -121,10 +121,12 @@ int qcom_mdt_load(struct device *dev, const struct 
firmware *fw,
if (!fw_name)
return -ENOMEM;
 
-   ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
-   if (ret) {
-   dev_err(dev, "invalid firmware metadata\n");
-   goto out;
+   if (qcom_scm_is_available()) {
+   ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
+   if (ret) {
+   dev_err(dev, "invalid firmware metadata\n");
+   goto out;
+   }
}
 
for (i = 0; i < ehdr->e_phnum; i++) {
@@ -144,10 +146,13 @@ int qcom_mdt_load(struct device *dev, const struct 
firmware *fw,
}
 
if (relocate) {
-   ret = qcom_scm_pas_mem_setup(pas_id, mem_phys, max_addr - 
min_addr);
-   if (ret) {
-   dev_err(dev, "unable to setup relocation\n");
-   goto out;
+   if (qcom_scm_is_available()) {
+   ret = qcom_scm_pas_mem_setup(pas_id, mem_phys,
+   max_addr - min_addr);
+   if (ret) {
+   dev_err(dev, "unable to setup relocation\n");
+   goto out;
+   }
}
 
/*
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/4] media: venus: add a routine to reset ARM9

[Vikash Garodia]

Add a new routine to reset the ARM9 and brings it
out of reset. This is in preparation to add PIL
functionality in venus driver.

Signed-off-by: Vikash Garodia <vgaro...@codeaurora.org>
---
 drivers/media/platform/qcom/venus/firmware.c | 26 
 drivers/media/platform/qcom/venus/firmware.h |  1 +
 drivers/media/platform/qcom/venus/hfi_venus_io.h |  5 +
 3 files changed, 32 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/firmware.c 
b/drivers/media/platform/qcom/venus/firmware.c
index c4a5778..8f25375 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -22,11 +23,36 @@
 #include 
 #include 
 
+#include "core.h"
 #include "firmware.h"
+#include "hfi_venus_io.h"
 
 #define VENUS_PAS_ID   9
 #define VENUS_FW_MEM_SIZE  (6 * SZ_1M)
 
+void venus_reset_hw(struct venus_core *core)
+{
+   void __iomem *reg_base = core->base;
+
+   writel(0, reg_base + WRAPPER_FW_START_ADDR);
+   writel(VENUS_FW_MEM_SIZE, reg_base + WRAPPER_FW_END_ADDR);
+   writel(0, reg_base + WRAPPER_CPA_START_ADDR);
+   writel(VENUS_FW_MEM_SIZE, reg_base + WRAPPER_CPA_END_ADDR);
+   writel(0x0, reg_base + WRAPPER_CPU_CGC_DIS);
+   writel(0x0, reg_base + WRAPPER_CPU_CLOCK_CONFIG);
+
+   /* Make sure all register writes are committed. */
+   mb();
+
+   /*
+* Need to wait 10 cycles of internal clocks before bringing ARM9
+* out of reset.
+*/
+   udelay(1);
+
+   /* Bring Arm9 out of reset */
+   writel_relaxed(0, reg_base + WRAPPER_A9SS_SW_RESET);
+}
 int venus_boot(struct device *dev, const char *fwname)
 {
const struct firmware *mdt;
diff --git a/drivers/media/platform/qcom/venus/firmware.h 
b/drivers/media/platform/qcom/venus/firmware.h
index 428efb5..d56f5b2 100644
--- a/drivers/media/platform/qcom/venus/firmware.h
+++ b/drivers/media/platform/qcom/venus/firmware.h
@@ -18,5 +18,6 @@
 
 int venus_boot(struct device *dev, const char *fwname);
 int venus_shutdown(struct device *dev);
+void venus_reset_hw(struct venus_core *core);
 
 #endif
diff --git a/drivers/media/platform/qcom/venus/hfi_venus_io.h 
b/drivers/media/platform/qcom/venus/hfi_venus_io.h
index 76f4793..39afa5d 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus_io.h
+++ b/drivers/media/platform/qcom/venus/hfi_venus_io.h
@@ -109,6 +109,11 @@
 #define WRAPPER_CPU_CGC_DIS(WRAPPER_BASE + 0x2010)
 #define WRAPPER_CPU_STATUS (WRAPPER_BASE + 0x2014)
 #define WRAPPER_SW_RESET   (WRAPPER_BASE + 0x3000)
+#define WRAPPER_CPA_START_ADDR (WRAPPER_BASE + 0x1020)
+#define WRAPPER_CPA_END_ADDR   (WRAPPER_BASE + 0x1024)
+#define WRAPPER_FW_START_ADDR  (WRAPPER_BASE + 0x1028)
+#define WRAPPER_FW_END_ADDR(WRAPPER_BASE + 0x102C)
+#define WRAPPER_A9SS_SW_RESET  (WRAPPER_BASE + 0x3000)
 
 /* Venus 4xx */
 #define WRAPPER_VCODEC0_MMCC_POWER_STATUS  (WRAPPER_BASE + 0x90)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 3/4] venus: add check to make scm calls

[Vikash Garodia]

In order to invoke scm calls, ensure that the platform
has the required support to invoke the scm calls in
secure world. This code is in preparation to add PIL
functionality in venus driver.

Signed-off-by: Vikash Garodia <vgaro...@codeaurora.org>
---
 drivers/media/platform/qcom/venus/hfi_venus.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c 
b/drivers/media/platform/qcom/venus/hfi_venus.c
index f61d34b..9bcce94 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -27,6 +27,7 @@
 #include "hfi_msgs.h"
 #include "hfi_venus.h"
 #include "hfi_venus_io.h"
+#include "firmware.h"
 
 #define HFI_MASK_QHDR_TX_TYPE  0xff00
 #define HFI_MASK_QHDR_RX_TYPE  0x00ff
@@ -570,13 +571,19 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
 static int venus_power_off(struct venus_hfi_device *hdev)
 {
int ret;
+   void __iomem *reg_base;
 
if (!hdev->power_enabled)
return 0;
 
-   ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
-   if (ret)
-   return ret;
+   if (qcom_scm_is_available()) {
+   ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
+   if (ret)
+   return ret;
+   } else {
+   reg_base = hdev->core->base;
+   writel_relaxed(1, reg_base + WRAPPER_A9SS_SW_RESET);
+   }
 
ret = venus_halt_axi(hdev);
if (ret)
@@ -594,9 +601,13 @@ static int venus_power_on(struct venus_hfi_device *hdev)
if (hdev->power_enabled)
return 0;
 
-   ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_RESUME, 0);
-   if (ret)
-   goto err;
+   if (qcom_scm_is_available()) {
+   ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_RESUME, 0);
+   if (ret)
+   goto err;
+   } else {
+   venus_reset_hw(hdev->core);
+   }
 
ret = venus_run(hdev);
if (ret)
@@ -607,7 +618,8 @@ static int venus_power_on(struct venus_hfi_device *hdev)
return 0;
 
 err_suspend:
-   qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
+   if (qcom_scm_is_available())
+   qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
 err:
hdev->power_enabled = false;
return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 0/4] Venus updates - PIL

[Vikash Garodia]

Hello,

The patch set mainly adds PIL functionality in video driver. 
There are boards with qcom video hardware but does not have
trustzone. For such boards, the PIL added now will load the
video firmware and reset the ARM9.

The patch set is based on top of recent venus updates v2 patches
posted by Stanimir Varbanov.

Comments are welcome!

regards,
Vikash

Vikash Garodia (4):
  soc: qcom: mdt_loader: Add check to make scm calls
  media: venus: add a routine to reset ARM9
  venus: add check to make scm calls
  media: venus: add PIL support

 .../devicetree/bindings/media/qcom,venus.txt   |   8 +-
 drivers/media/platform/qcom/venus/core.c   |  67 +++-
 drivers/media/platform/qcom/venus/core.h   |   6 +
 drivers/media/platform/qcom/venus/firmware.c   | 189 ++---
 drivers/media/platform/qcom/venus/firmware.h   |  11 +-
 drivers/media/platform/qcom/venus/hfi_venus.c  |  26 ++-
 drivers/media/platform/qcom/venus/hfi_venus_io.h   |   5 +
 drivers/soc/qcom/mdt_loader.c  |  21 ++-
 8 files changed, 281 insertions(+), 52 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 08/28] venus: hfi_venus: add suspend function for 4xx version

[Vikash Garodia]

Hi Stanimir,

On 2018-05-09 16:45, Stanimir Varbanov wrote:

Hi Vikash,

On 05/02/2018 09:07 AM, vgaro...@codeaurora.org wrote:

Hello Stanimir,

On 2018-04-24 18:14, Stanimir Varbanov wrote:

This adds suspend (power collapse) function with slightly
different order of calls comparing with Venus 3xx.

Signed-off-by: Stanimir Varbanov 
---
 drivers/media/platform/qcom/venus/hfi_venus.c | 52
+++
 1 file changed, 52 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
b/drivers/media/platform/qcom/venus/hfi_venus.c
index 53546174aab8..f61d34bf61b4 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -1443,6 +1443,55 @@ static int venus_suspend_1xx(struct venus_core
*core)
 return 0;
 }

+static int venus_suspend_4xx(struct venus_core *core)
+{
+    struct venus_hfi_device *hdev = to_hfi_priv(core);
+    struct device *dev = core->dev;
+    u32 val;
+    int ret;
+
+    if (!hdev->power_enabled || hdev->suspended)
+    return 0;
+
+    mutex_lock(>lock);
+    ret = venus_is_valid_state(hdev);
+    mutex_unlock(>lock);
+
+    if (!ret) {
+    dev_err(dev, "bad state, cannot suspend\n");
+    return -EINVAL;
+    }
+
+    ret = venus_prepare_power_collapse(hdev, false);
+    if (ret) {
+    dev_err(dev, "prepare for power collapse fail (%d)\n", ret);
+    return ret;
+    }
+
+    ret = readl_poll_timeout(core->base + CPU_CS_SCIACMDARG0, val,
+ val & CPU_CS_SCIACMDARG0_PC_READY,
+ POLL_INTERVAL_US, 10);
+    if (ret) {
+    dev_err(dev, "Polling power collapse ready timed out\n");
+    return ret;
+    }
+
+    mutex_lock(>lock);
+
+    ret = venus_power_off(hdev);
+    if (ret) {
+    dev_err(dev, "venus_power_off (%d)\n", ret);
+    mutex_unlock(>lock);
+    return ret;
+    }
+
+    hdev->suspended = true;
+
+    mutex_unlock(>lock);
+
+    return 0;
+}
+
 static int venus_suspend_3xx(struct venus_core *core)
 {
 struct venus_hfi_device *hdev = to_hfi_priv(core);
@@ -1507,6 +1556,9 @@ static int venus_suspend(struct venus_core 
*core)

 if (core->res->hfi_version == HFI_VERSION_3XX)
 return venus_suspend_3xx(core);

+    if (core->res->hfi_version == HFI_VERSION_4XX)
+    return venus_suspend_4xx(core);
+
 return venus_suspend_1xx(core);
 }


Let me brief on the power collapse sequence for Venus_4xx
1. Host checks for ARM9 and Video core to be idle.
   This can be done by checking for WFI bit (bit 0) in CPU status
register for ARM9 and by checking bit 30 in Control status reg for 
video

core/s.
2. Host then sends command to ARM9 to prepare for power collapse.
3. Host then checks for WFI bit and PC_READY bit before withdrawing
going for power off.

As per this patch, host is preparing for power collapse without 
checking

for #1.
Update the code to handle #3.


This looks similar to suspend for Venus 3xx. Can you confirm that the
sequence of checks for 4xx is the same as 3xx?


Do you mean the driver implementation for Suspend Venus 3xx or the 
hardware
expectation for Venus 3xx ? If hardware expectation wise, the sequence 
is

same for 3xx and 4xx.
In the suspend implementation for 3xx, i see that the host just reads 
the WFI
and idle status bits, but does not validate those bit value before 
preparing

Venus for power collapse. Sequence #2 and #3 is followed for Venus 3xx
implementation.

Re: [PATCH 10/28] venus: vdec: call session_continue in insufficient event

[Vikash Garodia]

Hi Stanimir,

On 2018-05-03 17:06, Stanimir Varbanov wrote:

Hi Vikash,

Thanks for the comments!

On  2.05.2018 09:26, Vikash Garodia wrote:

Hello Stanimir,

On 2018-04-24 18:14, Stanimir Varbanov wrote:

Call session_continue for Venus 4xx version even when the event
says that the buffer resources are not sufficient. Leaving a
comment with more information about the workaround.

Signed-off-by: Stanimir Varbanov <stanimir.varba...@linaro.org>
---
 drivers/media/platform/qcom/venus/vdec.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/vdec.c
b/drivers/media/platform/qcom/venus/vdec.c
index c45452634e7e..91c7384ff9c8 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -873,6 +873,14 @@ static void vdec_event_notify(struct venus_inst
*inst, u32 event,

 dev_dbg(dev, "event not sufficient resources (%ux%u)\n",
 data->width, data->height);
+    /*
+ * Workaround: Even that the firmware send and event for
+ * insufficient buffer resources it is safe to call
+ * session_continue because actually the event says that
+ * the number of capture buffers is lower.
+ */
+    if (IS_V4(core))
+    hfi_session_continue(inst);
 break;
 case HFI_EVENT_RELEASE_BUFFER_REFERENCE:
 venus_helper_release_buf_ref(inst, data->tag);


Insufficient event from video firmware could be sent either,
1. due to insufficient buffer resources
2. due to lower capture buffers

It cannot be assumed that the event received by the host is due to 
lower capture
buffers. Incase the buffer resource is insufficient, let say there is 
a bitstream
resolution switch from 720p to 1080p, capture buffers needs to be 
reallocated.


I agree with you. I will rework this part and call session_continue
only for case #2.


Even if the capture buffers are lower, driver should consider 
reallocation of capture
buffers with required higher count. Without this, it may happen that for 
a given video
frame, the decoded output will not be generated. The fact that the DPB 
buffer count is
same as capture buffers, will be lower than required. Hence the frame 
which needs YUV
reference beyond the DPB count, will get stuck as it cannot be decoded 
due to unavailability

of sufficient DPB buffers.
Say for ex. 10 DPB and capture buffers are allocated. For a given 
bitstream, firmware requested
the count to be 15. Frame 1 to 10 gets decoded and stored in DPB as 
references for future frame
decoding. Now when the 11th frame is queued to firmware, it can be 
decode but cannot be stored
as reference to decode future (12th) frame. Hence 11 frame will get 
stuck and will not be given

back to host driver.



The driver should be sending the V4L2_EVENT_SOURCE_CHANGE to client 
instead of ignoring

the event from firmware.


The v4l2 event is sent always to v4l clients.

regards,
Stan

Re: [PATCH 26/28] venus: implementing multi-stream support

[Vikash Garodia]

Hi Stanimir,

On 2018-05-03 12:42, Stanimir Varbanov wrote:

Hi Vikash,

Please write the comments for the chunk of code for which they are 
refer to.
I see that the patch was about handling multistream mode, but the code 
to handle
the dpb buffer response is missing. My comment is basically to add the 
required

code.


On  2.05.2018 10:40, Vikash Garodia wrote:

Hello Stanimir,

On 2018-04-24 18:14, Stanimir Varbanov wrote:

This is implementing a multi-stream decoder support. The multi
stream gives an option to use the secondary decoder output
with different raw format (or the same in case of crop).

Signed-off-by: Stanimir Varbanov <stanimir.varba...@linaro.org>
---
 drivers/media/platform/qcom/venus/core.h    |   1 +
 drivers/media/platform/qcom/venus/helpers.c | 204 
+++-

 drivers/media/platform/qcom/venus/helpers.h |   6 +
 drivers/media/platform/qcom/venus/vdec.c    |  91 -
 drivers/media/platform/qcom/venus/venc.c    |   1 +
 5 files changed, 299 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.h
b/drivers/media/platform/qcom/venus/core.h
index 4d6c05f156c4..85e66e2dd672 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -259,6 +259,7 @@ struct venus_inst {
 struct list_head list;
 struct mutex lock;
 struct venus_core *core;
+    struct list_head dpbbufs;
 struct list_head internalbufs;
 struct list_head registeredbufs;
 struct list_head delayed_process;






The dpb buffers queued to hardware will be returned back to host 
either during flush
or when the session is stopped. Host should not send these buffers to 
client.


That's correct.

vdec_buf_done should be handling in a way to drop dpb buffers from 
sending to client.


That is also correct, vdec_buf_done is trying to find the buffer by
index from a list of queued buffers from v4l2 clients. See
venus_helper_vb2_buf_queue where it is calling v4l2_m2m_buf_queue.

So for the dpb buffers venus_helper_find_buf() will return NULL.


My bad, i could see that the DPB buffers are not sent to client in the 
existing patch.


Instead of bailing out on NULL, i was thinking it is better to keep 
explicit check for dpb

buffers and exit with a debug log.



regards,
Stan

Re: [PATCH 26/28] venus: implementing multi-stream support

[Vikash Garodia]

On 2018-05-02 18:58, Nicolas Dufresne wrote:

Le mercredi 02 mai 2018 à 13:10 +0530, Vikash Garodia a écrit :

Hello Stanimir,

On 2018-04-24 18:14, Stanimir Varbanov wrote:
> This is implementing a multi-stream decoder support. The multi
> stream gives an option to use the secondary decoder output
> with different raw format (or the same in case of crop).
>
> Signed-off-by: Stanimir Varbanov <stanimir.varba...@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/core.h|   1 +
>  drivers/media/platform/qcom/venus/helpers.c | 204
> +++-
>  drivers/media/platform/qcom/venus/helpers.h |   6 +
>  drivers/media/platform/qcom/venus/vdec.c|  91 -
>  drivers/media/platform/qcom/venus/venc.c|   1 +
>  5 files changed, 299 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.h
> b/drivers/media/platform/qcom/venus/core.h
> index 4d6c05f156c4..85e66e2dd672 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -259,6 +259,7 @@ struct venus_inst {
>struct list_head list;
>struct mutex lock;
>struct venus_core *core;
> +  struct list_head dpbbufs;
>struct list_head internalbufs;
>struct list_head registeredbufs;
>struct list_head delayed_process;
> diff --git a/drivers/media/platform/qcom/venus/helpers.c
> b/drivers/media/platform/qcom/venus/helpers.c
> index ed569705ecac..87dcf9973e6f 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -85,6 +85,112 @@ bool venus_helper_check_codec(struct venus_inst
> *inst, u32 v4l2_pixfmt)
>  }
>  EXPORT_SYMBOL_GPL(venus_helper_check_codec);
>
> +static int venus_helper_queue_dpb_bufs(struct venus_inst *inst)
> +{
> +  struct intbuf *buf;
> +  int ret = 0;
> +
> +  if (list_empty(>dpbbufs))
> +  return 0;
> +
> +  list_for_each_entry(buf, >dpbbufs, list) {
> +  struct hfi_frame_data fdata;
> +
> +  memset(, 0, sizeof(fdata));
> +  fdata.alloc_len = buf->size;
> +  fdata.device_addr = buf->da;
> +  fdata.buffer_type = buf->type;
> +
> +  ret = hfi_session_process_buf(inst, );
> +  if (ret)
> +  goto fail;
> +  }
> +
> +fail:
> +  return ret;
> +}
> +
> +int venus_helper_free_dpb_bufs(struct venus_inst *inst)
> +{
> +  struct intbuf *buf, *n;
> +
> +  if (list_empty(>dpbbufs))
> +  return 0;
> +
> +  list_for_each_entry_safe(buf, n, >dpbbufs, list) {
> +  list_del_init(>list);
> +  dma_free_attrs(inst->core->dev, buf->size, buf-
> >va, buf->da,
> + buf->attrs);
> +  kfree(buf);
> +  }
> +
> +  INIT_LIST_HEAD(>dpbbufs);
> +
> +  return 0;
> +}
> +EXPORT_SYMBOL_GPL(venus_helper_free_dpb_bufs);
> +
> +int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
> +{
> +  struct venus_core *core = inst->core;
> +  struct device *dev = core->dev;
> +  enum hfi_version ver = core->res->hfi_version;
> +  struct hfi_buffer_requirements bufreq;
> +  u32 buftype = inst->dpb_buftype;
> +  unsigned int dpb_size = 0;
> +  struct intbuf *buf;
> +  unsigned int i;
> +  u32 count;
> +  int ret;
> +
> +  /* no need to allocate dpb buffers */
> +  if (!inst->dpb_fmt)
> +  return 0;
> +
> +  if (inst->dpb_buftype == HFI_BUFFER_OUTPUT)
> +  dpb_size = inst->output_buf_size;
> +  else if (inst->dpb_buftype == HFI_BUFFER_OUTPUT2)
> +  dpb_size = inst->output2_buf_size;
> +
> +  if (!dpb_size)
> +  return 0;
> +
> +  ret = venus_helper_get_bufreq(inst, buftype, );
> +  if (ret)
> +  return ret;
> +
> +  count = HFI_BUFREQ_COUNT_MIN(, ver);
> +
> +  for (i = 0; i < count; i++) {
> +  buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +  if (!buf) {
> +  ret = -ENOMEM;
> +  goto fail;
> +  }
> +
> +  buf->type = buftype;
> +  buf->size = dpb_size;
> +  buf->attrs = DMA_ATTR_WRITE_COMBINE |
> +   DMA_ATTR_NO_KERNEL_MAPPING;
> +  buf->va = dma_alloc_attrs(dev, buf->size, 
> >da, GFP_KERNEL,
> +buf->attrs);
> +  if (!buf->va) {
> +  kfree(buf);
> +  ret = -ENOMEM;
> +  goto fail;
> +  }
> +
> +  list_add_tail(>list, >dpbbufs);
> +  }
> +
> +  return 0;
> +
> +fail:
> +  venus_helper

Re: [PATCH 26/28] venus: implementing multi-stream support

[Vikash Garodia]

Hello Stanimir,

On 2018-04-24 18:14, Stanimir Varbanov wrote:

This is implementing a multi-stream decoder support. The multi
stream gives an option to use the secondary decoder output
with different raw format (or the same in case of crop).

Signed-off-by: Stanimir Varbanov 
---
 drivers/media/platform/qcom/venus/core.h|   1 +
 drivers/media/platform/qcom/venus/helpers.c | 204 
+++-

 drivers/media/platform/qcom/venus/helpers.h |   6 +
 drivers/media/platform/qcom/venus/vdec.c|  91 -
 drivers/media/platform/qcom/venus/venc.c|   1 +
 5 files changed, 299 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.h
b/drivers/media/platform/qcom/venus/core.h
index 4d6c05f156c4..85e66e2dd672 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -259,6 +259,7 @@ struct venus_inst {
struct list_head list;
struct mutex lock;
struct venus_core *core;
+   struct list_head dpbbufs;
struct list_head internalbufs;
struct list_head registeredbufs;
struct list_head delayed_process;
diff --git a/drivers/media/platform/qcom/venus/helpers.c
b/drivers/media/platform/qcom/venus/helpers.c
index ed569705ecac..87dcf9973e6f 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -85,6 +85,112 @@ bool venus_helper_check_codec(struct venus_inst
*inst, u32 v4l2_pixfmt)
 }
 EXPORT_SYMBOL_GPL(venus_helper_check_codec);

+static int venus_helper_queue_dpb_bufs(struct venus_inst *inst)
+{
+   struct intbuf *buf;
+   int ret = 0;
+
+   if (list_empty(>dpbbufs))
+   return 0;
+
+   list_for_each_entry(buf, >dpbbufs, list) {
+   struct hfi_frame_data fdata;
+
+   memset(, 0, sizeof(fdata));
+   fdata.alloc_len = buf->size;
+   fdata.device_addr = buf->da;
+   fdata.buffer_type = buf->type;
+
+   ret = hfi_session_process_buf(inst, );
+   if (ret)
+   goto fail;
+   }
+
+fail:
+   return ret;
+}
+
+int venus_helper_free_dpb_bufs(struct venus_inst *inst)
+{
+   struct intbuf *buf, *n;
+
+   if (list_empty(>dpbbufs))
+   return 0;
+
+   list_for_each_entry_safe(buf, n, >dpbbufs, list) {
+   list_del_init(>list);
+   dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
+  buf->attrs);
+   kfree(buf);
+   }
+
+   INIT_LIST_HEAD(>dpbbufs);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(venus_helper_free_dpb_bufs);
+
+int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
+{
+   struct venus_core *core = inst->core;
+   struct device *dev = core->dev;
+   enum hfi_version ver = core->res->hfi_version;
+   struct hfi_buffer_requirements bufreq;
+   u32 buftype = inst->dpb_buftype;
+   unsigned int dpb_size = 0;
+   struct intbuf *buf;
+   unsigned int i;
+   u32 count;
+   int ret;
+
+   /* no need to allocate dpb buffers */
+   if (!inst->dpb_fmt)
+   return 0;
+
+   if (inst->dpb_buftype == HFI_BUFFER_OUTPUT)
+   dpb_size = inst->output_buf_size;
+   else if (inst->dpb_buftype == HFI_BUFFER_OUTPUT2)
+   dpb_size = inst->output2_buf_size;
+
+   if (!dpb_size)
+   return 0;
+
+   ret = venus_helper_get_bufreq(inst, buftype, );
+   if (ret)
+   return ret;
+
+   count = HFI_BUFREQ_COUNT_MIN(, ver);
+
+   for (i = 0; i < count; i++) {
+   buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+   if (!buf) {
+   ret = -ENOMEM;
+   goto fail;
+   }
+
+   buf->type = buftype;
+   buf->size = dpb_size;
+   buf->attrs = DMA_ATTR_WRITE_COMBINE |
+DMA_ATTR_NO_KERNEL_MAPPING;
+   buf->va = dma_alloc_attrs(dev, buf->size, >da, GFP_KERNEL,
+ buf->attrs);
+   if (!buf->va) {
+   kfree(buf);
+   ret = -ENOMEM;
+   goto fail;
+   }
+
+   list_add_tail(>list, >dpbbufs);
+   }
+
+   return 0;
+
+fail:
+   venus_helper_free_dpb_bufs(inst);
+   return ret;
+}
+EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
+
 static int intbufs_set_buffer(struct venus_inst *inst, u32 type)
 {
struct venus_core *core = inst->core;
@@ -342,7 +448,10 @@ session_process_buf(struct venus_inst *inst,
struct vb2_v4l2_buffer *vbuf)
if (vbuf->flags & V4L2_BUF_FLAG_LAST || !fdata.filled_len)
fdata.flags |= HFI_BUFFERFLAG_EOS;
} else if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
-   fdata.buffer_type = 

Re: [PATCH 10/28] venus: vdec: call session_continue in insufficient event

[Vikash Garodia]

Hello Stanimir,

On 2018-04-24 18:14, Stanimir Varbanov wrote:

Call session_continue for Venus 4xx version even when the event
says that the buffer resources are not sufficient. Leaving a
comment with more information about the workaround.

Signed-off-by: Stanimir Varbanov 
---
 drivers/media/platform/qcom/venus/vdec.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/vdec.c
b/drivers/media/platform/qcom/venus/vdec.c
index c45452634e7e..91c7384ff9c8 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -873,6 +873,14 @@ static void vdec_event_notify(struct venus_inst
*inst, u32 event,

dev_dbg(dev, "event not sufficient resources (%ux%u)\n",
data->width, data->height);
+   /*
+* Workaround: Even that the firmware send and event for
+* insufficient buffer resources it is safe to call
+* session_continue because actually the event says that
+* the number of capture buffers is lower.
+*/
+   if (IS_V4(core))
+   hfi_session_continue(inst);
break;
case HFI_EVENT_RELEASE_BUFFER_REFERENCE:
venus_helper_release_buf_ref(inst, data->tag);


Insufficient event from video firmware could be sent either,
1. due to insufficient buffer resources
2. due to lower capture buffers

It cannot be assumed that the event received by the host is due to lower 
capture
buffers. Incase the buffer resource is insufficient, let say there is a 
bitstream
resolution switch from 720p to 1080p, capture buffers needs to be 
reallocated.


The driver should be sending the V4L2_EVENT_SOURCE_CHANGE to client 
instead of ignoring

the event from firmware.

Thanks,
Vikash