RE: [PATCH] venus: Add support for min/max qp range.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dmitry,

Thanks for your reviews.

> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> Sent: Thursday, March 16, 2023 4:11 PM
> To: Viswanath Boma (Temp) (QUIC) <quic_vboma@xxxxxxxxxxx>
> Cc: stanimir.varbanov@xxxxxxxxxx; Andy Gross <agross@xxxxxxxxxx>;
> bjorn.andersson@xxxxxxxxxx; Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>;
> linux-media@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Vikash Garodia (QUIC) <quic_vgarodia@xxxxxxxxxxx>
> Subject: Re: [PATCH] venus: Add support for min/max qp range.
> 
> 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:27, <quic_vboma@xxxxxxxxxxx> wrote:
> >
> > From: Viswanath Boma <quic_vboma@xxxxxxxxxxx>
> >
> > This change enables the support on firmware. Client's qp range values
> > will be set at session level by the driver.
> >
> > Signed-off-by: Viswanath Boma <quic_vboma@xxxxxxxxxxx>
> > Signed-off-by: Vikash Garodia <quic_vgarodia@xxxxxxxxxxx>
> > ---
> >  drivers/media/platform/qcom/venus/hfi_cmds.c  | 27 +++++++++++-
> >  .../media/platform/qcom/venus/hfi_helper.h    | 18 ++++++++
> >  drivers/media/platform/qcom/venus/venc.c      | 41 +++++++++++++++----
> >  3 files changed, 77 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c
> > b/drivers/media/platform/qcom/venus/hfi_cmds.c
> > index 930b743f225e..98ad4f4fba0a 100644
> > --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
> > +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
> > @@ -1189,6 +1189,7 @@ pkt_session_set_property_4xx(struct
> hfi_session_set_property_pkt *pkt,
> >                              void *cookie, u32 ptype, void *pdata)  {
> >         void *prop_data;
> > +       int ret = 0;
> >
> >         if (!pkt || !cookie || !pdata)
> >                 return -EINVAL;
> > @@ -1257,7 +1258,31 @@ pkt_session_set_property_4xx(struct
> hfi_session_set_property_pkt *pkt,
> >                 pkt->shdr.hdr.size += sizeof(u32) + sizeof(*tm);
> >                 break;
> >         }
> > +       case HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE_V2: {
> > +               struct hfi_quantization_range_v2 *in = pdata, *range = prop_data;
> > +               u32 min_qp, max_qp;
> >
> > +               min_qp = in->min_qp.qp_packed;
> > +               max_qp = in->max_qp.qp_packed;
> > +
> > +               /* We'll be packing in the qp, so make sure we
> > +                * won't be losing data when masking
> > +                */
> > +               if (min_qp > 0xff || max_qp > 0xff) {
> > +                       ret = -ERANGE;
> 
> Do you need any processing after the switchase? Can you just return
> -ERANGE here?
Ok, we can return from here. Will update in the next version set  by including any other comments from the team .
> > +                       break;
> > +               }
> > +               range->min_qp.layer_id = 0xFF;
> > +               range->max_qp.layer_id = 0xFF;
> > +               range->min_qp.qp_packed = (min_qp & 0xFF) | ((min_qp & 0xFF)
> << 8) |
> > +                       ((min_qp & 0xFF) << 16);
> > +               range->max_qp.qp_packed = (max_qp & 0xFF) | ((max_qp & 0xFF)
> << 8) |
> > +                       ((max_qp & 0xFF) << 16);
> > +               range->min_qp.enable = 7;
> > +               range->max_qp.enable = 7;
> > +               pkt->shdr.hdr.size += sizeof(u32) + sizeof(*range);
> > +               break;
> > +       }
> >         case HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE:
> >         case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER:
> >         case HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE:
> > @@ -1269,7 +1294,7 @@ pkt_session_set_property_4xx(struct
> hfi_session_set_property_pkt *pkt,
> >                 return pkt_session_set_property_3xx(pkt, cookie, ptype, pdata);
> >         }
> >
> > -       return 0;
> > +       return ret;
> >  }
> >
> >  static int
> > diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h
> b/drivers/media/platform/qcom/venus/hfi_helper.h
> > index d2d6719a2ba4..105792a68060 100644
> > --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> > +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> > @@ -487,6 +487,11 @@
> >  #define HFI_PROPERTY_PARAM_VENC_SESSION_QP                     0x2005006
> >  #define HFI_PROPERTY_PARAM_VENC_MPEG4_AC_PREDICTION
> 0x2005007
> >  #define HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE
> 0x2005008
> > +/*
> > + * Note: HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE_V2 is
> > + * specific to HFI_VERSION_6XX and HFI_VERSION_4XX only
> > + */
> > +#define HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE_V2
> 0x2005009
> >  #define HFI_PROPERTY_PARAM_VENC_MPEG4_TIME_RESOLUTION
> 0x2005009
> >  #define HFI_PROPERTY_PARAM_VENC_MPEG4_SHORT_HEADER
> 0x200500a
> >  #define HFI_PROPERTY_PARAM_VENC_MPEG4_HEADER_EXTENSION
> 0x200500b
> > @@ -827,6 +832,19 @@ struct hfi_quantization_range {
> >         u32 layer_id;
> >  };
> >
> > +struct hfi_quantization_v2 {
> > +       u32 qp_packed;
> > +       u32 layer_id;
> > +       u32 enable;
> > +       u32 reserved[3];
> > +};
> > +
> > +struct hfi_quantization_range_v2 {
> > +       struct hfi_quantization_v2 min_qp;
> > +       struct hfi_quantization_v2 max_qp;
> > +       u32 reserved[4];
> > +};
> > +
> >  #define HFI_LTR_MODE_DISABLE   0x0
> >  #define HFI_LTR_MODE_MANUAL    0x1
> >  #define HFI_LTR_MODE_PERIODIC  0x2
> > diff --git a/drivers/media/platform/qcom/venus/venc.c
> b/drivers/media/platform/qcom/venus/venc.c
> > index cdb12546c4fa..b01da4c1d47a 100644
> > --- a/drivers/media/platform/qcom/venus/venc.c
> > +++ b/drivers/media/platform/qcom/venus/venc.c
> > @@ -617,6 +617,7 @@ static int venc_set_properties(struct venus_inst *inst)
> >         struct hfi_idr_period idrp;
> >         struct hfi_quantization quant;
> >         struct hfi_quantization_range quant_range;
> > +       struct hfi_quantization_range_v2 quant_range_v2;
> >         struct hfi_enable en;
> >         struct hfi_ltr_mode ltr_mode;
> >         struct hfi_intra_refresh intra_refresh = {};
> > @@ -825,16 +826,40 @@ static int venc_set_properties(struct venus_inst
> *inst)
> >         if (ret)
> >                 return ret;
> >
> > -       ptype = HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE;
> > -       if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_HEVC) {
> > -               quant_range.min_qp = ctr->hevc_min_qp;
> > -               quant_range.max_qp = ctr->hevc_max_qp;
> > +       if (inst->core->res->hfi_version == HFI_VERSION_4XX ||
> > +           inst->core->res->hfi_version == HFI_VERSION_6XX) {
> > +               ptype = HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE_V2;
> > +
> > +               if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_HEVC) {
> > +                       quant_range_v2.min_qp.qp_packed = ctr->hevc_min_qp;
> > +                       quant_range_v2.max_qp.qp_packed = ctr->hevc_max_qp;
> > +               } else if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_VP8) {
> > +                       quant_range_v2.min_qp.qp_packed = ctr->vp8_min_qp;
> > +                       quant_range_v2.max_qp.qp_packed = ctr->vp8_max_qp;
> > +               } else {
> > +                       quant_range_v2.min_qp.qp_packed = ctr->h264_min_qp;
> > +                       quant_range_v2.max_qp.qp_packed = ctr->h264_max_qp;
> > +               }
> > +
> > +               ret = hfi_session_set_property(inst, ptype, &quant_range_v2);
> >         } else {
> > -               quant_range.min_qp = ctr->h264_min_qp;
> > -               quant_range.max_qp = ctr->h264_max_qp;
> > +               ptype = HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE;
> > +
> > +               if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_HEVC) {
> > +                       quant_range.min_qp = ctr->hevc_min_qp;
> > +                       quant_range.max_qp = ctr->hevc_max_qp;
> > +               } else if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_VP8) {
> > +                       quant_range.min_qp = ctr->vp8_min_qp;
> > +                       quant_range.max_qp = ctr->vp8_max_qp;
> > +               } else {
> > +                       quant_range.min_qp = ctr->h264_min_qp;
> > +                       quant_range.max_qp = ctr->h264_max_qp;
> > +               }
> > +
> > +               quant_range.layer_id = 0;
> > +               ret = hfi_session_set_property(inst, ptype, &quant_range);
> >         }
> > -       quant_range.layer_id = 0;
> > -       ret = hfi_session_set_property(inst, ptype, &quant_range);
> > +
> >         if (ret)
> >                 return ret;
> >
> > --
> > 2.17.1
> >
> 
> 
> --
> With best wishes
> Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux