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@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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,
+};
+
+static const struct venus_resources qcm2290_res = {
+ .bw_tbl_dec = qcm2290_bw_table_dec,
+ .bw_tbl_dec_size = ARRAY_SIZE(qcm2290_bw_table_dec),
+ .bw_tbl_enc = qcm2290_bw_table_enc,
+ .bw_tbl_enc_size = ARRAY_SIZE(qcm2290_bw_table_enc),
+ .clks = { "core", "iface", "bus", "throttle" },
+ .clks_num = 4,
+ .vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
+ .vcodec_clks_num = 2,
+ .vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
+ .vcodec_pmdomains_num = 2,
+ .opp_pmdomain = (const char *[]) { "cx" },
+ .vcodec_num = 1,
+ .hfi_version = HFI_VERSION_4XX,
+ .vpu_version = VPU_VERSION_AR50_LITE,
+ .max_load = 352800,
+ .num_vpp_pipes = 1,
+ .vmem_id = VIDC_RESOURCE_NONE,
+ .vmem_size = 0,
+ .vmem_addr = 0,
+ .cp_start = 0,
+ .cp_size = 0x70800000,
+ .cp_nonpixel_start = 0x1000000,
+ .cp_nonpixel_size = 0x24800000,
+ .dma_mask = 0xe0000000 - 1,
+ .fwname = "qcom/venus-6.0/venus.mbn",
+ .dec_nodename = "video-decoder",
+ .enc_nodename = "video-encoder",
+ .min_fw = &min_fw,
+};
+
static const struct of_device_id venus_dt_match[] = {
{ .compatible = "qcom,msm8916-venus", .data = &msm8916_res, },
{ .compatible = "qcom,msm8996-venus", .data = &msm8996_res, },
@@ -1080,6 +1129,7 @@ static const struct of_device_id venus_dt_match[] = {
{ .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },
{ .compatible = "qcom,sc7280-venus", .data = &sc7280_res, },
{ .compatible = "qcom,sm8250-venus", .data = &sm8250_res, },
+ { .compatible = "qcom,qcm2290-venus", .data = &qcm2290_res, },
{ }
};
MODULE_DEVICE_TABLE(of, venus_dt_match);
--
2.34.1On 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@oss.qualcomm.com>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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?
> @@ -1080,6 +1129,7 @@ static const struct of_device_id venus_dt_match[] = {
> { .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },
> { .compatible = "qcom,sc7280-venus", .data = &sc7280_res, },
> { .compatible = "qcom,sm8250-venus", .data = &sm8250_res, },
> + { .compatible = "qcom,qcm2290-venus", .data = &qcm2290_res, },
Please keep the table sorted.
> { }
> };
> MODULE_DEVICE_TABLE(of, venus_dt_match);
> --
> 2.34.1
>
--
With best wishes
Dmitry
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@oss.qualcomm.com>
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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).
This added some complexity to the driver. And so in internal discussions
it was agreed that it was not worth to carry it and that it should be dropped.
I'll let Vikash and Bryan comment on the decision.
>
> > @@ -1080,6 +1129,7 @@ static const struct of_device_id venus_dt_match[] = {
> > { .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },
> > { .compatible = "qcom,sc7280-venus", .data = &sc7280_res, },
> > { .compatible = "qcom,sm8250-venus", .data = &sm8250_res, },
> > + { .compatible = "qcom,qcm2290-venus", .data = &qcm2290_res, },
>
> Please keep the table sorted.
argh...sure
>
> > { }
> > };
> > MODULE_DEVICE_TABLE(of, venus_dt_match);
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
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@oss.qualcomm.com>
> > > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > > Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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).
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@oss.qualcomm.com>
> > > > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > > > Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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.
--
With best wishes
Dmitry
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@oss.qualcomm.com>
> > > > > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > > > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > > > > Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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).
At this point, I have v9 ready based on v8, addressing the latest round
of comments. However, if we need to revert to the features in v7, it
will require reworking a significant amount of code.
To avoid going in circles, I’d appreciate some clarity on the direction
from Vikash, Bryan, and Dkishita:
ok to post v9 based on v8 or should I bring back v7 support (decoding)
for firmwares before 6.0.55?
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@oss.qualcomm.com>
>>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
>>>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>>>> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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)
Konrad
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@oss.qualcomm.com>
> >>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> >>>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> >>>>>> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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.
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).
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.
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@oss.qualcomm.com>
> > >>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > >>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > >>>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > >>>>>> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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
On 07/08/25 14:52:08, Dmitry Baryshkov wrote:
> 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@oss.qualcomm.com>
> > > >>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > > >>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > >>>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > > >>>>>> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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).
My bad earlier — the EOS workaround applies to video decoding, not
encoding.
Video decoding does NOT require secure buffers, which is why it can be
enabled independently of the firmware update.
to clarify, the EOS workaround is necessary for decoding because:
- The current driver doesn't fully follow to the HFI spec WRT EOS
handling, which leads to issues like the one we're seeing.
- The firmware we're using doesn't accept the upstream driver's existing
workarounds — such as hardcoded buffer addresses like 0x0 or
0xdeadb000, which vary across firmware versions.
The way I see it sticking to the spec — that is, always passing a valid
buffer which was my preferred choice and my first implementation — would
make the driver more robust and less prone to this kind of problems.
Failing that, I dont see the issue with adding workarounds/quirks to the
EOS handling (in this case).
if (IS_AR50_LITE(core) && is_fw_rev_or_older(core, 6, 0, 53))
data->alloc_len = 1;
Even more considering we already have:
if (IS_V6(core) && is_fw_rev_or_older(core, 1, 0, 87))
data->device_addr = 0x0;
else
data->device_addr = 0xdeadb000;
In terms of an abstration, there is no meaning to these values since
these are not valid buffers: we are just filling whatever it makes the
firmware work.
On 8/6/2025 6:37 PM, 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@oss.qualcomm.com>
>>>>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
>>>>>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>>>>>> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.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.
>
> 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).
>
> 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.
the issue is in firmware for both encoder and decoder. Didn't like the idea of
driver carrying the hack for a firmware issue. Just because, for encoder, we are
unable to hack it in driver, we are ok to have it enabled in a newer version of
the firmware, we can follow the same for decoders as well.
Regards,
Vikash
On 07/08/25 16:36:41, Vikash Garodia wrote: > > > 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). > > > > 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. > > the issue is in firmware for both encoder and decoder. Didn't like the idea of > driver carrying the hack for a firmware issue. Just because, for encoder, we are > unable to hack it in driver, we are ok to have it enabled in a newer version of > the firmware, we can follow the same for decoders as well. if that is the only reason please do explain what do you mean by hack.
On 8/7/2025 7:22 PM, Jorge Ramirez wrote: > On 07/08/25 16:36:41, Vikash Garodia wrote: >> >>> 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). >>> >>> 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. >> >> the issue is in firmware for both encoder and decoder. Didn't like the idea of >> driver carrying the hack for a firmware issue. Just because, for encoder, we are >> unable to hack it in driver, we are ok to have it enabled in a newer version of >> the firmware, we can follow the same for decoders as well. > > if that is the only reason please do explain what do you mean by hack. I meant that the EOS handling was not needed in driver after fixing it in firmware, isn't it ? Was trying to avoid carrying this in driver. I tend to agree with the comment made by Dmitry in another thread to have decode enabled with existing firmware, no option but to support the *already* published bins. Having said that, these limitation of having a separate EOS dummy buffer is well sorted out in gen2 HFI which have an explicit DRAIN cmd for it. Hope this motivates you to migrate to iris soon for AR50LITE variants :) Regards, Vikash
On Thu, Aug 07, 2025 at 10:05:10PM +0530, Vikash Garodia wrote: > > > On 8/7/2025 7:22 PM, Jorge Ramirez wrote: > > On 07/08/25 16:36:41, Vikash Garodia wrote: > >> > >>> 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). > >>> > >>> 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. > >> > >> the issue is in firmware for both encoder and decoder. Didn't like the idea of > >> driver carrying the hack for a firmware issue. Just because, for encoder, we are > >> unable to hack it in driver, we are ok to have it enabled in a newer version of > >> the firmware, we can follow the same for decoders as well. > > > > if that is the only reason please do explain what do you mean by hack. > > I meant that the EOS handling was not needed in driver after fixing it in > firmware, isn't it ? Was trying to avoid carrying this in driver. > > I tend to agree with the comment made by Dmitry in another thread to have decode > enabled with existing firmware, no option but to support the *already* published > bins. > > Having said that, these limitation of having a separate EOS dummy buffer is well > sorted out in gen2 HFI which have an explicit DRAIN cmd for it. Hope this > motivates you to migrate to iris soon for AR50LITE variants :) Migrating to Iris won't bring gen2 HFI. Think about users which have OEM-fused hardware. For them it's not possible to switch firmware from gen1 to gen2. Thus, if the SoC has been released using gen1 HFI, we should think twice before upgrading it to gen2. -- With best wishes Dmitry
On 09/08/25 11:18:21, Dmitry Baryshkov wrote: > On Thu, Aug 07, 2025 at 10:05:10PM +0530, Vikash Garodia wrote: > > > > > > On 8/7/2025 7:22 PM, Jorge Ramirez wrote: > > > On 07/08/25 16:36:41, Vikash Garodia wrote: > > >> > > >>> 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). > > >>> > > >>> 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. > > >> > > >> the issue is in firmware for both encoder and decoder. Didn't like the idea of > > >> driver carrying the hack for a firmware issue. Just because, for encoder, we are > > >> unable to hack it in driver, we are ok to have it enabled in a newer version of > > >> the firmware, we can follow the same for decoders as well. > > > > > > if that is the only reason please do explain what do you mean by hack. > > > > I meant that the EOS handling was not needed in driver after fixing it in > > firmware, isn't it ? Was trying to avoid carrying this in driver. > > > > I tend to agree with the comment made by Dmitry in another thread to have decode > > enabled with existing firmware, no option but to support the *already* published > > bins. > > > > Having said that, these limitation of having a separate EOS dummy buffer is well > > sorted out in gen2 HFI which have an explicit DRAIN cmd for it. Hope this > > motivates you to migrate to iris soon for AR50LITE variants :) > > Migrating to Iris won't bring gen2 HFI. Think about users which have > OEM-fused hardware. For them it's not possible to switch firmware from > gen1 to gen2. Thus, if the SoC has been released using gen1 HFI, we > should think twice before upgrading it to gen2. > As I understand it now after the thread, any driver developer working on new features should not be constrained by users with OEM-fused hardware. Since only the OEM can provide signed firmware updates, it is their responsibility—not ours—to figure out how to deliver those updates if they want their users to benefit from new features (or new fixes). The EU Cyber Resilience Act supports this view by placing the update obligation on manufacturers (at least that is what I understand it, let me know if you understand it differently) Breaking backward compatibility is something we must avoid of course. However, guaranteeing compatibility between old firmwares (whether signed or not) and _new_ features is a separate matter...
On Sat, Aug 09, 2025 at 11:09:24AM +0200, Jorge Ramirez wrote: > On 09/08/25 11:18:21, Dmitry Baryshkov wrote: > > On Thu, Aug 07, 2025 at 10:05:10PM +0530, Vikash Garodia wrote: > > > > > > > > > On 8/7/2025 7:22 PM, Jorge Ramirez wrote: > > > > On 07/08/25 16:36:41, Vikash Garodia wrote: > > > >> > > > >>> 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). > > > >>> > > > >>> 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. > > > >> > > > >> the issue is in firmware for both encoder and decoder. Didn't like the idea of > > > >> driver carrying the hack for a firmware issue. Just because, for encoder, we are > > > >> unable to hack it in driver, we are ok to have it enabled in a newer version of > > > >> the firmware, we can follow the same for decoders as well. > > > > > > > > if that is the only reason please do explain what do you mean by hack. > > > > > > I meant that the EOS handling was not needed in driver after fixing it in > > > firmware, isn't it ? Was trying to avoid carrying this in driver. > > > > > > I tend to agree with the comment made by Dmitry in another thread to have decode > > > enabled with existing firmware, no option but to support the *already* published > > > bins. > > > > > > Having said that, these limitation of having a separate EOS dummy buffer is well > > > sorted out in gen2 HFI which have an explicit DRAIN cmd for it. Hope this > > > motivates you to migrate to iris soon for AR50LITE variants :) > > > > Migrating to Iris won't bring gen2 HFI. Think about users which have > > OEM-fused hardware. For them it's not possible to switch firmware from > > gen1 to gen2. Thus, if the SoC has been released using gen1 HFI, we > > should think twice before upgrading it to gen2. > > > > As I understand it now after the thread, any driver developer working on > new features should not be constrained by users with OEM-fused hardware. > > Since only the OEM can provide signed firmware updates, it is their > responsibility—not ours—to figure out how to deliver those updates if > they want their users to benefit from new features (or new fixes). The OEMs might go bankrupt, might stop supporting hardware, might not be bound by EU laws, etc. If the platform was shipped with gen1 HFI and we suddently provide gen2 HFI, the driver must support both firmware interfaces for that platform. > The EU Cyber Resilience Act supports this view by placing the update > obligation on manufacturers (at least that is what I understand it, let > me know if you understand it differently) > > Breaking backward compatibility is something we must avoid of > course. However, guaranteeing compatibility between old firmwares > (whether signed or not) and _new_ features is a separate matter... Anyway, the kernel is provided separately from the firmware. If we supported a particular firmware set, we can not break that. AR50_LITE is a corner case, as we have been shipping the firmware, but there was no corresponding open-source driver for that platform. -- With best wishes Dmitry
On 09/08/25 12:22:39, Dmitry Baryshkov wrote: > On Sat, Aug 09, 2025 at 11:09:24AM +0200, Jorge Ramirez wrote: > > On 09/08/25 11:18:21, Dmitry Baryshkov wrote: > > > On Thu, Aug 07, 2025 at 10:05:10PM +0530, Vikash Garodia wrote: > > > > > > > > > > > > On 8/7/2025 7:22 PM, Jorge Ramirez wrote: > > > > > On 07/08/25 16:36:41, Vikash Garodia wrote: > > > > >> > > > > >>> 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). > > > > >>> > > > > >>> 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. > > > > >> > > > > >> the issue is in firmware for both encoder and decoder. Didn't like the idea of > > > > >> driver carrying the hack for a firmware issue. Just because, for encoder, we are > > > > >> unable to hack it in driver, we are ok to have it enabled in a newer version of > > > > >> the firmware, we can follow the same for decoders as well. > > > > > > > > > > if that is the only reason please do explain what do you mean by hack. > > > > > > > > I meant that the EOS handling was not needed in driver after fixing it in > > > > firmware, isn't it ? Was trying to avoid carrying this in driver. > > > > > > > > I tend to agree with the comment made by Dmitry in another thread to have decode > > > > enabled with existing firmware, no option but to support the *already* published > > > > bins. > > > > > > > > Having said that, these limitation of having a separate EOS dummy buffer is well > > > > sorted out in gen2 HFI which have an explicit DRAIN cmd for it. Hope this > > > > motivates you to migrate to iris soon for AR50LITE variants :) > > > > > > Migrating to Iris won't bring gen2 HFI. Think about users which have > > > OEM-fused hardware. For them it's not possible to switch firmware from > > > gen1 to gen2. Thus, if the SoC has been released using gen1 HFI, we > > > should think twice before upgrading it to gen2. > > > > > > > As I understand it now after the thread, any driver developer working on > > new features should not be constrained by users with OEM-fused hardware. > > > > Since only the OEM can provide signed firmware updates, it is their > > responsibility—not ours—to figure out how to deliver those updates if > > they want their users to benefit from new features (or new fixes). > > The OEMs might go bankrupt, might stop supporting hardware, might not be > bound by EU laws, etc. If the platform was shipped with gen1 HFI and we > suddently provide gen2 HFI, the driver must support both firmware > interfaces for that platform. sure, that is backwards compatibility > > > The EU Cyber Resilience Act supports this view by placing the update > > obligation on manufacturers (at least that is what I understand it, let > > me know if you understand it differently) > > > > Breaking backward compatibility is something we must avoid of > > course. However, guaranteeing compatibility between old firmwares > > (whether signed or not) and _new_ features is a separate matter... > > Anyway, the kernel is provided separately from the firmware. If we > supported a particular firmware set, we can not break that. > > AR50_LITE is a corner case, as we have been shipping the firmware, but > there was no corresponding open-source driver for that platform. right > > -- > With best wishes > Dmitry
On 07/08/25 22:05:10, Vikash Garodia wrote: > > > On 8/7/2025 7:22 PM, Jorge Ramirez wrote: > > On 07/08/25 16:36:41, Vikash Garodia wrote: > >> > >>> 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). > >>> > >>> 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. > >> > >> the issue is in firmware for both encoder and decoder. Didn't like the idea of > >> driver carrying the hack for a firmware issue. Just because, for encoder, we are > >> unable to hack it in driver, we are ok to have it enabled in a newer version of > >> the firmware, we can follow the same for decoders as well. > > > > if that is the only reason please do explain what do you mean by hack. > > I meant that the EOS handling was not needed in driver after fixing it in > firmware, isn't it ? Was trying to avoid carrying this in driver. sure I agree with that, just that I dont call that a hack (more a quirk or workaround) > > I tend to agree with the comment made by Dmitry in another thread to have decode > enabled with existing firmware, no option but to support the *already* published > bins. The way I see it—and as we discussed the other day—I was fine with dropping decoding support for firmware versions < 6.0.55, as long as someone with internal insight could confirm we won't upset users. Maybe I’m overthinking it, but coming from OTA, I’ve seen how nice it is when users upgrade their kernel and suddenly get hardware video decode without having to worry about firmware upgrades. > > Having said that, these limitation of having a separate EOS dummy buffer is well > sorted out in gen2 HFI which have an explicit DRAIN cmd for it. Hope this > motivates you to migrate to iris soon for AR50LITE variants :) 100% , I saw it! > > Regards, > Vikash Dmitry, all ok then?
On 05/08/2025 11:44, Jorge Ramirez wrote: > yes, in V7 I did implement this functionality plus a fix for EOS > handling (broken in pre 6.0.55 firmwares). > > This added some complexity to the driver. And so in internal discussions > it was agreed that it was not worth to carry it and that it should be dropped. > > I'll let Vikash and Bryan comment on the decision. TBH I think there's not alot of value in supporting a broken firmware which only does decode. There's not alot of value to the user in that configuration. Provided you have done the work to get the fixed firmware into linux-firmware just cut at that point and have the driver reject lesser versions. I as a user have no use-case or value in a broken old firmware which supports decode only, I'd much rather have the full transcoder. Its Vikash/Dikshita's call though. --- bod
On 07/08/25 07:35:35, Bryan O'Donoghue wrote: > On 05/08/2025 11:44, Jorge Ramirez wrote: > > yes, in V7 I did implement this functionality plus a fix for EOS > > handling (broken in pre 6.0.55 firmwares). > > > > This added some complexity to the driver. And so in internal discussions > > it was agreed that it was not worth to carry it and that it should be dropped. > > > > I'll let Vikash and Bryan comment on the decision. > > TBH I think there's not alot of value in supporting a broken firmware which > only does decode. > > There's not alot of value to the user in that configuration. I dont know the user base but when I originally did the code (v7) I was thinking about security conscious users (signed firmwares) who might not be able to switch to the new fw release so easily (unnaccessible key management and updates). But I dont have those numbers so it might be none. > > Provided you have done the work to get the fixed firmware into > linux-firmware just cut at that point and have the driver reject lesser > versions. yep, that went smoothly: https://gitlab.com/kernel-firmware/linux-firmware/-/commit/8ecf764788f8dbf33fc1c483ddf91f882ad792b6 > > I as a user have no use-case or value in a broken old firmware which > supports decode only, I'd much rather have the full transcoder. > > Its Vikash/Dikshita's call though. I'll keep on holding v9 until then.
On 07/08/2025 07:48, Jorge Ramirez wrote: >> There's not alot of value to the user in that configuration. > I dont know the user base but when I originally did the code (v7) I was > thinking about security conscious users (signed firmwares) who might not > be able to switch to the new fw release so easily (unnaccessible key > management and updates). Since the driver for the LITE hasn't been upstreamed the # of users must be ... zero --- bod
On Thu, Aug 07, 2025 at 11:11:31AM +0100, Bryan O'Donoghue wrote: > On 07/08/2025 07:48, Jorge Ramirez wrote: > > > There's not alot of value to the user in that configuration. > > I dont know the user base but when I originally did the code (v7) I was > > thinking about security conscious users (signed firmwares) who might not > > be able to switch to the new fw release so easily (unnaccessible key > > management and updates). > > Since the driver for the LITE hasn't been upstreamed the # of users must be > ... zero The question is about the firmware side: if we support decoder for 6.0.55, users can get video decoding using existing distro firmware (and just update the kernel). -- With best wishes Dmitry
On 07/08/25 11:11:31, Bryan O'Donoghue wrote: > On 07/08/2025 07:48, Jorge Ramirez wrote: > > > There's not alot of value to the user in that configuration. > > I dont know the user base but when I originally did the code (v7) I was > > thinking about security conscious users (signed firmwares) who might not > > be able to switch to the new fw release so easily (unnaccessible key > > management and updates). > > Since the driver for the LITE hasn't been upstreamed the # of users must be > ... zero sure but that is just one dimension of the problem isnt it? the other dimension is the number of users with a signed firmware that could _now_ enable the driver and have a working decoder. that could be zero or plenty (I have no clue) > > --- > bod
© 2016 - 2025 Red Hat, Inc.