video: driver: add fix to avoid race condition in session close

Some clients attempt to invoke streamoff & driver fd close
(msm_vidc_close) sequence concurrently. So there might be chances
some commands willbe posted to command queue even after closing
hfi session(HFI_CMD_CLOSE) completed. So due to invalid session_id
firmware asserts.

Added change to avoid above mentioned issue and also amended minor
logging changes.

Change-Id: I2896567916cf5d18eb5b2a148efccf9bf07af3ea
Signed-off-by: Govindaraj Rajagopal <[email protected]>

driver/vidc/src/hfi_packet.c

@@ -657,7 +657,7 @@ int hfi_packet_session_command(struct msm_vidc_inst *inst,
 	int rc = 0;
 	struct msm_vidc_core *core;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: Invalid params\n", __func__);
 		return -EINVAL;
 	}

driver/vidc/src/msm_vdec.c

@@ -1765,7 +1765,7 @@ static int msm_vdec_qbuf_batch(struct msm_vidc_inst *inst,
 		i_vpr_e(inst, "%s: qbuf not allowed\n", __func__);
 		return -EINVAL;
 	} else if (allow == MSM_VIDC_DEFER) {
-		print_vidc_buffer(VIDC_LOW, "high", "qbuf deferred", inst, buf);
+		print_vidc_buffer(VIDC_LOW, "low ", "qbuf deferred", inst, buf);
 		return 0;
 	}
 
@@ -1773,7 +1773,7 @@ static int msm_vdec_qbuf_batch(struct msm_vidc_inst *inst,
 	if (inst->power.buffer_counter > SKIP_BATCH_WINDOW) {
 		count = msm_vidc_num_buffers(inst, MSM_VIDC_BUF_OUTPUT, MSM_VIDC_ATTR_DEFERRED);
 		if (count < inst->decode_batch.size) {
-			print_vidc_buffer(VIDC_LOW, "high", "batch-qbuf deferred", inst, buf);
+			print_vidc_buffer(VIDC_LOW, "low ", "batch-qbuf deferred", inst, buf);
 			schedule_batch_work(inst);
 			return 0;
 		}

driver/vidc/src/msm_vidc_driver.c

@@ -1009,9 +1009,9 @@ int msm_vidc_change_core_state(struct msm_vidc_core *core,
 		return -EINVAL;
 	}
 
-	d_vpr_h("%s: core state changed from %s to %s\n",
-		func, core_state_name(core->state),
-		core_state_name(request_state));
+	d_vpr_h("%s: core state changed to %s from %s\n",
+		func, core_state_name(request_state),
+		core_state_name(core->state));
 	core->state = request_state;
 	return 0;
 }
@@ -1036,9 +1036,15 @@ int msm_vidc_change_inst_state(struct msm_vidc_inst *inst,
 		return 0;
 	}
 
-	i_vpr_h(inst, "%s: state changed from %s to %s\n",
-		   func, state_name(inst->state), state_name(request_state));
+	if (request_state == MSM_VIDC_ERROR)
+		i_vpr_e(inst, "%s: state changed to %s from %s\n",
+		   func, state_name(request_state), state_name(inst->state));
+	else
+		i_vpr_h(inst, "%s: state changed to %s from %s\n",
+		   func, state_name(request_state), state_name(inst->state));
+
 	inst->state = request_state;
+
 	return 0;
 }
 
