[PATCH v3 0/7] add aspeed-jpeg support for aspeed-video

[PATCH v3 0/7] add aspeed-jpeg support for aspeed-video

[Jammy Huang]

The aim of this series is to add aspeed-jpeg support for aspeed-video
driver.

To achieve this major goal some refactors are included.

In the last, debugfs information is also updated per this change.

Jammy Huang (7):
  media: aspeed: move err-handling together to the bottom
  media: aspeed: use v4l2_info/v4l2_warn/v4l2_dbg for log
  media: aspeed: add more debug log messages
  media: aspeed: refactor to gather format/compress settings
  media: aspeed: Support aspeed mode to reduce compressed data
  media: aspeed: add comments and macro
  media: aspeed: Extend debug message

 drivers/media/platform/aspeed-video.c | 443 +++++++++++++++++++++-----
 1 file changed, 362 insertions(+), 81 deletions(-)

-- 
2.25.1

[PATCH v3 1/7] media: aspeed: move err-handling together to the bottom

[Jammy Huang]

refine aspeed_video_setup_video() flow.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 491447bf5186..6259cf17a7cc 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -1644,11 +1644,8 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
 
 	rc = video->ctrl_handler.error;
 	if (rc) {
-		v4l2_ctrl_handler_free(&video->ctrl_handler);
-		v4l2_device_unregister(v4l2_dev);
-
 		dev_err(video->dev, "Failed to init controls: %d\n", rc);
-		return rc;
+		goto err_ctrl_init;
 	}
 
 	v4l2_dev->ctrl_handler = &video->ctrl_handler;
@@ -1666,11 +1663,8 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
 
 	rc = vb2_queue_init(vbq);
 	if (rc) {
-		v4l2_ctrl_handler_free(&video->ctrl_handler);
-		v4l2_device_unregister(v4l2_dev);
-
 		dev_err(video->dev, "Failed to init vb2 queue\n");
-		return rc;
+		goto err_vb2_init;
 	}
 
 	vdev->queue = vbq;
@@ -1688,15 +1682,19 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
 	video_set_drvdata(vdev, video);
 	rc = video_register_device(vdev, VFL_TYPE_GRABBER, 0);
 	if (rc) {
-		vb2_queue_release(vbq);
-		v4l2_ctrl_handler_free(&video->ctrl_handler);
-		v4l2_device_unregister(v4l2_dev);
-
 		dev_err(video->dev, "Failed to register video device\n");
-		return rc;
+		goto err_video_reg;
 	}
 
 	return 0;
+
+err_video_reg:
+	vb2_queue_release(vbq);
+err_vb2_init:
+err_ctrl_init:
+	v4l2_ctrl_handler_free(&video->ctrl_handler);
+	v4l2_device_unregister(v4l2_dev);
+	return rc;
 }
 
 static int aspeed_video_init(struct aspeed_video *video)
-- 
2.25.1

[PATCH v3 2/7] media: aspeed: use v4l2_info/v4l2_warn/v4l2_dbg for log

[Jammy Huang]

The debug log level, 0~3, is controlled by module_param, debug.
The higher the value, the more the information.
  0: off
  1: info
  2: debug
  3: register operations

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 40 +++++++++++++++------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 6259cf17a7cc..789adc86d1af 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -30,6 +30,7 @@
 #include <media/v4l2-event.h>
 #include <media/v4l2-ioctl.h>
 #include <media/videobuf2-dma-contig.h>
+#include <linux/videodev2.h>
 
 #define DEVICE_NAME			"aspeed-video"
 
@@ -390,6 +391,8 @@ static const struct v4l2_dv_timings_cap aspeed_video_timings_cap = {
 	},
 };
 
+static unsigned int debug;
+
 static void aspeed_video_init_jpeg_table(u32 *table, bool yuv420)
 {
 	int i;
@@ -437,23 +440,23 @@ static void aspeed_video_update(struct aspeed_video *video, u32 reg, u32 clear,
 	t &= ~clear;
 	t |= bits;
 	writel(t, video->base + reg);
-	dev_dbg(video->dev, "update %03x[%08x -> %08x]\n", reg, before,
-		readl(video->base + reg));
+	v4l2_dbg(3, debug, &video->v4l2_dev, "update %03x[%08x -> %08x]\n",
+		 reg, before, readl(video->base + reg));
 }
 
 static u32 aspeed_video_read(struct aspeed_video *video, u32 reg)
 {
 	u32 t = readl(video->base + reg);
 
-	dev_dbg(video->dev, "read %03x[%08x]\n", reg, t);
+	v4l2_dbg(3, debug, &video->v4l2_dev, "read %03x[%08x]\n", reg, t);
 	return t;
 }
 
 static void aspeed_video_write(struct aspeed_video *video, u32 reg, u32 val)
 {
 	writel(val, video->base + reg);
-	dev_dbg(video->dev, "write %03x[%08x]\n", reg,
-		readl(video->base + reg));
+	v4l2_dbg(3, debug, &video->v4l2_dev, "write %03x[%08x]\n", reg,
+		 readl(video->base + reg));
 }
 
 static void update_perf(struct aspeed_video_perf *p)
@@ -474,13 +477,13 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
 	u32 seq_ctrl = aspeed_video_read(video, VE_SEQ_CTRL);
 
 	if (video->v4l2_input_status) {
-		dev_dbg(video->dev, "No signal; don't start frame\n");
+		v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
 		return 0;
 	}
 
 	if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
 	    !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
-		dev_dbg(video->dev, "Engine busy; don't start frame\n");
+		v4l2_warn(&video->v4l2_dev, "Engine busy; don't start frame\n");
 		return -EBUSY;
 	}
 
@@ -489,7 +492,7 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
 				       struct aspeed_video_buffer, link);
 	if (!buf) {
 		spin_unlock_irqrestore(&video->lock, flags);
-		dev_dbg(video->dev, "No buffers; don't start frame\n");
+		v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
 		return -EPROTO;
 	}
 
@@ -565,7 +568,7 @@ static void aspeed_video_bufs_done(struct aspeed_video *video,
 
 static void aspeed_video_irq_res_change(struct aspeed_video *video, ulong delay)
 {
-	dev_dbg(video->dev, "Resolution changed; resetting\n");
+	v4l2_dbg(1, debug, &video->v4l2_dev, "Resolution changed; resetting\n");
 
 	set_bit(VIDEO_RES_CHANGE, &video->flags);
 	clear_bit(VIDEO_FRAME_INPRG, &video->flags);
@@ -766,8 +769,8 @@ static void aspeed_video_calc_compressed_size(struct aspeed_video *video,
 	aspeed_video_write(video, VE_STREAM_BUF_SIZE,
 			   compression_buffer_size_reg);
 
-	dev_dbg(video->dev, "Max compressed size: %x\n",
-		video->max_compressed_size);
+	v4l2_dbg(1, debug, &video->v4l2_dev, "Max compressed size: %#x\n",
+		 video->max_compressed_size);
 }
 
 #define res_check(v) test_and_clear_bit(VIDEO_MODE_DETECT_DONE, &(v)->flags)
@@ -804,7 +807,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
 						      res_check(video),
 						      MODE_DETECT_TIMEOUT);
 		if (!rc) {
-			dev_dbg(video->dev, "Timed out; first mode detect\n");
+			v4l2_warn(&video->v4l2_dev, "Timed out; first mode detect\n");
 			clear_bit(VIDEO_RES_DETECT, &video->flags);
 			return;
 		}
@@ -822,7 +825,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
 						      MODE_DETECT_TIMEOUT);
 		clear_bit(VIDEO_RES_DETECT, &video->flags);
 		if (!rc) {
-			dev_dbg(video->dev, "Timed out; second mode detect\n");
+			v4l2_warn(&video->v4l2_dev, "Timed out; second mode detect\n");
 			return;
 		}
 
@@ -856,7 +859,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
 	} while (invalid_resolution && (tries++ < INVALID_RESOLUTION_RETRIES));
 
 	if (invalid_resolution) {
-		dev_dbg(video->dev, "Invalid resolution detected\n");
+		v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
 		return;
 	}
 
@@ -873,8 +876,8 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
 	aspeed_video_update(video, VE_SEQ_CTRL, 0,
 			    VE_SEQ_CTRL_AUTO_COMP | VE_SEQ_CTRL_EN_WATCHDOG);
 
-	dev_dbg(video->dev, "Got resolution: %dx%d\n", det->width,
-		det->height);
+	v4l2_dbg(1, debug, &video->v4l2_dev, "Got resolution: %dx%d\n",
+		 det->width, det->height);
 }
 
 static void aspeed_video_set_resolution(struct aspeed_video *video)
@@ -1501,7 +1504,7 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
 				!test_bit(VIDEO_FRAME_INPRG, &video->flags),
 				STOP_TIMEOUT);
 	if (!rc) {
-		dev_dbg(video->dev, "Timed out when stopping streaming\n");
+		v4l2_warn(&video->v4l2_dev, "Timed out when stopping streaming\n");
 
 		/*
 		 * Need to force stop any DMA and try and get HW into a good
@@ -1856,6 +1859,9 @@ static struct platform_driver aspeed_video_driver = {
 
 module_platform_driver(aspeed_video_driver);
 
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "Debug level (0=off,1=info,2=debug,3=reg ops)");
+
 MODULE_DESCRIPTION("ASPEED Video Engine Driver");
 MODULE_AUTHOR("Eddie James");
 MODULE_LICENSE("GPL v2");
-- 
2.25.1

[PATCH v3 3/7] media: aspeed: add more debug log messages

[Jammy Huang]

The new messages are listed as below:
1. jpeg header and capture buffer information
2. information for each irq
3. current capture mode, sync or direct-fetch
4. time consumed for each frame
5. input timing changed information

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 789adc86d1af..328f3ec125e2 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -461,12 +461,17 @@ static void aspeed_video_write(struct aspeed_video *video, u32 reg, u32 val)
 
 static void update_perf(struct aspeed_video_perf *p)
 {
+	struct aspeed_video *v = container_of(p, struct aspeed_video,
+					      perf);
+
 	p->duration =
 		ktime_to_ms(ktime_sub(ktime_get(),  p->last_sample));
 	p->totaltime += p->duration;
 
 	p->duration_max = max(p->duration, p->duration_max);
 	p->duration_min = min(p->duration, p->duration_min);
+	v4l2_dbg(2, debug, &v->v4l2_dev, "time consumed: %d ms\n",
+		 p->duration);
 }
 
 static int aspeed_video_start_frame(struct aspeed_video *video)
@@ -593,6 +598,12 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
 	struct aspeed_video *video = arg;
 	u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS);
 
+	v4l2_dbg(1, debug, &video->v4l2_dev, "irq sts=%#x %s%s%s%s\n", sts,
+		 sts & VE_INTERRUPT_MODE_DETECT_WD ? ", unlock" : "",
+		 sts & VE_INTERRUPT_MODE_DETECT ? ", lock" : "",
+		 sts & VE_INTERRUPT_CAPTURE_COMPLETE ? ", capture-done" : "",
+		 sts & VE_INTERRUPT_COMP_COMPLETE ? ", comp-done" : "");
+
 	/*
 	 * Resolution changed or signal was lost; reset the engine and
 	 * re-initialize
@@ -910,6 +921,7 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 
 	/* Don't use direct mode below 1024 x 768 (irqs don't fire) */
 	if (size < DIRECT_FETCH_THRESHOLD) {
+		v4l2_dbg(1, debug, &video->v4l2_dev, "Capture: Sync Mode\n");
 		aspeed_video_write(video, VE_TGS_0,
 				   FIELD_PREP(VE_TGS_FIRST,
 					      video->frame_left - 1) |
@@ -921,6 +933,7 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 					      video->frame_bottom + 1));
 		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
 	} else {
+		v4l2_dbg(1, debug, &video->v4l2_dev, "Capture: Direct Mode\n");
 		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
 	}
 
@@ -937,6 +950,10 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 		if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
 			goto err_mem;
 
+		v4l2_dbg(1, debug, &video->v4l2_dev, "src buf0 addr(%#x) size(%d)\n",
+			 video->srcs[0].dma, video->srcs[0].size);
+		v4l2_dbg(1, debug, &video->v4l2_dev, "src buf1 addr(%#x) size(%d)\n",
+			 video->srcs[1].dma, video->srcs[1].size);
 		aspeed_video_write(video, VE_SRC0_ADDR, video->srcs[0].dma);
 		aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
 	}
@@ -1201,6 +1218,9 @@ static int aspeed_video_set_dv_timings(struct file *file, void *fh,
 
 	timings->type = V4L2_DV_BT_656_1120;
 
+	v4l2_dbg(1, debug, &video->v4l2_dev, "set new timings(%dx%d)\n",
+		 timings->bt.width, timings->bt.height);
+
 	return 0;
 }
 
@@ -1383,6 +1403,7 @@ static void aspeed_video_resolution_work(struct work_struct *work)
 			.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
 		};
 
+		v4l2_dbg(1, debug, &video->v4l2_dev, "fire source change event\n");
 		v4l2_event_queue(&video->vdev, &ev);
 	} else if (test_bit(VIDEO_STREAMING, &video->flags)) {
 		/* No resolution change so just restart streaming */
@@ -1718,6 +1739,7 @@ static int aspeed_video_init(struct aspeed_video *video)
 		dev_err(dev, "Unable to request IRQ %d\n", irq);
 		return rc;
 	}
+	dev_info(video->dev, "irq %d\n", irq);
 
 	video->eclk = devm_clk_get(dev, "eclk");
 	if (IS_ERR(video->eclk)) {
@@ -1754,6 +1776,8 @@ static int aspeed_video_init(struct aspeed_video *video)
 		rc = -ENOMEM;
 		goto err_release_reserved_mem;
 	}
+	dev_info(video->dev, "alloc mem size(%d) at %#x for jpeg header\n",
+		 VE_JPEG_HEADER_SIZE, video->jpeg.dma);
 
 	aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
 
-- 
2.25.1

[PATCH v3 4/7] media: aspeed: refactor to gather format/compress settings

[Jammy Huang]

Add API, aspeed_video_update_regs(), to gather format/compress settings
which are controlled by user.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 69 ++++++++++++++-------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 328f3ec125e2..cafbffe1ef69 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -967,20 +967,42 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 		aspeed_video_free_buf(video, &video->srcs[0]);
 }
 
-static void aspeed_video_init_regs(struct aspeed_video *video)
+static void aspeed_video_update_regs(struct aspeed_video *video)
 {
 	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
 		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
 		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
-	u32 ctrl = VE_CTRL_AUTO_OR_CURSOR;
+	u32 ctrl = 0;
 	u32 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
 
+	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n",
+		 video->frame_rate);
+	v4l2_dbg(1, debug, &video->v4l2_dev, "subsample(%s)\n",
+		 video->yuv420 ? "420" : "444");
+	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d)\n",
+		 video->jpeg_quality);
+
 	if (video->frame_rate)
 		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
 
 	if (video->yuv420)
 		seq_ctrl |= VE_SEQ_CTRL_YUV420;
 
+	if (video->jpeg.virt)
+		aspeed_video_update_jpeg_table(video->jpeg.virt, video->yuv420);
+
+	/* Set control registers */
+	aspeed_video_update(video, VE_SEQ_CTRL,
+			    VE_SEQ_CTRL_JPEG_MODE | VE_SEQ_CTRL_YUV420,
+			    seq_ctrl);
+	aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, ctrl);
+	aspeed_video_update(video, VE_COMP_CTRL,
+			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
+			    comp_ctrl);
+}
+
+static void aspeed_video_init_regs(struct aspeed_video *video)
+{
 	/* Unlock VE registers */
 	aspeed_video_write(video, VE_PROTECTION_KEY, VE_PROTECTION_KEY_UNLOCK);
 
@@ -995,9 +1017,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
 	aspeed_video_write(video, VE_JPEG_ADDR, video->jpeg.dma);
 
 	/* Set control registers */
-	aspeed_video_write(video, VE_SEQ_CTRL, seq_ctrl);
-	aspeed_video_write(video, VE_CTRL, ctrl);
-	aspeed_video_write(video, VE_COMP_CTRL, comp_ctrl);
+	aspeed_video_write(video, VE_CTRL, VE_CTRL_AUTO_OR_CURSOR);
+	aspeed_video_write(video, VE_COMP_CTRL, VE_COMP_CTRL_RSVD);
 
 	/* Don't downscale */
 	aspeed_video_write(video, VE_SCALING_FACTOR, 0x10001000);
@@ -1326,27 +1347,6 @@ static const struct v4l2_ioctl_ops aspeed_video_ioctl_ops = {
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
-static void aspeed_video_update_jpeg_quality(struct aspeed_video *video)
-{
-	u32 comp_ctrl = FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
-		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
-
-	aspeed_video_update(video, VE_COMP_CTRL,
-			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
-			    comp_ctrl);
-}
-
-static void aspeed_video_update_subsampling(struct aspeed_video *video)
-{
-	if (video->jpeg.virt)
-		aspeed_video_update_jpeg_table(video->jpeg.virt, video->yuv420);
-
-	if (video->yuv420)
-		aspeed_video_update(video, VE_SEQ_CTRL, 0, VE_SEQ_CTRL_YUV420);
-	else
-		aspeed_video_update(video, VE_SEQ_CTRL, VE_SEQ_CTRL_YUV420, 0);
-}
-
 static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct aspeed_video *video = container_of(ctrl->handler,
@@ -1356,16 +1356,13 @@ static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
 	switch (ctrl->id) {
 	case V4L2_CID_JPEG_COMPRESSION_QUALITY:
 		video->jpeg_quality = ctrl->val;
-		aspeed_video_update_jpeg_quality(video);
+		if (test_bit(VIDEO_STREAMING, &video->flags))
+			aspeed_video_update_regs(video);
 		break;
 	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
-		if (ctrl->val == V4L2_JPEG_CHROMA_SUBSAMPLING_420) {
-			video->yuv420 = true;
-			aspeed_video_update_subsampling(video);
-		} else {
-			video->yuv420 = false;
-			aspeed_video_update_subsampling(video);
-		}
+		video->yuv420 = (ctrl->val == V4L2_JPEG_CHROMA_SUBSAMPLING_420);
+		if (test_bit(VIDEO_STREAMING, &video->flags))
+			aspeed_video_update_regs(video);
 		break;
 	default:
 		return -EINVAL;
@@ -1393,6 +1390,8 @@ static void aspeed_video_resolution_work(struct work_struct *work)
 
 	aspeed_video_init_regs(video);
 
+	aspeed_video_update_regs(video);
+
 	aspeed_video_get_resolution(video);
 
 	if (video->detected_timings.width != video->active_timings.width ||
@@ -1504,6 +1503,8 @@ static int aspeed_video_start_streaming(struct vb2_queue *q,
 	video->perf.duration_max = 0;
 	video->perf.duration_min = 0xffffffff;
 
+	aspeed_video_update_regs(video);
+
 	rc = aspeed_video_start_frame(video);
 	if (rc) {
 		aspeed_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
-- 
2.25.1

[PATCH v3 5/7] media: aspeed: Support aspeed mode to reduce compressed data

[Jammy Huang]

aspeed supports differential jpeg format which only compress the parts
which are changed. In this way, it reduces both the amount of data to be
transferred by network and those to be decoded on the client side.

4 new ctrls are added:
* Aspeed JPEG Format: to control aspeed's partial jpeg on/off
  0: standard jpeg, 1: aspeed jpeg
* Aspeed Compression Mode: to control aspeed's compression mode
  0: DCT Only, 1: DCT VQ mix 2-color, 2: DCT VQ mix 4-color
  This is AST2400 only. It will adapt JPEG or VQ encoding method according
  to the context automatically.
* Aspeed HQ Mode: to control aspeed's HQ mode on/off
  0: disabled, 1: enabled
* Aspeed HQ Quality: to control the quality of aspeed's HQ mode
  only useful if Aspeed HQ mode is enabled

Aspeed JPEG Format requires an additional buffer, called bcd, to store
the information about which macro block in the new frame is different
from the previous one.

To have bcd correctly working, we need to swap the buffers for src0/1 to
make src1 refer to previous frame and src0 to the coming new frame.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 222 +++++++++++++++++++++++---
 1 file changed, 203 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index cafbffe1ef69..94d17dee6f3d 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -32,6 +32,12 @@
 #include <media/videobuf2-dma-contig.h>
 #include <linux/videodev2.h>
 
+#define ASPEED_CID_CUSTOM_BASE			(V4L2_CID_USER_BASE | 0xf000)
+#define V4L2_CID_ASPEED_FORMAT			(ASPEED_CID_CUSTOM_BASE  + 1)
+#define V4L2_CID_ASPEED_COMPRESSION_MODE	(ASPEED_CID_CUSTOM_BASE  + 2)
+#define V4L2_CID_ASPEED_HQ_MODE			(ASPEED_CID_CUSTOM_BASE  + 3)
+#define V4L2_CID_ASPEED_HQ_JPEG_QUALITY		(ASPEED_CID_CUSTOM_BASE  + 4)
+
 #define DEVICE_NAME			"aspeed-video"
 
 #define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
@@ -55,6 +61,7 @@
 
 #define VE_MAX_SRC_BUFFER_SIZE		0x8ca000 /* 1920 * 1200, 32bpp */
 #define VE_JPEG_HEADER_SIZE		0x006000 /* 512 * 12 * 4 */
+#define VE_BCD_BUFF_SIZE		0x100000
 
 #define VE_PROTECTION_KEY		0x000
 #define  VE_PROTECTION_KEY_UNLOCK	0x1a038aa8
@@ -108,6 +115,13 @@
 #define VE_SCALING_FILTER2		0x020
 #define VE_SCALING_FILTER3		0x024
 
+#define VE_BCD_CTRL			0x02C
+#define  VE_BCD_CTRL_EN_BCD		BIT(0)
+#define  VE_BCD_CTRL_EN_ABCD		BIT(1)
+#define  VE_BCD_CTRL_EN_CB		BIT(2)
+#define  VE_BCD_CTRL_THR		GENMASK(23, 16)
+#define  VE_BCD_CTRL_ABCD_THR		GENMASK(31, 24)
+
 #define VE_CAP_WINDOW			0x030
 #define VE_COMP_WINDOW			0x034
 #define VE_COMP_PROC_OFFSET		0x038
@@ -116,6 +130,7 @@
 #define VE_SRC0_ADDR			0x044
 #define VE_SRC_SCANLINE_OFFSET		0x048
 #define VE_SRC1_ADDR			0x04c
+#define VE_BCD_ADDR			0x050
 #define VE_COMP_ADDR			0x054
 
 #define VE_STREAM_BUF_SIZE		0x058
@@ -136,6 +151,8 @@
 #define  VE_COMP_CTRL_HQ_DCT_CHR	GENMASK(26, 22)
 #define  VE_COMP_CTRL_HQ_DCT_LUM	GENMASK(31, 27)
 
+#define VE_CB_ADDR			0x06C
+
 #define VE_OFFSET_COMP_STREAM		0x078
 
 #define VE_JPEG_COMP_SIZE_READ_BACK	0x084
@@ -243,10 +260,15 @@ struct aspeed_video {
 	unsigned int max_compressed_size;
 	struct aspeed_video_addr srcs[2];
 	struct aspeed_video_addr jpeg;
+	struct aspeed_video_addr bcd;
 
 	bool yuv420;
+	bool partial_jpeg;
+	bool hq_mode;
 	unsigned int frame_rate;
 	unsigned int jpeg_quality;
+	unsigned int jpeg_hq_quality;
+	unsigned int compression_mode;
 
 	unsigned int frame_bottom;
 	unsigned int frame_left;
@@ -258,6 +280,13 @@ struct aspeed_video {
 
 #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
 
+static bool aspeed_video_alloc_buf(struct aspeed_video *video,
+				   struct aspeed_video_addr *addr,
+				   unsigned int size);
+
+static void aspeed_video_free_buf(struct aspeed_video *video,
+				  struct aspeed_video_addr *addr);
+
 static const u32 aspeed_video_jpeg_header[ASPEED_VIDEO_JPEG_HEADER_SIZE] = {
 	0xe0ffd8ff, 0x464a1000, 0x01004649, 0x60000101, 0x00006000, 0x0f00feff,
 	0x00002d05, 0x00000000, 0x00000000, 0x00dbff00
@@ -492,6 +521,20 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
 		return -EBUSY;
 	}
 
+	if (video->partial_jpeg && !video->bcd.size) {
+		if (!aspeed_video_alloc_buf(video, &video->bcd,
+					    VE_BCD_BUFF_SIZE)) {
+			dev_err(video->dev, "Failed to allocate BCD buffer\n");
+			dev_err(video->dev, "don't start frame\n");
+			return -ENOMEM;
+		}
+		aspeed_video_write(video, VE_BCD_ADDR, video->bcd.dma);
+		v4l2_dbg(1, debug, &video->v4l2_dev, "bcd addr(%#x) size(%d)\n",
+			video->bcd.dma, video->bcd.size);
+	} else if (!video->partial_jpeg && video->bcd.size) {
+		aspeed_video_free_buf(video, &video->bcd);
+	}
+
 	spin_lock_irqsave(&video->lock, flags);
 	buf = list_first_entry_or_null(&video->buffers,
 				       struct aspeed_video_buffer, link);
@@ -635,6 +678,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
 
 	if (sts & VE_INTERRUPT_COMP_COMPLETE) {
 		struct aspeed_video_buffer *buf;
+		bool empty = true;
 		u32 frame_size = aspeed_video_read(video,
 						   VE_JPEG_COMP_SIZE_READ_BACK);
 
@@ -648,13 +692,23 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
 		if (buf) {
 			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size);
 
-			if (!list_is_last(&buf->link, &video->buffers)) {
+			/*
+			 * partial_jpeg requires continuous update.
+			 * On the contrary, standard jpeg can keep last buffer
+			 * to always have the latest result.
+			 */
+			if (!video->partial_jpeg &&
+			    list_is_last(&buf->link, &video->buffers)) {
+				empty = false;
+				v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
+			} else {
 				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);
+				empty = list_empty(&video->buffers);
 			}
 		}
 		spin_unlock(&video->lock);
@@ -668,7 +722,18 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
 		aspeed_video_write(video, VE_INTERRUPT_STATUS,
 				   VE_INTERRUPT_COMP_COMPLETE);
 		sts &= ~VE_INTERRUPT_COMP_COMPLETE;
-		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
+
+		// swap src buffer if partial_jpeg
+		if (video->partial_jpeg) {
+			u32 src0, src1;
+
+			src0 = aspeed_video_read(video, VE_SRC0_ADDR);
+			src1 = aspeed_video_read(video, VE_SRC1_ADDR);
+			aspeed_video_write(video, VE_SRC0_ADDR, src1);
+			aspeed_video_write(video, VE_SRC1_ADDR, src0);
+		}
+
+		if (test_bit(VIDEO_STREAMING, &video->flags) && !empty)
 			aspeed_video_start_frame(video);
 	}
 
@@ -931,10 +996,14 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 				   FIELD_PREP(VE_TGS_FIRST, video->frame_top) |
 				   FIELD_PREP(VE_TGS_LAST,
 					      video->frame_bottom + 1));
-		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
+		aspeed_video_update(video, VE_CTRL,
+				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
+				    VE_CTRL_INT_DE);
 	} else {
 		v4l2_dbg(1, debug, &video->v4l2_dev, "Capture: Direct Mode\n");
-		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
+		aspeed_video_update(video, VE_CTRL,
+				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
+				    VE_CTRL_DIRECT_FETCH);
 	}
 
 	size *= 4;
@@ -969,35 +1038,70 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 
 static void aspeed_video_update_regs(struct aspeed_video *video)
 {
-	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
-		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
-		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
+	static const char * const compress_mode_str[] = {"DCT Only",
+		"DCT VQ mix 2-color", "DCT VQ mix 4-color"};
+	u32 comp_ctrl =	FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
+		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10) |
+		FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode) |
+		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_LUM, video->jpeg_hq_quality) |
+		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_CHR, video->jpeg_hq_quality |
+			   0x10);
 	u32 ctrl = 0;
-	u32 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
+	u32 seq_ctrl = 0;
 
-	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n",
-		 video->frame_rate);
-	v4l2_dbg(1, debug, &video->v4l2_dev, "subsample(%s)\n",
+	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n", video->frame_rate);
+	v4l2_dbg(1, debug, &video->v4l2_dev, "jpeg format(%s) subsample(%s)\n",
+		 video->partial_jpeg ? "partial" : "standard",
 		 video->yuv420 ? "420" : "444");
-	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d)\n",
-		 video->jpeg_quality);
+	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d) hq(%s) hq_quality(%d)\n",
+		 video->jpeg_quality, video->hq_mode ? "on" : "off",
+		 video->jpeg_hq_quality);
+	v4l2_dbg(1, debug, &video->v4l2_dev, "compression mode(%s)\n",
+		 compress_mode_str[video->compression_mode]);
+
+	if (video->partial_jpeg)
+		aspeed_video_update(video, VE_BCD_CTRL, 0, VE_BCD_CTRL_EN_BCD);
+	else
+		aspeed_video_update(video, VE_BCD_CTRL, VE_BCD_CTRL_EN_BCD, 0);
 
 	if (video->frame_rate)
 		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
 
+	if (!video->partial_jpeg) {
+		comp_ctrl &= ~FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode);
+		seq_ctrl |= VE_SEQ_CTRL_JPEG_MODE;
+	}
+
 	if (video->yuv420)
 		seq_ctrl |= VE_SEQ_CTRL_YUV420;
 
 	if (video->jpeg.virt)
 		aspeed_video_update_jpeg_table(video->jpeg.virt, video->yuv420);
 
+#ifdef CONFIG_MACH_ASPEED_G4
+	switch (video->compression_mode) {
+	case 0:	//DCT only
+		comp_ctrl |= VE_COMP_CTRL_VQ_DCT_ONLY;
+		break;
+	case 1:	//DCT VQ mix 2-color
+		comp_ctrl &= ~(VE_COMP_CTRL_VQ_4COLOR | VE_COMP_CTRL_VQ_DCT_ONLY);
+		break;
+	case 2:	//DCT VQ mix 4-color
+		comp_ctrl |= VE_COMP_CTRL_VQ_4COLOR;
+		break;
+	}
+#endif
+
 	/* Set control registers */
 	aspeed_video_update(video, VE_SEQ_CTRL,
 			    VE_SEQ_CTRL_JPEG_MODE | VE_SEQ_CTRL_YUV420,
 			    seq_ctrl);
 	aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, ctrl);
 	aspeed_video_update(video, VE_COMP_CTRL,
-			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
+			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR |
+			    VE_COMP_CTRL_EN_HQ | VE_COMP_CTRL_HQ_DCT_LUM |
+			    VE_COMP_CTRL_HQ_DCT_CHR | VE_COMP_CTRL_VQ_4COLOR |
+			    VE_COMP_CTRL_VQ_DCT_ONLY,
 			    comp_ctrl);
 }
 
@@ -1029,6 +1133,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
 
 	/* Set mode detection defaults */
 	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
+
+	aspeed_video_write(video, VE_BCD_CTRL, 0);
 }
 
 static void aspeed_video_start(struct aspeed_video *video)
@@ -1062,6 +1168,9 @@ static void aspeed_video_stop(struct aspeed_video *video)
 	if (video->srcs[1].size)
 		aspeed_video_free_buf(video, &video->srcs[1]);
 
+	if (video->bcd.size)
+		aspeed_video_free_buf(video, &video->bcd);
+
 	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
 	video->flags = 0;
 }
@@ -1364,6 +1473,28 @@ static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
 		if (test_bit(VIDEO_STREAMING, &video->flags))
 			aspeed_video_update_regs(video);
 		break;
+	case V4L2_CID_ASPEED_FORMAT:
+		video->partial_jpeg = ctrl->val;
+		if (test_bit(VIDEO_STREAMING, &video->flags))
+			aspeed_video_update_regs(video);
+		break;
+#ifdef CONFIG_MACH_ASPEED_G4
+	case V4L2_CID_ASPEED_COMPRESSION_MODE:
+		video->compression_mode = ctrl->val;
+		if (test_bit(VIDEO_STREAMING, &video->flags))
+			aspeed_video_update_regs(video);
+		break;
+#endif
+	case V4L2_CID_ASPEED_HQ_MODE:
+		video->hq_mode = ctrl->val;
+		if (test_bit(VIDEO_STREAMING, &video->flags))
+			aspeed_video_update_regs(video);
+		break;
+	case V4L2_CID_ASPEED_HQ_JPEG_QUALITY:
+		video->jpeg_hq_quality = ctrl->val;
+		if (test_bit(VIDEO_STREAMING, &video->flags))
+			aspeed_video_update_regs(video);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -1375,6 +1506,52 @@ static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
 	.s_ctrl = aspeed_video_set_ctrl,
 };
 
