Merge "video: driver: Add support to debugfs"

driver/vidc/src/msm_vidc.c

@@ -781,9 +781,6 @@ void *msm_vidc_open(void *vidc_core, u32 session_type)
 	for (i = 0; i < MAX_SIGNAL; i++)
 		init_completion(&inst->completions[i]);
 
-	//inst->debugfs_root =
-	//	msm_vidc_debugfs_init_inst(inst, core->debugfs_root);
-
 	if (is_decode_session(inst))
 		rc = msm_vdec_inst_init(inst);
 	else if (is_encode_session(inst))
@@ -805,6 +802,11 @@ void *msm_vidc_open(void *vidc_core, u32 session_type)
 	if (rc)
 		goto error;
 
+	inst->debugfs_root =
+		msm_vidc_debugfs_init_inst(inst, core->debugfs_root);
+	if (!inst->debugfs_root)
+		s_vpr_h(inst->sid, "%s: debugfs not available\n", __func__);
+
 	return inst;
 
 error:

driver/vidc/src/msm_vidc_debug.c

@@ -4,6 +4,15 @@
  */
 
 #include "msm_vidc_debug.h"
+#include "msm_vidc_driver.h"
+#include "msm_vidc_dt.h"
+#include "msm_vidc.h"
+#include "msm_vidc_core.h"
+#include "msm_vidc_inst.h"
+#include "msm_vidc_internal.h"
+
+#define MAX_SSR_STRING_LEN         10
+#define MAX_DEBUG_LEVEL_STRING_LEN 15
 
 int msm_vidc_debug = VIDC_HIGH | VIDC_PKT | VIDC_ERR | VIDC_PRINTK |
 	FW_ERROR | FW_FATAL;
@@ -35,3 +44,522 @@ const char *level_str(u32 level)
 		return "????";
 }
 
