Re: [PATCH v11 2/6] media: chips-media: wave5: Add vpuapi layer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 07/12/22 13:13, Sebastian Fricke ha scritto:
From: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>

Add the vpuapi layer of the wave5 codec driver.
This layer is used to configure the hardware according
to the parameters.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
Signed-off-by: Robert Beckett <bob.beckett@xxxxxxxxxxxxx>
Signed-off-by: Sebastian Fricke <sebastian.fricke@xxxxxxxxxxxxx>
Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
---
  .../platform/chips-media/wave5/wave5-hw.c     | 3359 +++++++++++++++++
  .../chips-media/wave5/wave5-regdefine.h       |  743 ++++
  .../platform/chips-media/wave5/wave5-vdi.c    |  245 ++
  .../platform/chips-media/wave5/wave5-vdi.h    |   67 +
  .../platform/chips-media/wave5/wave5-vpuapi.c | 1040 +++++
  .../platform/chips-media/wave5/wave5-vpuapi.h | 1136 ++++++
  .../chips-media/wave5/wave5-vpuconfig.h       |   90 +
  .../chips-media/wave5/wave5-vpuerror.h        |  454 +++
  .../media/platform/chips-media/wave5/wave5.h  |   94 +
  9 files changed, 7228 insertions(+)
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-hw.c
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-regdefine.h
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-vdi.c
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-vdi.h
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5-vpuerror.h
  create mode 100644 drivers/media/platform/chips-media/wave5/wave5.h

diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
new file mode 100644
index 000000000000..25705e61cdb3
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
@@ -0,0 +1,3359 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+/*
+ * Wave5 series multi-standard codec IP - wave5 backend logic
+ *
+ * Copyright (C) 2021 CHIPS&MEDIA INC
+ */
+
+#include <linux/iopoll.h>
+#include "wave5-vpu.h"
+#include "wave5.h"
+#include "wave5-regdefine.h"
+
+#define FIO_TIMEOUT			10000000

FIO_TIMEOUT_US looks better :-)

+#define FIO_CTRL_READY			BIT(31)
+#define FIO_CTRL_WRITE			BIT(16)
+#define VPU_BUSY_CHECK_TIMEOUT		10000000
+#define QUEUE_REPORT_MASK		0xffff
+
+static void wave5_print_reg_err(struct vpu_device *vpu_dev, u32 reg_fail_reason)
+{
+	char *caller = __builtin_return_address(0);
+	struct device *dev = vpu_dev->dev;
+	u32 reg_val;
+
+	switch (reg_fail_reason) {
+	case WAVE5_SYSERR_QUEUEING_FAIL:
+		reg_val = vpu_read_reg(vpu_dev, W5_RET_QUEUE_FAIL_REASON);
+		dev_dbg(dev, "%s: queueing failure: 0x%x\n", caller, reg_val);
+		break;
+	case WAVE5_SYSERR_RESULT_NOT_READY:
+		dev_err(dev, "%s: result not ready: 0x%x\n", caller, reg_fail_reason);
+		break;
+	case WAVE5_SYSERR_ACCESS_VIOLATION_HW:
+		dev_err(dev, "%s: access violation: 0x%x\n", caller, reg_fail_reason);
+		break;
+	case WAVE5_SYSERR_WATCHDOG_TIMEOUT:
+		dev_err(dev, "%s: watchdog timeout: 0x%x\n", caller, reg_fail_reason);
+		break;
+	case WAVE5_SYSERR_BUS_ERROR:
+		dev_err(dev, "%s: bus error: 0x%x\n", caller, reg_fail_reason);
+		break;
+	case WAVE5_SYSERR_DOUBLE_FAULT:
+		dev_err(dev, "%s: double fault: 0x%x\n", caller, reg_fail_reason);
+		break;
+	case WAVE5_SYSERR_VPU_STILL_RUNNING:
+		dev_err(dev, "%s: still running: 0x%x\n", caller, reg_fail_reason);
+		break;
+	case WAVE5_SYSERR_VLC_BUF_FULL:
+		dev_err(dev, "%s: vlc buf full: 0x%x\n", caller, reg_fail_reason);
+		break;
+	default:
+		dev_err(dev, "%s: failure:: 0x%x\n", caller, reg_fail_reason);
+		break;
+	}
+}
+
+static int wave5_wait_fio_readl(struct vpu_device *vpu_dev, u32 addr, u32 val)
+{
+	u32 ctrl;
+	int ret;
+
+	ctrl = addr & 0xffff;
+	wave5_vdi_write_register(vpu_dev, W5_VPU_FIO_CTRL_ADDR, ctrl);
+	ret = read_poll_timeout(wave5_vdi_readl, ctrl, ctrl & FIO_CTRL_READY,
+				0, FIO_TIMEOUT, false, vpu_dev, W5_VPU_FIO_CTRL_ADDR);
+	if (ret)
+		return ret;
+	if (wave5_vdi_readl(vpu_dev, W5_VPU_FIO_DATA) != val)
+		return -ETIMEDOUT;

Are you sure that this is a timeout?
if (read_data != expected_data) => invalid data => return -EINVAL ?

+	return 0;
+}
+

..snip..

