Re: [PATCH 02/33] iris: vidc: add core functions

Dikshita Agarwal
2023-08-14T18:52:28Z

On 8/1/2023 2:53 AM, Krzysztof Kozlowski wrote:
> On 28/07/2023 15:23, Vikash Garodia wrote:
>> From: Dikshita Agarwal <quic_dikshita@quicinc.com>
>>
>> This implements the platform driver methods, file
>> operations and v4l2 registration.
>
>
>> +
>> + core->debugfs_parent = msm_vidc_devm_debugfs_get(&pdev->dev);
>> + if (!core->debugfs_parent)
>> + d_vpr_h("Failed to create debugfs for msm_vidc\n");
>> +
>> + rc = msm_vidc_devm_init_core(&pdev->dev, core);
>> + if (rc) {
>> + d_vpr_e("%s: init core failed with %d\n", __func__, rc);
>> + goto init_core_failed;
>> + }
>> +
>> + rc = msm_vidc_init_platform(core);
>> + if (rc) {
>> + d_vpr_e("%s: init platform failed with %d\n", __func__, rc);
>> + rc = -EINVAL;
>> + goto init_plat_failed;
>> + }
>> +
>> + rc = msm_vidc_init_resources(core);
>> + if (rc) {
>> + d_vpr_e("%s: init resource failed with %d\n", __func__, rc);
>> + goto init_res_failed;
>
> NAK.
>
> return dev_err_probe.
Sure, will update in next version.
>
>> + }
>> +
>> + rc = msm_vidc_init_core_caps(core);
>> + if (rc) {
>> + d_vpr_e("%s: init core caps failed with %d\n", __func__, rc);
>> + goto init_res_failed;
>> + }
>> +
>> + rc = msm_vidc_init_instance_caps(core);
>> + if (rc) {
>> + d_vpr_e("%s: init inst cap failed with %d\n", __func__, rc);
>> + goto init_inst_caps_fail;
>> + }
>> +
>> + core->debugfs_root = msm_vidc_debugfs_init_core(core);
>> + if (!core->debugfs_root)
>> + d_vpr_h("Failed to init debugfs core\n");
>> +
>> + d_vpr_h("populating sub devices\n");
>> + /*
>> + * Trigger probe for each sub-device i.e. qcom,msm-vidc,context-bank.
>> + * When msm_vidc_probe is called for each sub-device, parse the
>> + * context-bank details.
>> + */
>> + rc = of_platform_populate(pdev->dev.of_node, msm_vidc_dt_match, NULL,
>> + &pdev->dev);
>> + if (rc) {
>> + d_vpr_e("Failed to trigger probe for sub-devices\n");
>> + goto sub_dev_failed;
>> + }
>> +
>> + rc = v4l2_device_register(&pdev->dev, &core->v4l2_dev);
>> + if (rc) {
>> + d_vpr_e("Failed to register v4l2 device\n");
>> + goto v4l2_reg_failed;
>> + }
>> +
>> + /* setup the decoder device */
>> + rc = msm_vidc_register_video_device(core, MSM_VIDC_DECODER, nr);
>> + if (rc) {
>> + d_vpr_e("Failed to register video decoder\n");
>> + goto dec_reg_failed;
>> + }
>> +
>> + /* setup the encoder device */
>> + rc = msm_vidc_register_video_device(core, MSM_VIDC_ENCODER, nr + 1);
>> + if (rc) {
>> + d_vpr_e("Failed to register video encoder\n");
>> + goto enc_reg_failed;
>> + }
>> +
>> + rc = venus_hfi_queue_init(core);
>> + if (rc) {
>> + d_vpr_e("%s: interface queues init failed\n", __func__);
>> + goto queues_init_failed;
>> + }
>> +
>> + rc = msm_vidc_core_init(core);
>> + if (rc) {
>> + d_vpr_e("%s: sys init failed\n", __func__);
>> + goto core_init_failed;
>> + }
>> +
>> + d_vpr_h("%s(): succssful\n", __func__);
>
> No need, drop.
Sure, will drop.
>
>> +
>> + return rc;
>> +
>> +core_init_failed:
>> + venus_hfi_queue_deinit(core);
>> +queues_init_failed:
>> + of_platform_depopulate(&pdev->dev);
>> +sub_dev_failed:
>> + msm_vidc_unregister_video_device(core, MSM_VIDC_ENCODER);
>> +enc_reg_failed:
>> + msm_vidc_unregister_video_device(core, MSM_VIDC_DECODER);
>> +dec_reg_failed:
>> + v4l2_device_unregister(&core->v4l2_dev);
>> +v4l2_reg_failed:
>> +init_inst_caps_fail:
>> +init_res_failed:
>> +init_plat_failed:
>> +init_core_failed:
>
> Ykes! No. This code needs more work.
>
I understand, will improve the design in next version.
> Best regards,
> Krzysztof
>