[PATCH 0/8] Reup: SM8350 and SC8280XP venus support

Bryan O'Donoghue posted 8 patches 11 months, 1 week ago
.../bindings/media/qcom,sm8350-venus.yaml          | 119 ++++++++++++++++++++
.../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   5 +
arch/arm64/boot/dts/qcom/sc8280xp.dtsi             |  82 ++++++++++++++
drivers/media/platform/qcom/venus/core.c           | 125 +++++++++++++++++++--
drivers/media/platform/qcom/venus/core.h           |   4 +
drivers/media/platform/qcom/venus/hfi_venus.c      |  15 ++-
drivers/media/platform/qcom/venus/pm_helpers.c     |   3 +
7 files changed, 341 insertions(+), 12 deletions(-)
[PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Bryan O'Donoghue 11 months, 1 week ago
This series is a re-up of Konrad's original venus series for sc8280xp and
sm8350.

Link: https://lore.kernel.org/all/20230731-topic-8280_venus-v1-0-8c8bbe1983a5@linaro.org/

The main obstacle to merging that series at the time was the longstanding
but invalid usage of "video-encoder" and "video-decoder" which is a
driver level configuration option not a description of hardware.

Following on from that discussion a backwards compatible means of
statically selecting transcoder mode was upstreamed

commit: 687bfbba5a1c ("media: venus: Add support for static video encoder/decoder declarations")

Reworking this series from Konrad to incorporate this simple change

- Removing dts dependencies/declarations on the offending compat strings
- Inclusion of necessary static configuration in the 8350/8280xp driver
  config
- A small update to interconnect tags which Konrad pointed out on IRC to me
- Fixed author and SOB on first patch to match

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Konrad Dybcio (8):
      media: dt-bindings: Document SC8280XP/SM8350 Venus
      media: venus: core: Remove trailing commas from of match entries
      media: venus: hfi_venus: Support only updating certain bits with presets
      media: platform: venus: Add optional LLCC path
      media: venus: core: Add SM8350 resource struct
      media: venus: core: Add SC8280XP resource struct
      arm64: dts: qcom: sc8280xp: Add Venus
      arm64: dts: qcom: sc8280xp-x13s: Enable Venus

 .../bindings/media/qcom,sm8350-venus.yaml          | 119 ++++++++++++++++++++
 .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   5 +
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi             |  82 ++++++++++++++
 drivers/media/platform/qcom/venus/core.c           | 125 +++++++++++++++++++--
 drivers/media/platform/qcom/venus/core.h           |   4 +
 drivers/media/platform/qcom/venus/hfi_venus.c      |  15 ++-
 drivers/media/platform/qcom/venus/pm_helpers.c     |   3 +
 7 files changed, 341 insertions(+), 12 deletions(-)
---
base-commit: d98e9213a768a3cc3a99f5e1abe09ad3baff2104
change-id: 20250301-b4-linux-media-comitters-sc8280xp-venus-e2cad579b4f0

Best regards,
-- 
Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Vikash Garodia 11 months, 1 week ago
On 3/4/2025 6:37 PM, Bryan O'Donoghue wrote:
> This series is a re-up of Konrad's original venus series for sc8280xp and
> sm8350.Why this is enabled on venus driver ? Why not iris driver ? This needs an
explanation on was this even tried to bring up on iris driver.

How different is this from sm8250 which is already enabled on iris driver ?

> Link: https://lore.kernel.org/all/20230731-topic-8280_venus-v1-0-8c8bbe1983a5@linaro.org/
> 
> The main obstacle to merging that series at the time was the longstanding
> but invalid usage of "video-encoder" and "video-decoder" which is a
> driver level configuration option not a description of hardware.
> 
> Following on from that discussion a backwards compatible means of
> statically selecting transcoder mode was upstreamed
> 
> commit: 687bfbba5a1c ("media: venus: Add support for static video encoder/decoder declarations")
> 
> Reworking this series from Konrad to incorporate this simple change
> 
> - Removing dts dependencies/declarations on the offending compat strings
> - Inclusion of necessary static configuration in the 8350/8280xp driver
>   config
> - A small update to interconnect tags which Konrad pointed out on IRC to me
> - Fixed author and SOB on first patch to match
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> Konrad Dybcio (8):
>       media: dt-bindings: Document SC8280XP/SM8350 Venus
>       media: venus: core: Remove trailing commas from of match entries
>       media: venus: hfi_venus: Support only updating certain bits with presets
>       media: platform: venus: Add optional LLCC path
>       media: venus: core: Add SM8350 resource struct
>       media: venus: core: Add SC8280XP resource struct
>       arm64: dts: qcom: sc8280xp: Add Venus
>       arm64: dts: qcom: sc8280xp-x13s: Enable Venus
> 
>  .../bindings/media/qcom,sm8350-venus.yaml          | 119 ++++++++++++++++++++
>  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   5 +
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi             |  82 ++++++++++++++
>  drivers/media/platform/qcom/venus/core.c           | 125 +++++++++++++++++++--
>  drivers/media/platform/qcom/venus/core.h           |   4 +
>  drivers/media/platform/qcom/venus/hfi_venus.c      |  15 ++-
>  drivers/media/platform/qcom/venus/pm_helpers.c     |   3 +
>  7 files changed, 341 insertions(+), 12 deletions(-)
> ---
> base-commit: d98e9213a768a3cc3a99f5e1abe09ad3baff2104
> change-id: 20250301-b4-linux-media-comitters-sc8280xp-venus-e2cad579b4f0
> 
> Best regards,

Regards,
Vikash
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Dmitry Baryshkov 10 months, 1 week ago
On Wed, Mar 05, 2025 at 08:49:37AM +0530, Vikash Garodia wrote:
> 
> On 3/4/2025 6:37 PM, Bryan O'Donoghue wrote:
> > This series is a re-up of Konrad's original venus series for sc8280xp and
> > sm8350.Why this is enabled on venus driver ? Why not iris driver ? This needs an
> explanation on was this even tried to bring up on iris driver.
> 
> How different is this from sm8250 which is already enabled on iris driver ?

As far as I remember, SM8250 support in Iris did not reach
feature-parity yet. So in my opinion it is fine to add new platforms to
the Venus driver, that will later migrate to the Iris driver.

Otherwise users of SC8280XP either have to use external patchsets (like
this one) or a non-full-featured driver (and still possibly external
patchsets, I didn't check if these two platforms can use
qcom,sm8250-venus as a fallback compat string).

Bryan, Konrad, in my opinion, let's get these patches merged :-)

> 
> > Link: https://lore.kernel.org/all/20230731-topic-8280_venus-v1-0-8c8bbe1983a5@linaro.org/
> > 
> > The main obstacle to merging that series at the time was the longstanding
> > but invalid usage of "video-encoder" and "video-decoder" which is a
> > driver level configuration option not a description of hardware.
> > 
> > Following on from that discussion a backwards compatible means of
> > statically selecting transcoder mode was upstreamed
> > 
> > commit: 687bfbba5a1c ("media: venus: Add support for static video encoder/decoder declarations")
> > 
> > Reworking this series from Konrad to incorporate this simple change
> > 

-- 
With best wishes
Dmitry
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Vikash Garodia 10 months, 1 week ago
Hi Dmitry,

On 4/3/2025 10:28 PM, Dmitry Baryshkov wrote:
> On Wed, Mar 05, 2025 at 08:49:37AM +0530, Vikash Garodia wrote:
>>
>> On 3/4/2025 6:37 PM, Bryan O'Donoghue wrote:
>>> This series is a re-up of Konrad's original venus series for sc8280xp and
>>> sm8350.Why this is enabled on venus driver ? Why not iris driver ? This needs an
>> explanation on was this even tried to bring up on iris driver.
>>
>> How different is this from sm8250 which is already enabled on iris driver ?
> 
> As far as I remember, SM8250 support in Iris did not reach
> feature-parity yet. So in my opinion it is fine to add new platforms to
> the Venus driver, that will later migrate to the Iris driver.
I would say, from decoder side all codecs are there now on Iris. H264 merged,
while h265 and VP9 dec are posted as RFC, there is one compliance failure which
is under debug to post them as regular patches.
If we are mainly looking for decode usecases, then we should be on Iris.
Preference would be to stay on Iris, otherwise we would have that extra ask to
port it later from venus to iris.
> 
> Otherwise users of SC8280XP either have to use external patchsets (like
> this one) or a non-full-featured driver (and still possibly external
> patchsets, I didn't check if these two platforms can use
> qcom,sm8250-venus as a fallback compat string).
It should, atleast from the hardware spec perspective, AFAIK.
> 
> Bryan, Konrad, in my opinion, let's get these patches merged :-)
> 
>>
>>> Link: https://lore.kernel.org/all/20230731-topic-8280_venus-v1-0-8c8bbe1983a5@linaro.org/
>>>
>>> The main obstacle to merging that series at the time was the longstanding
>>> but invalid usage of "video-encoder" and "video-decoder" which is a
>>> driver level configuration option not a description of hardware.
>>>
>>> Following on from that discussion a backwards compatible means of
>>> statically selecting transcoder mode was upstreamed
>>>
>>> commit: 687bfbba5a1c ("media: venus: Add support for static video encoder/decoder declarations")
>>>
>>> Reworking this series from Konrad to incorporate this simple change
>>>
> 
Regards,
Vikash
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Dmitry Baryshkov 10 months, 1 week ago
On 04/04/2025 08:24, Vikash Garodia wrote:
> Hi Dmitry,
> 
> On 4/3/2025 10:28 PM, Dmitry Baryshkov wrote:
>> On Wed, Mar 05, 2025 at 08:49:37AM +0530, Vikash Garodia wrote:
>>>
>>> On 3/4/2025 6:37 PM, Bryan O'Donoghue wrote:
>>>> This series is a re-up of Konrad's original venus series for sc8280xp and
>>>> sm8350.Why this is enabled on venus driver ? Why not iris driver ? This needs an
>>> explanation on was this even tried to bring up on iris driver.
>>>
>>> How different is this from sm8250 which is already enabled on iris driver ?
>>
>> As far as I remember, SM8250 support in Iris did not reach
>> feature-parity yet. So in my opinion it is fine to add new platforms to
>> the Venus driver, that will later migrate to the Iris driver.
> I would say, from decoder side all codecs are there now on Iris. H264 merged,
> while h265 and VP9 dec are posted as RFC, there is one compliance failure which
> is under debug to post them as regular patches.
> If we are mainly looking for decode usecases, then we should be on Iris.

No, we are not limited to the decode use case.

> Preference would be to stay on Iris, otherwise we would have that extra ask to
> port it later from venus to iris.

Yes, but that would (hopefully) be easy to handle.

>>
>> Otherwise users of SC8280XP either have to use external patchsets (like
>> this one) or a non-full-featured driver (and still possibly external
>> patchsets, I didn't check if these two platforms can use
>> qcom,sm8250-venus as a fallback compat string).
> It should, atleast from the hardware spec perspective, AFAIK.
>>
>> Bryan, Konrad, in my opinion, let's get these patches merged :-)
>>
>>>
>>>> Link: https://lore.kernel.org/all/20230731-topic-8280_venus-v1-0-8c8bbe1983a5@linaro.org/
>>>>
>>>> The main obstacle to merging that series at the time was the longstanding
>>>> but invalid usage of "video-encoder" and "video-decoder" which is a
>>>> driver level configuration option not a description of hardware.
>>>>
>>>> Following on from that discussion a backwards compatible means of
>>>> statically selecting transcoder mode was upstreamed
>>>>
>>>> commit: 687bfbba5a1c ("media: venus: Add support for static video encoder/decoder declarations")
>>>>
>>>> Reworking this series from Konrad to incorporate this simple change
>>>>
>>
> Regards,
> Vikash


-- 
With best wishes
Dmitry
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Bryan O'Donoghue 10 months, 1 week ago
On 04/04/2025 06:24, Vikash Garodia wrote:
>>> How different is this from sm8250 which is already enabled on iris driver ?
>> As far as I remember, SM8250 support in Iris did not reach
>> feature-parity yet. So in my opinion it is fine to add new platforms to
>> the Venus driver, that will later migrate to the Iris driver.
> I would say, from decoder side all codecs are there now on Iris. H264 merged,
> while h265 and VP9 dec are posted as RFC, there is one compliance failure which
> is under debug to post them as regular patches.
> If we are mainly looking for decode usecases, then we should be on Iris.
> Preference would be to stay on Iris, otherwise we would have that extra ask to
> port it later from venus to iris.

Right now venus represents 9/20 - 45% of the patches being churned for 
sc8280xp.

https://github.com/jhovold/linux/tree/wip/sc8280xp-6.14-rc7

This is a good debate to have, however my memory of what we collectively 
agreed both in public and private was to continue to merge new silicon 
<= HFI6XX into venus unless and until iris hit feature parity for HFI6XX 
and to continue with venus at that point for < HFI6XX.

So merging sc8280xp - HFI6XX is consistent with our agreement, the right 
thing to do for our users and a big win in terms of technical debt 
reduction.

I will post an update to this series ASAP.

---
bod
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Johan Hovold 8 months, 2 weeks ago
Hi Bryan,

On Fri, Apr 04, 2025 at 10:02:47AM +0100, Bryan O'Donoghue wrote:
> On 04/04/2025 06:24, Vikash Garodia wrote:
> >>> How different is this from sm8250 which is already enabled on iris driver ?
> >> As far as I remember, SM8250 support in Iris did not reach
> >> feature-parity yet. So in my opinion it is fine to add new platforms to
> >> the Venus driver, that will later migrate to the Iris driver.
> > I would say, from decoder side all codecs are there now on Iris. H264 merged,
> > while h265 and VP9 dec are posted as RFC, there is one compliance failure which
> > is under debug to post them as regular patches.
> > If we are mainly looking for decode usecases, then we should be on Iris.
> > Preference would be to stay on Iris, otherwise we would have that extra ask to
> > port it later from venus to iris.
> 
> Right now venus represents 9/20 - 45% of the patches being churned for 
> sc8280xp.
> 
> https://github.com/jhovold/linux/tree/wip/sc8280xp-6.14-rc7
> 
> This is a good debate to have, however my memory of what we collectively 
> agreed both in public and private was to continue to merge new silicon 
> <= HFI6XX into venus unless and until iris hit feature parity for HFI6XX 
> and to continue with venus at that point for < HFI6XX.
> 
> So merging sc8280xp - HFI6XX is consistent with our agreement, the right 
> thing to do for our users and a big win in terms of technical debt 
> reduction.
> 
> I will post an update to this series ASAP.

It seems things may be moving again on the firmware front, so could you
please respin this series so we can have video acceleration support for
the X13s in 6.17?

Johan
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Dmitry Baryshkov 8 months, 2 weeks ago
On Tue, May 27, 2025 at 12:53:12PM +0200, Johan Hovold wrote:
> Hi Bryan,
> 
> On Fri, Apr 04, 2025 at 10:02:47AM +0100, Bryan O'Donoghue wrote:
> > On 04/04/2025 06:24, Vikash Garodia wrote:
> > >>> How different is this from sm8250 which is already enabled on iris driver ?
> > >> As far as I remember, SM8250 support in Iris did not reach
> > >> feature-parity yet. So in my opinion it is fine to add new platforms to
> > >> the Venus driver, that will later migrate to the Iris driver.
> > > I would say, from decoder side all codecs are there now on Iris. H264 merged,
> > > while h265 and VP9 dec are posted as RFC, there is one compliance failure which
> > > is under debug to post them as regular patches.
> > > If we are mainly looking for decode usecases, then we should be on Iris.
> > > Preference would be to stay on Iris, otherwise we would have that extra ask to
> > > port it later from venus to iris.
> > 
> > Right now venus represents 9/20 - 45% of the patches being churned for 
> > sc8280xp.
> > 
> > https://github.com/jhovold/linux/tree/wip/sc8280xp-6.14-rc7
> > 
> > This is a good debate to have, however my memory of what we collectively 
> > agreed both in public and private was to continue to merge new silicon 
> > <= HFI6XX into venus unless and until iris hit feature parity for HFI6XX 
> > and to continue with venus at that point for < HFI6XX.
> > 
> > So merging sc8280xp - HFI6XX is consistent with our agreement, the right 
> > thing to do for our users and a big win in terms of technical debt 
> > reduction.
> > 
> > I will post an update to this series ASAP.
> 
> It seems things may be moving again on the firmware front, so could you
> please respin this series so we can have video acceleration support for
> the X13s in 6.17?

And thanks to Mark, we indeed now have qcvss8280.mbn in linux-firmware.
It would be nice to get driver bits reposted and hopefully merged

-- 
With best wishes
Dmitry
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Vikash Garodia 10 months, 1 week ago

On 4/4/2025 2:32 PM, Bryan O'Donoghue wrote:
> On 04/04/2025 06:24, Vikash Garodia wrote:
>>>> How different is this from sm8250 which is already enabled on iris driver ?
>>> As far as I remember, SM8250 support in Iris did not reach
>>> feature-parity yet. So in my opinion it is fine to add new platforms to
>>> the Venus driver, that will later migrate to the Iris driver.
>> I would say, from decoder side all codecs are there now on Iris. H264 merged,
>> while h265 and VP9 dec are posted as RFC, there is one compliance failure which
>> is under debug to post them as regular patches.
>> If we are mainly looking for decode usecases, then we should be on Iris.
>> Preference would be to stay on Iris, otherwise we would have that extra ask to
>> port it later from venus to iris.
> 
> Right now venus represents 9/20 - 45% of the patches being churned for sc8280xp.
> 
> https://github.com/jhovold/linux/tree/wip/sc8280xp-6.14-rc7
> 
> This is a good debate to have, however my memory of what we collectively agreed
> both in public and private was to continue to merge new silicon <= HFI6XX into
> venus unless and until iris hit feature parity for HFI6XX and to continue with
> venus at that point for < HFI6XX.
Agree. Would appreciate your help later as well when we migrate these SOCs to iris.

Regards,
Vikash
> 
> So merging sc8280xp - HFI6XX is consistent with our agreement, the right thing
> to do for our users and a big win in terms of technical debt reduction.
> 
> I will post an update to this series ASAP.
> 
> ---
> bod
Re: [PATCH 0/8] Reup: SM8350 and SC8280XP venus support
Posted by Bryan O'Donoghue 11 months, 1 week ago
On 05/03/2025 03:19, Vikash Garodia wrote:
>> This series is a re-up of Konrad's original venus series for sc8280xp and
>> sm8350.Why this is enabled on venus driver ? Why not iris driver ? This needs an
> explanation on was this even tried to bring up on iris driver.
> 
> How different is this from sm8250 which is already enabled on iris driver ?

Ah let me parse the previous feedback, I don't have the full context of 
this series in my head yet.

thx

---
bod