On Wed, 20 Dec 2023 at 10:14, Vikash Garodia <quic_vgarodia@xxxxxxxxxxx> wrote: > > Hi, > > On 12/20/2023 1:07 PM, Dmitry Baryshkov wrote: > > On Wed, 20 Dec 2023 at 08:32, Vikash Garodia <quic_vgarodia@xxxxxxxxxxx> wrote: > >> > >> Hi Dmitry, > >> > >> On 12/19/2023 12:08 AM, Dmitry Baryshkov wrote: > >>> On 18/12/2023 13:31, 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 doesn't answer one important point, you have been asked for v1. What is the > >>> actual change point between Venus and Iris? What has been changed so much that > >>> it demands a separate driver. This is the main question for the cover letter, > >>> which has not been answered so far. > >>> > >>> From what I see from you bindings, the hardware is pretty close to what we see > >>> in the latest venus generations. I asssme that there was a change in the vcodec > >>> inteface to the firmware and other similar changes. Could you please point out, > >>> which parts of Venus driver do no longer work or are not applicable for sm8550 > >> > >> The motivation behind having a separate IRIS driver was discussed earlier in [1] > >> In the same discussion, it was ellaborated on how the impact would be with > >> change in the new firmware interface and other video layers in the driver. I can > >> add this in cover letter in the next revision. > > > > Ok. So the changes cover the HFI interface. Is that correct? > Change wise, yes. > > >> We see some duplication of code and to handle the same, the series brings in a > >> common code reusability between iris and venus. Aligning the common peices of > >> venus and iris will be a work in progress, once we land the base driver for iris. > > > > This is not how it usually works. Especially not with the patches you > > have posted. > > > > I have the following suggestion how this story can continue: > > You can _start_ by reworking venus driver, separating the HFI / > > firmware / etc interface to an internal interface in the driver. Then > > implement Iris as a plug in for that interface. I might be mistaken > > here, but I think this is the way how this can be beneficial for both > > the video en/decoding on both old and new platforms. > > HFI/firmware interface is already confined to HFI layer in the existing venus > driver. We explained in the previous discussion [1], on how the HFI change > impacts the other layers by taking example of a DRC usecase. Please have a look > considering the usecase and the impact it brings to other layers in the driver. I have looked at it. And I still see huge change in the interface side, but it doesn't tell me about the hardware changes. Have you evaluated the other opportunity? To have a common platform interface and firmware-specific backend? You have already done a part of it, but from a different perspective. You have tried to move common code out of the driver. Instead we are asking you to do a different thing. Move non-common code within the driver. Then add your code on top of that. > > [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@xxxxxxxxxxx/ > > Short rationale: > > The venus driver has a history of supported platforms. There is > > already some kind of buffer management in place. Both developers and > > testers have spent their effort on finding issues there. Sending new > > driver means that we have to spend the same amount of efforts on this. > > Moreover, even from the porter point of view. You are creating new > > bindings for the new hardware. Which do not follow the > > venus-common.yaml. And they do not follow the defined bindings for the > > recent venus platforms. Which means that as a developer I have to care > > about two different ways to describe nearly the same hardware.>> Again qualcomm video team does not have a plan to support sm8550/x1e80100 on > >> venus as the changes are too interleaved to absorb in venus driver. And there is > >> significant interest in community to start validating video driver on sm8550 or > >> x1e80100. > >> > >> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@xxxxxxxxxxx/ > >> > >> Regards, > >> Vikash > > > > > > -- With best wishes Dmitry
