Hi, Sebastian,

On Fri, Sep 15, 2023 at 23:11:34 +0200, Sebastian Fricke wrote:

From: Nas Chung <nas.chung@chipsnmedia.com>

Add the decoder and encoder implementing the v4l2
API. This patch also adds the Makefile and the VIDEO_WAVE_VPU config

Signed-off-by: Sebastian Fricke <redacted>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Robert Beckett <redacted>
Signed-off-by: Dafna Hirschfeld <redacted>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

[...]

[...]

+static void wave5_vpu_get_interrupt_for_inst(struct vpu_instance *inst, u32 status)
+{
+	struct vpu_device *dev = inst->dev;
+	u32 seq_done;
+	u32 cmd_done;
+	int val;
+
+	seq_done = wave5_vdi_read_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO);
+	cmd_done = wave5_vdi_read_register(dev, W5_RET_QUEUE_CMD_DONE_INST);
+
+	if (status & BIT(INT_WAVE5_INIT_SEQ)) {
+		if (seq_done & BIT(inst->id)) {
+			seq_done &= ~BIT(inst->id);
+			wave5_vdi_write_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO, seq_done);
+			val = BIT(INT_WAVE5_INIT_SEQ);
+			kfifo_in(&inst->irq_status, &val, sizeof(int));
+		}
+	}
+	if (status & BIT(INT_WAVE5_ENC_SET_PARAM)) {
+		if (seq_done & BIT(inst->id)) {
+			seq_done &= ~BIT(inst->id);
+			wave5_vdi_write_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO, seq_done);
+			val = BIT(INT_WAVE5_ENC_SET_PARAM);
+			kfifo_in(&inst->irq_status, &val, sizeof(int));
+		}
+	}
+	if (status & BIT(INT_WAVE5_DEC_PIC) ||
+	    status & BIT(INT_WAVE5_ENC_PIC)) {
+		if (cmd_done & BIT(inst->id)) {
+			cmd_done &= ~BIT(inst->id);
+			wave5_vdi_write_register(dev, W5_RET_QUEUE_CMD_DONE_INST, cmd_done);
+			val = BIT(INT_WAVE5_DEC_PIC);
+			kfifo_in(&inst->irq_status, &val, sizeof(int));
+		}
+	}
+}
+
+static irqreturn_t wave5_vpu_irq(int irq, void *dev_id)
+{
+	struct vpu_device *dev = dev_id;
+
+	if (wave5_vdi_read_register(dev, W5_VPU_VPU_INT_STS)) {
+		struct vpu_instance *inst;
+		u32 irq_status = wave5_vdi_read_register(dev, W5_VPU_VINT_REASON);
+
+		list_for_each_entry(inst, &dev->instances, list) {
+			wave5_vpu_get_interrupt_for_inst(inst, irq_status);
+		}
+
+		wave5_vdi_write_register(dev, W5_VPU_VINT_REASON_CLR, irq_status);
+		wave5_vdi_write_register(dev, W5_VPU_VINT_CLEAR, 0x1);
+
+		return IRQ_WAKE_THREAD;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t wave5_vpu_irq_thread(int irq, void *dev_id)
+{
+	struct vpu_device *dev = dev_id;
+	struct vpu_instance *inst;
+	int irq_status, ret;
+
+	list_for_each_entry(inst, &dev->instances, list) {
+		while (kfifo_len(&inst->irq_status)) {
+			ret = kfifo_out(&inst->irq_status, &irq_status, sizeof(int));
+			if (!ret)
+				break;
+
+			if (irq_status == BIT(INT_WAVE5_INIT_SEQ) ||
+			    irq_status == BIT(INT_WAVE5_ENC_SET_PARAM))
+				complete(&inst->irq_done);
+			else /* DEC/ENC_PIC */
+				inst->ops->finish_process(inst);
+
+			wave5_vpu_clear_interrupt(inst, irq_status);
+		}
+	}
+
+	return IRQ_HANDLED;
+}

I believe, instead of
wave5_vpu_irq() + wave5_vpu_get_interrupt_for_inst() + wave5_vpu_irq_thread()
you can reduce interrupt handling to only threaded part with something like this:

static irqreturn_t wave5_vpu_irq_thread(int irq, void *dev_id)
{
	u32 irq_status, seq_done, cmd_done;
	struct vpu_device *dev = dev_id;
	struct vpu_instance *inst;

	while (wave5_vdi_read_register(dev, W5_VPU_VPU_INT_STS)) {
		irq_status = wave5_vdi_read_register(dev, W5_VPU_VINT_REASON);
		seq_done = wave5_vdi_read_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO);
		cmd_done = wave5_vdi_read_register(dev, W5_RET_QUEUE_CMD_DONE_INST);

		list_for_each_entry(inst, &dev->instances, list) {
			if (irq_status & BIT(INT_WAVE5_INIT_SEQ) ||
			    irq_status & BIT(INT_WAVE5_ENC_SET_PARAM)) {
				if (seq_done & BIT(inst->id)) {
					seq_done &= ~BIT(inst->id);
					wave5_vdi_write_register(dev,
								 W5_RET_SEQ_DONE_INSTANCE_INFO,
								 seq_done);
					complete(&inst->irq_done);
				}
			}

			if (status & BIT(INT_WAVE5_DEC_PIC) ||
			    status & BIT(INT_WAVE5_ENC_PIC)) {
				if (cmd_done & BIT(inst->id)) {
					cmd_done &= ~BIT(inst->id);
					wave5_vdi_write_register(dev,
								 W5_RET_QUEUE_CMD_DONE_INST,
								 cmd_done);
					inst->ops->finish_process(inst);
				}
			}

			wave5_vpu_clear_interrupt(inst, irq_status);
		}
			
		wave5_vdi_write_register(dev, W5_VPU_VINT_REASON_CLR, irq_status);
		wave5_vdi_write_register(dev, W5_VPU_VINT_CLEAR, 0x1);
	}

	return IRQ_HANDLED;
}

Is it better?

[...]

+static int wave5_vpu_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct vpu_device *dev;
+	const struct wave5_match_data *match_data;
+	u32 fw_revision;
+
+	match_data = device_get_match_data(&pdev->dev);
+	if (!match_data) {
+		dev_err(&pdev->dev, "missing device match data\n");
+		return -EINVAL;
+	}
+
+	/* physical addresses limited to 32 bits */
+	dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+	dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));

dma_set_mask_and_coherent()? Also error check?