This series includes the changes to
- add firmware version based check to enable/disable some feature.
- add support of new HFI to notify sequence change event to
driver during resolution change at interframe.
- use firmware version based check to fix EOS handling for different
firmware versions.
With this series, divided the previous version [1] into
multiple patches as suggested in review comments.
[1] https://patchwork.kernel.org/project/linux-media/list/?series=733169
Dikshita Agarwal (3):
venus: add firmware version based check
venus: enable sufficient sequence change support for vp9
venus: fix EOS handling in decoder stop command
drivers/media/platform/qcom/venus/core.h | 18 ++++++++++++++++++
drivers/media/platform/qcom/venus/hfi_cmds.c | 1 +
drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
drivers/media/platform/qcom/venus/hfi_msgs.c | 11 +++++++++--
drivers/media/platform/qcom/venus/vdec.c | 10 +++++++++-
5 files changed, 39 insertions(+), 3 deletions(-)
--
2.7.4
Add firmware version based checks to enable/disable
features for different SOCs.
Signed-off-by: Dikshita Agarwal <[email protected]>
Signed-off-by: Vikash Garodia <[email protected]>
Signed-off-by: Viswanath Boma <[email protected]>
Tested-by: Nathan Hebert <[email protected]>
---
drivers/media/platform/qcom/venus/core.h | 18 ++++++++++++++++++
drivers/media/platform/qcom/venus/hfi_msgs.c | 11 +++++++++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 32551c2..ee8b70a 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -202,6 +202,11 @@ struct venus_core {
unsigned int core0_usage_count;
unsigned int core1_usage_count;
struct dentry *root;
+ struct venus_img_version {
+ u32 major;
+ u32 minor;
+ u32 rev;
+ } venus_ver;
};
struct vdec_controls {
@@ -500,4 +505,17 @@ venus_caps_by_codec(struct venus_core *core, u32 codec, u32 domain)
return NULL;
}
+static inline int
+is_fw_rev_or_newer(struct venus_core *core, u32 vmajor, u32 vminor, u32 vrev)
+{
+ return ((core)->venus_ver.major == vmajor && (core)->venus_ver.minor ==
+ vminor && (core)->venus_ver.rev >= vrev);
+}
+
+static inline int
+is_fw_rev_or_older(struct venus_core *core, u32 vmajor, u32 vminor, u32 vrev)
+{
+ return ((core)->venus_ver.major == vmajor && (core)->venus_ver.minor ==
+ vminor && (core)->venus_ver.rev <= vrev);
+}
#endif
diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
index df96db3..07ac0fc 100644
--- a/drivers/media/platform/qcom/venus/hfi_msgs.c
+++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
@@ -248,9 +248,10 @@ static void hfi_sys_init_done(struct venus_core *core, struct venus_inst *inst,
}
static void
-sys_get_prop_image_version(struct device *dev,
+sys_get_prop_image_version(struct venus_core *core,
struct hfi_msg_sys_property_info_pkt *pkt)
{
+ struct device *dev = core->dev;
u8 *smem_tbl_ptr;
u8 *img_ver;
int req_bytes;
@@ -263,6 +264,12 @@ sys_get_prop_image_version(struct device *dev,
return;
img_ver = pkt->data;
+ if (IS_V4(core))
+ sscanf(img_ver, "14:VIDEO.VE.%u.%u-%u-PROD",
+ &core->venus_ver.major, &core->venus_ver.minor, &core->venus_ver.rev);
+ else if (IS_V6(core))
+ sscanf(img_ver, "14:VIDEO.VPU.%u.%u-%u-PROD",
+ &core->venus_ver.major, &core->venus_ver.minor, &core->venus_ver.rev);
dev_dbg(dev, VDBGL "F/W version: %s\n", img_ver);
@@ -286,7 +293,7 @@ static void hfi_sys_property_info(struct venus_core *core,
switch (pkt->property) {
case HFI_PROPERTY_SYS_IMAGE_VERSION:
- sys_get_prop_image_version(dev, pkt);
+ sys_get_prop_image_version(core, pkt);
break;
default:
dev_dbg(dev, VDBGL "unknown property data\n");
--
2.7.4
VP9 supports resolution change at interframe.
Currenlty, if sequence change is detected at interframe and
resources are sufficient, sequence change event is not raised
by firmware to driver until the next keyframe.
This change add the HFI to notify the sequence change in this
case to driver.
Signed-off-by: Dikshita Agarwal <[email protected]>
Signed-off-by: Vikash Garodia <[email protected]>
Signed-off-by: Viswanath Boma <[email protected]>
Tested-by: Nathan Hebert <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_cmds.c | 1 +
drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
drivers/media/platform/qcom/venus/vdec.c | 8 ++++++++
3 files changed, 11 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 930b743..e2539b5 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.c
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
@@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
break;
}
+ case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
struct hfi_enable *in = pdata;
struct hfi_enable *en = prop_data;
diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index d2d6719..20516b4 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -469,6 +469,8 @@
#define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH 0x1003007
#define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT 0x1003009
#define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE 0x100300a
+#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
+ 0x0100300b
/*
* HFI_PROPERTY_CONFIG_VDEC_COMMON_START
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 4ceaba3..f0394b9 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
return ret;
}
+ /* Enabling sufficient sequence change support for VP9 */
+ if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
+ ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
+ ret = hfi_session_set_property(inst, ptype, &en);
+ if (ret)
+ return ret;
+ }
+
ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
conceal = ctr->conceal_color & 0xffff;
conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
--
2.7.4
Use firmware version based check to assign correct
device address for EOS buffer to fix the EOS handling
with different firmware version.
Signed-off-by: Dikshita Agarwal <[email protected]>
Signed-off-by: Vikash Garodia <[email protected]>
Signed-off-by: Viswanath Boma <[email protected]>
Tested-by: Nathan Hebert <[email protected]>
---
drivers/media/platform/qcom/venus/vdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index f0394b9..c59b34f 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -545,7 +545,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
fdata.buffer_type = HFI_BUFFER_INPUT;
fdata.flags |= HFI_BUFFERFLAG_EOS;
- if (IS_V6(inst->core))
+ if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
fdata.device_addr = 0;
else
fdata.device_addr = 0xdeadb000;
--
2.7.4
On 4.04.2023 08:17, Dikshita Agarwal wrote:
> Add firmware version based checks to enable/disable
> features for different SOCs.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> Signed-off-by: Vikash Garodia <[email protected]>
> Signed-off-by: Viswanath Boma <[email protected]>
> Tested-by: Nathan Hebert <[email protected]>
> ---
> drivers/media/platform/qcom/venus/core.h | 18 ++++++++++++++++++
> drivers/media/platform/qcom/venus/hfi_msgs.c | 11 +++++++++--
> 2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 32551c2..ee8b70a 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -202,6 +202,11 @@ struct venus_core {
> unsigned int core0_usage_count;
> unsigned int core1_usage_count;
> struct dentry *root;
> + struct venus_img_version {
> + u32 major;
> + u32 minor;
> + u32 rev;
> + } venus_ver;
> };
>
> struct vdec_controls {
> @@ -500,4 +505,17 @@ venus_caps_by_codec(struct venus_core *core, u32 codec, u32 domain)
> return NULL;
> }
>
> +static inline int
> +is_fw_rev_or_newer(struct venus_core *core, u32 vmajor, u32 vminor, u32 vrev)
So this is trying to find the revision of a MAJ.MIN firmware release..
Is the MAJ.MIN part supposed to stay constant for a specific version of
a Venus block (say idk, IRIS2) and only the VREV part is supposed to
be updated with both feature and security developments?
> +{
> + return ((core)->venus_ver.major == vmajor && (core)->venus_ver.minor ==
> + vminor && (core)->venus_ver.rev >= vrev);
return ((core)->venus_ver.major == vmajor &&
(core)->venus_ver.minor == vminor &&
(core)->venus_ver.rev >= vrev);
for readability
Konrad
> +}
> +
> +static inline int
> +is_fw_rev_or_older(struct venus_core *core, u32 vmajor, u32 vminor, u32 vrev)
> +{
> + return ((core)->venus_ver.major == vmajor && (core)->venus_ver.minor ==
> + vminor && (core)->venus_ver.rev <= vrev);
> +}
> #endif
> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index df96db3..07ac0fc 100644
> --- a/drivers/media/platform/qcom/venus/hfi_msgs.c
> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
> @@ -248,9 +248,10 @@ static void hfi_sys_init_done(struct venus_core *core, struct venus_inst *inst,
> }
>
> static void
> -sys_get_prop_image_version(struct device *dev,
> +sys_get_prop_image_version(struct venus_core *core,
> struct hfi_msg_sys_property_info_pkt *pkt)
> {
> + struct device *dev = core->dev;
> u8 *smem_tbl_ptr;
> u8 *img_ver;
> int req_bytes;
> @@ -263,6 +264,12 @@ sys_get_prop_image_version(struct device *dev,
> return;
>
> img_ver = pkt->data;
> + if (IS_V4(core))
> + sscanf(img_ver, "14:VIDEO.VE.%u.%u-%u-PROD",
> + &core->venus_ver.major, &core->venus_ver.minor, &core->venus_ver.rev);
> + else if (IS_V6(core))
> + sscanf(img_ver, "14:VIDEO.VPU.%u.%u-%u-PROD",
> + &core->venus_ver.major, &core->venus_ver.minor, &core->venus_ver.rev);
>
> dev_dbg(dev, VDBGL "F/W version: %s\n", img_ver);
>
> @@ -286,7 +293,7 @@ static void hfi_sys_property_info(struct venus_core *core,
>
> switch (pkt->property) {
> case HFI_PROPERTY_SYS_IMAGE_VERSION:
> - sys_get_prop_image_version(dev, pkt);
> + sys_get_prop_image_version(core, pkt);
> break;
> default:
> dev_dbg(dev, VDBGL "unknown property data\n");
On 4.04.2023 08:17, Dikshita Agarwal wrote:
> VP9 supports resolution change at interframe.
> Currenlty, if sequence change is detected at interframe and
> resources are sufficient, sequence change event is not raised
> by firmware to driver until the next keyframe.
> This change add the HFI to notify the sequence change in this
> case to driver.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> Signed-off-by: Vikash Garodia <[email protected]>
> Signed-off-by: Viswanath Boma <[email protected]>
> Tested-by: Nathan Hebert <[email protected]>
> ---
> drivers/media/platform/qcom/venus/hfi_cmds.c | 1 +
> drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
> drivers/media/platform/qcom/venus/vdec.c | 8 ++++++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743..e2539b5 100644
> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
> pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
> break;
> }
> + case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
> case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
> struct hfi_enable *in = pdata;
> struct hfi_enable *en = prop_data;
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index d2d6719..20516b4 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -469,6 +469,8 @@
> #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH 0x1003007
> #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT 0x1003009
> #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE 0x100300a
> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
> + 0x0100300b
>
> /*
> * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba3..f0394b9 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
> return ret;
> }
>
> + /* Enabling sufficient sequence change support for VP9 */
> + if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
> + ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
> + ret = hfi_session_set_property(inst, ptype, &en);
> + if (ret)
> + return ret;
> + }
Does it never have to be turned off? Or does it happen automatically
at session closure?
Konrad
> +
> ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
> conceal = ctr->conceal_color & 0xffff;
> conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
On 4.04.2023 08:17, Dikshita Agarwal wrote:
> Use firmware version based check to assign correct
> device address for EOS buffer to fix the EOS handling
> with different firmware version.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> Signed-off-by: Vikash Garodia <[email protected]>
> Signed-off-by: Viswanath Boma <[email protected]>
> Tested-by: Nathan Hebert <[email protected]>
> ---
Does it only concern fw 1.0.xx?
Konrad
> drivers/media/platform/qcom/venus/vdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index f0394b9..c59b34f 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -545,7 +545,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>
> fdata.buffer_type = HFI_BUFFER_INPUT;
> fdata.flags |= HFI_BUFFERFLAG_EOS;
> - if (IS_V6(inst->core))
> + if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
> fdata.device_addr = 0;
> else
> fdata.device_addr = 0xdeadb000;
On 4.04.2023 08:17, Dikshita Agarwal wrote:
> VP9 supports resolution change at interframe.
> Currenlty, if sequence change is detected at interframe and
> resources are sufficient, sequence change event is not raised
> by firmware to driver until the next keyframe.
> This change add the HFI to notify the sequence change in this
> case to driver.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> Signed-off-by: Vikash Garodia <[email protected]>
> Signed-off-by: Viswanath Boma <[email protected]>
> Tested-by: Nathan Hebert <[email protected]>
> ---
> drivers/media/platform/qcom/venus/hfi_cmds.c | 1 +
> drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
> drivers/media/platform/qcom/venus/vdec.c | 8 ++++++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743..e2539b5 100644
> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
> pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
> break;
> }
> + case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
> case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
> struct hfi_enable *in = pdata;
> struct hfi_enable *en = prop_data;
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index d2d6719..20516b4 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -469,6 +469,8 @@
> #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH 0x1003007
> #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT 0x1003009
> #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE 0x100300a
> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
> + 0x0100300b
Also, nit: this one has a leading zero, whereas other properties don't
Konrad
>
> /*
> * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba3..f0394b9 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
> return ret;
> }
>
> + /* Enabling sufficient sequence change support for VP9 */
> + if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
> + ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
> + ret = hfi_session_set_property(inst, ptype, &en);
> + if (ret)
> + return ret;
> + }
> +
> ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
> conceal = ctr->conceal_color & 0xffff;
> conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
On 4/4/2023 11:52 PM, Konrad Dybcio wrote:
>
> On 4.04.2023 08:17, Dikshita Agarwal wrote:
>> VP9 supports resolution change at interframe.
>> Currenlty, if sequence change is detected at interframe and
>> resources are sufficient, sequence change event is not raised
>> by firmware to driver until the next keyframe.
>> This change add the HFI to notify the sequence change in this
>> case to driver.
>>
>> Signed-off-by: Dikshita Agarwal <[email protected]>
>> Signed-off-by: Vikash Garodia <[email protected]>
>> Signed-off-by: Viswanath Boma <[email protected]>
>> Tested-by: Nathan Hebert <[email protected]>
>> ---
>> drivers/media/platform/qcom/venus/hfi_cmds.c | 1 +
>> drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
>> drivers/media/platform/qcom/venus/vdec.c | 8 ++++++++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> index 930b743..e2539b5 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
>> pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
>> break;
>> }
>> + case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
>> case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
>> struct hfi_enable *in = pdata;
>> struct hfi_enable *en = prop_data;
>> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
>> index d2d6719..20516b4 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
>> @@ -469,6 +469,8 @@
>> #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH 0x1003007
>> #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT 0x1003009
>> #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE 0x100300a
>> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
>> + 0x0100300b
>>
>> /*
>> * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 4ceaba3..f0394b9 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
>> return ret;
>> }
>>
>> + /* Enabling sufficient sequence change support for VP9 */
>> + if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
>> + ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
>> + ret = hfi_session_set_property(inst, ptype, &en);
>> + if (ret)
>> + return ret;
>> + }
> Does it never have to be turned off? Or does it happen automatically
> at session closure?
>
> Konrad
Any property set to FW is applied for entire video session and it
doesn't need to change so
there is no need to turn it off or unset it.
Thanks,
Dikshita
>> +
>> ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
>> conceal = ctr->conceal_color & 0xffff;
>> conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
On 4/4/2023 11:54 PM, Konrad Dybcio wrote:
>
> On 4.04.2023 08:17, Dikshita Agarwal wrote:
>> Use firmware version based check to assign correct
>> device address for EOS buffer to fix the EOS handling
>> with different firmware version.
>>
>> Signed-off-by: Dikshita Agarwal <[email protected]>
>> Signed-off-by: Vikash Garodia <[email protected]>
>> Signed-off-by: Viswanath Boma <[email protected]>
>> Tested-by: Nathan Hebert <[email protected]>
>> ---
> Does it only concern fw 1.0.xx?
>
> Konrad
that's right.
changes were made in later firmware (newer than 1.0.87) to
make the behavior generic across all supported SOCs.
Thanks,
Dikshita
>> drivers/media/platform/qcom/venus/vdec.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index f0394b9..c59b34f 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -545,7 +545,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>>
>> fdata.buffer_type = HFI_BUFFER_INPUT;
>> fdata.flags |= HFI_BUFFERFLAG_EOS;
>> - if (IS_V6(inst->core))
>> + if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
>> fdata.device_addr = 0;
>> else
>> fdata.device_addr = 0xdeadb000;
On 5.04.2023 08:41, Dikshita Agarwal wrote:
>
> On 4/4/2023 11:54 PM, Konrad Dybcio wrote:
>>
>> On 4.04.2023 08:17, Dikshita Agarwal wrote:
>>> Use firmware version based check to assign correct
>>> device address for EOS buffer to fix the EOS handling
>>> with different firmware version.
>>>
>>> Signed-off-by: Dikshita Agarwal <[email protected]>
>>> Signed-off-by: Vikash Garodia <[email protected]>
>>> Signed-off-by: Viswanath Boma <[email protected]>
>>> Tested-by: Nathan Hebert <[email protected]>
>>> ---
>> Does it only concern fw 1.0.xx?
>>
>> Konrad
>
> that's right.
>
> changes were made in later firmware (newer than 1.0.87) to
>
> make the behavior generic across all supported SOCs.
>
> Thanks,
OK
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
>
> Dikshita
>
>>> drivers/media/platform/qcom/venus/vdec.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>>> index f0394b9..c59b34f 100644
>>> --- a/drivers/media/platform/qcom/venus/vdec.c
>>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>>> @@ -545,7 +545,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>>> fdata.buffer_type = HFI_BUFFER_INPUT;
>>> fdata.flags |= HFI_BUFFERFLAG_EOS;
>>> - if (IS_V6(inst->core))
>>> + if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
>>> fdata.device_addr = 0;
>>> else
>>> fdata.device_addr = 0xdeadb000;
On 4/5/2023 12:35 AM, Konrad Dybcio wrote:
>
> On 4.04.2023 08:17, Dikshita Agarwal wrote:
>> VP9 supports resolution change at interframe.
>> Currenlty, if sequence change is detected at interframe and
>> resources are sufficient, sequence change event is not raised
>> by firmware to driver until the next keyframe.
>> This change add the HFI to notify the sequence change in this
>> case to driver.
>>
>> Signed-off-by: Dikshita Agarwal <[email protected]>
>> Signed-off-by: Vikash Garodia <[email protected]>
>> Signed-off-by: Viswanath Boma <[email protected]>
>> Tested-by: Nathan Hebert <[email protected]>
>> ---
>> drivers/media/platform/qcom/venus/hfi_cmds.c | 1 +
>> drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
>> drivers/media/platform/qcom/venus/vdec.c | 8 ++++++++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> index 930b743..e2539b5 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
>> pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
>> break;
>> }
>> + case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
>> case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
>> struct hfi_enable *in = pdata;
>> struct hfi_enable *en = prop_data;
>> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
>> index d2d6719..20516b4 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
>> @@ -469,6 +469,8 @@
>> #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH 0x1003007
>> #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT 0x1003009
>> #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE 0x100300a
>> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
>> + 0x0100300b
> Also, nit: this one has a leading zero, whereas other properties don't
>
> Konrad
Sure, Will fix in next version.
Thanks,
Dikshita
>>
>> /*
>> * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 4ceaba3..f0394b9 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
>> return ret;
>> }
>>
>> + /* Enabling sufficient sequence change support for VP9 */
>> + if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
>> + ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
>> + ret = hfi_session_set_property(inst, ptype, &en);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
>> conceal = ctr->conceal_color & 0xffff;
>> conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
On 4/4/2023 11:48 PM, Konrad Dybcio wrote:
>
> On 4.04.2023 08:17, Dikshita Agarwal wrote:
>> Add firmware version based checks to enable/disable
>> features for different SOCs.
>>
>> Signed-off-by: Dikshita Agarwal <[email protected]>
>> Signed-off-by: Vikash Garodia <[email protected]>
>> Signed-off-by: Viswanath Boma <[email protected]>
>> Tested-by: Nathan Hebert <[email protected]>
>> ---
>> drivers/media/platform/qcom/venus/core.h | 18 ++++++++++++++++++
>> drivers/media/platform/qcom/venus/hfi_msgs.c | 11 +++++++++--
>> 2 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>> index 32551c2..ee8b70a 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -202,6 +202,11 @@ struct venus_core {
>> unsigned int core0_usage_count;
>> unsigned int core1_usage_count;
>> struct dentry *root;
>> + struct venus_img_version {
>> + u32 major;
>> + u32 minor;
>> + u32 rev;
>> + } venus_ver;
>> };
>>
>> struct vdec_controls {
>> @@ -500,4 +505,17 @@ venus_caps_by_codec(struct venus_core *core, u32 codec, u32 domain)
>> return NULL;
>> }
>>
>> +static inline int
>> +is_fw_rev_or_newer(struct venus_core *core, u32 vmajor, u32 vminor, u32 vrev)
> So this is trying to find the revision of a MAJ.MIN firmware release..
>
> Is the MAJ.MIN part supposed to stay constant for a specific version of
> a Venus block (say idk, IRIS2) and only the VREV part is supposed to
> be updated with both feature and security developments?
Yes Major and minor numbers will be constant for a SPC and only the
revision will be updated with any new release.
>> +{
>> + return ((core)->venus_ver.major == vmajor && (core)->venus_ver.minor ==
>> + vminor && (core)->venus_ver.rev >= vrev);
> return ((core)->venus_ver.major == vmajor &&
> (core)->venus_ver.minor == vminor &&
> (core)->venus_ver.rev >= vrev);
>
> for readability
Sure, will take care of this in next version.
> Konrad
>> +}
>> +
>> +static inline int
>> +is_fw_rev_or_older(struct venus_core *core, u32 vmajor, u32 vminor, u32 vrev)
>> +{
>> + return ((core)->venus_ver.major == vmajor && (core)->venus_ver.minor ==
>> + vminor && (core)->venus_ver.rev <= vrev);
>> +}
>> #endif
>> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
>> index df96db3..07ac0fc 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_msgs.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
>> @@ -248,9 +248,10 @@ static void hfi_sys_init_done(struct venus_core *core, struct venus_inst *inst,
>> }
>>
>> static void
>> -sys_get_prop_image_version(struct device *dev,
>> +sys_get_prop_image_version(struct venus_core *core,
>> struct hfi_msg_sys_property_info_pkt *pkt)
>> {
>> + struct device *dev = core->dev;
>> u8 *smem_tbl_ptr;
>> u8 *img_ver;
>> int req_bytes;
>> @@ -263,6 +264,12 @@ sys_get_prop_image_version(struct device *dev,
>> return;
>>
>> img_ver = pkt->data;
>> + if (IS_V4(core))
>> + sscanf(img_ver, "14:VIDEO.VE.%u.%u-%u-PROD",
>> + &core->venus_ver.major, &core->venus_ver.minor, &core->venus_ver.rev);
>> + else if (IS_V6(core))
>> + sscanf(img_ver, "14:VIDEO.VPU.%u.%u-%u-PROD",
>> + &core->venus_ver.major, &core->venus_ver.minor, &core->venus_ver.rev);
>>
>> dev_dbg(dev, VDBGL "F/W version: %s\n", img_ver);
>>
>> @@ -286,7 +293,7 @@ static void hfi_sys_property_info(struct venus_core *core,
>>
>> switch (pkt->property) {
>> case HFI_PROPERTY_SYS_IMAGE_VERSION:
>> - sys_get_prop_image_version(dev, pkt);
>> + sys_get_prop_image_version(core, pkt);
>> break;
>> default:
>> dev_dbg(dev, VDBGL "unknown property data\n");