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

[Konrad Dybcio]

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@xxxxxxxxxx>
>> ---
>>   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 */
>> +    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.

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
>