video: driver: fix video driver remove

While removing driver fw should be unloaded.
Also improved error handling.

Change-Id: Ia9f224da6f176c8a50c153f67fe8a77a92b7b08f
Signed-off-by: Chinmay Sawarkar <[email protected]>

driver/vidc/inc/msm_vidc_driver.h

@@ -301,5 +301,9 @@ bool inst_lock_check(struct msm_vidc_inst *inst, const char *function);
 int msm_vidc_update_meta_port_settings(struct msm_vidc_inst *inst);
 void msm_vidc_schedule_core_deinit(struct msm_vidc_core *core);
 bool msm_vidc_is_super_buffer(struct msm_vidc_inst *inst);
+int msm_vidc_init_core_caps(struct msm_vidc_core* core);
+int msm_vidc_init_instance_caps(struct msm_vidc_core* core);
+int msm_vidc_deinit_core_caps(struct msm_vidc_core* core);
+int msm_vidc_deinit_instance_caps(struct msm_vidc_core* core);
 #endif // _MSM_VIDC_DRIVER_H_
 

driver/vidc/src/msm_vidc_driver.c

@@ -2085,7 +2085,7 @@ int msm_vidc_remove_session(struct msm_vidc_inst *inst)
 	list_for_each_entry_safe(i, temp, &core->instances, list) {
 		if (i->session_id == inst->session_id) {
 			list_del_init(&i->list);
-			d_vpr_h("%s: removed session %d\n",
+			d_vpr_h("%s: removed session %#x\n",
 				__func__, i->session_id);
 			inst->sid = 0;
 		}
@@ -2341,7 +2341,7 @@ int msm_vidc_get_inst_capability(struct msm_vidc_inst *inst)
 	return rc;
 }
 
-static int msm_vidc_deinit_core_caps(struct msm_vidc_core *core)
+int msm_vidc_deinit_core_caps(struct msm_vidc_core *core)
 {
 	int rc = 0;
 
@@ -2349,14 +2349,15 @@ static int msm_vidc_deinit_core_caps(struct msm_vidc_core *core)
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
-	d_vpr_h("%s: skip freeing core capabilities\n", __func__);
-	//kfree(core->capabilities);
-	//core->capabilities = NULL;
+
+	kfree(core->capabilities);
+	core->capabilities = NULL;
+	d_vpr_h("%s: Core capabilities freed\n", __func__);
 
 	return rc;
 }
 
-static int msm_vidc_init_core_caps(struct msm_vidc_core *core)
+int msm_vidc_init_core_caps(struct msm_vidc_core *core)
 {
 	int rc = 0;
 	int i, num_platform_caps;
@@ -2376,32 +2377,25 @@ static int msm_vidc_init_core_caps(struct msm_vidc_core *core)
 			goto exit;
 	}
 
+	core->capabilities = kcalloc(1,
+		(sizeof(struct msm_vidc_core_capability) *
+		(CORE_CAP_MAX + 1)), GFP_KERNEL);
 	if (!core->capabilities) {
-		core->capabilities = kcalloc(1,
-			(sizeof(struct msm_vidc_core_capability) *
-			CORE_CAP_MAX), GFP_KERNEL);
-		if (!core->capabilities) {
-			d_vpr_e("%s: failed to allocate core capabilities\n",
-				__func__);
-			rc = -ENOMEM;
-			goto exit;
-		}
-	} else {
-		d_vpr_h("%s: capabilities memory is expected to be freed\n",
+		d_vpr_e("%s: failed to allocate core capabilities\n",
 			__func__);
+		rc = -ENOMEM;
+		goto exit;
 	}
 
 	num_platform_caps = core->platform->data.core_data_size;
 
 	/* loop over platform caps */
-	for (i = 0; i < num_platform_caps; i++) {
+	for (i = 0; i < num_platform_caps && i < CORE_CAP_MAX; i++) {
 		core->capabilities[platform_data[i].type].type = platform_data[i].type;
 		core->capabilities[platform_data[i].type].value = platform_data[i].value;
 	}
 
 exit:
-	if (rc)
-		msm_vidc_deinit_core_caps(core);
 	return rc;
 }
 
@@ -2434,7 +2428,7 @@ static void update_inst_capability(struct msm_platform_inst_capability *in,
 	}
 }
 
-static int msm_vidc_deinit_instance_caps(struct msm_vidc_core *core)
+int msm_vidc_deinit_instance_caps(struct msm_vidc_core *core)
 {
 	int rc = 0;
 
@@ -2442,14 +2436,15 @@ static int msm_vidc_deinit_instance_caps(struct msm_vidc_core *core)
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
-	d_vpr_h("%s: skip freeing core->instance capabilities\n", __func__);
-	//kfree(core->inst_caps);
-	//core->inst_caps = NULL;
+
+	kfree(core->inst_caps);
+	core->inst_caps = NULL;
+	d_vpr_h("%s: core->inst_caps freed\n", __func__);
 
 	return rc;
 }
 
-static int msm_vidc_init_instance_caps(struct msm_vidc_core *core)
+int msm_vidc_init_instance_caps(struct msm_vidc_core *core)
 {
 	int rc = 0;
 	u8 enc_valid_codecs, dec_valid_codecs;
@@ -2482,20 +2477,14 @@ static int msm_vidc_init_instance_caps(struct msm_vidc_core *core)
 	COUNT_BITS(count_bits, codecs_count);
 
 	core->codecs_count = codecs_count;
-
+	core->inst_caps = kcalloc(codecs_count,
+		sizeof(struct msm_vidc_inst_capability),
+		GFP_KERNEL);
 	if (!core->inst_caps) {
-		core->inst_caps = kcalloc(codecs_count,
-			sizeof(struct msm_vidc_inst_capability),
-			GFP_KERNEL);
-		if (!core->inst_caps) {
-			d_vpr_e("%s: failed to allocate core capabilities\n",
-				__func__);
-			rc = -ENOMEM;
-			goto error;
-		}
-	} else {
-		d_vpr_h("%s: capabilities memory is expected to be freed\n",
+		d_vpr_e("%s: failed to allocate core capabilities\n",
 			__func__);
+		rc = -ENOMEM;
+		goto error;
 	}
 
 	check_bit = 0;
@@ -2547,10 +2536,7 @@ static int msm_vidc_init_instance_caps(struct msm_vidc_core *core)
 		}
 	}
 
-	return 0;
 error:
-	if (rc)
-		msm_vidc_deinit_instance_caps(core);
 	return rc;
 }
 
@@ -2568,13 +2554,13 @@ int msm_vidc_core_deinit(struct msm_vidc_core *core, bool force)
 	d_vpr_h("%s()\n", __func__);
 	if (core->state == MSM_VIDC_CORE_DEINIT)
 		goto unlock;
+
 	if (!force)
 		if (!list_empty(&core->instances))
 			goto unlock;
 
 	venus_hfi_core_deinit(core);
-	msm_vidc_deinit_instance_caps(core);
-	msm_vidc_deinit_core_caps(core);
+
 	/* unlink all sessions from core, if any */
 	list_for_each_entry_safe(inst, dummy, &core->instances, list) {
 		msm_vidc_change_inst_state(inst, MSM_VIDC_ERROR, __func__);
@@ -2602,13 +2588,6 @@ int msm_vidc_core_init(struct msm_vidc_core *core)
 		goto unlock;
 	}
 
-	rc = msm_vidc_init_core_caps(core);
-	if (rc)
-		goto unlock;
-	rc = msm_vidc_init_instance_caps(core);
-	if (rc)
-		goto unlock;
-
 	msm_vidc_change_core_state(core, MSM_VIDC_CORE_INIT, __func__);
 	init_completion(&core->init_done);
 	core->smmu_fault_handled = false;

driver/vidc/src/msm_vidc_probe.c

@@ -230,6 +230,49 @@ exit:
 	return rc;
 }
 
+static int msm_vidc_remove(struct platform_device* pdev)
+{
+	struct msm_vidc_core* core;
+
+	if (!pdev) {
+		d_vpr_e("%s: invalid input %pK", __func__, pdev);
+		return -EINVAL;
+	}
+	core = dev_get_drvdata(&pdev->dev);
+	if (!core) {
+		d_vpr_e("%s: invalid core", __func__);
+		return -EINVAL;
+	}
+
+	d_vpr_h("%s()\n", __func__);
+
+	msm_vidc_core_deinit(core, true);
+
+	msm_vidc_unregister_video_device(core, MSM_VIDC_ENCODER);
+	msm_vidc_unregister_video_device(core, MSM_VIDC_DECODER);
+	//device_remove_file(&core->vdev[MSM_VIDC_ENCODER].vdev.dev,
+		//&dev_attr_link_name);
+	//device_remove_file(&core->vdev[MSM_VIDC_DECODER].vdev.dev,
+		//&dev_attr_link_name);
+	v4l2_device_unregister(&core->v4l2_dev);
+	sysfs_remove_group(&pdev->dev.kobj, &msm_vidc_core_attr_group);
+
+	msm_vidc_deinit_instance_caps(core);
+	msm_vidc_deinit_core_caps(core);
+
+	msm_vidc_deinit_irq(core);
+	msm_vidc_deinit_platform(pdev);
+	msm_vidc_deinit_dt(pdev);
+	msm_vidc_deinitialize_core(core);
+
+	dev_set_drvdata(&pdev->dev, NULL);
+	debugfs_remove_recursive(core->debugfs_parent);
+	kfree(core);
+	g_core = NULL;
+
+	return 0;
+}
+
 static int msm_vidc_probe_video_device(struct platform_device *pdev)
 {
 	int rc = 0;
@@ -253,49 +296,65 @@ static int msm_vidc_probe_video_device(struct platform_device *pdev)
 	rc = msm_vidc_initialize_core(core);
 	if (rc) {
 		d_vpr_e("%s: init core failed with %d\n", __func__, rc);
-		goto exit;
+		goto init_core_failed;
 	}
 
 	rc = msm_vidc_init_dt(pdev);
 	if (rc) {
 		d_vpr_e("%s: init dt failed with %d\n", __func__, rc);
-		return -EINVAL;
+		rc = -EINVAL;
+		goto init_dt_failed;
 	}
 
 	rc = msm_vidc_init_platform(pdev);
 	if (rc) {
 		d_vpr_e("%s: init platform failed with %d\n", __func__, rc);
-		return -EINVAL;
+		rc = -EINVAL;
+		goto init_plat_failed;
 	}
 
 	rc = msm_vidc_init_irq(core);
-	if (rc)
-		return rc;
+	if (rc) {
+		d_vpr_e("%s: init irq failed with %d\n", __func__, rc);
+		goto init_irq_failed;
+	}
+
+	rc = msm_vidc_init_core_caps(core);
+	if (rc) {
+		d_vpr_e("%s: init core caps failed with %d\n", __func__, rc);
+		goto init_core_caps_fail;
+	}
+
+	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;
+	}
 
 	rc = sysfs_create_group(&pdev->dev.kobj, &msm_vidc_core_attr_group);
 	if (rc) {
 		d_vpr_e("Failed to create attributes\n");
-		goto exit;
+		goto init_group_failed;
 	}
 
 	rc = v4l2_device_register(&pdev->dev, &core->v4l2_dev);
 	if (rc) {
 		d_vpr_e("Failed to register v4l2 device\n");
-		goto exit;
+		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 exit;
+		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 exit;
+		goto enc_reg_failed;
 	}
 
 	core->debugfs_root = msm_vidc_debugfs_init_core(core);