+#define MAX_DBG_BUF_SIZE 4096
+
+struct core_inst_pair {
+	struct msm_vidc_core *core;
+	struct msm_vidc_inst *inst;
+};
+
+/* debug fs support */
+
+static inline void tic(struct msm_vidc_inst *i, enum profiling_points p,
+				 char *b)
+{
+	struct timeval __ddl_tv;
+
+	if (!i->debug.pdata[p].name[0])
+		memcpy(i->debug.pdata[p].name, b, 64);
+	if ((msm_vidc_debug & VIDC_PERF) &&
+		i->debug.pdata[p].sampling) {
+		do_gettimeofday(&__ddl_tv);
+		i->debug.pdata[p].start =
+			(__ddl_tv.tv_sec * 1000) + (__ddl_tv.tv_usec / 1000);
+			i->debug.pdata[p].sampling = false;
+	}
+}
+
+static inline void toc(struct msm_vidc_inst *i, enum profiling_points p)
+{
+	struct timeval __ddl_tv;
+
+	if ((msm_vidc_debug & VIDC_PERF) &&
+		!i->debug.pdata[p].sampling) {
+		do_gettimeofday(&__ddl_tv);
+		i->debug.pdata[p].stop = (__ddl_tv.tv_sec * 1000)
+			+ (__ddl_tv.tv_usec / 1000);
+		i->debug.pdata[p].cumulative += i->debug.pdata[p].stop -
+			i->debug.pdata[p].start;
+		i->debug.pdata[p].sampling = true;
+	}
+}
+
+static u32 write_str(char *buffer,
+		size_t size, const char *fmt, ...)
+{
+	va_list args;
+	u32 len;
+
+	va_start(args, fmt);
+	len = vscnprintf(buffer, size, fmt, args);
+	va_end(args);
+	return len;
+}
+
+static ssize_t core_info_read(struct file* file, char __user* buf,
+	size_t count, loff_t* ppos)
+{
+	struct msm_vidc_core *core = file->private_data;
+	char* dbuf, * cur, * end;
+	ssize_t len = 0;
+
+	if (!core || !core->dt) {
+		d_vpr_e("%s: invalid params %pK\n", __func__, core);
+		return 0;
+	}
+
+	dbuf = kzalloc(MAX_DBG_BUF_SIZE, GFP_KERNEL);
+	if (!dbuf) {
+		d_vpr_e("%s: Allocation failed!\n", __func__);
+		return -ENOMEM;
+	}
+	cur = dbuf;
+	end = cur + MAX_DBG_BUF_SIZE;
+
+	cur += write_str(cur, end - cur, "Core state: %d\n", core->state);
+
+	cur += write_str(cur, end - cur,
+		"FW version : %s\n", core->fw_version);
+	cur += write_str(cur, end - cur,
+		"register_base: 0x%x\n", core->dt->register_base);
+	cur += write_str(cur, end - cur,
+		"register_size: %u\n", core->dt->register_size);
+	cur += write_str(cur, end - cur, "irq: %u\n", core->dt->irq);
+
+	len = simple_read_from_buffer(buf, count, ppos,
+		dbuf, cur - dbuf);
+
+	kfree(dbuf);
+	return len;
+}
+
+static const struct file_operations core_info_fops = {
+	.open = simple_open,
+	.read = core_info_read,
+};
+
+static ssize_t trigger_ssr_write(struct file* filp, const char __user* buf,
+	size_t count, loff_t* ppos)
+{
+	unsigned long ssr_trigger_val = 0;
+	int rc = 0;
+	struct msm_vidc_core* core = filp->private_data;
+	size_t size = MAX_SSR_STRING_LEN;
+	char kbuf[MAX_SSR_STRING_LEN + 1] = { 0 };
+
+	if (!buf)
+		return -EINVAL;
+
+	if (!count)
+		goto exit;
+
+	if (count < size)
+		size = count;
+
+	if (copy_from_user(kbuf, buf, size)) {
+		d_vpr_e("%s: User memory fault\n", __func__);
+		rc = -EFAULT;
+		goto exit;
+	}
+
+	rc = kstrtoul(kbuf, 0, &ssr_trigger_val);
+	if (rc) {
+		d_vpr_e("returning error err %d\n", rc);
+		rc = -EINVAL;
+	}
+	else {
+		msm_vidc_trigger_ssr(core, ssr_trigger_val);
+		rc = count;
+	}
+exit:
+	return rc;
+}
+
+static const struct file_operations ssr_fops = {
+	.open = simple_open,
+	.write = trigger_ssr_write,
+};
+
+static ssize_t debug_level_write(struct file* filp, const char __user* buf,
+	size_t count, loff_t* ppos)
+{
+	int rc = 0;
+	struct msm_vidc_core* core = filp->private_data;
+	char kbuf[MAX_DEBUG_LEVEL_STRING_LEN] = { 0 };
+
+	/* filter partial writes and invalid commands */
+	if (*ppos != 0 || count >= sizeof(kbuf) || count == 0) {
+		d_vpr_e("returning error - pos %d, count %d\n", *ppos, count);
+		rc = -EINVAL;
+	}
+
+	rc = simple_write_to_buffer(kbuf, sizeof(kbuf) - 1, ppos, buf, count);
+	if (rc < 0) {
+		d_vpr_e("%s: User memory fault\n", __func__);
+		rc = -EFAULT;
+		goto exit;
+	}
+
+	rc = kstrtoint(kbuf, 0, &msm_vidc_debug);
+	if (rc) {
+		d_vpr_e("returning error err %d\n", rc);
+		rc = -EINVAL;
+		goto exit;
+	}
+	core->capabilities[HW_RESPONSE_TIMEOUT].value =
+		((msm_vidc_debug & 0xFF) >
+		(VIDC_ERR | VIDC_HIGH)) ? 1500 : 1000;
+	rc = count;
+	d_vpr_h("debug timeout updated to - %d ms\n",
+		core->capabilities[HW_RESPONSE_TIMEOUT].value);
+
+exit:
+	return rc;
+}
+
+static ssize_t debug_level_read(struct file* file, char __user* buf,
+	size_t count, loff_t* ppos)
+{
+	size_t len;
+	char kbuf[MAX_DEBUG_LEVEL_STRING_LEN];
+
+	len = scnprintf(kbuf, sizeof(kbuf), "0x%08x\n", msm_vidc_debug);
+	return simple_read_from_buffer(buf, count, ppos, kbuf, len);
+}
+
+static const struct file_operations debug_level_fops = {
+	.open = simple_open,
+	.write = debug_level_write,
+	.read = debug_level_read,
+};
+
+struct dentry* msm_vidc_debugfs_init_drv()
+{
+	bool ok = false;
+	struct dentry *dir = NULL;
+
+	dir = debugfs_create_dir("msm_vidc", NULL);
+	if (IS_ERR_OR_NULL(dir)) {
+		dir = NULL;
+		goto failed_create_dir;
+	}
+
+#define __debugfs_create(__type, __name, __value) ({                \
+	struct dentry *f = debugfs_create_##__type(__name, 0644,    \
+		dir, __value);                                      \
+	if (IS_ERR_OR_NULL(f)) {                                    \
+		d_vpr_e("Failed creating debugfs file '%pd/%s'\n",  \
+			dir, __name);                               \
+		f = NULL;                                           \
+	}                                                           \
+	f;                                                          \
+})
+
+	ok =
+	__debugfs_create(u32, "core_clock_voting",
+			&msm_vidc_clock_voting) &&
+	__debugfs_create(bool, "disable_video_syscache",
+			&msm_vidc_syscache_disable) &&
+	__debugfs_create(bool, "lossless_encoding",
+			&msm_vidc_lossless_encode);
+
+#undef __debugfs_create
+
+	if (!ok)
+		goto failed_create_dir;
+
+	return dir;
+
+failed_create_dir:
+	if (dir)
+		debugfs_remove_recursive(dir);
+
+	return NULL;
+}
+
+struct dentry *msm_vidc_debugfs_init_core(void *core_in)
+{
+	struct dentry *dir = NULL;
+	char debugfs_name[MAX_DEBUGFS_NAME];
+	struct msm_vidc_core *core = (struct msm_vidc_core *) core_in;
+	struct dentry *parent;
+
+	if (!core || !core->debugfs_parent) {
+		d_vpr_e("%s: invalid params\n", __func__);
+		goto failed_create_dir;
+	}
+	parent = core->debugfs_parent;
+
+	snprintf(debugfs_name, MAX_DEBUGFS_NAME, "core");
+	dir = debugfs_create_dir(debugfs_name, parent);
+	if (IS_ERR_OR_NULL(dir)) {
+		dir = NULL;
+		d_vpr_e("Failed to create debugfs for msm_vidc\n");
+		goto failed_create_dir;
+	}
+	if (!debugfs_create_file("info", 0444, dir, core, &core_info_fops)) {
+		d_vpr_e("debugfs_create_file: fail\n");
+		goto failed_create_dir;
+	}
+	if (!debugfs_create_file("trigger_ssr", 0200,
+			dir, core, &ssr_fops)) {
+		d_vpr_e("debugfs_create_file: fail\n");
+		goto failed_create_dir;
+	}
+	if (!debugfs_create_file("debug_level", 0644,
+			parent, core, &debug_level_fops)) {
+		d_vpr_e("debugfs_create_file: fail\n");
+		goto failed_create_dir;
+	}
+failed_create_dir:
+	return dir;
+}
+
+static int inst_info_open(struct inode *inode, struct file *file)
+{
+	d_vpr_l("Open inode ptr: %pK\n", inode->i_private);
+	file->private_data = inode->i_private;
+	return 0;
+}
+
+static int publish_unreleased_reference(struct msm_vidc_inst *inst,
+		char **dbuf, char *end)
+{
+	return 0;
+}
+
+static ssize_t inst_info_read(struct file *file, char __user *buf,
+		size_t count, loff_t *ppos)
+{
+	struct core_inst_pair *idata = file->private_data;
+	struct msm_vidc_core *core;
+	struct msm_vidc_inst *inst;
+	char *dbuf, *cur, *end;
+	int i, j;
+	ssize_t len = 0;
+	struct v4l2_format *f;
+
+	if (!idata || !idata->core || !idata->inst ||
+		!idata->inst->capabilities) {
+		d_vpr_e("%s: invalid params %pK\n", __func__, idata);
+		return 0;
+	}
+
+	core = idata->core;
+	inst = idata->inst;
+
+	inst = get_inst(core, inst->session_id);
+	if (!inst) {
+		d_vpr_h("%s: instance has become obsolete", __func__);
+		return 0;
+	}
+
+	dbuf = kzalloc(MAX_DBG_BUF_SIZE, GFP_KERNEL);
+	if (!dbuf) {
+		s_vpr_e(inst->sid, "%s: Allocation failed!\n", __func__);
+		len = -ENOMEM;
+		goto failed_alloc;
+	}
+	cur = dbuf;
+	end = cur + MAX_DBG_BUF_SIZE;
+
+	f = &inst->fmts[OUTPUT_PORT];
+	cur += write_str(cur, end - cur, "==============================\n");
+	cur += write_str(cur, end - cur, "INSTANCE: %pK (%s)\n", inst,
+		inst->domain == MSM_VIDC_ENCODER ? "Encoder" : "Decoder");
+	cur += write_str(cur, end - cur, "==============================\n");
+	cur += write_str(cur, end - cur, "core: %pK\n", inst->core);
+	cur += write_str(cur, end - cur, "height: %d\n", f->fmt.pix_mp.height);
+	cur += write_str(cur, end - cur, "width: %d\n", f->fmt.pix_mp.width);
+	cur += write_str(cur, end - cur, "fps: %d\n",
+			inst->capabilities->cap[FRAME_RATE].value >> 16);
+	cur += write_str(cur, end - cur, "state: %d\n", inst->state);
+	cur += write_str(cur, end - cur, "secure: %d\n",
+		!!(inst->flags & VIDC_SECURE));
+	cur += write_str(cur, end - cur, "-----------Formats-------------\n");
+	for (i = 0; i < MAX_PORT; i++) {
+		if (i != INPUT_PORT && i != OUTPUT_PORT)
+			continue;
+		f = &inst->fmts[i];
+		cur += write_str(cur, end - cur, "capability: %s\n",
+			i == INPUT_PORT ? "Output" : "Capture");
+		cur += write_str(cur, end - cur, "planes : %d\n",
+			f->fmt.pix_mp.num_planes);
+		cur += write_str(cur, end - cur,
+			"type: %s\n", i == INPUT_PORT ?
+			"Output" : "Capture");
+		cur += write_str(cur, end - cur, "count: %u\n",
+				inst->vb2q[i].num_buffers);
+
+		for (j = 0; j < f->fmt.pix_mp.num_planes; j++)
+			cur += write_str(cur, end - cur,
+				"size for plane %d: %u\n",
+				j, f->fmt.pix_mp.plane_fmt[j].sizeimage);
+
+		cur += write_str(cur, end - cur, "\n");
+	}
+	cur += write_str(cur, end - cur, "-------------------------------\n");
+	cur += write_str(cur, end - cur, "ETB Count: %d\n",
+		inst->debug_count.etb);
+	cur += write_str(cur, end - cur, "EBD Count: %d\n",
+		inst->debug_count.ebd);
+	cur += write_str(cur, end - cur, "FTB Count: %d\n",
+		inst->debug_count.ftb);
+	cur += write_str(cur, end - cur, "FBD Count: %d\n",
+		inst->debug_count.fbd);
+
+	publish_unreleased_reference(inst, &cur, end);
+	len = simple_read_from_buffer(buf, count, ppos,
+		dbuf, cur - dbuf);
+
+	kfree(dbuf);
+failed_alloc:
+	put_inst(inst);
+	return len;
+}
+
+static int inst_info_release(struct inode *inode, struct file *file)
+{
+	d_vpr_l("Release inode ptr: %pK\n", inode->i_private);
+	file->private_data = NULL;
+	return 0;
+}
+
+static const struct file_operations inst_info_fops = {
+	.open = inst_info_open,
+	.read = inst_info_read,
+	.release = inst_info_release,
+};
+
+struct dentry *msm_vidc_debugfs_init_inst(void *instance, struct dentry *parent)
+{
+	struct dentry *dir = NULL, *info = NULL;
+	char debugfs_name[MAX_DEBUGFS_NAME];
+	struct core_inst_pair *idata = NULL;
+	struct msm_vidc_inst *inst = (struct msm_vidc_inst *) instance;
+
+	if (!inst) {
+		d_vpr_e("%s: invalid params\n", __func__);
+		goto exit;
+	}
+	snprintf(debugfs_name, MAX_DEBUGFS_NAME, "inst_%d", inst->sid);
+
+	idata = kzalloc(sizeof(struct core_inst_pair), GFP_KERNEL);
+	if (!idata) {
+		s_vpr_e(inst->sid, "%s: Allocation failed!\n", __func__);
+		goto exit;
+	}
+
+	idata->core = inst->core;
+	idata->inst = inst;
+
+	dir = debugfs_create_dir(debugfs_name, parent);
+	if (IS_ERR_OR_NULL(dir)) {
+		dir = NULL;
+		s_vpr_e(inst->sid,
+			"%s: Failed to create debugfs for msm_vidc\n",
+			__func__);
+		goto failed_create_dir;
+	}
+
+	info = debugfs_create_file("info", 0444, dir,
+			idata, &inst_info_fops);
+	if (IS_ERR_OR_NULL(info)) {
+		s_vpr_e(inst->sid, "%s: debugfs_create_file: fail\n",
+			__func__);
+		goto failed_create_file;
+	}
+
+	dir->d_inode->i_private = info->d_inode->i_private;
+	inst->debug.pdata[FRAME_PROCESSING].sampling = true;
+	return dir;
+
+failed_create_file:
+	debugfs_remove_recursive(dir);
+	dir = NULL;
+failed_create_dir:
+	kfree(idata);
+exit:
+	return dir;
+}
+
+void msm_vidc_debugfs_deinit_inst(void *instance)
+{
+	struct dentry *dentry = NULL;
+	struct msm_vidc_inst *inst = (struct msm_vidc_inst *) instance;
+
+	if (!inst || !inst->debugfs_root)
+		return;
+
+	dentry = inst->debugfs_root;
+	if (dentry->d_inode) {
+		s_vpr_l(inst->sid, "%s: Destroy %pK\n",
+			__func__, dentry->d_inode->i_private);
+		kfree(dentry->d_inode->i_private);
+		dentry->d_inode->i_private = NULL;
+	}
+	debugfs_remove_recursive(dentry);
+	inst->debugfs_root = NULL;
+}
+
+void msm_vidc_debugfs_update(void *instance,
+	enum msm_vidc_debugfs_event e)
+{
+	struct msm_vidc_inst *inst = (struct msm_vidc_inst *) instance;
+	struct msm_vidc_debug *d;
+	char a[64] = "Frame processing";
+
+	if (!inst) {
+		d_vpr_e("%s: invalid params\n", __func__);
+		return;
+	}
+	d = &inst->debug;
+
+	switch (e) {
+	case MSM_VIDC_DEBUGFS_EVENT_ETB:
+		inst->debug_count.etb++;
+		if (inst->debug_count.ebd &&
+			inst->debug_count.ftb > inst->debug_count.fbd) {
+			d->pdata[FRAME_PROCESSING].name[0] = '\0';
+			tic(inst, FRAME_PROCESSING, a);
+		}
+		break;
+	case MSM_VIDC_DEBUGFS_EVENT_EBD:
+		inst->debug_count.ebd++;
+		if (inst->debug_count.ebd &&
+			inst->debug_count.ebd == inst->debug_count.etb) {
+			toc(inst, FRAME_PROCESSING);
+			s_vpr_p(inst->sid, "EBD: FW needs input buffers\n");
+		}
+		if (inst->debug_count.ftb == inst->debug_count.fbd)
+			s_vpr_p(inst->sid, "EBD: FW needs output buffers\n");
+		break;
+	case MSM_VIDC_DEBUGFS_EVENT_FTB:
+		inst->debug_count.ftb++;
+		if (inst->debug_count.ebd &&
+			inst->debug_count.etb > inst->debug_count.ebd) {
+			d->pdata[FRAME_PROCESSING].name[0] = '\0';
+			tic(inst, FRAME_PROCESSING, a);
+		}
+		break;
+	case MSM_VIDC_DEBUGFS_EVENT_FBD:
+		inst->debug_count.fbd++;
+		inst->debug.samples++;
+		if (inst->debug_count.fbd &&
+			inst->debug_count.fbd == inst->debug_count.ftb) {
+			toc(inst, FRAME_PROCESSING);
+			s_vpr_p(inst->sid, "FBD: FW needs output buffers\n");
+		}
+		if (inst->debug_count.etb == inst->debug_count.ebd)
+			s_vpr_p(inst->sid, "FBD: FW needs input buffers\n");
+		break;
+	default:
+		s_vpr_e(inst->sid, "invalid event in debugfs: %d\n", e);
+		break;
+	}
+}
+
+void do_gettimeofday(struct timeval *__ddl_tv)
+{
+}
+