+
+static int wave5_wait_bus_busy(struct vpu_device *vpu_dev, unsigned int addr)
+{
+	u32 gdi_status_check_value = 0x3f;
+
+	if (vpu_dev->product_code == WAVE521C_CODE ||
+	    vpu_dev->product_code == WAVE521_CODE ||
+	    vpu_dev->product_code == WAVE521E1_CODE)
+		gdi_status_check_value = 0x00ff1f3f;

#define SOME_VALUE 0x3f
#define ANOTHER_VALUE 0xff1f3f

+
+	return wave5_wait_fio_readl(vpu_dev, addr, gdi_status_check_value);
+}
+

..snip..

+
+static int setup_wave5_properties(struct device *dev)
+{
+	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
+	struct vpu_attr *p_attr = &vpu_dev->attr;
+	u32 reg_val;
+	u8 *str;
+	int ret;
+	u32 hw_config_def0, hw_config_def1, hw_config_feature, hw_config_rev;
+
+	vpu_write_reg(vpu_dev, W5_QUERY_OPTION, GET_VPU_INFO);
+	vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
+	vpu_write_reg(vpu_dev, W5_COMMAND, W5_QUERY);
+	vpu_write_reg(vpu_dev, W5_VPU_HOST_INT_REQ, 1);
+	ret = wave5_wait_vpu_busy(vpu_dev, W5_VPU_BUSY_STATUS);
+	if (ret)
+		return ret;
+
+	if (!vpu_read_reg(vpu_dev, W5_RET_SUCCESS))
+		return -EIO;
+
+	reg_val = vpu_read_reg(vpu_dev, W5_RET_PRODUCT_NAME);
+	str = (u8 *)&reg_val;
+	p_attr->product_name[0] = str[3];
+	p_attr->product_name[1] = str[2];
+	p_attr->product_name[2] = str[1];
+	p_attr->product_name[3] = str[0];
+	p_attr->product_name[4] = 0;
+
+	p_attr->product_id = wave5_vpu_get_product_id(vpu_dev);
+	p_attr->product_version = vpu_read_reg(vpu_dev, W5_RET_PRODUCT_VERSION);
+	p_attr->fw_version = vpu_read_reg(vpu_dev, W5_RET_FW_VERSION);
+	p_attr->customer_id = vpu_read_reg(vpu_dev, W5_RET_CUSTOMER_ID);
+	hw_config_def0 = vpu_read_reg(vpu_dev, W5_RET_STD_DEF0);
+	hw_config_def1 = vpu_read_reg(vpu_dev, W5_RET_STD_DEF1);
+	hw_config_feature = vpu_read_reg(vpu_dev, W5_RET_CONF_FEATURE);
+	hw_config_rev = vpu_read_reg(vpu_dev, W5_RET_CONF_REVISION);
+
+	p_attr->support_hevc10bit_enc = (hw_config_feature >> 3) & 1;

This looks like being BIT(3), and the latter is BIT(11)...

#define W5_CONF_FEATURE_HEVC10_ENC	BIT(3)
#define W5_CONF_FEATURE_AVC10_ENC	BIT(11)

p_attr->support_hevc10bit_enc = FIELD_GET(W5_CONF_FEATURE_HEVC10_ENC, hw_config_feature);

if (hw_config_rev > W5_REVISION_SOMETHING)
	p_attr->support_avc10bit_enc = FIELD_GET(W5_CONF_FEATURE_AVC10_ENC,
						 hw_config_feature);
else
	p_attr->support_avc10bit_enc = p_attr->support_hevc10bit_enc;


+	if (hw_config_rev > 167455) //20190321
+		p_attr->support_avc10bit_enc = (hw_config_feature >> 11) & 1;
+	else
+		p_attr->support_avc10bit_enc = p_attr->support_hevc10bit_enc;
+
+	p_attr->support_decoders = 0;
+	p_attr->support_encoders = 0;
+	if (p_attr->product_id == PRODUCT_ID_521) {
+		p_attr->support_dual_core = ((hw_config_def1 >> 26) & 0x01);

p_attr->support_dual_core = FIELD_GET(W5_CONF_DEF_HW_DUAL_CORE, hw_config_def1);

....and there are others below, but I think I gave enough examples... :-)

+		if (p_attr->support_dual_core || hw_config_rev < 206116) {
+			p_attr->support_decoders = BIT(STD_AVC);
+			p_attr->support_decoders |= BIT(STD_HEVC);
+			p_attr->support_encoders = BIT(STD_AVC);
+			p_attr->support_encoders |= BIT(STD_HEVC);
+		} else {
+			p_attr->support_decoders |= (((hw_config_def1 >> 3) & 0x01) << STD_AVC);
+			p_attr->support_decoders |= (((hw_config_def1 >> 2) & 0x01) << STD_HEVC);
+			p_attr->support_encoders = (((hw_config_def1 >> 1) & 0x01) << STD_AVC);
+			p_attr->support_encoders |= ((hw_config_def1 & 0x01) << STD_HEVC);
+		}
+	} else if (p_attr->product_id == PRODUCT_ID_511) {
+		p_attr->support_decoders = BIT(STD_HEVC);
+		p_attr->support_decoders |= BIT(STD_AVC);
+	} else if (p_attr->product_id == PRODUCT_ID_517) {
+		p_attr->support_decoders = (((hw_config_def1 >> 4) & 0x01) << STD_AV1);
+		p_attr->support_decoders |= (((hw_config_def1 >> 3) & 0x01) << STD_AVS2);
+		p_attr->support_decoders |= (((hw_config_def1 >> 2) & 0x01) << STD_AVC);
+		p_attr->support_decoders |= (((hw_config_def1 >> 1) & 0x01) << STD_VP9);
+		p_attr->support_decoders |= ((hw_config_def1 & 0x01) << STD_HEVC);
+	}
+
+	p_attr->support_backbone = (hw_config_def0 >> 16) & 0x01;
+	p_attr->support_vcpu_backbone = (hw_config_def0 >> 28) & 0x01;
+	p_attr->support_vcore_backbone = (hw_config_def0 >> 22) & 0x01;
+	p_attr->support_dual_core = (hw_config_def1 >> 26) & 0x01;
+	p_attr->support_endian_mask = BIT(VDI_LITTLE_ENDIAN) |
+				      BIT(VDI_BIG_ENDIAN) |
+				      BIT(VDI_32BIT_LITTLE_ENDIAN) |
+				      BIT(VDI_32BIT_BIG_ENDIAN) |
+				      (0xffffUL << 16);
+	p_attr->support_bitstream_mode = BIT(BS_MODE_INTERRUPT) |
+		BIT(BS_MODE_PIC_END);
+
+	return 0;
+}
+
+int wave5_vpu_get_version(struct vpu_device *vpu_dev, u32 *revision)
+{
+	u32 reg_val;
+	int ret;
+
+	vpu_write_reg(vpu_dev, W5_QUERY_OPTION, GET_VPU_INFO);
+	vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
+	vpu_write_reg(vpu_dev, W5_COMMAND, W5_QUERY);
+	vpu_write_reg(vpu_dev, W5_VPU_HOST_INT_REQ, 1);
+	ret = wave5_wait_vpu_busy(vpu_dev, W5_VPU_BUSY_STATUS);
+	if (ret) {
+		dev_err(vpu_dev->dev, "%s: timeout\n", __func__);
+		return ret;
+	}
+
+	if (!vpu_read_reg(vpu_dev, W5_RET_SUCCESS)) {
+		dev_err(vpu_dev->dev, "%s: failed\n", __func__);
+		return -EIO;
+	}
+
+	reg_val = vpu_read_reg(vpu_dev, W5_RET_FW_VERSION);
+	if (revision)

Move the revision pointer null check at the beginning and return an error
if that happens to be null: it doesn't make a lot of sense to read many
registers before the check as the whole point of this function is to get
the version and return it to that variable.


+		*revision = reg_val;
+
+	return 0;
+}
+
+static void remap_page(struct vpu_device *vpu_dev, dma_addr_t code_base, u32 index)
+{
+	u32 remap_size = (W5_REMAP_MAX_SIZE >> 12) & 0x1ff;
+	u32 reg_val = 0x80000000 | (WAVE5_UPPER_PROC_AXI_ID << 20) | (index << 12) | BIT(11)
+		| remap_size;
+
+	vpu_write_reg(vpu_dev, W5_VPU_REMAP_CTRL, reg_val);
+	vpu_write_reg(vpu_dev, W5_VPU_REMAP_VADDR, index * W5_REMAP_MAX_SIZE);
+	vpu_write_reg(vpu_dev, W5_VPU_REMAP_PADDR, code_base + index * W5_REMAP_MAX_SIZE);
+}
+

