On 4/9/2025 9:56 PM, Neil Armstrong wrote:
On 09/04/2025 16:29, Bryan O'Donoghue wrote:
On 08/04/2025 16:54, Dikshita Agarwal wrote:
Hi All,
This patch series adds initial support for the HEVC(H.265) and VP9 codecs in iris decoder. The objective of this work is to extend the decoder's capabilities to handle HEVC and VP9 codec streams, including necessary format handling and buffer management. In addition, the series also includes a set of fixes to address issues identified during testing of these additional codecs.
These patches also address the comments and feedback received from the RFC patches previously sent. I have made the necessary improvements based on the community's suggestions.
Changes sinces RFC:
- Added additional fixes to address issues identified during further
testing.
- Moved typo fix to a seperate patch [Neil]
- Reordered the patches for better logical flow and clarity [Neil,
Dmitry]
- Added fixes tag wherever applicable [Neil, Dmitry]
- Removed the default case in the switch statement for codecs [Bryan]
- Replaced if-else statements with switch-case [Bryan]
- Added comments for mbpf [Bryan]
- RFC:
https://lore.kernel.org/linux-media/20250305104335.3629945-1-quic_dikshita@q...
These patches are tested on SM8250 and SM8550 with v4l2-ctl and Gstreamer for HEVC and VP9 decoders, at the same time ensured that the existing H264 decoder functionality remains uneffected.
Note: 1 of the fluster compliance test is fixed with firmware [1] [1]: https://lore.kernel.org/linux-firmware/1a511921-446d-cdc4-0203-084c88a5dc1e@...
<snip>
Dikshita Agarwal (20): media: iris: Skip destroying internal buffer if not dequeued media: iris: Update CAPTURE format info based on OUTPUT format media: iris: Add handling for corrupt and drop frames media: iris: Avoid updating frame size to firmware during reconfig media: iris: Send V4L2_BUF_FLAG_ERROR for buffers with 0 filled length media: iris: Add handling for no show frames media: iris: Improve last flag handling media: iris: Skip flush on first sequence change media: iris: Prevent HFI queue writes when core is in deinit state media: iris: Remove redundant buffer count check in stream off media: iris: Remove deprecated property setting to firmware media: iris: Fix missing function pointer initialization media: iris: Fix NULL pointer dereference media: iris: Fix typo in depth variable media: iris: Add a comment to explain usage of MBPS media: iris: Add HEVC and VP9 formats for decoder media: iris: Add platform capabilities for HEVC and VP9 decoders media: iris: Set mandatory properties for HEVC and VP9 decoders. media: iris: Add internal buffer calculation for HEVC and VP9 decoders media: iris: Add codec specific check for VP9 decoder drain handling
drivers/media/platform/qcom/iris/iris_buffer.c | 22 +- drivers/media/platform/qcom/iris/iris_ctrls.c | 35 +- drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 + .../platform/qcom/iris/iris_hfi_gen1_command.c | 44 ++- .../platform/qcom/iris/iris_hfi_gen1_defines.h | 5 +- .../platform/qcom/iris/iris_hfi_gen1_response.c | 22 +- .../platform/qcom/iris/iris_hfi_gen2_command.c | 143 +++++++- .../platform/qcom/iris/iris_hfi_gen2_defines.h | 5 + .../platform/qcom/iris/iris_hfi_gen2_response.c | 57 ++- drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- drivers/media/platform/qcom/iris/iris_instance.h | 6 + .../platform/qcom/iris/iris_platform_common.h | 28 +- .../platform/qcom/iris/iris_platform_sm8250.c | 15 +- .../platform/qcom/iris/iris_platform_sm8550.c | 143 +++++++- drivers/media/platform/qcom/iris/iris_vb2.c | 3 +- drivers/media/platform/qcom/iris/iris_vdec.c | 113 +++--- drivers/media/platform/qcom/iris/iris_vdec.h | 11 + drivers/media/platform/qcom/iris/iris_vidc.c | 3 - drivers/media/platform/qcom/iris/iris_vpu_buffer.c | 397 ++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_vpu_buffer.h | 46 ++- 20 files changed, 948 insertions(+), 153 deletions(-)
base-commit: 7824b91d23e9f255f0e9d2acaa74265c9cac2e9c change-id: 20250402-iris-dec-hevc-vp9-2654a1fc4d0d
Best regards,
Assuming we merge Neils sm8650 stuff first, which I think we should merge first, you'll have a subsequent build error to fix [1]
I agree, it would be simpler, I prepared a fix to apply on top of this patchset.
Lets sort out the platform data handling. More so, when i see that the patch you are adding more of 8650 specific data into 8550 file.
https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linaro/arm-laptop/w...
Testing your series in isolation. I can confirm vp9 decodes also getting some strange prinouts which we need to follow up to see if they exist with the baseline driver [2].
https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linaro/arm-laptop/w...
<snip>
[ 126.582170] qcom-iris aa00000.video-codec: session error received 0x1000006: unknown [ 126.582177] qcom-iris aa00000.video-codec: session error received 0x4000004: invalid operation for current state
With the following on top of the last SM8650 patchet + this patchset, I have the same HEVC errors on SM8650, but VP9 works fine: [ 115.185745] qcom-iris aa00000.video-codec: session error received 0x4000004: invalid operation for current state [ 115.221058] qcom-iris aa00000.video-codec: session error received 0x1000006: unknown
==========================================><============================================== diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c index 65f3accc2fb2..7d5116528fca 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c @@ -213,6 +213,22 @@ static void iris_set_sm8550_preset_registers(struct iris_core *core) writel(0x0, core->reg_base + 0xB0088); }
+static void iris_set_sm8650_preset_registers(struct iris_core *core) +{ + writel(0x0, core->reg_base + 0xB0088); + writel(0x33332222, core->reg_base + 0x13030); + writel(0x44444444, core->reg_base + 0x13034); + writel(0x1022, core->reg_base + 0x13038); + writel(0x0, core->reg_base + 0x13040); + writel(0xFFFF, core->reg_base + 0x13048); + writel(0x33332222, core->reg_base + 0x13430); + writel(0x44444444, core->reg_base + 0x13434); + writel(0x1022, core->reg_base + 0x13438); + writel(0x0, core->reg_base + 0x13440); + writel(0xFFFF, core->reg_base + 0x13448); + writel(0x99, core->reg_base + 0xA013C); +}
This is strange, h264 decoder does not need any of those while VP9 needed it to work. I could see the same set of registers in downstream code, but cannot recollect now on the need to add those.
Regards, Vikash
static const struct icc_info sm8550_icc_table[] = { { "cpu-cfg", 1000, 1000 }, { "video-mem", 1000, 15000000 }, @@ -390,6 +406,7 @@ struct iris_platform_data sm8550_data = {
/* * Shares most of SM8550 data except:
- set_preset_registers to iris_set_sm8650_preset_registers
* - vpu_ops to iris_vpu33_ops * - clk_rst_tbl to sm8650_clk_reset_table * - controller_rst_tbl to sm8650_controller_reset_table @@ -400,7 +417,7 @@ struct iris_platform_data sm8650_data = { .init_hfi_command_ops = iris_hfi_gen2_command_ops_init, .init_hfi_response_ops = iris_hfi_gen2_response_ops_init, .vpu_ops = &iris_vpu33_ops, - .set_preset_registers = iris_set_sm8550_preset_registers, + .set_preset_registers = iris_set_sm8650_preset_registers, .icc_tbl = sm8550_icc_table, .icc_tbl_size = ARRAY_SIZE(sm8550_icc_table), .clk_rst_tbl = sm8650_clk_reset_table, @@ -428,20 +445,34 @@ struct iris_platform_data sm8650_data = { .ubwc_config = &ubwc_config_sm8550, .num_vpp_pipe = 4, .max_session_count = 16, - .max_core_mbpf = ((8192 * 4352) / 256) * 2, - .input_config_params = - sm8550_vdec_input_config_params, - .input_config_params_size = - ARRAY_SIZE(sm8550_vdec_input_config_params), + .max_core_mbpf = NUM_MBS_8K * 2, + .input_config_params_default = + sm8550_vdec_input_config_params_default, + .input_config_params_default_size = + ARRAY_SIZE(sm8550_vdec_input_config_params_default), + .input_config_params_hevc = + sm8550_vdec_input_config_param_hevc, + .input_config_params_hevc_size = + ARRAY_SIZE(sm8550_vdec_input_config_param_hevc), + .input_config_params_vp9 = + sm8550_vdec_input_config_param_vp9, + .input_config_params_vp9_size = + ARRAY_SIZE(sm8550_vdec_input_config_param_vp9), .output_config_params = sm8550_vdec_output_config_params, .output_config_params_size = ARRAY_SIZE(sm8550_vdec_output_config_params), .dec_input_prop = sm8550_vdec_subscribe_input_properties, .dec_input_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_input_properties), - .dec_output_prop = sm8550_vdec_subscribe_output_properties, - .dec_output_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
+ .dec_output_prop_avc = sm8550_vdec_subscribe_output_properties_avc, + .dec_output_prop_avc_size = + ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_avc), + .dec_output_prop_hevc = sm8550_vdec_subscribe_output_properties_hevc, + .dec_output_prop_hevc_size = + ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_hevc), + .dec_output_prop_vp9 = sm8550_vdec_subscribe_output_properties_vp9, + .dec_output_prop_vp9_size = + ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_vp9), .dec_ip_int_buf_tbl = sm8550_dec_ip_int_buf_tbl, .dec_ip_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_ip_int_buf_tbl), .dec_op_int_buf_tbl = sm8550_dec_op_int_buf_tbl, ==========================================><==============================================
Thanks, Neil