Hi Dmirty ,
Thanks for reviews .
> -----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