..snip..

+
+int wave5_vpu_build_up_dec_param(struct vpu_instance *inst,
+				 struct dec_open_param *param)
+{
+	int ret;
+	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
+	u32 bs_endian;
+	struct dma_vpu_buf *sram_vb;
+	struct vpu_device *vpu_dev = inst->dev;
+
+	p_dec_info->cycle_per_tick = 256;
+	switch (inst->std) {
+	case W_HEVC_DEC:
+		p_dec_info->seq_change_mask = SEQ_CHANGE_ENABLE_ALL_HEVC;
+		break;
+	case W_VP9_DEC:
+		p_dec_info->seq_change_mask = SEQ_CHANGE_ENABLE_ALL_VP9;
+		break;
+	case W_AVS2_DEC:
+		p_dec_info->seq_change_mask = SEQ_CHANGE_ENABLE_ALL_AVS2;
+		break;
+	case W_AVC_DEC:
+		p_dec_info->seq_change_mask = SEQ_CHANGE_ENABLE_ALL_AVC;
+		break;
+	case W_AV1_DEC:
+		p_dec_info->seq_change_mask = SEQ_CHANGE_ENABLE_ALL_AV1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (vpu_dev->product == PRODUCT_ID_517)

Another switch would be good here.

+		p_dec_info->vb_work.size = WAVE517_WORKBUF_SIZE;
+	else if (vpu_dev->product == PRODUCT_ID_521)
+		p_dec_info->vb_work.size = WAVE521DEC_WORKBUF_SIZE;
+	else if (vpu_dev->product == PRODUCT_ID_511)
+		p_dec_info->vb_work.size = WAVE521DEC_WORKBUF_SIZE;
+
+	ret = wave5_vdi_allocate_dma_memory(inst->dev, &p_dec_info->vb_work);
+	if (ret)
+		return ret;
+
+	vpu_write_reg(inst->dev, W5_CMD_DEC_VCORE_INFO, 1);
+
+	sram_vb = &vpu_dev->sram_buf;
+	p_dec_info->sec_axi_info.buf_base = sram_vb->daddr;
+	p_dec_info->sec_axi_info.buf_size = sram_vb->size;
+
+	wave5_vdi_clear_memory(inst->dev, &p_dec_info->vb_work);
+
+	vpu_write_reg(inst->dev, W5_ADDR_WORK_BASE, p_dec_info->vb_work.daddr);
+	vpu_write_reg(inst->dev, W5_WORK_SIZE, p_dec_info->vb_work.size);
+
+	vpu_write_reg(inst->dev, W5_CMD_DEC_BS_START_ADDR, p_dec_info->stream_buf_start_addr);
+	vpu_write_reg(inst->dev, W5_CMD_DEC_BS_SIZE, p_dec_info->stream_buf_size);
+
+	/* NOTE: when endian mode is 0, SDMA reads MSB first */
+	bs_endian = wave5_vdi_convert_endian(inst->dev, param->stream_endian);
+	bs_endian = (~bs_endian & VDI_128BIT_ENDIAN_MASK);
+	vpu_write_reg(inst->dev, W5_CMD_BS_PARAM, bs_endian);
+	vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, (param->pri_axprot << 20) |
+			(param->pri_axcache << 16) | param->pri_ext_addr);
+	vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1, (COMMAND_QUEUE_DEPTH - 1));
+	vpu_write_reg(inst->dev, W5_CMD_ERR_CONCEAL, (param->error_conceal_unit << 2) |
+			(param->error_conceal_mode));
+
+	wave5_bit_issue_command(inst, W5_CREATE_INSTANCE);
+	// check QUEUE_DONE

