Message-ID: <s3rr3p5axi3iu4zvgwgjyhjtxmv7sgp6bqkmsgv2l76p7zxu2k@rxzbblyr57an>
Date: Thu, 7 Aug 2025 14:52:08 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Jorge Ramirez <jorge.ramirez@....qualcomm.com>
Cc: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, bryan.odonoghue@...aro.org,
        quic_dikshita@...cinc.com, quic_vgarodia@...cinc.com,
        konradybcio@...nel.org, krzk+dt@...nel.org, mchehab@...nel.org,
        conor+dt@...nel.org, andersson@...nel.org, linux-media@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 5/7] media: venus: core: Add qcm2290 DT compatible and
 resource data

On Wed, Aug 06, 2025 at 03:07:22PM +0200, Jorge Ramirez wrote:
> On 06/08/25 11:01:09, Konrad Dybcio wrote:
> > On 8/6/25 10:04 AM, Jorge Ramirez wrote:
> > > On 06/08/25 04:37:05, Dmitry Baryshkov wrote:
> > >> On Tue, Aug 05, 2025 at 01:27:34PM +0200, Jorge Ramirez wrote:
> > >>> On 05/08/25 12:44:23, Jorge Ramirez wrote:
> > >>>> On 05/08/25 13:04:50, Dmitry Baryshkov wrote:
> > >>>>> On Tue, Aug 05, 2025 at 08:44:28AM +0200, Jorge Ramirez-Ortiz wrote:
> > >>>>>> Add a qcm2290 compatible binding to the Cenus core.
> > >>>>>>
> > >>>>>> The maximum concurrency is video decode at 1920x1080 (FullHD) with video
> > >>>>>> encode at 1280x720 (HD).
> > >>>>>>
> > >>>>>> The driver is not available to firmware versions below 6.0.55 due to an
> > >>>>>> internal requirement for secure buffers.
> > >>>>>>
> > >>>>>> The bandwidth tables incorporate a conservative safety margin to ensure
> > >>>>>> stability under peak DDR and interconnect load conditions.
> > >>>>>>
> > >>>>>> Co-developed-by: Loic Poulain <loic.poulain@....qualcomm.com>
> > >>>>>> Signed-off-by: Loic Poulain <loic.poulain@....qualcomm.com>
> > >>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@....qualcomm.com>
> > >>>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> > >>>>>> Reviewed-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
> > >>>>>> ---
> > >>>>>>  drivers/media/platform/qcom/venus/core.c | 50 ++++++++++++++++++++++++
> > >>>>>>  1 file changed, 50 insertions(+)
> > >>>>>>
> > >>>>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> > >>>>>> index adc38fbc9d79..753a16f53622 100644
> > >>>>>> --- a/drivers/media/platform/qcom/venus/core.c
> > >>>>>> +++ b/drivers/media/platform/qcom/venus/core.c
> > >>>>>> @@ -1070,6 +1070,55 @@ static const struct venus_resources sc7280_res = {
> > >>>>>>  	.enc_nodename = "video-encoder",
> > >>>>>>  };
> > >>>>>>  
> > >>>>>> +static const struct bw_tbl qcm2290_bw_table_dec[] = {
> > >>>>>> +	{ 352800, 597000, 0, 746000, 0 }, /* 1080p@30 + 720p@30 */
> > >>>>>> +	{ 244800, 413000, 0, 516000, 0 }, /* 1080p@30 */
> > >>>>>> +	{ 216000, 364000, 0, 454000, 0 }, /* 720p@60  */
> > >>>>>> +	{ 108000, 182000, 0, 227000, 0 }, /* 720p@30  */
> > >>>>>> +};
> > >>>>>> +
> > >>>>>> +static const struct bw_tbl qcm2290_bw_table_enc[] = {
> > >>>>>> +	{ 352800, 396000, 0, 0, 0 }, /* 1080p@30 + 720p@30 */
> > >>>>>> +	{ 244800, 275000, 0, 0, 0 }, /* 1080p@30 */
> > >>>>>> +	{ 216000, 242000, 0, 0, 0 }, /* 720p@60  */
> > >>>>>> +	{ 108000, 121000, 0, 0, 0 }, /* 720p@30  */
> > >>>>>> +};
> > >>>>>> +
> > >>>>>> +static const struct firmware_version min_fw = {
> > >>>>>> +	.major = 6, .minor = 0, .rev = 55,
> > >>>>>> +};
> > >>>>>
> > >>>>> This will make venus driver error out with the firmware which is
> > >>>>> available in Debian trixie (and possibly other distributions). If I
> > >>>>> remember correctly, the driver can work with that firmware with the
> > >>>>> limited functionality. Can we please support that instead of erroring
> > >>>>> out completely?
> > >>>>
> > >>>> yes, in V7 I did implement this functionality plus a fix for EOS
> > >>>> handling (broken in pre 6.0.55 firmwares).
> > >>>
> > >>> just re-reading your note, in case this was not clear, the _current_
> > >>> driver upstream will never work with the current firmware if that is
> > >>> what you were thinking (it would need v7 of this series to enable video
> > >>> decoding).
> > >>
> > >> I'd really prefer if we could support firmware that is present in Debian
> > >> trixie and that has been upstreamed more than a year ago.
> > > 
> > > 
> > > I share your view — which is why I put the effort into v7 — but I also
> > > understand that maintaining the extra code and EOS workaround for
> > > decoding needs to be justifiable. So I chose to align with the
> > > maintainers' perspective on this and removed it on v8 (partially also
> > > because I wanted to unblock the current EOS discussion).
> > 
> > +$0.05
> > 
> > I thought we were going to eventually relax/drop the fw requirement
> > when the driver learns some new cool tricks, but are we now straying
> > away from that? (particularly thinking about the EOS part)
> > 
> 
> um, no not really: the decision was to simply drop support for pre
> 6.0.55 firmwares for the AR50_LITE.
> 
> Pre 6.0.55:
> 
> -  has a requirement for secure buffers to support encoding
> -  requires a driver workaround for EOS (providing a dummy length)
> -  during video encoding.

If it requires secure buffers to support encoding (which we do not
implement), then EOS workaround is also not required (at this point).

When we get secure buffers support, we can either lift the requirement
on encode side (and add  EOS workaround) or keep the requirement for
newer firmware.

> 
> To support < 6.0.55, v7 of the driver patchset:
> 
> - uses the version to disable the encode node
> - enables the video decode node
> - implements the EOS workaround.
> 
> It was agreed that this complexity was not necessary and that we should
> just drop <6.0.55 firmware support (which would in any case only include
> video decode).

Limiting < 6.0.55 to decode only sounds fine.

> 
> And so on v8, I removed the above.
> 
> Now I have v9 ready to post it, but Dmitry is asking why cant we have
> the v7 functionality so I am waiting for direction.

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists