[PATCH v2 0/5] Wave515 decoder IP support

[PATCH v2 0/5] Wave515 decoder IP support

[Ivan Bornyakov]

Initial support for Wave515 multi-decoder IP among other refinements.
This was tested on FPGA prototype, so wave5_dt_ids[] was not expanded.

ChangeLog:
  v1:
https://lore.kernel.org/linux-media/20240318144225.30835-1-brnkv.i1@gmail.com/
  v2:
    * drop patch "dt-bindings: media: cnm,wave521c: drop resets restriction"
      The only user of Wave5 in mainline is TI K3 boards, thus there is
      no real need to alter dt-bindings
    * in patch "media: chips-media: wave5: support decoding HEVC Main10 profile"
      add check for flag "support_hevc10bit_dec"
    * in patch "media: chips-media: wave5: support reset lines" move
      reset_control_deassert() out of else branch, add
      reset_control_assert() to probe error path.
    * rework patch "media: chips-media: wave5: drop "sram-size" DT prop"
       - don't move alloc/free form device open/close
       - intead of exact configuration of reserved SRAM memory in DT and
	 allocating all of it, allocate all available SRAM memory up to
	 WAVE5_MAX_SRAM_SIZE from whatever pool provided.
    * adjust patch "media: chips-media: wave5: support Wave515 decoder"
      according to changes in patches
      "media: chips-media: wave5: support decoding HEVC Main10 profile" and
      "media: chips-media: wave5: drop "sram-size" DT prop"

Ivan Bornyakov (5):
  media: chips-media: wave5: support decoding HEVC Main10 profile
  media: chips-media: wave5: support reset lines
  media: chips-media: wave5: separate irq setup routine
  media: chips-media: wave5: drop "sram-size" DT prop
  media: chips-media: wave5: support Wave515 decoder

 .../platform/chips-media/wave5/wave5-helper.c |   3 +-
 .../platform/chips-media/wave5/wave5-hw.c     | 296 +++++++++++++-----
 .../chips-media/wave5/wave5-regdefine.h       |   5 +
 .../platform/chips-media/wave5/wave5-vdi.c    |  27 +-
 .../chips-media/wave5/wave5-vpu-dec.c         |  44 ++-
 .../platform/chips-media/wave5/wave5-vpu.c    |  31 +-
 .../platform/chips-media/wave5/wave5-vpuapi.h |   4 +-
 .../chips-media/wave5/wave5-vpuconfig.h       |  11 +-
 .../media/platform/chips-media/wave5/wave5.h  |   1 +
 9 files changed, 303 insertions(+), 119 deletions(-)

-- 
2.44.0

[PATCH v2 1/5] media: chips-media: wave5: support decoding HEVC Main10 profile

[Ivan Bornyakov]

Add support for decoding HEVC Main10 profile by scaling FBC buffer
stride and size by the factor of (bitdepth / 8).

Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
---
 .../chips-media/wave5/wave5-vpu-dec.c         | 30 +++++++++++--------
 .../platform/chips-media/wave5/wave5-vpuapi.h |  1 +
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index ef227af72348..5a71a711f2e8 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -1055,6 +1055,22 @@ static int wave5_prepare_fb(struct vpu_instance *inst)
 	int ret, i;
 	struct v4l2_m2m_buffer *buf, *n;
 	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
+	u32 bitdepth = inst->codec_info->dec_info.initial_info.luma_bitdepth;
+
+	switch (bitdepth) {
+	case 8:
+		break;
+	case 10:
+		if (inst->std == W_HEVC_DEC &&
+		    inst->dev->attr.support_hevc10bit_dec)
+			break;
+
+		fallthrough;
+	default:
+		dev_err(inst->dev->dev, "no support for %d bit depth\n", bitdepth);
+
+		return -EINVAL;
+	}
 
 	linear_num = v4l2_m2m_num_dst_bufs_ready(m2m_ctx);
 	non_linear_num = inst->fbc_buf_count;
@@ -1063,7 +1079,7 @@ static int wave5_prepare_fb(struct vpu_instance *inst)
 		struct frame_buffer *frame = &inst->frame_buf[i];
 		struct vpu_buf *vframe = &inst->frame_vbuf[i];
 
-		fb_stride = inst->dst_fmt.width;
+		fb_stride = ALIGN(inst->dst_fmt.width * bitdepth / 8, 32);
 		fb_height = ALIGN(inst->dst_fmt.height, 32);
 		luma_size = fb_stride * fb_height;
 
@@ -1408,22 +1424,10 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
 		if (ret)
 			goto free_bitstream_vbuf;
 	} else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
-		struct dec_initial_info *initial_info =
-			&inst->codec_info->dec_info.initial_info;
-
 		if (inst->state == VPU_INST_STATE_STOP)
 			ret = switch_state(inst, VPU_INST_STATE_INIT_SEQ);
 		if (ret)
 			goto return_buffers;
-
-		if (inst->state == VPU_INST_STATE_INIT_SEQ) {
-			if (initial_info->luma_bitdepth != 8) {
-				dev_info(inst->dev->dev, "%s: no support for %d bit depth",
-					 __func__, initial_info->luma_bitdepth);
-				ret = -EINVAL;
-				goto return_buffers;
-			}
-		}
 	}
 
 	return ret;
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
index 352f6e904e50..465ff9dfe8b1 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
@@ -327,6 +327,7 @@ struct vpu_attr {
 	u32 support_backbone: 1;
 	u32 support_avc10bit_enc: 1;
 	u32 support_hevc10bit_enc: 1;
+	u32 support_hevc10bit_dec: 1;
 	u32 support_vcore_backbone: 1;
 	u32 support_vcpu_backbone: 1;
 };
-- 
2.44.0

[PATCH v2 2/5] media: chips-media: wave5: support reset lines

[Ivan Bornyakov]

Add initial support for optional reset lines. For now, simply deassert
resets on driver probe and assert them back on driver remove.

Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
---
 .../media/platform/chips-media/wave5/wave5-vpu.c | 16 +++++++++++++++-
 .../platform/chips-media/wave5/wave5-vpuapi.h    |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 1b3df5b04249..1e631da58e15 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/firmware.h>
 #include <linux/interrupt.h>
+#include <linux/reset.h>
 #include "wave5-vpu.h"
 #include "wave5-regdefine.h"
 #include "wave5-vpuconfig.h"
@@ -151,6 +152,16 @@ static int wave5_vpu_probe(struct platform_device *pdev)
 	dev_set_drvdata(&pdev->dev, dev);
 	dev->dev = &pdev->dev;
 
+	dev->resets = devm_reset_control_array_get_optional_exclusive(&pdev->dev);
+	if (IS_ERR(dev->resets)) {
+		return dev_err_probe(&pdev->dev, PTR_ERR(dev->resets),
+				     "Failed to get reset control\n");
+	}
+
+	ret = reset_control_deassert(dev->resets);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to deassert resets\n");
+
 	ret = devm_clk_bulk_get_all(&pdev->dev, &dev->clks);
 
 	/* continue without clock, assume externally managed */
@@ -163,7 +174,7 @@ static int wave5_vpu_probe(struct platform_device *pdev)
 	ret = clk_bulk_prepare_enable(dev->num_clks, dev->clks);
 	if (ret) {
 		dev_err(&pdev->dev, "Enabling clocks, fail: %d\n", ret);
-		return ret;
+		goto err_reset_assert;
 	}
 
 	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
@@ -246,6 +257,8 @@ static int wave5_vpu_probe(struct platform_device *pdev)
 	wave5_vdi_release(&pdev->dev);
 err_clk_dis:
 	clk_bulk_disable_unprepare(dev->num_clks, dev->clks);
+err_reset_assert:
+	reset_control_assert(dev->resets);
 
 	return ret;
 }
@@ -256,6 +269,7 @@ static void wave5_vpu_remove(struct platform_device *pdev)
 
 	mutex_destroy(&dev->dev_lock);
 	mutex_destroy(&dev->hw_lock);
+	reset_control_assert(dev->resets);
 	clk_bulk_disable_unprepare(dev->num_clks, dev->clks);
 	wave5_vpu_enc_unregister_device(dev);
 	wave5_vpu_dec_unregister_device(dev);
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
index 465ff9dfe8b1..da530fd98964 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
@@ -758,6 +758,7 @@ struct vpu_device {
 	struct ida inst_ida;
 	struct clk_bulk_data *clks;
 	int num_clks;
+	struct reset_control *resets;
 };
 
 struct vpu_instance;
-- 
2.44.0

Re: [PATCH v2 2/5] media: chips-media: wave5: support reset lines

[Sebastian Fricke]

Hey Ivan,

On 25.03.2024 09:40, Ivan Bornyakov wrote:
>Add initial support for optional reset lines. For now, simply deassert
>resets on driver probe and assert them back on driver remove.

LGTM.

Greetings,
Sebastian
>
>Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>---
> .../media/platform/chips-media/wave5/wave5-vpu.c | 16 +++++++++++++++-
> .../platform/chips-media/wave5/wave5-vpuapi.h    |  1 +
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>index 1b3df5b04249..1e631da58e15 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>@@ -10,6 +10,7 @@
> #include <linux/clk.h>
> #include <linux/firmware.h>
> #include <linux/interrupt.h>
>+#include <linux/reset.h>
> #include "wave5-vpu.h"
> #include "wave5-regdefine.h"
> #include "wave5-vpuconfig.h"
>@@ -151,6 +152,16 @@ static int wave5_vpu_probe(struct platform_device *pdev)
> 	dev_set_drvdata(&pdev->dev, dev);
> 	dev->dev = &pdev->dev;
>
>+	dev->resets = devm_reset_control_array_get_optional_exclusive(&pdev->dev);
>+	if (IS_ERR(dev->resets)) {
>+		return dev_err_probe(&pdev->dev, PTR_ERR(dev->resets),
>+				     "Failed to get reset control\n");
>+	}
>+
>+	ret = reset_control_deassert(dev->resets);
>+	if (ret)
>+		return dev_err_probe(&pdev->dev, ret, "Failed to deassert resets\n");
>+
> 	ret = devm_clk_bulk_get_all(&pdev->dev, &dev->clks);
>
> 	/* continue without clock, assume externally managed */
>@@ -163,7 +174,7 @@ static int wave5_vpu_probe(struct platform_device *pdev)
> 	ret = clk_bulk_prepare_enable(dev->num_clks, dev->clks);
> 	if (ret) {
> 		dev_err(&pdev->dev, "Enabling clocks, fail: %d\n", ret);
>-		return ret;
>+		goto err_reset_assert;
> 	}
>
> 	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
>@@ -246,6 +257,8 @@ static int wave5_vpu_probe(struct platform_device *pdev)
> 	wave5_vdi_release(&pdev->dev);
> err_clk_dis:
> 	clk_bulk_disable_unprepare(dev->num_clks, dev->clks);
>+err_reset_assert:
>+	reset_control_assert(dev->resets);
>
> 	return ret;
> }
>@@ -256,6 +269,7 @@ static void wave5_vpu_remove(struct platform_device *pdev)
>
> 	mutex_destroy(&dev->dev_lock);
> 	mutex_destroy(&dev->hw_lock);
>+	reset_control_assert(dev->resets);
> 	clk_bulk_disable_unprepare(dev->num_clks, dev->clks);
> 	wave5_vpu_enc_unregister_device(dev);
> 	wave5_vpu_dec_unregister_device(dev);
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>index 465ff9dfe8b1..da530fd98964 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>@@ -758,6 +758,7 @@ struct vpu_device {
> 	struct ida inst_ida;
> 	struct clk_bulk_data *clks;
> 	int num_clks;
>+	struct reset_control *resets;
> };
>
> struct vpu_instance;
>-- 
>2.44.0
>
>

[PATCH v2 3/5] media: chips-media: wave5: separate irq setup routine

[Ivan Bornyakov]

Separate interrupts setup routine to reduce code duplication. Also
enable interrupts based on vpu_attr->support_encoders and
vpu_attr->support_decoders fields to facilitate other Wave5xx IPs
support, because not all of them are both encoders and decoders.

Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
---
 .../platform/chips-media/wave5/wave5-hw.c     | 53 +++++++++----------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
index 2d82791f575e..cdd0a0948a94 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
@@ -299,6 +299,27 @@ static int wave5_send_query(struct vpu_device *vpu_dev, struct vpu_instance *ins
 	return wave5_vpu_firmware_command_queue_error_check(vpu_dev, NULL);
 }
 
+static void setup_wave5_interrupts(struct vpu_device *vpu_dev)
+{
+	u32 reg_val = 0;
+
+	if (vpu_dev->attr.support_encoders) {
+		/* Encoder interrupt */
+		reg_val |= BIT(INT_WAVE5_ENC_SET_PARAM);
+		reg_val |= BIT(INT_WAVE5_ENC_PIC);
+		reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
+	}
+
+	if (vpu_dev->attr.support_decoders) {
+		/* Decoder interrupt */
+		reg_val |= BIT(INT_WAVE5_INIT_SEQ);
+		reg_val |= BIT(INT_WAVE5_DEC_PIC);
+		reg_val |= BIT(INT_WAVE5_BSBUF_EMPTY);
+	}
+
+	return vpu_write_reg(vpu_dev, W5_VPU_VINT_ENABLE, reg_val);
+}
+
 static int setup_wave5_properties(struct device *dev)
 {
 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
@@ -340,6 +361,8 @@ static int setup_wave5_properties(struct device *dev)
 	p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE, hw_config_def0);
 	p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE, hw_config_def0);
 
+	setup_wave5_interrupts(vpu_dev);
+
 	return 0;
 }
 
@@ -417,16 +440,6 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
 	wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
 	vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
 
-	/* Encoder interrupt */
-	reg_val = BIT(INT_WAVE5_ENC_SET_PARAM);
-	reg_val |= BIT(INT_WAVE5_ENC_PIC);
-	reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
-	/* Decoder interrupt */
-	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 (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
 		reg_val = ((WAVE5_PROC_AXI_ID << 28) |
@@ -1034,16 +1047,6 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
 		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
 		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
 
-		/* Encoder interrupt */
-		reg_val = BIT(INT_WAVE5_ENC_SET_PARAM);
-		reg_val |= BIT(INT_WAVE5_ENC_PIC);
-		reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
-		/* Decoder interrupt */
-		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 (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
 			reg_val = ((WAVE5_PROC_AXI_ID << 28) |
@@ -1134,15 +1137,7 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
 		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
 		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
 
-		/* Encoder interrupt */
-		reg_val = BIT(INT_WAVE5_ENC_SET_PARAM);
-		reg_val |= BIT(INT_WAVE5_ENC_PIC);
-		reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
-		/* Decoder interrupt */
-		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);
+		setup_wave5_interrupts(vpu_dev);
 
 		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
 		if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
-- 
2.44.0

Re: [PATCH v2 3/5] media: chips-media: wave5: separate irq setup routine

[Sebastian Fricke]

Hey Ivan,

On 25.03.2024 09:40, Ivan Bornyakov wrote:
>Separate interrupts setup routine to reduce code duplication. Also

s/Separate interrupts setup routine/Implement a separate setup routine for interrupts/

>enable interrupts based on vpu_attr->support_encoders and
>vpu_attr->support_decoders fields to facilitate other Wave5xx IPs
>support, because not all of them are both encoders and decoders.

s/other Wave5xx IPs support/support for other Wave5xx IPs/

The rest looks good to me.

Greetings,
Sebastian

>
>Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>---
> .../platform/chips-media/wave5/wave5-hw.c     | 53 +++++++++----------
> 1 file changed, 24 insertions(+), 29 deletions(-)
>
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>index 2d82791f575e..cdd0a0948a94 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>@@ -299,6 +299,27 @@ static int wave5_send_query(struct vpu_device *vpu_dev, struct vpu_instance *ins
> 	return wave5_vpu_firmware_command_queue_error_check(vpu_dev, NULL);
> }
>
>+static void setup_wave5_interrupts(struct vpu_device *vpu_dev)
>+{
>+	u32 reg_val = 0;
>+
>+	if (vpu_dev->attr.support_encoders) {
>+		/* Encoder interrupt */
>+		reg_val |= BIT(INT_WAVE5_ENC_SET_PARAM);
>+		reg_val |= BIT(INT_WAVE5_ENC_PIC);
>+		reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
>+	}
>+
>+	if (vpu_dev->attr.support_decoders) {
>+		/* Decoder interrupt */
>+		reg_val |= BIT(INT_WAVE5_INIT_SEQ);
>+		reg_val |= BIT(INT_WAVE5_DEC_PIC);
>+		reg_val |= BIT(INT_WAVE5_BSBUF_EMPTY);
>+	}
>+
>+	return vpu_write_reg(vpu_dev, W5_VPU_VINT_ENABLE, reg_val);
>+}
>+
> static int setup_wave5_properties(struct device *dev)
> {
> 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
>@@ -340,6 +361,8 @@ static int setup_wave5_properties(struct device *dev)
> 	p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE, hw_config_def0);
> 	p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE, hw_config_def0);
>
>+	setup_wave5_interrupts(vpu_dev);
>+
> 	return 0;
> }
>
>@@ -417,16 +440,6 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
> 	wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> 	vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>
>-	/* Encoder interrupt */
>-	reg_val = BIT(INT_WAVE5_ENC_SET_PARAM);
>-	reg_val |= BIT(INT_WAVE5_ENC_PIC);
>-	reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
>-	/* Decoder interrupt */
>-	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 (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
> 		reg_val = ((WAVE5_PROC_AXI_ID << 28) |
>@@ -1034,16 +1047,6 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
> 		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> 		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>
>-		/* Encoder interrupt */
>-		reg_val = BIT(INT_WAVE5_ENC_SET_PARAM);
>-		reg_val |= BIT(INT_WAVE5_ENC_PIC);
>-		reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
>-		/* Decoder interrupt */
>-		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 (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
> 			reg_val = ((WAVE5_PROC_AXI_ID << 28) |
>@@ -1134,15 +1137,7 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
> 		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> 		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>
>-		/* Encoder interrupt */
>-		reg_val = BIT(INT_WAVE5_ENC_SET_PARAM);
>-		reg_val |= BIT(INT_WAVE5_ENC_PIC);
>-		reg_val |= BIT(INT_WAVE5_BSBUF_FULL);
>-		/* Decoder interrupt */
>-		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);
>+		setup_wave5_interrupts(vpu_dev);
>
> 		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> 		if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
>-- 
>2.44.0
>
>

[PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Ivan Bornyakov]

Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
excessive "sram-size" device-tree property as genalloc is already able
to determine available memory.

Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
---
 .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++---------
 .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
 .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
 .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
 4 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
index 3809f70bc0b4..a63fffed55e9 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct vpu_device *vpu_dev, struct vpu_buf *array,
 void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
 {
 	struct vpu_buf *vb = &vpu_dev->sram_buf;
+	dma_addr_t daddr;
+	void *vaddr;
+	size_t size;
 
-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
+	if (!vpu_dev->sram_pool || vb->vaddr)
 		return;
 
-	if (!vb->vaddr) {
-		vb->size = vpu_dev->sram_size;
-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb->size,
-					       &vb->daddr);
-		if (!vb->vaddr)
-			vb->size = 0;
+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE, gen_pool_avail(vpu_dev->sram_pool));
+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
+	if (vaddr) {
+		vb->vaddr = vaddr;
+		vb->daddr = daddr;
+		vb->size = size;
 	}
 
 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu, vaddr: 0x%p\n",
@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device *vpu_dev)
 	if (!vb->size || !vb->vaddr)
 		return;
 
-	if (vb->vaddr)
-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
-			      vb->size);
+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr, vb->size);
 
 	memset(vb, 0, sizeof(*vb));
 }
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 1e631da58e15..2a972cddf4a6 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct platform_device *pdev)
 		goto err_reset_assert;
 	}
 
-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
-				   &dev->sram_size);
-	if (ret) {
-		dev_warn(&pdev->dev, "sram-size not found\n");
-		dev->sram_size = 0;
-	}
-
 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
 	if (!dev->sram_pool)
 		dev_warn(&pdev->dev, "sram node not found\n");
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
index da530fd98964..975d96b22191 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
@@ -750,7 +750,6 @@ struct vpu_device {
 	struct vpu_attr attr;
 	struct vpu_buf common_mem;
 	u32 last_performance_cycles;
-	u32 sram_size;
 	struct gen_pool *sram_pool;
 	struct vpu_buf sram_buf;
 	void __iomem *vdb_register;
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
index d9751eedb0f9..9d99afb78c89 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
@@ -28,6 +28,8 @@
 #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC 40K
 #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
 
+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
+
 #define MAX_NUM_INSTANCE                32
 
 #define W5_MIN_ENC_PIC_WIDTH            256
-- 
2.44.0

RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Nas Chung]

Hi, Ivan.

>-----Original Message-----
>From: Ivan Bornyakov <brnkv.i1@gmail.com>
>Sent: Monday, March 25, 2024 3:41 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
>Philipp Zabel <p.zabel@pengutronix.de>
>Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
>prop
>
>Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
>excessive "sram-size" device-tree property as genalloc is already able
>to determine available memory.
>
>Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>---
> .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++---------
> .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
> .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
> .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
> 4 files changed, 13 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>index 3809f70bc0b4..a63fffed55e9 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct vpu_device
>*vpu_dev, struct vpu_buf *array,
> void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
> {
> 	struct vpu_buf *vb = &vpu_dev->sram_buf;
>+	dma_addr_t daddr;
>+	void *vaddr;
>+	size_t size;
>
>-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
>+	if (!vpu_dev->sram_pool || vb->vaddr)
> 		return;
>
>-	if (!vb->vaddr) {
>-		vb->size = vpu_dev->sram_size;
>-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb->size,
>-					       &vb->daddr);
>-		if (!vb->vaddr)
>-			vb->size = 0;
>+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE, gen_pool_avail(vpu_dev-
>>sram_pool));
>+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
>+	if (vaddr) {
>+		vb->vaddr = vaddr;
>+		vb->daddr = daddr;
>+		vb->size = size;
> 	}
>
> 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu, vaddr:
>0x%p\n",
>@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device *vpu_dev)
> 	if (!vb->size || !vb->vaddr)
> 		return;
>
>-	if (vb->vaddr)
>-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
>-			      vb->size);
>+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr, vb-
>>size);
>
> 	memset(vb, 0, sizeof(*vb));
> }
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>index 1e631da58e15..2a972cddf4a6 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct platform_device
>*pdev)
> 		goto err_reset_assert;
> 	}
>
>-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
>-				   &dev->sram_size);
>-	if (ret) {
>-		dev_warn(&pdev->dev, "sram-size not found\n");
>-		dev->sram_size = 0;
>-	}
>-
> 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
> 	if (!dev->sram_pool)
> 		dev_warn(&pdev->dev, "sram node not found\n");
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>index da530fd98964..975d96b22191 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>@@ -750,7 +750,6 @@ struct vpu_device {
> 	struct vpu_attr attr;
> 	struct vpu_buf common_mem;
> 	u32 last_performance_cycles;
>-	u32 sram_size;
> 	struct gen_pool *sram_pool;
> 	struct vpu_buf sram_buf;
> 	void __iomem *vdb_register;
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>index d9751eedb0f9..9d99afb78c89 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>@@ -28,6 +28,8 @@
> #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC
>40K
> #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>
>+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)

WAVE521 can support 8K stream decoding/encoding.
So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).

And, Current driver always enable sec_axi_info option if sram buffer is allocated.
But, we have to enable/disable the sec_axi_info option after checking the allocated sram size is enough to decode/encode current bitstream resolution.
Wave5 can enable/disable the sec_axi_info option for each instance.

How about handle sram-size through match_data ?
I can find some drivers which use match_data to configure the sram size.

We can use current "ti,k3-j721s2-wave521c" device as a 4K supported device.
- .sram_size = (64 * 1024);
Driver just allocate the sram-size for max supported resolution of each device, and we don't need to check the sram-size is enough or not.

Thanks.
Nas.

>+
> #define MAX_NUM_INSTANCE                32
>
> #define W5_MIN_ENC_PIC_WIDTH            256
>--
>2.44.0

Re: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Ivan Bornyakov]

On Wed, Mar 27, 2024 at 10:27:19AM +0000, Nas Chung wrote:
> Hi, Ivan.
> 
> >-----Original Message-----
> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
> >Sent: Monday, March 25, 2024 3:41 PM
> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
> >Philipp Zabel <p.zabel@pengutronix.de>
> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
> >linux-kernel@vger.kernel.org
> >Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
> >prop
> >
> >Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
> >excessive "sram-size" device-tree property as genalloc is already able
> >to determine available memory.
> >
> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
> >---
> > .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++---------
> > .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
> > .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
> > .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
> > 4 files changed, 13 insertions(+), 18 deletions(-)
> >
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >index 3809f70bc0b4..a63fffed55e9 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct vpu_device
> >*vpu_dev, struct vpu_buf *array,
> > void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
> > {
> > 	struct vpu_buf *vb = &vpu_dev->sram_buf;
> >+	dma_addr_t daddr;
> >+	void *vaddr;
> >+	size_t size;
> >
> >-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
> >+	if (!vpu_dev->sram_pool || vb->vaddr)
> > 		return;
> >
> >-	if (!vb->vaddr) {
> >-		vb->size = vpu_dev->sram_size;
> >-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb->size,
> >-					       &vb->daddr);
> >-		if (!vb->vaddr)
> >-			vb->size = 0;
> >+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE, gen_pool_avail(vpu_dev-
> >>sram_pool));
> >+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
> >+	if (vaddr) {
> >+		vb->vaddr = vaddr;
> >+		vb->daddr = daddr;
> >+		vb->size = size;
> > 	}
> >
> > 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu, vaddr:
> >0x%p\n",
> >@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device *vpu_dev)
> > 	if (!vb->size || !vb->vaddr)
> > 		return;
> >
> >-	if (vb->vaddr)
> >-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
> >-			      vb->size);
> >+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr, vb-
> >>size);
> >
> > 	memset(vb, 0, sizeof(*vb));
> > }
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >index 1e631da58e15..2a972cddf4a6 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct platform_device
> >*pdev)
> > 		goto err_reset_assert;
> > 	}
> >
> >-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
> >-				   &dev->sram_size);
> >-	if (ret) {
> >-		dev_warn(&pdev->dev, "sram-size not found\n");
> >-		dev->sram_size = 0;
> >-	}
> >-
> > 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
> > 	if (!dev->sram_pool)
> > 		dev_warn(&pdev->dev, "sram node not found\n");
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >index da530fd98964..975d96b22191 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >@@ -750,7 +750,6 @@ struct vpu_device {
> > 	struct vpu_attr attr;
> > 	struct vpu_buf common_mem;
> > 	u32 last_performance_cycles;
> >-	u32 sram_size;
> > 	struct gen_pool *sram_pool;
> > 	struct vpu_buf sram_buf;
> > 	void __iomem *vdb_register;
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >index d9751eedb0f9..9d99afb78c89 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >@@ -28,6 +28,8 @@
> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC
> >40K
> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
> >
> >+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
> 
> WAVE521 can support 8K stream decoding/encoding.
> So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).
> 
> And, Current driver always enable sec_axi_info option if sram buffer is allocated.
> But, we have to enable/disable the sec_axi_info option after checking the allocated sram size is enough to decode/encode current bitstream resolution.
> Wave5 can enable/disable the sec_axi_info option for each instance.
> 
> How about handle sram-size through match_data ?
> I can find some drivers which use match_data to configure the sram size.
> 
> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported device.
> - .sram_size = (64 * 1024);
> Driver just allocate the sram-size for max supported resolution of each device, and we don't need to check the sram-size is enough or not.

Sure, sounds reasonable.

> 
> Thanks.
> Nas.
> 
> >+
> > #define MAX_NUM_INSTANCE                32
> >
> > #define W5_MIN_ENC_PIC_WIDTH            256
> >--
> >2.44.0
> 

Re: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Ivan Bornyakov]

On Wed, Mar 27, 2024 at 10:27:19AM +0000, Nas Chung wrote:
> Hi, Ivan.
> 
> >-----Original Message-----
> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
> >Sent: Monday, March 25, 2024 3:41 PM
> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
> >Philipp Zabel <p.zabel@pengutronix.de>
> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
> >linux-kernel@vger.kernel.org
> >Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
> >prop
> >
> >Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
> >excessive "sram-size" device-tree property as genalloc is already able
> >to determine available memory.
> >
> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
> >---
> > .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++---------
> > .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
> > .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
> > .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
> > 4 files changed, 13 insertions(+), 18 deletions(-)
> >
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >index 3809f70bc0b4..a63fffed55e9 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct vpu_device
> >*vpu_dev, struct vpu_buf *array,
> > void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
> > {
> > 	struct vpu_buf *vb = &vpu_dev->sram_buf;
> >+	dma_addr_t daddr;
> >+	void *vaddr;
> >+	size_t size;
> >
> >-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
> >+	if (!vpu_dev->sram_pool || vb->vaddr)
> > 		return;
> >
> >-	if (!vb->vaddr) {
> >-		vb->size = vpu_dev->sram_size;
> >-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb->size,
> >-					       &vb->daddr);
> >-		if (!vb->vaddr)
> >-			vb->size = 0;
> >+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE, gen_pool_avail(vpu_dev-
> >>sram_pool));
> >+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
> >+	if (vaddr) {
> >+		vb->vaddr = vaddr;
> >+		vb->daddr = daddr;
> >+		vb->size = size;
> > 	}
> >
> > 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu, vaddr:
> >0x%p\n",
> >@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device *vpu_dev)
> > 	if (!vb->size || !vb->vaddr)
> > 		return;
> >
> >-	if (vb->vaddr)
> >-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
> >-			      vb->size);
> >+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr, vb-
> >>size);
> >
> > 	memset(vb, 0, sizeof(*vb));
> > }
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >index 1e631da58e15..2a972cddf4a6 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct platform_device
> >*pdev)
> > 		goto err_reset_assert;
> > 	}
> >
> >-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
> >-				   &dev->sram_size);
> >-	if (ret) {
> >-		dev_warn(&pdev->dev, "sram-size not found\n");
> >-		dev->sram_size = 0;
> >-	}
> >-
> > 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
> > 	if (!dev->sram_pool)
> > 		dev_warn(&pdev->dev, "sram node not found\n");
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >index da530fd98964..975d96b22191 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >@@ -750,7 +750,6 @@ struct vpu_device {
> > 	struct vpu_attr attr;
> > 	struct vpu_buf common_mem;
> > 	u32 last_performance_cycles;
> >-	u32 sram_size;
> > 	struct gen_pool *sram_pool;
> > 	struct vpu_buf sram_buf;
> > 	void __iomem *vdb_register;
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >index d9751eedb0f9..9d99afb78c89 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >@@ -28,6 +28,8 @@
> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC
> >40K
> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
> >
> >+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
> 
> WAVE521 can support 8K stream decoding/encoding.
> So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).
> 
> And, Current driver always enable sec_axi_info option if sram buffer is allocated.
> But, we have to enable/disable the sec_axi_info option after checking the allocated sram size is enough to decode/encode current bitstream resolution.

Do we really? As an experiment I tried to provide to Wave515 1KB of SRAM
memory and decoded 4k sample file was fine...

> Wave5 can enable/disable the sec_axi_info option for each instance.
> 
> How about handle sram-size through match_data ?
> I can find some drivers which use match_data to configure the sram size.
> 
> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported device.
> - .sram_size = (64 * 1024);
> Driver just allocate the sram-size for max supported resolution of each device, and we don't need to check the sram-size is enough or not.
> 
> Thanks.
> Nas.
> 
> >+
> > #define MAX_NUM_INSTANCE                32
> >
> > #define W5_MIN_ENC_PIC_WIDTH            256
> >--
> >2.44.0
> 

RE: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Nas Chung]

Hi, Ivan.

>-----Original Message-----
>From: Ivan Bornyakov <brnkv.i1@gmail.com>
>Sent: Wednesday, March 27, 2024 9:27 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>
>Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>jackson.lee <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
><mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>
>Subject: Re: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-
>size" DT prop
>
>On Wed, Mar 27, 2024 at 10:27:19AM +0000, Nas Chung wrote:
>> Hi, Ivan.
>>
>> >-----Original Message-----
>> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
>> >Sent: Monday, March 25, 2024 3:41 PM
>> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
>> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
><mchehab@kernel.org>;
>> >Philipp Zabel <p.zabel@pengutronix.de>
>> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
>> >linux-kernel@vger.kernel.org
>> >Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
>> >prop
>> >
>> >Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
>> >excessive "sram-size" device-tree property as genalloc is already able
>> >to determine available memory.
>> >
>> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>> >---
>> > .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++---------
>> > .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
>> > .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
>> > .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
>> > 4 files changed, 13 insertions(+), 18 deletions(-)
>> >
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >index 3809f70bc0b4..a63fffed55e9 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct vpu_device
>> >*vpu_dev, struct vpu_buf *array,
>> > void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
>> > {
>> > 	struct vpu_buf *vb = &vpu_dev->sram_buf;
>> >+	dma_addr_t daddr;
>> >+	void *vaddr;
>> >+	size_t size;
>> >
>> >-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
>> >+	if (!vpu_dev->sram_pool || vb->vaddr)
>> > 		return;
>> >
>> >-	if (!vb->vaddr) {
>> >-		vb->size = vpu_dev->sram_size;
>> >-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb->size,
>> >-					       &vb->daddr);
>> >-		if (!vb->vaddr)
>> >-			vb->size = 0;
>> >+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE, gen_pool_avail(vpu_dev-
>> >>sram_pool));
>> >+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
>> >+	if (vaddr) {
>> >+		vb->vaddr = vaddr;
>> >+		vb->daddr = daddr;
>> >+		vb->size = size;
>> > 	}
>> >
>> > 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu, vaddr:
>> >0x%p\n",
>> >@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device
>*vpu_dev)
>> > 	if (!vb->size || !vb->vaddr)
>> > 		return;
>> >
>> >-	if (vb->vaddr)
>> >-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
>> >-			      vb->size);
>> >+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr, vb-
>> >>size);
>> >
>> > 	memset(vb, 0, sizeof(*vb));
>> > }
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >index 1e631da58e15..2a972cddf4a6 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct platform_device
>> >*pdev)
>> > 		goto err_reset_assert;
>> > 	}
>> >
>> >-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
>> >-				   &dev->sram_size);
>> >-	if (ret) {
>> >-		dev_warn(&pdev->dev, "sram-size not found\n");
>> >-		dev->sram_size = 0;
>> >-	}
>> >-
>> > 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
>> > 	if (!dev->sram_pool)
>> > 		dev_warn(&pdev->dev, "sram node not found\n");
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >index da530fd98964..975d96b22191 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >@@ -750,7 +750,6 @@ struct vpu_device {
>> > 	struct vpu_attr attr;
>> > 	struct vpu_buf common_mem;
>> > 	u32 last_performance_cycles;
>> >-	u32 sram_size;
>> > 	struct gen_pool *sram_pool;
>> > 	struct vpu_buf sram_buf;
>> > 	void __iomem *vdb_register;
>> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-
>vpuconfig.h
>> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >index d9751eedb0f9..9d99afb78c89 100644
>> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >@@ -28,6 +28,8 @@
>> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K,
>AVC
>> >40K
>> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>> >
>> >+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
>>
>> WAVE521 can support 8K stream decoding/encoding.
>> So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).
>>
>> And, Current driver always enable sec_axi_info option if sram buffer is
>allocated.
>> But, we have to enable/disable the sec_axi_info option after checking
>the allocated sram size is enough to decode/encode current bitstream
>resolution.
>
>Do we really? As an experiment I tried to provide to Wave515 1KB of SRAM
>memory and decoded 4k sample file was fine...
>

You can think It seems like driver works fine.
But, This is not the behavior we expect.
There is a possibility that unexpected problems may occur.

>> Wave5 can enable/disable the sec_axi_info option for each instance.
>>
>> How about handle sram-size through match_data ?
>> I can find some drivers which use match_data to configure the sram size.
>>
>> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported
>device.
>> - .sram_size = (64 * 1024);
>> Driver just allocate the sram-size for max supported resolution of each
>device, and we don't need to check the sram-size is enough or not.
>>
>> Thanks.
>> Nas.
>>
>> >+
>> > #define MAX_NUM_INSTANCE                32
>> >
>> > #define W5_MIN_ENC_PIC_WIDTH            256
>> >--
>> >2.44.0
>>

Re: RE: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Ivan Bornyakov]

On Thu, Mar 28, 2024 at 10:16:53AM +0000, Nas Chung wrote:
> Hi, Ivan.
> 
> >-----Original Message-----
> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
> >Sent: Wednesday, March 27, 2024 9:27 PM
> >To: Nas Chung <nas.chung@chipsnmedia.com>
> >Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
> >jackson.lee <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
> ><mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>
> >Subject: Re: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-
> >size" DT prop
> >
> >On Wed, Mar 27, 2024 at 10:27:19AM +0000, Nas Chung wrote:
> >> Hi, Ivan.
> >>
> >> >-----Original Message-----
> >> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
> >> >Sent: Monday, March 25, 2024 3:41 PM
> >> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
> >> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
> ><mchehab@kernel.org>;
> >> >Philipp Zabel <p.zabel@pengutronix.de>
> >> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
> >> >linux-kernel@vger.kernel.org
> >> >Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT
> >> >prop
> >> >
> >> >Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
> >> >excessive "sram-size" device-tree property as genalloc is already able
> >> >to determine available memory.
> >> >
> >> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
> >> >---
> >> > .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++---------
> >> > .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
> >> > .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
> >> > .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
> >> > 4 files changed, 13 insertions(+), 18 deletions(-)
> >> >
> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >> >index 3809f70bc0b4..a63fffed55e9 100644
> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >> >@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct vpu_device
> >> >*vpu_dev, struct vpu_buf *array,
> >> > void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
> >> > {
> >> > 	struct vpu_buf *vb = &vpu_dev->sram_buf;
> >> >+	dma_addr_t daddr;
> >> >+	void *vaddr;
> >> >+	size_t size;
> >> >
> >> >-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
> >> >+	if (!vpu_dev->sram_pool || vb->vaddr)
> >> > 		return;
> >> >
> >> >-	if (!vb->vaddr) {
> >> >-		vb->size = vpu_dev->sram_size;
> >> >-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb->size,
> >> >-					       &vb->daddr);
> >> >-		if (!vb->vaddr)
> >> >-			vb->size = 0;
> >> >+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE, gen_pool_avail(vpu_dev-
> >> >>sram_pool));
> >> >+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
> >> >+	if (vaddr) {
> >> >+		vb->vaddr = vaddr;
> >> >+		vb->daddr = daddr;
> >> >+		vb->size = size;
> >> > 	}
> >> >
> >> > 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu, vaddr:
> >> >0x%p\n",
> >> >@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device
> >*vpu_dev)
> >> > 	if (!vb->size || !vb->vaddr)
> >> > 		return;
> >> >
> >> >-	if (vb->vaddr)
> >> >-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
> >> >-			      vb->size);
> >> >+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr, vb-
> >> >>size);
> >> >
> >> > 	memset(vb, 0, sizeof(*vb));
> >> > }
> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> >index 1e631da58e15..2a972cddf4a6 100644
> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> >@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct platform_device
> >> >*pdev)
> >> > 		goto err_reset_assert;
> >> > 	}
> >> >
> >> >-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
> >> >-				   &dev->sram_size);
> >> >-	if (ret) {
> >> >-		dev_warn(&pdev->dev, "sram-size not found\n");
> >> >-		dev->sram_size = 0;
> >> >-	}
> >> >-
> >> > 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
> >> > 	if (!dev->sram_pool)
> >> > 		dev_warn(&pdev->dev, "sram node not found\n");
> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >> >index da530fd98964..975d96b22191 100644
> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >> >@@ -750,7 +750,6 @@ struct vpu_device {
> >> > 	struct vpu_attr attr;
> >> > 	struct vpu_buf common_mem;
> >> > 	u32 last_performance_cycles;
> >> >-	u32 sram_size;
> >> > 	struct gen_pool *sram_pool;
> >> > 	struct vpu_buf sram_buf;
> >> > 	void __iomem *vdb_register;
> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-
> >vpuconfig.h
> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >> >index d9751eedb0f9..9d99afb78c89 100644
> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >> >@@ -28,6 +28,8 @@
> >> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K,
> >AVC
> >> >40K
> >> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
> >> >
> >> >+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
> >>
> >> WAVE521 can support 8K stream decoding/encoding.
> >> So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).
> >>
> >> And, Current driver always enable sec_axi_info option if sram buffer is
> >allocated.
> >> But, we have to enable/disable the sec_axi_info option after checking
> >the allocated sram size is enough to decode/encode current bitstream
> >resolution.
> >
> >Do we really? As an experiment I tried to provide to Wave515 1KB of SRAM
> >memory and decoded 4k sample file was fine...
> >
> 
> You can think It seems like driver works fine.
> But, This is not the behavior we expect.
> There is a possibility that unexpected problems may occur.
> 

Ok, then we either

 1) don't try to allocate any availible SRAM memory up to
    match_data->sram_size, but allocate exact match_data->sram_size

or

 2) allocate any available SRAM memory up to match_data->sram_size, but
    check for allocated size before writing to registers W5_USE_SEC_AXI
    and W5_CMD_ENC_PIC_USE_SEC_AXI

With second variant I won't be able to add said check for Wave521, as I
don't know its memory requirements.

Also would this check be SoC specific or would it be common for any SoC
with same Wave5xx IP?

> >> Wave5 can enable/disable the sec_axi_info option for each instance.
> >>
> >> How about handle sram-size through match_data ?
> >> I can find some drivers which use match_data to configure the sram size.
> >>
> >> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported
> >device.
> >> - .sram_size = (64 * 1024);
> >> Driver just allocate the sram-size for max supported resolution of each
> >device, and we don't need to check the sram-size is enough or not.
> >>
> >> Thanks.
> >> Nas.
> >>
> >> >+
> >> > #define MAX_NUM_INSTANCE                32
> >> >
> >> > #define W5_MIN_ENC_PIC_WIDTH            256
> >> >--
> >> >2.44.0
> >>

RE: RE: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Nas Chung]

Hi, Sebastian.

>-----Original Message-----
>From: Ivan Bornyakov <brnkv.i1@gmail.com>
>Sent: Thursday, March 28, 2024 8:36 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>
>Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>jackson.lee <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
><mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>
>Subject: Re: RE: RE: [PATCH v2 4/5] media: chips-media: wave5: drop
>"sram-size" DT prop
>
>On Thu, Mar 28, 2024 at 10:16:53AM +0000, Nas Chung wrote:
>> Hi, Ivan.
>>
>> >-----Original Message-----
>> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
>> >Sent: Wednesday, March 27, 2024 9:27 PM
>> >To: Nas Chung <nas.chung@chipsnmedia.com>
>> >Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>> >jackson.lee <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
>> ><mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>
>> >Subject: Re: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-
>> >size" DT prop
>> >
>> >On Wed, Mar 27, 2024 at 10:27:19AM +0000, Nas Chung wrote:
>> >> Hi, Ivan.
>> >>
>> >> >-----Original Message-----
>> >> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
>> >> >Sent: Monday, March 25, 2024 3:41 PM
>> >> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
>> >> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
>> ><mchehab@kernel.org>;
>> >> >Philipp Zabel <p.zabel@pengutronix.de>
>> >> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-
>media@vger.kernel.org;
>> >> >linux-kernel@vger.kernel.org
>> >> >Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size"
>DT
>> >> >prop
>> >> >
>> >> >Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
>> >> >excessive "sram-size" device-tree property as genalloc is already
>able
>> >> >to determine available memory.
>> >> >
>> >> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>> >> >---
>> >> > .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++------
>---
>> >> > .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
>> >> > .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
>> >> > .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
>> >> > 4 files changed, 13 insertions(+), 18 deletions(-)
>> >> >
>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >> >index 3809f70bc0b4..a63fffed55e9 100644
>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>> >> >@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct
>vpu_device
>> >> >*vpu_dev, struct vpu_buf *array,
>> >> > void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
>> >> > {
>> >> > 	struct vpu_buf *vb = &vpu_dev->sram_buf;
>> >> >+	dma_addr_t daddr;
>> >> >+	void *vaddr;
>> >> >+	size_t size;
>> >> >
>> >> >-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
>> >> >+	if (!vpu_dev->sram_pool || vb->vaddr)
>> >> > 		return;
>> >> >
>> >> >-	if (!vb->vaddr) {
>> >> >-		vb->size = vpu_dev->sram_size;
>> >> >-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb-
>>size,
>> >> >-					       &vb->daddr);
>> >> >-		if (!vb->vaddr)
>> >> >-			vb->size = 0;
>> >> >+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE,
>gen_pool_avail(vpu_dev-
>> >> >>sram_pool));
>> >> >+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
>> >> >+	if (vaddr) {
>> >> >+		vb->vaddr = vaddr;
>> >> >+		vb->daddr = daddr;
>> >> >+		vb->size = size;
>> >> > 	}
>> >> >
>> >> > 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu,
>vaddr:
>> >> >0x%p\n",
>> >> >@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device
>> >*vpu_dev)
>> >> > 	if (!vb->size || !vb->vaddr)
>> >> > 		return;
>> >> >
>> >> >-	if (vb->vaddr)
>> >> >-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb-
>>vaddr,
>> >> >-			      vb->size);
>> >> >+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
>vb-
>> >> >>size);
>> >> >
>> >> > 	memset(vb, 0, sizeof(*vb));
>> >> > }
>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >> >index 1e631da58e15..2a972cddf4a6 100644
>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> >> >@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct
>platform_device
>> >> >*pdev)
>> >> > 		goto err_reset_assert;
>> >> > 	}
>> >> >
>> >> >-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
>> >> >-				   &dev->sram_size);
>> >> >-	if (ret) {
>> >> >-		dev_warn(&pdev->dev, "sram-size not found\n");
>> >> >-		dev->sram_size = 0;
>> >> >-	}
>> >> >-
>> >> > 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram",
>0);
>> >> > 	if (!dev->sram_pool)
>> >> > 		dev_warn(&pdev->dev, "sram node not found\n");
>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-
>vpuapi.h
>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >> >index da530fd98964..975d96b22191 100644
>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>> >> >@@ -750,7 +750,6 @@ struct vpu_device {
>> >> > 	struct vpu_attr attr;
>> >> > 	struct vpu_buf common_mem;
>> >> > 	u32 last_performance_cycles;
>> >> >-	u32 sram_size;
>> >> > 	struct gen_pool *sram_pool;
>> >> > 	struct vpu_buf sram_buf;
>> >> > 	void __iomem *vdb_register;
>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-
>> >vpuconfig.h
>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >> >index d9751eedb0f9..9d99afb78c89 100644
>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>> >> >@@ -28,6 +28,8 @@
>> >> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K,
>> >AVC
>> >> >40K
>> >> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>> >> >
>> >> >+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
>> >>
>> >> WAVE521 can support 8K stream decoding/encoding.
>> >> So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).
>> >>
>> >> And, Current driver always enable sec_axi_info option if sram buffer
>is
>> >allocated.
>> >> But, we have to enable/disable the sec_axi_info option after
>checking
>> >the allocated sram size is enough to decode/encode current bitstream
>> >resolution.
>> >
>> >Do we really? As an experiment I tried to provide to Wave515 1KB of
>SRAM
>> >memory and decoded 4k sample file was fine...
>> >
>>
>> You can think It seems like driver works fine.
>> But, This is not the behavior we expect.
>> There is a possibility that unexpected problems may occur.
>>
>
>Ok, then we either
>
> 1) don't try to allocate any availible SRAM memory up to
>    match_data->sram_size, but allocate exact match_data->sram_size
>
>or
>
> 2) allocate any available SRAM memory up to match_data->sram_size, but
>    check for allocated size before writing to registers W5_USE_SEC_AXI
>    and W5_CMD_ENC_PIC_USE_SEC_AXI
>
>With second variant I won't be able to add said check for Wave521, as I
>don't know its memory requirements.
>
>Also would this check be SoC specific or would it be common for any SoC
>with same Wave5xx IP?
>
>> >> Wave5 can enable/disable the sec_axi_info option for each instance.
>> >>
>> >> How about handle sram-size through match_data ?
>> >> I can find some drivers which use match_data to configure the sram
>size.

I proposed using match_data to allocate different sram size for each device.
Do you think this is a reasonable approach ?

Thanks.
Nas.

>> >>
>> >> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported
>> >device.
>> >> - .sram_size = (64 * 1024);
>> >> Driver just allocate the sram-size for max supported resolution of
>each
>> >device, and we don't need to check the sram-size is enough or not.
>> >>
>> >> Thanks.
>> >> Nas.
>> >>
>> >> >+
>> >> > #define MAX_NUM_INSTANCE                32
>> >> >
>> >> > #define W5_MIN_ENC_PIC_WIDTH            256
>> >> >--
>> >> >2.44.0
>> >>

Re: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[sebastian.fricke]

Hey Nas,