+static const struct v4l2_ctrl_config aspeed_ctrl_format = {
+	.ops = &aspeed_video_ctrl_ops,
+	.id = V4L2_CID_ASPEED_FORMAT,
+	.name = "Aspeed JPEG Format",
+	.type = V4L2_CTRL_TYPE_BOOLEAN,
+	.min = false,
+	.max = true,
+	.step = 1,
+	.def = false,
+};
+
+#ifdef CONFIG_MACH_ASPEED_G4
+static const struct v4l2_ctrl_config aspeed_ctrl_compression_mode = {
+	.ops = &aspeed_video_ctrl_ops,
+	.id = V4L2_CID_ASPEED_COMPRESSION_MODE,
+	.name = "Aspeed Compression Mode",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.min = 0,
+	.max = 2,
+	.step = 1,
+	.def = 0,
+};
+#endif
+
+static const struct v4l2_ctrl_config aspeed_ctrl_HQ_mode = {
+	.ops = &aspeed_video_ctrl_ops,
+	.id = V4L2_CID_ASPEED_HQ_MODE,
+	.name = "Aspeed HQ Mode",
+	.type = V4L2_CTRL_TYPE_BOOLEAN,
+	.min = false,
+	.max = true,
+	.step = 1,
+	.def = false,
+};
+
+static const struct v4l2_ctrl_config aspeed_ctrl_HQ_jpeg_quality = {
+	.ops = &aspeed_video_ctrl_ops,
+	.id = V4L2_CID_ASPEED_HQ_JPEG_QUALITY,
+	.name = "Aspeed HQ Quality",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.min = 0,
+	.max = ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1,
+	.step = 1,
+	.def = 0,
+};
+
 static void aspeed_video_resolution_work(struct work_struct *work)
 {
 	struct delayed_work *dwork = to_delayed_work(work);
@@ -1644,6 +1821,7 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
 	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
 	struct vb2_queue *vbq = &video->queue;
 	struct video_device *vdev = &video->vdev;
+	struct v4l2_ctrl_handler *hdl = &video->ctrl_handler;
 	int rc;
 
 	video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
@@ -1658,22 +1836,28 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
 		return rc;
 	}
 
-	v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
-	v4l2_ctrl_new_std(&video->ctrl_handler, &aspeed_video_ctrl_ops,
+	v4l2_ctrl_handler_init(hdl, 6);
+	v4l2_ctrl_new_std(hdl, &aspeed_video_ctrl_ops,
 			  V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
 			  ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1, 1, 0);
-	v4l2_ctrl_new_std_menu(&video->ctrl_handler, &aspeed_video_ctrl_ops,
+	v4l2_ctrl_new_std_menu(hdl, &aspeed_video_ctrl_ops,
 			       V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
 			       V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
 			       V4L2_JPEG_CHROMA_SUBSAMPLING_444);
+	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_format, NULL);
+#ifdef CONFIG_MACH_ASPEED_G4
+	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_compression_mode, NULL);
+#endif
+	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_mode, NULL);
+	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_jpeg_quality, NULL);
 
-	rc = video->ctrl_handler.error;
+	rc = hdl->error;
 	if (rc) {
 		dev_err(video->dev, "Failed to init controls: %d\n", rc);
 		goto err_ctrl_init;
 	}
 
-	v4l2_dev->ctrl_handler = &video->ctrl_handler;
+	v4l2_dev->ctrl_handler = hdl;
 
 	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
-- 
2.25.1

Re: [PATCH v3 5/7] media: aspeed: Support aspeed mode to reduce compressed data

[Hans Verkuil]

On 22/10/2021 08:55, Jammy Huang wrote:
> aspeed supports differential jpeg format which only compress the parts
> which are changed. In this way, it reduces both the amount of data to be
> transferred by network and those to be decoded on the client side.
> 
> 4 new ctrls are added:
> * Aspeed JPEG Format: to control aspeed's partial jpeg on/off
>   0: standard jpeg, 1: aspeed jpeg

What exactly is 'aspeed jpeg'?

> * Aspeed Compression Mode: to control aspeed's compression mode
>   0: DCT Only, 1: DCT VQ mix 2-color, 2: DCT VQ mix 4-color
>   This is AST2400 only. It will adapt JPEG or VQ encoding method according
>   to the context automatically.

What exactly does this do?

Is this very aspeed-specific, or could this be a standard JPEG control?

> * Aspeed HQ Mode: to control aspeed's HQ mode on/off
>   0: disabled, 1: enabled
> * Aspeed HQ Quality: to control the quality of aspeed's HQ mode
>   only useful if Aspeed HQ mode is enabled

Can these two controls be replaced by the existing V4L2_CID_JPEG_COMPRESSION_QUALITY
control? I.e.: range 1..12 is non-HQ, 13-24 is HQ. Note that the spec recommends
that value 0 is not used in the V4L2_CID_JPEG_COMPRESSION_QUALITY range:

https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/ext-ctrls-jpeg.html

> 
> Aspeed JPEG Format requires an additional buffer, called bcd, to store
> the information about which macro block in the new frame is different
> from the previous one.
> 
> To have bcd correctly working, we need to swap the buffers for src0/1 to
> make src1 refer to previous frame and src0 to the coming new frame.
> 
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>  drivers/media/platform/aspeed-video.c | 222 +++++++++++++++++++++++---
>  1 file changed, 203 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index cafbffe1ef69..94d17dee6f3d 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -32,6 +32,12 @@
>  #include <media/videobuf2-dma-contig.h>
>  #include <linux/videodev2.h>
>  
> +#define ASPEED_CID_CUSTOM_BASE			(V4L2_CID_USER_BASE | 0xf000)

Driver-specific control ID ranges must be reserved in include/uapi/linux/v4l2-controls.h.
See e.g. V4L2_CID_USER_ALLEGRO_BASE.

Regards,

	Hans

> +#define V4L2_CID_ASPEED_FORMAT			(ASPEED_CID_CUSTOM_BASE  + 1)
> +#define V4L2_CID_ASPEED_COMPRESSION_MODE	(ASPEED_CID_CUSTOM_BASE  + 2)
> +#define V4L2_CID_ASPEED_HQ_MODE			(ASPEED_CID_CUSTOM_BASE  + 3)
> +#define V4L2_CID_ASPEED_HQ_JPEG_QUALITY		(ASPEED_CID_CUSTOM_BASE  + 4)
> +
>  #define DEVICE_NAME			"aspeed-video"
>  
>  #define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
> @@ -55,6 +61,7 @@
>  
>  #define VE_MAX_SRC_BUFFER_SIZE		0x8ca000 /* 1920 * 1200, 32bpp */
>  #define VE_JPEG_HEADER_SIZE		0x006000 /* 512 * 12 * 4 */
> +#define VE_BCD_BUFF_SIZE		0x100000
>  
>  #define VE_PROTECTION_KEY		0x000
>  #define  VE_PROTECTION_KEY_UNLOCK	0x1a038aa8
> @@ -108,6 +115,13 @@
>  #define VE_SCALING_FILTER2		0x020
>  #define VE_SCALING_FILTER3		0x024
>  
> +#define VE_BCD_CTRL			0x02C
> +#define  VE_BCD_CTRL_EN_BCD		BIT(0)
> +#define  VE_BCD_CTRL_EN_ABCD		BIT(1)
> +#define  VE_BCD_CTRL_EN_CB		BIT(2)
> +#define  VE_BCD_CTRL_THR		GENMASK(23, 16)
> +#define  VE_BCD_CTRL_ABCD_THR		GENMASK(31, 24)
> +
>  #define VE_CAP_WINDOW			0x030
>  #define VE_COMP_WINDOW			0x034
>  #define VE_COMP_PROC_OFFSET		0x038
> @@ -116,6 +130,7 @@
>  #define VE_SRC0_ADDR			0x044
>  #define VE_SRC_SCANLINE_OFFSET		0x048
>  #define VE_SRC1_ADDR			0x04c
> +#define VE_BCD_ADDR			0x050
>  #define VE_COMP_ADDR			0x054
>  
>  #define VE_STREAM_BUF_SIZE		0x058
> @@ -136,6 +151,8 @@
>  #define  VE_COMP_CTRL_HQ_DCT_CHR	GENMASK(26, 22)
>  #define  VE_COMP_CTRL_HQ_DCT_LUM	GENMASK(31, 27)
>  
> +#define VE_CB_ADDR			0x06C
> +
>  #define VE_OFFSET_COMP_STREAM		0x078
>  
>  #define VE_JPEG_COMP_SIZE_READ_BACK	0x084
> @@ -243,10 +260,15 @@ struct aspeed_video {
>  	unsigned int max_compressed_size;
>  	struct aspeed_video_addr srcs[2];
>  	struct aspeed_video_addr jpeg;
> +	struct aspeed_video_addr bcd;
>  
>  	bool yuv420;
> +	bool partial_jpeg;
> +	bool hq_mode;
>  	unsigned int frame_rate;
>  	unsigned int jpeg_quality;
> +	unsigned int jpeg_hq_quality;
> +	unsigned int compression_mode;
>  
>  	unsigned int frame_bottom;
>  	unsigned int frame_left;
> @@ -258,6 +280,13 @@ struct aspeed_video {
>  
>  #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
>  
> +static bool aspeed_video_alloc_buf(struct aspeed_video *video,
> +				   struct aspeed_video_addr *addr,
> +				   unsigned int size);
> +
> +static void aspeed_video_free_buf(struct aspeed_video *video,
> +				  struct aspeed_video_addr *addr);
> +
>  static const u32 aspeed_video_jpeg_header[ASPEED_VIDEO_JPEG_HEADER_SIZE] = {
>  	0xe0ffd8ff, 0x464a1000, 0x01004649, 0x60000101, 0x00006000, 0x0f00feff,
>  	0x00002d05, 0x00000000, 0x00000000, 0x00dbff00
> @@ -492,6 +521,20 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>  		return -EBUSY;
>  	}
>  
> +	if (video->partial_jpeg && !video->bcd.size) {
> +		if (!aspeed_video_alloc_buf(video, &video->bcd,
> +					    VE_BCD_BUFF_SIZE)) {
> +			dev_err(video->dev, "Failed to allocate BCD buffer\n");
> +			dev_err(video->dev, "don't start frame\n");
> +			return -ENOMEM;
> +		}
> +		aspeed_video_write(video, VE_BCD_ADDR, video->bcd.dma);
> +		v4l2_dbg(1, debug, &video->v4l2_dev, "bcd addr(%#x) size(%d)\n",
> +			video->bcd.dma, video->bcd.size);
> +	} else if (!video->partial_jpeg && video->bcd.size) {
> +		aspeed_video_free_buf(video, &video->bcd);
> +	}
> +
>  	spin_lock_irqsave(&video->lock, flags);
>  	buf = list_first_entry_or_null(&video->buffers,
>  				       struct aspeed_video_buffer, link);
> @@ -635,6 +678,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>  
>  	if (sts & VE_INTERRUPT_COMP_COMPLETE) {
>  		struct aspeed_video_buffer *buf;
> +		bool empty = true;
>  		u32 frame_size = aspeed_video_read(video,
>  						   VE_JPEG_COMP_SIZE_READ_BACK);
>  
> @@ -648,13 +692,23 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>  		if (buf) {
>  			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size);
>  
> -			if (!list_is_last(&buf->link, &video->buffers)) {
> +			/*
> +			 * partial_jpeg requires continuous update.
> +			 * On the contrary, standard jpeg can keep last buffer
> +			 * to always have the latest result.
> +			 */
> +			if (!video->partial_jpeg &&
> +			    list_is_last(&buf->link, &video->buffers)) {
> +				empty = false;
> +				v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
> +			} else {
>  				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);
> +				empty = list_empty(&video->buffers);
>  			}
>  		}
>  		spin_unlock(&video->lock);
> @@ -668,7 +722,18 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>  		aspeed_video_write(video, VE_INTERRUPT_STATUS,
>  				   VE_INTERRUPT_COMP_COMPLETE);
>  		sts &= ~VE_INTERRUPT_COMP_COMPLETE;
> -		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
> +
> +		// swap src buffer if partial_jpeg
> +		if (video->partial_jpeg) {
> +			u32 src0, src1;
> +
> +			src0 = aspeed_video_read(video, VE_SRC0_ADDR);
> +			src1 = aspeed_video_read(video, VE_SRC1_ADDR);
> +			aspeed_video_write(video, VE_SRC0_ADDR, src1);
> +			aspeed_video_write(video, VE_SRC1_ADDR, src0);
> +		}
> +
> +		if (test_bit(VIDEO_STREAMING, &video->flags) && !empty)
>  			aspeed_video_start_frame(video);
>  	}
>  
> @@ -931,10 +996,14 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>  				   FIELD_PREP(VE_TGS_FIRST, video->frame_top) |
>  				   FIELD_PREP(VE_TGS_LAST,
>  					      video->frame_bottom + 1));
> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
> +		aspeed_video_update(video, VE_CTRL,
> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
> +				    VE_CTRL_INT_DE);
>  	} else {
>  		v4l2_dbg(1, debug, &video->v4l2_dev, "Capture: Direct Mode\n");
> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
> +		aspeed_video_update(video, VE_CTRL,
> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
> +				    VE_CTRL_DIRECT_FETCH);
>  	}
>  
>  	size *= 4;
> @@ -969,35 +1038,70 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>  
>  static void aspeed_video_update_regs(struct aspeed_video *video)
>  {
> -	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
> -		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
> -		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
> +	static const char * const compress_mode_str[] = {"DCT Only",
> +		"DCT VQ mix 2-color", "DCT VQ mix 4-color"};
> +	u32 comp_ctrl =	FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
> +		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10) |
> +		FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode) |
> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_LUM, video->jpeg_hq_quality) |
> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_CHR, video->jpeg_hq_quality |
> +			   0x10);
>  	u32 ctrl = 0;
> -	u32 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
> +	u32 seq_ctrl = 0;
>  
> -	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n",
> -		 video->frame_rate);
> -	v4l2_dbg(1, debug, &video->v4l2_dev, "subsample(%s)\n",
> +	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n", video->frame_rate);
> +	v4l2_dbg(1, debug, &video->v4l2_dev, "jpeg format(%s) subsample(%s)\n",
> +		 video->partial_jpeg ? "partial" : "standard",
>  		 video->yuv420 ? "420" : "444");
> -	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d)\n",
> -		 video->jpeg_quality);
> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d) hq(%s) hq_quality(%d)\n",
> +		 video->jpeg_quality, video->hq_mode ? "on" : "off",
> +		 video->jpeg_hq_quality);
> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression mode(%s)\n",
> +		 compress_mode_str[video->compression_mode]);
> +
> +	if (video->partial_jpeg)
> +		aspeed_video_update(video, VE_BCD_CTRL, 0, VE_BCD_CTRL_EN_BCD);
> +	else
> +		aspeed_video_update(video, VE_BCD_CTRL, VE_BCD_CTRL_EN_BCD, 0);
>  
>  	if (video->frame_rate)
>  		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
>  
> +	if (!video->partial_jpeg) {
> +		comp_ctrl &= ~FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode);
> +		seq_ctrl |= VE_SEQ_CTRL_JPEG_MODE;
> +	}
> +
>  	if (video->yuv420)
>  		seq_ctrl |= VE_SEQ_CTRL_YUV420;
>  
>  	if (video->jpeg.virt)
>  		aspeed_video_update_jpeg_table(video->jpeg.virt, video->yuv420);
>  
> +#ifdef CONFIG_MACH_ASPEED_G4
> +	switch (video->compression_mode) {
> +	case 0:	//DCT only
> +		comp_ctrl |= VE_COMP_CTRL_VQ_DCT_ONLY;
> +		break;
> +	case 1:	//DCT VQ mix 2-color
> +		comp_ctrl &= ~(VE_COMP_CTRL_VQ_4COLOR | VE_COMP_CTRL_VQ_DCT_ONLY);
> +		break;
> +	case 2:	//DCT VQ mix 4-color
> +		comp_ctrl |= VE_COMP_CTRL_VQ_4COLOR;
> +		break;
> +	}
> +#endif
> +
>  	/* Set control registers */
>  	aspeed_video_update(video, VE_SEQ_CTRL,
>  			    VE_SEQ_CTRL_JPEG_MODE | VE_SEQ_CTRL_YUV420,
>  			    seq_ctrl);
>  	aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, ctrl);
>  	aspeed_video_update(video, VE_COMP_CTRL,
> -			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
> +			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR |
> +			    VE_COMP_CTRL_EN_HQ | VE_COMP_CTRL_HQ_DCT_LUM |
> +			    VE_COMP_CTRL_HQ_DCT_CHR | VE_COMP_CTRL_VQ_4COLOR |
> +			    VE_COMP_CTRL_VQ_DCT_ONLY,
>  			    comp_ctrl);
>  }
>  
> @@ -1029,6 +1133,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>  
>  	/* Set mode detection defaults */
>  	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
> +
> +	aspeed_video_write(video, VE_BCD_CTRL, 0);
>  }
>  
>  static void aspeed_video_start(struct aspeed_video *video)
> @@ -1062,6 +1168,9 @@ static void aspeed_video_stop(struct aspeed_video *video)
>  	if (video->srcs[1].size)
>  		aspeed_video_free_buf(video, &video->srcs[1]);
>  
> +	if (video->bcd.size)
> +		aspeed_video_free_buf(video, &video->bcd);
> +
>  	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
>  	video->flags = 0;
>  }
> @@ -1364,6 +1473,28 @@ static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
>  		if (test_bit(VIDEO_STREAMING, &video->flags))
>  			aspeed_video_update_regs(video);
>  		break;
> +	case V4L2_CID_ASPEED_FORMAT:
> +		video->partial_jpeg = ctrl->val;
> +		if (test_bit(VIDEO_STREAMING, &video->flags))
> +			aspeed_video_update_regs(video);
> +		break;
> +#ifdef CONFIG_MACH_ASPEED_G4
> +	case V4L2_CID_ASPEED_COMPRESSION_MODE:
> +		video->compression_mode = ctrl->val;
> +		if (test_bit(VIDEO_STREAMING, &video->flags))
> +			aspeed_video_update_regs(video);
> +		break;
> +#endif
> +	case V4L2_CID_ASPEED_HQ_MODE:
> +		video->hq_mode = ctrl->val;
> +		if (test_bit(VIDEO_STREAMING, &video->flags))
> +			aspeed_video_update_regs(video);
> +		break;
> +	case V4L2_CID_ASPEED_HQ_JPEG_QUALITY:
> +		video->jpeg_hq_quality = ctrl->val;
> +		if (test_bit(VIDEO_STREAMING, &video->flags))
> +			aspeed_video_update_regs(video);
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1375,6 +1506,52 @@ static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
>  	.s_ctrl = aspeed_video_set_ctrl,
>  };
>  
> +static const struct v4l2_ctrl_config aspeed_ctrl_format = {
> +	.ops = &aspeed_video_ctrl_ops,
> +	.id = V4L2_CID_ASPEED_FORMAT,
> +	.name = "Aspeed JPEG Format",
> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
> +	.min = false,
> +	.max = true,
> +	.step = 1,
> +	.def = false,
> +};
> +
> +#ifdef CONFIG_MACH_ASPEED_G4
> +static const struct v4l2_ctrl_config aspeed_ctrl_compression_mode = {
> +	.ops = &aspeed_video_ctrl_ops,
> +	.id = V4L2_CID_ASPEED_COMPRESSION_MODE,
> +	.name = "Aspeed Compression Mode",
> +	.type = V4L2_CTRL_TYPE_INTEGER,
> +	.min = 0,
> +	.max = 2,
> +	.step = 1,
> +	.def = 0,
> +};
> +#endif
> +
> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_mode = {
> +	.ops = &aspeed_video_ctrl_ops,
> +	.id = V4L2_CID_ASPEED_HQ_MODE,
> +	.name = "Aspeed HQ Mode",
> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
> +	.min = false,
> +	.max = true,
> +	.step = 1,
> +	.def = false,
> +};
> +
> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_jpeg_quality = {
> +	.ops = &aspeed_video_ctrl_ops,
> +	.id = V4L2_CID_ASPEED_HQ_JPEG_QUALITY,
> +	.name = "Aspeed HQ Quality",
> +	.type = V4L2_CTRL_TYPE_INTEGER,
> +	.min = 0,
> +	.max = ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1,
> +	.step = 1,
> +	.def = 0,
> +};
> +
>  static void aspeed_video_resolution_work(struct work_struct *work)
>  {
>  	struct delayed_work *dwork = to_delayed_work(work);
> @@ -1644,6 +1821,7 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>  	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
>  	struct vb2_queue *vbq = &video->queue;
>  	struct video_device *vdev = &video->vdev;
> +	struct v4l2_ctrl_handler *hdl = &video->ctrl_handler;
>  	int rc;
>  
>  	video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
> @@ -1658,22 +1836,28 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>  		return rc;
>  	}
>  
> -	v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
> -	v4l2_ctrl_new_std(&video->ctrl_handler, &aspeed_video_ctrl_ops,
> +	v4l2_ctrl_handler_init(hdl, 6);
> +	v4l2_ctrl_new_std(hdl, &aspeed_video_ctrl_ops,
>  			  V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
>  			  ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1, 1, 0);
> -	v4l2_ctrl_new_std_menu(&video->ctrl_handler, &aspeed_video_ctrl_ops,
> +	v4l2_ctrl_new_std_menu(hdl, &aspeed_video_ctrl_ops,
>  			       V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
>  			       V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
>  			       V4L2_JPEG_CHROMA_SUBSAMPLING_444);
> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_format, NULL);
> +#ifdef CONFIG_MACH_ASPEED_G4
> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_compression_mode, NULL);
> +#endif
> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_mode, NULL);
> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_jpeg_quality, NULL);
>  
> -	rc = video->ctrl_handler.error;
> +	rc = hdl->error;
>  	if (rc) {
>  		dev_err(video->dev, "Failed to init controls: %d\n", rc);
>  		goto err_ctrl_init;
>  	}
>  
> -	v4l2_dev->ctrl_handler = &video->ctrl_handler;
> +	v4l2_dev->ctrl_handler = hdl;
>  
>  	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
> 

Re: [PATCH v3 5/7] media: aspeed: Support aspeed mode to reduce compressed data

[Jammy Huang]

Dear Hans,

Thanks for your review.

On 2021/11/8 下午 05:09, Hans Verkuil wrote:
> On 22/10/2021 08:55, Jammy Huang wrote:
>> aspeed supports differential jpeg format which only compress the parts
>> which are changed. In this way, it reduces both the amount of data to be
>> transferred by network and those to be decoded on the client side.
>>
>> 4 new ctrls are added:
>> * Aspeed JPEG Format: to control aspeed's partial jpeg on/off
>>    0: standard jpeg, 1: aspeed jpeg
> What exactly is 'aspeed jpeg'?

'aspeed jpeg' is aspeed proprietary compression format. It will compare 
the new frame
with previous one to decide which macroblock's data is changed, and only 
the changed
macroblock will be compressed. In this way, the amount of compressed data is
significantly reduced. This is similar to the concept of I/P-frames of 
video compression.

For example, the video is static in first 3 frames and then a object in 
the video moved in
4th frame.

         Frame Content      |    Standard    |    Aspeed
-----------------------------------------------------------------------
1th                                 |    Full             | Full
2th    identical with 1st   |    Full             |   none (only about 
12 Bytes for header data)
3th    identical with 1st   |    Full             |   none
4th    a object moved     |    Full             |   Only the Macroblocks 
that have data changed are compressed

I have implemented a javascript aspeed decoder in novnc to support this 
format, but
the performance isn't good enough. I am working on a web-assembly to 
improve it.

>> * Aspeed Compression Mode: to control aspeed's compression mode
>>    0: DCT Only, 1: DCT VQ mix 2-color, 2: DCT VQ mix 4-color
>>    This is AST2400 only. It will adapt JPEG or VQ encoding method according
>>    to the context automatically.
> What exactly does this do?
>
> Is this very aspeed-specific, or could this be a standard JPEG control?
Yes, this is aspeed-specific. Its compression algorithm is a modified 
JPEG algorithm.
>
>> * Aspeed HQ Mode: to control aspeed's HQ mode on/off
>>    0: disabled, 1: enabled
>> * Aspeed HQ Quality: to control the quality of aspeed's HQ mode
>>    only useful if Aspeed HQ mode is enabled
> Can these two controls be replaced by the existing V4L2_CID_JPEG_COMPRESSION_QUALITY
> control? I.e.: range 1..12 is non-HQ, 13-24 is HQ. Note that the spec recommends
> that value 0 is not used in the V4L2_CID_JPEG_COMPRESSION_QUALITY range:
>
> https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/ext-ctrls-jpeg.html
HQ mode is aspeed-specific and not related to stanard jpeg's compression 
quality. These two
quality attribute can be controlled independently. So, I think it is not 
proper to integrate them
together.

Noted, I will modified the min value of these 2 quality control.

>
>> Aspeed JPEG Format requires an additional buffer, called bcd, to store
>> the information about which macro block in the new frame is different
>> from the previous one.
>>
>> To have bcd correctly working, we need to swap the buffers for src0/1 to
>> make src1 refer to previous frame and src0 to the coming new frame.
>>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>>   drivers/media/platform/aspeed-video.c | 222 +++++++++++++++++++++++---
>>   1 file changed, 203 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index cafbffe1ef69..94d17dee6f3d 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -32,6 +32,12 @@
>>   #include <media/videobuf2-dma-contig.h>
>>   #include <linux/videodev2.h>
>>   
>> +#define ASPEED_CID_CUSTOM_BASE			(V4L2_CID_USER_BASE | 0xf000)
> Driver-specific control ID ranges must be reserved in include/uapi/linux/v4l2-controls.h.
> See e.g. V4L2_CID_USER_ALLEGRO_BASE.
OKay, I will update in the next patch.
>
> Regards,
>
> 	Hans
>
>> +#define V4L2_CID_ASPEED_FORMAT			(ASPEED_CID_CUSTOM_BASE  + 1)
>> +#define V4L2_CID_ASPEED_COMPRESSION_MODE	(ASPEED_CID_CUSTOM_BASE  + 2)
>> +#define V4L2_CID_ASPEED_HQ_MODE			(ASPEED_CID_CUSTOM_BASE  + 3)
>> +#define V4L2_CID_ASPEED_HQ_JPEG_QUALITY		(ASPEED_CID_CUSTOM_BASE  + 4)
>> +
>>   #define DEVICE_NAME			"aspeed-video"
>>   
>>   #define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
>> @@ -55,6 +61,7 @@
>>   
>>   #define VE_MAX_SRC_BUFFER_SIZE		0x8ca000 /* 1920 * 1200, 32bpp */
>>   #define VE_JPEG_HEADER_SIZE		0x006000 /* 512 * 12 * 4 */
>> +#define VE_BCD_BUFF_SIZE		0x100000
>>   
>>   #define VE_PROTECTION_KEY		0x000
>>   #define  VE_PROTECTION_KEY_UNLOCK	0x1a038aa8
>> @@ -108,6 +115,13 @@
>>   #define VE_SCALING_FILTER2		0x020
>>   #define VE_SCALING_FILTER3		0x024
>>   
>> +#define VE_BCD_CTRL			0x02C
>> +#define  VE_BCD_CTRL_EN_BCD		BIT(0)
>> +#define  VE_BCD_CTRL_EN_ABCD		BIT(1)
>> +#define  VE_BCD_CTRL_EN_CB		BIT(2)
>> +#define  VE_BCD_CTRL_THR		GENMASK(23, 16)
>> +#define  VE_BCD_CTRL_ABCD_THR		GENMASK(31, 24)
>> +
>>   #define VE_CAP_WINDOW			0x030
>>   #define VE_COMP_WINDOW			0x034
>>   #define VE_COMP_PROC_OFFSET		0x038
>> @@ -116,6 +130,7 @@
>>   #define VE_SRC0_ADDR			0x044
>>   #define VE_SRC_SCANLINE_OFFSET		0x048
>>   #define VE_SRC1_ADDR			0x04c
>> +#define VE_BCD_ADDR			0x050
>>   #define VE_COMP_ADDR			0x054
>>   
>>   #define VE_STREAM_BUF_SIZE		0x058
>> @@ -136,6 +151,8 @@
>>   #define  VE_COMP_CTRL_HQ_DCT_CHR	GENMASK(26, 22)
>>   #define  VE_COMP_CTRL_HQ_DCT_LUM	GENMASK(31, 27)
>>   
>> +#define VE_CB_ADDR			0x06C
>> +
>>   #define VE_OFFSET_COMP_STREAM		0x078
>>   
>>   #define VE_JPEG_COMP_SIZE_READ_BACK	0x084
>> @@ -243,10 +260,15 @@ struct aspeed_video {
>>   	unsigned int max_compressed_size;
>>   	struct aspeed_video_addr srcs[2];
>>   	struct aspeed_video_addr jpeg;
>> +	struct aspeed_video_addr bcd;
>>   
>>   	bool yuv420;
>> +	bool partial_jpeg;
>> +	bool hq_mode;
>>   	unsigned int frame_rate;
>>   	unsigned int jpeg_quality;
>> +	unsigned int jpeg_hq_quality;
>> +	unsigned int compression_mode;
>>   
>>   	unsigned int frame_bottom;
>>   	unsigned int frame_left;
>> @@ -258,6 +280,13 @@ struct aspeed_video {
>>   
>>   #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
>>   
>> +static bool aspeed_video_alloc_buf(struct aspeed_video *video,
>> +				   struct aspeed_video_addr *addr,
>> +				   unsigned int size);
>> +
>> +static void aspeed_video_free_buf(struct aspeed_video *video,
>> +				  struct aspeed_video_addr *addr);
>> +
>>   static const u32 aspeed_video_jpeg_header[ASPEED_VIDEO_JPEG_HEADER_SIZE] = {
>>   	0xe0ffd8ff, 0x464a1000, 0x01004649, 0x60000101, 0x00006000, 0x0f00feff,
>>   	0x00002d05, 0x00000000, 0x00000000, 0x00dbff00
>> @@ -492,6 +521,20 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>>   		return -EBUSY;
>>   	}
>>   
>> +	if (video->partial_jpeg && !video->bcd.size) {
>> +		if (!aspeed_video_alloc_buf(video, &video->bcd,
>> +					    VE_BCD_BUFF_SIZE)) {
>> +			dev_err(video->dev, "Failed to allocate BCD buffer\n");
>> +			dev_err(video->dev, "don't start frame\n");
>> +			return -ENOMEM;
>> +		}
>> +		aspeed_video_write(video, VE_BCD_ADDR, video->bcd.dma);
>> +		v4l2_dbg(1, debug, &video->v4l2_dev, "bcd addr(%#x) size(%d)\n",
>> +			video->bcd.dma, video->bcd.size);
>> +	} else if (!video->partial_jpeg && video->bcd.size) {
>> +		aspeed_video_free_buf(video, &video->bcd);
>> +	}
>> +
>>   	spin_lock_irqsave(&video->lock, flags);
>>   	buf = list_first_entry_or_null(&video->buffers,
>>   				       struct aspeed_video_buffer, link);
>> @@ -635,6 +678,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>   
>>   	if (sts & VE_INTERRUPT_COMP_COMPLETE) {
>>   		struct aspeed_video_buffer *buf;
>> +		bool empty = true;
>>   		u32 frame_size = aspeed_video_read(video,
>>   						   VE_JPEG_COMP_SIZE_READ_BACK);
>>   
>> @@ -648,13 +692,23 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>   		if (buf) {
>>   			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size);
>>   
>> -			if (!list_is_last(&buf->link, &video->buffers)) {
>> +			/*
>> +			 * partial_jpeg requires continuous update.
>> +			 * On the contrary, standard jpeg can keep last buffer
>> +			 * to always have the latest result.
>> +			 */
>> +			if (!video->partial_jpeg &&
>> +			    list_is_last(&buf->link, &video->buffers)) {
>> +				empty = false;
>> +				v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
>> +			} else {
>>   				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);
>> +				empty = list_empty(&video->buffers);
>>   			}
>>   		}
>>   		spin_unlock(&video->lock);
>> @@ -668,7 +722,18 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>   		aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>   				   VE_INTERRUPT_COMP_COMPLETE);
>>   		sts &= ~VE_INTERRUPT_COMP_COMPLETE;
>> -		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
>> +
>> +		// swap src buffer if partial_jpeg
>> +		if (video->partial_jpeg) {
>> +			u32 src0, src1;
>> +
>> +			src0 = aspeed_video_read(video, VE_SRC0_ADDR);
>> +			src1 = aspeed_video_read(video, VE_SRC1_ADDR);
>> +			aspeed_video_write(video, VE_SRC0_ADDR, src1);
>> +			aspeed_video_write(video, VE_SRC1_ADDR, src0);
>> +		}
>> +
>> +		if (test_bit(VIDEO_STREAMING, &video->flags) && !empty)
>>   			aspeed_video_start_frame(video);
>>   	}
>>   
>> @@ -931,10 +996,14 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>>   				   FIELD_PREP(VE_TGS_FIRST, video->frame_top) |
>>   				   FIELD_PREP(VE_TGS_LAST,
>>   					      video->frame_bottom + 1));
>> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
>> +		aspeed_video_update(video, VE_CTRL,
>> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
>> +				    VE_CTRL_INT_DE);
>>   	} else {
>>   		v4l2_dbg(1, debug, &video->v4l2_dev, "Capture: Direct Mode\n");
>> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
>> +		aspeed_video_update(video, VE_CTRL,
>> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
>> +				    VE_CTRL_DIRECT_FETCH);
>>   	}
>>   
>>   	size *= 4;
>> @@ -969,35 +1038,70 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>>   
>>   static void aspeed_video_update_regs(struct aspeed_video *video)
>>   {
>> -	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
>> -		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>> -		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
>> +	static const char * const compress_mode_str[] = {"DCT Only",
>> +		"DCT VQ mix 2-color", "DCT VQ mix 4-color"};
>> +	u32 comp_ctrl =	FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>> +		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10) |
>> +		FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode) |
>> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_LUM, video->jpeg_hq_quality) |
>> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_CHR, video->jpeg_hq_quality |
>> +			   0x10);
>>   	u32 ctrl = 0;
>> -	u32 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
>> +	u32 seq_ctrl = 0;
>>   
>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n",
>> -		 video->frame_rate);
>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "subsample(%s)\n",
>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n", video->frame_rate);
>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "jpeg format(%s) subsample(%s)\n",
>> +		 video->partial_jpeg ? "partial" : "standard",
>>   		 video->yuv420 ? "420" : "444");
>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d)\n",
>> -		 video->jpeg_quality);
>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d) hq(%s) hq_quality(%d)\n",
>> +		 video->jpeg_quality, video->hq_mode ? "on" : "off",
>> +		 video->jpeg_hq_quality);
>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression mode(%s)\n",
>> +		 compress_mode_str[video->compression_mode]);
>> +
>> +	if (video->partial_jpeg)
>> +		aspeed_video_update(video, VE_BCD_CTRL, 0, VE_BCD_CTRL_EN_BCD);
>> +	else
>> +		aspeed_video_update(video, VE_BCD_CTRL, VE_BCD_CTRL_EN_BCD, 0);
>>   
>>   	if (video->frame_rate)
>>   		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
>>   
>> +	if (!video->partial_jpeg) {
>> +		comp_ctrl &= ~FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode);
>> +		seq_ctrl |= VE_SEQ_CTRL_JPEG_MODE;
>> +	}
>> +
>>   	if (video->yuv420)
>>   		seq_ctrl |= VE_SEQ_CTRL_YUV420;
>>   
>>   	if (video->jpeg.virt)
>>   		aspeed_video_update_jpeg_table(video->jpeg.virt, video->yuv420);
>>   
>> +#ifdef CONFIG_MACH_ASPEED_G4
>> +	switch (video->compression_mode) {
>> +	case 0:	//DCT only
>> +		comp_ctrl |= VE_COMP_CTRL_VQ_DCT_ONLY;
>> +		break;
>> +	case 1:	//DCT VQ mix 2-color
>> +		comp_ctrl &= ~(VE_COMP_CTRL_VQ_4COLOR | VE_COMP_CTRL_VQ_DCT_ONLY);
>> +		break;
>> +	case 2:	//DCT VQ mix 4-color
>> +		comp_ctrl |= VE_COMP_CTRL_VQ_4COLOR;
>> +		break;
>> +	}
>> +#endif
>> +
>>   	/* Set control registers */
>>   	aspeed_video_update(video, VE_SEQ_CTRL,
>>   			    VE_SEQ_CTRL_JPEG_MODE | VE_SEQ_CTRL_YUV420,
>>   			    seq_ctrl);
>>   	aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, ctrl);
>>   	aspeed_video_update(video, VE_COMP_CTRL,
>> -			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
>> +			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR |
>> +			    VE_COMP_CTRL_EN_HQ | VE_COMP_CTRL_HQ_DCT_LUM |
>> +			    VE_COMP_CTRL_HQ_DCT_CHR | VE_COMP_CTRL_VQ_4COLOR |
>> +			    VE_COMP_CTRL_VQ_DCT_ONLY,
>>   			    comp_ctrl);
>>   }
>>   
>> @@ -1029,6 +1133,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>>   
>>   	/* Set mode detection defaults */
>>   	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
>> +
>> +	aspeed_video_write(video, VE_BCD_CTRL, 0);
>>   }
>>   
>>   static void aspeed_video_start(struct aspeed_video *video)
>> @@ -1062,6 +1168,9 @@ static void aspeed_video_stop(struct aspeed_video *video)
>>   	if (video->srcs[1].size)
>>   		aspeed_video_free_buf(video, &video->srcs[1]);
>>   
>> +	if (video->bcd.size)
>> +		aspeed_video_free_buf(video, &video->bcd);
>> +
>>   	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
>>   	video->flags = 0;
>>   }
>> @@ -1364,6 +1473,28 @@ static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
>>   		if (test_bit(VIDEO_STREAMING, &video->flags))
>>   			aspeed_video_update_regs(video);
>>   		break;
>> +	case V4L2_CID_ASPEED_FORMAT:
>> +		video->partial_jpeg = ctrl->val;
>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>> +			aspeed_video_update_regs(video);
>> +		break;
>> +#ifdef CONFIG_MACH_ASPEED_G4
>> +	case V4L2_CID_ASPEED_COMPRESSION_MODE:
>> +		video->compression_mode = ctrl->val;
>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>> +			aspeed_video_update_regs(video);
>> +		break;
>> +#endif
>> +	case V4L2_CID_ASPEED_HQ_MODE:
>> +		video->hq_mode = ctrl->val;
>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>> +			aspeed_video_update_regs(video);
>> +		break;
>> +	case V4L2_CID_ASPEED_HQ_JPEG_QUALITY:
>> +		video->jpeg_hq_quality = ctrl->val;
>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>> +			aspeed_video_update_regs(video);
>> +		break;
>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -1375,6 +1506,52 @@ static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
>>   	.s_ctrl = aspeed_video_set_ctrl,
>>   };
>>   
>> +static const struct v4l2_ctrl_config aspeed_ctrl_format = {
>> +	.ops = &aspeed_video_ctrl_ops,
>> +	.id = V4L2_CID_ASPEED_FORMAT,
>> +	.name = "Aspeed JPEG Format",
>> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
>> +	.min = false,
>> +	.max = true,
>> +	.step = 1,
>> +	.def = false,
>> +};
>> +
>> +#ifdef CONFIG_MACH_ASPEED_G4
>> +static const struct v4l2_ctrl_config aspeed_ctrl_compression_mode = {
>> +	.ops = &aspeed_video_ctrl_ops,
>> +	.id = V4L2_CID_ASPEED_COMPRESSION_MODE,
>> +	.name = "Aspeed Compression Mode",
>> +	.type = V4L2_CTRL_TYPE_INTEGER,
>> +	.min = 0,
>> +	.max = 2,
>> +	.step = 1,
>> +	.def = 0,
>> +};
>> +#endif
>> +
>> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_mode = {
>> +	.ops = &aspeed_video_ctrl_ops,
>> +	.id = V4L2_CID_ASPEED_HQ_MODE,
>> +	.name = "Aspeed HQ Mode",
>> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
>> +	.min = false,
>> +	.max = true,
>> +	.step = 1,
>> +	.def = false,
>> +};
>> +
>> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_jpeg_quality = {
>> +	.ops = &aspeed_video_ctrl_ops,
>> +	.id = V4L2_CID_ASPEED_HQ_JPEG_QUALITY,
>> +	.name = "Aspeed HQ Quality",
>> +	.type = V4L2_CTRL_TYPE_INTEGER,
>> +	.min = 0,
>> +	.max = ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1,
>> +	.step = 1,
>> +	.def = 0,
>> +};
>> +
>>   static void aspeed_video_resolution_work(struct work_struct *work)
>>   {
>>   	struct delayed_work *dwork = to_delayed_work(work);
>> @@ -1644,6 +1821,7 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>>   	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
>>   	struct vb2_queue *vbq = &video->queue;
>>   	struct video_device *vdev = &video->vdev;
>> +	struct v4l2_ctrl_handler *hdl = &video->ctrl_handler;
>>   	int rc;
>>   
>>   	video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
>> @@ -1658,22 +1836,28 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>>   		return rc;
>>   	}
>>   
>> -	v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
>> -	v4l2_ctrl_new_std(&video->ctrl_handler, &aspeed_video_ctrl_ops,
>> +	v4l2_ctrl_handler_init(hdl, 6);
>> +	v4l2_ctrl_new_std(hdl, &aspeed_video_ctrl_ops,
>>   			  V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
>>   			  ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1, 1, 0);
>> -	v4l2_ctrl_new_std_menu(&video->ctrl_handler, &aspeed_video_ctrl_ops,
>> +	v4l2_ctrl_new_std_menu(hdl, &aspeed_video_ctrl_ops,
>>   			       V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
>>   			       V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
>>   			       V4L2_JPEG_CHROMA_SUBSAMPLING_444);
>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_format, NULL);
>> +#ifdef CONFIG_MACH_ASPEED_G4
>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_compression_mode, NULL);
>> +#endif
>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_mode, NULL);
>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_jpeg_quality, NULL);
>>   
>> -	rc = video->ctrl_handler.error;
>> +	rc = hdl->error;
>>   	if (rc) {
>>   		dev_err(video->dev, "Failed to init controls: %d\n", rc);
>>   		goto err_ctrl_init;
>>   	}
>>   
>> -	v4l2_dev->ctrl_handler = &video->ctrl_handler;
>> +	v4l2_dev->ctrl_handler = hdl;
>>   
>>   	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>   	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
>>
-- 
Best Regards
Jammy

