Received-SPF: pass (google.com: domain of linux-kernel+bounces-100131-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <d49012ae-4e67-47d0-8e1b-7b0c4b118f7e@xs4all.nl>
Date: Tue, 12 Mar 2024 11:37:31 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 00/34] Qualcomm video encoder and decoder driver
Content-Language: en-US
To: Vikash Garodia <quic_vgarodia@quicinc.com>,
 Hans Verkuil <hans.verkuil@cisco.com>, mchehab@kernel.org,
 stanimir.k.varbanov@gmail.com, andersson@kernel.org,
 konrad.dybcio@linaro.org, bryan.odonoghue@linaro.org, agross@kernel.org,
 linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org, quic_abhinavk@quicinc.com,
 Dikshita Agarwal <quic_dikshita@quicinc.com>
References: <1702899149-21321-1-git-send-email-quic_dikshita@quicinc.com>
 <8bf182b1-e05f-0c90-a358-e5c8bf6bd430@quicinc.com>
From: Hans Verkuil <hverkuil@xs4all.nl>
In-Reply-To: <8bf182b1-e05f-0c90-a358-e5c8bf6bd430@quicinc.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi Vikash,

On 2/29/24 16:09, Vikash Garodia wrote:
> Hello All,
> 
> On 12/18/2023 5:01 PM, Dikshita Agarwal wrote:
>> This patch series introduces support for Qualcomm new video acceleration
>> hardware architecture, used for video stream decoding/encoding. This driver
>> is based on new communication protocol between video hardware and application
>> processor.
>> This driver comes with below capabilities:
>> - V4L2 complaint video driver with M2M and STREAMING capability.
>> - Supports H264, H265, VP9 decoders.
>> - Supports H264, H265 encoders.
>> This driver comes with below features:
>> - Centralized resource and memory management.
>> - Centralized management of core and instance states.
>> - Defines platform specific capabilities and features. As a results, it provides
>>   a single point of control to enable/disable a given feature depending on 
>>   specific platform capabilities.
>> - Handles hardware interdependent configurations. For a given configuration from
>>   client, the driver checks for hardware dependent configuration/s and updates
>>   the same.
>> - Handles multiple complex video scenarios involving state transitions - DRC,
>>   Drain, Seek, back to back DRC, DRC during Drain sequence, DRC during Seek
>>   sequence.
>> - Introduces a flexible way for driver to subscribe for any property with
>>   hardware. Hardware would inform driver with those subscribed property with any
>>   change in value.
>> - Introduces performance (clock and bus) model based on new hardware
>>   architecture.
>> - Introduces multi thread safe design to handle communication between client and
>>   hardware.
>> - Adapts encoder quality improvements available in new hardware architecture.
>> - Implements asynchronous communication with hardware to achieve better
>>   experience in low latency usecases.
>> - Supports multi stage hardware architecture for encode/decode.
>> - Output and capture planes are controlled independently. Thereby providing a
>>   way to reconfigure individual plane.
>> - Hardware packetization layer supports synchronization between configuration
>>   packet and data packet.
>> - Introduces a flexibility to receive a hardware response for a given command
>>   packet.
>> - Native hardware support of LAST flag which is mandatory to align with port
>>   reconfiguration and DRAIN sequence as per V4L guidelines.
>> - Native hardware support for drain sequence.
> 
> 1. We considered enhancing the venus driver to support iris functionality but
> concluded that the result would essentially be two parallel drivers implemented
> in the same folder.
> 2. Although the underlying hardware for venus and iris are quite similar, but
> there are other factors which brings the need of new driver:
>    a. the host firmware interface (HFI) changed between the two. Venus supports
> HFI protocol 1.0 while iris supports HFI protocol 2.0.
>    b. unlike HFI protocol 1.0, HFI protocol 2.0 is better aligned to V4L2 APIs.
>    c. iris driver modularize platforms, hardware variants, and states to extend
> it for any upcoming SOC. Thereby more extendable to newer SOCs in future.
>    d. Iris also supports many advanced features.
> 3. Based on the comments received on posted iris driver [1], we evaluated it
> further and to better align with the upstream standard and practices, we
> acknowledge that even though iris driver is the way forward, it would be ideal
> to bring in the Venus hardwares enabled in this driver.
> 4. Hence, we decided to rework iris driver to add support of HFI protocol 1.0 as
> well.
> 5. Initially we would start with adding support for one HFI protocol 1.0 based
> platform which would be SM8250.
> 6. SM8250 enablement on iris driver would be incremental, starting with basic
> decode for H264 codec.
> 7. In long-term, all venus supported platforms would be migrated to iris.
> 8. Iris driver and venus driver will co-exist till remaining supported targets
> also gets migrated to iris driver.
> 9. We would continue to support and maintain venus driver during this phased out
> approach.
> 
> Please share your thoughts on the above proposal.

I think this is a reasonable approach: the venus driver is quite old and it was
created when we were at more-or-less the same time also developing better codec
frameworks. So it is not quite up-to-date with all the latest requirements.

Starting with a clean slate for the iris driver and then add support for venus
platforms to the iris driver makes sense.

Just one serious concern: who will do the venus platform migration? Are there resources
available to do that work? Or is this just wishful thinking?

Regards,

	Hans

> 
> [1]
> https://patchwork.kernel.org/project/linux-media/cover/1702899149-21321-1-git-send-email-quic_dikshita@quicinc.com/#25643473
> 
> Regards,
> Vikash
>