On 01.04.2024 09:28, Nas Chung wrote:
>Hi, Sebastian.
>
>>-----Original Message-----
>>From: Ivan Bornyakov <brnkv.i1@gmail.com>
>>Sent: Thursday, March 28, 2024 8:36 PM
>>To: Nas Chung <nas.chung@chipsnmedia.com>
>>Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>>jackson.lee <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
>><mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>
>>Subject: Re: RE: RE: [PATCH v2 4/5] media: chips-media: wave5: drop
>>"sram-size" DT prop
>>
>>On Thu, Mar 28, 2024 at 10:16:53AM +0000, Nas Chung wrote:
>>> Hi, Ivan.
>>>
>>> >-----Original Message-----
>>> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
>>> >Sent: Wednesday, March 27, 2024 9:27 PM
>>> >To: Nas Chung <nas.chung@chipsnmedia.com>
>>> >Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>>> >jackson.lee <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
>>> ><mchehab@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>
>>> >Subject: Re: RE: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-
>>> >size" DT prop
>>> >
>>> >On Wed, Mar 27, 2024 at 10:27:19AM +0000, Nas Chung wrote:
>>> >> Hi, Ivan.
>>> >>
>>> >> >-----Original Message-----
>>> >> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
>>> >> >Sent: Monday, March 25, 2024 3:41 PM
>>> >> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
>>> >> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab
>>> ><mchehab@kernel.org>;
>>> >> >Philipp Zabel <p.zabel@pengutronix.de>
>>> >> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-
>>media@vger.kernel.org;
>>> >> >linux-kernel@vger.kernel.org
>>> >> >Subject: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size"
>>DT
>>> >> >prop
>>> >> >
>>> >> >Use all available SRAM memory up to WAVE5_MAX_SRAM_SIZE. Remove
>>> >> >excessive "sram-size" device-tree property as genalloc is already
>>able
>>> >> >to determine available memory.
>>> >> >
>>> >> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>>> >> >---
>>> >> > .../platform/chips-media/wave5/wave5-vdi.c    | 21 ++++++++++------
>>---
>>> >> > .../platform/chips-media/wave5/wave5-vpu.c    |  7 -------
>>> >> > .../platform/chips-media/wave5/wave5-vpuapi.h |  1 -
>>> >> > .../chips-media/wave5/wave5-vpuconfig.h       |  2 ++
>>> >> > 4 files changed, 13 insertions(+), 18 deletions(-)
>>> >> >
>>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>>> >> >index 3809f70bc0b4..a63fffed55e9 100644
>>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>>> >> >@@ -174,16 +174,19 @@ int wave5_vdi_allocate_array(struct
>>vpu_device
>>> >> >*vpu_dev, struct vpu_buf *array,
>>> >> > void wave5_vdi_allocate_sram(struct vpu_device *vpu_dev)
>>> >> > {
>>> >> > 	struct vpu_buf *vb = &vpu_dev->sram_buf;
>>> >> >+	dma_addr_t daddr;
>>> >> >+	void *vaddr;
>>> >> >+	size_t size;
>>> >> >
>>> >> >-	if (!vpu_dev->sram_pool || !vpu_dev->sram_size)
>>> >> >+	if (!vpu_dev->sram_pool || vb->vaddr)
>>> >> > 		return;
>>> >> >
>>> >> >-	if (!vb->vaddr) {
>>> >> >-		vb->size = vpu_dev->sram_size;
>>> >> >-		vb->vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, vb-
>>>size,
>>> >> >-					       &vb->daddr);
>>> >> >-		if (!vb->vaddr)
>>> >> >-			vb->size = 0;
>>> >> >+	size = min_t(size_t, WAVE5_MAX_SRAM_SIZE,
>>gen_pool_avail(vpu_dev-
>>> >> >>sram_pool));
>>> >> >+	vaddr = gen_pool_dma_alloc(vpu_dev->sram_pool, size, &daddr);
>>> >> >+	if (vaddr) {
>>> >> >+		vb->vaddr = vaddr;
>>> >> >+		vb->daddr = daddr;
>>> >> >+		vb->size = size;
>>> >> > 	}
>>> >> >
>>> >> > 	dev_dbg(vpu_dev->dev, "%s: sram daddr: %pad, size: %zu,
>>vaddr:
>>> >> >0x%p\n",
>>> >> >@@ -197,9 +200,7 @@ void wave5_vdi_free_sram(struct vpu_device
>>> >*vpu_dev)
>>> >> > 	if (!vb->size || !vb->vaddr)
>>> >> > 		return;
>>> >> >
>>> >> >-	if (vb->vaddr)
>>> >> >-		gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb-
>>>vaddr,
>>> >> >-			      vb->size);
>>> >> >+	gen_pool_free(vpu_dev->sram_pool, (unsigned long)vb->vaddr,
>>vb-
>>> >> >>size);
>>> >> >
>>> >> > 	memset(vb, 0, sizeof(*vb));
>>> >> > }
>>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>>> >> >index 1e631da58e15..2a972cddf4a6 100644
>>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>>> >> >@@ -177,13 +177,6 @@ static int wave5_vpu_probe(struct
>>platform_device
>>> >> >*pdev)
>>> >> > 		goto err_reset_assert;
>>> >> > 	}
>>> >> >
>>> >> >-	ret = of_property_read_u32(pdev->dev.of_node, "sram-size",
>>> >> >-				   &dev->sram_size);
>>> >> >-	if (ret) {
>>> >> >-		dev_warn(&pdev->dev, "sram-size not found\n");
>>> >> >-		dev->sram_size = 0;
>>> >> >-	}
>>> >> >-
>>> >> > 	dev->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram",
>>0);
>>> >> > 	if (!dev->sram_pool)
>>> >> > 		dev_warn(&pdev->dev, "sram node not found\n");
>>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-
>>vpuapi.h
>>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>>> >> >index da530fd98964..975d96b22191 100644
>>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>>> >> >@@ -750,7 +750,6 @@ struct vpu_device {
>>> >> > 	struct vpu_attr attr;
>>> >> > 	struct vpu_buf common_mem;
>>> >> > 	u32 last_performance_cycles;
>>> >> >-	u32 sram_size;
>>> >> > 	struct gen_pool *sram_pool;
>>> >> > 	struct vpu_buf sram_buf;
>>> >> > 	void __iomem *vdb_register;
>>> >> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-
>>> >vpuconfig.h
>>> >> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>>> >> >index d9751eedb0f9..9d99afb78c89 100644
>>> >> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>>> >> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>>> >> >@@ -28,6 +28,8 @@
>>> >> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K,
>>> >AVC
>>> >> >40K
>>> >> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>>> >> >
>>> >> >+#define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
>>> >>
>>> >> WAVE521 can support 8K stream decoding/encoding.
>>> >> So, I suggest the MAX_SRAME_SIZE to 128 * 1024 (128KB).
>>> >>
>>> >> And, Current driver always enable sec_axi_info option if sram buffer
>>is
>>> >allocated.
>>> >> But, we have to enable/disable the sec_axi_info option after
>>checking
>>> >the allocated sram size is enough to decode/encode current bitstream
>>> >resolution.
>>> >
>>> >Do we really? As an experiment I tried to provide to Wave515 1KB of
>>SRAM
>>> >memory and decoded 4k sample file was fine...
>>> >
>>>
>>> You can think It seems like driver works fine.
>>> But, This is not the behavior we expect.
>>> There is a possibility that unexpected problems may occur.
>>>
>>
>>Ok, then we either
>>
>> 1) don't try to allocate any availible SRAM memory up to
>>    match_data->sram_size, but allocate exact match_data->sram_size
>>
>>or
>>
>> 2) allocate any available SRAM memory up to match_data->sram_size, but
>>    check for allocated size before writing to registers W5_USE_SEC_AXI
>>    and W5_CMD_ENC_PIC_USE_SEC_AXI
>>
>>With second variant I won't be able to add said check for Wave521, as I
>>don't know its memory requirements.
>>
>>Also would this check be SoC specific or would it be common for any SoC
>>with same Wave5xx IP?
>>
>>> >> Wave5 can enable/disable the sec_axi_info option for each instance.
>>> >>
>>> >> How about handle sram-size through match_data ?
>>> >> I can find some drivers which use match_data to configure the sram
>>size.
>
>I proposed using match_data to allocate different sram size for each device.
>Do you think this is a reasonable approach ?

As discussed here:
https://patchwork.linuxtv.org/project/linux-media/patch/20240201184238.2542695-1-b-brnich@ti.com/

To quote Brandon Brnich from TI:

If static SRAM allocation is the correct method to go, then this series
can be ignored and I will add section in device tree and remove check
for parameter in driver as that will now be a bug.

I would like to adhere to that.

>
>Thanks.
>Nas.

Greetings,
Sebastian

>
>>> >>
>>> >> We can use current "ti,k3-j721s2-wave521c" device as a 4K supported
>>> >device.
>>> >> - .sram_size = (64 * 1024);
>>> >> Driver just allocate the sram-size for max supported resolution of
>>each
>>> >device, and we don't need to check the sram-size is enough or not.
>>> >>
>>> >> Thanks.
>>> >> Nas.
>>> >>
>>> >> >+
>>> >> > #define MAX_NUM_INSTANCE                32
>>> >> >
>>> >> > #define W5_MIN_ENC_PIC_WIDTH            256
>>> >> >--
>>> >> >2.44.0
>>> >>

Re: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[Nicolas Dufresne]

Hi,

Le jeudi 04 avril 2024 à 10:02 +0200, sebastian.fricke@collabora.com a écrit :
> > > > > > Wave5 can enable/disable the sec_axi_info option for each instance.
> > > > > > 
> > > > > > How about handle sram-size through match_data ?
> > > > > > I can find some drivers which use match_data to configure the sram
> > > size.
> > 
> > I proposed using match_data to allocate different sram size for each device.
> > Do you think this is a reasonable approach ?
> 
> As discussed here:
> https://patchwork.linuxtv.org/project/linux-media/patch/20240201184238.2542695-1-b-brnich@ti.com/
> 
> To quote Brandon Brnich from TI:
> 
> If static SRAM allocation is the correct method to go, then this series
> can be ignored and I will add section in device tree and remove check
> for parameter in driver as that will now be a bug.
> 
> I would like to adhere to that.

I feel your statement is a bit ambiguous. Can you clarify what you adhere too ?
That we should hardcode fixed sram size in the DT ?

When I read earlier today "How about handle sram-size through match_data ?",
what I saw was a static C structure in the driver. Like what we do with HW
variants usually.

I was pretty convince that the right solution is to allocate sram when the
driver is used (open() seems appropriate), and drop it when its not used. With
the clear benefit that we can change our mind later if we find valid arguments.

So with that in mind, dropping (cleaning up) that old code seems helpful to
create a clean slate to re-introduce a better version.

Nicolas

Re: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

[sebastian.fricke]

Hey Nicolas,

On 04.04.2024 14:56, Nicolas Dufresne wrote:
>Hi,
>
>Le jeudi 04 avril 2024 à 10:02 +0200, sebastian.fricke@collabora.com a écrit :
>> > > > > > Wave5 can enable/disable the sec_axi_info option for each instance.
>> > > > > >
>> > > > > > How about handle sram-size through match_data ?
>> > > > > > I can find some drivers which use match_data to configure the sram
>> > > size.
>> >
>> > I proposed using match_data to allocate different sram size for each device.
>> > Do you think this is a reasonable approach ?
>>
>> As discussed here:
>> https://patchwork.linuxtv.org/project/linux-media/patch/20240201184238.2542695-1-b-brnich@ti.com/
>>
>> To quote Brandon Brnich from TI:
>>
>> If static SRAM allocation is the correct method to go, then this series
>> can be ignored and I will add section in device tree and remove check
>> for parameter in driver as that will now be a bug.
>>
>> I would like to adhere to that.
>
>I feel your statement is a bit ambiguous. Can you clarify what you adhere too ?
>That we should hardcode fixed sram size in the DT ?

Sorry wasn't aware that my statement is ambiguous, my intention was to
say that we do not add the sram size to the DT as it was discussed with
the DT maintainer in the thread above, my adherence was pointed towards
the statement from Brandon: "then this series can be ignored".

>
>When I read earlier today "How about handle sram-size through match_data ?",
>what I saw was a static C structure in the driver. Like what we do with HW
>variants usually.
>
>I was pretty convince that the right solution is to allocate sram when the
>driver is used (open() seems appropriate), and drop it when its not used. With
>the clear benefit that we can change our mind later if we find valid arguments.
>
>So with that in mind, dropping (cleaning up) that old code seems helpful to
>create a clean slate to re-introduce a better version.
>
>Nicolas

Greetings,
Sebastian

[PATCH v2 5/5] media: chips-media: wave5: support Wave515 decoder

[Ivan Bornyakov]

Add initial support for Wave515 multi-decoder IP. For now it is only
able to decode HEVC Main/Main10 profile videos.

This was tested on FPGA prototype, so wave5_dt_ids[] was not expanded.
Users of the real hardware with Wave515 IP will have to
 * provide firmware specific to their SoC
 * add struct wave5_match_data like this:

	static const struct wave5_match_data platform_name_wave515_data = {
		.flags = WAVE5_IS_DEC,
		.fw_name = "cnm/wave515_platform_name_fw.bin",
	};

 * add item to wave5_dt_ids[] like this:

	{
		.compatible = "vendor,soc-wave515",
		.data = &platform_name_wave515_data,
	},

 * describe new compatible in
   Documentation/devicetree/bindings/media/cnm,wave521c.yaml

Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
---
 .../platform/chips-media/wave5/wave5-helper.c |   3 +-
 .../platform/chips-media/wave5/wave5-hw.c     | 245 ++++++++++++++----
 .../chips-media/wave5/wave5-regdefine.h       |   5 +
 .../platform/chips-media/wave5/wave5-vdi.c    |   6 +-
 .../chips-media/wave5/wave5-vpu-dec.c         |  14 +-
 .../platform/chips-media/wave5/wave5-vpu.c    |   8 +-
 .../platform/chips-media/wave5/wave5-vpuapi.h |   1 +
 .../chips-media/wave5/wave5-vpuconfig.h       |   9 +-
 .../media/platform/chips-media/wave5/wave5.h  |   1 +
 9 files changed, 233 insertions(+), 59 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c b/drivers/media/platform/chips-media/wave5/wave5-helper.c
index 8433ecab230c..c68f1e110ed9 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
@@ -29,7 +29,8 @@ void wave5_cleanup_instance(struct vpu_instance *inst)
 {
 	int i;
 
-	if (list_is_singular(&inst->list))
+	if (list_is_singular(&inst->list) &&
+	    inst->dev->product_code != WAVE515_CODE)
 		wave5_vdi_free_sram(inst->dev);
 
 	for (i = 0; i < inst->fbc_buf_count; i++)
diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
index cdd0a0948a94..e38995de6870 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
@@ -24,8 +24,10 @@
 #define FEATURE_HEVC_ENCODER		BIT(0)
 
 /* Decoder support fields */
+#define W515_FEATURE_HEVC10BIT_DEC	BIT(1)
 #define FEATURE_AVC_DECODER		BIT(3)
 #define FEATURE_HEVC_DECODER		BIT(2)
+#define W515_FEATURE_HEVC_DECODER	BIT(0)
 
 #define FEATURE_BACKBONE		BIT(16)
 #define FEATURE_VCORE_BACKBONE		BIT(22)
@@ -155,6 +157,8 @@ 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 == WAVE515_CODE)
+		gdi_status_check_value = 0x0738;
 	if (vpu_dev->product_code == WAVE521C_CODE ||
 	    vpu_dev->product_code == WAVE521_CODE ||
 	    vpu_dev->product_code == WAVE521E1_CODE)
@@ -186,6 +190,8 @@ unsigned int wave5_vpu_get_product_id(struct vpu_device *vpu_dev)
 	u32 val = vpu_read_reg(vpu_dev, W5_PRODUCT_NUMBER);
 
 	switch (val) {
+	case WAVE515_CODE:
+		return PRODUCT_ID_515;
 	case WAVE521C_CODE:
 		return PRODUCT_ID_521;
 	case WAVE521_CODE:
@@ -349,17 +355,33 @@ static int setup_wave5_properties(struct device *dev)
 	hw_config_def1 = vpu_read_reg(vpu_dev, W5_RET_STD_DEF1);
 	hw_config_feature = vpu_read_reg(vpu_dev, W5_RET_CONF_FEATURE);
 
-	p_attr->support_hevc10bit_enc = FIELD_GET(FEATURE_HEVC10BIT_ENC, hw_config_feature);
-	p_attr->support_avc10bit_enc = FIELD_GET(FEATURE_AVC10BIT_ENC, hw_config_feature);
-
-	p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER, hw_config_def1) << STD_AVC;
-	p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER, hw_config_def1) << STD_HEVC;
-	p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER, hw_config_def1) << STD_AVC;
-	p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER, hw_config_def1) << STD_HEVC;
-
-	p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE, hw_config_def0);
-	p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE, hw_config_def0);
-	p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE, hw_config_def0);
+	if (vpu_dev->product_code == WAVE515_CODE) {
+		p_attr->support_hevc10bit_dec = FIELD_GET(W515_FEATURE_HEVC10BIT_DEC,
+							  hw_config_feature);
+		p_attr->support_decoders = FIELD_GET(W515_FEATURE_HEVC_DECODER,
+						     hw_config_def1) << STD_HEVC;
+	} else {
+		p_attr->support_hevc10bit_enc = FIELD_GET(FEATURE_HEVC10BIT_ENC,
+							  hw_config_feature);
+		p_attr->support_avc10bit_enc = FIELD_GET(FEATURE_AVC10BIT_ENC,
+							 hw_config_feature);
+
+		p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
+						     hw_config_def1) << STD_AVC;
+		p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
+						      hw_config_def1) << STD_HEVC;
+		p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
+						     hw_config_def1) << STD_AVC;
+		p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
+						      hw_config_def1) << STD_HEVC;
+
+		p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
+						     hw_config_def0);
+		p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE,
+							  hw_config_def0);
+		p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE,
+							   hw_config_def0);
+	}
 
 	setup_wave5_interrupts(vpu_dev);
 
@@ -403,12 +425,18 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
 	common_vb = &vpu_dev->common_mem;
 
 	code_base = common_vb->daddr;
+
+	if (vpu_dev->product_code == WAVE515_CODE)
+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
+	else
+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
+
 	/* ALIGN TO 4KB */
-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
+	code_size &= ~0xfff;
 	if (code_size < size * 2)
 		return -EINVAL;
 
-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
+	temp_base = code_base + code_size;
 	temp_size = WAVE5_TEMPBUF_SIZE;
 
 	ret = wave5_vdi_write_memory(vpu_dev, common_vb, 0, fw, size);
@@ -436,9 +464,12 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
 
 	/* These register must be reset explicitly */
 	vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
-	wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
-	wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
-	vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
+
+	if (vpu_dev->product_code != WAVE515_CODE) {
+		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
+		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
+		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
+	}
 
 	reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
 	if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
@@ -453,6 +484,24 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
 		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
 	}
 
+	if (vpu_dev->product_code == WAVE515_CODE) {
+		dma_addr_t task_buf_base;
+
+		vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF, WAVE515_COMMAND_QUEUE_DEPTH);
+		vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE, WAVE515_ONE_TASKBUF_SIZE);
+
+		for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
+			task_buf_base = temp_base + temp_size +
+					(i * WAVE515_ONE_TASKBUF_SIZE);
+			vpu_write_reg(vpu_dev,
+				      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
+				      task_buf_base);
+		}
+
+		vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI, vpu_dev->sram_buf.daddr);
+		vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE, vpu_dev->sram_buf.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);
@@ -493,29 +542,39 @@ int wave5_vpu_build_up_dec_param(struct vpu_instance *inst,
 		return -EINVAL;
 	}
 
-	p_dec_info->vb_work.size = WAVE521DEC_WORKBUF_SIZE;
+	if (vpu_dev->product == PRODUCT_ID_515)
+		p_dec_info->vb_work.size = WAVE515DEC_WORKBUF_SIZE;
+	else
+		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);
+	if (inst->dev->product_code != WAVE515_CODE)
+		vpu_write_reg(inst->dev, W5_CMD_DEC_VCORE_INFO, 1);
 
 	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_ADDR_SEC_AXI, vpu_dev->sram_buf.daddr);
-	vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev->sram_buf.size);
+	if (inst->dev->product_code != WAVE515_CODE) {
+		vpu_write_reg(inst->dev, W5_CMD_ADDR_SEC_AXI, vpu_dev->sram_buf.daddr);
+		vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev->sram_buf.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: SDMA reads MSB first */
 	vpu_write_reg(inst->dev, W5_CMD_BS_PARAM, BITSTREAM_ENDIANNESS_BIG_ENDIAN);
-	/* This register must be reset explicitly */
-	vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
-	vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1, (COMMAND_QUEUE_DEPTH - 1));
+
+	if (inst->dev->product_code != WAVE515_CODE) {
+		/* This register must be reset explicitly */
+		vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
+		vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1, (COMMAND_QUEUE_DEPTH - 1));
+	}
 
 	ret = send_firmware_command(inst, W5_CREATE_INSTANCE, true, NULL, NULL);
 	if (ret) {
@@ -566,7 +625,7 @@ static u32 get_bitstream_options(struct dec_info *info)
 int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
 {
 	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
-	u32 cmd_option = INIT_SEQ_NORMAL;
+	u32 bs_option, cmd_option = INIT_SEQ_NORMAL;
 	u32 reg_val, fail_res;
 	int ret;
 
@@ -576,7 +635,12 @@ int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
 	vpu_write_reg(inst->dev, W5_BS_RD_PTR, p_dec_info->stream_rd_ptr);
 	vpu_write_reg(inst->dev, W5_BS_WR_PTR, p_dec_info->stream_wr_ptr);
 
-	vpu_write_reg(inst->dev, W5_BS_OPTION, get_bitstream_options(p_dec_info));
+	bs_option = get_bitstream_options(p_dec_info);
+
+	if (inst->dev->product_code == WAVE515_CODE)
+		bs_option |= BSOPTION_RD_PTR_VALID_FLAG;
+
+	vpu_write_reg(inst->dev, W5_BS_OPTION, bs_option);
 
 	vpu_write_reg(inst->dev, W5_COMMAND_OPTION, cmd_option);
 	vpu_write_reg(inst->dev, W5_CMD_DEC_USER_MASK, p_dec_info->user_data_enable);
@@ -642,10 +706,12 @@ static void wave5_get_dec_seq_result(struct vpu_instance *inst, struct dec_initi
 		info->profile = FIELD_GET(SEQ_PARAM_PROFILE_MASK, reg_val);
 	}
 
-	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;
+	if (inst->dev->product_code != WAVE515_CODE) {
+		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;
+	}
 }
 
 int wave5_vpu_dec_get_seq_info(struct vpu_instance *inst, struct dec_initial_info *info)
@@ -747,22 +813,27 @@ int wave5_vpu_dec_register_framebuffer(struct vpu_instance *inst, struct frame_b
 
 		pic_size = (init_info->pic_width << 16) | (init_info->pic_height);
 
-		vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM) +
+		if (inst->dev->product_code != WAVE515_CODE) {
+			vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM) +
 				(p_dec_info->param_buf_size * COMMAND_QUEUE_DEPTH);
-		vb_buf.daddr = 0;
+			vb_buf.daddr = 0;
 
-		if (vb_buf.size != p_dec_info->vb_task.size) {
-			wave5_vdi_free_dma_memory(inst->dev, &p_dec_info->vb_task);
-			ret = wave5_vdi_allocate_dma_memory(inst->dev, &vb_buf);
-			if (ret)
-				goto free_fbc_c_tbl_buffers;
+			if (vb_buf.size != p_dec_info->vb_task.size) {
+				wave5_vdi_free_dma_memory(inst->dev,
+							  &p_dec_info->vb_task);
+				ret = wave5_vdi_allocate_dma_memory(inst->dev,
+								    &vb_buf);
+				if (ret)
+					goto free_fbc_c_tbl_buffers;
 
-			p_dec_info->vb_task = vb_buf;
-		}
+				p_dec_info->vb_task = vb_buf;
+			}
 
-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
-			      p_dec_info->vb_task.daddr);
-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE, vb_buf.size);
+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
+				      p_dec_info->vb_task.daddr);
+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
+				      vb_buf.size);
+		}
 	} else {
 		pic_size = (init_info->pic_width << 16) | (init_info->pic_height);
 
@@ -999,17 +1070,24 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
 	dma_addr_t code_base, temp_base;
 	dma_addr_t old_code_base, temp_size;
 	u32 code_size, reason_code;
-	u32 reg_val;
+	u32 i, reg_val;
 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
 
 	common_vb = &vpu_dev->common_mem;
 
 	code_base = common_vb->daddr;
+
+	if (vpu_dev->product_code == WAVE515_CODE)
+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
+	else
+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
+
 	/* ALIGN TO 4KB */
-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
+	code_size &= ~0xfff;
 	if (code_size < size * 2)
 		return -EINVAL;
-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
+
+	temp_base = code_base + code_size;
 	temp_size = WAVE5_TEMPBUF_SIZE;
 
 	old_code_base = vpu_read_reg(vpu_dev, W5_VPU_REMAP_PADDR);
@@ -1043,9 +1121,12 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
 
 		/* These register must be reset explicitly */
 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
+
+		if (vpu_dev->product_code != WAVE515_CODE) {
+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
+		}
 
 		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
 		if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
@@ -1060,6 +1141,28 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
 		}
 
+		if (vpu_dev->product_code == WAVE515_CODE) {
+			dma_addr_t task_buf_base;
+
+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
+				      WAVE515_COMMAND_QUEUE_DEPTH);
+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
+				      WAVE515_ONE_TASKBUF_SIZE);
+
+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
+				task_buf_base = temp_base + temp_size +
+						(i * WAVE515_ONE_TASKBUF_SIZE);
+				vpu_write_reg(vpu_dev,
+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
+					      task_buf_base);
+			}
+
+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
+				      vpu_dev->sram_buf.daddr);
+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
+				      vpu_dev->sram_buf.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);
@@ -1081,10 +1184,10 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
 static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
 				size_t size)
 {
-	u32 reg_val;
+	u32 i, reg_val;
 	struct vpu_buf *common_vb;
-	dma_addr_t code_base;
-	u32 code_size, reason_code;
+	dma_addr_t code_base, temp_base;
+	u32 code_size, temp_size, reason_code;
 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
 	int ret;
 
@@ -1114,13 +1217,22 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
 		common_vb = &vpu_dev->common_mem;
 
 		code_base = common_vb->daddr;
+
+		if (vpu_dev->product_code == WAVE515_CODE)
+			code_size = WAVE515_MAX_CODE_BUF_SIZE;
+		else
+			code_size = WAVE5_MAX_CODE_BUF_SIZE;
+
 		/* ALIGN TO 4KB */
-		code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
+		code_size &= ~0xfff;
 		if (code_size < size * 2) {
 			dev_err(dev, "size too small\n");
 			return -EINVAL;
 		}
 
+		temp_base = code_base + code_size;
+		temp_size = WAVE5_TEMPBUF_SIZE;
+
 		/* Power on without DEBUG mode */
 		vpu_write_reg(vpu_dev, W5_PO_CONF, 0);
 
@@ -1133,9 +1245,12 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
 
 		/* These register must be reset explicitly */
 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
+
+		if (vpu_dev->product_code != WAVE515_CODE) {
+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
+		}
 
 		setup_wave5_interrupts(vpu_dev);
 
@@ -1152,6 +1267,28 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
 		}
 
+		if (vpu_dev->product_code == WAVE515_CODE) {
+			dma_addr_t task_buf_base;
+
+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
+				      WAVE515_COMMAND_QUEUE_DEPTH);
+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
+				      WAVE515_ONE_TASKBUF_SIZE);
+
+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
+				task_buf_base = temp_base + temp_size +
+						(i * WAVE515_ONE_TASKBUF_SIZE);
+				vpu_write_reg(vpu_dev,
+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
+					      task_buf_base);
+			}
+
+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
+				      vpu_dev->sram_buf.daddr);
+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
+				      vpu_dev->sram_buf.size);
+		}
+
 		vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
 		vpu_write_reg(vpu_dev, W5_COMMAND, W5_WAKEUP_VPU);
 		/* Start VPU after settings */
diff --git a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
index a15c6b2c3d8b..557344754c4c 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
@@ -205,6 +205,9 @@ enum query_opt {
 #define W5_ADDR_TEMP_BASE                       (W5_REG_BASE + 0x011C)
 #define W5_TEMP_SIZE                            (W5_REG_BASE + 0x0120)
 #define W5_HW_OPTION                            (W5_REG_BASE + 0x012C)
+#define W5_CMD_INIT_NUM_TASK_BUF		(W5_REG_BASE + 0x0134)
+#define W5_CMD_INIT_ADDR_TASK_BUF0		(W5_REG_BASE + 0x0138)
+#define W5_CMD_INIT_TASK_BUF_SIZE		(W5_REG_BASE + 0x0178)
 #define W5_SEC_AXI_PARAM                        (W5_REG_BASE + 0x0180)
 
 /************************************************************************/
@@ -216,7 +219,9 @@ enum query_opt {
 #define W5_CMD_DEC_BS_SIZE                      (W5_REG_BASE + 0x0120)
 #define W5_CMD_BS_PARAM                         (W5_REG_BASE + 0x0124)
 #define W5_CMD_ADDR_SEC_AXI                     (W5_REG_BASE + 0x0130)
+#define W515_CMD_ADDR_SEC_AXI			(W5_REG_BASE + 0x0124)
 #define W5_CMD_SEC_AXI_SIZE                     (W5_REG_BASE + 0x0134)
+#define W515_CMD_SEC_AXI_SIZE			(W5_REG_BASE + 0x0128)
 #define W5_CMD_EXT_ADDR                         (W5_REG_BASE + 0x0138)
 #define W5_CMD_NUM_CQ_DEPTH_M1                  (W5_REG_BASE + 0x013C)
 #define W5_CMD_ERR_CONCEAL                      (W5_REG_BASE + 0x0140)
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
index a63fffed55e9..78888c57b6da 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
@@ -18,7 +18,11 @@ static int wave5_vdi_allocate_common_memory(struct device *dev)
 	if (!vpu_dev->common_mem.vaddr) {
 		int ret;
 
-		vpu_dev->common_mem.size = SIZE_COMMON;
+		if (vpu_dev->product_code == WAVE515_CODE)
+			vpu_dev->common_mem.size = WAVE515_SIZE_COMMON;
+		else
+			vpu_dev->common_mem.size = SIZE_COMMON;
+
 		ret = wave5_vdi_allocate_dma_memory(vpu_dev, &vpu_dev->common_mem);
 		if (ret) {
 			dev_err(dev, "unable to allocate common buffer\n");
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 5a71a711f2e8..65a99053c5e8 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -1869,7 +1869,8 @@ static int wave5_vpu_open_dec(struct file *filp)
 		goto cleanup_inst;
 	}
 
-	wave5_vdi_allocate_sram(inst->dev);
+	if (inst->dev->product_code != WAVE515_CODE)
+		wave5_vdi_allocate_sram(inst->dev);
 
 	return 0;
 
@@ -1897,6 +1898,14 @@ int wave5_vpu_dec_register_device(struct vpu_device *dev)
 	struct video_device *vdev_dec;
 	int ret;
 
+	/*
+	 * secondary AXI setup for Wave515 is done by INIT_VPU command,
+	 * that's why SRAM memory is allocated at VPU device register
+	 * rather than at device open.
+	 */
+	if (dev->product_code == WAVE515_CODE)
+		wave5_vdi_allocate_sram(dev);
+
 	vdev_dec = devm_kzalloc(dev->v4l2_dev.dev, sizeof(*vdev_dec), GFP_KERNEL);
 	if (!vdev_dec)
 		return -ENOMEM;
@@ -1930,6 +1939,9 @@ int wave5_vpu_dec_register_device(struct vpu_device *dev)
 
 void wave5_vpu_dec_unregister_device(struct vpu_device *dev)
 {
+	if (dev->product_code == WAVE515_CODE)
+		wave5_vdi_free_sram(dev);
+
 	video_unregister_device(dev->video_dev_dec);
 	if (dev->v4l2_m2m_dec_dev)
 		v4l2_m2m_release(dev->v4l2_m2m_dec_dev);
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 2a972cddf4a6..fc267348399e 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -60,7 +60,13 @@ static irqreturn_t wave5_vpu_irq_thread(int irq, void *dev_id)
 
 			if (irq_reason & BIT(INT_WAVE5_INIT_SEQ) ||
 			    irq_reason & BIT(INT_WAVE5_ENC_SET_PARAM)) {
-				if (seq_done & BIT(inst->id)) {
+				if ((dev->product_code == WAVE515_CODE) &&
+				    (cmd_done & BIT(inst->id))) {
+					cmd_done &= ~BIT(inst->id);
+					wave5_vdi_write_register(dev, W5_RET_QUEUE_CMD_DONE_INST,
+								 cmd_done);
+					complete(&inst->irq_done);
+				} else if (seq_done & BIT(inst->id)) {
 					seq_done &= ~BIT(inst->id);
 					wave5_vdi_write_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO,
 								 seq_done);
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
index 975d96b22191..601205df4433 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
@@ -18,6 +18,7 @@
 #include "wave5-vdi.h"
 
 enum product_id {
+	PRODUCT_ID_515,
 	PRODUCT_ID_521,
 	PRODUCT_ID_511,
 	PRODUCT_ID_517,
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
index 9d99afb78c89..b4128524fcfd 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
@@ -8,6 +8,7 @@
 #ifndef _VPU_CONFIG_H_
 #define _VPU_CONFIG_H_
 
+#define WAVE515_CODE			0x5150
 #define WAVE517_CODE                    0x5170
 #define WAVE537_CODE                    0x5370
 #define WAVE511_CODE                    0x5110
@@ -21,12 +22,13 @@
 		((c) == WAVE517_CODE ||	(c) == WAVE537_CODE ||		\
 		 (c) == WAVE511_CODE || (c) == WAVE521_CODE ||		\
 		 (c) == WAVE521E1_CODE || (c) == WAVE521C_CODE ||	\
-		 (c) == WAVE521C_DUAL_CODE);				\
+		 (c) == WAVE521C_DUAL_CODE) || (c) == WAVE515_CODE;	\
 })
 
 #define WAVE517_WORKBUF_SIZE            (2 * 1024 * 1024)
 #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC 40K
 #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
+#define WAVE515DEC_WORKBUF_SIZE		(2 * 1024 * 1024)
 
 #define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
 
@@ -52,16 +54,21 @@
 #define VLC_BUF_NUM                     (2)
 
 #define COMMAND_QUEUE_DEPTH             (2)
+#define WAVE515_COMMAND_QUEUE_DEPTH	(4)
 
 #define W5_REMAP_INDEX0                 0
 #define W5_REMAP_INDEX1                 1
 #define W5_REMAP_MAX_SIZE               (1024 * 1024)
 
 #define WAVE5_MAX_CODE_BUF_SIZE         (2 * 1024 * 1024)
+#define WAVE515_MAX_CODE_BUF_SIZE	(1024 * 1024)
 #define WAVE5_TEMPBUF_OFFSET            WAVE5_MAX_CODE_BUF_SIZE
 #define WAVE5_TEMPBUF_SIZE              (1024 * 1024)
+#define WAVE515_TASKBUF_OFFSET		(WAVE515_MAX_CODE_BUF_SIZE + WAVE5_TEMPBUF_SIZE)
 
 #define SIZE_COMMON                 (WAVE5_MAX_CODE_BUF_SIZE + WAVE5_TEMPBUF_SIZE)
+#define WAVE515_ONE_TASKBUF_SIZE	(8 * 1024 * 1024)
+#define WAVE515_SIZE_COMMON	(WAVE515_TASKBUF_OFFSET + WAVE515_COMMAND_QUEUE_DEPTH * WAVE515_ONE_TASKBUF_SIZE)
 
 //=====4. VPU REPORT MEMORY  ======================//
 
diff --git a/drivers/media/platform/chips-media/wave5/wave5.h b/drivers/media/platform/chips-media/wave5/wave5.h
index 063028eccd3b..57b00e182b6e 100644
--- a/drivers/media/platform/chips-media/wave5/wave5.h
+++ b/drivers/media/platform/chips-media/wave5/wave5.h
@@ -22,6 +22,7 @@
  */
 #define BSOPTION_ENABLE_EXPLICIT_END		BIT(0)
 #define BSOPTION_HIGHLIGHT_STREAM_END		BIT(1)
+#define BSOPTION_RD_PTR_VALID_FLAG		BIT(31)
 
 /*
  * Currently the driver only supports hardware with little endian but for source
-- 
2.44.0

RE: [PATCH v2 5/5] media: chips-media: wave5: support Wave515 decoder

[Nas Chung]

Hi, Ivan.

>-----Original Message-----
>From: Ivan Bornyakov <brnkv.i1@gmail.com>
>Sent: Monday, March 25, 2024 3:41 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
>Philipp Zabel <p.zabel@pengutronix.de>
>Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Subject: [PATCH v2 5/5] media: chips-media: wave5: support Wave515
>decoder
>
>Add initial support for Wave515 multi-decoder IP. For now it is only
>able to decode HEVC Main/Main10 profile videos.

Thanks for your updates.
Could you share some test result for HEVC decoding ? (ex. Fluster test)
As you know, wave515 can support VP9 and AVS2.
Did you have any chance to test VP9 or AVS2 streams ?

Thanks.
Nas.

>
>This was tested on FPGA prototype, so wave5_dt_ids[] was not expanded.
>Users of the real hardware with Wave515 IP will have to
> * provide firmware specific to their SoC
> * add struct wave5_match_data like this:
>
>	static const struct wave5_match_data platform_name_wave515_data = {
>		.flags = WAVE5_IS_DEC,
>		.fw_name = "cnm/wave515_platform_name_fw.bin",
>	};
>
> * add item to wave5_dt_ids[] like this:
>
>	{
>		.compatible = "vendor,soc-wave515",
>		.data = &platform_name_wave515_data,
>	},
>
> * describe new compatible in
>   Documentation/devicetree/bindings/media/cnm,wave521c.yaml
>
>Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>---
> .../platform/chips-media/wave5/wave5-helper.c |   3 +-
> .../platform/chips-media/wave5/wave5-hw.c     | 245 ++++++++++++++----
> .../chips-media/wave5/wave5-regdefine.h       |   5 +
> .../platform/chips-media/wave5/wave5-vdi.c    |   6 +-
> .../chips-media/wave5/wave5-vpu-dec.c         |  14 +-
> .../platform/chips-media/wave5/wave5-vpu.c    |   8 +-
> .../platform/chips-media/wave5/wave5-vpuapi.h |   1 +
> .../chips-media/wave5/wave5-vpuconfig.h       |   9 +-
> .../media/platform/chips-media/wave5/wave5.h  |   1 +
> 9 files changed, 233 insertions(+), 59 deletions(-)
>
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c
>b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>index 8433ecab230c..c68f1e110ed9 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>@@ -29,7 +29,8 @@ void wave5_cleanup_instance(struct vpu_instance *inst)
> {
> 	int i;
>
>-	if (list_is_singular(&inst->list))
>+	if (list_is_singular(&inst->list) &&
>+	    inst->dev->product_code != WAVE515_CODE)
> 		wave5_vdi_free_sram(inst->dev);
>
> 	for (i = 0; i < inst->fbc_buf_count; i++)
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>index cdd0a0948a94..e38995de6870 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>@@ -24,8 +24,10 @@
> #define FEATURE_HEVC_ENCODER		BIT(0)
>
> /* Decoder support fields */
>+#define W515_FEATURE_HEVC10BIT_DEC	BIT(1)
> #define FEATURE_AVC_DECODER		BIT(3)
> #define FEATURE_HEVC_DECODER		BIT(2)
>+#define W515_FEATURE_HEVC_DECODER	BIT(0)
>
> #define FEATURE_BACKBONE		BIT(16)
> #define FEATURE_VCORE_BACKBONE		BIT(22)
>@@ -155,6 +157,8 @@ 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 == WAVE515_CODE)
>+		gdi_status_check_value = 0x0738;
> 	if (vpu_dev->product_code == WAVE521C_CODE ||
> 	    vpu_dev->product_code == WAVE521_CODE ||
> 	    vpu_dev->product_code == WAVE521E1_CODE)
>@@ -186,6 +190,8 @@ unsigned int wave5_vpu_get_product_id(struct
>vpu_device *vpu_dev)
> 	u32 val = vpu_read_reg(vpu_dev, W5_PRODUCT_NUMBER);
>
> 	switch (val) {
>+	case WAVE515_CODE:
>+		return PRODUCT_ID_515;
> 	case WAVE521C_CODE:
> 		return PRODUCT_ID_521;
> 	case WAVE521_CODE:
>@@ -349,17 +355,33 @@ static int setup_wave5_properties(struct device
>*dev)
> 	hw_config_def1 = vpu_read_reg(vpu_dev, W5_RET_STD_DEF1);
> 	hw_config_feature = vpu_read_reg(vpu_dev, W5_RET_CONF_FEATURE);
>
>-	p_attr->support_hevc10bit_enc = FIELD_GET(FEATURE_HEVC10BIT_ENC,
>hw_config_feature);
>-	p_attr->support_avc10bit_enc = FIELD_GET(FEATURE_AVC10BIT_ENC,
>hw_config_feature);
>-
>-	p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
>hw_config_def1) << STD_AVC;
>-	p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
>hw_config_def1) << STD_HEVC;
>-	p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
>hw_config_def1) << STD_AVC;
>-	p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
>hw_config_def1) << STD_HEVC;
>-
>-	p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
>hw_config_def0);
>-	p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE,
>hw_config_def0);
>-	p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE,
>hw_config_def0);
>+	if (vpu_dev->product_code == WAVE515_CODE) {
>+		p_attr->support_hevc10bit_dec =
>FIELD_GET(W515_FEATURE_HEVC10BIT_DEC,
>+							  hw_config_feature);
>+		p_attr->support_decoders =
>FIELD_GET(W515_FEATURE_HEVC_DECODER,
>+						     hw_config_def1) << STD_HEVC;
>+	} else {
>+		p_attr->support_hevc10bit_enc =
>FIELD_GET(FEATURE_HEVC10BIT_ENC,
>+							  hw_config_feature);
>+		p_attr->support_avc10bit_enc =
>FIELD_GET(FEATURE_AVC10BIT_ENC,
>+							 hw_config_feature);
>+
>+		p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
>+						     hw_config_def1) << STD_AVC;
>+		p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
>+						      hw_config_def1) << STD_HEVC;
>+		p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
>+						     hw_config_def1) << STD_AVC;
>+		p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
>+						      hw_config_def1) << STD_HEVC;
>+
>+		p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
>+						     hw_config_def0);
>+		p_attr->support_vcpu_backbone =
>FIELD_GET(FEATURE_VCPU_BACKBONE,
>+							  hw_config_def0);
>+		p_attr->support_vcore_backbone =
>FIELD_GET(FEATURE_VCORE_BACKBONE,
>+							   hw_config_def0);
>+	}
>
> 	setup_wave5_interrupts(vpu_dev);
>
>@@ -403,12 +425,18 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
>size_t size)
> 	common_vb = &vpu_dev->common_mem;
>
> 	code_base = common_vb->daddr;
>+
>+	if (vpu_dev->product_code == WAVE515_CODE)
>+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
>+	else
>+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
>+
> 	/* ALIGN TO 4KB */
>-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>+	code_size &= ~0xfff;
> 	if (code_size < size * 2)
> 		return -EINVAL;
>
>-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
>+	temp_base = code_base + code_size;
> 	temp_size = WAVE5_TEMPBUF_SIZE;
>
> 	ret = wave5_vdi_write_memory(vpu_dev, common_vb, 0, fw, size);
>@@ -436,9 +464,12 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
>size_t size)
>
> 	/* These register must be reset explicitly */
> 	vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>-	wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>-	wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>-	vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+
>+	if (vpu_dev->product_code != WAVE515_CODE) {
>+		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>+		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>+		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+	}
>
> 	reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> 	if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
>@@ -453,6 +484,24 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
>size_t size)
> 		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
> 	}
>
>+	if (vpu_dev->product_code == WAVE515_CODE) {
>+		dma_addr_t task_buf_base;
>+
>+		vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>WAVE515_COMMAND_QUEUE_DEPTH);
>+		vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>WAVE515_ONE_TASKBUF_SIZE);
>+
>+		for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>+			task_buf_base = temp_base + temp_size +
>+					(i * WAVE515_ONE_TASKBUF_SIZE);
>+			vpu_write_reg(vpu_dev,
>+				      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>+				      task_buf_base);
>+		}
>+
>+		vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI, vpu_dev-
>>sram_buf.daddr);
>+		vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE, vpu_dev-
>>sram_buf.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);
>@@ -493,29 +542,39 @@ int wave5_vpu_build_up_dec_param(struct
>vpu_instance *inst,
> 		return -EINVAL;
> 	}
>
>-	p_dec_info->vb_work.size = WAVE521DEC_WORKBUF_SIZE;
>+	if (vpu_dev->product == PRODUCT_ID_515)
>+		p_dec_info->vb_work.size = WAVE515DEC_WORKBUF_SIZE;
>+	else
>+		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);
>+	if (inst->dev->product_code != WAVE515_CODE)
>+		vpu_write_reg(inst->dev, W5_CMD_DEC_VCORE_INFO, 1);
>
> 	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_ADDR_SEC_AXI, vpu_dev-
>>sram_buf.daddr);
>-	vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev-
>>sram_buf.size);
>+	if (inst->dev->product_code != WAVE515_CODE) {
>+		vpu_write_reg(inst->dev, W5_CMD_ADDR_SEC_AXI, vpu_dev-
>>sram_buf.daddr);
>+		vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev-
>>sram_buf.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: SDMA reads MSB first */
> 	vpu_write_reg(inst->dev, W5_CMD_BS_PARAM,
>BITSTREAM_ENDIANNESS_BIG_ENDIAN);
>-	/* This register must be reset explicitly */
>-	vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
>-	vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1,
>(COMMAND_QUEUE_DEPTH - 1));
>+
>+	if (inst->dev->product_code != WAVE515_CODE) {
>+		/* This register must be reset explicitly */
>+		vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
>+		vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1,
>(COMMAND_QUEUE_DEPTH - 1));
>+	}
>
> 	ret = send_firmware_command(inst, W5_CREATE_INSTANCE, true, NULL,
>NULL);
> 	if (ret) {
>@@ -566,7 +625,7 @@ static u32 get_bitstream_options(struct dec_info
>*info)
> int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
> {
> 	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
>-	u32 cmd_option = INIT_SEQ_NORMAL;
>+	u32 bs_option, cmd_option = INIT_SEQ_NORMAL;
> 	u32 reg_val, fail_res;
> 	int ret;
>
>@@ -576,7 +635,12 @@ int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
> 	vpu_write_reg(inst->dev, W5_BS_RD_PTR, p_dec_info->stream_rd_ptr);
> 	vpu_write_reg(inst->dev, W5_BS_WR_PTR, p_dec_info->stream_wr_ptr);
>
>-	vpu_write_reg(inst->dev, W5_BS_OPTION,
>get_bitstream_options(p_dec_info));
>+	bs_option = get_bitstream_options(p_dec_info);
>+
>+	if (inst->dev->product_code == WAVE515_CODE)
>+		bs_option |= BSOPTION_RD_PTR_VALID_FLAG;
>+
>+	vpu_write_reg(inst->dev, W5_BS_OPTION, bs_option);
>
> 	vpu_write_reg(inst->dev, W5_COMMAND_OPTION, cmd_option);
> 	vpu_write_reg(inst->dev, W5_CMD_DEC_USER_MASK, p_dec_info-
>>user_data_enable);
>@@ -642,10 +706,12 @@ static void wave5_get_dec_seq_result(struct
>vpu_instance *inst, struct dec_initi
> 		info->profile = FIELD_GET(SEQ_PARAM_PROFILE_MASK, reg_val);
> 	}
>
>-	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;
>+	if (inst->dev->product_code != WAVE515_CODE) {
>+		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;
>+	}
> }
>
> int wave5_vpu_dec_get_seq_info(struct vpu_instance *inst, struct
>dec_initial_info *info)
>@@ -747,22 +813,27 @@ int wave5_vpu_dec_register_framebuffer(struct
>vpu_instance *inst, struct frame_b
>
> 		pic_size = (init_info->pic_width << 16) | (init_info-
>>pic_height);
>
>-		vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM) +
>+		if (inst->dev->product_code != WAVE515_CODE) {
>+			vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM)
>+
> 				(p_dec_info->param_buf_size *
>COMMAND_QUEUE_DEPTH);
>-		vb_buf.daddr = 0;
>+			vb_buf.daddr = 0;
>
>-		if (vb_buf.size != p_dec_info->vb_task.size) {
>-			wave5_vdi_free_dma_memory(inst->dev, &p_dec_info-
>>vb_task);
>-			ret = wave5_vdi_allocate_dma_memory(inst->dev,
>&vb_buf);
>-			if (ret)
>-				goto free_fbc_c_tbl_buffers;
>+			if (vb_buf.size != p_dec_info->vb_task.size) {
>+				wave5_vdi_free_dma_memory(inst->dev,
>+							  &p_dec_info->vb_task);
>+				ret = wave5_vdi_allocate_dma_memory(inst->dev,
>+								    &vb_buf);
>+				if (ret)
>+					goto free_fbc_c_tbl_buffers;
>
>-			p_dec_info->vb_task = vb_buf;
>-		}
>+				p_dec_info->vb_task = vb_buf;
>+			}
>
>-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
>-			      p_dec_info->vb_task.daddr);
>-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
>vb_buf.size);
>+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
>+				      p_dec_info->vb_task.daddr);
>+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
>+				      vb_buf.size);
>+		}
> 	} else {
> 		pic_size = (init_info->pic_width << 16) | (init_info-
>>pic_height);
>
>@@ -999,17 +1070,24 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
> 	dma_addr_t code_base, temp_base;
> 	dma_addr_t old_code_base, temp_size;
> 	u32 code_size, reason_code;
>-	u32 reg_val;
>+	u32 i, reg_val;
> 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
>
> 	common_vb = &vpu_dev->common_mem;
>
> 	code_base = common_vb->daddr;
>+
>+	if (vpu_dev->product_code == WAVE515_CODE)
>+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
>+	else
>+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
>+
> 	/* ALIGN TO 4KB */
>-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>+	code_size &= ~0xfff;
> 	if (code_size < size * 2)
> 		return -EINVAL;
>-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
>+
>+	temp_base = code_base + code_size;
> 	temp_size = WAVE5_TEMPBUF_SIZE;
>
> 	old_code_base = vpu_read_reg(vpu_dev, W5_VPU_REMAP_PADDR);
>@@ -1043,9 +1121,12 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
>
> 		/* These register must be reset explicitly */
> 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+
>+		if (vpu_dev->product_code != WAVE515_CODE) {
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR,
>0);
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+		}
>
> 		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> 		if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
>@@ -1060,6 +1141,28 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
> 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID,
>reg_val);
> 		}
>
>+		if (vpu_dev->product_code == WAVE515_CODE) {
>+			dma_addr_t task_buf_base;
>+
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>+				      WAVE515_COMMAND_QUEUE_DEPTH);
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>+				      WAVE515_ONE_TASKBUF_SIZE);
>+
>+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>+				task_buf_base = temp_base + temp_size +
>+						(i * WAVE515_ONE_TASKBUF_SIZE);
>+				vpu_write_reg(vpu_dev,
>+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>+					      task_buf_base);
>+			}
>+
>+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
>+				      vpu_dev->sram_buf.daddr);
>+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
>+				      vpu_dev->sram_buf.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);
>@@ -1081,10 +1184,10 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
> static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake,
>const uint16_t *code,
> 				size_t size)
> {
>-	u32 reg_val;
>+	u32 i, reg_val;
> 	struct vpu_buf *common_vb;
>-	dma_addr_t code_base;
>-	u32 code_size, reason_code;
>+	dma_addr_t code_base, temp_base;
>+	u32 code_size, temp_size, reason_code;
> 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
> 	int ret;
>
>@@ -1114,13 +1217,22 @@ static int wave5_vpu_sleep_wake(struct device
>*dev, bool i_sleep_wake, const uin
> 		common_vb = &vpu_dev->common_mem;
>
> 		code_base = common_vb->daddr;
>+
>+		if (vpu_dev->product_code == WAVE515_CODE)
>+			code_size = WAVE515_MAX_CODE_BUF_SIZE;
>+		else
>+			code_size = WAVE5_MAX_CODE_BUF_SIZE;
>+
> 		/* ALIGN TO 4KB */
>-		code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>+		code_size &= ~0xfff;
> 		if (code_size < size * 2) {
> 			dev_err(dev, "size too small\n");
> 			return -EINVAL;
> 		}
>
>+		temp_base = code_base + code_size;
>+		temp_size = WAVE5_TEMPBUF_SIZE;
>+
> 		/* Power on without DEBUG mode */
> 		vpu_write_reg(vpu_dev, W5_PO_CONF, 0);
>
>@@ -1133,9 +1245,12 @@ static int wave5_vpu_sleep_wake(struct device *dev,
>bool i_sleep_wake, const uin
>
> 		/* These register must be reset explicitly */
> 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+
>+		if (vpu_dev->product_code != WAVE515_CODE) {
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR,
>0);
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+		}
>
> 		setup_wave5_interrupts(vpu_dev);
>
>@@ -1152,6 +1267,28 @@ static int wave5_vpu_sleep_wake(struct device *dev,
>bool i_sleep_wake, const uin
> 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID,
>reg_val);
> 		}
>
>+		if (vpu_dev->product_code == WAVE515_CODE) {
>+			dma_addr_t task_buf_base;
>+
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>+				      WAVE515_COMMAND_QUEUE_DEPTH);
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>+				      WAVE515_ONE_TASKBUF_SIZE);
>+
>+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>+				task_buf_base = temp_base + temp_size +
>+						(i * WAVE515_ONE_TASKBUF_SIZE);
>+				vpu_write_reg(vpu_dev,
>+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>+					      task_buf_base);
>+			}
>+
>+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
>+				      vpu_dev->sram_buf.daddr);
>+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
>+				      vpu_dev->sram_buf.size);
>+		}
>+
> 		vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
> 		vpu_write_reg(vpu_dev, W5_COMMAND, W5_WAKEUP_VPU);
> 		/* Start VPU after settings */
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>index a15c6b2c3d8b..557344754c4c 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>@@ -205,6 +205,9 @@ enum query_opt {
> #define W5_ADDR_TEMP_BASE                       (W5_REG_BASE + 0x011C)
> #define W5_TEMP_SIZE                            (W5_REG_BASE + 0x0120)
> #define W5_HW_OPTION                            (W5_REG_BASE + 0x012C)
>+#define W5_CMD_INIT_NUM_TASK_BUF		(W5_REG_BASE + 0x0134)
>+#define W5_CMD_INIT_ADDR_TASK_BUF0		(W5_REG_BASE + 0x0138)
>+#define W5_CMD_INIT_TASK_BUF_SIZE		(W5_REG_BASE + 0x0178)
> #define W5_SEC_AXI_PARAM                        (W5_REG_BASE + 0x0180)
>
>
>/************************************************************************
>/
>@@ -216,7 +219,9 @@ enum query_opt {
> #define W5_CMD_DEC_BS_SIZE                      (W5_REG_BASE + 0x0120)
> #define W5_CMD_BS_PARAM                         (W5_REG_BASE + 0x0124)
> #define W5_CMD_ADDR_SEC_AXI                     (W5_REG_BASE + 0x0130)
>+#define W515_CMD_ADDR_SEC_AXI			(W5_REG_BASE + 0x0124)
> #define W5_CMD_SEC_AXI_SIZE                     (W5_REG_BASE + 0x0134)
>+#define W515_CMD_SEC_AXI_SIZE			(W5_REG_BASE + 0x0128)
> #define W5_CMD_EXT_ADDR                         (W5_REG_BASE + 0x0138)
> #define W5_CMD_NUM_CQ_DEPTH_M1                  (W5_REG_BASE + 0x013C)
> #define W5_CMD_ERR_CONCEAL                      (W5_REG_BASE + 0x0140)
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>index a63fffed55e9..78888c57b6da 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>@@ -18,7 +18,11 @@ static int wave5_vdi_allocate_common_memory(struct
>device *dev)
> 	if (!vpu_dev->common_mem.vaddr) {
> 		int ret;
>
>-		vpu_dev->common_mem.size = SIZE_COMMON;
>+		if (vpu_dev->product_code == WAVE515_CODE)
>+			vpu_dev->common_mem.size = WAVE515_SIZE_COMMON;
>+		else
>+			vpu_dev->common_mem.size = SIZE_COMMON;
>+
> 		ret = wave5_vdi_allocate_dma_memory(vpu_dev, &vpu_dev-
>>common_mem);
> 		if (ret) {
> 			dev_err(dev, "unable to allocate common buffer\n");
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>index 5a71a711f2e8..65a99053c5e8 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>@@ -1869,7 +1869,8 @@ static int wave5_vpu_open_dec(struct file *filp)
> 		goto cleanup_inst;
> 	}
>
>-	wave5_vdi_allocate_sram(inst->dev);
>+	if (inst->dev->product_code != WAVE515_CODE)
>+		wave5_vdi_allocate_sram(inst->dev);
>
> 	return 0;
>
>@@ -1897,6 +1898,14 @@ int wave5_vpu_dec_register_device(struct
>vpu_device *dev)
> 	struct video_device *vdev_dec;
> 	int ret;
>
>+	/*
>+	 * secondary AXI setup for Wave515 is done by INIT_VPU command,
>+	 * that's why SRAM memory is allocated at VPU device register
>+	 * rather than at device open.
>+	 */
>+	if (dev->product_code == WAVE515_CODE)
>+		wave5_vdi_allocate_sram(dev);
>+
> 	vdev_dec = devm_kzalloc(dev->v4l2_dev.dev, sizeof(*vdev_dec),
>GFP_KERNEL);
> 	if (!vdev_dec)
> 		return -ENOMEM;
>@@ -1930,6 +1939,9 @@ int wave5_vpu_dec_register_device(struct vpu_device
>*dev)
>
> void wave5_vpu_dec_unregister_device(struct vpu_device *dev)
> {
>+	if (dev->product_code == WAVE515_CODE)
>+		wave5_vdi_free_sram(dev);
>+
> 	video_unregister_device(dev->video_dev_dec);
> 	if (dev->v4l2_m2m_dec_dev)
> 		v4l2_m2m_release(dev->v4l2_m2m_dec_dev);
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>index 2a972cddf4a6..fc267348399e 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>@@ -60,7 +60,13 @@ static irqreturn_t wave5_vpu_irq_thread(int irq, void
>*dev_id)
>
> 			if (irq_reason & BIT(INT_WAVE5_INIT_SEQ) ||
> 			    irq_reason & BIT(INT_WAVE5_ENC_SET_PARAM)) {
>-				if (seq_done & BIT(inst->id)) {
>+				if ((dev->product_code == WAVE515_CODE) &&
>+				    (cmd_done & BIT(inst->id))) {
>+					cmd_done &= ~BIT(inst->id);
>+					wave5_vdi_write_register(dev,
>W5_RET_QUEUE_CMD_DONE_INST,
>+								 cmd_done);
>+					complete(&inst->irq_done);
>+				} else if (seq_done & BIT(inst->id)) {
> 					seq_done &= ~BIT(inst->id);
> 					wave5_vdi_write_register(dev,
>W5_RET_SEQ_DONE_INSTANCE_INFO,
> 								 seq_done);
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>index 975d96b22191..601205df4433 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>@@ -18,6 +18,7 @@
> #include "wave5-vdi.h"
>
> enum product_id {
>+	PRODUCT_ID_515,
> 	PRODUCT_ID_521,
> 	PRODUCT_ID_511,
> 	PRODUCT_ID_517,
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>index 9d99afb78c89..b4128524fcfd 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>@@ -8,6 +8,7 @@
> #ifndef _VPU_CONFIG_H_
> #define _VPU_CONFIG_H_
>
>+#define WAVE515_CODE			0x5150
> #define WAVE517_CODE                    0x5170
> #define WAVE537_CODE                    0x5370
> #define WAVE511_CODE                    0x5110
>@@ -21,12 +22,13 @@
> 		((c) == WAVE517_CODE ||	(c) == WAVE537_CODE ||		\
> 		 (c) == WAVE511_CODE || (c) == WAVE521_CODE ||		\
> 		 (c) == WAVE521E1_CODE || (c) == WAVE521C_CODE ||	\
>-		 (c) == WAVE521C_DUAL_CODE);				\
>+		 (c) == WAVE521C_DUAL_CODE) || (c) == WAVE515_CODE;	\
> })
>
> #define WAVE517_WORKBUF_SIZE            (2 * 1024 * 1024)
> #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC
>40K
> #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>+#define WAVE515DEC_WORKBUF_SIZE		(2 * 1024 * 1024)
>
> #define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
>
>@@ -52,16 +54,21 @@
> #define VLC_BUF_NUM                     (2)
>
> #define COMMAND_QUEUE_DEPTH             (2)
>+#define WAVE515_COMMAND_QUEUE_DEPTH	(4)
>
> #define W5_REMAP_INDEX0                 0
> #define W5_REMAP_INDEX1                 1
> #define W5_REMAP_MAX_SIZE               (1024 * 1024)
>
> #define WAVE5_MAX_CODE_BUF_SIZE         (2 * 1024 * 1024)
>+#define WAVE515_MAX_CODE_BUF_SIZE	(1024 * 1024)
> #define WAVE5_TEMPBUF_OFFSET            WAVE5_MAX_CODE_BUF_SIZE
> #define WAVE5_TEMPBUF_SIZE              (1024 * 1024)
>+#define WAVE515_TASKBUF_OFFSET		(WAVE515_MAX_CODE_BUF_SIZE +
>WAVE5_TEMPBUF_SIZE)
>
> #define SIZE_COMMON                 (WAVE5_MAX_CODE_BUF_SIZE +
>WAVE5_TEMPBUF_SIZE)
>+#define WAVE515_ONE_TASKBUF_SIZE	(8 * 1024 * 1024)
>+#define WAVE515_SIZE_COMMON	(WAVE515_TASKBUF_OFFSET +
>WAVE515_COMMAND_QUEUE_DEPTH * WAVE515_ONE_TASKBUF_SIZE)
>
> //=====4. VPU REPORT MEMORY  ======================//
>
>diff --git a/drivers/media/platform/chips-media/wave5/wave5.h
>b/drivers/media/platform/chips-media/wave5/wave5.h
>index 063028eccd3b..57b00e182b6e 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5.h
>@@ -22,6 +22,7 @@
>  */
> #define BSOPTION_ENABLE_EXPLICIT_END		BIT(0)
> #define BSOPTION_HIGHLIGHT_STREAM_END		BIT(1)
>+#define BSOPTION_RD_PTR_VALID_FLAG		BIT(31)
>
> /*
>  * Currently the driver only supports hardware with little endian but for
>source
>--
>2.44.0

Re: RE: [PATCH v2 5/5] media: chips-media: wave5: support Wave515 decoder

[Ivan Bornyakov]

On Wed, Mar 27, 2024 at 10:34:15AM +0000, Nas Chung wrote:
> Hi, Ivan.
> 
> >-----Original Message-----
> >From: Ivan Bornyakov <brnkv.i1@gmail.com>
> >Sent: Monday, March 25, 2024 3:41 PM
> >To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
> ><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
> >Philipp Zabel <p.zabel@pengutronix.de>
> >Cc: Ivan Bornyakov <brnkv.i1@gmail.com>; linux-media@vger.kernel.org;
> >linux-kernel@vger.kernel.org
> >Subject: [PATCH v2 5/5] media: chips-media: wave5: support Wave515
> >decoder
> >
> >Add initial support for Wave515 multi-decoder IP. For now it is only
> >able to decode HEVC Main/Main10 profile videos.
> 
> Thanks for your updates.
> Could you share some test result for HEVC decoding ? (ex. Fluster test)

I've tested decoding with ffmpeg like so:
  ffmpeg -c:v hevc_v4l2m2m -i coded_file.h265 -fps_mode passthrough decoded_file.yuv

I didn't run fluster test, but I will.

> As you know, wave515 can support VP9 and AVS2.
> Did you have any chance to test VP9 or AVS2 streams ?

The plan is to settle initial support, then start working on support for
other formats.

> 
> Thanks.
> Nas.
> 
> >
> >This was tested on FPGA prototype, so wave5_dt_ids[] was not expanded.
> >Users of the real hardware with Wave515 IP will have to
> > * provide firmware specific to their SoC
> > * add struct wave5_match_data like this:
> >
> >	static const struct wave5_match_data platform_name_wave515_data = {
> >		.flags = WAVE5_IS_DEC,
> >		.fw_name = "cnm/wave515_platform_name_fw.bin",
> >	};
> >
> > * add item to wave5_dt_ids[] like this:
> >
> >	{
> >		.compatible = "vendor,soc-wave515",
> >		.data = &platform_name_wave515_data,
> >	},
> >
> > * describe new compatible in
> >   Documentation/devicetree/bindings/media/cnm,wave521c.yaml
> >
> >Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
> >---
> > .../platform/chips-media/wave5/wave5-helper.c |   3 +-
> > .../platform/chips-media/wave5/wave5-hw.c     | 245 ++++++++++++++----
> > .../chips-media/wave5/wave5-regdefine.h       |   5 +
> > .../platform/chips-media/wave5/wave5-vdi.c    |   6 +-
> > .../chips-media/wave5/wave5-vpu-dec.c         |  14 +-
> > .../platform/chips-media/wave5/wave5-vpu.c    |   8 +-
> > .../platform/chips-media/wave5/wave5-vpuapi.h |   1 +
> > .../chips-media/wave5/wave5-vpuconfig.h       |   9 +-
> > .../media/platform/chips-media/wave5/wave5.h  |   1 +
> > 9 files changed, 233 insertions(+), 59 deletions(-)
> >
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >index 8433ecab230c..c68f1e110ed9 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >@@ -29,7 +29,8 @@ void wave5_cleanup_instance(struct vpu_instance *inst)
> > {
> > 	int i;
> >
> >-	if (list_is_singular(&inst->list))
> >+	if (list_is_singular(&inst->list) &&
> >+	    inst->dev->product_code != WAVE515_CODE)
> > 		wave5_vdi_free_sram(inst->dev);
> >
> > 	for (i = 0; i < inst->fbc_buf_count; i++)
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >index cdd0a0948a94..e38995de6870 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >@@ -24,8 +24,10 @@
> > #define FEATURE_HEVC_ENCODER		BIT(0)
> >
> > /* Decoder support fields */
> >+#define W515_FEATURE_HEVC10BIT_DEC	BIT(1)
> > #define FEATURE_AVC_DECODER		BIT(3)
> > #define FEATURE_HEVC_DECODER		BIT(2)
> >+#define W515_FEATURE_HEVC_DECODER	BIT(0)
> >
> > #define FEATURE_BACKBONE		BIT(16)
> > #define FEATURE_VCORE_BACKBONE		BIT(22)
> >@@ -155,6 +157,8 @@ 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 == WAVE515_CODE)
> >+		gdi_status_check_value = 0x0738;
> > 	if (vpu_dev->product_code == WAVE521C_CODE ||
> > 	    vpu_dev->product_code == WAVE521_CODE ||
> > 	    vpu_dev->product_code == WAVE521E1_CODE)
> >@@ -186,6 +190,8 @@ unsigned int wave5_vpu_get_product_id(struct
> >vpu_device *vpu_dev)
> > 	u32 val = vpu_read_reg(vpu_dev, W5_PRODUCT_NUMBER);
> >
> > 	switch (val) {
> >+	case WAVE515_CODE:
> >+		return PRODUCT_ID_515;
> > 	case WAVE521C_CODE:
> > 		return PRODUCT_ID_521;
> > 	case WAVE521_CODE:
> >@@ -349,17 +355,33 @@ static int setup_wave5_properties(struct device
> >*dev)
> > 	hw_config_def1 = vpu_read_reg(vpu_dev, W5_RET_STD_DEF1);
> > 	hw_config_feature = vpu_read_reg(vpu_dev, W5_RET_CONF_FEATURE);
> >
> >-	p_attr->support_hevc10bit_enc = FIELD_GET(FEATURE_HEVC10BIT_ENC,
> >hw_config_feature);
> >-	p_attr->support_avc10bit_enc = FIELD_GET(FEATURE_AVC10BIT_ENC,
> >hw_config_feature);
> >-
> >-	p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
> >hw_config_def1) << STD_AVC;
> >-	p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
> >hw_config_def1) << STD_HEVC;
> >-	p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
> >hw_config_def1) << STD_AVC;
> >-	p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
> >hw_config_def1) << STD_HEVC;
> >-
> >-	p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
> >hw_config_def0);
> >-	p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE,
> >hw_config_def0);
> >-	p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE,
> >hw_config_def0);
> >+	if (vpu_dev->product_code == WAVE515_CODE) {
> >+		p_attr->support_hevc10bit_dec =
> >FIELD_GET(W515_FEATURE_HEVC10BIT_DEC,
> >+							  hw_config_feature);
> >+		p_attr->support_decoders =
> >FIELD_GET(W515_FEATURE_HEVC_DECODER,
> >+						     hw_config_def1) << STD_HEVC;
> >+	} else {
> >+		p_attr->support_hevc10bit_enc =
> >FIELD_GET(FEATURE_HEVC10BIT_ENC,
> >+							  hw_config_feature);
> >+		p_attr->support_avc10bit_enc =
> >FIELD_GET(FEATURE_AVC10BIT_ENC,
> >+							 hw_config_feature);
> >+
> >+		p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
> >+						     hw_config_def1) << STD_AVC;
> >+		p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
> >+						      hw_config_def1) << STD_HEVC;
> >+		p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
> >+						     hw_config_def1) << STD_AVC;
> >+		p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
> >+						      hw_config_def1) << STD_HEVC;
> >+
> >+		p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
> >+						     hw_config_def0);
> >+		p_attr->support_vcpu_backbone =
> >FIELD_GET(FEATURE_VCPU_BACKBONE,
> >+							  hw_config_def0);
> >+		p_attr->support_vcore_backbone =
> >FIELD_GET(FEATURE_VCORE_BACKBONE,
> >+							   hw_config_def0);
> >+	}
> >
> > 	setup_wave5_interrupts(vpu_dev);
> >
> >@@ -403,12 +425,18 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
> >size_t size)
> > 	common_vb = &vpu_dev->common_mem;
> >
> > 	code_base = common_vb->daddr;
> >+
> >+	if (vpu_dev->product_code == WAVE515_CODE)
> >+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
> >+	else
> >+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
> >+
> > 	/* ALIGN TO 4KB */
> >-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
> >+	code_size &= ~0xfff;
> > 	if (code_size < size * 2)
> > 		return -EINVAL;
> >
> >-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
> >+	temp_base = code_base + code_size;
> > 	temp_size = WAVE5_TEMPBUF_SIZE;
> >
> > 	ret = wave5_vdi_write_memory(vpu_dev, common_vb, 0, fw, size);
> >@@ -436,9 +464,12 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
> >size_t size)
> >
> > 	/* These register must be reset explicitly */
> > 	vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
> >-	wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
> >-	wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> >-	vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
> >+
> >+	if (vpu_dev->product_code != WAVE515_CODE) {
> >+		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
> >+		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> >+		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
> >+	}
> >
> > 	reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> > 	if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
> >@@ -453,6 +484,24 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
> >size_t size)
> > 		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
> > 	}
> >
> >+	if (vpu_dev->product_code == WAVE515_CODE) {
> >+		dma_addr_t task_buf_base;
> >+
> >+		vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
> >WAVE515_COMMAND_QUEUE_DEPTH);
> >+		vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
> >WAVE515_ONE_TASKBUF_SIZE);
> >+
> >+		for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
> >+			task_buf_base = temp_base + temp_size +
> >+					(i * WAVE515_ONE_TASKBUF_SIZE);
> >+			vpu_write_reg(vpu_dev,
> >+				      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
> >+				      task_buf_base);
> >+		}
> >+
> >+		vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI, vpu_dev-
> >>sram_buf.daddr);
> >+		vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE, vpu_dev-
> >>sram_buf.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);
> >@@ -493,29 +542,39 @@ int wave5_vpu_build_up_dec_param(struct
> >vpu_instance *inst,
> > 		return -EINVAL;
> > 	}
> >
> >-	p_dec_info->vb_work.size = WAVE521DEC_WORKBUF_SIZE;
> >+	if (vpu_dev->product == PRODUCT_ID_515)
> >+		p_dec_info->vb_work.size = WAVE515DEC_WORKBUF_SIZE;
> >+	else
> >+		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);
> >+	if (inst->dev->product_code != WAVE515_CODE)
> >+		vpu_write_reg(inst->dev, W5_CMD_DEC_VCORE_INFO, 1);
> >
> > 	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_ADDR_SEC_AXI, vpu_dev-
> >>sram_buf.daddr);
> >-	vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev-
> >>sram_buf.size);
> >+	if (inst->dev->product_code != WAVE515_CODE) {
> >+		vpu_write_reg(inst->dev, W5_CMD_ADDR_SEC_AXI, vpu_dev-
> >>sram_buf.daddr);
> >+		vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev-
> >>sram_buf.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: SDMA reads MSB first */
> > 	vpu_write_reg(inst->dev, W5_CMD_BS_PARAM,
> >BITSTREAM_ENDIANNESS_BIG_ENDIAN);
> >-	/* This register must be reset explicitly */
> >-	vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
> >-	vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1,
> >(COMMAND_QUEUE_DEPTH - 1));
> >+
> >+	if (inst->dev->product_code != WAVE515_CODE) {
> >+		/* This register must be reset explicitly */
> >+		vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
> >+		vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1,
> >(COMMAND_QUEUE_DEPTH - 1));
> >+	}
> >
> > 	ret = send_firmware_command(inst, W5_CREATE_INSTANCE, true, NULL,
> >NULL);
> > 	if (ret) {
> >@@ -566,7 +625,7 @@ static u32 get_bitstream_options(struct dec_info
> >*info)
> > int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
> > {
> > 	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
> >-	u32 cmd_option = INIT_SEQ_NORMAL;
> >+	u32 bs_option, cmd_option = INIT_SEQ_NORMAL;
> > 	u32 reg_val, fail_res;
> > 	int ret;
> >
> >@@ -576,7 +635,12 @@ int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
> > 	vpu_write_reg(inst->dev, W5_BS_RD_PTR, p_dec_info->stream_rd_ptr);
> > 	vpu_write_reg(inst->dev, W5_BS_WR_PTR, p_dec_info->stream_wr_ptr);
> >
> >-	vpu_write_reg(inst->dev, W5_BS_OPTION,
> >get_bitstream_options(p_dec_info));
> >+	bs_option = get_bitstream_options(p_dec_info);
> >+
> >+	if (inst->dev->product_code == WAVE515_CODE)
> >+		bs_option |= BSOPTION_RD_PTR_VALID_FLAG;
> >+
> >+	vpu_write_reg(inst->dev, W5_BS_OPTION, bs_option);
> >
> > 	vpu_write_reg(inst->dev, W5_COMMAND_OPTION, cmd_option);
> > 	vpu_write_reg(inst->dev, W5_CMD_DEC_USER_MASK, p_dec_info-
> >>user_data_enable);
> >@@ -642,10 +706,12 @@ static void wave5_get_dec_seq_result(struct
> >vpu_instance *inst, struct dec_initi
> > 		info->profile = FIELD_GET(SEQ_PARAM_PROFILE_MASK, reg_val);
> > 	}
> >
> >-	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;
> >+	if (inst->dev->product_code != WAVE515_CODE) {
> >+		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;
> >+	}
> > }
> >
> > int wave5_vpu_dec_get_seq_info(struct vpu_instance *inst, struct
> >dec_initial_info *info)
> >@@ -747,22 +813,27 @@ int wave5_vpu_dec_register_framebuffer(struct
> >vpu_instance *inst, struct frame_b
> >
> > 		pic_size = (init_info->pic_width << 16) | (init_info-
> >>pic_height);
> >
> >-		vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM) +
> >+		if (inst->dev->product_code != WAVE515_CODE) {
> >+			vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM)
> >+
> > 				(p_dec_info->param_buf_size *
> >COMMAND_QUEUE_DEPTH);
> >-		vb_buf.daddr = 0;
> >+			vb_buf.daddr = 0;
> >
> >-		if (vb_buf.size != p_dec_info->vb_task.size) {
> >-			wave5_vdi_free_dma_memory(inst->dev, &p_dec_info-
> >>vb_task);
> >-			ret = wave5_vdi_allocate_dma_memory(inst->dev,
> >&vb_buf);
> >-			if (ret)
> >-				goto free_fbc_c_tbl_buffers;
> >+			if (vb_buf.size != p_dec_info->vb_task.size) {
> >+				wave5_vdi_free_dma_memory(inst->dev,
> >+							  &p_dec_info->vb_task);
> >+				ret = wave5_vdi_allocate_dma_memory(inst->dev,
> >+								    &vb_buf);
> >+				if (ret)
> >+					goto free_fbc_c_tbl_buffers;
> >
> >-			p_dec_info->vb_task = vb_buf;
> >-		}
> >+				p_dec_info->vb_task = vb_buf;
> >+			}
> >
> >-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
> >-			      p_dec_info->vb_task.daddr);
> >-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
> >vb_buf.size);
> >+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
> >+				      p_dec_info->vb_task.daddr);
> >+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
> >+				      vb_buf.size);
> >+		}
> > 	} else {
> > 		pic_size = (init_info->pic_width << 16) | (init_info-
> >>pic_height);
> >
> >@@ -999,17 +1070,24 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
> >size_t size)
> > 	dma_addr_t code_base, temp_base;
> > 	dma_addr_t old_code_base, temp_size;
> > 	u32 code_size, reason_code;
> >-	u32 reg_val;
> >+	u32 i, reg_val;
> > 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
> >
> > 	common_vb = &vpu_dev->common_mem;
> >
> > 	code_base = common_vb->daddr;
> >+
> >+	if (vpu_dev->product_code == WAVE515_CODE)
> >+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
> >+	else
> >+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
> >+
> > 	/* ALIGN TO 4KB */
> >-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
> >+	code_size &= ~0xfff;
> > 	if (code_size < size * 2)
> > 		return -EINVAL;
> >-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
> >+
> >+	temp_base = code_base + code_size;
> > 	temp_size = WAVE5_TEMPBUF_SIZE;
> >
> > 	old_code_base = vpu_read_reg(vpu_dev, W5_VPU_REMAP_PADDR);
> >@@ -1043,9 +1121,12 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
> >size_t size)
> >
> > 		/* These register must be reset explicitly */
> > 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
> >-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
> >-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> >-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
> >+
> >+		if (vpu_dev->product_code != WAVE515_CODE) {
> >+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR,
> >0);
> >+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> >+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
> >+		}
> >
> > 		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> > 		if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
> >@@ -1060,6 +1141,28 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
> >size_t size)
> > 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID,
> >reg_val);
> > 		}
> >
> >+		if (vpu_dev->product_code == WAVE515_CODE) {
> >+			dma_addr_t task_buf_base;
> >+
> >+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
> >+				      WAVE515_COMMAND_QUEUE_DEPTH);
> >+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
> >+				      WAVE515_ONE_TASKBUF_SIZE);
> >+
> >+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
> >+				task_buf_base = temp_base + temp_size +
> >+						(i * WAVE515_ONE_TASKBUF_SIZE);
> >+				vpu_write_reg(vpu_dev,
> >+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
> >+					      task_buf_base);
> >+			}
> >+
> >+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
> >+				      vpu_dev->sram_buf.daddr);
> >+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
> >+				      vpu_dev->sram_buf.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);
> >@@ -1081,10 +1184,10 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
> >size_t size)
> > static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake,
> >const uint16_t *code,
> > 				size_t size)
> > {
> >-	u32 reg_val;
> >+	u32 i, reg_val;
> > 	struct vpu_buf *common_vb;
> >-	dma_addr_t code_base;
> >-	u32 code_size, reason_code;
> >+	dma_addr_t code_base, temp_base;
> >+	u32 code_size, temp_size, reason_code;
> > 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
> > 	int ret;
> >
> >@@ -1114,13 +1217,22 @@ static int wave5_vpu_sleep_wake(struct device
> >*dev, bool i_sleep_wake, const uin
> > 		common_vb = &vpu_dev->common_mem;
> >
> > 		code_base = common_vb->daddr;
> >+
> >+		if (vpu_dev->product_code == WAVE515_CODE)
> >+			code_size = WAVE515_MAX_CODE_BUF_SIZE;
> >+		else
> >+			code_size = WAVE5_MAX_CODE_BUF_SIZE;
> >+
> > 		/* ALIGN TO 4KB */
> >-		code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
> >+		code_size &= ~0xfff;
> > 		if (code_size < size * 2) {
> > 			dev_err(dev, "size too small\n");
> > 			return -EINVAL;
> > 		}
> >
> >+		temp_base = code_base + code_size;
> >+		temp_size = WAVE5_TEMPBUF_SIZE;
> >+
> > 		/* Power on without DEBUG mode */
> > 		vpu_write_reg(vpu_dev, W5_PO_CONF, 0);
> >
> >@@ -1133,9 +1245,12 @@ static int wave5_vpu_sleep_wake(struct device *dev,
> >bool i_sleep_wake, const uin
> >
> > 		/* These register must be reset explicitly */
> > 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
> >-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
> >-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> >-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
> >+
> >+		if (vpu_dev->product_code != WAVE515_CODE) {
> >+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR,
> >0);
> >+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
> >+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
> >+		}
> >
> > 		setup_wave5_interrupts(vpu_dev);
> >
> >@@ -1152,6 +1267,28 @@ static int wave5_vpu_sleep_wake(struct device *dev,
> >bool i_sleep_wake, const uin
> > 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID,
> >reg_val);
> > 		}
> >
> >+		if (vpu_dev->product_code == WAVE515_CODE) {
> >+			dma_addr_t task_buf_base;
> >+
> >+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
> >+				      WAVE515_COMMAND_QUEUE_DEPTH);
> >+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
> >+				      WAVE515_ONE_TASKBUF_SIZE);
> >+
> >+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
> >+				task_buf_base = temp_base + temp_size +
> >+						(i * WAVE515_ONE_TASKBUF_SIZE);
> >+				vpu_write_reg(vpu_dev,
> >+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
> >+					      task_buf_base);
> >+			}
> >+
> >+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
> >+				      vpu_dev->sram_buf.daddr);
> >+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
> >+				      vpu_dev->sram_buf.size);
> >+		}
> >+
> > 		vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
> > 		vpu_write_reg(vpu_dev, W5_COMMAND, W5_WAKEUP_VPU);
> > 		/* Start VPU after settings */
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
> >b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
> >index a15c6b2c3d8b..557344754c4c 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
> >@@ -205,6 +205,9 @@ enum query_opt {
> > #define W5_ADDR_TEMP_BASE                       (W5_REG_BASE + 0x011C)
> > #define W5_TEMP_SIZE                            (W5_REG_BASE + 0x0120)
> > #define W5_HW_OPTION                            (W5_REG_BASE + 0x012C)
> >+#define W5_CMD_INIT_NUM_TASK_BUF		(W5_REG_BASE + 0x0134)
> >+#define W5_CMD_INIT_ADDR_TASK_BUF0		(W5_REG_BASE + 0x0138)
> >+#define W5_CMD_INIT_TASK_BUF_SIZE		(W5_REG_BASE + 0x0178)
> > #define W5_SEC_AXI_PARAM                        (W5_REG_BASE + 0x0180)
> >
> >
> >/************************************************************************
> >/
> >@@ -216,7 +219,9 @@ enum query_opt {
> > #define W5_CMD_DEC_BS_SIZE                      (W5_REG_BASE + 0x0120)
> > #define W5_CMD_BS_PARAM                         (W5_REG_BASE + 0x0124)
> > #define W5_CMD_ADDR_SEC_AXI                     (W5_REG_BASE + 0x0130)
> >+#define W515_CMD_ADDR_SEC_AXI			(W5_REG_BASE + 0x0124)
> > #define W5_CMD_SEC_AXI_SIZE                     (W5_REG_BASE + 0x0134)
> >+#define W515_CMD_SEC_AXI_SIZE			(W5_REG_BASE + 0x0128)
> > #define W5_CMD_EXT_ADDR                         (W5_REG_BASE + 0x0138)
> > #define W5_CMD_NUM_CQ_DEPTH_M1                  (W5_REG_BASE + 0x013C)
> > #define W5_CMD_ERR_CONCEAL                      (W5_REG_BASE + 0x0140)
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >index a63fffed55e9..78888c57b6da 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
> >@@ -18,7 +18,11 @@ static int wave5_vdi_allocate_common_memory(struct
> >device *dev)
> > 	if (!vpu_dev->common_mem.vaddr) {
> > 		int ret;
> >
> >-		vpu_dev->common_mem.size = SIZE_COMMON;
> >+		if (vpu_dev->product_code == WAVE515_CODE)
> >+			vpu_dev->common_mem.size = WAVE515_SIZE_COMMON;
> >+		else
> >+			vpu_dev->common_mem.size = SIZE_COMMON;
> >+
> > 		ret = wave5_vdi_allocate_dma_memory(vpu_dev, &vpu_dev-
> >>common_mem);
> > 		if (ret) {
> > 			dev_err(dev, "unable to allocate common buffer\n");
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >index 5a71a711f2e8..65a99053c5e8 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >@@ -1869,7 +1869,8 @@ static int wave5_vpu_open_dec(struct file *filp)
> > 		goto cleanup_inst;
> > 	}
> >
> >-	wave5_vdi_allocate_sram(inst->dev);
> >+	if (inst->dev->product_code != WAVE515_CODE)
> >+		wave5_vdi_allocate_sram(inst->dev);
> >
> > 	return 0;
> >
> >@@ -1897,6 +1898,14 @@ int wave5_vpu_dec_register_device(struct
> >vpu_device *dev)
> > 	struct video_device *vdev_dec;
> > 	int ret;
> >
> >+	/*
> >+	 * secondary AXI setup for Wave515 is done by INIT_VPU command,
> >+	 * that's why SRAM memory is allocated at VPU device register
> >+	 * rather than at device open.
> >+	 */
> >+	if (dev->product_code == WAVE515_CODE)
> >+		wave5_vdi_allocate_sram(dev);
> >+
> > 	vdev_dec = devm_kzalloc(dev->v4l2_dev.dev, sizeof(*vdev_dec),
> >GFP_KERNEL);
> > 	if (!vdev_dec)
> > 		return -ENOMEM;
> >@@ -1930,6 +1939,9 @@ int wave5_vpu_dec_register_device(struct vpu_device
> >*dev)
> >
> > void wave5_vpu_dec_unregister_device(struct vpu_device *dev)
> > {
> >+	if (dev->product_code == WAVE515_CODE)
> >+		wave5_vdi_free_sram(dev);
> >+
> > 	video_unregister_device(dev->video_dev_dec);
> > 	if (dev->v4l2_m2m_dec_dev)
> > 		v4l2_m2m_release(dev->v4l2_m2m_dec_dev);
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >index 2a972cddf4a6..fc267348399e 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >@@ -60,7 +60,13 @@ static irqreturn_t wave5_vpu_irq_thread(int irq, void
> >*dev_id)
> >
> > 			if (irq_reason & BIT(INT_WAVE5_INIT_SEQ) ||
> > 			    irq_reason & BIT(INT_WAVE5_ENC_SET_PARAM)) {
> >-				if (seq_done & BIT(inst->id)) {
> >+				if ((dev->product_code == WAVE515_CODE) &&
> >+				    (cmd_done & BIT(inst->id))) {
> >+					cmd_done &= ~BIT(inst->id);
> >+					wave5_vdi_write_register(dev,
> >W5_RET_QUEUE_CMD_DONE_INST,
> >+								 cmd_done);
> >+					complete(&inst->irq_done);
> >+				} else if (seq_done & BIT(inst->id)) {
> > 					seq_done &= ~BIT(inst->id);
> > 					wave5_vdi_write_register(dev,
> >W5_RET_SEQ_DONE_INSTANCE_INFO,
> > 								 seq_done);
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >index 975d96b22191..601205df4433 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> >@@ -18,6 +18,7 @@
> > #include "wave5-vdi.h"
> >
> > enum product_id {
> >+	PRODUCT_ID_515,
> > 	PRODUCT_ID_521,
> > 	PRODUCT_ID_511,
> > 	PRODUCT_ID_517,
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >index 9d99afb78c89..b4128524fcfd 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> >@@ -8,6 +8,7 @@
> > #ifndef _VPU_CONFIG_H_
> > #define _VPU_CONFIG_H_
> >
> >+#define WAVE515_CODE			0x5150
> > #define WAVE517_CODE                    0x5170
> > #define WAVE537_CODE                    0x5370
> > #define WAVE511_CODE                    0x5110
> >@@ -21,12 +22,13 @@
> > 		((c) == WAVE517_CODE ||	(c) == WAVE537_CODE ||		\
> > 		 (c) == WAVE511_CODE || (c) == WAVE521_CODE ||		\
> > 		 (c) == WAVE521E1_CODE || (c) == WAVE521C_CODE ||	\
> >-		 (c) == WAVE521C_DUAL_CODE);				\
> >+		 (c) == WAVE521C_DUAL_CODE) || (c) == WAVE515_CODE;	\
> > })
> >
> > #define WAVE517_WORKBUF_SIZE            (2 * 1024 * 1024)
> > #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC
> >40K
> > #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
> >+#define WAVE515DEC_WORKBUF_SIZE		(2 * 1024 * 1024)
> >
> > #define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
> >
> >@@ -52,16 +54,21 @@
> > #define VLC_BUF_NUM                     (2)
> >
> > #define COMMAND_QUEUE_DEPTH             (2)
> >+#define WAVE515_COMMAND_QUEUE_DEPTH	(4)
> >
> > #define W5_REMAP_INDEX0                 0
> > #define W5_REMAP_INDEX1                 1
> > #define W5_REMAP_MAX_SIZE               (1024 * 1024)
> >
> > #define WAVE5_MAX_CODE_BUF_SIZE         (2 * 1024 * 1024)
> >+#define WAVE515_MAX_CODE_BUF_SIZE	(1024 * 1024)
> > #define WAVE5_TEMPBUF_OFFSET            WAVE5_MAX_CODE_BUF_SIZE
> > #define WAVE5_TEMPBUF_SIZE              (1024 * 1024)
> >+#define WAVE515_TASKBUF_OFFSET		(WAVE515_MAX_CODE_BUF_SIZE +
> >WAVE5_TEMPBUF_SIZE)
> >
> > #define SIZE_COMMON                 (WAVE5_MAX_CODE_BUF_SIZE +
> >WAVE5_TEMPBUF_SIZE)
> >+#define WAVE515_ONE_TASKBUF_SIZE	(8 * 1024 * 1024)
> >+#define WAVE515_SIZE_COMMON	(WAVE515_TASKBUF_OFFSET +
> >WAVE515_COMMAND_QUEUE_DEPTH * WAVE515_ONE_TASKBUF_SIZE)
> >
> > //=====4. VPU REPORT MEMORY  ======================//
> >
> >diff --git a/drivers/media/platform/chips-media/wave5/wave5.h
> >b/drivers/media/platform/chips-media/wave5/wave5.h
> >index 063028eccd3b..57b00e182b6e 100644
> >--- a/drivers/media/platform/chips-media/wave5/wave5.h
> >+++ b/drivers/media/platform/chips-media/wave5/wave5.h
> >@@ -22,6 +22,7 @@
> >  */
> > #define BSOPTION_ENABLE_EXPLICIT_END		BIT(0)
> > #define BSOPTION_HIGHLIGHT_STREAM_END		BIT(1)
> >+#define BSOPTION_RD_PTR_VALID_FLAG		BIT(31)
> >
> > /*
> >  * Currently the driver only supports hardware with little endian but for
> >source
> >--
> >2.44.0
> 

Re: [PATCH v2 5/5] media: chips-media: wave5: support Wave515 decoder

[Sebastian Fricke]

Hey Ivan,

@Nas or Jackson can you please provide your tested-by to ensure that
this doesn't break 521C. Thanks!

Thanks for the patches Ivan, see my comments inline below.

On 25.03.2024 09:41, Ivan Bornyakov wrote:
>Add initial support for Wave515 multi-decoder IP. For now it is only
>able to decode HEVC Main/Main10 profile videos.
>
>This was tested on FPGA prototype, so wave5_dt_ids[] was not expanded.
>Users of the real hardware with Wave515 IP will have to
> * provide firmware specific to their SoC
> * add struct wave5_match_data like this:
>
>	static const struct wave5_match_data platform_name_wave515_data = {
>		.flags = WAVE5_IS_DEC,
>		.fw_name = "cnm/wave515_platform_name_fw.bin",
>	};
>
> * add item to wave5_dt_ids[] like this:
>
>	{
>		.compatible = "vendor,soc-wave515",
>		.data = &platform_name_wave515_data,
>	},
>
> * describe new compatible in
>   Documentation/devicetree/bindings/media/cnm,wave521c.yaml
>
>Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>---
> .../platform/chips-media/wave5/wave5-helper.c |   3 +-
> .../platform/chips-media/wave5/wave5-hw.c     | 245 ++++++++++++++----
> .../chips-media/wave5/wave5-regdefine.h       |   5 +
> .../platform/chips-media/wave5/wave5-vdi.c    |   6 +-
> .../chips-media/wave5/wave5-vpu-dec.c         |  14 +-
> .../platform/chips-media/wave5/wave5-vpu.c    |   8 +-
> .../platform/chips-media/wave5/wave5-vpuapi.h |   1 +
> .../chips-media/wave5/wave5-vpuconfig.h       |   9 +-
> .../media/platform/chips-media/wave5/wave5.h  |   1 +
> 9 files changed, 233 insertions(+), 59 deletions(-)
>
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>index 8433ecab230c..c68f1e110ed9 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>@@ -29,7 +29,8 @@ void wave5_cleanup_instance(struct vpu_instance *inst)
> {
> 	int i;
>
>-	if (list_is_singular(&inst->list))
>+	if (list_is_singular(&inst->list) &&
>+	    inst->dev->product_code != WAVE515_CODE)
> 		wave5_vdi_free_sram(inst->dev);

So you free the sram instead in unregister device, can you note that
here please and maybe briefly explain why that is needed otherwise, one
might assume the 515 doesn't use an SRAM.

>
> 	for (i = 0; i < inst->fbc_buf_count; i++)
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>index cdd0a0948a94..e38995de6870 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>@@ -24,8 +24,10 @@
> #define FEATURE_HEVC_ENCODER		BIT(0)
>
> /* Decoder support fields */
>+#define W515_FEATURE_HEVC10BIT_DEC	BIT(1)
> #define FEATURE_AVC_DECODER		BIT(3)
> #define FEATURE_HEVC_DECODER		BIT(2)
>+#define W515_FEATURE_HEVC_DECODER	BIT(0)