Re: [PATCH v3 5/7] media: aspeed: Support aspeed mode to reduce compressed data

[Hans Verkuil]

On 11/9/21 2:56 AM, Jammy Huang wrote:
> Dear Hans,
> 
> Thanks for your review.
> 
> On 2021/11/8 下午 05:09, Hans Verkuil wrote:
>> On 22/10/2021 08:55, Jammy Huang wrote:
>>> aspeed supports differential jpeg format which only compress the parts
>>> which are changed. In this way, it reduces both the amount of data to be
>>> transferred by network and those to be decoded on the client side.
>>>
>>> 4 new ctrls are added:
>>> * Aspeed JPEG Format: to control aspeed's partial jpeg on/off
>>>    0: standard jpeg, 1: aspeed jpeg
>> What exactly is 'aspeed jpeg'?
> 
> 'aspeed jpeg' is aspeed proprietary compression format. It will compare 
> the new frame
> with previous one to decide which macroblock's data is changed, and only 
> the changed
> macroblock will be compressed. In this way, the amount of compressed data is
> significantly reduced. This is similar to the concept of I/P-frames of 
> video compression.

Right, but that makes this a new vendor-specific pixelformat, not a control.

> 
> For example, the video is static in first 3 frames and then a object in 
> the video moved in
> 4th frame.
> 
>          Frame Content      |    Standard    |    Aspeed
> -----------------------------------------------------------------------
> 1th                                 |    Full             | Full
> 2th    identical with 1st   |    Full             |   none (only about 
> 12 Bytes for header data)
> 3th    identical with 1st   |    Full             |   none
> 4th    a object moved     |    Full             |   Only the Macroblocks 
> that have data changed are compressed
> 
> I have implemented a javascript aspeed decoder in novnc to support this 
> format, but
> the performance isn't good enough. I am working on a web-assembly to 
> improve it.

Is this format documented in a datasheet or something similar, ideally freely
available?

> 
>>> * Aspeed Compression Mode: to control aspeed's compression mode
>>>    0: DCT Only, 1: DCT VQ mix 2-color, 2: DCT VQ mix 4-color
>>>    This is AST2400 only. It will adapt JPEG or VQ encoding method according
>>>    to the context automatically.
>> What exactly does this do?
>>
>> Is this very aspeed-specific, or could this be a standard JPEG control?
> Yes, this is aspeed-specific. Its compression algorithm is a modified 
> JPEG algorithm.

Is this specific to the aspeed jpeg format, or also to the 'regular' jpeg format?

Regards,

	Hans

