Hi, Sebastian,
On Fri, Sep 15, 2023 at 23:11:34 +0200, Sebastian Fricke wrote:
> From: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
>
> 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 <sebastian.fricke@xxxxxxxxxxxxx>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> Signed-off-by: Robert Beckett <bob.beckett@xxxxxxxxxxxxx>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
> Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
[...]
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> new file mode 100644
> index 000000000000..a13d968f5d04
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
[...]
> +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?
