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