driver/vidc/src/msm_vidc_driver.c

@@ -1505,6 +1505,11 @@ int msm_vidc_queue_buffer(struct msm_vidc_inst *inst, struct vb2_buffer *vb2)
 		meta->attr |= MSM_VIDC_ATTR_QUEUED;
 	}
 
+	if (buf->type == MSM_VIDC_BUF_INPUT)
+		msm_vidc_debugfs_update(inst, MSM_VIDC_DEBUGFS_EVENT_ETB);
+	else if (buf->type == MSM_VIDC_BUF_OUTPUT)
+		msm_vidc_debugfs_update(inst, MSM_VIDC_DEBUGFS_EVENT_FTB);
+
 	return rc;
 }
 
@@ -2731,6 +2736,7 @@ static void msm_vidc_close_helper(struct kref *kref)
 	s_vpr_h(inst->sid, "%s()\n", __func__);
 	msm_vidc_event_queue_deinit(inst);
 	msm_vidc_vb2_queue_deinit(inst);
+	msm_vidc_debugfs_deinit_inst(inst);
 	if (is_decode_session(inst))
 		msm_vdec_inst_deinit(inst);
 	else if (is_encode_session(inst))

driver/vidc/src/msm_vidc_probe.c

@@ -243,6 +243,10 @@ static int msm_vidc_probe_video_device(struct platform_device *pdev)
 		return -ENOMEM;
 	g_core = core;
 