I don't understand the order of these entries.
Either group the W515 ones or sort them by bit value or by codec but
this just seems random.
Also, as mentioned below please rename the other feature values, as you
show that they are clearly not general purpose but for a specific
device/device group.

>
> #define FEATURE_BACKBONE		BIT(16)
> #define FEATURE_VCORE_BACKBONE		BIT(22)
>@@ -155,6 +157,8 @@ 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 == WAVE515_CODE)
>+		gdi_status_check_value = 0x0738;
> 	if (vpu_dev->product_code == WAVE521C_CODE ||
> 	    vpu_dev->product_code == WAVE521_CODE ||
> 	    vpu_dev->product_code == WAVE521E1_CODE)
>@@ -186,6 +190,8 @@ unsigned int wave5_vpu_get_product_id(struct vpu_device *vpu_dev)
> 	u32 val = vpu_read_reg(vpu_dev, W5_PRODUCT_NUMBER);
>
> 	switch (val) {
>+	case WAVE515_CODE:
>+		return PRODUCT_ID_515;
> 	case WAVE521C_CODE:
> 		return PRODUCT_ID_521;
> 	case WAVE521_CODE:
>@@ -349,17 +355,33 @@ static int setup_wave5_properties(struct device *dev)
> 	hw_config_def1 = vpu_read_reg(vpu_dev, W5_RET_STD_DEF1);
> 	hw_config_feature = vpu_read_reg(vpu_dev, W5_RET_CONF_FEATURE);
>
>-	p_attr->support_hevc10bit_enc = FIELD_GET(FEATURE_HEVC10BIT_ENC, hw_config_feature);
>-	p_attr->support_avc10bit_enc = FIELD_GET(FEATURE_AVC10BIT_ENC, hw_config_feature);
>-
>-	p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER, hw_config_def1) << STD_AVC;
>-	p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER, hw_config_def1) << STD_HEVC;
>-	p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER, hw_config_def1) << STD_AVC;
>-	p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER, hw_config_def1) << STD_HEVC;
>-
>-	p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE, hw_config_def0);
>-	p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE, hw_config_def0);
>-	p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE, hw_config_def0);
>+	if (vpu_dev->product_code == WAVE515_CODE) {
>+		p_attr->support_hevc10bit_dec = FIELD_GET(W515_FEATURE_HEVC10BIT_DEC,
>+							  hw_config_feature);
>+		p_attr->support_decoders = FIELD_GET(W515_FEATURE_HEVC_DECODER,
>+						     hw_config_def1) << STD_HEVC;
>+	} else {
>+		p_attr->support_hevc10bit_enc = FIELD_GET(FEATURE_HEVC10BIT_ENC,

Now that the Wave515 features have a product code, this will become
quite confusing in a bit. Can you please rename the ones from WAVE521C
like you did for Wave515?

>+							  hw_config_feature);
>+		p_attr->support_avc10bit_enc = FIELD_GET(FEATURE_AVC10BIT_ENC,
>+							 hw_config_feature);
>+
>+		p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
>+						     hw_config_def1) << STD_AVC;
>+		p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
>+						      hw_config_def1) << STD_HEVC;
>+		p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
>+						     hw_config_def1) << STD_AVC;
>+		p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
>+						      hw_config_def1) << STD_HEVC;
>+
>+		p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
>+						     hw_config_def0);
>+		p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE,
>+							  hw_config_def0);
>+		p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE,
>+							   hw_config_def0);
>+	}
>
> 	setup_wave5_interrupts(vpu_dev);
>
>@@ -403,12 +425,18 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
> 	common_vb = &vpu_dev->common_mem;
>
> 	code_base = common_vb->daddr;
>+
>+	if (vpu_dev->product_code == WAVE515_CODE)
>+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
>+	else
>+		code_size = WAVE5_MAX_CODE_BUF_SIZE;