>>
>>> * Aspeed HQ Mode: to control aspeed's HQ mode on/off
>>>    0: disabled, 1: enabled
>>> * Aspeed HQ Quality: to control the quality of aspeed's HQ mode
>>>    only useful if Aspeed HQ mode is enabled
>> Can these two controls be replaced by the existing V4L2_CID_JPEG_COMPRESSION_QUALITY
>> control? I.e.: range 1..12 is non-HQ, 13-24 is HQ. Note that the spec recommends
>> that value 0 is not used in the V4L2_CID_JPEG_COMPRESSION_QUALITY range:
>>
>> https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/ext-ctrls-jpeg.html
> HQ mode is aspeed-specific and not related to stanard jpeg's compression 
> quality. These two
> quality attribute can be controlled independently. So, I think it is not 
> proper to integrate them
> together.
> 
> Noted, I will modified the min value of these 2 quality control.
> 
>>
>>> Aspeed JPEG Format requires an additional buffer, called bcd, to store
>>> the information about which macro block in the new frame is different
>>> from the previous one.
>>>
>>> To have bcd correctly working, we need to swap the buffers for src0/1 to
>>> make src1 refer to previous frame and src0 to the coming new frame.
>>>
>>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>>> ---
>>>   drivers/media/platform/aspeed-video.c | 222 +++++++++++++++++++++++---
>>>   1 file changed, 203 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>>> index cafbffe1ef69..94d17dee6f3d 100644
>>> --- a/drivers/media/platform/aspeed-video.c
>>> +++ b/drivers/media/platform/aspeed-video.c
>>> @@ -32,6 +32,12 @@
>>>   #include <media/videobuf2-dma-contig.h>
>>>   #include <linux/videodev2.h>
>>>   
>>> +#define ASPEED_CID_CUSTOM_BASE			(V4L2_CID_USER_BASE | 0xf000)
>> Driver-specific control ID ranges must be reserved in include/uapi/linux/v4l2-controls.h.
>> See e.g. V4L2_CID_USER_ALLEGRO_BASE.
> OKay, I will update in the next patch.
>>
>> Regards,
>>
>> 	Hans
>>
>>> +#define V4L2_CID_ASPEED_FORMAT			(ASPEED_CID_CUSTOM_BASE  + 1)
>>> +#define V4L2_CID_ASPEED_COMPRESSION_MODE	(ASPEED_CID_CUSTOM_BASE  + 2)
>>> +#define V4L2_CID_ASPEED_HQ_MODE			(ASPEED_CID_CUSTOM_BASE  + 3)
>>> +#define V4L2_CID_ASPEED_HQ_JPEG_QUALITY		(ASPEED_CID_CUSTOM_BASE  + 4)
>>> +
>>>   #define DEVICE_NAME			"aspeed-video"
>>>   
>>>   #define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
>>> @@ -55,6 +61,7 @@
>>>   
>>>   #define VE_MAX_SRC_BUFFER_SIZE		0x8ca000 /* 1920 * 1200, 32bpp */
>>>   #define VE_JPEG_HEADER_SIZE		0x006000 /* 512 * 12 * 4 */
>>> +#define VE_BCD_BUFF_SIZE		0x100000
>>>   
>>>   #define VE_PROTECTION_KEY		0x000
>>>   #define  VE_PROTECTION_KEY_UNLOCK	0x1a038aa8
>>> @@ -108,6 +115,13 @@
>>>   #define VE_SCALING_FILTER2		0x020
>>>   #define VE_SCALING_FILTER3		0x024
>>>   
>>> +#define VE_BCD_CTRL			0x02C
>>> +#define  VE_BCD_CTRL_EN_BCD		BIT(0)
>>> +#define  VE_BCD_CTRL_EN_ABCD		BIT(1)
>>> +#define  VE_BCD_CTRL_EN_CB		BIT(2)
>>> +#define  VE_BCD_CTRL_THR		GENMASK(23, 16)
>>> +#define  VE_BCD_CTRL_ABCD_THR		GENMASK(31, 24)
>>> +
>>>   #define VE_CAP_WINDOW			0x030
>>>   #define VE_COMP_WINDOW			0x034
>>>   #define VE_COMP_PROC_OFFSET		0x038
>>> @@ -116,6 +130,7 @@
>>>   #define VE_SRC0_ADDR			0x044
>>>   #define VE_SRC_SCANLINE_OFFSET		0x048
>>>   #define VE_SRC1_ADDR			0x04c
>>> +#define VE_BCD_ADDR			0x050
>>>   #define VE_COMP_ADDR			0x054
>>>   
>>>   #define VE_STREAM_BUF_SIZE		0x058
>>> @@ -136,6 +151,8 @@
>>>   #define  VE_COMP_CTRL_HQ_DCT_CHR	GENMASK(26, 22)
>>>   #define  VE_COMP_CTRL_HQ_DCT_LUM	GENMASK(31, 27)
>>>   
>>> +#define VE_CB_ADDR			0x06C
>>> +
>>>   #define VE_OFFSET_COMP_STREAM		0x078
>>>   
>>>   #define VE_JPEG_COMP_SIZE_READ_BACK	0x084
>>> @@ -243,10 +260,15 @@ struct aspeed_video {
>>>   	unsigned int max_compressed_size;
>>>   	struct aspeed_video_addr srcs[2];
>>>   	struct aspeed_video_addr jpeg;
>>> +	struct aspeed_video_addr bcd;
>>>   
>>>   	bool yuv420;
>>> +	bool partial_jpeg;
>>> +	bool hq_mode;
>>>   	unsigned int frame_rate;
>>>   	unsigned int jpeg_quality;
>>> +	unsigned int jpeg_hq_quality;
>>> +	unsigned int compression_mode;
>>>   
>>>   	unsigned int frame_bottom;
>>>   	unsigned int frame_left;
>>> @@ -258,6 +280,13 @@ struct aspeed_video {
>>>   
>>>   #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
>>>   
>>> +static bool aspeed_video_alloc_buf(struct aspeed_video *video,
>>> +				   struct aspeed_video_addr *addr,
>>> +				   unsigned int size);
>>> +
>>> +static void aspeed_video_free_buf(struct aspeed_video *video,
>>> +				  struct aspeed_video_addr *addr);
>>> +
>>>   static const u32 aspeed_video_jpeg_header[ASPEED_VIDEO_JPEG_HEADER_SIZE] = {
>>>   	0xe0ffd8ff, 0x464a1000, 0x01004649, 0x60000101, 0x00006000, 0x0f00feff,
>>>   	0x00002d05, 0x00000000, 0x00000000, 0x00dbff00
>>> @@ -492,6 +521,20 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>>>   		return -EBUSY;
>>>   	}
>>>   
>>> +	if (video->partial_jpeg && !video->bcd.size) {
>>> +		if (!aspeed_video_alloc_buf(video, &video->bcd,
>>> +					    VE_BCD_BUFF_SIZE)) {
>>> +			dev_err(video->dev, "Failed to allocate BCD buffer\n");
>>> +			dev_err(video->dev, "don't start frame\n");
>>> +			return -ENOMEM;
>>> +		}
>>> +		aspeed_video_write(video, VE_BCD_ADDR, video->bcd.dma);
>>> +		v4l2_dbg(1, debug, &video->v4l2_dev, "bcd addr(%#x) size(%d)\n",
>>> +			video->bcd.dma, video->bcd.size);
>>> +	} else if (!video->partial_jpeg && video->bcd.size) {
>>> +		aspeed_video_free_buf(video, &video->bcd);
>>> +	}
>>> +
>>>   	spin_lock_irqsave(&video->lock, flags);
>>>   	buf = list_first_entry_or_null(&video->buffers,
>>>   				       struct aspeed_video_buffer, link);
>>> @@ -635,6 +678,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>>   
>>>   	if (sts & VE_INTERRUPT_COMP_COMPLETE) {
>>>   		struct aspeed_video_buffer *buf;
>>> +		bool empty = true;
>>>   		u32 frame_size = aspeed_video_read(video,
>>>   						   VE_JPEG_COMP_SIZE_READ_BACK);
>>>   
>>> @@ -648,13 +692,23 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>>   		if (buf) {
>>>   			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size);
>>>   
>>> -			if (!list_is_last(&buf->link, &video->buffers)) {
>>> +			/*
>>> +			 * partial_jpeg requires continuous update.
>>> +			 * On the contrary, standard jpeg can keep last buffer
>>> +			 * to always have the latest result.
>>> +			 */
>>> +			if (!video->partial_jpeg &&
>>> +			    list_is_last(&buf->link, &video->buffers)) {
>>> +				empty = false;
>>> +				v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
>>> +			} else {
>>>   				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);
>>> +				empty = list_empty(&video->buffers);
>>>   			}
>>>   		}
>>>   		spin_unlock(&video->lock);
>>> @@ -668,7 +722,18 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>>   		aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>>   				   VE_INTERRUPT_COMP_COMPLETE);
>>>   		sts &= ~VE_INTERRUPT_COMP_COMPLETE;
>>> -		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
>>> +
>>> +		// swap src buffer if partial_jpeg
>>> +		if (video->partial_jpeg) {
>>> +			u32 src0, src1;
>>> +
>>> +			src0 = aspeed_video_read(video, VE_SRC0_ADDR);
>>> +			src1 = aspeed_video_read(video, VE_SRC1_ADDR);
>>> +			aspeed_video_write(video, VE_SRC0_ADDR, src1);
>>> +			aspeed_video_write(video, VE_SRC1_ADDR, src0);
>>> +		}
>>> +
>>> +		if (test_bit(VIDEO_STREAMING, &video->flags) && !empty)
>>>   			aspeed_video_start_frame(video);
>>>   	}
>>>   
>>> @@ -931,10 +996,14 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>>>   				   FIELD_PREP(VE_TGS_FIRST, video->frame_top) |
>>>   				   FIELD_PREP(VE_TGS_LAST,
>>>   					      video->frame_bottom + 1));
>>> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
>>> +		aspeed_video_update(video, VE_CTRL,
>>> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
>>> +				    VE_CTRL_INT_DE);
>>>   	} else {
>>>   		v4l2_dbg(1, debug, &video->v4l2_dev, "Capture: Direct Mode\n");
>>> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
>>> +		aspeed_video_update(video, VE_CTRL,
>>> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
>>> +				    VE_CTRL_DIRECT_FETCH);
>>>   	}
>>>   
>>>   	size *= 4;
>>> @@ -969,35 +1038,70 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>>>   
>>>   static void aspeed_video_update_regs(struct aspeed_video *video)
>>>   {
>>> -	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
>>> -		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>>> -		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
>>> +	static const char * const compress_mode_str[] = {"DCT Only",
>>> +		"DCT VQ mix 2-color", "DCT VQ mix 4-color"};
>>> +	u32 comp_ctrl =	FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>>> +		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10) |
>>> +		FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode) |
>>> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_LUM, video->jpeg_hq_quality) |
>>> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_CHR, video->jpeg_hq_quality |
>>> +			   0x10);
>>>   	u32 ctrl = 0;
>>> -	u32 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
>>> +	u32 seq_ctrl = 0;
>>>   
>>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n",
>>> -		 video->frame_rate);
>>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "subsample(%s)\n",
>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n", video->frame_rate);
>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "jpeg format(%s) subsample(%s)\n",
>>> +		 video->partial_jpeg ? "partial" : "standard",
>>>   		 video->yuv420 ? "420" : "444");
>>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d)\n",
>>> -		 video->jpeg_quality);
>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d) hq(%s) hq_quality(%d)\n",
>>> +		 video->jpeg_quality, video->hq_mode ? "on" : "off",
>>> +		 video->jpeg_hq_quality);
>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression mode(%s)\n",
>>> +		 compress_mode_str[video->compression_mode]);
>>> +
>>> +	if (video->partial_jpeg)
>>> +		aspeed_video_update(video, VE_BCD_CTRL, 0, VE_BCD_CTRL_EN_BCD);
>>> +	else
>>> +		aspeed_video_update(video, VE_BCD_CTRL, VE_BCD_CTRL_EN_BCD, 0);
>>>   
>>>   	if (video->frame_rate)
>>>   		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
>>>   
>>> +	if (!video->partial_jpeg) {
>>> +		comp_ctrl &= ~FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode);
>>> +		seq_ctrl |= VE_SEQ_CTRL_JPEG_MODE;
>>> +	}
>>> +
>>>   	if (video->yuv420)
>>>   		seq_ctrl |= VE_SEQ_CTRL_YUV420;
>>>   
>>>   	if (video->jpeg.virt)
>>>   		aspeed_video_update_jpeg_table(video->jpeg.virt, video->yuv420);
>>>   
>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>> +	switch (video->compression_mode) {
>>> +	case 0:	//DCT only
>>> +		comp_ctrl |= VE_COMP_CTRL_VQ_DCT_ONLY;
>>> +		break;
>>> +	case 1:	//DCT VQ mix 2-color
>>> +		comp_ctrl &= ~(VE_COMP_CTRL_VQ_4COLOR | VE_COMP_CTRL_VQ_DCT_ONLY);
>>> +		break;
>>> +	case 2:	//DCT VQ mix 4-color
>>> +		comp_ctrl |= VE_COMP_CTRL_VQ_4COLOR;
>>> +		break;
>>> +	}
>>> +#endif
>>> +
>>>   	/* Set control registers */
>>>   	aspeed_video_update(video, VE_SEQ_CTRL,
>>>   			    VE_SEQ_CTRL_JPEG_MODE | VE_SEQ_CTRL_YUV420,
>>>   			    seq_ctrl);
>>>   	aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, ctrl);
>>>   	aspeed_video_update(video, VE_COMP_CTRL,
>>> -			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
>>> +			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR |
>>> +			    VE_COMP_CTRL_EN_HQ | VE_COMP_CTRL_HQ_DCT_LUM |
>>> +			    VE_COMP_CTRL_HQ_DCT_CHR | VE_COMP_CTRL_VQ_4COLOR |
>>> +			    VE_COMP_CTRL_VQ_DCT_ONLY,
>>>   			    comp_ctrl);
>>>   }
>>>   
>>> @@ -1029,6 +1133,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>>>   
>>>   	/* Set mode detection defaults */
>>>   	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
>>> +
>>> +	aspeed_video_write(video, VE_BCD_CTRL, 0);
>>>   }
>>>   
>>>   static void aspeed_video_start(struct aspeed_video *video)
>>> @@ -1062,6 +1168,9 @@ static void aspeed_video_stop(struct aspeed_video *video)
>>>   	if (video->srcs[1].size)
>>>   		aspeed_video_free_buf(video, &video->srcs[1]);
>>>   
>>> +	if (video->bcd.size)
>>> +		aspeed_video_free_buf(video, &video->bcd);
>>> +
>>>   	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
>>>   	video->flags = 0;
>>>   }
>>> @@ -1364,6 +1473,28 @@ static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
>>>   		if (test_bit(VIDEO_STREAMING, &video->flags))
>>>   			aspeed_video_update_regs(video);
>>>   		break;
>>> +	case V4L2_CID_ASPEED_FORMAT:
>>> +		video->partial_jpeg = ctrl->val;
>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>> +			aspeed_video_update_regs(video);
>>> +		break;
>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>> +	case V4L2_CID_ASPEED_COMPRESSION_MODE:
>>> +		video->compression_mode = ctrl->val;
>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>> +			aspeed_video_update_regs(video);
>>> +		break;
>>> +#endif
>>> +	case V4L2_CID_ASPEED_HQ_MODE:
>>> +		video->hq_mode = ctrl->val;
>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>> +			aspeed_video_update_regs(video);
>>> +		break;
>>> +	case V4L2_CID_ASPEED_HQ_JPEG_QUALITY:
>>> +		video->jpeg_hq_quality = ctrl->val;
>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>> +			aspeed_video_update_regs(video);
>>> +		break;
>>>   	default:
>>>   		return -EINVAL;
>>>   	}
>>> @@ -1375,6 +1506,52 @@ static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
>>>   	.s_ctrl = aspeed_video_set_ctrl,
>>>   };
>>>   
>>> +static const struct v4l2_ctrl_config aspeed_ctrl_format = {
>>> +	.ops = &aspeed_video_ctrl_ops,
>>> +	.id = V4L2_CID_ASPEED_FORMAT,
>>> +	.name = "Aspeed JPEG Format",
>>> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
>>> +	.min = false,
>>> +	.max = true,
>>> +	.step = 1,
>>> +	.def = false,
>>> +};
>>> +
>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>> +static const struct v4l2_ctrl_config aspeed_ctrl_compression_mode = {
>>> +	.ops = &aspeed_video_ctrl_ops,
>>> +	.id = V4L2_CID_ASPEED_COMPRESSION_MODE,
>>> +	.name = "Aspeed Compression Mode",
>>> +	.type = V4L2_CTRL_TYPE_INTEGER,
>>> +	.min = 0,
>>> +	.max = 2,
>>> +	.step = 1,
>>> +	.def = 0,
>>> +};
>>> +#endif
>>> +
>>> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_mode = {
>>> +	.ops = &aspeed_video_ctrl_ops,
>>> +	.id = V4L2_CID_ASPEED_HQ_MODE,
>>> +	.name = "Aspeed HQ Mode",
>>> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
>>> +	.min = false,
>>> +	.max = true,
>>> +	.step = 1,
>>> +	.def = false,
>>> +};
>>> +
>>> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_jpeg_quality = {
>>> +	.ops = &aspeed_video_ctrl_ops,
>>> +	.id = V4L2_CID_ASPEED_HQ_JPEG_QUALITY,
>>> +	.name = "Aspeed HQ Quality",
>>> +	.type = V4L2_CTRL_TYPE_INTEGER,
>>> +	.min = 0,
>>> +	.max = ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1,
>>> +	.step = 1,
>>> +	.def = 0,
>>> +};
>>> +
>>>   static void aspeed_video_resolution_work(struct work_struct *work)
>>>   {
>>>   	struct delayed_work *dwork = to_delayed_work(work);
>>> @@ -1644,6 +1821,7 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>>>   	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
>>>   	struct vb2_queue *vbq = &video->queue;
>>>   	struct video_device *vdev = &video->vdev;
>>> +	struct v4l2_ctrl_handler *hdl = &video->ctrl_handler;
>>>   	int rc;
>>>   
>>>   	video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
>>> @@ -1658,22 +1836,28 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>>>   		return rc;
>>>   	}
>>>   
>>> -	v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
>>> -	v4l2_ctrl_new_std(&video->ctrl_handler, &aspeed_video_ctrl_ops,
>>> +	v4l2_ctrl_handler_init(hdl, 6);
>>> +	v4l2_ctrl_new_std(hdl, &aspeed_video_ctrl_ops,
>>>   			  V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
>>>   			  ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1, 1, 0);
>>> -	v4l2_ctrl_new_std_menu(&video->ctrl_handler, &aspeed_video_ctrl_ops,
>>> +	v4l2_ctrl_new_std_menu(hdl, &aspeed_video_ctrl_ops,
>>>   			       V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
>>>   			       V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
>>>   			       V4L2_JPEG_CHROMA_SUBSAMPLING_444);
>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_format, NULL);
>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_compression_mode, NULL);
>>> +#endif
>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_mode, NULL);
>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_jpeg_quality, NULL);
>>>   
>>> -	rc = video->ctrl_handler.error;
>>> +	rc = hdl->error;
>>>   	if (rc) {
>>>   		dev_err(video->dev, "Failed to init controls: %d\n", rc);
>>>   		goto err_ctrl_init;
>>>   	}
>>>   
>>> -	v4l2_dev->ctrl_handler = &video->ctrl_handler;
>>> +	v4l2_dev->ctrl_handler = hdl;
>>>   
>>>   	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>>   	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
>>>

Re: [PATCH v3 5/7] media: aspeed: Support aspeed mode to reduce compressed data

[Jammy Huang]

Dear Hans,

On 2021/11/9 下午 06:17, Hans Verkuil wrote:
> On 11/9/21 2:56 AM, Jammy Huang wrote:
>> Dear Hans,
>>
>> Thanks for your review.
>>
>> On 2021/11/8 下午 05:09, Hans Verkuil wrote:
>>> On 22/10/2021 08:55, Jammy Huang wrote:
>>>> aspeed supports differential jpeg format which only compress the parts
>>>> which are changed. In this way, it reduces both the amount of data to be
>>>> transferred by network and those to be decoded on the client side.
>>>>
>>>> 4 new ctrls are added:
>>>> * Aspeed JPEG Format: to control aspeed's partial jpeg on/off
>>>>     0: standard jpeg, 1: aspeed jpeg
>>> What exactly is 'aspeed jpeg'?
>> 'aspeed jpeg' is aspeed proprietary compression format. It will compare
>> the new frame
>> with previous one to decide which macroblock's data is changed, and only
>> the changed
>> macroblock will be compressed. In this way, the amount of compressed data is
>> significantly reduced. This is similar to the concept of I/P-frames of
>> video compression.
> Right, but that makes this a new vendor-specific pixelformat, not a control.
OK, I will add a new vendor-specific pixelformat for this.
>
>> For example, the video is static in first 3 frames and then a object in
>> the video moved in
>> 4th frame.
>>
>>           Frame Content      |    Standard    |    Aspeed
>> -----------------------------------------------------------------------
>> 1th                                 |    Full             | Full
>> 2th    identical with 1st   |    Full             |   none (only about
>> 12 Bytes for header data)
>> 3th    identical with 1st   |    Full             |   none
>> 4th    a object moved     |    Full             |   Only the Macroblocks
>> that have data changed are compressed
>>
>> I have implemented a javascript aspeed decoder in novnc to support this
>> format, but
>> the performance isn't good enough. I am working on a web-assembly to
>> improve it.
> Is this format documented in a datasheet or something similar, ideally freely
> available?
This format is documented in ast2400/2500/2600's datasheet, but it's not 
freely available.
>
>>>> * Aspeed Compression Mode: to control aspeed's compression mode
>>>>     0: DCT Only, 1: DCT VQ mix 2-color, 2: DCT VQ mix 4-color
>>>>     This is AST2400 only. It will adapt JPEG or VQ encoding method according
>>>>     to the context automatically.
>>> What exactly does this do?
>>>
>>> Is this very aspeed-specific, or could this be a standard JPEG control?
>> Yes, this is aspeed-specific. Its compression algorithm is a modified
>> JPEG algorithm.
> Is this specific to the aspeed jpeg format, or also to the 'regular' jpeg format?

It's specific to the aspeed jpeg format. Regular jpeg is based on DCT 
and doesn't support VQ
compression