Please be consistent in comments format. Use C-style comments.

+	ret = wave5_wait_vpu_busy(inst->dev, W5_VPU_BUSY_STATUS);
+	if (ret) {
+		dev_warn(inst->dev->dev, "command: 'W5_CREATE_INSTANCE' timed out\n");
+		goto free_vb_work;
+	}
+
+	// Check if we were able to add the parameters into the VCPU QUEUE
+	if (!vpu_read_reg(inst->dev, W5_RET_SUCCESS)) {
+		ret = -EIO;
+		goto free_vb_work;
+	}
+
+	p_dec_info->product_code = vpu_read_reg(inst->dev, W5_PRODUCT_NUMBER);
+
+	return 0;
+free_vb_work:
+	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_work);
+	return ret;
+}
+

..snip..

+
+static void wave5_get_dec_seq_result(struct vpu_instance *inst, struct dec_initial_info *info)
+{
+	u32 reg_val, sub_layer_info;
+	u32 profile_compatibility_flag;
+	u32 output_bit_depth_minus8;
+	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
+
+	p_dec_info->stream_rd_ptr = wave5_vpu_dec_get_rd_ptr(inst);
+	info->rd_ptr = p_dec_info->stream_rd_ptr;
+
+	p_dec_info->frame_display_flag = vpu_read_reg(inst->dev, W5_RET_DEC_DISP_IDC);
+
+	reg_val = vpu_read_reg(inst->dev, W5_RET_DEC_PIC_SIZE);
+	info->pic_width = ((reg_val >> 16) & 0xffff);
+	info->pic_height = (reg_val & 0xffff);
+	info->min_frame_buffer_count = vpu_read_reg(inst->dev, W5_RET_DEC_NUM_REQUIRED_FB);
+	info->frame_buf_delay = vpu_read_reg(inst->dev, W5_RET_DEC_NUM_REORDER_DELAY);
+
+	reg_val = vpu_read_reg(inst->dev, W5_RET_DEC_CROP_LEFT_RIGHT);
+	info->pic_crop_rect.left = (reg_val >> 16) & 0xffff;
+	info->pic_crop_rect.right = reg_val & 0xffff;
+	reg_val = vpu_read_reg(inst->dev, W5_RET_DEC_CROP_TOP_BOTTOM);
+	info->pic_crop_rect.top = (reg_val >> 16) & 0xffff;
+	info->pic_crop_rect.bottom = reg_val & 0xffff;
+
+	info->f_rate_numerator = vpu_read_reg(inst->dev, W5_RET_DEC_FRAME_RATE_NR);
+	info->f_rate_denominator = vpu_read_reg(inst->dev, W5_RET_DEC_FRAME_RATE_DR);
+
+	reg_val = vpu_read_reg(inst->dev, W5_RET_DEC_COLOR_SAMPLE_INFO);
+	info->luma_bitdepth = reg_val & 0xf;
+	info->chroma_bitdepth = (reg_val >> 4) & 0xf;
+	info->chroma_format_idc = (reg_val >> 8) & 0xf;
+	info->aspect_rate_info = (reg_val >> 16) & 0xff;

Bitfield macros would make this way more readable.

+	info->is_ext_sar = ((info->aspect_rate_info == 255) ? true : false);
+	/* [0:15] - vertical size, [16:31] - horizontal size */
+	if (info->is_ext_sar)
+		info->aspect_rate_info = vpu_read_reg(inst->dev, W5_RET_DEC_ASPECT_RATIO);
+	info->bit_rate = vpu_read_reg(inst->dev, W5_RET_DEC_BIT_RATE);
+
+	sub_layer_info = vpu_read_reg(inst->dev, W5_RET_DEC_SUB_LAYER_INFO);
+	info->max_temporal_layers = (sub_layer_info >> 8) & 0x7;
+
+	reg_val = vpu_read_reg(inst->dev, W5_RET_DEC_SEQ_PARAM);
+	info->level = reg_val & 0xff;
+	profile_compatibility_flag = (reg_val >> 12) & 0xff;
+	info->profile = (reg_val >> 24) & 0x1f;
+	info->tier = (reg_val >> 29) & 0x01;
+	output_bit_depth_minus8 = (reg_val >> 30) & 0x03;
+
+	if (inst->std == W_HEVC_DEC) {
+		/* guessing profile */
+		if (!info->profile) {
+			if ((profile_compatibility_flag & 0x06) == 0x06)
+				info->profile = HEVC_PROFILE_MAIN; /* main profile */

main/main10 profile comments are stating the obvious, please remove them.

+			else if ((profile_compatibility_flag & 0x04) == 0x04)
+				info->profile = HEVC_PROFILE_MAIN10; /* main10 profile */
+			else if ((profile_compatibility_flag & 0x08) == 0x08)
+				/* main still picture profile */
+				info->profile = HEVC_PROFILE_STILLPICTURE;
+			else
+				info->profile = HEVC_PROFILE_MAIN; /* for old version HM */
+		}
+
+	} else if (inst->std == W_AVS2_DEC) {
+		if (info->luma_bitdepth == 10 && output_bit_depth_minus8 == 2)
+			info->output_bit_depth = 10;
+		else
+			info->output_bit_depth = 8;
+
+	} else if (inst->std == W_AVC_DEC) {
+		info->profile = (reg_val >> 24) & 0x7f;
+	}
+
+	info->vlc_buf_size = vpu_read_reg(inst->dev, W5_RET_VLC_BUF_SIZE);
+	info->param_buf_size = vpu_read_reg(inst->dev, W5_RET_PARAM_BUF_SIZE);
+	p_dec_info->vlc_buf_size = info->vlc_buf_size;
+	p_dec_info->param_buf_size = info->param_buf_size;
+}
+

..snip..

+
+static u32 calculate_table_size(u32 bit_depth, u32 frame_width, u32 frame_height, u32 ot_bg_width)
+{
+	u32 bgs_width = ((bit_depth > 8) ? 256 : 512);
+	u32 comp_frame_width = ALIGN(ALIGN(frame_width, 16) + 16, 16);
+	u32 ot_frame_width = ALIGN(comp_frame_width, ot_bg_width);
+
+	// sizeof_offset_table()
+	u32 ot_bg_height = 32;
+	u32 bgs_height = BIT(14) / bgs_width / ((bit_depth > 8) ? 2 : 1);

Please, no magic BIT(x) usage: add a definition for that bit.

+	u32 comp_frame_height = ALIGN(ALIGN(frame_height, 4) + 4, bgs_height);
+	u32 ot_frame_height = ALIGN(comp_frame_height, ot_bg_height);
+
+	return (ot_frame_width / 16) * (ot_frame_height / 4) * 2;
+}
+

..snip..

+
+int wave5_vpu_decode(struct vpu_instance *inst, struct dec_param *option, u32 *fail_res)
+{
+	u32 mode_option = DEC_PIC_NORMAL, bs_option, reg_val;
+	u32 force_latency = 0;
+	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
+	struct dec_open_param *p_open_param = &p_dec_info->open_param;
+	int ret;
+

switch (option->skipframe_mode) {
case ...
...
default:
	break;
};

if (p_dec_info->thumbnail_mode) {
	mode_option = DEC_PIC_W_THUMBNAIL;
	if (mode_option != DEC_PIC_NORMAL)
		... do something: as I read the code, this is not a supported case.
}

^^^^ this should improve the flow.

+	if (p_dec_info->thumbnail_mode) {
+		mode_option = DEC_PIC_W_THUMBNAIL;
+	} else if (option->skipframe_mode) {
+		switch (option->skipframe_mode) {
+		case WAVE_SKIPMODE_NON_IRAP:
+			mode_option = SKIP_NON_IRAP;
+			force_latency = 1;
+			break;
+		case WAVE_SKIPMODE_NON_REF:
+			mode_option = SKIP_NON_REF_PIC;
+			break;
+		default:
+			// skip mode off
+			break;
+		}
+	}
+
+	// set disable reorder
+	if (!p_dec_info->reorder_enable)
+		force_latency = 1;
+
+	/* set attributes of bitstream buffer controller */
+	bs_option = 0;

You don't have to initialize this variable at all, as you're either writing twice
or failing.

+	switch (p_open_param->bitstream_mode) {
+	case BS_MODE_INTERRUPT:
+		bs_option = BSOPTION_ENABLE_EXPLICIT_END;
+		break;
+	case BS_MODE_PIC_END:
+		bs_option = BSOPTION_ENABLE_EXPLICIT_END;
+		break;
+	default:
+		return -EINVAL;
+	}
+

..snip..

+
+int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
+{
+	struct vpu_buf *common_vb;
+	dma_addr_t code_base, temp_base;
+	dma_addr_t old_code_base, temp_size;
+	u32 code_size;
+	u32 reg_val;
+	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
+
+	common_vb = &vpu_dev->common_mem;
+
+	code_base = common_vb->daddr;
+	/* ALIGN TO 4KB */
+	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
+	if (code_size < size * 2)
+		return -EINVAL;
+	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
+	temp_size = WAVE5_TEMPBUF_SIZE;
+
+	old_code_base = vpu_read_reg(vpu_dev, W5_VPU_REMAP_PADDR);
+
+	if (old_code_base != code_base + W5_REMAP_INDEX1 * W5_REMAP_MAX_SIZE) {

Put the contents of this branch into another function maybe?

	if (old_code_base != code_base + W5_REMAP_INDEX1 * W5_REMAP_MAX_SIZE) {
		ret = do_that_vpu_init_flow(things);
		if (ret)
			return ret;
	}


	return setup_wave5_properties(dev);
};

Alternatively, invert the conditional and use a goto, but I personally don't
really like using gotos unless it's *totally* necessary.

+		int ret;
+		struct dma_vpu_buf *sram_vb;
+
+		ret = wave5_vdi_write_memory(vpu_dev, common_vb, 0, fw, size,
+					     VDI_128BIT_LITTLE_ENDIAN);
+		if (ret < 0) {
+			dev_err(vpu_dev->dev,
+				"VPU init, Writing firmware to common buffer, fail: %d\n", ret);
+			return ret;
+		}
+
+		vpu_write_reg(vpu_dev, W5_PO_CONF, 0);
+
+		ret = wave5_vpu_reset(dev, SW_RESET_ON_BOOT);
+		if (ret < 0) {
+			dev_err(vpu_dev->dev, "VPU init, Resetting the VPU, fail: %d\n", ret);
+			return ret;
+		}
+
+		remap_page(vpu_dev, code_base, W5_REMAP_INDEX0);
+		remap_page(vpu_dev, code_base, W5_REMAP_INDEX1);
+
+		vpu_write_reg(vpu_dev, W5_ADDR_CODE_BASE, code_base);
+		vpu_write_reg(vpu_dev, W5_CODE_SIZE, code_size);
+		vpu_write_reg(vpu_dev, W5_CODE_PARAM, (WAVE5_UPPER_PROC_AXI_ID << 4) | 0);
+		vpu_write_reg(vpu_dev, W5_ADDR_TEMP_BASE, temp_base);
+		vpu_write_reg(vpu_dev, W5_TEMP_SIZE, temp_size);
+
+		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
+
+		reg_val = (WAVE5_PROC_AXI_EXT_ADDR & 0xFFFF);
+		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, reg_val);
+		reg_val = ((WAVE5_PROC_AXI_AXPROT & 0x7) << 4) |
+			(WAVE5_PROC_AXI_AXCACHE & 0xF);
+		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, reg_val);
+		reg_val = ((WAVE5_SEC_AXI_AXPROT & 0x7) << 20) |
+			((WAVE5_SEC_AXI_AXCACHE & 0xF) << 16) |
+			(WAVE5_SEC_AXI_EXT_ADDR & 0xFFFF);
+		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, reg_val);
+
+		/* interrupt */
+		// encoder
+		reg_val = BIT(INT_WAVE5_ENC_SET_PARAM);
+		reg_val |= BIT(INT_WAVE5_ENC_PIC);
+		reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
+		// decoder
+		reg_val |= BIT(INT_WAVE5_INIT_SEQ);
+		reg_val |= BIT(INT_WAVE5_DEC_PIC);
+		reg_val |= BIT(INT_WAVE5_BSBUF_EMPTY);
+		vpu_write_reg(vpu_dev, W5_VPU_VINT_ENABLE, reg_val);
+
+		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
+		if ((reg_val >> 16) & 1) {
+			reg_val = ((WAVE5_PROC_AXI_ID << 28) |
+					(WAVE5_PRP_AXI_ID << 24) |
+					(WAVE5_FBD_Y_AXI_ID << 20) |
+					(WAVE5_FBC_Y_AXI_ID << 16) |
+					(WAVE5_FBD_C_AXI_ID << 12) |
+					(WAVE5_FBC_C_AXI_ID << 8) |
+					(WAVE5_PRI_AXI_ID << 4) |
+					WAVE5_SEC_AXI_ID);
+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
+		}
+
+		sram_vb = &vpu_dev->sram_buf;
+
+		vpu_write_reg(vpu_dev, W5_ADDR_SEC_AXI, sram_vb->daddr);
+		vpu_write_reg(vpu_dev, W5_SEC_AXI_SIZE, sram_vb->size);
+		vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
+		vpu_write_reg(vpu_dev, W5_COMMAND, W5_INIT_VPU);
+		vpu_write_reg(vpu_dev, W5_VPU_REMAP_CORE_START, 1);
+
+		ret = wave5_wait_vpu_busy(vpu_dev, W5_VPU_BUSY_STATUS);
+		if (ret) {
+			dev_err(vpu_dev->dev, "VPU reinit(W5_VPU_REMAP_CORE_START) timeout\n");
+			return ret;
+		}
+
+		reg_val = vpu_read_reg(vpu_dev, W5_RET_SUCCESS);
+		if (!reg_val) {
+			u32 reason_code = vpu_read_reg(vpu_dev, W5_RET_FAIL_REASON);
+
+			wave5_print_reg_err(vpu_dev, reason_code);
+			return -EIO;
+		}
+	}
+
+	return setup_wave5_properties(dev);
+}
+

