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@xxxxxxxxxx> >> Sent: Thursday, March 16, 2023 3:50 PM >> To: Viswanath Boma (Temp) (QUIC) <quic_vboma@xxxxxxxxxxx> >> Cc: stanimir.varbanov@xxxxxxxxxx; Vikash Garodia (QUIC) >> <quic_vgarodia@xxxxxxxxxxx>; Andy Gross <agross@xxxxxxxxxx>; >> bjorn.andersson@xxxxxxxxxx; Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>; >> Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>; linux- >> media@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx; linux- >> kernel@xxxxxxxxxxxxxxx; Vikash Garodia <vgarodia@xxxxxxxxxxxxxxxx> >> 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@xxxxxxxxxxx> wrote: >>> >>> From: Viswanath Boma <quic_vboma@xxxxxxxxxxx> >>> >>> 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@xxxxxxxxxxxxxxxx> >>> Signed-off-by: Viswanath Boma <quic_vboma@xxxxxxxxxxx> >>> Tested-by: Nathan Hebert <nhebert@xxxxxxxxxxxx> >>> --- >> >> 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