+	core->debugfs_parent = msm_vidc_debugfs_init_drv();
+	if (!core->debugfs_parent)
+		d_vpr_h("Failed to create debugfs for msm_vidc\n");
+
 	core->pdev = pdev;
 	dev_set_drvdata(&pdev->dev, core);
 
@@ -294,7 +298,9 @@ static int msm_vidc_probe_video_device(struct platform_device *pdev)
 		goto exit;
 	}
 
-	//rc = msm_vidc_debugfs_init_core(core);
+	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");
 	/*
@@ -311,6 +317,9 @@ static int msm_vidc_probe_video_device(struct platform_device *pdev)
 	}
 
 exit:
+	if (rc)
+		debugfs_remove_recursive(core->debugfs_parent);
+
 	return rc;
 }
 
@@ -371,6 +380,7 @@ static int msm_vidc_remove(struct platform_device *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;

driver/vidc/src/venus_hfi_response.c

@@ -427,6 +427,7 @@ static int handle_input_buffer(struct msm_vidc_inst *inst,
 	}*/
 
 	print_vidc_buffer(VIDC_HIGH, "EBD", inst, buf);
+	msm_vidc_debugfs_update(inst, MSM_VIDC_DEBUGFS_EVENT_EBD);
 
 	return rc;
 }
