mbox series

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

Message ID 20250304-b4-linux-media-comitters-sc8280xp-venus-v1-0-279c7ea55493@linaro.org
Headers show
Series Reup: SM8350 and SC8280XP venus support | expand

Message

Bryan O'Donoghue March 4, 2025, 1:07 p.m. UTC
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,

Comments

Bryan O'Donoghue March 5, 2025, 9:37 a.m. UTC | #1
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
Dmitry Baryshkov April 3, 2025, 4:58 p.m. UTC | #2
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
> >
Bryan O'Donoghue April 4, 2025, 9:02 a.m. UTC | #3
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
Vikash Garodia April 4, 2025, 10:33 a.m. UTC | #4
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