* [PATCH 0/1] Fix for VP9 DRC and Decoder STOP issue.
@ 2022-11-15 12:12 quic_vboma
2022-11-15 12:12 ` [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue quic_vboma
0 siblings, 1 reply; 22+ messages in thread
From: quic_vboma @ 2022-11-15 12:12 UTC (permalink / raw)
To: Stanimir Varbanov, Andy Gross, Bjorn Andersson,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel
Cc: Viswanath Boma
From: Viswanath Boma <quic_vboma@quicinc.com>
Tested the changes on v5.15 and v5.4 kernels .
For testing Chrome Utilities were used .
Viswanath Boma (1):
venus: Enable sufficient sequence change support for sc7180 and Fix
for Decoder STOP command issue.
drivers/media/platform/qcom/venus/core.h | 10 ++++++++++
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 | 12 +++++++++++-
5 files changed, 33 insertions(+), 3 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue.
2022-11-15 12:12 [PATCH 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
@ 2022-11-15 12:12 ` quic_vboma
2022-12-02 0:54 ` Nathan Hebert
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: quic_vboma @ 2022-11-15 12:12 UTC (permalink / raw)
To: Stanimir Varbanov, Andy Gross, Bjorn Andersson,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel
Cc: Viswanath Boma, Vikash Garodia
From: Viswanath Boma <quic_vboma@quicinc.com>
For VP9 bitstreams, there could be a change in resolution at interframe,
for driver to get notified of such resolution change, enable the property in video firmware.
Also, EOS handling is now made same in video firmware across all V6 SOCs,
hence above a certain firmware version, the driver handling is made generic for all V6s
Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
---
drivers/media/platform/qcom/venus/core.h | 10 ++++++++++
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 | 12 +++++++++++-
5 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 32551c2602a9..538d7cd41edd 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 {
@@ -472,6 +477,11 @@ struct venus_inst {
#define IS_V3(core) ((core)->res->hfi_version == HFI_VERSION_3XX)
#define IS_V4(core) ((core)->res->hfi_version == HFI_VERSION_4XX)
#define IS_V6(core) ((core)->res->hfi_version == HFI_VERSION_6XX)
+#define IS_FW_REV_OR_NEWER(core, vmajor, vminor, vrev) ((core)->venus_ver.major == vmajor && \
+ (core)->venus_ver.minor == vminor && (core)->venus_ver.rev >= vrev)
+#define IS_FW_REV_OR_OLDER(core, vmajor, vminor, vrev) ((core)->venus_ver.major == vmajor && \
+ (core)->venus_ver.minor == vminor && (core)->venus_ver.rev <= vrev)
+
#define ctrl_to_inst(ctrl) \
container_of((ctrl)->handler, struct venus_inst, ctrl_handler)
diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
index df96db3761a7..4a398f77fe9c 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");
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 4ceaba37e2e5..cc19eb62b723 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;
@@ -671,6 +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
return ret;
}
+ /* Enabling sufficient sequence change support for VP9 */
+ if (of_device_is_compatible(inst->core->dev->of_node, "qcom,sc7180-venus")) {
+ 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.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue.
2022-11-15 12:12 ` [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue quic_vboma
@ 2022-12-02 0:54 ` Nathan Hebert
2022-12-02 1:03 ` Bryan O'Donoghue
2023-02-02 6:47 ` [PATCH V2 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2 siblings, 0 replies; 22+ messages in thread
From: Nathan Hebert @ 2022-12-02 0:54 UTC (permalink / raw)
To: quic_vboma
Cc: Stanimir Varbanov, Andy Gross, Bjorn Andersson,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel,
Vikash Garodia
On Tue, Nov 15, 2022 at 4:12 AM <quic_vboma@quicinc.com> wrote:
>
> From: Viswanath Boma <quic_vboma@quicinc.com>
>
> For VP9 bitstreams, there could be a change in resolution at interframe,
> for driver to get notified of such resolution change, enable the property in video firmware.
> Also, EOS handling is now made same in video firmware across all V6 SOCs,
> hence above a certain firmware version, the driver handling is made generic for all V6s
>
With this patch and the latest Venus SC7180 firmware, VP9 interframe
resolution changes are handled and decoded correctly.
Tested-by: Nathan Hebert <nhebert@chromium.org>
> Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> ---
> drivers/media/platform/qcom/venus/core.h | 10 ++++++++++
> 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 | 12 +++++++++++-
> 5 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 32551c2602a9..538d7cd41edd 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 {
> @@ -472,6 +477,11 @@ struct venus_inst {
> #define IS_V3(core) ((core)->res->hfi_version == HFI_VERSION_3XX)
> #define IS_V4(core) ((core)->res->hfi_version == HFI_VERSION_4XX)
> #define IS_V6(core) ((core)->res->hfi_version == HFI_VERSION_6XX)
> +#define IS_FW_REV_OR_NEWER(core, vmajor, vminor, vrev) ((core)->venus_ver.major == vmajor && \
> + (core)->venus_ver.minor == vminor && (core)->venus_ver.rev >= vrev)
> +#define IS_FW_REV_OR_OLDER(core, vmajor, vminor, vrev) ((core)->venus_ver.major == vmajor && \
> + (core)->venus_ver.minor == vminor && (core)->venus_ver.rev <= vrev)
> +
>
> #define ctrl_to_inst(ctrl) \
> container_of((ctrl)->handler, struct venus_inst, ctrl_handler)
> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index df96db3761a7..4a398f77fe9c 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");
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba37e2e5..cc19eb62b723 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;
> @@ -671,6 +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
> return ret;
> }
>
> + /* Enabling sufficient sequence change support for VP9 */
> + if (of_device_is_compatible(inst->core->dev->of_node, "qcom,sc7180-venus")) {
> + 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.17.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue.
2022-11-15 12:12 ` [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue quic_vboma
2022-12-02 0:54 ` Nathan Hebert
@ 2022-12-02 1:03 ` Bryan O'Donoghue
2023-02-02 6:47 ` [PATCH V2 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2 siblings, 0 replies; 22+ messages in thread
From: Bryan O'Donoghue @ 2022-12-02 1:03 UTC (permalink / raw)
To: quic_vboma, Stanimir Varbanov, Andy Gross, Bjorn Andersson,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel
Cc: Vikash Garodia
On 15/11/2022 12:12, quic_vboma@quicinc.com wrote:
> From: Viswanath Boma <quic_vboma@quicinc.com>
>
> For VP9 bitstreams, there could be a change in resolution at interframe,
> for driver to get notified of such resolution change, enable the property in video firmware.
> Also, EOS handling is now made same in video firmware across all V6 SOCs,
> hence above a certain firmware version, the driver handling is made generic for all V6s
Could you figure out the right Fixes: and Cc: stable to add here ?
---
bod
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V2 0/1] Fix for VP9 DRC and Decoder STOP issue.
2022-11-15 12:12 ` [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue quic_vboma
2022-12-02 0:54 ` Nathan Hebert
2022-12-02 1:03 ` Bryan O'Donoghue
@ 2023-02-02 6:47 ` quic_vboma
2023-02-02 6:47 ` [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
2023-02-02 10:02 ` [PATCH V2 " Dmitry Baryshkov
2 siblings, 2 replies; 22+ messages in thread
From: quic_vboma @ 2023-02-02 6:47 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Viswanath Boma, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel
From: Viswanath Boma <quic_vboma@quicinc.com>
Tested the changes on v5.15 and v5.4 kernels .
For testing Chrome Utilities were used .
Viswanath Boma (1):
venus: Enable sufficient sequence change support for sc7180 and fix
for Decoder STOP command issue.
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 | 12 +++++++++++-
5 files changed, 41 insertions(+), 3 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-02-02 6:47 ` [PATCH V2 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
@ 2023-02-02 6:47 ` quic_vboma
2023-02-02 10:10 ` Dmitry Baryshkov
2023-03-16 8:15 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2023-02-02 10:02 ` [PATCH V2 " Dmitry Baryshkov
1 sibling, 2 replies; 22+ messages in thread
From: quic_vboma @ 2023-02-02 6:47 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Viswanath Boma, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel
Cc: Vikash Garodia
From: Viswanath Boma <quic_vboma@quicinc.com>
For VP9 bitstreams, there could be a change in resolution at interframe,
for driver to get notified of such resolution change,
enable the property in video firmware.
Also, EOS handling is now made same in video firmware across all V6 SOCs,
hence above a certain firmware version, the driver handling is
made generic for all V6s
Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
Tested-by: Nathan Hebert <nhebert@chromium.org>
---
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 | 12 +++++++++++-
5 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 32551c2602a9..8f94d795cc2b 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_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
index df96db3761a7..07ac0fcd2852 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");
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 4ceaba37e2e5..36c88858ea9d 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;
@@ -671,6 +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
return ret;
}
+ /* Enabling sufficient sequence change support for VP9 */
+ if (of_device_is_compatible(inst->core->dev->of_node, "qcom,sc7180-venus")) {
+ 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.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V2 0/1] Fix for VP9 DRC and Decoder STOP issue.
2023-02-02 6:47 ` [PATCH V2 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2023-02-02 6:47 ` [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
@ 2023-02-02 10:02 ` Dmitry Baryshkov
1 sibling, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-02-02 10:02 UTC (permalink / raw)
To: quic_vboma
Cc: Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson,
Konrad Dybcio, Mauro Carvalho Chehab, linux-media, linux-arm-msm,
linux-kernel
On Thu, 2 Feb 2023 at 08:47, <quic_vboma@quicinc.com> wrote:
>
> From: Viswanath Boma <quic_vboma@quicinc.com>
>
> Tested the changes on v5.15 and v5.4 kernels .
Please test your patches properly. It is 6.2-rc already.
> For testing Chrome Utilities were used .
>
> Viswanath Boma (1):
> venus: Enable sufficient sequence change support for sc7180 and fix
> for Decoder STOP command issue.
>
> 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 | 12 +++++++++++-
> 5 files changed, 41 insertions(+), 3 deletions(-)
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-02-02 6:47 ` [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
@ 2023-02-02 10:10 ` Dmitry Baryshkov
2023-02-03 4:24 ` Viswanath Boma (Temp)
2023-03-16 8:15 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
1 sibling, 1 reply; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-02-02 10:10 UTC (permalink / raw)
To: quic_vboma
Cc: Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson,
Konrad Dybcio, Mauro Carvalho Chehab, linux-media, linux-arm-msm,
linux-kernel, Vikash Garodia
On Thu, 2 Feb 2023 at 08:47, <quic_vboma@quicinc.com> wrote:
>
> From: Viswanath Boma <quic_vboma@quicinc.com>
>
> For VP9 bitstreams, there could be a change in resolution at interframe,
> for driver to get notified of such resolution change,
> enable the property in video firmware.
> Also, EOS handling is now made same in video firmware across all V6 SOCs,
> hence above a certain firmware version, the driver handling is
> made generic for all V6s
>
> Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> Tested-by: Nathan Hebert <nhebert@chromium.org>
> ---
> 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 | 12 +++++++++++-
> 5 files changed, 41 insertions(+), 3 deletions(-)
Several generic comments:
- Please move your work on top of the recent kernels. 5.15 was
released half of the year ago. I'm not going to mention 5.4 age.
- Please split your patch into smaller logical patches.
>
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 32551c2602a9..8f94d795cc2b 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);
Please make the indentation logical here (and below).
Also is 5.6.1 (e.g.) newer than 5.4.51? Or 5.4.1 newer than 4.2.0?
> +}
> +
> +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_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index df96db3761a7..07ac0fcd2852 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);
What about older (V1/V3) cores?
>
> 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");
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba37e2e5..36c88858ea9d 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;
> @@ -671,6 +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
> return ret;
> }
>
> + /* Enabling sufficient sequence change support for VP9 */
> + if (of_device_is_compatible(inst->core->dev->of_node, "qcom,sc7180-venus")) {
Do newer chips support this property? Do you intend to list all of them here?
> + 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;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-02-02 10:10 ` Dmitry Baryshkov
@ 2023-02-03 4:24 ` Viswanath Boma (Temp)
2023-03-16 10:27 ` Dmitry Baryshkov
0 siblings, 1 reply; 22+ messages in thread
From: Viswanath Boma (Temp) @ 2023-02-03 4:24 UTC (permalink / raw)
To: dmitry.baryshkov, Viswanath Boma (Temp) (QUIC)
Cc: stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Konrad Dybcio,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel,
Vikash Garodia
Will ensure more on V3 patch if any comments from Stan/Vikash .
Thanks,
viswanath
-----Original Message-----
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Sent: Thursday, February 2, 2023 3:41 PM
To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>
Cc: stanimir.varbanov@linaro.org; Vikash Garodia (QUIC) <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>; bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>; Mauro Carvalho Chehab <mchehab@kernel.org>; linux-media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-kernel@vger.kernel.org; Vikash Garodia <vgarodia@qti.qualcomm.com>
Subject: Re: [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
On Thu, 2 Feb 2023 at 08:47, <quic_vboma@quicinc.com> wrote:
>
> From: Viswanath Boma <quic_vboma@quicinc.com>
>
> For VP9 bitstreams, there could be a change in resolution at
> interframe, for driver to get notified of such resolution change,
> enable the property in video firmware.
> Also, EOS handling is now made same in video firmware across all V6
> SOCs, hence above a certain firmware version, the driver handling is
> made generic for all V6s
>
> Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> Tested-by: Nathan Hebert <nhebert@chromium.org>
> ---
> 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 | 12 +++++++++++-
> 5 files changed, 41 insertions(+), 3 deletions(-)
Several generic comments:
- Please move your work on top of the recent kernels. 5.15 was released half of the year ago. I'm not going to mention 5.4 age.
- Please split your patch into smaller logical patches.
[vboma]
>> As per the current client environment working on 5.15 kernel and the same changes were also ensured on 5.4 .
>> Current changes related bringing up the utility functions to fix couple of bugs on latest firmware versions.
>> In future , As sugggested will split the changes if they can be isolated as smaller meaningful part .
>
> diff --git a/drivers/media/platform/qcom/venus/core.h
> b/drivers/media/platform/qcom/venus/core.h
> index 32551c2602a9..8f94d795cc2b 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);
Please make the indentation logical here (and below).
Also is 5.6.1 (e.g.) newer than 5.4.51? Or 5.4.1 newer than 4.2.0?
[vboma]
>> Expected one more indent to right ? will ensure .
>> These versions check related to major/minor versions of the Firmware releases to address the mentioned issues and also if any role back preserves the older behavior.
> +}
> +
> +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_cmds.c
> b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c
> b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index df96db3761a7..07ac0fcd2852 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);
What about older (V1/V3) cores?
[vboma]
>> Older cores not having these issues , hence as required V4 and V6 were handled as per Current client issues.
>
> 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"); diff
> --git a/drivers/media/platform/qcom/venus/vdec.c
> b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba37e2e5..36c88858ea9d 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; @@ -671,6
> +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
> return ret;
> }
>
> + /* Enabling sufficient sequence change support for VP9 */
> + if (of_device_is_compatible(inst->core->dev->of_node,
> + "qcom,sc7180-venus")) {
Do newer chips support this property? Do you intend to list all of them here?
[vboma]
>> Basing on capability of the valid chipset vs firmware support ,current changes were added .
> + 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;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue.
2023-02-02 6:47 ` [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
2023-02-02 10:10 ` Dmitry Baryshkov
@ 2023-03-16 8:15 ` quic_vboma
2023-03-16 8:15 ` [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
` (2 more replies)
1 sibling, 3 replies; 22+ messages in thread
From: quic_vboma @ 2023-03-16 8:15 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Viswanath Boma, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel
From: Viswanath Boma <quic_vboma@quicinc.com>
Fixed indent comments, ensured rebase and checkpatch with --strict.
Tested the changes on v5.15 and v5.4 kernels .
For testing Chrome Utilities were used .
Viswanath Boma (1):
venus: Enable sufficient sequence change support for sc7180 and fix
for Decoder STOP command issue.
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 | 12 +++++++++++-
5 files changed, 41 insertions(+), 3 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-03-16 8:15 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
@ 2023-03-16 8:15 ` quic_vboma
2023-03-16 10:20 ` Dmitry Baryshkov
2023-03-16 11:53 ` Bryan O'Donoghue
2023-03-16 8:41 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue Konrad Dybcio
2023-03-16 12:37 ` Dmitry Baryshkov
2 siblings, 2 replies; 22+ messages in thread
From: quic_vboma @ 2023-03-16 8:15 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Viswanath Boma, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel
Cc: Vikash Garodia
From: Viswanath Boma <quic_vboma@quicinc.com>
For VP9 bitstreams, there could be a change in resolution at interframe,
for driver to get notified of such resolution change,
enable the property in video firmware.
Also, EOS handling is now made same in video firmware across all V6 SOCs,
hence above a certain firmware version, the driver handling is
made generic for all V6s
Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
Tested-by: Nathan Hebert <nhebert@chromium.org>
---
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 | 12 +++++++++++-
5 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 32551c2602a9..ee8b70a34656 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_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
index df96db3761a7..07ac0fcd2852 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");
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 4ceaba37e2e5..36c88858ea9d 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;
@@ -671,6 +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
return ret;
}
+ /* Enabling sufficient sequence change support for VP9 */
+ if (of_device_is_compatible(inst->core->dev->of_node, "qcom,sc7180-venus")) {
+ 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.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue.
2023-03-16 8:15 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2023-03-16 8:15 ` [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
@ 2023-03-16 8:41 ` Konrad Dybcio
2023-03-16 12:37 ` Dmitry Baryshkov
2 siblings, 0 replies; 22+ messages in thread
From: Konrad Dybcio @ 2023-03-16 8:41 UTC (permalink / raw)
To: quic_vboma, Stanimir Varbanov, Vikash Garodia, Andy Gross,
Bjorn Andersson, Mauro Carvalho Chehab, linux-media,
linux-arm-msm, linux-kernel
On 16.03.2023 09:15, quic_vboma@quicinc.com wrote:
> From: Viswanath Boma <quic_vboma@quicinc.com>
Please fix your git send-email config to include your first and last
name in the emails you send.
Please send next versions as a standalone thread, not in reply to the
previous ones.
>
> Fixed indent comments, ensured rebase and checkpatch with --strict.
> Tested the changes on v5.15 and v5.4 kernels .
You're submitting changes to the mainline kernel, not to v5.15 or v5.4.
Please validate it against linux-next/master.
Konrad
> For testing Chrome Utilities were used .
>
> Viswanath Boma (1):
> venus: Enable sufficient sequence change support for sc7180 and fix
> for Decoder STOP command issue.
>
> 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 | 12 +++++++++++-
> 5 files changed, 41 insertions(+), 3 deletions(-)
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-03-16 8:15 ` [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
@ 2023-03-16 10:20 ` Dmitry Baryshkov
2023-03-23 9:16 ` Viswanath Boma (Temp)
2023-03-16 11:53 ` Bryan O'Donoghue
1 sibling, 1 reply; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-03-16 10:20 UTC (permalink / raw)
To: quic_vboma
Cc: Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson,
Konrad Dybcio, Mauro Carvalho Chehab, linux-media, linux-arm-msm,
linux-kernel, Vikash Garodia
On Thu, 16 Mar 2023 at 10:17, <quic_vboma@quicinc.com> wrote:
>
> From: Viswanath Boma <quic_vboma@quicinc.com>
>
> For VP9 bitstreams, there could be a change in resolution at interframe,
> for driver to get notified of such resolution change,
> enable the property in video firmware.
> Also, EOS handling is now made same in video firmware across all V6 SOCs,
> hence above a certain firmware version, the driver handling is
> made generic for all V6s
>
> Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> Tested-by: Nathan Hebert <nhebert@chromium.org>
> ---
Which version of the patch is this? Were there any changes compared to
the previous version? Please include a changelog below the dashed line
to let other people know what has changed
> 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 | 12 +++++++++++-
> 5 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 32551c2602a9..ee8b70a34656 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_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index df96db3761a7..07ac0fcd2852 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");
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba37e2e5..36c88858ea9d 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;
> @@ -671,6 +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
> return ret;
> }
>
> + /* Enabling sufficient sequence change support for VP9 */
> + if (of_device_is_compatible(inst->core->dev->of_node, "qcom,sc7180-venus")) {
Is it really specific just to sc7180 or will it be applicable to any
other platform using venus-5.4 firmware?
> + 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.17.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-02-03 4:24 ` Viswanath Boma (Temp)
@ 2023-03-16 10:27 ` Dmitry Baryshkov
2023-03-23 9:19 ` Viswanath Boma (Temp)
0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-03-16 10:27 UTC (permalink / raw)
To: Viswanath Boma (Temp)
Cc: Viswanath Boma (Temp) (QUIC),
stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Konrad Dybcio,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel,
Vikash Garodia
On Fri, 3 Feb 2023 at 06:24, Viswanath Boma (Temp)
<vboma@qti.qualcomm.com> wrote:
>
>
> Will ensure more on V3 patch if any comments from Stan/Vikash .
> Thanks,
> viswanath
Please fix your email configuration. Top-posting is generally frowned
upon. All the headers bellow are frowned upon too.
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Sent: Thursday, February 2, 2023 3:41 PM
> To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>
> Cc: stanimir.varbanov@linaro.org; Vikash Garodia (QUIC) <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>; bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>; Mauro Carvalho Chehab <mchehab@kernel.org>; linux-media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-kernel@vger.kernel.org; Vikash Garodia <vgarodia@qti.qualcomm.com>
> Subject: Re: [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> On Thu, 2 Feb 2023 at 08:47, <quic_vboma@quicinc.com> wrote:
> >
> > From: Viswanath Boma <quic_vboma@quicinc.com>
> >
> > For VP9 bitstreams, there could be a change in resolution at
> > interframe, for driver to get notified of such resolution change,
> > enable the property in video firmware.
> > Also, EOS handling is now made same in video firmware across all V6
> > SOCs, hence above a certain firmware version, the driver handling is
> > made generic for all V6s
> >
> > Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
> > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> > Tested-by: Nathan Hebert <nhebert@chromium.org>
> > ---
> > 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 | 12 +++++++++++-
> > 5 files changed, 41 insertions(+), 3 deletions(-)
>
> Several generic comments:
> - Please move your work on top of the recent kernels. 5.15 was released half of the year ago. I'm not going to mention 5.4 age.
> - Please split your patch into smaller logical patches.
> [vboma]
> >> As per the current client environment working on 5.15 kernel and the same changes were also ensured on 5.4 .
> >> Current changes related bringing up the utility functions to fix couple of bugs on latest firmware versions.
> >> In future , As sugggested will split the changes if they can be isolated as smaller meaningful part .
UGH. This looks like a third level quotation, not like your reply.
Could you please spend some time and fix your email client? It is
close to impossible to notice your replies otherwise.
Were these changes validated on top of linux-master or linux-next? If
not, please go ahead and validate them before sending the next patch
revision.
> >
> > diff --git a/drivers/media/platform/qcom/venus/core.h
> > b/drivers/media/platform/qcom/venus/core.h
> > index 32551c2602a9..8f94d795cc2b 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);
>
> Please make the indentation logical here (and below).
> Also is 5.6.1 (e.g.) newer than 5.4.51? Or 5.4.1 newer than 4.2.0?
> [vboma]
> >> Expected one more indent to right ? will ensure .
Expected is to have the indentation logical. Saying if ((major == A)
&& (minor ==
B)) is not logical.
A good (and easier to comprehend) example might be:
if ((major == A) &&
(minor == B)) {
}
> >> These versions check related to major/minor versions of the Firmware releases to address the mentioned issues and also if any role back preserves the older behavior.
>
> > +}
> > +
> > +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_cmds.c
> > b/drivers/media/platform/qcom/venus/hfi_cmds.c
> > index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c
> > b/drivers/media/platform/qcom/venus/hfi_msgs.c
> > index df96db3761a7..07ac0fcd2852 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);
>
> What about older (V1/V3) cores?
> [vboma]
> >> Older cores not having these issues , hence as required V4 and V6 were handled as per Current client issues.
There are no clients here and no client issues. If you are handling
parsing versions, please make it work for all supported devices.
>
> >
> > 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"); diff
> > --git a/drivers/media/platform/qcom/venus/vdec.c
> > b/drivers/media/platform/qcom/venus/vdec.c
> > index 4ceaba37e2e5..36c88858ea9d 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; @@ -671,6
> > +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
> > return ret;
> > }
> >
> > + /* Enabling sufficient sequence change support for VP9 */
> > + if (of_device_is_compatible(inst->core->dev->of_node,
> > + "qcom,sc7180-venus")) {
>
> Do newer chips support this property? Do you intend to list all of them here?
> [vboma]
> >> Basing on capability of the valid chipset vs firmware support ,current changes were added .
Are there any other chipsets using 5.4 firmware. If they are, are you
going to list them here? If not, you can skip the
of_device_is_compatible and just check for fw 5.4
>
> > + 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;
>
>
>
> --
> With best wishes
> Dmitry
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-03-16 8:15 ` [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
2023-03-16 10:20 ` Dmitry Baryshkov
@ 2023-03-16 11:53 ` Bryan O'Donoghue
2023-03-16 16:26 ` Vikash Garodia
1 sibling, 1 reply; 22+ messages in thread
From: Bryan O'Donoghue @ 2023-03-16 11:53 UTC (permalink / raw)
To: quic_vboma, Stanimir Varbanov, Vikash Garodia, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel
Cc: Vikash Garodia, Dikshita Agarwal
On 16/03/2023 08:15, quic_vboma@quicinc.com wrote:
> + if (IS_V4(core))
Hi Viswanath,
Could you please take in the change to base on on IRIS version and
rebase your patch on _that_ much at least, not necessarily all of the
changes in the series below. Dikshita should be able help.
https://lore.kernel.org/linux-arm-msm/c9c324aa-6192-f878-9189-635626e76b13@quicinc.com/
IRIS version is more granular/accurate than V4/V6 etc.
---
bod
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue.
2023-03-16 8:15 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2023-03-16 8:15 ` [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
2023-03-16 8:41 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue Konrad Dybcio
@ 2023-03-16 12:37 ` Dmitry Baryshkov
2023-03-23 9:22 ` Viswanath Boma (Temp)
2 siblings, 1 reply; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-03-16 12:37 UTC (permalink / raw)
To: quic_vboma, Stanimir Varbanov, Vikash Garodia, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel
On 16/03/2023 10:15, quic_vboma@quicinc.com wrote:
> From: Viswanath Boma <quic_vboma@quicinc.com>
>
> Fixed indent comments, ensured rebase and checkpatch with --strict.
> Tested the changes on v5.15 and v5.4 kernels .
Was it tested on top of the recent kernels?
> For testing Chrome Utilities were used .
>
> Viswanath Boma (1):
> venus: Enable sufficient sequence change support for sc7180 and fix
> for Decoder STOP command issue.
>
> 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 | 12 +++++++++++-
> 5 files changed, 41 insertions(+), 3 deletions(-)
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-03-16 11:53 ` Bryan O'Donoghue
@ 2023-03-16 16:26 ` Vikash Garodia
0 siblings, 0 replies; 22+ messages in thread
From: Vikash Garodia @ 2023-03-16 16:26 UTC (permalink / raw)
To: bryan.odonoghue, Viswanath Boma (Temp) (QUIC),
stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Konrad Dybcio,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel
Cc: Dikshita Agarwal
Hi Bryan,
>-----Original Message-----
>From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>Sent: Thursday, March 16, 2023 5:24 PM
>To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>;
>stanimir.varbanov@linaro.org; Vikash Garodia (QUIC)
><quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>;
>bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>;
>Mauro Carvalho Chehab <mchehab@kernel.org>; linux-media@vger.kernel.org;
>linux-arm-msm@vger.kernel.org; linux-kernel@vger.kernel.org
>Cc: Vikash Garodia <vgarodia@qti.qualcomm.com>; Dikshita Agarwal
><dikshita@qti.qualcomm.com>
>Subject: Re: [PATCH] venus: Enable sufficient sequence change support for
>sc7180 and fix for Decoder STOP command issue.
>
>WARNING: This email originated from outside of Qualcomm. Please be wary of
>any links or attachments, and do not enable macros.
>
>On 16/03/2023 08:15, quic_vboma@quicinc.com wrote:
>> + if (IS_V4(core))
>
>Hi Viswanath,
>
>Could you please take in the change to base on on IRIS version and rebase your
>patch on _that_ much at least, not necessarily all of the changes in the series
>below. Dikshita should be able help.
Let have this fix go separately and not tie this with the AR50LT series. This fix is a pending
for sometime due to delay in raising the pull request. Working with Stan to get the pull
request raised in next couple of days.
>https://lore.kernel.org/linux-arm-msm/c9c324aa-6192-f878-9189-
>635626e76b13@quicinc.com/
>
>IRIS version is more granular/accurate than V4/V6 etc.
>
>---
>bod
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-03-16 10:20 ` Dmitry Baryshkov
@ 2023-03-23 9:16 ` Viswanath Boma (Temp)
2023-03-23 16:18 ` Konrad Dybcio
0 siblings, 1 reply; 22+ messages in thread
From: Viswanath Boma (Temp) @ 2023-03-23 9:16 UTC (permalink / raw)
To: dmitry.baryshkov, Viswanath Boma (Temp) (QUIC)
Cc: stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Konrad Dybcio,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel,
Vikash Garodia
Hi Dmirty ,
Thanks for reviews .
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Sent: Thursday, March 16, 2023 3:50 PM
> To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>
> Cc: stanimir.varbanov@linaro.org; Vikash Garodia (QUIC)
> <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>;
> bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>;
> Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-
> kernel@vger.kernel.org; Vikash Garodia <vgarodia@qti.qualcomm.com>
> Subject: Re: [PATCH] venus: Enable sufficient sequence change support for
> sc7180 and fix for Decoder STOP command issue.
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
>
> On Thu, 16 Mar 2023 at 10:17, <quic_vboma@quicinc.com> wrote:
> >
> > From: Viswanath Boma <quic_vboma@quicinc.com>
> >
> > For VP9 bitstreams, there could be a change in resolution at
> > interframe, for driver to get notified of such resolution change,
> > enable the property in video firmware.
> > Also, EOS handling is now made same in video firmware across all V6
> > SOCs, hence above a certain firmware version, the driver handling is
> > made generic for all V6s
> >
> > Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
> > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> > Tested-by: Nathan Hebert <nhebert@chromium.org>
> > ---
>
> Which version of the patch is this? Were there any changes compared to the
> previous version? Please include a changelog below the dashed line to let other
> people know what has changed
>
This is version v2 or v3 --> Keep this as per the version.
Sure, let me fix the version and changes info in the next revision.
> > 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 | 12 +++++++++++-
> > 5 files changed, 41 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/venus/core.h
> > b/drivers/media/platform/qcom/venus/core.h
> > index 32551c2602a9..ee8b70a34656 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_cmds.c
> > b/drivers/media/platform/qcom/venus/hfi_cmds.c
> > index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c
> > b/drivers/media/platform/qcom/venus/hfi_msgs.c
> > index df96db3761a7..07ac0fcd2852 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"); diff
> > --git a/drivers/media/platform/qcom/venus/vdec.c
> > b/drivers/media/platform/qcom/venus/vdec.c
> > index 4ceaba37e2e5..36c88858ea9d 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; @@ -671,6
> > +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
> > return ret;
> > }
> >
> > + /* Enabling sufficient sequence change support for VP9 */
> > + if (of_device_is_compatible(inst->core->dev->of_node,
> > + "qcom,sc7180-venus")) {
>
> Is it really specific just to sc7180 or will it be applicable to any other platform
> using venus-5.4 firmware?
>
> > + 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.17.1
> >
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-03-16 10:27 ` Dmitry Baryshkov
@ 2023-03-23 9:19 ` Viswanath Boma (Temp)
0 siblings, 0 replies; 22+ messages in thread
From: Viswanath Boma (Temp) @ 2023-03-23 9:19 UTC (permalink / raw)
To: dmitry.baryshkov
Cc: Viswanath Boma (Temp) (QUIC),
stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Konrad Dybcio,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel,
Vikash Garodia
HI Dmirty ,
Thanks for reviews .
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Sent: Thursday, March 16, 2023 3:58 PM
> To: Viswanath Boma (Temp) <vboma@qti.qualcomm.com>
> Cc: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>;
> stanimir.varbanov@linaro.org; Vikash Garodia (QUIC)
> <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>;
> bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>;
> Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-
> kernel@vger.kernel.org; Vikash Garodia <vgarodia@qti.qualcomm.com>
> Subject: Re: [PATCH V2 1/1] venus: Enable sufficient sequence change support
> for sc7180 and fix for Decoder STOP command issue.
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
>
> On Fri, 3 Feb 2023 at 06:24, Viswanath Boma (Temp)
> <vboma@qti.qualcomm.com> wrote:
> >
> >
> > Will ensure more on V3 patch if any comments from Stan/Vikash .
> > Thanks,
> > viswanath
>
> Please fix your email configuration. Top-posting is generally frowned upon. All
> the headers bellow are frowned upon too.
>
Sure, will ensure proper version tag in the next patch set.
> > -----Original Message-----
> > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Sent: Thursday, February 2, 2023 3:41 PM
> > To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>
> > Cc: stanimir.varbanov@linaro.org; Vikash Garodia (QUIC)
> > <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>;
> > bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>;
> > Mauro Carvalho Chehab <mchehab@kernel.org>;
> > linux-media@vger.kernel.org; linux-arm-msm@vger.kernel.org;
> > linux-kernel@vger.kernel.org; Vikash Garodia
> > <vgarodia@qti.qualcomm.com>
> > Subject: Re: [PATCH V2 1/1] venus: Enable sufficient sequence change support
> for sc7180 and fix for Decoder STOP command issue.
> >
> > WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> >
> > On Thu, 2 Feb 2023 at 08:47, <quic_vboma@quicinc.com> wrote:
> > >
> > > From: Viswanath Boma <quic_vboma@quicinc.com>
> > >
> > > For VP9 bitstreams, there could be a change in resolution at
> > > interframe, for driver to get notified of such resolution change,
> > > enable the property in video firmware.
> > > Also, EOS handling is now made same in video firmware across all V6
> > > SOCs, hence above a certain firmware version, the driver handling is
> > > made generic for all V6s
> > >
> > > Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
> > > Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> > > Tested-by: Nathan Hebert <nhebert@chromium.org>
> > > ---
> > > 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 | 12 +++++++++++-
> > > 5 files changed, 41 insertions(+), 3 deletions(-)
> >
> > Several generic comments:
> > - Please move your work on top of the recent kernels. 5.15 was released half
> of the year ago. I'm not going to mention 5.4 age.
> > - Please split your patch into smaller logical patches.
> > [vboma]
> > >> As per the current client environment working on 5.15 kernel and the
> same changes were also ensured on 5.4 .
> > >> Current changes related bringing up the utility functions to fix couple of
> bugs on latest firmware versions.
> > >> In future , As sugggested will split the changes if they can be isolated as
> smaller meaningful part .
>
> UGH. This looks like a third level quotation, not like your reply.
> Could you please spend some time and fix your email client? It is close to
> impossible to notice your replies otherwise.
>
> Were these changes validated on top of linux-master or linux-next? If not,
> please go ahead and validate them before sending the next patch revision.
>
>
> > >
> > > diff --git a/drivers/media/platform/qcom/venus/core.h
> > > b/drivers/media/platform/qcom/venus/core.h
> > > index 32551c2602a9..8f94d795cc2b 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);
> >
> > Please make the indentation logical here (and below).
> > Also is 5.6.1 (e.g.) newer than 5.4.51? Or 5.4.1 newer than 4.2.0?
> > [vboma]
> > >> Expected one more indent to right ? will ensure .
>
> Expected is to have the indentation logical. Saying if ((major == A) && (minor
> ==
> B)) is not logical.
>
> A good (and easier to comprehend) example might be:
>
> if ((major == A) &&
> (minor == B)) {
> }
>
> > >> These versions check related to major/minor versions of the Firmware
> releases to address the mentioned issues and also if any role back preserves
> the older behavior.
> >
> > > +}
> > > +
> > > +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_cmds.c
> > > b/drivers/media/platform/qcom/venus/hfi_cmds.c
> > > index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c
> > > b/drivers/media/platform/qcom/venus/hfi_msgs.c
> > > index df96db3761a7..07ac0fcd2852 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);
> >
> > What about older (V1/V3) cores?
> > [vboma]
> > >> Older cores not having these issues , hence as required V4 and V6 were
> handled as per Current client issues.
>
> There are no clients here and no client issues. If you are handling parsing
> versions, please make it work for all supported devices.
>
> >
> > >
> > > 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"); diff
> > > --git a/drivers/media/platform/qcom/venus/vdec.c
> > > b/drivers/media/platform/qcom/venus/vdec.c
> > > index 4ceaba37e2e5..36c88858ea9d 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; @@ -671,6
> > > +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
> > > return ret;
> > > }
> > >
> > > + /* Enabling sufficient sequence change support for VP9 */
> > > + if (of_device_is_compatible(inst->core->dev->of_node,
> > > + "qcom,sc7180-venus")) {
> >
> > Do newer chips support this property? Do you intend to list all of them here?
> > [vboma]
> > >> Basing on capability of the valid chipset vs firmware support ,current
> changes were added .
>
> Are there any other chipsets using 5.4 firmware. If they are, are you going to
> list them here? If not, you can skip the of_device_is_compatible and just check
> for fw 5.4
>
> >
> > > + 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;
> >
> >
> >
> > --
> > With best wishes
> > Dmitry
>
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue.
2023-03-16 12:37 ` Dmitry Baryshkov
@ 2023-03-23 9:22 ` Viswanath Boma (Temp)
2023-03-23 9:37 ` Dmitry Baryshkov
0 siblings, 1 reply; 22+ messages in thread
From: Viswanath Boma (Temp) @ 2023-03-23 9:22 UTC (permalink / raw)
To: dmitry.baryshkov, Viswanath Boma (Temp) (QUIC),
stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Konrad Dybcio,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel
HI Dmirty,
Thanks for Reviews .
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Sent: Thursday, March 16, 2023 6:08 PM
> To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>;
> stanimir.varbanov@linaro.org; Vikash Garodia (QUIC)
> <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>;
> bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>;
> Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue.
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
>
> On 16/03/2023 10:15, quic_vboma@quicinc.com wrote:
> > From: Viswanath Boma <quic_vboma@quicinc.com>
> >
> > Fixed indent comments, ensured rebase and checkpatch with --strict.
> > Tested the changes on v5.15 and v5.4 kernels .
>
> Was it tested on top of the recent kernels?
>
Yes, Ensured on the latest .
> > For testing Chrome Utilities were used .
> >
> > Viswanath Boma (1):
> > venus: Enable sufficient sequence change support for sc7180 and fix
> > for Decoder STOP command issue.
> >
> > 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 | 12 +++++++++++-
> > 5 files changed, 41 insertions(+), 3 deletions(-)
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue.
2023-03-23 9:22 ` Viswanath Boma (Temp)
@ 2023-03-23 9:37 ` Dmitry Baryshkov
0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-03-23 9:37 UTC (permalink / raw)
To: Viswanath Boma (Temp)
Cc: Viswanath Boma (Temp) (QUIC),
stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Konrad Dybcio,
Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel
On Thu, 23 Mar 2023 at 11:22, Viswanath Boma (Temp)
<vboma@qti.qualcomm.com> wrote:
>
> HI Dmirty,
>
> Thanks for Reviews .
>
> > -----Original Message-----
> > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Sent: Thursday, March 16, 2023 6:08 PM
> > To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>;
> > stanimir.varbanov@linaro.org; Vikash Garodia (QUIC)
> > <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>;
> > bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>;
> > Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
> > media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue.
> >
> > WARNING: This email originated from outside of Qualcomm. Please be wary of
> > any links or attachments, and do not enable macros.
Can we please get rid of this in the replies? There is no need to
duplicate headers.
> >
> > On 16/03/2023 10:15, quic_vboma@quicinc.com wrote:
> > > From: Viswanath Boma <quic_vboma@quicinc.com>
> > >
> > > Fixed indent comments, ensured rebase and checkpatch with --strict.
> > > Tested the changes on v5.15 and v5.4 kernels .
> >
> > Was it tested on top of the recent kernels?
> >
> Yes, Ensured on the latest .
Then why do you mention old kernels at all? Also the email addresses
you have used do not correspond to the latest kernels.
>
> > > For testing Chrome Utilities were used .
> > >
> > > Viswanath Boma (1):
> > > venus: Enable sufficient sequence change support for sc7180 and fix
> > > for Decoder STOP command issue.
> > >
> > > 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 | 12 +++++++++++-
> > > 5 files changed, 41 insertions(+), 3 deletions(-)
> > >
> >
> > --
> > With best wishes
> > Dmitry
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue.
2023-03-23 9:16 ` Viswanath Boma (Temp)
@ 2023-03-23 16:18 ` Konrad Dybcio
0 siblings, 0 replies; 22+ messages in thread
From: Konrad Dybcio @ 2023-03-23 16:18 UTC (permalink / raw)
To: Viswanath Boma (Temp), dmitry.baryshkov, Viswanath Boma (Temp) (QUIC)
Cc: stanimir.varbanov, Vikash Garodia (QUIC),
Andy Gross, bjorn.andersson, Mauro Carvalho Chehab, linux-media,
linux-arm-msm, linux-kernel, Vikash Garodia
On 23.03.2023 10:16, Viswanath Boma (Temp) wrote:
> Hi Dmirty ,
>
> Thanks for reviews .
Please don't top-post.
A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
A: No.
Q: Should I include quotations after my reply?
Konrad
>
>> -----Original Message-----
>> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Sent: Thursday, March 16, 2023 3:50 PM
>> To: Viswanath Boma (Temp) (QUIC) <quic_vboma@quicinc.com>
>> Cc: stanimir.varbanov@linaro.org; Vikash Garodia (QUIC)
>> <quic_vgarodia@quicinc.com>; Andy Gross <agross@kernel.org>;
>> bjorn.andersson@linaro.org; Konrad Dybcio <konrad.dybcio@linaro.org>;
>> Mauro Carvalho Chehab <mchehab@kernel.org>; linux-
>> media@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-
>> kernel@vger.kernel.org; Vikash Garodia <vgarodia@qti.qualcomm.com>
>> Subject: Re: [PATCH] venus: Enable sufficient sequence change support for
>> sc7180 and fix for Decoder STOP command issue.
>>
>> WARNING: This email originated from outside of Qualcomm. Please be wary of
>> any links or attachments, and do not enable macros.
>>
>> On Thu, 16 Mar 2023 at 10:17, <quic_vboma@quicinc.com> wrote:
>>>
>>> From: Viswanath Boma <quic_vboma@quicinc.com>
>>>
>>> For VP9 bitstreams, there could be a change in resolution at
>>> interframe, for driver to get notified of such resolution change,
>>> enable the property in video firmware.
>>> Also, EOS handling is now made same in video firmware across all V6
>>> SOCs, hence above a certain firmware version, the driver handling is
>>> made generic for all V6s
>>>
>>> Signed-off-by: Vikash Garodia <vgarodia@qti.qualcomm.com>
>>> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
>>> Tested-by: Nathan Hebert <nhebert@chromium.org>
>>> ---
>>
>> Which version of the patch is this? Were there any changes compared to the
>> previous version? Please include a changelog below the dashed line to let other
>> people know what has changed
>>
> This is version v2 or v3 --> Keep this as per the version.
> Sure, let me fix the version and changes info in the next revision.
>
>>> 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 | 12 +++++++++++-
>>> 5 files changed, 41 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/core.h
>>> b/drivers/media/platform/qcom/venus/core.h
>>> index 32551c2602a9..ee8b70a34656 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_cmds.c
>>> b/drivers/media/platform/qcom/venus/hfi_cmds.c
>>> index 930b743f225e..e2539b58340f 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 d2d6719a2ba4..20516b4361d3 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/hfi_msgs.c
>>> b/drivers/media/platform/qcom/venus/hfi_msgs.c
>>> index df96db3761a7..07ac0fcd2852 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"); diff
>>> --git a/drivers/media/platform/qcom/venus/vdec.c
>>> b/drivers/media/platform/qcom/venus/vdec.c
>>> index 4ceaba37e2e5..36c88858ea9d 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; @@ -671,6
>>> +671,16 @@ static int vdec_set_properties(struct venus_inst *inst)
>>> return ret;
>>> }
>>>
>>> + /* Enabling sufficient sequence change support for VP9 */
>>> + if (of_device_is_compatible(inst->core->dev->of_node,
>>> + "qcom,sc7180-venus")) {
>>
>> Is it really specific just to sc7180 or will it be applicable to any other platform
>> using venus-5.4 firmware?
>>
>>> + 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.17.1
>>>
>>
>>
>> --
>> With best wishes
>> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-03-23 16:18 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-15 12:12 [PATCH 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2022-11-15 12:12 ` [PATCH 1/1] venus: Enable sufficient sequence change support for sc7180 and Fix for Decoder STOP command issue quic_vboma
2022-12-02 0:54 ` Nathan Hebert
2022-12-02 1:03 ` Bryan O'Donoghue
2023-02-02 6:47 ` [PATCH V2 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2023-02-02 6:47 ` [PATCH V2 1/1] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
2023-02-02 10:10 ` Dmitry Baryshkov
2023-02-03 4:24 ` Viswanath Boma (Temp)
2023-03-16 10:27 ` Dmitry Baryshkov
2023-03-23 9:19 ` Viswanath Boma (Temp)
2023-03-16 8:15 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue quic_vboma
2023-03-16 8:15 ` [PATCH] venus: Enable sufficient sequence change support for sc7180 and fix for Decoder STOP command issue quic_vboma
2023-03-16 10:20 ` Dmitry Baryshkov
2023-03-23 9:16 ` Viswanath Boma (Temp)
2023-03-23 16:18 ` Konrad Dybcio
2023-03-16 11:53 ` Bryan O'Donoghue
2023-03-16 16:26 ` Vikash Garodia
2023-03-16 8:41 ` [PATCH V3 0/1] Fix for VP9 DRC and Decoder STOP issue Konrad Dybcio
2023-03-16 12:37 ` Dmitry Baryshkov
2023-03-23 9:22 ` Viswanath Boma (Temp)
2023-03-23 9:37 ` Dmitry Baryshkov
2023-02-02 10:02 ` [PATCH V2 " Dmitry Baryshkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.