>
> Regards,
>
> 	Hans
>
>>>> * Aspeed HQ Mode: to control aspeed's HQ mode on/off
>>>>     0: disabled, 1: enabled
>>>> * Aspeed HQ Quality: to control the quality of aspeed's HQ mode
>>>>     only useful if Aspeed HQ mode is enabled
>>> Can these two controls be replaced by the existing V4L2_CID_JPEG_COMPRESSION_QUALITY
>>> control? I.e.: range 1..12 is non-HQ, 13-24 is HQ. Note that the spec recommends
>>> that value 0 is not used in the V4L2_CID_JPEG_COMPRESSION_QUALITY range:
>>>
>>> https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/ext-ctrls-jpeg.html
>> HQ mode is aspeed-specific and not related to stanard jpeg's compression
>> quality. These two
>> quality attribute can be controlled independently. So, I think it is not
>> proper to integrate them
>> together.
>>
>> Noted, I will modified the min value of these 2 quality control.
>>
>>>> Aspeed JPEG Format requires an additional buffer, called bcd, to store
>>>> the information about which macro block in the new frame is different
>>>> from the previous one.
>>>>
>>>> To have bcd correctly working, we need to swap the buffers for src0/1 to
>>>> make src1 refer to previous frame and src0 to the coming new frame.
>>>>
>>>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>>>> ---
>>>>    drivers/media/platform/aspeed-video.c | 222 +++++++++++++++++++++++---
>>>>    1 file changed, 203 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>>>> index cafbffe1ef69..94d17dee6f3d 100644
>>>> --- a/drivers/media/platform/aspeed-video.c
>>>> +++ b/drivers/media/platform/aspeed-video.c
>>>> @@ -32,6 +32,12 @@
>>>>    #include <media/videobuf2-dma-contig.h>
>>>>    #include <linux/videodev2.h>
>>>>    
>>>> +#define ASPEED_CID_CUSTOM_BASE			(V4L2_CID_USER_BASE | 0xf000)
>>> Driver-specific control ID ranges must be reserved in include/uapi/linux/v4l2-controls.h.
>>> See e.g. V4L2_CID_USER_ALLEGRO_BASE.
>> OKay, I will update in the next patch.
>>> Regards,
>>>
>>> 	Hans
>>>
>>>> +#define V4L2_CID_ASPEED_FORMAT			(ASPEED_CID_CUSTOM_BASE  + 1)
>>>> +#define V4L2_CID_ASPEED_COMPRESSION_MODE	(ASPEED_CID_CUSTOM_BASE  + 2)
>>>> +#define V4L2_CID_ASPEED_HQ_MODE			(ASPEED_CID_CUSTOM_BASE  + 3)
>>>> +#define V4L2_CID_ASPEED_HQ_JPEG_QUALITY		(ASPEED_CID_CUSTOM_BASE  + 4)
>>>> +
>>>>    #define DEVICE_NAME			"aspeed-video"
>>>>    
>>>>    #define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
>>>> @@ -55,6 +61,7 @@
>>>>    
>>>>    #define VE_MAX_SRC_BUFFER_SIZE		0x8ca000 /* 1920 * 1200, 32bpp */
>>>>    #define VE_JPEG_HEADER_SIZE		0x006000 /* 512 * 12 * 4 */
>>>> +#define VE_BCD_BUFF_SIZE		0x100000
>>>>    
>>>>    #define VE_PROTECTION_KEY		0x000
>>>>    #define  VE_PROTECTION_KEY_UNLOCK	0x1a038aa8
>>>> @@ -108,6 +115,13 @@
>>>>    #define VE_SCALING_FILTER2		0x020
>>>>    #define VE_SCALING_FILTER3		0x024
>>>>    
>>>> +#define VE_BCD_CTRL			0x02C
>>>> +#define  VE_BCD_CTRL_EN_BCD		BIT(0)
>>>> +#define  VE_BCD_CTRL_EN_ABCD		BIT(1)
>>>> +#define  VE_BCD_CTRL_EN_CB		BIT(2)
>>>> +#define  VE_BCD_CTRL_THR		GENMASK(23, 16)
>>>> +#define  VE_BCD_CTRL_ABCD_THR		GENMASK(31, 24)
>>>> +
>>>>    #define VE_CAP_WINDOW			0x030
>>>>    #define VE_COMP_WINDOW			0x034
>>>>    #define VE_COMP_PROC_OFFSET		0x038
>>>> @@ -116,6 +130,7 @@
>>>>    #define VE_SRC0_ADDR			0x044
>>>>    #define VE_SRC_SCANLINE_OFFSET		0x048
>>>>    #define VE_SRC1_ADDR			0x04c
>>>> +#define VE_BCD_ADDR			0x050
>>>>    #define VE_COMP_ADDR			0x054
>>>>    
>>>>    #define VE_STREAM_BUF_SIZE		0x058
>>>> @@ -136,6 +151,8 @@
>>>>    #define  VE_COMP_CTRL_HQ_DCT_CHR	GENMASK(26, 22)
>>>>    #define  VE_COMP_CTRL_HQ_DCT_LUM	GENMASK(31, 27)
>>>>    
>>>> +#define VE_CB_ADDR			0x06C
>>>> +
>>>>    #define VE_OFFSET_COMP_STREAM		0x078
>>>>    
>>>>    #define VE_JPEG_COMP_SIZE_READ_BACK	0x084
>>>> @@ -243,10 +260,15 @@ struct aspeed_video {
>>>>    	unsigned int max_compressed_size;
>>>>    	struct aspeed_video_addr srcs[2];
>>>>    	struct aspeed_video_addr jpeg;
>>>> +	struct aspeed_video_addr bcd;
>>>>    
>>>>    	bool yuv420;
>>>> +	bool partial_jpeg;
>>>> +	bool hq_mode;
>>>>    	unsigned int frame_rate;
>>>>    	unsigned int jpeg_quality;
>>>> +	unsigned int jpeg_hq_quality;
>>>> +	unsigned int compression_mode;
>>>>    
>>>>    	unsigned int frame_bottom;
>>>>    	unsigned int frame_left;
>>>> @@ -258,6 +280,13 @@ struct aspeed_video {
>>>>    
>>>>    #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
>>>>    
>>>> +static bool aspeed_video_alloc_buf(struct aspeed_video *video,
>>>> +				   struct aspeed_video_addr *addr,
>>>> +				   unsigned int size);
>>>> +
>>>> +static void aspeed_video_free_buf(struct aspeed_video *video,
>>>> +				  struct aspeed_video_addr *addr);
>>>> +
>>>>    static const u32 aspeed_video_jpeg_header[ASPEED_VIDEO_JPEG_HEADER_SIZE] = {
>>>>    	0xe0ffd8ff, 0x464a1000, 0x01004649, 0x60000101, 0x00006000, 0x0f00feff,
>>>>    	0x00002d05, 0x00000000, 0x00000000, 0x00dbff00
>>>> @@ -492,6 +521,20 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>>>>    		return -EBUSY;
>>>>    	}
>>>>    
>>>> +	if (video->partial_jpeg && !video->bcd.size) {
>>>> +		if (!aspeed_video_alloc_buf(video, &video->bcd,
>>>> +					    VE_BCD_BUFF_SIZE)) {
>>>> +			dev_err(video->dev, "Failed to allocate BCD buffer\n");
>>>> +			dev_err(video->dev, "don't start frame\n");
>>>> +			return -ENOMEM;
>>>> +		}
>>>> +		aspeed_video_write(video, VE_BCD_ADDR, video->bcd.dma);
>>>> +		v4l2_dbg(1, debug, &video->v4l2_dev, "bcd addr(%#x) size(%d)\n",
>>>> +			video->bcd.dma, video->bcd.size);
>>>> +	} else if (!video->partial_jpeg && video->bcd.size) {
>>>> +		aspeed_video_free_buf(video, &video->bcd);
>>>> +	}
>>>> +
>>>>    	spin_lock_irqsave(&video->lock, flags);
>>>>    	buf = list_first_entry_or_null(&video->buffers,
>>>>    				       struct aspeed_video_buffer, link);
>>>> @@ -635,6 +678,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>>>    
>>>>    	if (sts & VE_INTERRUPT_COMP_COMPLETE) {
>>>>    		struct aspeed_video_buffer *buf;
>>>> +		bool empty = true;
>>>>    		u32 frame_size = aspeed_video_read(video,
>>>>    						   VE_JPEG_COMP_SIZE_READ_BACK);
>>>>    
>>>> @@ -648,13 +692,23 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>>>    		if (buf) {
>>>>    			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size);
>>>>    
>>>> -			if (!list_is_last(&buf->link, &video->buffers)) {
>>>> +			/*
>>>> +			 * partial_jpeg requires continuous update.
>>>> +			 * On the contrary, standard jpeg can keep last buffer
>>>> +			 * to always have the latest result.
>>>> +			 */
>>>> +			if (!video->partial_jpeg &&
>>>> +			    list_is_last(&buf->link, &video->buffers)) {
>>>> +				empty = false;
>>>> +				v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
>>>> +			} else {
>>>>    				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);
>>>> +				empty = list_empty(&video->buffers);
>>>>    			}
>>>>    		}
>>>>    		spin_unlock(&video->lock);
>>>> @@ -668,7 +722,18 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>>>    		aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>>>    				   VE_INTERRUPT_COMP_COMPLETE);
>>>>    		sts &= ~VE_INTERRUPT_COMP_COMPLETE;
>>>> -		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
>>>> +
>>>> +		// swap src buffer if partial_jpeg
>>>> +		if (video->partial_jpeg) {
>>>> +			u32 src0, src1;
>>>> +
>>>> +			src0 = aspeed_video_read(video, VE_SRC0_ADDR);
>>>> +			src1 = aspeed_video_read(video, VE_SRC1_ADDR);
>>>> +			aspeed_video_write(video, VE_SRC0_ADDR, src1);
>>>> +			aspeed_video_write(video, VE_SRC1_ADDR, src0);
>>>> +		}
>>>> +
>>>> +		if (test_bit(VIDEO_STREAMING, &video->flags) && !empty)
>>>>    			aspeed_video_start_frame(video);
>>>>    	}
>>>>    
>>>> @@ -931,10 +996,14 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>>>>    				   FIELD_PREP(VE_TGS_FIRST, video->frame_top) |
>>>>    				   FIELD_PREP(VE_TGS_LAST,
>>>>    					      video->frame_bottom + 1));
>>>> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
>>>> +		aspeed_video_update(video, VE_CTRL,
>>>> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
>>>> +				    VE_CTRL_INT_DE);
>>>>    	} else {
>>>>    		v4l2_dbg(1, debug, &video->v4l2_dev, "Capture: Direct Mode\n");
>>>> -		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
>>>> +		aspeed_video_update(video, VE_CTRL,
>>>> +				    VE_CTRL_INT_DE | VE_CTRL_DIRECT_FETCH,
>>>> +				    VE_CTRL_DIRECT_FETCH);
>>>>    	}
>>>>    
>>>>    	size *= 4;
>>>> @@ -969,35 +1038,70 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>>>>    
>>>>    static void aspeed_video_update_regs(struct aspeed_video *video)
>>>>    {
>>>> -	u32 comp_ctrl = VE_COMP_CTRL_RSVD |
>>>> -		FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>>>> -		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
>>>> +	static const char * const compress_mode_str[] = {"DCT Only",
>>>> +		"DCT VQ mix 2-color", "DCT VQ mix 4-color"};
>>>> +	u32 comp_ctrl =	FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
>>>> +		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10) |
>>>> +		FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode) |
>>>> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_LUM, video->jpeg_hq_quality) |
>>>> +		FIELD_PREP(VE_COMP_CTRL_HQ_DCT_CHR, video->jpeg_hq_quality |
>>>> +			   0x10);
>>>>    	u32 ctrl = 0;
>>>> -	u32 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
>>>> +	u32 seq_ctrl = 0;
>>>>    
>>>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n",
>>>> -		 video->frame_rate);
>>>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "subsample(%s)\n",
>>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "framerate(%d)\n", video->frame_rate);
>>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "jpeg format(%s) subsample(%s)\n",
>>>> +		 video->partial_jpeg ? "partial" : "standard",
>>>>    		 video->yuv420 ? "420" : "444");
>>>> -	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d)\n",
>>>> -		 video->jpeg_quality);
>>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression quality(%d) hq(%s) hq_quality(%d)\n",
>>>> +		 video->jpeg_quality, video->hq_mode ? "on" : "off",
>>>> +		 video->jpeg_hq_quality);
>>>> +	v4l2_dbg(1, debug, &video->v4l2_dev, "compression mode(%s)\n",
>>>> +		 compress_mode_str[video->compression_mode]);
>>>> +
>>>> +	if (video->partial_jpeg)
>>>> +		aspeed_video_update(video, VE_BCD_CTRL, 0, VE_BCD_CTRL_EN_BCD);
>>>> +	else
>>>> +		aspeed_video_update(video, VE_BCD_CTRL, VE_BCD_CTRL_EN_BCD, 0);
>>>>    
>>>>    	if (video->frame_rate)
>>>>    		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
>>>>    
>>>> +	if (!video->partial_jpeg) {
>>>> +		comp_ctrl &= ~FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode);
>>>> +		seq_ctrl |= VE_SEQ_CTRL_JPEG_MODE;
>>>> +	}
>>>> +
>>>>    	if (video->yuv420)
>>>>    		seq_ctrl |= VE_SEQ_CTRL_YUV420;
>>>>    
>>>>    	if (video->jpeg.virt)
>>>>    		aspeed_video_update_jpeg_table(video->jpeg.virt, video->yuv420);
>>>>    
>>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>>> +	switch (video->compression_mode) {
>>>> +	case 0:	//DCT only
>>>> +		comp_ctrl |= VE_COMP_CTRL_VQ_DCT_ONLY;
>>>> +		break;
>>>> +	case 1:	//DCT VQ mix 2-color
>>>> +		comp_ctrl &= ~(VE_COMP_CTRL_VQ_4COLOR | VE_COMP_CTRL_VQ_DCT_ONLY);
>>>> +		break;
>>>> +	case 2:	//DCT VQ mix 4-color
>>>> +		comp_ctrl |= VE_COMP_CTRL_VQ_4COLOR;
>>>> +		break;
>>>> +	}
>>>> +#endif
>>>> +
>>>>    	/* Set control registers */
>>>>    	aspeed_video_update(video, VE_SEQ_CTRL,
>>>>    			    VE_SEQ_CTRL_JPEG_MODE | VE_SEQ_CTRL_YUV420,
>>>>    			    seq_ctrl);
>>>>    	aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, ctrl);
>>>>    	aspeed_video_update(video, VE_COMP_CTRL,
>>>> -			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
>>>> +			    VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR |
>>>> +			    VE_COMP_CTRL_EN_HQ | VE_COMP_CTRL_HQ_DCT_LUM |
>>>> +			    VE_COMP_CTRL_HQ_DCT_CHR | VE_COMP_CTRL_VQ_4COLOR |
>>>> +			    VE_COMP_CTRL_VQ_DCT_ONLY,
>>>>    			    comp_ctrl);
>>>>    }
>>>>    
>>>> @@ -1029,6 +1133,8 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
>>>>    
>>>>    	/* Set mode detection defaults */
>>>>    	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
>>>> +
>>>> +	aspeed_video_write(video, VE_BCD_CTRL, 0);
>>>>    }
>>>>    
>>>>    static void aspeed_video_start(struct aspeed_video *video)
>>>> @@ -1062,6 +1168,9 @@ static void aspeed_video_stop(struct aspeed_video *video)
>>>>    	if (video->srcs[1].size)
>>>>    		aspeed_video_free_buf(video, &video->srcs[1]);
>>>>    
>>>> +	if (video->bcd.size)
>>>> +		aspeed_video_free_buf(video, &video->bcd);
>>>> +
>>>>    	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
>>>>    	video->flags = 0;
>>>>    }
>>>> @@ -1364,6 +1473,28 @@ static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
>>>>    		if (test_bit(VIDEO_STREAMING, &video->flags))
>>>>    			aspeed_video_update_regs(video);
>>>>    		break;
>>>> +	case V4L2_CID_ASPEED_FORMAT:
>>>> +		video->partial_jpeg = ctrl->val;
>>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>>> +			aspeed_video_update_regs(video);
>>>> +		break;
>>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>>> +	case V4L2_CID_ASPEED_COMPRESSION_MODE:
>>>> +		video->compression_mode = ctrl->val;
>>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>>> +			aspeed_video_update_regs(video);
>>>> +		break;
>>>> +#endif
>>>> +	case V4L2_CID_ASPEED_HQ_MODE:
>>>> +		video->hq_mode = ctrl->val;
>>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>>> +			aspeed_video_update_regs(video);
>>>> +		break;
>>>> +	case V4L2_CID_ASPEED_HQ_JPEG_QUALITY:
>>>> +		video->jpeg_hq_quality = ctrl->val;
>>>> +		if (test_bit(VIDEO_STREAMING, &video->flags))
>>>> +			aspeed_video_update_regs(video);
>>>> +		break;
>>>>    	default:
>>>>    		return -EINVAL;
>>>>    	}
>>>> @@ -1375,6 +1506,52 @@ static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
>>>>    	.s_ctrl = aspeed_video_set_ctrl,
>>>>    };
>>>>    
>>>> +static const struct v4l2_ctrl_config aspeed_ctrl_format = {
>>>> +	.ops = &aspeed_video_ctrl_ops,
>>>> +	.id = V4L2_CID_ASPEED_FORMAT,
>>>> +	.name = "Aspeed JPEG Format",
>>>> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
>>>> +	.min = false,
>>>> +	.max = true,
>>>> +	.step = 1,
>>>> +	.def = false,
>>>> +};
>>>> +
>>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>>> +static const struct v4l2_ctrl_config aspeed_ctrl_compression_mode = {
>>>> +	.ops = &aspeed_video_ctrl_ops,
>>>> +	.id = V4L2_CID_ASPEED_COMPRESSION_MODE,
>>>> +	.name = "Aspeed Compression Mode",
>>>> +	.type = V4L2_CTRL_TYPE_INTEGER,
>>>> +	.min = 0,
>>>> +	.max = 2,
>>>> +	.step = 1,
>>>> +	.def = 0,
>>>> +};
>>>> +#endif
>>>> +
>>>> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_mode = {
>>>> +	.ops = &aspeed_video_ctrl_ops,
>>>> +	.id = V4L2_CID_ASPEED_HQ_MODE,
>>>> +	.name = "Aspeed HQ Mode",
>>>> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
>>>> +	.min = false,
>>>> +	.max = true,
>>>> +	.step = 1,
>>>> +	.def = false,
>>>> +};
>>>> +
>>>> +static const struct v4l2_ctrl_config aspeed_ctrl_HQ_jpeg_quality = {
>>>> +	.ops = &aspeed_video_ctrl_ops,
>>>> +	.id = V4L2_CID_ASPEED_HQ_JPEG_QUALITY,
>>>> +	.name = "Aspeed HQ Quality",
>>>> +	.type = V4L2_CTRL_TYPE_INTEGER,
>>>> +	.min = 0,
>>>> +	.max = ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1,
>>>> +	.step = 1,
>>>> +	.def = 0,
>>>> +};
>>>> +
>>>>    static void aspeed_video_resolution_work(struct work_struct *work)
>>>>    {
>>>>    	struct delayed_work *dwork = to_delayed_work(work);
>>>> @@ -1644,6 +1821,7 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>>>>    	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
>>>>    	struct vb2_queue *vbq = &video->queue;
>>>>    	struct video_device *vdev = &video->vdev;
>>>> +	struct v4l2_ctrl_handler *hdl = &video->ctrl_handler;
>>>>    	int rc;
>>>>    
>>>>    	video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
>>>> @@ -1658,22 +1836,28 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
>>>>    		return rc;
>>>>    	}
>>>>    
>>>> -	v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
>>>> -	v4l2_ctrl_new_std(&video->ctrl_handler, &aspeed_video_ctrl_ops,
>>>> +	v4l2_ctrl_handler_init(hdl, 6);
>>>> +	v4l2_ctrl_new_std(hdl, &aspeed_video_ctrl_ops,
>>>>    			  V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
>>>>    			  ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1, 1, 0);
>>>> -	v4l2_ctrl_new_std_menu(&video->ctrl_handler, &aspeed_video_ctrl_ops,
>>>> +	v4l2_ctrl_new_std_menu(hdl, &aspeed_video_ctrl_ops,
>>>>    			       V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
>>>>    			       V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
>>>>    			       V4L2_JPEG_CHROMA_SUBSAMPLING_444);
>>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_format, NULL);
>>>> +#ifdef CONFIG_MACH_ASPEED_G4
>>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_compression_mode, NULL);
>>>> +#endif
>>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_mode, NULL);
>>>> +	v4l2_ctrl_new_custom(hdl, &aspeed_ctrl_HQ_jpeg_quality, NULL);
>>>>    
>>>> -	rc = video->ctrl_handler.error;
>>>> +	rc = hdl->error;
>>>>    	if (rc) {
>>>>    		dev_err(video->dev, "Failed to init controls: %d\n", rc);
>>>>    		goto err_ctrl_init;
>>>>    	}
>>>>    
>>>> -	v4l2_dev->ctrl_handler = &video->ctrl_handler;
>>>> +	v4l2_dev->ctrl_handler = hdl;
>>>>    
>>>>    	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>>>    	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
>>>>
-- 
Best Regards
Jammy

