Received-SPF: pass (google.com: domain of linux-kernel+bounces-6496-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <1702899149-21321-1-git-send-email-quic_dikshita@quicinc.com>
 <a033dfc5-dcf1-4969-ad4d-1836ff9ff0a3@linaro.org> <d0ea23ae-8fba-d229-b0f6-dc522f285233@quicinc.com>
In-Reply-To: <d0ea23ae-8fba-d229-b0f6-dc522f285233@quicinc.com>
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date: Wed, 20 Dec 2023 09:37:38 +0200
Message-ID: <CAA8EJpouBOLJ_1Pz_YauuOX+97ud9RkLYRaui4GM6ZFJUKYJMw@mail.gmail.com>
Subject: Re: [PATCH v2 00/34] Qualcomm video encoder and decoder driver
To: Vikash Garodia <quic_vgarodia@quicinc.com>
Cc: Dikshita Agarwal <quic_dikshita@quicinc.com>, linux-media@vger.kernel.org, 
	linux-kernel@vger.kernel.org, stanimir.k.varbanov@gmail.com, 
	agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org, 
	mchehab@kernel.org, bryan.odonoghue@linaro.org, linux-arm-msm@vger.kernel.org, 
	quic_abhinavk@quicinc.com
Content-Type: text/plain; charset="UTF-8"

On Wed, 20 Dec 2023 at 08:32, Vikash Garodia <quic_vgarodia@quicinc.com> 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?

> 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.

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@quicinc.com/
>
> Regards,
> Vikash


-- 
With best wishes
Dmitry