video: driver: fix release issue for superbuffer

In VP9 superframe usecase one ETB has multiple FBDs,
and the EBD is after the first FBD.When driver receives
first FBD that is not in DPB_LIST, it releases this buffer.
Later on when the EBD arrives with dpb list property and
it turns out that first FBD for which release was sent is
actually in DPB_LIST, causing FW assertion.

To fix this issue, in the reverse path, when handling
dpb list property, mark those read only buffers which
are not in dpb list as release eligible. Later, in the
qbuf path for output buffers, send release for all those
buffers which were earlier marked as release eligible.

Change-Id: I5d6035b808653478311f41ebe53f60e728c7dd2a
Signed-off-by: Darshana Patil <[email protected]>

driver/vidc/inc/msm_vidc_inst.h

@@ -102,10 +102,6 @@ struct msm_vidc_inst {
 	struct msm_vidc_mem_list_info      mem_info;
 	struct msm_vidc_timestamps         timestamps;
 	struct msm_vidc_timestamps         ts_reorder; /* list of struct msm_vidc_timestamp */
-	bool                               subscribed_input_psc;
-	bool                               subscribed_output_psc;
-	bool                               subscribed_input_prop;
-	bool                               subscribed_output_prop;
 	struct msm_vidc_subscription_params       subcr_params[MAX_PORT];
 	struct msm_vidc_hfi_frame_info     hfi_frame_info;
 	struct msm_vidc_decode_batch       decode_batch;
@@ -140,6 +136,8 @@ struct msm_vidc_inst {
 	u64                                initial_time_us;
 	u32                                max_input_data_size;
 	u32                                dpb_list_payload[MAX_DPB_LIST_ARRAY_SIZE];
+	bool                               input_dpb_list_enabled;
+	bool                               output_dpb_list_enabled;
 	u32                                max_map_output_count;
 	u32                                auto_framerate;
 	u32                                max_rate;

driver/vidc/inc/msm_vidc_internal.h

@@ -490,6 +490,7 @@ enum msm_vidc_buffer_attributes {
 	MSM_VIDC_ATTR_QUEUED                    = BIT(3),
 	MSM_VIDC_ATTR_DEQUEUED                  = BIT(4),
 	MSM_VIDC_ATTR_BUFFER_DONE               = BIT(5),
+	MSM_VIDC_ATTR_RELEASE_ELIGIBLE          = BIT(6),
 };
 
 enum msm_vidc_buffer_region {
@@ -817,7 +818,6 @@ struct msm_vidc_subscription_params {
 	u32                    tier;
 	u32                    av1_film_grain_present;
 	u32                    av1_super_block_enabled;
-	u32                    dpb_list_enabled;
 };
 
 struct msm_vidc_hfi_frame_info {

driver/vidc/src/msm_vdec.c

@@ -973,7 +973,7 @@ static int msm_vdec_subscribe_property(struct msm_vidc_inst *inst,
 			}
 
 			if (subcribe_prop[i] == HFI_PROP_DPB_LIST) {
-				inst->subcr_params[port].dpb_list_enabled = true;
+				inst->input_dpb_list_enabled = true;
 				i_vpr_h(inst, "%s: DPB_LIST suscribed on input port", __func__);
 			}
 		}
@@ -989,7 +989,7 @@ static int msm_vdec_subscribe_property(struct msm_vidc_inst *inst,
 			}
 
 			if (subcribe_prop[i] == HFI_PROP_DPB_LIST) {
-				inst->subcr_params[port].dpb_list_enabled = true;
+				inst->output_dpb_list_enabled = true;
 				i_vpr_h(inst, "%s: DPB_LIST suscribed on output port", __func__);
 			}
 		}
@@ -1951,6 +1951,34 @@ static int msm_vdec_qbuf_batch(struct msm_vidc_inst *inst,
 	return rc;
 }
 
+static int msm_vdec_release_eligible_buffers(struct msm_vidc_inst *inst)
+{
+	int rc = 0;
+	struct msm_vidc_buffer *ro_buf;
+
+	if (!inst) {
+		d_vpr_e("%s: invalid params\n", __func__);
+		return -EINVAL;
+	}
+
+	list_for_each_entry(ro_buf, &inst->buffers.read_only.list, list) {
+		/* release only release eligible read-only buffers */
+		if (!(ro_buf->attr & MSM_VIDC_ATTR_RELEASE_ELIGIBLE))
+			continue;
+		/* skip releasing buffers for which release cmd was already sent */
+		if (ro_buf->attr & MSM_VIDC_ATTR_PENDING_RELEASE)
+			continue;
+		rc = venus_hfi_release_buffer(inst, ro_buf);
+		if (rc)
+			return rc;
+		ro_buf->attr |= MSM_VIDC_ATTR_PENDING_RELEASE;
+		ro_buf->attr &= ~MSM_VIDC_ATTR_RELEASE_ELIGIBLE;
+		print_vidc_buffer(VIDC_LOW, "low ", "release buf", inst, ro_buf);
+	}
+
+	return rc;
+}
+
 static int msm_vdec_release_nonref_buffers(struct msm_vidc_inst *inst)
 {
 	int rc = 0;
@@ -1968,7 +1996,7 @@ static int msm_vdec_release_nonref_buffers(struct msm_vidc_inst *inst)
 	 * if DPB_LIST subscribed on output port then driver need to
 	 * hold MAX_BPB_COUNT of read only buffer at least.
 	 */
-	if (!inst->subcr_params[OUTPUT_PORT].dpb_list_enabled)
+	if (!inst->output_dpb_list_enabled)
 		goto release_buffers;
 
 	/* count read_only buffers which are not pending release in read_only list */
@@ -2069,8 +2097,17 @@ int msm_vdec_qbuf(struct msm_vidc_inst *inst, struct vb2_buffer *vb2)
 	if (rc)
 		return rc;
 
+	/*
+	 * if DPB_LIST property is subscribed on output port, then
+	 * driver needs to hold at least MAX_BPB_COUNT of read only
+	 * buffers. So call msm_vdec_release_nonref_buffers() to handle
+	 * the same.
+	 */
 	if (vb2->type == OUTPUT_MPLANE) {
-		rc = msm_vdec_release_nonref_buffers(inst);
+		if (inst->input_dpb_list_enabled)
+			rc = msm_vdec_release_eligible_buffers(inst);
+		else if (inst->output_dpb_list_enabled)
+			rc = msm_vdec_release_nonref_buffers(inst);
 		if (rc)
 			return rc;
 	}
@@ -2743,8 +2780,8 @@ int msm_vdec_inst_init(struct msm_vidc_inst *inst)
 	inst->buffers.output_meta.extra_count = 0;
 	inst->buffers.output_meta.actual_count = 0;
 	inst->buffers.output_meta.size = 0;
-	inst->subcr_params[INPUT_PORT].dpb_list_enabled = 0;
-	inst->subcr_params[OUTPUT_PORT].dpb_list_enabled = 0;
+	inst->input_dpb_list_enabled = false;
+	inst->output_dpb_list_enabled = false;
 
 	rc = msm_vdec_codec_change(inst,
 			inst->fmts[INPUT_PORT].fmt.pix_mp.pixelformat);

driver/vidc/src/venus_hfi_response.c

@@ -1669,6 +1669,9 @@ static int handle_dpb_list_property(struct msm_vidc_inst *inst,
 	u32 payload_size, num_words_in_payload;
 	u8 *payload_start;
 	int i = 0;
+	struct msm_vidc_buffer *ro_buf;
+	bool found = false;
+	u64 device_addr;
 
 	payload_size = pkt->size - sizeof(struct hfi_packet);
 	num_words_in_payload = payload_size / 4;
@@ -1684,6 +1687,12 @@ static int handle_dpb_list_property(struct msm_vidc_inst *inst,
 	}
 	memcpy(inst->dpb_list_payload, payload_start, payload_size);
 
+	/*
+	 * dpb_list_payload details:
+	 * payload[0-1]           : 64 bits base_address of DPB-1
+	 * payload[2]             : 32 bits addr_offset  of DPB-1
+	 * payload[3]             : 32 bits data_offset  of DPB-1
+	 */
 	for (i = 0; (i + 3) < num_words_in_payload; i = i + 4) {
 		i_vpr_l(inst,
 			"%s: base addr %#x %#x, addr offset %#x, data offset %#x\n",
@@ -1691,6 +1700,32 @@ static int handle_dpb_list_property(struct msm_vidc_inst *inst,
 			inst->dpb_list_payload[i + 2], inst->dpb_list_payload[i + 3]);
 	}
 
+	list_for_each_entry(ro_buf, &inst->buffers.read_only.list, list) {
+		found = false;
+		/* do not mark RELEASE_ELIGIBLE for non-read only buffers */
+		if (!(ro_buf->attr & MSM_VIDC_ATTR_READ_ONLY))
+			continue;
+		/* no need to mark RELEASE_ELIGIBLE again */
+		if (ro_buf->attr & MSM_VIDC_ATTR_RELEASE_ELIGIBLE)
+			continue;
+		/*
+		 * do not add RELEASE_ELIGIBLE to buffers for which driver
+		 * sent release cmd already
+		 */
+		if (ro_buf->attr & MSM_VIDC_ATTR_PENDING_RELEASE)
+			continue;
+		for (i = 0; (i + 3) < num_words_in_payload; i = i + 4) {
+			device_addr = *((u64 *)(&inst->dpb_list_payload[i]));
+			if (ro_buf->device_addr == device_addr) {
+				found = true;
+				break;
+			}
+		}
+		/* mark a buffer as RELEASE_ELIGIBLE if not found in dpb list */
+		if (!found)
+			ro_buf->attr |= MSM_VIDC_ATTR_RELEASE_ELIGIBLE;
+	}
+
 	return 0;
 }