Re: [PATCH v3 5/7] media: aspeed: Support aspeed mode to reduce compressed data

[Hans Verkuil]

On 10/11/2021 03:40, Jammy Huang wrote:
> Dear Hans,
> 
> On 2021/11/9 下午 06:17, Hans Verkuil wrote:
>> On 11/9/21 2:56 AM, Jammy Huang wrote:
>>> Dear Hans,
>>>
>>> Thanks for your review.
>>>
>>> On 2021/11/8 下午 05:09, Hans Verkuil wrote:
>>>> On 22/10/2021 08:55, Jammy Huang wrote:
>>>>> aspeed supports differential jpeg format which only compress the parts
>>>>> which are changed. In this way, it reduces both the amount of data to be
>>>>> transferred by network and those to be decoded on the client side.
>>>>>
>>>>> 4 new ctrls are added:
>>>>> * Aspeed JPEG Format: to control aspeed's partial jpeg on/off
>>>>>     0: standard jpeg, 1: aspeed jpeg
>>>> What exactly is 'aspeed jpeg'?
>>> 'aspeed jpeg' is aspeed proprietary compression format. It will compare
>>> the new frame
>>> with previous one to decide which macroblock's data is changed, and only
>>> the changed
>>> macroblock will be compressed. In this way, the amount of compressed data is
>>> significantly reduced. This is similar to the concept of I/P-frames of
>>> video compression.
>> Right, but that makes this a new vendor-specific pixelformat, not a control.
> OK, I will add a new vendor-specific pixelformat for this.
>>
>>> For example, the video is static in first 3 frames and then a object in
>>> the video moved in
>>> 4th frame.
>>>
>>>           Frame Content      |    Standard    |    Aspeed
>>> -----------------------------------------------------------------------
>>> 1th                                 |    Full             | Full
>>> 2th    identical with 1st   |    Full             |   none (only about
>>> 12 Bytes for header data)
>>> 3th    identical with 1st   |    Full             |   none
>>> 4th    a object moved     |    Full             |   Only the Macroblocks
>>> that have data changed are compressed
>>>
>>> I have implemented a javascript aspeed decoder in novnc to support this
>>> format, but
>>> the performance isn't good enough. I am working on a web-assembly to
>>> improve it.
>> Is this format documented in a datasheet or something similar, ideally freely
>> available?
> This format is documented in ast2400/2500/2600's datasheet, but it's not 
> freely available.

OK. Make sure to document the new pixelformat in
Documentation/userspace-api/media/v4l/pixfmt-reserved.rst with a reference to where it
is defined in the datasheet.

If there is publicly available source code that converts it to 'normal' JPEG, then
a reference to that would be very nice as well.

>>
>>>>> * Aspeed Compression Mode: to control aspeed's compression mode
>>>>>     0: DCT Only, 1: DCT VQ mix 2-color, 2: DCT VQ mix 4-color
>>>>>     This is AST2400 only. It will adapt JPEG or VQ encoding method according
>>>>>     to the context automatically.
>>>> What exactly does this do?
>>>>
>>>> Is this very aspeed-specific, or could this be a standard JPEG control?
>>> Yes, this is aspeed-specific. Its compression algorithm is a modified
>>> JPEG algorithm.
>> Is this specific to the aspeed jpeg format, or also to the 'regular' jpeg format?
> 
> It's specific to the aspeed jpeg format. Regular jpeg is based on DCT 
> and doesn't support VQ
> compression

OK, that makes it an aspeed-specific control. Good to know.

Regards,

	Hans

[PATCH v3 6/7] media: aspeed: add comments and macro

[Jammy Huang]

Add comments to describe video-stat and 'struct aspeed_video'.
Add macro, ASPEED_VIDEO_V4L2_MIN_BUF_REQ, to describe the buffers
needed.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 39 ++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 94d17dee6f3d..8e0c29551ffc 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -38,6 +38,8 @@
 #define V4L2_CID_ASPEED_HQ_MODE			(ASPEED_CID_CUSTOM_BASE  + 3)
 #define V4L2_CID_ASPEED_HQ_JPEG_QUALITY		(ASPEED_CID_CUSTOM_BASE  + 4)
 
+#define ASPEED_VIDEO_V4L2_MIN_BUF_REQ 3
+
 #define DEVICE_NAME			"aspeed-video"
 
 #define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
@@ -202,6 +204,15 @@
 #define VE_MEM_RESTRICT_START		0x310
 #define VE_MEM_RESTRICT_END		0x314
 
+/*
+ * @VIDEO_MODE_DETECT_DONE:	a flag raised if signal lock
+ * @VIDEO_RES_CHANGE:		a flag raised if res_change work on-going
+ * @VIDEO_RES_DETECT:		a flag raised if res. detection on-going
+ * @VIDEO_STREAMING:		a flag raised if user requires stream-on
+ * @VIDEO_FRAME_INPRG:		a flag raised if hw working on a frame
+ * @VIDEO_STOPPED:		a flag raised if device release
+ * @VIDEO_CLOCKS_ON:		a flag raised if clk is on
+ */
 enum {
 	VIDEO_MODE_DETECT_DONE,
 	VIDEO_RES_CHANGE,
@@ -234,6 +245,28 @@ struct aspeed_video_perf {
 #define to_aspeed_video_buffer(x) \
 	container_of((x), struct aspeed_video_buffer, vb)
 
+/**
+ * struct aspeed_video - driver data
+ *
+ * @flags:		holds the state of video
+ * @sequence:		holds the last number of frame completed
+ * @max_compressed_size:holds max compressed stream's size
+ * @srcs:		holds the buffer information for srcs
+ * @jpeg:		holds the buffer information for jpeg header
+ * @bcd:		holds the buffer information for bcd work
+ * @yuv420:		a flag raised if JPEG subsampling is 420
+ * @partial_jpeg:	a flag raised if JPEG supports partial capture
+ * @hq_mode:		a flag raised if HQ is enabled. Only for partial_jpeg
+ * @frame_rate:		holds the frame_rate
+ * @jpeg_quality:	holds jpeq's quality (0~11)
+ * @jpeg_hq_quality:	holds hq's quality (0~11) only if hq_mode enabled
+ * @compression_mode:	holds jpeg compression mode
+ * @frame_bottom:	end position of video data in vertical direction
+ * @frame_left:		start position of video data in horizontal direction
+ * @frame_right:	end position of video data in horizontal direction
+ * @frame_top:		start position of video data in vertical direction
+ * @perf:		holds the statistics primary for debugfs
+ */
 struct aspeed_video {
 	void __iomem *base;
 	struct clk *eclk;
@@ -1244,7 +1277,7 @@ static int aspeed_video_get_parm(struct file *file, void *fh,
 	struct aspeed_video *video = video_drvdata(file);
 
 	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
-	a->parm.capture.readbuffers = 3;
+	a->parm.capture.readbuffers = ASPEED_VIDEO_V4L2_MIN_BUF_REQ;
 	a->parm.capture.timeperframe.numerator = 1;
 	if (!video->frame_rate)
 		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
@@ -1261,7 +1294,7 @@ static int aspeed_video_set_parm(struct file *file, void *fh,
 	struct aspeed_video *video = video_drvdata(file);
 
 	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
-	a->parm.capture.readbuffers = 3;
+	a->parm.capture.readbuffers = ASPEED_VIDEO_V4L2_MIN_BUF_REQ;
 
 	if (a->parm.capture.timeperframe.numerator)
 		frame_rate = a->parm.capture.timeperframe.denominator /
@@ -1868,7 +1901,7 @@ static int aspeed_video_setup_video(struct aspeed_video *video)
 	vbq->drv_priv = video;
 	vbq->buf_struct_size = sizeof(struct aspeed_video_buffer);
 	vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
-	vbq->min_buffers_needed = 3;
+	vbq->min_buffers_needed = ASPEED_VIDEO_V4L2_MIN_BUF_REQ;
 
 	rc = vb2_queue_init(vbq);
 	if (rc) {
-- 
2.25.1

[PATCH v3 7/7] media: aspeed: Extend debug message

[Jammy Huang]

updated as below:

Capture:
  Mode                : Direct fetch
  VGA bpp mode        : 32
  Signal              : lock
  Width               : 1920
  Height              : 1080
  FRC                 : 0

Compression:
  Format              : JPEG
  Subsampling         : 444
  Quality             : 4
  HQ Mode             : N/A
  HQ Quality          : 0
  Mode                : N/A

Performance:
  Frame#              : 4
  Frame Duration(ms)  :
    Now               : 22
    Min               : 21
    Max               : 22
  FPS                 : 47

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 41 +++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 8e0c29551ffc..7b7a03d44af7 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -453,6 +453,9 @@ static const struct v4l2_dv_timings_cap aspeed_video_timings_cap = {
 	},
 };
 
+static const char * const compress_mode_str[] = {"DCT Only",
+	"DCT VQ mix 2-color", "DCT VQ mix 4-color"};
+
 static unsigned int debug;
 
 static void aspeed_video_init_jpeg_table(u32 *table, bool yuv420)
@@ -1071,8 +1074,6 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
 
 static void aspeed_video_update_regs(struct aspeed_video *video)
 {
-	static const char * const compress_mode_str[] = {"DCT Only",
-		"DCT VQ mix 2-color", "DCT VQ mix 4-color"};
 	u32 comp_ctrl =	FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
 		FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10) |
 		FIELD_PREP(VE_COMP_CTRL_EN_HQ, video->hq_mode) |
@@ -1785,9 +1786,29 @@ static const struct vb2_ops aspeed_video_vb2_ops = {
 static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
 {
 	struct aspeed_video *v = s->private;
+	u32 val08;
 
 	seq_puts(s, "\n");
 
+	val08 = aspeed_video_read(v, VE_CTRL);
+	seq_puts(s, "Capture:\n");
+	if (FIELD_GET(VE_CTRL_DIRECT_FETCH, val08)) {
+		seq_printf(s, "  %-20s:\tDirect fetch\n", "Mode");
+		seq_printf(s, "  %-20s:\t%s\n", "VGA bpp mode",
+			   FIELD_GET(VE_CTRL_INT_DE, val08) ? "16" : "32");
+	} else {
+		seq_printf(s, "  %-20s:\tSync\n", "Mode");
+		seq_printf(s, "  %-20s:\t%s\n", "Video source",
+			   FIELD_GET(VE_CTRL_SOURCE, val08) ?
+			   "external" : "internal");
+		seq_printf(s, "  %-20s:\t%s\n", "DE source",
+			   FIELD_GET(VE_CTRL_INT_DE, val08) ?
+			   "internal" : "external");
+		seq_printf(s, "  %-20s:\t%s\n", "Cursor overlay",
+			   FIELD_GET(VE_CTRL_AUTO_OR_CURSOR, val08) ?
+			   "Without" : "With");
+	}
+
 	seq_printf(s, "  %-20s:\t%s\n", "Signal",
 		   v->v4l2_input_status ? "Unlock" : "Lock");
 	seq_printf(s, "  %-20s:\t%d\n", "Width", v->pix_fmt.width);
@@ -1796,6 +1817,21 @@ static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
 
 	seq_puts(s, "\n");
 
+	seq_puts(s, "Compression:\n");
+	seq_printf(s, "  %-20s:\t%s\n", "Format",
+		   v->partial_jpeg ? "Aspeed" : "JPEG");
+	seq_printf(s, "  %-20s:\t%s\n", "Subsampling",
+		   v->yuv420 ? "420" : "444");
+	seq_printf(s, "  %-20s:\t%d\n", "Quality", v->jpeg_quality);
+	seq_printf(s, "  %-20s:\t%s\n", "HQ Mode",
+		   v->partial_jpeg ? (v->hq_mode ? "on" : "off") : "N/A");
+	seq_printf(s, "  %-20s:\t%d\n", "HQ Quality", v->jpeg_hq_quality);
+	seq_printf(s, "  %-20s:\t%s\n", "Mode",
+		   v->partial_jpeg ? compress_mode_str[v->compression_mode]
+				   : "N/A");
+
+	seq_puts(s, "\n");
+
 	seq_puts(s, "Performance:\n");
 	seq_printf(s, "  %-20s:\t%d\n", "Frame#", v->sequence);
 	seq_printf(s, "  %-20s:\n", "Frame Duration(ms)");
@@ -1804,7 +1840,6 @@ static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
 	seq_printf(s, "    %-18s:\t%d\n", "Max", v->perf.duration_max);
 	seq_printf(s, "  %-20s:\t%d\n", "FPS", 1000/(v->perf.totaltime/v->sequence));
 
-
 	return 0;
 }
 
-- 
2.25.1