..snip..

+
+int wave5_vpu_reset(struct device *dev, enum sw_reset_mode reset_mode)
+{
+	u32 val = 0;
+	int ret = 0;
+	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
+	struct vpu_attr *p_attr = &vpu_dev->attr;
+	// VPU doesn't send response. force to set BUSY flag to 0.
+	vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 0);
+
+	if (reset_mode == SW_RESET_SAFETY) {
+		ret = wave5_vpu_sleep_wake(dev, true, NULL, 0);
+		if (ret)
+			return ret;
+	}
+
+	val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
+	if ((val >> 16) & 0x1)
+		p_attr->support_backbone = true;

bitfield macros ftw.

+	if ((val >> 22) & 0x1)
+		p_attr->support_vcore_backbone = true;
+	if ((val >> 28) & 0x1)
+		p_attr->support_vcpu_backbone = true;
+
+	val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG1);
+	if ((val >> 26) & 0x1)
+		p_attr->support_dual_core = true;
+


..snip..

+static void wave5_set_enc_crop_info(u32 codec, struct enc_wave_param *param, int rot_mode,
+				    int src_width, int src_height)
+{
+	int aligned_width = (codec == W_HEVC_ENC) ? ALIGN(src_width, 32) : ALIGN(src_width, 16);
+	int aligned_height = (codec == W_HEVC_ENC) ? ALIGN(src_height, 32) : ALIGN(src_height, 16);
+	int pad_right, pad_bot;
+	int crop_right, crop_left, crop_top, crop_bot;
+	int prp_mode = rot_mode >> 1; // remove prp_enable bit
+
+	if (codec == W_HEVC_ENC &&
+	    (!rot_mode || prp_mode == 14)) // prp_mode 14 : hor_mir && ver_mir && rot_180
+		return;
+
+	pad_right = aligned_width - src_width;
+	pad_bot = aligned_height - src_height;
+
+	if (param->conf_win_right > 0)
+		crop_right = param->conf_win_right + pad_right;
+	else
+		crop_right = pad_right;
+
+	if (param->conf_win_bot > 0)
+		crop_bot = param->conf_win_bot + pad_bot;
+	else
+		crop_bot = pad_bot;
+
+	crop_top = param->conf_win_top;
+	crop_left = param->conf_win_left;
+
+	param->conf_win_top = crop_top;
+	param->conf_win_left = crop_left;
+	param->conf_win_bot = crop_bot;
+	param->conf_win_right = crop_right;
+
+	if (prp_mode == 1 || prp_mode == 15) {

#define PRP_MODE_SOMETHING 1
#define PRP_MODE_SOMETHING_ELSE 2

or use an enumeration... otherwise it's not really understandable...

+		param->conf_win_top = crop_right;
+		param->conf_win_left = crop_top;
+		param->conf_win_bot = crop_left;
+		param->conf_win_right = crop_bot;
+	} else if (prp_mode == 2 || prp_mode == 12) {
+		param->conf_win_top = crop_bot;
+		param->conf_win_left = crop_right;
+		param->conf_win_bot = crop_top;
+		param->conf_win_right = crop_left;
+	} else if (prp_mode == 3 || prp_mode == 13) {
+		param->conf_win_top = crop_left;
+		param->conf_win_left = crop_bot;
+		param->conf_win_bot = crop_right;
+		param->conf_win_right = crop_top;
+	} else if (prp_mode == 4 || prp_mode == 10) {
+		param->conf_win_top = crop_bot;
+		param->conf_win_bot = crop_top;
+	} else if (prp_mode == 8 || prp_mode == 6) {
+		param->conf_win_left = crop_right;
+		param->conf_win_right = crop_left;
+	} else if (prp_mode == 5 || prp_mode == 11) {
+		param->conf_win_top = crop_left;
+		param->conf_win_left = crop_top;
+		param->conf_win_bot = crop_right;
+		param->conf_win_right = crop_bot;
+	} else if (prp_mode == 7 || prp_mode == 9) {
+		param->conf_win_top = crop_right;
+		param->conf_win_left = crop_bot;
+		param->conf_win_bot = crop_left;
+		param->conf_win_right = crop_top;
+	}
+}
+
+int wave5_vpu_enc_init_seq(struct vpu_instance *inst)
+{
+	u32 reg_val = 0, rot_mir_mode, fixed_cu_size_mode = 0x7;
+	struct enc_info *p_enc_info = &inst->codec_info->enc_info;
+	struct enc_open_param *p_open_param = &p_enc_info->open_param;
+	struct enc_wave_param *p_param = &p_open_param->wave_param;
+	int ret;
+
+	if (inst->dev->product != PRODUCT_ID_521)
+		return -EINVAL;
+
+	/*==============================================*/
+	/* OPT_CUSTOM_GOP */
+	/*==============================================*/

Comments like these are usually like

	/*
	 * OPT_CUSTOM_GOP
	 *
	 * SET_PARAM + CUSTOM_GOP
	 * only when... blah
	 */

+	/*
+	 * SET_PARAM + CUSTOM_GOP
+	 * only when gop_preset_idx == custom_gop, custom_gop related registers should be set
+	 */

..snip..

+}
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
new file mode 100644
index 000000000000..1b3ffb737925
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
@@ -0,0 +1,1136 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Wave5 series multi-standard codec IP - helper definitions
+ *
+ * Copyright (C) 2021 CHIPS&MEDIA INC
+ */
+
+#ifndef VPUAPI_H_INCLUDED
+#define VPUAPI_H_INCLUDED
+
+#include <linux/kfifo.h>
+#include <linux/idr.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-mem2mem.h>
+#include <media/v4l2-ctrls.h>
+#include "wave5-vpuerror.h"
+#include "wave5-vpuconfig.h"
+#include "wave5-vdi.h"
+
+enum product_id {
+	PRODUCT_ID_521,
+	PRODUCT_ID_511,
+	PRODUCT_ID_517,
+	PRODUCT_ID_NONE,
+};
+
+struct vpu_attr;
+
+enum vpu_instance_type {
+	VPU_INST_TYPE_DEC = 0,

The default for the first enum entry is always zero, and the next one will always
be 1, 2, 3, 4.....

.....so you don't need to assign any number.

+	VPU_INST_TYPE_ENC = 1
+};
+
+enum vpu_instance_state {
+	VPU_INST_STATE_NONE = 0,
+	VPU_INST_STATE_OPEN = 1,
+	VPU_INST_STATE_INIT_SEQ = 2,
+	VPU_INST_STATE_PIC_RUN = 3,
+	VPU_INST_STATE_STOP = 4

ditto

+};
+
+#define WAVE5_MAX_FBS 32
+
+#define MAX_REG_FRAME (WAVE5_MAX_FBS * 2)
+
+#define WAVE5_DEC_HEVC_BUF_SIZE(_w, _h) (DIV_ROUND_UP(_w, 64) * DIV_ROUND_UP(_h, 64) * 256 + 64)
+#define WAVE5_DEC_AVC_BUF_SIZE(_w, _h) ((((ALIGN(_w, 256) / 16) * (ALIGN(_h, 16) / 16)) + 16) * 80)
+#define WAVE5_DEC_VP9_BUF_SIZE(_w, _h) (((ALIGN(_w, 64) * ALIGN(_h, 64)) >> 2))
+#define WAVE5_DEC_AVS2_BUF_SIZE(_w, _h) (((ALIGN(_w, 64) * ALIGN(_h, 64)) >> 5))
+// AV1 BUF SIZE : MFMV + segment ID + CDF probs table + film grain param Y+ film graim param C
+#define WAVE5_DEC_AV1_BUF_SZ_1(_w, _h)	\
+	(((ALIGN(_w, 64) / 64) * (ALIGN(_h, 64) / 64) * 512) + 41984 + 8192 + 4864)
+#define WAVE5_DEC_AV1_BUF_SZ_2(_w1, _w2, _h)	\
+	(((ALIGN(_w1, 64) / 64) * 256 + (ALIGN(_w2, 256) / 64) * 128) * (ALIGN(_h, 64) / 64))
+
+#define WAVE5_FBC_LUMA_TABLE_SIZE(_w, _h) (ALIGN(_h, 64) * ALIGN(_w, 256) / 32)
+#define WAVE5_FBC_CHROMA_TABLE_SIZE(_w, _h) (ALIGN((_h), 64) * ALIGN((_w) / 2, 256) / 32)
+#define WAVE5_ENC_AVC_BUF_SIZE(_w, _h) (ALIGN(_w, 64) * ALIGN(_h, 64) / 32)
+#define WAVE5_ENC_HEVC_BUF_SIZE(_w, _h) (ALIGN(_w, 64) / 64 * ALIGN(_h, 64) / 64 * 128)
+
+/*
+ * common struct and definition
+ */
+enum cod_std {
+	STD_AVC = 0,
+	STD_VC1 = 1,
+	STD_MPEG2 = 2,
+	STD_MPEG4 = 3,
+	STD_H263 = 4,
+	STD_DIV3 = 5,
+	STD_RV = 6,
+	STD_AVS = 7,

and same here, so that becomes

	.....
	STD_AVS,
	STD_RESERVED,
	STD_THO,


+	STD_THO = 9 > +	STD_VP3 = 10,
+	STD_VP8 = 11,
+	STD_HEVC = 12,
+	STD_VP9 = 13,
+	STD_AVS2 = 14,

STD_RESERVED2, (which will be 15)...

+	STD_AV1 = 16,
+	STD_MAX
+};
+
+enum wave_std {
+	W_HEVC_DEC = 0x00,
+	W_HEVC_ENC = 0x01,
+	W_AVC_DEC = 0x02,
+	W_AVC_ENC = 0x03,
+	W_VP9_DEC = 0x16,
+	W_AVS2_DEC = 0x18,
+	W_AV1_DEC = 0x1A,
+	STD_UNKNOWN = 0xFF
+};
+
+enum SET_PARAM_OPTION {

Lowercase names for enums please.

+	OPT_COMMON = 0, /* SET_PARAM command option for encoding sequence */
+	OPT_CUSTOM_GOP = 1, /* SET_PARAM command option for setting custom GOP */
+	OPT_CUSTOM_HEADER = 2, /* SET_PARAM command option for setting custom VPS/SPS/PPS */
+	OPT_VUI = 3, /* SET_PARAM command option for encoding VUI */
+	OPT_CHANGE_PARAM = 0x10,
+};
+
+enum DEC_PIC_HDR_OPTION {
+	INIT_SEQ_NORMAL = 0x01,
+	INIT_SEQ_W_THUMBNAIL = 0x11,
+};
+
+enum DEC_PIC_OPTION {
+	DEC_PIC_NORMAL = 0x00, /* it is normal mode of DEC_PIC command */
+	DEC_PIC_W_THUMBNAIL = 0x10, /* thumbnail mode (skip non-IRAP without reference reg) */
+	SKIP_NON_IRAP = 0x11, /* it skips to decode non-IRAP pictures */
+	SKIP_NON_REF_PIC = 0x13
+};
+


There's probably more, but starting with that is surely something :-)


Regards,
Angelo




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux