| Message ID | 20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-2-ef7e5f85f302@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: venus: Provide support for selecting encoder/decoder from in-driver | expand |
Hi Bryan, Le lundi 09 décembre 2024 à 11:52 +0000, Bryan O'Donoghue a écrit : > Populate encoder and decoder node-name entries for the upstream parts. Once > done the compat="video-encoder" and compat="video-decoder" in the dtsi can > be dropped though the venus driver will continue to favour DT declared > video-encoder/video-decoder declarations over static declarations for > compatibility. Hope this hardcoding of node name is historical ? And not done for newer chips ? We discourage userspace on relying on node names cause it always leads to complication and non-portable code. Nicolas > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/media/platform/qcom/venus/core.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index 88dfa9f240dc6d18a7f58dc06b1bf10274b7121e..deef391d78770b8ae0f486dd3a3ab44c4ea6a581 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -674,6 +674,8 @@ static const struct venus_resources msm8916_res = { > .vmem_addr = 0, > .dma_mask = 0xddc00000 - 1, > .fwname = "qcom/venus-1.8/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct freq_tbl msm8996_freq_table[] = { > @@ -883,6 +885,8 @@ static const struct venus_resources sdm845_res_v2 = { > .cp_nonpixel_start = 0x1000000, > .cp_nonpixel_size = 0x24800000, > .fwname = "qcom/venus-5.2/venus.mbn", > + .dec_nodename = "video-core0", > + .enc_nodename = "video-core1", > }; > > static const struct freq_tbl sc7180_freq_table[] = { > @@ -931,6 +935,8 @@ static const struct venus_resources sc7180_res = { > .cp_nonpixel_start = 0x1000000, > .cp_nonpixel_size = 0x24800000, > .fwname = "qcom/venus-5.4/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct freq_tbl sm8250_freq_table[] = { > @@ -986,6 +992,8 @@ static const struct venus_resources sm8250_res = { > .vmem_addr = 0, > .dma_mask = 0xe0000000 - 1, > .fwname = "qcom/vpu-1.0/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct freq_tbl sc7280_freq_table[] = { > @@ -1048,6 +1056,8 @@ static const struct venus_resources sc7280_res = { > .cp_nonpixel_start = 0x1000000, > .cp_nonpixel_size = 0x24800000, > .fwname = "qcom/vpu-2.0/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct of_device_id venus_dt_match[] = { >
On 13/12/2024 20:00, Nicolas Dufresne wrote: > Hope this hardcoding of node name is historical ? Hardcoding is historical in dts. We need to add two more chips into venus before iris is merged and at feature parity for HFI_6XX and above - HFI_GEN2 Something like this. enum { HFI_1XX .. HFI_6XX HFI_GEN2 .. }; > And not done for newer chips ? HFI_6XX and above will be fully supported in "iris" with encoder/decoder selection done at session creation time. Iris is being added phased. Basic decoder with one format, followed by decoder and additional formats. Once we get to feature parity HFI_6XX and above will be supported in Iris and removed from venus. Leaving HFI_4XX and below. That's a long winded way of saying new chips minted from the fab will either be HFI_GEN2+ or HFI_6XX. > We discourage userspace on relying on node names cause it always leads to > complication and non-portable code. Writing this driver from scratch - basically what HFI_6XX in Iris does, you'd select encoder/decoder when you create the initial session - the initial state. For venus that's an unknown amount of work to do. What we _could_ certainly do is make the static assignment in this series assignable via a kernel parameter. I'd say though that's an additional series on top of this. First pass here is just to fix up the original sin, not to improve selectivity, just yet. --- bod
On 12/9/2024 7:52 PM, Bryan O'Donoghue wrote: > Populate encoder and decoder node-name entries for the upstream parts. Once > done the compat="video-encoder" and compat="video-decoder" in the dtsi can > be dropped though the venus driver will continue to favour DT declared > video-encoder/video-decoder declarations over static declarations for > compatibility. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/media/platform/qcom/venus/core.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index 88dfa9f240dc6d18a7f58dc06b1bf10274b7121e..deef391d78770b8ae0f486dd3a3ab44c4ea6a581 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -674,6 +674,8 @@ static const struct venus_resources msm8916_res = { > .vmem_addr = 0, > .dma_mask = 0xddc00000 - 1, > .fwname = "qcom/venus-1.8/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct freq_tbl msm8996_freq_table[] = { > @@ -883,6 +885,8 @@ static const struct venus_resources sdm845_res_v2 = { > .cp_nonpixel_start = 0x1000000, > .cp_nonpixel_size = 0x24800000, > .fwname = "qcom/venus-5.2/venus.mbn", > + .dec_nodename = "video-core0", > + .enc_nodename = "video-core1", > }; > > static const struct freq_tbl sc7180_freq_table[] = { > @@ -931,6 +935,8 @@ static const struct venus_resources sc7180_res = { > .cp_nonpixel_start = 0x1000000, > .cp_nonpixel_size = 0x24800000, > .fwname = "qcom/venus-5.4/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct freq_tbl sm8250_freq_table[] = { > @@ -986,6 +992,8 @@ static const struct venus_resources sm8250_res = { > .vmem_addr = 0, > .dma_mask = 0xe0000000 - 1, > .fwname = "qcom/vpu-1.0/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct freq_tbl sc7280_freq_table[] = { > @@ -1048,6 +1056,8 @@ static const struct venus_resources sc7280_res = { > .cp_nonpixel_start = 0x1000000, > .cp_nonpixel_size = 0x24800000, > .fwname = "qcom/vpu-2.0/venus.mbn", > + .dec_nodename = "video-decoder", > + .enc_nodename = "video-encoder", > }; > > static const struct of_device_id venus_dt_match[] = { It is working fine on QCS615. Tested-by: Renjiang Han <quic_renjiang@quicinc.com>
diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 88dfa9f240dc6d18a7f58dc06b1bf10274b7121e..deef391d78770b8ae0f486dd3a3ab44c4ea6a581 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -674,6 +674,8 @@ static const struct venus_resources msm8916_res = { .vmem_addr = 0, .dma_mask = 0xddc00000 - 1, .fwname = "qcom/venus-1.8/venus.mbn", + .dec_nodename = "video-decoder", + .enc_nodename = "video-encoder", }; static const struct freq_tbl msm8996_freq_table[] = { @@ -883,6 +885,8 @@ static const struct venus_resources sdm845_res_v2 = { .cp_nonpixel_start = 0x1000000, .cp_nonpixel_size = 0x24800000, .fwname = "qcom/venus-5.2/venus.mbn", + .dec_nodename = "video-core0", + .enc_nodename = "video-core1", }; static const struct freq_tbl sc7180_freq_table[] = { @@ -931,6 +935,8 @@ static const struct venus_resources sc7180_res = { .cp_nonpixel_start = 0x1000000, .cp_nonpixel_size = 0x24800000, .fwname = "qcom/venus-5.4/venus.mbn", + .dec_nodename = "video-decoder", + .enc_nodename = "video-encoder", }; static const struct freq_tbl sm8250_freq_table[] = { @@ -986,6 +992,8 @@ static const struct venus_resources sm8250_res = { .vmem_addr = 0, .dma_mask = 0xe0000000 - 1, .fwname = "qcom/vpu-1.0/venus.mbn", + .dec_nodename = "video-decoder", + .enc_nodename = "video-encoder", }; static const struct freq_tbl sc7280_freq_table[] = { @@ -1048,6 +1056,8 @@ static const struct venus_resources sc7280_res = { .cp_nonpixel_start = 0x1000000, .cp_nonpixel_size = 0x24800000, .fwname = "qcom/vpu-2.0/venus.mbn", + .dec_nodename = "video-decoder", + .enc_nodename = "video-encoder", }; static const struct of_device_id venus_dt_match[] = {
Populate encoder and decoder node-name entries for the upstream parts. Once done the compat="video-encoder" and compat="video-decoder" in the dtsi can be dropped though the venus driver will continue to favour DT declared video-encoder/video-decoder declarations over static declarations for compatibility. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/media/platform/qcom/venus/core.c | 10 ++++++++++ 1 file changed, 10 insertions(+)