This one is similar, if WAVE5_MAX_CODE_BUF_SIZE is not the maximum for
all wave5 variations then we should rename it to be fitting for the one
it was originally used for, e.g. Wave521C.
>+
> 	/* ALIGN TO 4KB */
>-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>+	code_size &= ~0xfff;
> 	if (code_size < size * 2)
> 		return -EINVAL;
>
>-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
>+	temp_base = code_base + code_size;
> 	temp_size = WAVE5_TEMPBUF_SIZE;
>
> 	ret = wave5_vdi_write_memory(vpu_dev, common_vb, 0, fw, size);
>@@ -436,9 +464,12 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
>
> 	/* These register must be reset explicitly */
> 	vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>-	wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>-	wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>-	vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+
>+	if (vpu_dev->product_code != WAVE515_CODE) {
>+		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>+		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>+		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+	}
>
> 	reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> 	if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
>@@ -453,6 +484,24 @@ int wave5_vpu_init(struct device *dev, u8 *fw, size_t size)
> 		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
> 	}
>
>+	if (vpu_dev->product_code == WAVE515_CODE) {
>+		dma_addr_t task_buf_base;
>+
>+		vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF, WAVE515_COMMAND_QUEUE_DEPTH);
>+		vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE, WAVE515_ONE_TASKBUF_SIZE);
>+
>+		for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>+			task_buf_base = temp_base + temp_size +
>+					(i * WAVE515_ONE_TASKBUF_SIZE);
>+			vpu_write_reg(vpu_dev,
>+				      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>+				      task_buf_base);
>+		}
>+
>+		vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI, vpu_dev->sram_buf.daddr);
>+		vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE, vpu_dev->sram_buf.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);
>@@ -493,29 +542,39 @@ int wave5_vpu_build_up_dec_param(struct vpu_instance *inst,
> 		return -EINVAL;
> 	}
>
>-	p_dec_info->vb_work.size = WAVE521DEC_WORKBUF_SIZE;
>+	if (vpu_dev->product == PRODUCT_ID_515)
>+		p_dec_info->vb_work.size = WAVE515DEC_WORKBUF_SIZE;
>+	else
>+		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);
>+	if (inst->dev->product_code != WAVE515_CODE)
>+		vpu_write_reg(inst->dev, W5_CMD_DEC_VCORE_INFO, 1);
>
> 	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_ADDR_SEC_AXI, vpu_dev->sram_buf.daddr);
>-	vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev->sram_buf.size);
>+	if (inst->dev->product_code != WAVE515_CODE) {
>+		vpu_write_reg(inst->dev, W5_CMD_ADDR_SEC_AXI, vpu_dev->sram_buf.daddr);
>+		vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev->sram_buf.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: SDMA reads MSB first */
> 	vpu_write_reg(inst->dev, W5_CMD_BS_PARAM, BITSTREAM_ENDIANNESS_BIG_ENDIAN);
>-	/* This register must be reset explicitly */
>-	vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
>-	vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1, (COMMAND_QUEUE_DEPTH - 1));
>+
>+	if (inst->dev->product_code != WAVE515_CODE) {
>+		/* This register must be reset explicitly */
>+		vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
>+		vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1, (COMMAND_QUEUE_DEPTH - 1));
>+	}
>
> 	ret = send_firmware_command(inst, W5_CREATE_INSTANCE, true, NULL, NULL);
> 	if (ret) {
>@@ -566,7 +625,7 @@ static u32 get_bitstream_options(struct dec_info *info)
> int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
> {
> 	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
>-	u32 cmd_option = INIT_SEQ_NORMAL;
>+	u32 bs_option, cmd_option = INIT_SEQ_NORMAL;
> 	u32 reg_val, fail_res;
> 	int ret;
>
>@@ -576,7 +635,12 @@ int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
> 	vpu_write_reg(inst->dev, W5_BS_RD_PTR, p_dec_info->stream_rd_ptr);
> 	vpu_write_reg(inst->dev, W5_BS_WR_PTR, p_dec_info->stream_wr_ptr);
>
>-	vpu_write_reg(inst->dev, W5_BS_OPTION, get_bitstream_options(p_dec_info));
>+	bs_option = get_bitstream_options(p_dec_info);
>+
>+	if (inst->dev->product_code == WAVE515_CODE)
>+		bs_option |= BSOPTION_RD_PTR_VALID_FLAG;

Is it possible to add a brief comment here that describes why this is
needed for that device and what this will change for the process?

>+
>+	vpu_write_reg(inst->dev, W5_BS_OPTION, bs_option);
>
> 	vpu_write_reg(inst->dev, W5_COMMAND_OPTION, cmd_option);
> 	vpu_write_reg(inst->dev, W5_CMD_DEC_USER_MASK, p_dec_info->user_data_enable);
>@@ -642,10 +706,12 @@ static void wave5_get_dec_seq_result(struct vpu_instance *inst, struct dec_initi
> 		info->profile = FIELD_GET(SEQ_PARAM_PROFILE_MASK, reg_val);
> 	}
>
>-	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;
>+	if (inst->dev->product_code != WAVE515_CODE) {
>+		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;
>+	}
> }
>
> int wave5_vpu_dec_get_seq_info(struct vpu_instance *inst, struct dec_initial_info *info)
>@@ -747,22 +813,27 @@ int wave5_vpu_dec_register_framebuffer(struct vpu_instance *inst, struct frame_b
>
> 		pic_size = (init_info->pic_width << 16) | (init_info->pic_height);
>
>-		vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM) +
>+		if (inst->dev->product_code != WAVE515_CODE) {
>+			vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM) +
> 				(p_dec_info->param_buf_size * COMMAND_QUEUE_DEPTH);
>-		vb_buf.daddr = 0;
>+			vb_buf.daddr = 0;
>
>-		if (vb_buf.size != p_dec_info->vb_task.size) {
>-			wave5_vdi_free_dma_memory(inst->dev, &p_dec_info->vb_task);
>-			ret = wave5_vdi_allocate_dma_memory(inst->dev, &vb_buf);
>-			if (ret)
>-				goto free_fbc_c_tbl_buffers;
>+			if (vb_buf.size != p_dec_info->vb_task.size) {
>+				wave5_vdi_free_dma_memory(inst->dev,
>+							  &p_dec_info->vb_task);
>+				ret = wave5_vdi_allocate_dma_memory(inst->dev,
>+								    &vb_buf);
>+				if (ret)
>+					goto free_fbc_c_tbl_buffers;
>
>-			p_dec_info->vb_task = vb_buf;
>-		}
>+				p_dec_info->vb_task = vb_buf;
>+			}
>
>-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
>-			      p_dec_info->vb_task.daddr);
>-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE, vb_buf.size);
>+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
>+				      p_dec_info->vb_task.daddr);
>+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
>+				      vb_buf.size);
>+		}
> 	} else {
> 		pic_size = (init_info->pic_width << 16) | (init_info->pic_height);
>
>@@ -999,17 +1070,24 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
> 	dma_addr_t code_base, temp_base;
> 	dma_addr_t old_code_base, temp_size;
> 	u32 code_size, reason_code;
>-	u32 reg_val;
>+	u32 i, reg_val;

You only use the variable i within the conditional branch below, so you
can just declare it there.

> 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
>
> 	common_vb = &vpu_dev->common_mem;
>
> 	code_base = common_vb->daddr;
>+
>+	if (vpu_dev->product_code == WAVE515_CODE)
>+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
>+	else
>+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
>+
> 	/* ALIGN TO 4KB */
>-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>+	code_size &= ~0xfff;
> 	if (code_size < size * 2)
> 		return -EINVAL;
>-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
>+
>+	temp_base = code_base + code_size;
> 	temp_size = WAVE5_TEMPBUF_SIZE;
>
> 	old_code_base = vpu_read_reg(vpu_dev, W5_VPU_REMAP_PADDR);
>@@ -1043,9 +1121,12 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
>
> 		/* These register must be reset explicitly */
> 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+
>+		if (vpu_dev->product_code != WAVE515_CODE) {
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+		}
>
> 		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> 		if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
>@@ -1060,6 +1141,28 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
> 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
> 		}
>
>+		if (vpu_dev->product_code == WAVE515_CODE) {
>+			dma_addr_t task_buf_base;

As mentioned above you can declare the variable i here.

>+
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>+				      WAVE515_COMMAND_QUEUE_DEPTH);
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>+				      WAVE515_ONE_TASKBUF_SIZE);
>+
>+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>+				task_buf_base = temp_base + temp_size +
>+						(i * WAVE515_ONE_TASKBUF_SIZE);
>+				vpu_write_reg(vpu_dev,
>+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>+					      task_buf_base);
>+			}
>+
>+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
>+				      vpu_dev->sram_buf.daddr);
>+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
>+				      vpu_dev->sram_buf.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);
>@@ -1081,10 +1184,10 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
> static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
> 				size_t size)
> {
>-	u32 reg_val;
>+	u32 i, reg_val;

Like suggested in vpu_re_init please.

> 	struct vpu_buf *common_vb;
>-	dma_addr_t code_base;
>-	u32 code_size, reason_code;
>+	dma_addr_t code_base, temp_base;
>+	u32 code_size, temp_size, reason_code;
> 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
> 	int ret;
>
>@@ -1114,13 +1217,22 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
> 		common_vb = &vpu_dev->common_mem;
>
> 		code_base = common_vb->daddr;
>+
>+		if (vpu_dev->product_code == WAVE515_CODE)
>+			code_size = WAVE515_MAX_CODE_BUF_SIZE;
>+		else
>+			code_size = WAVE5_MAX_CODE_BUF_SIZE;
>+
> 		/* ALIGN TO 4KB */
>-		code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>+		code_size &= ~0xfff;
> 		if (code_size < size * 2) {
> 			dev_err(dev, "size too small\n");
> 			return -EINVAL;
> 		}
>
>+		temp_base = code_base + code_size;
>+		temp_size = WAVE5_TEMPBUF_SIZE;
>+
> 		/* Power on without DEBUG mode */
> 		vpu_write_reg(vpu_dev, W5_PO_CONF, 0);
>
>@@ -1133,9 +1245,12 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
>
> 		/* These register must be reset explicitly */
> 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+
>+		if (vpu_dev->product_code != WAVE515_CODE) {
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>+		}
>
> 		setup_wave5_interrupts(vpu_dev);
>
>@@ -1152,6 +1267,28 @@ static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uin
> 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
> 		}
>
>+		if (vpu_dev->product_code == WAVE515_CODE) {
>+			dma_addr_t task_buf_base;
>+
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>+				      WAVE515_COMMAND_QUEUE_DEPTH);
>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>+				      WAVE515_ONE_TASKBUF_SIZE);
>+
>+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>+				task_buf_base = temp_base + temp_size +
>+						(i * WAVE515_ONE_TASKBUF_SIZE);
>+				vpu_write_reg(vpu_dev,
>+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>+					      task_buf_base);
>+			}
>+
>+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
>+				      vpu_dev->sram_buf.daddr);
>+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
>+				      vpu_dev->sram_buf.size);
>+		}
>+
> 		vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
> 		vpu_write_reg(vpu_dev, W5_COMMAND, W5_WAKEUP_VPU);
> 		/* Start VPU after settings */
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>index a15c6b2c3d8b..557344754c4c 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>@@ -205,6 +205,9 @@ enum query_opt {
> #define W5_ADDR_TEMP_BASE                       (W5_REG_BASE + 0x011C)
> #define W5_TEMP_SIZE                            (W5_REG_BASE + 0x0120)
> #define W5_HW_OPTION                            (W5_REG_BASE + 0x012C)
>+#define W5_CMD_INIT_NUM_TASK_BUF		(W5_REG_BASE + 0x0134)
>+#define W5_CMD_INIT_ADDR_TASK_BUF0		(W5_REG_BASE + 0x0138)
>+#define W5_CMD_INIT_TASK_BUF_SIZE		(W5_REG_BASE + 0x0178)

It looks like you are using tabs here, while the others utilize spaces.
In general your tabs should expand to 8 whitespaces.
(https://www.kernel.org/doc/html/v4.10/process/coding-style.html#indentation)

> #define W5_SEC_AXI_PARAM                        (W5_REG_BASE + 0x0180)
>
> /************************************************************************/
>@@ -216,7 +219,9 @@ enum query_opt {
> #define W5_CMD_DEC_BS_SIZE                      (W5_REG_BASE + 0x0120)
> #define W5_CMD_BS_PARAM                         (W5_REG_BASE + 0x0124)
> #define W5_CMD_ADDR_SEC_AXI                     (W5_REG_BASE + 0x0130)
>+#define W515_CMD_ADDR_SEC_AXI			(W5_REG_BASE + 0x0124)
> #define W5_CMD_SEC_AXI_SIZE                     (W5_REG_BASE + 0x0134)
>+#define W515_CMD_SEC_AXI_SIZE			(W5_REG_BASE + 0x0128)

Same as above.

> #define W5_CMD_EXT_ADDR                         (W5_REG_BASE + 0x0138)
> #define W5_CMD_NUM_CQ_DEPTH_M1                  (W5_REG_BASE + 0x013C)
> #define W5_CMD_ERR_CONCEAL                      (W5_REG_BASE + 0x0140)
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>index a63fffed55e9..78888c57b6da 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>@@ -18,7 +18,11 @@ static int wave5_vdi_allocate_common_memory(struct device *dev)
> 	if (!vpu_dev->common_mem.vaddr) {
> 		int ret;
>
>-		vpu_dev->common_mem.size = SIZE_COMMON;
>+		if (vpu_dev->product_code == WAVE515_CODE)
>+			vpu_dev->common_mem.size = WAVE515_SIZE_COMMON;
>+		else
>+			vpu_dev->common_mem.size = SIZE_COMMON;
>+
> 		ret = wave5_vdi_allocate_dma_memory(vpu_dev, &vpu_dev->common_mem);
> 		if (ret) {
> 			dev_err(dev, "unable to allocate common buffer\n");
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>index 5a71a711f2e8..65a99053c5e8 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>@@ -1869,7 +1869,8 @@ static int wave5_vpu_open_dec(struct file *filp)
> 		goto cleanup_inst;
> 	}
>
>-	wave5_vdi_allocate_sram(inst->dev);
>+	if (inst->dev->product_code != WAVE515_CODE)
>+		wave5_vdi_allocate_sram(inst->dev);
>
> 	return 0;
>
>@@ -1897,6 +1898,14 @@ int wave5_vpu_dec_register_device(struct vpu_device *dev)
> 	struct video_device *vdev_dec;
> 	int ret;
>
>+	/*
>+	 * secondary AXI setup for Wave515 is done by INIT_VPU command,
>+	 * that's why SRAM memory is allocated at VPU device register
>+	 * rather than at device open.

Just a nitpick, but if you use something that resembles more the actual
function names it makes it easier to grep this part or to refer to the
right function from reading this comment.
So for example: vpu_open_dec & vpu_dec_register_device

>+	 */
>+	if (dev->product_code == WAVE515_CODE)
>+		wave5_vdi_allocate_sram(dev);
>+
> 	vdev_dec = devm_kzalloc(dev->v4l2_dev.dev, sizeof(*vdev_dec), GFP_KERNEL);
> 	if (!vdev_dec)
> 		return -ENOMEM;
>@@ -1930,6 +1939,9 @@ int wave5_vpu_dec_register_device(struct vpu_device *dev)
>
> void wave5_vpu_dec_unregister_device(struct vpu_device *dev)
> {
>+	if (dev->product_code == WAVE515_CODE)
>+		wave5_vdi_free_sram(dev);

Why does that need to happen here just for this one version of the Wave5
codec?

>+
> 	video_unregister_device(dev->video_dev_dec);
> 	if (dev->v4l2_m2m_dec_dev)
> 		v4l2_m2m_release(dev->v4l2_m2m_dec_dev);
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>index 2a972cddf4a6..fc267348399e 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>@@ -60,7 +60,13 @@ static irqreturn_t wave5_vpu_irq_thread(int irq, void *dev_id)
>
> 			if (irq_reason & BIT(INT_WAVE5_INIT_SEQ) ||
> 			    irq_reason & BIT(INT_WAVE5_ENC_SET_PARAM)) {
>-				if (seq_done & BIT(inst->id)) {
>+				if ((dev->product_code == WAVE515_CODE) &&
>+				    (cmd_done & BIT(inst->id))) {
>+					cmd_done &= ~BIT(inst->id);
>+					wave5_vdi_write_register(dev, W5_RET_QUEUE_CMD_DONE_INST,
>+								 cmd_done);
>+					complete(&inst->irq_done);
>+				} else if (seq_done & BIT(inst->id)) {
> 					seq_done &= ~BIT(inst->id);
> 					wave5_vdi_write_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO,
> 								 seq_done);
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>index 975d96b22191..601205df4433 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>@@ -18,6 +18,7 @@
> #include "wave5-vdi.h"
>
> enum product_id {
>+	PRODUCT_ID_515,
> 	PRODUCT_ID_521,
> 	PRODUCT_ID_511,
> 	PRODUCT_ID_517,
>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>index 9d99afb78c89..b4128524fcfd 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>@@ -8,6 +8,7 @@
> #ifndef _VPU_CONFIG_H_
> #define _VPU_CONFIG_H_
>
>+#define WAVE515_CODE			0x5150

Indentation again

> #define WAVE517_CODE                    0x5170
> #define WAVE537_CODE                    0x5370
> #define WAVE511_CODE                    0x5110
>@@ -21,12 +22,13 @@
> 		((c) == WAVE517_CODE ||	(c) == WAVE537_CODE ||		\
> 		 (c) == WAVE511_CODE || (c) == WAVE521_CODE ||		\
> 		 (c) == WAVE521E1_CODE || (c) == WAVE521C_CODE ||	\
>-		 (c) == WAVE521C_DUAL_CODE);				\
>+		 (c) == WAVE521C_DUAL_CODE) || (c) == WAVE515_CODE;	\
> })
>
> #define WAVE517_WORKBUF_SIZE            (2 * 1024 * 1024)
> #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC 40K
> #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>+#define WAVE515DEC_WORKBUF_SIZE		(2 * 1024 * 1024)
>
> #define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
>
>@@ -52,16 +54,21 @@
> #define VLC_BUF_NUM                     (2)
>
> #define COMMAND_QUEUE_DEPTH             (2)
>+#define WAVE515_COMMAND_QUEUE_DEPTH	(4)
>
> #define W5_REMAP_INDEX0                 0
> #define W5_REMAP_INDEX1                 1
> #define W5_REMAP_MAX_SIZE               (1024 * 1024)
>
> #define WAVE5_MAX_CODE_BUF_SIZE         (2 * 1024 * 1024)
>+#define WAVE515_MAX_CODE_BUF_SIZE	(1024 * 1024)
> #define WAVE5_TEMPBUF_OFFSET            WAVE5_MAX_CODE_BUF_SIZE
> #define WAVE5_TEMPBUF_SIZE              (1024 * 1024)
>+#define WAVE515_TASKBUF_OFFSET		(WAVE515_MAX_CODE_BUF_SIZE + WAVE5_TEMPBUF_SIZE)
>
> #define SIZE_COMMON                 (WAVE5_MAX_CODE_BUF_SIZE + WAVE5_TEMPBUF_SIZE)

Just as above, this macro looks like a general macro now, but as we have
a Wave515 version this clearly isn't valid for all of them so please
rename the others.

>+#define WAVE515_ONE_TASKBUF_SIZE	(8 * 1024 * 1024)

Would something speak against: (8 * WAVE5_TEMPBUF_SIZE)?
(Especially as you use that macro for the calculation of the offset)
And why the gap between the the size and the offset? I think it makes
more sense to have them on top of each other

Just as above the indentation don't look right here.

>+#define WAVE515_SIZE_COMMON	(WAVE515_TASKBUF_OFFSET + WAVE515_COMMAND_QUEUE_DEPTH * WAVE515_ONE_TASKBUF_SIZE)
>
> //=====4. VPU REPORT MEMORY  ======================//
>
>diff --git a/drivers/media/platform/chips-media/wave5/wave5.h b/drivers/media/platform/chips-media/wave5/wave5.h
>index 063028eccd3b..57b00e182b6e 100644
>--- a/drivers/media/platform/chips-media/wave5/wave5.h
>+++ b/drivers/media/platform/chips-media/wave5/wave5.h
>@@ -22,6 +22,7 @@
>  */
> #define BSOPTION_ENABLE_EXPLICIT_END		BIT(0)
> #define BSOPTION_HIGHLIGHT_STREAM_END		BIT(1)
>+#define BSOPTION_RD_PTR_VALID_FLAG		BIT(31)

As explained above when you use it, I think an explanatory comment is
helpful here.

Greetings,
Sebastian
>
> /*
>  * Currently the driver only supports hardware with little endian but for source
>-- 
>2.44.0
>
>

RE: [PATCH v2 5/5] media: chips-media: wave5: support Wave515 decoder

[Nas Chung]

Hi, Sebastian.

>-----Original Message-----
>From: Sebastian Fricke <sebastian.fricke@collabora.com>
>Sent: Friday, March 29, 2024 1:18 AM
>To: Ivan Bornyakov <brnkv.i1@gmail.com>
>Cc: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
><jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
>Philipp Zabel <p.zabel@pengutronix.de>; linux-media@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Subject: Re: [PATCH v2 5/5] media: chips-media: wave5: support Wave515
>decoder
>
>Hey Ivan,
>
>@Nas or Jackson can you please provide your tested-by to ensure that
>this doesn't break 521C. Thanks!

Okay, I will test it and share some results.

Thanks.
Nas.

>
>Thanks for the patches Ivan, see my comments inline below.
>
>On 25.03.2024 09:41, Ivan Bornyakov wrote:
>>Add initial support for Wave515 multi-decoder IP. For now it is only
>>able to decode HEVC Main/Main10 profile videos.
>>
>>This was tested on FPGA prototype, so wave5_dt_ids[] was not expanded.
>>Users of the real hardware with Wave515 IP will have to
>> * provide firmware specific to their SoC
>> * add struct wave5_match_data like this:
>>
>>	static const struct wave5_match_data platform_name_wave515_data = {
>>		.flags = WAVE5_IS_DEC,
>>		.fw_name = "cnm/wave515_platform_name_fw.bin",
>>	};
>>
>> * add item to wave5_dt_ids[] like this:
>>
>>	{
>>		.compatible = "vendor,soc-wave515",
>>		.data = &platform_name_wave515_data,
>>	},
>>
>> * describe new compatible in
>>   Documentation/devicetree/bindings/media/cnm,wave521c.yaml
>>
>>Signed-off-by: Ivan Bornyakov <brnkv.i1@gmail.com>
>>---
>> .../platform/chips-media/wave5/wave5-helper.c |   3 +-
>> .../platform/chips-media/wave5/wave5-hw.c     | 245 ++++++++++++++----
>> .../chips-media/wave5/wave5-regdefine.h       |   5 +
>> .../platform/chips-media/wave5/wave5-vdi.c    |   6 +-
>> .../chips-media/wave5/wave5-vpu-dec.c         |  14 +-
>> .../platform/chips-media/wave5/wave5-vpu.c    |   8 +-
>> .../platform/chips-media/wave5/wave5-vpuapi.h |   1 +
>> .../chips-media/wave5/wave5-vpuconfig.h       |   9 +-
>> .../media/platform/chips-media/wave5/wave5.h  |   1 +
>> 9 files changed, 233 insertions(+), 59 deletions(-)
>>
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c
>b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>>index 8433ecab230c..c68f1e110ed9 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>>@@ -29,7 +29,8 @@ void wave5_cleanup_instance(struct vpu_instance *inst)
>> {
>> 	int i;
>>
>>-	if (list_is_singular(&inst->list))
>>+	if (list_is_singular(&inst->list) &&
>>+	    inst->dev->product_code != WAVE515_CODE)
>> 		wave5_vdi_free_sram(inst->dev);
>
>So you free the sram instead in unregister device, can you note that
>here please and maybe briefly explain why that is needed otherwise, one
>might assume the 515 doesn't use an SRAM.
>
>>
>> 	for (i = 0; i < inst->fbc_buf_count; i++)
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>>index cdd0a0948a94..e38995de6870 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>>@@ -24,8 +24,10 @@
>> #define FEATURE_HEVC_ENCODER		BIT(0)
>>
>> /* Decoder support fields */
>>+#define W515_FEATURE_HEVC10BIT_DEC	BIT(1)
>> #define FEATURE_AVC_DECODER		BIT(3)
>> #define FEATURE_HEVC_DECODER		BIT(2)
>>+#define W515_FEATURE_HEVC_DECODER	BIT(0)
>
>I don't understand the order of these entries.
>Either group the W515 ones or sort them by bit value or by codec but
>this just seems random.
>Also, as mentioned below please rename the other feature values, as you
>show that they are clearly not general purpose but for a specific
>device/device group.
>
>>
>> #define FEATURE_BACKBONE		BIT(16)
>> #define FEATURE_VCORE_BACKBONE		BIT(22)
>>@@ -155,6 +157,8 @@ 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 == WAVE515_CODE)
>>+		gdi_status_check_value = 0x0738;
>> 	if (vpu_dev->product_code == WAVE521C_CODE ||
>> 	    vpu_dev->product_code == WAVE521_CODE ||
>> 	    vpu_dev->product_code == WAVE521E1_CODE)
>>@@ -186,6 +190,8 @@ unsigned int wave5_vpu_get_product_id(struct
>vpu_device *vpu_dev)
>> 	u32 val = vpu_read_reg(vpu_dev, W5_PRODUCT_NUMBER);
>>
>> 	switch (val) {
>>+	case WAVE515_CODE:
>>+		return PRODUCT_ID_515;
>> 	case WAVE521C_CODE:
>> 		return PRODUCT_ID_521;
>> 	case WAVE521_CODE:
>>@@ -349,17 +355,33 @@ static int setup_wave5_properties(struct device
>*dev)
>> 	hw_config_def1 = vpu_read_reg(vpu_dev, W5_RET_STD_DEF1);
>> 	hw_config_feature = vpu_read_reg(vpu_dev, W5_RET_CONF_FEATURE);
>>
>>-	p_attr->support_hevc10bit_enc = FIELD_GET(FEATURE_HEVC10BIT_ENC,
>hw_config_feature);
>>-	p_attr->support_avc10bit_enc = FIELD_GET(FEATURE_AVC10BIT_ENC,
>hw_config_feature);
>>-
>>-	p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
>hw_config_def1) << STD_AVC;
>>-	p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
>hw_config_def1) << STD_HEVC;
>>-	p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
>hw_config_def1) << STD_AVC;
>>-	p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
>hw_config_def1) << STD_HEVC;
>>-
>>-	p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
>hw_config_def0);
>>-	p_attr->support_vcpu_backbone = FIELD_GET(FEATURE_VCPU_BACKBONE,
>hw_config_def0);
>>-	p_attr->support_vcore_backbone = FIELD_GET(FEATURE_VCORE_BACKBONE,
>hw_config_def0);
>>+	if (vpu_dev->product_code == WAVE515_CODE) {
>>+		p_attr->support_hevc10bit_dec =
>FIELD_GET(W515_FEATURE_HEVC10BIT_DEC,
>>+							  hw_config_feature);
>>+		p_attr->support_decoders =
>FIELD_GET(W515_FEATURE_HEVC_DECODER,
>>+						     hw_config_def1) << STD_HEVC;
>>+	} else {
>>+		p_attr->support_hevc10bit_enc =
>FIELD_GET(FEATURE_HEVC10BIT_ENC,
>
>Now that the Wave515 features have a product code, this will become
>quite confusing in a bit. Can you please rename the ones from WAVE521C
>like you did for Wave515?
>
>>+							  hw_config_feature);
>>+		p_attr->support_avc10bit_enc =
>FIELD_GET(FEATURE_AVC10BIT_ENC,
>>+							 hw_config_feature);
>>+
>>+		p_attr->support_decoders = FIELD_GET(FEATURE_AVC_DECODER,
>>+						     hw_config_def1) << STD_AVC;
>>+		p_attr->support_decoders |= FIELD_GET(FEATURE_HEVC_DECODER,
>>+						      hw_config_def1) << STD_HEVC;
>>+		p_attr->support_encoders = FIELD_GET(FEATURE_AVC_ENCODER,
>>+						     hw_config_def1) << STD_AVC;
>>+		p_attr->support_encoders |= FIELD_GET(FEATURE_HEVC_ENCODER,
>>+						      hw_config_def1) << STD_HEVC;
>>+
>>+		p_attr->support_backbone = FIELD_GET(FEATURE_BACKBONE,
>>+						     hw_config_def0);
>>+		p_attr->support_vcpu_backbone =
>FIELD_GET(FEATURE_VCPU_BACKBONE,
>>+							  hw_config_def0);
>>+		p_attr->support_vcore_backbone =
>FIELD_GET(FEATURE_VCORE_BACKBONE,
>>+							   hw_config_def0);
>>+	}
>>
>> 	setup_wave5_interrupts(vpu_dev);
>>
>>@@ -403,12 +425,18 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
>size_t size)
>> 	common_vb = &vpu_dev->common_mem;
>>
>> 	code_base = common_vb->daddr;
>>+
>>+	if (vpu_dev->product_code == WAVE515_CODE)
>>+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
>>+	else
>>+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
>
>This one is similar, if WAVE5_MAX_CODE_BUF_SIZE is not the maximum for
>all wave5 variations then we should rename it to be fitting for the one
>it was originally used for, e.g. Wave521C.
>>+
>> 	/* ALIGN TO 4KB */
>>-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>>+	code_size &= ~0xfff;
>> 	if (code_size < size * 2)
>> 		return -EINVAL;
>>
>>-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
>>+	temp_base = code_base + code_size;
>> 	temp_size = WAVE5_TEMPBUF_SIZE;
>>
>> 	ret = wave5_vdi_write_memory(vpu_dev, common_vb, 0, fw, size);
>>@@ -436,9 +464,12 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
>size_t size)
>>
>> 	/* These register must be reset explicitly */
>> 	vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>>-	wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>>-	wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>>-	vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>>+
>>+	if (vpu_dev->product_code != WAVE515_CODE) {
>>+		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>>+		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>>+		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>>+	}
>>
>> 	reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
>> 	if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
>>@@ -453,6 +484,24 @@ int wave5_vpu_init(struct device *dev, u8 *fw,
>size_t size)
>> 		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID, reg_val);
>> 	}
>>
>>+	if (vpu_dev->product_code == WAVE515_CODE) {
>>+		dma_addr_t task_buf_base;
>>+
>>+		vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>WAVE515_COMMAND_QUEUE_DEPTH);
>>+		vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>WAVE515_ONE_TASKBUF_SIZE);
>>+
>>+		for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>>+			task_buf_base = temp_base + temp_size +
>>+					(i * WAVE515_ONE_TASKBUF_SIZE);
>>+			vpu_write_reg(vpu_dev,
>>+				      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>>+				      task_buf_base);
>>+		}
>>+
>>+		vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI, vpu_dev-
>>sram_buf.daddr);
>>+		vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE, vpu_dev-
>>sram_buf.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);
>>@@ -493,29 +542,39 @@ int wave5_vpu_build_up_dec_param(struct
>vpu_instance *inst,
>> 		return -EINVAL;
>> 	}
>>
>>-	p_dec_info->vb_work.size = WAVE521DEC_WORKBUF_SIZE;
>>+	if (vpu_dev->product == PRODUCT_ID_515)
>>+		p_dec_info->vb_work.size = WAVE515DEC_WORKBUF_SIZE;
>>+	else
>>+		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);
>>+	if (inst->dev->product_code != WAVE515_CODE)
>>+		vpu_write_reg(inst->dev, W5_CMD_DEC_VCORE_INFO, 1);
>>
>> 	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_ADDR_SEC_AXI, vpu_dev-
>>sram_buf.daddr);
>>-	vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev-
>>sram_buf.size);
>>+	if (inst->dev->product_code != WAVE515_CODE) {
>>+		vpu_write_reg(inst->dev, W5_CMD_ADDR_SEC_AXI, vpu_dev-
>>sram_buf.daddr);
>>+		vpu_write_reg(inst->dev, W5_CMD_SEC_AXI_SIZE, vpu_dev-
>>sram_buf.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: SDMA reads MSB first */
>> 	vpu_write_reg(inst->dev, W5_CMD_BS_PARAM,
>BITSTREAM_ENDIANNESS_BIG_ENDIAN);
>>-	/* This register must be reset explicitly */
>>-	vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
>>-	vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1,
>(COMMAND_QUEUE_DEPTH - 1));
>>+
>>+	if (inst->dev->product_code != WAVE515_CODE) {
>>+		/* This register must be reset explicitly */
>>+		vpu_write_reg(inst->dev, W5_CMD_EXT_ADDR, 0);
>>+		vpu_write_reg(inst->dev, W5_CMD_NUM_CQ_DEPTH_M1,
>(COMMAND_QUEUE_DEPTH - 1));
>>+	}
>>
>> 	ret = send_firmware_command(inst, W5_CREATE_INSTANCE, true, NULL,
>NULL);
>> 	if (ret) {
>>@@ -566,7 +625,7 @@ static u32 get_bitstream_options(struct dec_info
>*info)
>> int wave5_vpu_dec_init_seq(struct vpu_instance *inst)
>> {
>> 	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
>>-	u32 cmd_option = INIT_SEQ_NORMAL;
>>+	u32 bs_option, cmd_option = INIT_SEQ_NORMAL;
>> 	u32 reg_val, fail_res;
>> 	int ret;
>>
>>@@ -576,7 +635,12 @@ int wave5_vpu_dec_init_seq(struct vpu_instance
>*inst)
>> 	vpu_write_reg(inst->dev, W5_BS_RD_PTR, p_dec_info->stream_rd_ptr);
>> 	vpu_write_reg(inst->dev, W5_BS_WR_PTR, p_dec_info->stream_wr_ptr);
>>
>>-	vpu_write_reg(inst->dev, W5_BS_OPTION,
>get_bitstream_options(p_dec_info));
>>+	bs_option = get_bitstream_options(p_dec_info);
>>+
>>+	if (inst->dev->product_code == WAVE515_CODE)
>>+		bs_option |= BSOPTION_RD_PTR_VALID_FLAG;
>
>Is it possible to add a brief comment here that describes why this is
>needed for that device and what this will change for the process?
>
>>+
>>+	vpu_write_reg(inst->dev, W5_BS_OPTION, bs_option);
>>
>> 	vpu_write_reg(inst->dev, W5_COMMAND_OPTION, cmd_option);
>> 	vpu_write_reg(inst->dev, W5_CMD_DEC_USER_MASK, p_dec_info-
>>user_data_enable);
>>@@ -642,10 +706,12 @@ static void wave5_get_dec_seq_result(struct
>vpu_instance *inst, struct dec_initi
>> 		info->profile = FIELD_GET(SEQ_PARAM_PROFILE_MASK, reg_val);
>> 	}
>>
>>-	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;
>>+	if (inst->dev->product_code != WAVE515_CODE) {
>>+		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;
>>+	}
>> }
>>
>> int wave5_vpu_dec_get_seq_info(struct vpu_instance *inst, struct
>dec_initial_info *info)
>>@@ -747,22 +813,27 @@ int wave5_vpu_dec_register_framebuffer(struct
>vpu_instance *inst, struct frame_b
>>
>> 		pic_size = (init_info->pic_width << 16) | (init_info-
>>pic_height);
>>
>>-		vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM) +
>>+		if (inst->dev->product_code != WAVE515_CODE) {
>>+			vb_buf.size = (p_dec_info->vlc_buf_size * VLC_BUF_NUM)
>+
>> 				(p_dec_info->param_buf_size *
>COMMAND_QUEUE_DEPTH);
>>-		vb_buf.daddr = 0;
>>+			vb_buf.daddr = 0;
>>
>>-		if (vb_buf.size != p_dec_info->vb_task.size) {
>>-			wave5_vdi_free_dma_memory(inst->dev, &p_dec_info-
>>vb_task);
>>-			ret = wave5_vdi_allocate_dma_memory(inst->dev,
>&vb_buf);
>>-			if (ret)
>>-				goto free_fbc_c_tbl_buffers;
>>+			if (vb_buf.size != p_dec_info->vb_task.size) {
>>+				wave5_vdi_free_dma_memory(inst->dev,
>>+							  &p_dec_info->vb_task);
>>+				ret = wave5_vdi_allocate_dma_memory(inst->dev,
>>+								    &vb_buf);
>>+				if (ret)
>>+					goto free_fbc_c_tbl_buffers;
>>
>>-			p_dec_info->vb_task = vb_buf;
>>-		}
>>+				p_dec_info->vb_task = vb_buf;
>>+			}
>>
>>-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
>>-			      p_dec_info->vb_task.daddr);
>>-		vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
>vb_buf.size);
>>+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_ADDR_TASK_BUF,
>>+				      p_dec_info->vb_task.daddr);
>>+			vpu_write_reg(inst->dev, W5_CMD_SET_FB_TASK_BUF_SIZE,
>>+				      vb_buf.size);
>>+		}
>> 	} else {
>> 		pic_size = (init_info->pic_width << 16) | (init_info-
>>pic_height);
>>
>>@@ -999,17 +1070,24 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
>> 	dma_addr_t code_base, temp_base;
>> 	dma_addr_t old_code_base, temp_size;
>> 	u32 code_size, reason_code;
>>-	u32 reg_val;
>>+	u32 i, reg_val;
>
>You only use the variable i within the conditional branch below, so you
>can just declare it there.
>
>> 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
>>
>> 	common_vb = &vpu_dev->common_mem;
>>
>> 	code_base = common_vb->daddr;
>>+
>>+	if (vpu_dev->product_code == WAVE515_CODE)
>>+		code_size = WAVE515_MAX_CODE_BUF_SIZE;
>>+	else
>>+		code_size = WAVE5_MAX_CODE_BUF_SIZE;
>>+
>> 	/* ALIGN TO 4KB */
>>-	code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>>+	code_size &= ~0xfff;
>> 	if (code_size < size * 2)
>> 		return -EINVAL;
>>-	temp_base = common_vb->daddr + WAVE5_TEMPBUF_OFFSET;
>>+
>>+	temp_base = code_base + code_size;
>> 	temp_size = WAVE5_TEMPBUF_SIZE;
>>
>> 	old_code_base = vpu_read_reg(vpu_dev, W5_VPU_REMAP_PADDR);
>>@@ -1043,9 +1121,12 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
>>
>> 		/* These register must be reset explicitly */
>> 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>>-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>>+
>>+		if (vpu_dev->product_code != WAVE515_CODE) {
>>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR,
>0);
>>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>>+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>>+		}
>>
>> 		reg_val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
>> 		if (FIELD_GET(FEATURE_BACKBONE, reg_val)) {
>>@@ -1060,6 +1141,28 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
>> 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID,
>reg_val);
>> 		}
>>
>>+		if (vpu_dev->product_code == WAVE515_CODE) {
>>+			dma_addr_t task_buf_base;
>
>As mentioned above you can declare the variable i here.
>
>>+
>>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>>+				      WAVE515_COMMAND_QUEUE_DEPTH);
>>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>>+				      WAVE515_ONE_TASKBUF_SIZE);
>>+
>>+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>>+				task_buf_base = temp_base + temp_size +
>>+						(i * WAVE515_ONE_TASKBUF_SIZE);
>>+				vpu_write_reg(vpu_dev,
>>+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>>+					      task_buf_base);
>>+			}
>>+
>>+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
>>+				      vpu_dev->sram_buf.daddr);
>>+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
>>+				      vpu_dev->sram_buf.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);
>>@@ -1081,10 +1184,10 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>size_t size)
>> static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake,
>const uint16_t *code,
>> 				size_t size)
>> {
>>-	u32 reg_val;
>>+	u32 i, reg_val;
>
>Like suggested in vpu_re_init please.
>
>> 	struct vpu_buf *common_vb;
>>-	dma_addr_t code_base;
>>-	u32 code_size, reason_code;
>>+	dma_addr_t code_base, temp_base;
>>+	u32 code_size, temp_size, reason_code;
>> 	struct vpu_device *vpu_dev = dev_get_drvdata(dev);
>> 	int ret;
>>
>>@@ -1114,13 +1217,22 @@ static int wave5_vpu_sleep_wake(struct device
>*dev, bool i_sleep_wake, const uin
>> 		common_vb = &vpu_dev->common_mem;
>>
>> 		code_base = common_vb->daddr;
>>+
>>+		if (vpu_dev->product_code == WAVE515_CODE)
>>+			code_size = WAVE515_MAX_CODE_BUF_SIZE;
>>+		else
>>+			code_size = WAVE5_MAX_CODE_BUF_SIZE;
>>+
>> 		/* ALIGN TO 4KB */
>>-		code_size = (WAVE5_MAX_CODE_BUF_SIZE & ~0xfff);
>>+		code_size &= ~0xfff;
>> 		if (code_size < size * 2) {
>> 			dev_err(dev, "size too small\n");
>> 			return -EINVAL;
>> 		}
>>
>>+		temp_base = code_base + code_size;
>>+		temp_size = WAVE5_TEMPBUF_SIZE;
>>+
>> 		/* Power on without DEBUG mode */
>> 		vpu_write_reg(vpu_dev, W5_PO_CONF, 0);
>>
>>@@ -1133,9 +1245,12 @@ static int wave5_vpu_sleep_wake(struct device
>*dev, bool i_sleep_wake, const uin
>>
>> 		/* These register must be reset explicitly */
>> 		vpu_write_reg(vpu_dev, W5_HW_OPTION, 0);
>>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR, 0);
>>-		wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>>-		vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>>+
>>+		if (vpu_dev->product_code != WAVE515_CODE) {
>>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROC_EXT_ADDR,
>0);
>>+			wave5_fio_writel(vpu_dev, W5_BACKBONE_AXI_PARAM, 0);
>>+			vpu_write_reg(vpu_dev, W5_SEC_AXI_PARAM, 0);
>>+		}
>>
>> 		setup_wave5_interrupts(vpu_dev);
>>
>>@@ -1152,6 +1267,28 @@ static int wave5_vpu_sleep_wake(struct device
>*dev, bool i_sleep_wake, const uin
>> 			wave5_fio_writel(vpu_dev, W5_BACKBONE_PROG_AXI_ID,
>reg_val);
>> 		}
>>
>>+		if (vpu_dev->product_code == WAVE515_CODE) {
>>+			dma_addr_t task_buf_base;
>>+
>>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_NUM_TASK_BUF,
>>+				      WAVE515_COMMAND_QUEUE_DEPTH);
>>+			vpu_write_reg(vpu_dev, W5_CMD_INIT_TASK_BUF_SIZE,
>>+				      WAVE515_ONE_TASKBUF_SIZE);
>>+
>>+			for (i = 0; i < WAVE515_COMMAND_QUEUE_DEPTH; i++) {
>>+				task_buf_base = temp_base + temp_size +
>>+						(i * WAVE515_ONE_TASKBUF_SIZE);
>>+				vpu_write_reg(vpu_dev,
>>+					      W5_CMD_INIT_ADDR_TASK_BUF0 + (i * 4),
>>+					      task_buf_base);
>>+			}
>>+
>>+			vpu_write_reg(vpu_dev, W515_CMD_ADDR_SEC_AXI,
>>+				      vpu_dev->sram_buf.daddr);
>>+			vpu_write_reg(vpu_dev, W515_CMD_SEC_AXI_SIZE,
>>+				      vpu_dev->sram_buf.size);
>>+		}
>>+
>> 		vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 1);
>> 		vpu_write_reg(vpu_dev, W5_COMMAND, W5_WAKEUP_VPU);
>> 		/* Start VPU after settings */
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>>index a15c6b2c3d8b..557344754c4c 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-regdefine.h
>>@@ -205,6 +205,9 @@ enum query_opt {
>> #define W5_ADDR_TEMP_BASE                       (W5_REG_BASE + 0x011C)
>> #define W5_TEMP_SIZE                            (W5_REG_BASE + 0x0120)
>> #define W5_HW_OPTION                            (W5_REG_BASE + 0x012C)
>>+#define W5_CMD_INIT_NUM_TASK_BUF		(W5_REG_BASE + 0x0134)
>>+#define W5_CMD_INIT_ADDR_TASK_BUF0		(W5_REG_BASE + 0x0138)
>>+#define W5_CMD_INIT_TASK_BUF_SIZE		(W5_REG_BASE + 0x0178)
>
>It looks like you are using tabs here, while the others utilize spaces.
>In general your tabs should expand to 8 whitespaces.
>(https://www.kernel.org/doc/html/v4.10/process/coding-
>style.html#indentation)
>
>> #define W5_SEC_AXI_PARAM                        (W5_REG_BASE + 0x0180)
>>
>>
>/************************************************************************
>/
>>@@ -216,7 +219,9 @@ enum query_opt {
>> #define W5_CMD_DEC_BS_SIZE                      (W5_REG_BASE + 0x0120)
>> #define W5_CMD_BS_PARAM                         (W5_REG_BASE + 0x0124)
>> #define W5_CMD_ADDR_SEC_AXI                     (W5_REG_BASE + 0x0130)
>>+#define W515_CMD_ADDR_SEC_AXI			(W5_REG_BASE + 0x0124)
>> #define W5_CMD_SEC_AXI_SIZE                     (W5_REG_BASE + 0x0134)
>>+#define W515_CMD_SEC_AXI_SIZE			(W5_REG_BASE + 0x0128)
>
>Same as above.
>
>> #define W5_CMD_EXT_ADDR                         (W5_REG_BASE + 0x0138)
>> #define W5_CMD_NUM_CQ_DEPTH_M1                  (W5_REG_BASE + 0x013C)
>> #define W5_CMD_ERR_CONCEAL                      (W5_REG_BASE + 0x0140)
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>>index a63fffed55e9..78888c57b6da 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-vdi.c
>>@@ -18,7 +18,11 @@ static int wave5_vdi_allocate_common_memory(struct
>device *dev)
>> 	if (!vpu_dev->common_mem.vaddr) {
>> 		int ret;
>>
>>-		vpu_dev->common_mem.size = SIZE_COMMON;
>>+		if (vpu_dev->product_code == WAVE515_CODE)
>>+			vpu_dev->common_mem.size = WAVE515_SIZE_COMMON;
>>+		else
>>+			vpu_dev->common_mem.size = SIZE_COMMON;
>>+
>> 		ret = wave5_vdi_allocate_dma_memory(vpu_dev, &vpu_dev-
>>common_mem);
>> 		if (ret) {
>> 			dev_err(dev, "unable to allocate common buffer\n");
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>>index 5a71a711f2e8..65a99053c5e8 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>>@@ -1869,7 +1869,8 @@ static int wave5_vpu_open_dec(struct file *filp)
>> 		goto cleanup_inst;
>> 	}
>>
>>-	wave5_vdi_allocate_sram(inst->dev);
>>+	if (inst->dev->product_code != WAVE515_CODE)
>>+		wave5_vdi_allocate_sram(inst->dev);
>>
>> 	return 0;
>>
>>@@ -1897,6 +1898,14 @@ int wave5_vpu_dec_register_device(struct
>vpu_device *dev)
>> 	struct video_device *vdev_dec;
>> 	int ret;
>>
>>+	/*
>>+	 * secondary AXI setup for Wave515 is done by INIT_VPU command,
>>+	 * that's why SRAM memory is allocated at VPU device register
>>+	 * rather than at device open.
>
>Just a nitpick, but if you use something that resembles more the actual
>function names it makes it easier to grep this part or to refer to the
>right function from reading this comment.
>So for example: vpu_open_dec & vpu_dec_register_device
>
>>+	 */
>>+	if (dev->product_code == WAVE515_CODE)
>>+		wave5_vdi_allocate_sram(dev);
>>+
>> 	vdev_dec = devm_kzalloc(dev->v4l2_dev.dev, sizeof(*vdev_dec),
>GFP_KERNEL);
>> 	if (!vdev_dec)
>> 		return -ENOMEM;
>>@@ -1930,6 +1939,9 @@ int wave5_vpu_dec_register_device(struct
>vpu_device *dev)
>>
>> void wave5_vpu_dec_unregister_device(struct vpu_device *dev)
>> {
>>+	if (dev->product_code == WAVE515_CODE)
>>+		wave5_vdi_free_sram(dev);
>
>Why does that need to happen here just for this one version of the Wave5
>codec?
>
>>+
>> 	video_unregister_device(dev->video_dev_dec);
>> 	if (dev->v4l2_m2m_dec_dev)
>> 		v4l2_m2m_release(dev->v4l2_m2m_dec_dev);
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>>index 2a972cddf4a6..fc267348399e 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>>@@ -60,7 +60,13 @@ static irqreturn_t wave5_vpu_irq_thread(int irq, void
>*dev_id)
>>
>> 			if (irq_reason & BIT(INT_WAVE5_INIT_SEQ) ||
>> 			    irq_reason & BIT(INT_WAVE5_ENC_SET_PARAM)) {
>>-				if (seq_done & BIT(inst->id)) {
>>+				if ((dev->product_code == WAVE515_CODE) &&
>>+				    (cmd_done & BIT(inst->id))) {
>>+					cmd_done &= ~BIT(inst->id);
>>+					wave5_vdi_write_register(dev,
>W5_RET_QUEUE_CMD_DONE_INST,
>>+								 cmd_done);
>>+					complete(&inst->irq_done);
>>+				} else if (seq_done & BIT(inst->id)) {
>> 					seq_done &= ~BIT(inst->id);
>> 					wave5_vdi_write_register(dev,
>W5_RET_SEQ_DONE_INSTANCE_INFO,
>> 								 seq_done);
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>>index 975d96b22191..601205df4433 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
>>@@ -18,6 +18,7 @@
>> #include "wave5-vdi.h"
>>
>> enum product_id {
>>+	PRODUCT_ID_515,
>> 	PRODUCT_ID_521,
>> 	PRODUCT_ID_511,
>> 	PRODUCT_ID_517,
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>>index 9d99afb78c89..b4128524fcfd 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>>+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
>>@@ -8,6 +8,7 @@
>> #ifndef _VPU_CONFIG_H_
>> #define _VPU_CONFIG_H_
>>
>>+#define WAVE515_CODE			0x5150
>
>Indentation again
>
>> #define WAVE517_CODE                    0x5170
>> #define WAVE537_CODE                    0x5370
>> #define WAVE511_CODE                    0x5110
>>@@ -21,12 +22,13 @@
>> 		((c) == WAVE517_CODE ||	(c) == WAVE537_CODE ||		\
>> 		 (c) == WAVE511_CODE || (c) == WAVE521_CODE ||		\
>> 		 (c) == WAVE521E1_CODE || (c) == WAVE521C_CODE ||	\
>>-		 (c) == WAVE521C_DUAL_CODE);				\
>>+		 (c) == WAVE521C_DUAL_CODE) || (c) == WAVE515_CODE;	\
>> })
>>
>> #define WAVE517_WORKBUF_SIZE            (2 * 1024 * 1024)
>> #define WAVE521ENC_WORKBUF_SIZE         (128 * 1024)      //HEVC 128K, AVC
>40K
>> #define WAVE521DEC_WORKBUF_SIZE         (1784 * 1024)
>>+#define WAVE515DEC_WORKBUF_SIZE		(2 * 1024 * 1024)
>>
>> #define WAVE5_MAX_SRAM_SIZE		(64 * 1024)
>>
>>@@ -52,16 +54,21 @@
>> #define VLC_BUF_NUM                     (2)
>>
>> #define COMMAND_QUEUE_DEPTH             (2)
>>+#define WAVE515_COMMAND_QUEUE_DEPTH	(4)
>>
>> #define W5_REMAP_INDEX0                 0
>> #define W5_REMAP_INDEX1                 1
>> #define W5_REMAP_MAX_SIZE               (1024 * 1024)
>>
>> #define WAVE5_MAX_CODE_BUF_SIZE         (2 * 1024 * 1024)
>>+#define WAVE515_MAX_CODE_BUF_SIZE	(1024 * 1024)
>> #define WAVE5_TEMPBUF_OFFSET            WAVE5_MAX_CODE_BUF_SIZE
>> #define WAVE5_TEMPBUF_SIZE              (1024 * 1024)
>>+#define WAVE515_TASKBUF_OFFSET		(WAVE515_MAX_CODE_BUF_SIZE +
>WAVE5_TEMPBUF_SIZE)
>>
>> #define SIZE_COMMON                 (WAVE5_MAX_CODE_BUF_SIZE +
>WAVE5_TEMPBUF_SIZE)
>
>Just as above, this macro looks like a general macro now, but as we have
>a Wave515 version this clearly isn't valid for all of them so please
>rename the others.
>
>>+#define WAVE515_ONE_TASKBUF_SIZE	(8 * 1024 * 1024)
>
>Would something speak against: (8 * WAVE5_TEMPBUF_SIZE)?
>(Especially as you use that macro for the calculation of the offset)
>And why the gap between the the size and the offset? I think it makes
>more sense to have them on top of each other
>
>Just as above the indentation don't look right here.
>
>>+#define WAVE515_SIZE_COMMON	(WAVE515_TASKBUF_OFFSET +
>WAVE515_COMMAND_QUEUE_DEPTH * WAVE515_ONE_TASKBUF_SIZE)
>>
>> //=====4. VPU REPORT MEMORY  ======================//
>>
>>diff --git a/drivers/media/platform/chips-media/wave5/wave5.h
>b/drivers/media/platform/chips-media/wave5/wave5.h
>>index 063028eccd3b..57b00e182b6e 100644
>>--- a/drivers/media/platform/chips-media/wave5/wave5.h
>>+++ b/drivers/media/platform/chips-media/wave5/wave5.h
>>@@ -22,6 +22,7 @@
>>  */
>> #define BSOPTION_ENABLE_EXPLICIT_END		BIT(0)
>> #define BSOPTION_HIGHLIGHT_STREAM_END		BIT(1)
>>+#define BSOPTION_RD_PTR_VALID_FLAG		BIT(31)
>
>As explained above when you use it, I think an explanatory comment is
>helpful here.
>
>Greetings,
>Sebastian
>>
>> /*
>>  * Currently the driver only supports hardware with little endian but
>for source
>>--
>>2.44.0
>>
>>