@@ -313,12 +372,36 @@ static int msm_vidc_probe_video_device(struct platform_device *pdev)
 			&pdev->dev);
 	if (rc) {
 		d_vpr_e("Failed to trigger probe for sub-devices\n");
-		goto exit;
+		goto sub_dev_failed;
 	}
 
-exit:
-	if (rc)
-		debugfs_remove_recursive(core->debugfs_parent);
+	return rc;
+
+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:
+	sysfs_remove_group(&pdev->dev.kobj, &msm_vidc_core_attr_group);
+init_group_failed:
+	msm_vidc_deinit_instance_caps(core);
+init_inst_caps_fail:
+	msm_vidc_deinit_core_caps(core);
+init_core_caps_fail:
+	msm_vidc_deinit_irq(core);
+init_irq_failed:
+	msm_vidc_deinit_platform(pdev);
+init_plat_failed:
+	msm_vidc_deinit_dt(pdev);
+init_dt_failed:
+	msm_vidc_deinitialize_core(core);
+init_core_failed:
+	dev_set_drvdata(&pdev->dev, NULL);
+	debugfs_remove_recursive(core->debugfs_parent);
+	kfree(core);
+	g_core = NULL;
 
 	return rc;
 }
@@ -351,41 +434,6 @@ static int msm_vidc_probe(struct platform_device *pdev)
 	return -EINVAL;
 }
 
-static int msm_vidc_remove(struct platform_device *pdev)
-{
-	struct msm_vidc_core *core;
-
-	if (!pdev) {
-		d_vpr_e("%s: invalid input %pK", __func__, pdev);
-		return -EINVAL;
-	}
-	core = dev_get_drvdata(&pdev->dev);
-	if (!core) {
-		d_vpr_e("%s: invalid core", __func__);
-		return -EINVAL;
-	}
-
-	d_vpr_h("%s()\n", __func__);
-
-	msm_vidc_unregister_video_device(core, MSM_VIDC_ENCODER);
-	msm_vidc_unregister_video_device(core, MSM_VIDC_DECODER);
-	//device_remove_file(&core->vdev[MSM_VIDC_ENCODER].vdev.dev,
-				//&dev_attr_link_name);
-	//device_remove_file(&core->vdev[MSM_VIDC_DECODER].vdev.dev,
-				//&dev_attr_link_name);
-	v4l2_device_unregister(&core->v4l2_dev);
-	sysfs_remove_group(&pdev->dev.kobj, &msm_vidc_core_attr_group);
-	msm_vidc_deinit_irq(core);
-	msm_vidc_deinit_platform(pdev);
-	msm_vidc_deinit_dt(pdev);
-	msm_vidc_deinitialize_core(core);
-	dev_set_drvdata(&pdev->dev, NULL);
-	debugfs_remove_recursive(core->debugfs_parent);
-	kfree(core);
-	g_core = NULL;
-	return 0;
-}
-
 struct platform_driver msm_vidc_driver = {
 	.probe = msm_vidc_probe,
 	.remove = msm_vidc_remove,

driver/vidc/src/venus_hfi.c

@@ -2484,6 +2484,8 @@ fail_init_res:
 
 void __unload_fw(struct msm_vidc_core *core)
 {
+	int rc = 0;
+
 	if (!core->dt->fw_cookie)
 		return;
 
@@ -2491,13 +2493,16 @@ void __unload_fw(struct msm_vidc_core *core)
 	if (core->state != MSM_VIDC_CORE_DEINIT)
 		flush_workqueue(core->pm_workq);
 
-	qcom_scm_pas_shutdown(core->dt->fw_cookie);
+	rc = qcom_scm_pas_shutdown(core->dt->fw_cookie);
+	if (rc)
+		d_vpr_e("Firmware unload failed rc=%d\n", rc);
+
 	core->dt->fw_cookie = 0;
 
 	__venus_power_off(core);
 	__deinit_resources(core);
 
-	d_vpr_h("Firmware unloaded successfully\n");
+	d_vpr_h("%s done\n", __func__);
 }
 
 static int __response_handler(struct msm_vidc_core *core)