On 20/09/2023 04:28, Marvin Lin wrote:
> Add driver for Video Capture/Differentiation Engine (VCD) and Encoding
> Compression Engine (ECE) present on Nuvoton NPCM SoCs. As described in
> the datasheet NPCM750D_DS_Rev_1.0, the VCD can capture frames from
> digital video input and compare two frames in memory, and then the ECE
> can compress the frame data into HEXTILE format. This driver implements
> V4L2 interfaces and provides user controls to support KVM feature, also
> tested with VNC Viewer ver.6.22.826 and openbmc/obmc-ikvm.
>
> Signed-off-by: Marvin Lin <milkfafa@xxxxxxxxx>
I'm getting two sparse warnings:
drivers/media/platform/nuvoton/npcm-video.c:227:27: warning: incorrect type in argument 1 (different address spaces)
drivers/media/platform/nuvoton/npcm-video.c:227:27: expected void const volatile [noderef] __iomem *addr
drivers/media/platform/nuvoton/npcm-video.c:227:27: got void *
drivers/media/platform/nuvoton/npcm-video.c:1050:20: warning: context imbalance in 'npcm_video_irq' - different lock contexts for basic block
That last one is a missing unlock:
> +static irqreturn_t npcm_video_irq(int irq, void *arg)
> +{
> + struct npcm_video *video = arg;
> + struct regmap *vcd = video->vcd_regmap;
> + struct npcm_video_buffer *buf;
> + unsigned int index, size, status, fmt;
> + dma_addr_t dma_addr;
> + void *addr;
> + static const struct v4l2_event ev = {
> + .type = V4L2_EVENT_SOURCE_CHANGE,
> + .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> + };
> +
> + regmap_read(vcd, VCD_STAT, &status);
> + dev_dbg(video->dev, "VCD irq status 0x%x\n", status);
> +
> + regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
> +
> + if (test_bit(VIDEO_STOPPED, &video->flags) ||
> + !test_bit(VIDEO_STREAMING, &video->flags))
> + return IRQ_NONE;
> +
> + if (status & VCD_STAT_DONE) {
> + regmap_write(vcd, VCD_INTE, 0);
> + spin_lock(&video->lock);
> + clear_bit(VIDEO_CAPTURING, &video->flags);
> + buf = list_first_entry_or_null(&video->buffers,
> + struct npcm_video_buffer, link);
> + if (!buf) {
> + spin_unlock(&video->lock);
> + return IRQ_NONE;
> + }
> +
> + addr = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
> + index = buf->vb.vb2_buf.index;
> + fmt = video->pix_fmt.pixelformat;
> +
> + switch (fmt) {
> + case V4L2_PIX_FMT_RGB565:
> + size = npcm_video_raw(video, index, addr);
> + break;
> + case V4L2_PIX_FMT_HEXTILE:
> + dma_addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
> + size = npcm_video_hextile(video, index, dma_addr, addr);
> + break;
> + default:
Missing unlock here.
> + return IRQ_NONE;
> + }
> +
> + vb2_set_plane_payload(&buf->vb.vb2_buf, 0, size);
> + buf->vb.vb2_buf.timestamp = ktime_get_ns();
> + buf->vb.sequence = video->sequence++;
> + buf->vb.field = V4L2_FIELD_NONE;
> +
> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> + list_del(&buf->link);
> + spin_unlock(&video->lock);
> +
> + if (npcm_video_start_frame(video))
> + dev_err(video->dev, "Failed to capture next frame\n");
> + }
> +
> + /* Resolution changed */
> + if (status & VCD_STAT_VHT_CHG || status & VCD_STAT_HAC_CHG) {
> + if (!test_bit(VIDEO_RES_CHANGING, &video->flags)) {
> + set_bit(VIDEO_RES_CHANGING, &video->flags);
> +
> + vb2_queue_error(&video->queue);
> + v4l2_event_queue(&video->vdev, &ev);
> + }
> + }
> +
> + if (status & VCD_STAT_IFOR || status & VCD_STAT_IFOT) {
> + dev_warn(video->dev, "VCD FIFO overrun or over thresholds\n");
> + if (npcm_video_start_frame(video))
> + dev_err(video->dev, "Failed to recover from FIFO overrun\n");
> + }
> +
> + return IRQ_HANDLED;
> +}
Regards,
Hans
