lkml.org 
[lkml]   [2018]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 04/29] venus: hfi_cmds: add set_properties for 4xx version
    From
    Date
    Hi Tomasz,

    On 05/18/2018 05:16 PM, Tomasz Figa wrote:
    > On Tue, May 15, 2018 at 5:13 PM Stanimir Varbanov <
    > stanimir.varbanov@linaro.org> wrote:
    >
    >> Adds set_properties method to handle newer 4xx properties and
    >> fall-back to 3xx for the rest.
    >
    >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
    >> ---
    >> drivers/media/platform/qcom/venus/hfi_cmds.c | 64
    > +++++++++++++++++++++++++++-
    >> 1 file changed, 63 insertions(+), 1 deletion(-)
    >
    >> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c
    > b/drivers/media/platform/qcom/venus/hfi_cmds.c
    >> index 1cfeb7743041..6bd287154796 100644
    >> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
    >> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
    >
    > [snip]
    >
    >> + case HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE:
    >> + /* not implemented on Venus 4xx */
    >
    > Shouldn't return -EINVAL here, similar to what
    > pkt_session_set_property_1x() does for unknown property?

    Probably the right error code should be ENOTSUPP, but I kind of
    following the rule to silently not return the error to simplify the
    callers of set_property (otherwise I have to have a version conditional
    code in the callers).

    >
    >> + break;
    >> + default:
    >> + ret = pkt_session_set_property_3xx(pkt, cookie, ptype,
    > pdata);
    >> + break;
    >
    > nit: How about simply return pkt_session_set_property_3xx(pkt, cookie,
    > ptype, pdata); and removing the |ret| variable completely, since the return
    > below the switch can just return 0 all the time?

    OK, I will do that way.

    >
    >> + }
    >> +
    >> + return ret;
    >> +}
    >> +
    >> int pkt_session_get_property(struct hfi_session_get_property_pkt *pkt,
    >> void *cookie, u32 ptype)
    >> {
    >> @@ -1181,7 +1240,10 @@ int pkt_session_set_property(struct
    > hfi_session_set_property_pkt *pkt,
    >> if (hfi_ver == HFI_VERSION_1XX)
    >> return pkt_session_set_property_1x(pkt, cookie, ptype,
    > pdata);
    >
    >> - return pkt_session_set_property_3xx(pkt, cookie, ptype, pdata);
    >> + if (hfi_ver == HFI_VERSION_3XX)
    >> + return pkt_session_set_property_3xx(pkt, cookie, ptype,
    > pdata);
    >> +
    >> + return pkt_session_set_property_4xx(pkt, cookie, ptype, pdata);
    >
    > nit: Since we're adding third variant, I'd consider using function pointers
    > here, but no strong opinion.

    Let's keep that for future improvements.

    --
    regards,
    Stan

    \
     
     \ /
      Last update: 2018-05-21 16:53    [W:4.298 / U:0.176 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site