@@ -482,6 +483,7 @@ static int handle_output_buffer(struct msm_vidc_inst *inst,
 	if (buffer->flags & HFI_BUF_FW_FLAG_LAST)
 		buf->flags |= MSM_VIDC_BUF_FLAG_LAST;
 	print_vidc_buffer(VIDC_HIGH, "FBD", inst, buf);
+	msm_vidc_debugfs_update(inst, MSM_VIDC_DEBUGFS_EVENT_FBD);
 
 	return rc;
 }
@@ -1004,16 +1006,15 @@ static int handle_session_property(struct msm_vidc_inst *inst,
 	return 0;
 }
 
-static int handle_image_version_property(struct hfi_packet *pkt)
+static int handle_image_version_property(struct msm_vidc_core *core,
+	struct hfi_packet *pkt)
 {
 	u32 i = 0;
-	char version[256];
-	const u32 version_string_size = 128;
 	u8 *str_image_version;
 	u32 req_bytes;
 
 	req_bytes = pkt->size - sizeof(*pkt);
-	if (req_bytes < version_string_size) {
+	if (req_bytes < VENUS_VERSION_LENGTH - 1) {
 		d_vpr_e("%s: bad_pkt: %d\n", __func__, req_bytes);
 		return -EINVAL;
 	}
@@ -1023,14 +1024,15 @@ static int handle_image_version_property(struct hfi_packet *pkt)
 	 * characters at the start and in between. Replace the null
 	 * characters with space, to print the version info.
 	 */
-	for (i = 0; i < version_string_size; i++) {
+	for (i = 0; i < VENUS_VERSION_LENGTH - 1; i++) {
 		if (str_image_version[i] != '\0')
-			version[i] = str_image_version[i];
+			core->fw_version[i] = str_image_version[i];
 		else
-			version[i] = ' ';
+			core->fw_version[i] = ' ';
 	}
-	version[i] = '\0';
-	d_vpr_h("%s: F/W version: %s\n", __func__, version);
+	core->fw_version[i] = '\0';
+
+	d_vpr_h("%s: F/W version: %s\n", __func__, core->fw_version);
 	return 0;
 }
 
@@ -1047,7 +1049,7 @@ static int handle_system_property(struct msm_vidc_core *core,
 
 	switch (pkt->type) {
 	case HFI_PROP_IMAGE_VERSION:
-		rc = handle_image_version_property(pkt);
+		rc = handle_image_version_property(core, pkt);
 		break;
 	default:
 		d_vpr_h("%s: property type %#x successful\n",