linux-kernel - Re: [PATCH 02/18] media: venus: Introduce VPU version distinction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0e85caec-f6ec-fbd4-1815-8df9769bffcf@quicinc.com>
Date:   Thu, 30 Mar 2023 16:32:51 +0530
From:   Vikash Garodia <quic_vgarodia@...cinc.com>
To:     Konrad Dybcio <konrad.dybcio@...aro.org>,
        Dikshita Agarwal <quic_dikshita@...cinc.com>,
        Stanimir Varbanov <stanimir.k.varbanov@...il.com>,
        Andy Gross <agross@...nel.org>,
        "Bjorn Andersson" <andersson@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Dikshita Agarwal <dikshita@....qualcomm.com>,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Dikshita Agarwal <dikshita@...eaurora.org>,
        Mansur Alisha Shaik <mansur@...eaurora.org>,
        Jonathan Marek <jonathan@...ek.ca>
CC:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Stanimir Varbanov <stanimir.varbanov@...aro.org>,
        <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        "Vikash Garodia" <vgarodia@...eaurora.org>
Subject: Re: [PATCH 02/18] media: venus: Introduce VPU version distinction

On 3/2/2023 5:07 PM, Konrad Dybcio wrote:
>
> On 2.03.2023 08:12, Dikshita Agarwal wrote:
>> On 2/28/2023 8:54 PM, Konrad Dybcio wrote:
>>> The Video Processing Unit hardware version is the differentiator,
>>> based on which we should decide which code paths to take in hw
>>> init. Up until now, we've relied on HFI versions, but that was
>>> just a happy accident between recent SoCs. Add a field in the
>>> res struct and add correlated definitions that will be used to
>>> account for the aforementioned differences.
>>>
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
>>> ---
>>>    drivers/media/platform/qcom/venus/core.h | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>>> index 32551c2602a9..4b785205c5b1 100644
>>> --- a/drivers/media/platform/qcom/venus/core.h
>>> +++ b/drivers/media/platform/qcom/venus/core.h
>>> @@ -48,6 +48,14 @@ struct bw_tbl {
>>>        u32 peak_10bit;
>>>    };
>>>    +enum vpu_version {
>>> +    VPU_VERSION_AR50, /* VPU4 */
>>> +    VPU_VERSION_AR50_LITE, /* VPU4.4 */
>>> +    VPU_VERSION_IRIS1, /* VPU5 */

There was Venus3X, followed by a different generation of video hardware. 
Driver just extended the versions for next generation incrementally.

Existing versions in driver are not the VPU versions, so we can drop 
them from comments.

>>> +    VPU_VERSION_IRIS2,
>>> +    VPU_VERSION_IRIS2_1,
>>> +};
>>> +
>>>    struct venus_resources {
>>>        u64 dma_mask;
>>>        const struct freq_tbl *freq_tbl;
>>> @@ -71,6 +79,7 @@ struct venus_resources {
>>>        const char * const resets[VIDC_RESETS_NUM_MAX];
>>>        unsigned int resets_num;
>>>        enum hfi_version hfi_version;
>>> +    enum vpu_version vpu_version;
>>>        u8 num_vpp_pipes;
>>>        u32 max_load;
>>>        unsigned int vmem_id;
>>> @@ -473,6 +482,12 @@ struct venus_inst {
>>>    #define IS_V4(core)    ((core)->res->hfi_version == HFI_VERSION_4XX)
>>>    #define IS_V6(core)    ((core)->res->hfi_version == HFI_VERSION_6XX)
>>>    +#define IS_AR50(core)        ((core)->res->vpu_version == VPU_VERSION_AR50)
>>> +#define IS_AR50_LITE(core)    ((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
>>> +#define IS_IRIS1(core)        ((core)->res->vpu_version == VPU_VERSION_IRIS1)
>>> +#define IS_IRIS2(core)        ((core)->res->vpu_version == VPU_VERSION_IRIS2)
>>> +#define IS_IRIS2_1(core)    ((core)->res->vpu_version == VPU_VERSION_IRIS2_1)
>>> +
>>>    #define ctrl_to_inst(ctrl)    \
>>>        container_of((ctrl)->handler, struct venus_inst, ctrl_handler)
>>>    
>> Adding VPU version check seems a good idea to me. Can we remove HFI Version checks now?
> If all implementations using VPU x.y *always* use the
> same HFI generation for given x, y, we could.

HFIs generally does not change, so we can be sure that they would always 
use the same HFI.

We might add a new interface (HFI) for a feature requirement, but always 
support the existing ones.

>
> That said, I think keeping it as-is would be convenient
> from the maintainability standpoint if nothing else.. For
> example functions that only appear in ancient msm-3.10
> releases can be easily guarded with IS_V1 or what have you
> without having to dig up all n VPU revisions.
>
> Konrad