@@ -1864,7 +1870,7 @@ int msm_vidc_process_readonly_buffers(struct msm_vidc_inst *inst,
 	list_for_each_entry_safe(ro_buf, dummy, &ro_buffers->list, list) {
 		if (ro_buf->device_addr == buf->device_addr) {
 			buf->attr |= MSM_VIDC_ATTR_READ_ONLY;
-			print_vidc_buffer(VIDC_LOW, "low", "ro buf removed", inst, ro_buf);
+			print_vidc_buffer(VIDC_LOW, "low ", "ro buf removed", inst, ro_buf);
 			list_del(&ro_buf->list);
 			msm_vidc_put_vidc_buffer(inst, ro_buf);
 			break;
@@ -2609,7 +2615,7 @@ int msm_vidc_queue_buffer_single(struct msm_vidc_inst *inst, struct vb2_buffer *
 		i_vpr_e(inst, "%s: qbuf not allowed\n", __func__);
 		return -EINVAL;
 	} else if (allow == MSM_VIDC_DEFER) {
-		print_vidc_buffer(VIDC_LOW, "high", "qbuf deferred", inst, buf);
+		print_vidc_buffer(VIDC_LOW, "low ", "qbuf deferred", inst, buf);
 		return 0;
 	}
 
@@ -3389,6 +3395,11 @@ int msm_vidc_session_close(struct msm_vidc_inst *inst)
 	if (rc)
 		return rc;
 
+	/* we are not supposed to send any more commands after close */
+	i_vpr_h(inst, "%s: free session packet data\n", __func__);
+	kfree(inst->packet);
+	inst->packet = NULL;
+
 	core = inst->core;
 	i_vpr_h(inst, "%s: wait on close for time: %d ms\n",
 		__func__, core->capabilities[HW_RESPONSE_TIMEOUT].value);
@@ -3409,10 +3420,6 @@ int msm_vidc_session_close(struct msm_vidc_inst *inst)
 
 	msm_vidc_remove_session(inst);
 
-	i_vpr_h(inst, "%s: free session packet data\n", __func__);
-	kfree(inst->packet);
-	inst->packet = NULL;
-
 	return rc;
 }
 
@@ -4173,13 +4180,13 @@ void msm_vidc_destroy_buffers(struct msm_vidc_inst *inst)
 	}
 
 	list_for_each_entry_safe(buf, dummy, &inst->buffers.read_only.list, list) {
-		print_vidc_buffer(VIDC_ERR, "err", "destroying ro buffer", inst, buf);
+		print_vidc_buffer(VIDC_ERR, "err ", "destroying ro buffer", inst, buf);
 		list_del(&buf->list);
 		msm_vidc_put_vidc_buffer(inst, buf);
 	}
 
 	list_for_each_entry_safe(buf, dummy, &inst->buffers.release.list, list) {
-		print_vidc_buffer(VIDC_ERR, "err", "destroying release buffer", inst, buf);
+		print_vidc_buffer(VIDC_ERR, "err ", "destroying release buffer", inst, buf);
 		list_del(&buf->list);
 		msm_vidc_put_vidc_buffer(inst, buf);
 	}

driver/vidc/src/venus_hfi.c

@@ -2943,7 +2943,7 @@ int venus_hfi_session_open(struct msm_vidc_inst *inst)
 	int rc = 0;
 	struct msm_vidc_core *core;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -2985,7 +2985,7 @@ int venus_hfi_session_set_codec(struct msm_vidc_inst *inst)
 	struct msm_vidc_core *core;
 	u32 codec;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -3030,7 +3030,7 @@ int venus_hfi_session_property(struct msm_vidc_inst *inst,
 	int rc = 0;
 	struct msm_vidc_core *core;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -3103,7 +3103,7 @@ int venus_hfi_start(struct msm_vidc_inst *inst, enum msm_vidc_port_type port)
 	int rc = 0;
 	struct msm_vidc_core* core;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -3146,7 +3146,7 @@ int venus_hfi_stop(struct msm_vidc_inst *inst, enum msm_vidc_port_type port)
 	int rc = 0;
 	struct msm_vidc_core* core;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -3192,7 +3192,7 @@ int venus_hfi_session_command(struct msm_vidc_inst *inst,
 	int rc = 0;
 	struct msm_vidc_core *core;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -3242,7 +3242,7 @@ int venus_hfi_queue_super_buffer(struct msm_vidc_inst *inst,
 	u32 frame_size, meta_size, batch_size, cnt = 0;
 	u64 ts_delta_us;
 
-	if (!inst || !inst->core || !inst->capabilities) {
+	if (!inst || !inst->core || !inst->capabilities || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -3357,7 +3357,7 @@ int venus_hfi_queue_buffer(struct msm_vidc_inst *inst,
 	struct msm_vidc_core *core;
 	struct hfi_buffer hfi_buffer;
 
-	if (!inst || !inst->core) {
+	if (!inst || !inst->core || !inst->packet) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}
@@ -3423,7 +3423,7 @@ int venus_hfi_release_buffer(struct msm_vidc_inst *inst,
 	struct msm_vidc_core *core;
 	struct hfi_buffer hfi_buffer;
 
-	if (!inst || !inst->core || !buffer) {
+	if (!inst || !inst->core || !inst->packet || !buffer) {
 		d_vpr_e("%s: invalid params\n", __func__);
 		return -EINVAL;
 	}

driver/vidc/src/venus_hfi_response.c

@@ -523,7 +523,7 @@ static int handle_read_only_buffer(struct msm_vidc_inst *inst,
 		memcpy(ro_buf, buf, sizeof(struct msm_vidc_buffer));
 		INIT_LIST_HEAD(&ro_buf->list);
 		list_add_tail(&ro_buf->list, &ro_buffers->list);
-		print_vidc_buffer(VIDC_LOW, "low", "ro buf added", inst, ro_buf);
+		print_vidc_buffer(VIDC_LOW, "low ", "ro buf added", inst, ro_buf);
 	}
 	ro_buf->attr |= MSM_VIDC_ATTR_READ_ONLY;
 
@@ -564,7 +564,7 @@ static int handle_non_read_only_buffer(struct msm_vidc_inst *inst,
 	 *          if not present, do not error out
 	 */
 	if (found) {
-		print_vidc_buffer(VIDC_LOW, "low", "ro buf deleted", inst, ro_buf);
+		print_vidc_buffer(VIDC_LOW, "low ", "ro buf deleted", inst, ro_buf);
 		list_del(&ro_buf->list);
 		msm_vidc_put_vidc_buffer(inst, ro_buf);
 	}
@@ -729,7 +729,7 @@ static int handle_output_buffer(struct msm_vidc_inst *inst,
 			(buffer->flags & HFI_BUF_FW_FLAG_READONLY)) {
 			buffer->flags &= ~HFI_BUF_FW_FLAG_READONLY;
 			print_vidc_buffer(
-				VIDC_HIGH, "err", "RO flag in linear colorformat", inst, buf);
+				VIDC_HIGH, "high", "RO flag in linear colorformat", inst, buf);
 		}
 
 		if (buffer->flags & HFI_BUF_FW_FLAG_READONLY) {