Linux-Aspeed Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net/ncsi: Add NCSI Mellanox OEM command
From: Vijay Khemka @ 2018-11-20 20:35 UTC (permalink / raw)
  To: linux-aspeed

This patch adds OEM Mellanox commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.

ncsi_oem_gma_handler_mlx: This handler send NCSI mellanox command for
getting mac address.
ncsi_rsp_handler_oem_mlx: This handles response received for all
mellanox OEM commands.
ncsi_rsp_handler_oem_mlx_gma: This handles get mac address response and
set it to device.

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 net/ncsi/internal.h    |  5 +++++
 net/ncsi/ncsi-manage.c | 25 ++++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  9 +++++++++
 net/ncsi/ncsi-rsp.c    | 41 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 1dae77c54009..7f3eb1360b9b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -73,10 +73,15 @@ enum {
 #define NCSI_OEM_MFR_BCM_ID             0x113d
 /* Broadcom specific OEM Command */
 #define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC */
+/* Mellanox specific OEM Command */
+#define NCSI_OEM_MLX_CMD_GMA            0x00   /* CMD ID for Get MAC */
+#define NCSI_OEM_MLX_CMD_GMA_PARAM      0x1b   /* Parameter for GMA  */
 /* OEM Command payload lengths*/
 #define NCSI_OEM_BCM_CMD_GMA_LEN        12
+#define NCSI_OEM_MLX_CMD_GMA_LEN        8
 /* Mac address offset in OEM response */
 #define BCM_MAC_ADDR_OFFSET             28
+#define MLX_MAC_ADDR_OFFSET             8
 
 
 struct ncsi_channel_version {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index bfc43b28c7a6..eacb653ff987 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -675,12 +675,35 @@ static int ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
 	return ret;
 }
 
+static int ncsi_oem_gma_handler_mlx(struct ncsi_cmd_arg *nca)
+{
+	unsigned char data[NCSI_OEM_MLX_CMD_GMA_LEN];
+	int ret = 0;
+
+	nca->payload = NCSI_OEM_MLX_CMD_GMA_LEN;
+
+	memset(data, 0, NCSI_OEM_MLX_CMD_GMA_LEN);
+	*(unsigned int *)data = ntohl(NCSI_OEM_MFR_MLX_ID);
+	data[5] = NCSI_OEM_MLX_CMD_GMA;
+	data[6] = NCSI_OEM_MLX_CMD_GMA_PARAM;
+
+	nca->data = data;
+
+	ret = ncsi_xmit_cmd(nca);
+	if (ret)
+		netdev_err(nca->ndp->ndev.dev,
+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
+			   nca->type);
+	return ret;
+}
+
 /* OEM Command handlers initialization */
 static struct ncsi_oem_gma_handler {
 	unsigned int	mfr_id;
 	int		(*handler)(struct ncsi_cmd_arg *nca);
 } ncsi_oem_gma_handlers[] = {
-	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
 };
 
 static int ncsi_gma_handler(struct ncsi_cmd_arg *nca, unsigned int mf_id)
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 4d3f06be38bd..2a6d83a596c9 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,15 @@ struct ncsi_rsp_oem_pkt {
 	unsigned char           data[];      /* Payload data      */
 };
 
+/* Mellanox Response Data */
+struct ncsi_rsp_oem_mlx_pkt {
+	unsigned char           cmd_rev;     /* Command Revision  */
+	unsigned char           cmd;         /* Command ID        */
+	unsigned char           param;       /* Parameter         */
+	unsigned char           optional;    /* Optional data     */
+	unsigned char           data[];      /* Data              */
+};
+
 /* Broadcom Response Data */
 struct ncsi_rsp_oem_bcm_pkt {
 	unsigned char           ver;         /* Payload Version   */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 77e07ba3f493..ba9a4ba97c64 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -611,6 +611,45 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
 	return 0;
 }
 
+/* Response handler for Mellanox command Get Mac Address */
+static int ncsi_rsp_handler_oem_mlx_gma(struct ncsi_request *nr)
+{
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct net_device *ndev = ndp->ndev.dev;
+	const struct net_device_ops *ops = ndev->netdev_ops;
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct sockaddr saddr;
+	int ret = 0;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+	saddr.sa_family = ndev->type;
+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	memcpy(saddr.sa_data, &rsp->data[MLX_MAC_ADDR_OFFSET], ETH_ALEN);
+	ret = ops->ndo_set_mac_address(ndev, &saddr);
+	if (ret < 0)
+		netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
+
+	return ret;
+}
+
+/* Response handler for Mellanox card */
+static int ncsi_rsp_handler_oem_mlx(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_oem_mlx_pkt *mlx;
+	struct ncsi_rsp_oem_pkt *rsp;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+	mlx = (struct ncsi_rsp_oem_mlx_pkt *)(rsp->data);
+
+	if (mlx->cmd == NCSI_OEM_MLX_CMD_GMA &&
+	    mlx->param == NCSI_OEM_MLX_CMD_GMA_PARAM)
+		return ncsi_rsp_handler_oem_mlx_gma(nr);
+	return 0;
+}
+
 /* Response handler for Broadcom command Get Mac Address */
 static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
 {
@@ -655,7 +694,7 @@ static struct ncsi_rsp_oem_handler {
 	unsigned int	mfr_id;
 	int		(*handler)(struct ncsi_request *nr);
 } ncsi_rsp_oem_handlers[] = {
-	{ NCSI_OEM_MFR_MLX_ID, NULL },
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_rsp_handler_oem_mlx },
 	{ NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
 };
 
-- 
2.17.1


^ permalink raw reply related

* Question on plan for dev-4.19
From: Shivah Shankar @ 2018-11-19  7:48 UTC (permalink / raw)
  To: linux-aspeed

Hi,
Just wanted to understand if there is a plan to create a dev-4.19 branch
anytime soon. Is there any tentative timeline for the same.

The reason for my ask, we are trying to push our changes and additions
to the openbmc/linux, wanted to know if the current dev-4.18 is a good
place to start or if there is any plan for dev-4.19 then can we consider
looking at 4.19.

Thank you in advance for your response.

regards,
shivas



^ permalink raw reply

* [PATCH v5 0/2] media: platform: Add Aspeed Video Engine Driver
From: Hans Verkuil @ 2018-11-16 11:12 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <9b21fb44-2248-7921-2205-4a3f43d22d4b@xs4all.nl>

On 11/15/2018 10:20 AM, Hans Verkuil wrote:
> On 11/12/2018 10:00 PM, Eddie James wrote:
>> From: Eddie James <eajames@linux.vnet.ibm.com>
>>
>> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
>> can capture and compress video data from digital or analog sources. With
>> the Aspeed chip acting as a service processor, the Video Engine can
>> capture the host processor graphics output.
>>
>> This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming
>> interface by way of videobuf2. Each frame, the driver triggers the hardware to
>> capture the host graphics output and compress it to JPEG format.
> 
> I reviewed the driver, and there is still confusion about active timings and
> detected timings. You are really close, but it is still not right.
> 
> The timings returned by G_DV_TIMINGS should never change, unless by calling
> S_DV_TIMINGS. The format returned by G_FMT should never change, unless by
> calling S_DV_TIMINGS (which implicitly changes the format) or S_FMT.
> 
> Timings changes from the video or calling QUERY_DV_TIMINGS should have NO
> effect on the timings/format returned by G_DV_TIMINGS/G_FMT.
> 
> Obviously, if the video timings change, then streaming will halt and an event
> is issued so userspace can take action.
> 
>>
>> v4l-utils HEAD dd3ff81f58c4e1e6f33765dc61ad33c48ae6bb07
>>
>> v4l2-compliance SHA: not available, 32 bits
> 
> <snip>
> 
>> Control ioctls (Input 0):
>> 	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
>> 	test VIDIOC_QUERYCTRL: OK
>> 	test VIDIOC_G/S_CTRL: OK
>> 	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
>> 		fail: v4l2-test-controls.cpp(823): failed to find event for control 'Chroma Subsampling'
> 
> That's weird. It's not clear to me why this fails.

Ah, this is caused by a bug in our tree. It's fixed by this patch:

https://patchwork.linuxtv.org/patch/52790/

Regards,

	Hans

> 
>> 	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
>> 	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>> 	Standard Controls: 3 Private Controls: 0
>>
>> Format ioctls (Input 0):
>> 	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>> 	test VIDIOC_G/S_PARM: OK
>> 	test VIDIOC_G_FBUF: OK (Not Supported)
>> 	test VIDIOC_G_FMT: OK
>> 	test VIDIOC_TRY_FMT: OK
>> 	test VIDIOC_S_FMT: OK
>> 	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>> 	test Cropping: OK (Not Supported)
>> 	test Composing: OK (Not Supported)
>> 	test Scaling: OK (Not Supported)
>>
>> Codec ioctls (Input 0):
>> 	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>> 	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>> 	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>>
>> Buffer ioctls (Input 0):
>> 		fail: v4l2-test-buffers.cpp(422): dmabuf_valid ^ !!(caps & V4L2_BUF_CAP_SUPPORTS_DMABUF)
> 
> I updated v4l2-compliance, as this test was a bit too strict.
> 
>> 	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
>> 	test VIDIOC_EXPBUF: OK (Not Supported)
>>
>> Test input 0:
>>
>> Streaming ioctls:
>> 	test read/write: OK
>> 	test blocking wait: OK
>> 	test MMAP: OK                                     
>> 	test USERPTR: OK (Not Supported)
>> 		fail: v4l2-test-buffers.cpp(1102): exp_q.reqbufs(expbuf_node, q.g_buffers())
>> 		fail: v4l2-test-buffers.cpp(1192): setupDmaBuf(expbuf_node, node, q, exp_q)
> 
> You need to add:
> 
> 	.vidioc_expbuf = vb2_ioctl_expbuf,
> 
> to aspeed_video_ioctl_ops.
> 
>> 	test DMABUF: FAIL
>>
>> Total: 48, Succeeded: 45, Failed: 3, Warnings: 0
>>
>> The apparent rate of change to the v4l2/vb2 API makes it difficult to pass
>> these tests. None of the failing tests even ran last time I submitted my
>> series. And V4L2_BUF_CAP_SUPPORTS_DMABUF is undefined in 4.19.
> 
> When developing new drivers you should use the master branch of
> https://git.linuxtv.org/media_tree.git/
> 
> The v4l-utils repo is kept in sync with the latest code from that master branch.
> 
> Regards,
> 
> 	Hans
> 
>>
>> Changes since v4:
>>  - Set real min and max resolution in enum_dv_timings
>>  - Add check for vb2_is_busy before settings the timings
>>  - Set max frame rate to the actual max rather than max + 1
>>  - Correct the input status to 0.
>>  - Rework resolution change to only set the relevant h/w regs during startup or
>>    when set_timings is called.
>>
>> Changes since v3:
>>  - Switch update reg function to use u32 clear rather than unsigned long mask
>>  - Add timing information from host VGA signal
>>  - Fix binding documentation mispelling
>>  - Fix upper case hex values
>>  - Add wait_prepare and wait_finish
>>  - Set buffer state to queued (rather than error) if streaming fails to start
>>  - Switch engine busy print statement to error
>>  - Removed a couple unecessary type assignments in v4l2 ioctls
>>  - Added query_dv_timings, fixed get_dv_timings
>>  - Corrected source change event to fire if and only if size actually changes
>>  - Locked open and release
>>
>> Changes since v2:
>>  - Switch to streaming interface. This involved a lot of changes.
>>  - Rework memory allocation due to using videobuf2 buffers, but also only
>>    allocate the necessary size of source buffer rather than the max size
>>
>> Changes since v1:
>>  - Removed le32_to_cpu calls for JPEG header data
>>  - Reworked v4l2 ioctls to be compliant.
>>  - Added JPEG controls
>>  - Updated devicetree docs according to Rob's suggestions.
>>  - Added myself to MAINTAINERS
>>
>> Eddie James (2):
>>   dt-bindings: media: Add Aspeed Video Engine binding documentation
>>   media: platform: Add Aspeed Video Engine driver
>>
>>  .../devicetree/bindings/media/aspeed-video.txt     |   26 +
>>  MAINTAINERS                                        |    8 +
>>  drivers/media/platform/Kconfig                     |    9 +
>>  drivers/media/platform/Makefile                    |    1 +
>>  drivers/media/platform/aspeed-video.c              | 1711 ++++++++++++++++++++
>>  5 files changed, 1755 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt
>>  create mode 100644 drivers/media/platform/aspeed-video.c
>>
> 


^ permalink raw reply

* [PATCH v5 0/2] media: platform: Add Aspeed Video Engine Driver
From: Hans Verkuil @ 2018-11-15  9:20 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1542056431-18146-1-git-send-email-eajames@linux.ibm.com>

On 11/12/2018 10:00 PM, Eddie James wrote:
> From: Eddie James <eajames@linux.vnet.ibm.com>
> 
> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
> can capture and compress video data from digital or analog sources. With
> the Aspeed chip acting as a service processor, the Video Engine can
> capture the host processor graphics output.
> 
> This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming
> interface by way of videobuf2. Each frame, the driver triggers the hardware to
> capture the host graphics output and compress it to JPEG format.

I reviewed the driver, and there is still confusion about active timings and
detected timings. You are really close, but it is still not right.

The timings returned by G_DV_TIMINGS should never change, unless by calling
S_DV_TIMINGS. The format returned by G_FMT should never change, unless by
calling S_DV_TIMINGS (which implicitly changes the format) or S_FMT.

Timings changes from the video or calling QUERY_DV_TIMINGS should have NO
effect on the timings/format returned by G_DV_TIMINGS/G_FMT.

Obviously, if the video timings change, then streaming will halt and an event
is issued so userspace can take action.

> 
> v4l-utils HEAD dd3ff81f58c4e1e6f33765dc61ad33c48ae6bb07
> 
> v4l2-compliance SHA: not available, 32 bits

<snip>

> Control ioctls (Input 0):
> 	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> 	test VIDIOC_QUERYCTRL: OK
> 	test VIDIOC_G/S_CTRL: OK
> 	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> 		fail: v4l2-test-controls.cpp(823): failed to find event for control 'Chroma Subsampling'

That's weird. It's not clear to me why this fails.

> 	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
> 	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> 	Standard Controls: 3 Private Controls: 0
> 
> Format ioctls (Input 0):
> 	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> 	test VIDIOC_G/S_PARM: OK
> 	test VIDIOC_G_FBUF: OK (Not Supported)
> 	test VIDIOC_G_FMT: OK
> 	test VIDIOC_TRY_FMT: OK
> 	test VIDIOC_S_FMT: OK
> 	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> 	test Cropping: OK (Not Supported)
> 	test Composing: OK (Not Supported)
> 	test Scaling: OK (Not Supported)
> 
> Codec ioctls (Input 0):
> 	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> 	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> 	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> 
> Buffer ioctls (Input 0):
> 		fail: v4l2-test-buffers.cpp(422): dmabuf_valid ^ !!(caps & V4L2_BUF_CAP_SUPPORTS_DMABUF)

I updated v4l2-compliance, as this test was a bit too strict.

> 	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
> 	test VIDIOC_EXPBUF: OK (Not Supported)
> 
> Test input 0:
> 
> Streaming ioctls:
> 	test read/write: OK
> 	test blocking wait: OK
> 	test MMAP: OK                                     
> 	test USERPTR: OK (Not Supported)
> 		fail: v4l2-test-buffers.cpp(1102): exp_q.reqbufs(expbuf_node, q.g_buffers())
> 		fail: v4l2-test-buffers.cpp(1192): setupDmaBuf(expbuf_node, node, q, exp_q)

You need to add:

	.vidioc_expbuf = vb2_ioctl_expbuf,

to aspeed_video_ioctl_ops.

> 	test DMABUF: FAIL
> 
> Total: 48, Succeeded: 45, Failed: 3, Warnings: 0
> 
> The apparent rate of change to the v4l2/vb2 API makes it difficult to pass
> these tests. None of the failing tests even ran last time I submitted my
> series. And V4L2_BUF_CAP_SUPPORTS_DMABUF is undefined in 4.19.

When developing new drivers you should use the master branch of
https://git.linuxtv.org/media_tree.git/

The v4l-utils repo is kept in sync with the latest code from that master branch.

Regards,

	Hans

> 
> Changes since v4:
>  - Set real min and max resolution in enum_dv_timings
>  - Add check for vb2_is_busy before settings the timings
>  - Set max frame rate to the actual max rather than max + 1
>  - Correct the input status to 0.
>  - Rework resolution change to only set the relevant h/w regs during startup or
>    when set_timings is called.
> 
> Changes since v3:
>  - Switch update reg function to use u32 clear rather than unsigned long mask
>  - Add timing information from host VGA signal
>  - Fix binding documentation mispelling
>  - Fix upper case hex values
>  - Add wait_prepare and wait_finish
>  - Set buffer state to queued (rather than error) if streaming fails to start
>  - Switch engine busy print statement to error
>  - Removed a couple unecessary type assignments in v4l2 ioctls
>  - Added query_dv_timings, fixed get_dv_timings
>  - Corrected source change event to fire if and only if size actually changes
>  - Locked open and release
> 
> Changes since v2:
>  - Switch to streaming interface. This involved a lot of changes.
>  - Rework memory allocation due to using videobuf2 buffers, but also only
>    allocate the necessary size of source buffer rather than the max size
> 
> Changes since v1:
>  - Removed le32_to_cpu calls for JPEG header data
>  - Reworked v4l2 ioctls to be compliant.
>  - Added JPEG controls
>  - Updated devicetree docs according to Rob's suggestions.
>  - Added myself to MAINTAINERS
> 
> Eddie James (2):
>   dt-bindings: media: Add Aspeed Video Engine binding documentation
>   media: platform: Add Aspeed Video Engine driver
> 
>  .../devicetree/bindings/media/aspeed-video.txt     |   26 +
>  MAINTAINERS                                        |    8 +
>  drivers/media/platform/Kconfig                     |    9 +
>  drivers/media/platform/Makefile                    |    1 +
>  drivers/media/platform/aspeed-video.c              | 1711 ++++++++++++++++++++
>  5 files changed, 1755 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt
>  create mode 100644 drivers/media/platform/aspeed-video.c
> 


^ permalink raw reply

* [PATCH v5 2/2] media: platform: Add Aspeed Video Engine driver
From: Hans Verkuil @ 2018-11-15  8:56 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1542056431-18146-3-git-send-email-eajames@linux.ibm.com>

On 11/12/2018 10:00 PM, Eddie James wrote:
> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
> can capture and compress video data from digital or analog sources. With
> the Aspeed chip acting a service processor, the Video Engine can capture
> the host processor graphics output.
> 
> Add a V4L2 driver to capture video data and compress it to JPEG images.
> Make the video frames available through the V4L2 streaming interface.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  MAINTAINERS                           |    8 +
>  drivers/media/platform/Kconfig        |    9 +
>  drivers/media/platform/Makefile       |    1 +
>  drivers/media/platform/aspeed-video.c | 1711 +++++++++++++++++++++++++++++++++
>  4 files changed, 1729 insertions(+)
>  create mode 100644 drivers/media/platform/aspeed-video.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fa45ff3..f8f08a4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2414,6 +2414,14 @@ S:	Maintained
>  F:	Documentation/hwmon/asc7621
>  F:	drivers/hwmon/asc7621.c
>  
> +ASPEED VIDEO ENGINE DRIVER
> +M:	Eddie James <eajames@linux.ibm.com>
> +L:	linux-media at vger.kernel.org
> +L:	openbmc at lists.ozlabs.org (moderated for non-subscribers)
> +S:	Maintained
> +F:	drivers/media/platform/aspeed-video.c
> +F:	Documentation/devicetree/bindings/media/aspeed-video.txt
> +
>  ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
>  M:	Corentin Chary <corentin.chary@gmail.com>
>  L:	acpi4asus-user at lists.sourceforge.net
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 70c4f6c..ba78dd5 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig"
>  
>  source "drivers/media/platform/omap/Kconfig"
>  
> +config VIDEO_ASPEED
> +	tristate "Aspeed AST2400 and AST2500 Video Engine driver"
> +	depends on VIDEO_V4L2
> +	select VIDEOBUF2_DMA_CONTIG
> +	help
> +	  Support for the Aspeed Video Engine (VE) embedded in the Aspeed
> +	  AST2400 and AST2500 SOCs. The VE can capture and compress video data
> +	  from digital or analog sources.
> +
>  config VIDEO_SH_VOU
>  	tristate "SuperH VOU video output driver"
>  	depends on MEDIA_CAMERA_SUPPORT
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 6ab6200..2973953 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for the video capture/playback device drivers.
>  #
>  
> +obj-$(CONFIG_VIDEO_ASPEED)		+= aspeed-video.o
>  obj-$(CONFIG_VIDEO_CADENCE)		+= cadence/
>  obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o
>  obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> new file mode 100644
> index 0000000..a2fd0bf
> --- /dev/null
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -0,0 +1,1711 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <linux/atomic.h>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/sched.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
> +#include <linux/v4l2-controls.h>
> +#include <linux/videodev2.h>
> +#include <linux/wait.h>
> +#include <linux/workqueue.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +
> +#define DEVICE_NAME			"aspeed-video"
> +
> +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
> +#define ASPEED_VIDEO_JPEG_HEADER_SIZE	10
> +#define ASPEED_VIDEO_JPEG_QUANT_SIZE	116
> +#define ASPEED_VIDEO_JPEG_DCT_SIZE	34
> +
> +#define MAX_FRAME_RATE			60
> +#define MAX_HEIGHT			1200
> +#define MAX_WIDTH			1920
> +#define MIN_HEIGHT			480
> +#define MIN_WIDTH			640
> +
> +#define NUM_POLARITY_CHECKS		10
> +#define INVALID_RESOLUTION_RETRIES	2
> +#define INVALID_RESOLUTION_DELAY	msecs_to_jiffies(250)
> +#define RESOLUTION_CHANGE_DELAY		msecs_to_jiffies(500)
> +#define MODE_DETECT_TIMEOUT		msecs_to_jiffies(500)
> +#define STOP_TIMEOUT			msecs_to_jiffies(250)
> +#define DIRECT_FETCH_THRESHOLD		0x0c0000 /* 1024 * 768 */
> +
> +#define VE_MAX_SRC_BUFFER_SIZE		0x8ca000 /* 1920 * 1200, 32bpp */
> +#define VE_JPEG_HEADER_SIZE		0x006000 /* 512 * 12 * 4 */
> +
> +#define VE_PROTECTION_KEY		0x000
> +#define  VE_PROTECTION_KEY_UNLOCK	0x1a038aa8
> +
> +#define VE_SEQ_CTRL			0x004
> +#define  VE_SEQ_CTRL_TRIG_MODE_DET	BIT(0)
> +#define  VE_SEQ_CTRL_TRIG_CAPTURE	BIT(1)
> +#define  VE_SEQ_CTRL_FORCE_IDLE		BIT(2)
> +#define  VE_SEQ_CTRL_MULT_FRAME		BIT(3)
> +#define  VE_SEQ_CTRL_TRIG_COMP		BIT(4)
> +#define  VE_SEQ_CTRL_AUTO_COMP		BIT(5)
> +#define  VE_SEQ_CTRL_EN_WATCHDOG	BIT(7)
> +#define  VE_SEQ_CTRL_YUV420		BIT(10)
> +#define  VE_SEQ_CTRL_COMP_FMT		GENMASK(11, 10)
> +#define  VE_SEQ_CTRL_HALT		BIT(12)
> +#define  VE_SEQ_CTRL_EN_WATCHDOG_COMP	BIT(14)
> +#define  VE_SEQ_CTRL_TRIG_JPG		BIT(15)
> +#define  VE_SEQ_CTRL_CAP_BUSY		BIT(16)
> +#define  VE_SEQ_CTRL_COMP_BUSY		BIT(18)
> +
> +#ifdef CONFIG_MACH_ASPEED_G5
> +#define  VE_SEQ_CTRL_JPEG_MODE		BIT(13)	/* AST2500 */
> +#else
> +#define  VE_SEQ_CTRL_JPEG_MODE		BIT(8)	/* AST2400 */
> +#endif /* CONFIG_MACH_ASPEED_G5 */
> +
> +#define VE_CTRL				0x008
> +#define  VE_CTRL_HSYNC_POL		BIT(0)
> +#define  VE_CTRL_VSYNC_POL		BIT(1)
> +#define  VE_CTRL_SOURCE			BIT(2)
> +#define  VE_CTRL_INT_DE			BIT(4)
> +#define  VE_CTRL_DIRECT_FETCH		BIT(5)
> +#define  VE_CTRL_YUV			BIT(6)
> +#define  VE_CTRL_RGB			BIT(7)
> +#define  VE_CTRL_CAPTURE_FMT		GENMASK(7, 6)
> +#define  VE_CTRL_AUTO_OR_CURSOR		BIT(8)
> +#define  VE_CTRL_CLK_INVERSE		BIT(11)
> +#define  VE_CTRL_CLK_DELAY		GENMASK(11, 9)
> +#define  VE_CTRL_INTERLACE		BIT(14)
> +#define  VE_CTRL_HSYNC_POL_CTRL		BIT(15)
> +#define  VE_CTRL_FRC			GENMASK(23, 16)
> +
> +#define VE_TGS_0			0x00c
> +#define VE_TGS_1			0x010
> +#define  VE_TGS_FIRST			GENMASK(28, 16)
> +#define  VE_TGS_LAST			GENMASK(12, 0)
> +
> +#define VE_SCALING_FACTOR		0x014
> +#define VE_SCALING_FILTER0		0x018
> +#define VE_SCALING_FILTER1		0x01c
> +#define VE_SCALING_FILTER2		0x020
> +#define VE_SCALING_FILTER3		0x024
> +
> +#define VE_CAP_WINDOW			0x030
> +#define VE_COMP_WINDOW			0x034
> +#define VE_COMP_PROC_OFFSET		0x038
> +#define VE_COMP_OFFSET			0x03c
> +#define VE_JPEG_ADDR			0x040
> +#define VE_SRC0_ADDR			0x044
> +#define VE_SRC_SCANLINE_OFFSET		0x048
> +#define VE_SRC1_ADDR			0x04c
> +#define VE_COMP_ADDR			0x054
> +
> +#define VE_STREAM_BUF_SIZE		0x058
> +#define  VE_STREAM_BUF_SIZE_N_PACKETS	GENMASK(5, 3)
> +#define  VE_STREAM_BUF_SIZE_P_SIZE	GENMASK(2, 0)
> +
> +#define VE_COMP_CTRL			0x060
> +#define  VE_COMP_CTRL_VQ_DCT_ONLY	BIT(0)
> +#define  VE_COMP_CTRL_VQ_4COLOR		BIT(1)
> +#define  VE_COMP_CTRL_QUANTIZE		BIT(2)
> +#define  VE_COMP_CTRL_EN_BQ		BIT(4)
> +#define  VE_COMP_CTRL_EN_CRYPTO		BIT(5)
> +#define  VE_COMP_CTRL_DCT_CHR		GENMASK(10, 6)
> +#define  VE_COMP_CTRL_DCT_LUM		GENMASK(15, 11)
> +#define  VE_COMP_CTRL_EN_HQ		BIT(16)
> +#define  VE_COMP_CTRL_RSVD		BIT(19)
> +#define  VE_COMP_CTRL_ENCODE		GENMASK(21, 20)
> +#define  VE_COMP_CTRL_HQ_DCT_CHR	GENMASK(26, 22)
> +#define  VE_COMP_CTRL_HQ_DCT_LUM	GENMASK(31, 27)
> +
> +#define VE_OFFSET_COMP_STREAM		0x078
> +
> +#define VE_SRC_LR_EDGE_DET		0x090
> +#define  VE_SRC_LR_EDGE_DET_LEFT	GENMASK(11, 0)
> +#define  VE_SRC_LR_EDGE_DET_NO_V	BIT(12)
> +#define  VE_SRC_LR_EDGE_DET_NO_H	BIT(13)
> +#define  VE_SRC_LR_EDGE_DET_NO_DISP	BIT(14)
> +#define  VE_SRC_LR_EDGE_DET_NO_CLK	BIT(15)
> +#define  VE_SRC_LR_EDGE_DET_RT_SHF	16
> +#define  VE_SRC_LR_EDGE_DET_RT		GENMASK(27, VE_SRC_LR_EDGE_DET_RT_SHF)
> +#define  VE_SRC_LR_EDGE_DET_INTERLACE	BIT(31)
> +
> +#define VE_SRC_TB_EDGE_DET		0x094
> +#define  VE_SRC_TB_EDGE_DET_TOP		GENMASK(12, 0)
> +#define  VE_SRC_TB_EDGE_DET_BOT_SHF	16
> +#define  VE_SRC_TB_EDGE_DET_BOT		GENMASK(28, VE_SRC_TB_EDGE_DET_BOT_SHF)
> +
> +#define VE_MODE_DETECT_STATUS		0x098
> +#define  VE_MODE_DETECT_H_PIXELS	GENMASK(11, 0)
> +#define  VE_MODE_DETECT_V_LINES_SHF	16
> +#define  VE_MODE_DETECT_V_LINES		GENMASK(27, VE_MODE_DETECT_V_LINES_SHF)
> +#define  VE_MODE_DETECT_STATUS_VSYNC	BIT(28)
> +#define  VE_MODE_DETECT_STATUS_HSYNC	BIT(29)
> +
> +#define VE_SYNC_STATUS			0x09c
> +#define  VE_SYNC_STATUS_HSYNC		GENMASK(11, 0)
> +#define  VE_SYNC_STATUS_VSYNC_SHF	16
> +#define  VE_SYNC_STATUS_VSYNC		GENMASK(27, VE_SYNC_STATUS_VSYNC_SHF)
> +
> +#define VE_INTERRUPT_CTRL		0x304
> +#define VE_INTERRUPT_STATUS		0x308
> +#define  VE_INTERRUPT_MODE_DETECT_WD	BIT(0)
> +#define  VE_INTERRUPT_CAPTURE_COMPLETE	BIT(1)
> +#define  VE_INTERRUPT_COMP_READY	BIT(2)
> +#define  VE_INTERRUPT_COMP_COMPLETE	BIT(3)
> +#define  VE_INTERRUPT_MODE_DETECT	BIT(4)
> +#define  VE_INTERRUPT_FRAME_COMPLETE	BIT(5)
> +#define  VE_INTERRUPT_DECODE_ERR	BIT(6)
> +#define  VE_INTERRUPT_HALT_READY	BIT(8)
> +#define  VE_INTERRUPT_HANG_WD		BIT(9)
> +#define  VE_INTERRUPT_STREAM_DESC	BIT(10)
> +#define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
> +
> +#define VE_MODE_DETECT			0x30c
> +#define VE_MEM_RESTRICT_START		0x310
> +#define VE_MEM_RESTRICT_END		0x314
> +
> +enum {
> +	VIDEO_MODE_DETECT_DONE,
> +	VIDEO_RES_CHANGE,
> +	VIDEO_STREAMING,
> +	VIDEO_FRAME_INPRG,
> +};
> +
> +struct aspeed_video_addr {
> +	unsigned int size;
> +	dma_addr_t dma;
> +	void *virt;
> +};
> +
> +struct aspeed_video_buffer {
> +	struct vb2_v4l2_buffer vb;
> +	struct list_head link;
> +};
> +
> +#define to_aspeed_video_buffer(x) \
> +	container_of((x), struct aspeed_video_buffer, vb)
> +
> +struct aspeed_video {
> +	void __iomem *base;
> +	struct clk *eclk;
> +	struct clk *vclk;
> +	struct reset_control *rst;
> +
> +	struct device *dev;
> +	struct v4l2_ctrl_handler ctrl_handler;
> +	struct v4l2_device v4l2_dev;
> +	struct v4l2_pix_format pix_fmt;
> +	struct v4l2_bt_timings timings;

You need two v4l2_bt_timings structs here:

	struct v4l2_bt_timings active_timings;
	struct v4l2_bt_timings detected_timings;

The first is what s_dv_timings set (and must also be initialized to
some timing at probe time), the second is what aspeed_video_get_resolution
fills in.

See more about this below.

> +	struct vb2_queue queue;
> +	struct video_device vdev;
> +	struct mutex video_lock;
> +
> +	atomic_t clients;
> +	wait_queue_head_t wait;
> +	spinlock_t lock;
> +	struct delayed_work res_work;
> +	struct list_head buffers;
> +	unsigned long flags;
> +	unsigned int sequence;
> +
> +	unsigned int max_compressed_size;
> +	struct aspeed_video_addr srcs[2];
> +	struct aspeed_video_addr jpeg;
> +	struct aspeed_video_addr detect;
> +
> +	bool yuv420;
> +	unsigned int frame_rate;
> +	unsigned int jpeg_quality;
> +	unsigned int capture_height;
> +	unsigned int capture_width;
> +};
> +
> +#define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
> +
> +static const u32 aspeed_video_jpeg_header[ASPEED_VIDEO_JPEG_HEADER_SIZE] = {
> +	0xe0ffd8ff, 0x464a1000, 0x01004649, 0x60000101, 0x00006000, 0x0f00feff,
> +	0x00002d05, 0x00000000, 0x00000000, 0x00dbff00
> +};
> +
> +static const u32 aspeed_video_jpeg_quant[ASPEED_VIDEO_JPEG_QUANT_SIZE] = {
> +	0x081100c0, 0x00000000, 0x00110103, 0x03011102, 0xc4ff0111, 0x00001f00,
> +	0x01010501, 0x01010101, 0x00000000, 0x00000000, 0x04030201, 0x08070605,
> +	0xff0b0a09, 0x10b500c4, 0x03010200, 0x03040203, 0x04040505, 0x7d010000,
> +	0x00030201, 0x12051104, 0x06413121, 0x07615113, 0x32147122, 0x08a19181,
> +	0xc1b14223, 0xf0d15215, 0x72623324, 0x160a0982, 0x1a191817, 0x28272625,
> +	0x35342a29, 0x39383736, 0x4544433a, 0x49484746, 0x5554534a, 0x59585756,
> +	0x6564635a, 0x69686766, 0x7574736a, 0x79787776, 0x8584837a, 0x89888786,
> +	0x9493928a, 0x98979695, 0xa3a29a99, 0xa7a6a5a4, 0xb2aaa9a8, 0xb6b5b4b3,
> +	0xbab9b8b7, 0xc5c4c3c2, 0xc9c8c7c6, 0xd4d3d2ca, 0xd8d7d6d5, 0xe2e1dad9,
> +	0xe6e5e4e3, 0xeae9e8e7, 0xf4f3f2f1, 0xf8f7f6f5, 0xc4fffaf9, 0x00011f00,
> +	0x01010103, 0x01010101, 0x00000101, 0x00000000, 0x04030201, 0x08070605,
> +	0xff0b0a09, 0x11b500c4, 0x02010200, 0x04030404, 0x04040507, 0x77020100,
> +	0x03020100, 0x21050411, 0x41120631, 0x71610751, 0x81322213, 0x91421408,
> +	0x09c1b1a1, 0xf0523323, 0xd1726215, 0x3424160a, 0x17f125e1, 0x261a1918,
> +	0x2a292827, 0x38373635, 0x44433a39, 0x48474645, 0x54534a49, 0x58575655,
> +	0x64635a59, 0x68676665, 0x74736a69, 0x78777675, 0x83827a79, 0x87868584,
> +	0x928a8988, 0x96959493, 0x9a999897, 0xa5a4a3a2, 0xa9a8a7a6, 0xb4b3b2aa,
> +	0xb8b7b6b5, 0xc3c2bab9, 0xc7c6c5c4, 0xd2cac9c8, 0xd6d5d4d3, 0xdad9d8d7,
> +	0xe5e4e3e2, 0xe9e8e7e6, 0xf4f3f2ea, 0xf8f7f6f5, 0xdafffaf9, 0x01030c00,
> +	0x03110200, 0x003f0011
> +};
> +
> +static const u32 aspeed_video_jpeg_dct[ASPEED_VIDEO_JPEG_NUM_QUALITIES]
> +				      [ASPEED_VIDEO_JPEG_DCT_SIZE] = {
> +	{ 0x0d140043, 0x0c0f110f, 0x11101114, 0x17141516, 0x1e20321e,
> +	  0x3d1e1b1b, 0x32242e2b, 0x4b4c3f48, 0x44463f47, 0x61735a50,
> +	  0x566c5550, 0x88644644, 0x7a766c65, 0x4d808280, 0x8c978d60,
> +	  0x7e73967d, 0xdbff7b80, 0x1f014300, 0x272d2121, 0x3030582d,
> +	  0x697bb958, 0xb8b9b97b, 0xb9b8a6a6, 0xb9b9b9b9, 0xb9b9b9b9,
> +	  0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9,
> +	  0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9, 0xffb9b9b9 },
> +	{ 0x0c110043, 0x0a0d0f0d, 0x0f0e0f11, 0x14111213, 0x1a1c2b1a,
> +	  0x351a1818, 0x2b1f2826, 0x4142373f, 0x3c3d373e, 0x55644e46,
> +	  0x4b5f4a46, 0x77573d3c, 0x6b675f58, 0x43707170, 0x7a847b54,
> +	  0x6e64836d, 0xdbff6c70, 0x1b014300, 0x22271d1d, 0x2a2a4c27,
> +	  0x5b6ba04c, 0xa0a0a06b, 0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0,
> +	  0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0,
> +	  0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0, 0xffa0a0a0 },
> +	{ 0x090e0043, 0x090a0c0a, 0x0c0b0c0e, 0x110e0f10, 0x15172415,
> +	  0x2c151313, 0x241a211f, 0x36372e34, 0x31322e33, 0x4653413a,
> +	  0x3e4e3d3a, 0x62483231, 0x58564e49, 0x385d5e5d, 0x656d6645,
> +	  0x5b536c5a, 0xdbff595d, 0x16014300, 0x1c201818, 0x22223f20,
> +	  0x4b58853f, 0x85858558, 0x85858585, 0x85858585, 0x85858585,
> +	  0x85858585, 0x85858585, 0x85858585, 0x85858585, 0x85858585,
> +	  0x85858585, 0x85858585, 0x85858585, 0xff858585 },
> +	{ 0x070b0043, 0x07080a08, 0x0a090a0b, 0x0d0b0c0c, 0x11121c11,
> +	  0x23110f0f, 0x1c141a19, 0x2b2b2429, 0x27282428, 0x3842332e,
> +	  0x313e302e, 0x4e392827, 0x46443e3a, 0x2c4a4a4a, 0x50565137,
> +	  0x48425647, 0xdbff474a, 0x12014300, 0x161a1313, 0x1c1c331a,
> +	  0x3d486c33, 0x6c6c6c48, 0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c,
> +	  0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c,
> +	  0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c, 0xff6c6c6c },
> +	{ 0x06090043, 0x05060706, 0x07070709, 0x0a09090a, 0x0d0e160d,
> +	  0x1b0d0c0c, 0x16101413, 0x21221c20, 0x1e1f1c20, 0x2b332824,
> +	  0x26302624, 0x3d2d1f1e, 0x3735302d, 0x22393a39, 0x3f443f2b,
> +	  0x38334338, 0xdbff3739, 0x0d014300, 0x11130e0e, 0x15152613,
> +	  0x2d355026, 0x50505035, 0x50505050, 0x50505050, 0x50505050,
> +	  0x50505050, 0x50505050, 0x50505050, 0x50505050, 0x50505050,
> +	  0x50505050, 0x50505050, 0x50505050, 0xff505050 },
> +	{ 0x04060043, 0x03040504, 0x05040506, 0x07060606, 0x09090f09,
> +	  0x12090808, 0x0f0a0d0d, 0x16161315, 0x14151315, 0x1d221b18,
> +	  0x19201918, 0x281e1514, 0x2423201e, 0x17262726, 0x2a2d2a1c,
> +	  0x25222d25, 0xdbff2526, 0x09014300, 0x0b0d0a0a, 0x0e0e1a0d,
> +	  0x1f25371a, 0x37373725, 0x37373737, 0x37373737, 0x37373737,
> +	  0x37373737, 0x37373737, 0x37373737, 0x37373737, 0x37373737,
> +	  0x37373737, 0x37373737, 0x37373737, 0xff373737 },
> +	{ 0x02030043, 0x01020202, 0x02020203, 0x03030303, 0x04040704,
> +	  0x09040404, 0x07050606, 0x0b0b090a, 0x0a0a090a, 0x0e110d0c,
> +	  0x0c100c0c, 0x140f0a0a, 0x1211100f, 0x0b131313, 0x1516150e,
> +	  0x12111612, 0xdbff1213, 0x04014300, 0x05060505, 0x07070d06,
> +	  0x0f121b0d, 0x1b1b1b12, 0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b,
> +	  0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b,
> +	  0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b, 0xff1b1b1b },
> +	{ 0x01020043, 0x01010101, 0x01010102, 0x02020202, 0x03030503,
> +	  0x06030202, 0x05030404, 0x07070607, 0x06070607, 0x090b0908,
> +	  0x080a0808, 0x0d0a0706, 0x0c0b0a0a, 0x070c0d0c, 0x0e0f0e09,
> +	  0x0c0b0f0c, 0xdbff0c0c, 0x03014300, 0x03040303, 0x04040804,
> +	  0x0a0c1208, 0x1212120c, 0x12121212, 0x12121212, 0x12121212,
> +	  0x12121212, 0x12121212, 0x12121212, 0x12121212, 0x12121212,
> +	  0x12121212, 0x12121212, 0x12121212, 0xff121212 },
> +	{ 0x01020043, 0x01010101, 0x01010102, 0x02020202, 0x03030503,
> +	  0x06030202, 0x05030404, 0x07070607, 0x06070607, 0x090b0908,
> +	  0x080a0808, 0x0d0a0706, 0x0c0b0a0a, 0x070c0d0c, 0x0e0f0e09,
> +	  0x0c0b0f0c, 0xdbff0c0c, 0x02014300, 0x03030202, 0x04040703,
> +	  0x080a0f07, 0x0f0f0f0a, 0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f,
> +	  0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f,
> +	  0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f, 0xff0f0f0f },
> +	{ 0x01010043, 0x01010101, 0x01010101, 0x01010101, 0x02020302,
> +	  0x04020202, 0x03020303, 0x05050405, 0x05050405, 0x07080606,
> +	  0x06080606, 0x0a070505, 0x09080807, 0x05090909, 0x0a0b0a07,
> +	  0x09080b09, 0xdbff0909, 0x02014300, 0x02030202, 0x03030503,
> +	  0x07080c05, 0x0c0c0c08, 0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c,
> +	  0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c,
> +	  0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c, 0xff0c0c0c },
> +	{ 0x01010043, 0x01010101, 0x01010101, 0x01010101, 0x01010201,
> +	  0x03010101, 0x02010202, 0x03030303, 0x03030303, 0x04050404,
> +	  0x04050404, 0x06050303, 0x06050505, 0x03060606, 0x07070704,
> +	  0x06050706, 0xdbff0606, 0x01014300, 0x01020101, 0x02020402,
> +	  0x05060904, 0x09090906, 0x09090909, 0x09090909, 0x09090909,
> +	  0x09090909, 0x09090909, 0x09090909, 0x09090909, 0x09090909,
> +	  0x09090909, 0x09090909, 0x09090909, 0xff090909 },
> +	{ 0x01010043, 0x01010101, 0x01010101, 0x01010101, 0x01010101,
> +	  0x01010101, 0x01010101, 0x01010101, 0x01010101, 0x02020202,
> +	  0x02020202, 0x03020101, 0x03020202, 0x01030303, 0x03030302,
> +	  0x03020303, 0xdbff0403, 0x01014300, 0x01010101, 0x01010201,
> +	  0x03040602, 0x06060604, 0x06060606, 0x06060606, 0x06060606,
> +	  0x06060606, 0x06060606, 0x06060606, 0x06060606, 0x06060606,
> +	  0x06060606, 0x06060606, 0x06060606, 0xff060606 }
> +};
> +
> +static void aspeed_video_init_jpeg_table(u32 *table, bool yuv420)
> +{
> +	int i;
> +	unsigned int base;
> +
> +	for (i = 0; i < ASPEED_VIDEO_JPEG_NUM_QUALITIES; i++) {
> +		base = 256 * i;	/* AST HW requires this header spacing */
> +		memcpy(&table[base], aspeed_video_jpeg_header,
> +		       sizeof(aspeed_video_jpeg_header));
> +
> +		base += ASPEED_VIDEO_JPEG_HEADER_SIZE;
> +		memcpy(&table[base], aspeed_video_jpeg_dct[i],
> +		       sizeof(aspeed_video_jpeg_dct[i]));
> +
> +		base += ASPEED_VIDEO_JPEG_DCT_SIZE;
> +		memcpy(&table[base], aspeed_video_jpeg_quant,
> +		       sizeof(aspeed_video_jpeg_quant));
> +
> +		if (yuv420)
> +			table[base + 2] = 0x00220103;
> +	}
> +}
> +
> +static void aspeed_video_update(struct aspeed_video *video, u32 reg, u32 clear,
> +				u32 bits)
> +{
> +	u32 t = readl(video->base + reg);
> +	u32 before = t;
> +
> +	t &= ~clear;
> +	t |= bits;
> +	writel(t, video->base + reg);
> +	dev_dbg(video->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);
> +	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));
> +}
> +
> +static bool aspeed_video_engine_busy(struct aspeed_video *video)
> +{
> +	u32 seq_ctrl = aspeed_video_read(video, VE_SEQ_CTRL);
> +
> +	if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
> +	    !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
> +		dev_err(video->dev, "video engine busy\n");
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static int aspeed_video_start_frame(struct aspeed_video *video)
> +{
> +	dma_addr_t addr;
> +	unsigned long flags;
> +	struct aspeed_video_buffer *buf;
> +
> +	if (aspeed_video_engine_busy(video))
> +		return -EBUSY;
> +
> +	spin_lock_irqsave(&video->lock, flags);
> +	buf = list_first_entry_or_null(&video->buffers,
> +				       struct aspeed_video_buffer, link);
> +	if (!buf) {
> +		spin_unlock_irqrestore(&video->lock, flags);
> +		return -EPROTO;
> +	}
> +
> +	set_bit(VIDEO_FRAME_INPRG, &video->flags);
> +	addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
> +	spin_unlock_irqrestore(&video->lock, flags);
> +
> +	aspeed_video_write(video, VE_COMP_PROC_OFFSET, 0);
> +	aspeed_video_write(video, VE_COMP_OFFSET, 0);
> +	aspeed_video_write(video, VE_COMP_ADDR, addr);
> +
> +	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
> +			    VE_INTERRUPT_COMP_COMPLETE |
> +			    VE_INTERRUPT_CAPTURE_COMPLETE);
> +
> +	aspeed_video_update(video, VE_SEQ_CTRL, 0,
> +			    VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
> +
> +	return 0;
> +}
> +
> +static void aspeed_video_start_mode_detect(struct aspeed_video *video)
> +{
> +	/* Enable mode detect interrupts */
> +	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
> +			    VE_INTERRUPT_MODE_DETECT);
> +
> +	/* Trigger mode detect */
> +	aspeed_video_update(video, VE_SEQ_CTRL, 0, VE_SEQ_CTRL_TRIG_MODE_DET);
> +}
> +
> +static void aspeed_video_disable_mode_detect(struct aspeed_video *video)

Somewhat weird naming for this pair of functions: either use start/stop or
enable/disable (my preference), not start/disable.

> +{
> +	/* Disable mode detect interrupts */
> +	aspeed_video_update(video, VE_INTERRUPT_CTRL,
> +			    VE_INTERRUPT_MODE_DETECT, 0);
> +
> +	/* Disable mode detect */
> +	aspeed_video_update(video, VE_SEQ_CTRL, VE_SEQ_CTRL_TRIG_MODE_DET, 0);
> +}
> +
> +static void aspeed_video_off(struct aspeed_video *video)
> +{
> +	/* Reset the engine */
> +	reset_control_assert(video->rst);
> +	udelay(100);
> +	reset_control_deassert(video->rst);
> +
> +	/* Turn off the relevant clocks */
> +	clk_disable_unprepare(video->vclk);
> +	clk_disable_unprepare(video->eclk);
> +}
> +
> +static void aspeed_video_on(struct aspeed_video *video)
> +{
> +	/* Turn on the relevant clocks */
> +	clk_prepare_enable(video->eclk);
> +	clk_prepare_enable(video->vclk);
> +
> +	/* Reset the engine */
> +	reset_control_assert(video->rst);
> +	udelay(100);
> +	reset_control_deassert(video->rst);
> +}
> +
> +static void aspeed_video_bufs_done(struct aspeed_video *video,
> +				   enum vb2_buffer_state state)
> +{
> +	unsigned long flags;
> +	struct aspeed_video_buffer *buf;
> +
> +	spin_lock_irqsave(&video->lock, flags);
> +	list_for_each_entry(buf, &video->buffers, link) {
> +		if (list_is_last(&buf->link, &video->buffers))
> +			buf->vb.flags |= V4L2_BUF_FLAG_LAST;

You don't need to use this flag. It's meant for hardware codecs,
not for a regular video capture driver.

> +		vb2_buffer_done(&buf->vb.vb2_buf, state);
> +	}
> +	INIT_LIST_HEAD(&video->buffers);
> +	spin_unlock_irqrestore(&video->lock, flags);
> +}
> +
> +static irqreturn_t aspeed_video_irq(int irq, void *arg)
> +{
> +	struct aspeed_video *video = arg;
> +	u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS);
> +
> +	if (atomic_read(&video->clients) == 0) {
> +		dev_info(video->dev, "irq with no client; disabling irqs\n");
> +
> +		aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
> +		aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
> +		return IRQ_HANDLED;
> +	}
> +
> +	/* Resolution changed; reset entire engine and reinitialize */
> +	if (sts & VE_INTERRUPT_MODE_DETECT_WD) {
> +		dev_info(video->dev, "resolution changed; resetting\n");
> +		set_bit(VIDEO_RES_CHANGE, &video->flags);
> +		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
> +		clear_bit(VIDEO_STREAMING, &video->flags);
> +
> +		aspeed_video_off(video);
> +		aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
> +
> +		schedule_delayed_work(&video->res_work,
> +				      RESOLUTION_CHANGE_DELAY);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (sts & VE_INTERRUPT_MODE_DETECT) {
> +		aspeed_video_update(video, VE_INTERRUPT_CTRL,
> +				    VE_INTERRUPT_MODE_DETECT, 0);
> +		aspeed_video_write(video, VE_INTERRUPT_STATUS,
> +				   VE_INTERRUPT_MODE_DETECT);
> +
> +		set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
> +		wake_up_interruptible_all(&video->wait);
> +	}
> +
> +	if ((sts & VE_INTERRUPT_COMP_COMPLETE) &&
> +	    (sts & VE_INTERRUPT_CAPTURE_COMPLETE)) {
> +		struct aspeed_video_buffer *buf;
> +		u32 frame_size = aspeed_video_read(video,
> +						   VE_OFFSET_COMP_STREAM);
> +
> +		spin_lock(&video->lock);
> +		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
> +		buf = list_first_entry_or_null(&video->buffers,
> +					       struct aspeed_video_buffer,
> +					       link);
> +		if (buf) {
> +			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size);
> +
> +			if (!list_is_last(&buf->link, &video->buffers)) {
> +				buf->vb.vb2_buf.timestamp = ktime_get_ns();
> +				buf->vb.sequence = video->sequence++;
> +				buf->vb.field = V4L2_FIELD_NONE;
> +				vb2_buffer_done(&buf->vb.vb2_buf,
> +						VB2_BUF_STATE_DONE);
> +				list_del(&buf->link);
> +			}
> +		}
> +		spin_unlock(&video->lock);
> +
> +		aspeed_video_update(video, VE_SEQ_CTRL,
> +				    VE_SEQ_CTRL_TRIG_CAPTURE |
> +				    VE_SEQ_CTRL_FORCE_IDLE |
> +				    VE_SEQ_CTRL_TRIG_COMP, 0);
> +		aspeed_video_update(video, VE_INTERRUPT_CTRL,
> +				    VE_INTERRUPT_COMP_COMPLETE |
> +				    VE_INTERRUPT_CAPTURE_COMPLETE, 0);
> +		aspeed_video_write(video, VE_INTERRUPT_STATUS,
> +				   VE_INTERRUPT_COMP_COMPLETE |
> +				   VE_INTERRUPT_CAPTURE_COMPLETE);
> +
> +		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
> +			aspeed_video_start_frame(video);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void aspeed_video_check_polarity(struct aspeed_video *video)

This function should be moved to just before the get_resolution function,
since that's the one that calls this.

> +{
> +	int i;
> +	int hsync_counter = 0;
> +	int vsync_counter = 0;
> +	u32 sts;
> +
> +	for (i = 0; i < NUM_POLARITY_CHECKS; ++i) {
> +		sts = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
> +		if (sts & VE_MODE_DETECT_STATUS_VSYNC)
> +			vsync_counter--;
> +		else
> +			vsync_counter++;
> +
> +		if (sts & VE_MODE_DETECT_STATUS_HSYNC)
> +			hsync_counter--;
> +		else
> +			hsync_counter++;
> +	}
> +
> +	if (hsync_counter < 0 || vsync_counter < 0) {
> +		u32 ctrl;
> +
> +		if (hsync_counter < 0)
> +			ctrl = VE_CTRL_HSYNC_POL;
> +		else
> +			video->timings.polarities |= V4L2_DV_HSYNC_POS_POL;

You should set this for video->detected_timings.
You also don't clear the polarities flag if the polarity is negative.

> +
> +		if (vsync_counter < 0)
> +			ctrl = VE_CTRL_VSYNC_POL;
> +		else
> +			video->timings.polarities |= V4L2_DV_VSYNC_POS_POL;
> +
> +		aspeed_video_update(video, VE_CTRL, 0, ctrl);

It would make more sense to write to this register either when you
start streaming or when s_dv_timings is called. That's when you need it,
not when detecting a new timing.

> +	}
> +}
> +
> +static bool aspeed_video_alloc_buf(struct aspeed_video *video,
> +				   struct aspeed_video_addr *addr,
> +				   unsigned int size)
> +{
> +	addr->virt = dma_alloc_coherent(video->dev, size, &addr->dma,
> +					GFP_KERNEL);
> +	if (!addr->virt)
> +		return false;
> +
> +	addr->size = size;
> +	return true;
> +}
> +
> +static void aspeed_video_free_buf(struct aspeed_video *video,
> +				  struct aspeed_video_addr *addr)
> +{
> +	dma_free_coherent(video->dev, addr->size, addr->virt, addr->dma);
> +	addr->size = 0;
> +	addr->dma = 0ULL;
> +	addr->virt = NULL;
> +}
> +
> +/*
> + * Get the minimum HW-supported compression buffer size for the frame size.
> + * Assume worst-case JPEG compression size is 1/8 raw size. This should be
> + * plenty even for maximum quality; any worse and the engine will simply return
> + * incomplete JPEGs.
> + */
> +static void aspeed_video_calc_compressed_size(struct aspeed_video *video)
> +{
> +	int i, j;
> +	u32 compression_buffer_size_reg = 0;
> +	unsigned int size;
> +	const unsigned int num_compression_packets = 4;
> +	const unsigned int compression_packet_size = 1024;
> +	const unsigned int max_compressed_size =
> +		video->capture_width * video->capture_height / 2;
> +
> +	video->max_compressed_size = UINT_MAX;
> +
> +	for (i = 0; i < 6; ++i) {
> +		for (j = 0; j < 8; ++j) {
> +			size = (num_compression_packets << i) *
> +				(compression_packet_size << j);
> +			if (size < max_compressed_size)
> +				continue;
> +
> +			if (size < video->max_compressed_size) {
> +				compression_buffer_size_reg = (i << 3) | j;
> +				video->max_compressed_size = size;
> +			}
> +		}
> +	}
> +
> +	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);
> +}
> +
> +#define res_check(v) test_and_clear_bit(VIDEO_MODE_DETECT_DONE, &(v)->flags)
> +
> +static int aspeed_video_get_resolution(struct aspeed_video *video)
> +{
> +	bool invalid_resolution = true;
> +	int rc;
> +	int tries = 0;
> +	unsigned int bottom;
> +	unsigned int left;
> +	unsigned int right;
> +	unsigned int top;
> +	u32 mds;
> +	u32 src_lr_edge;
> +	u32 src_tb_edge;
> +	u32 sync;
> +

If I understand the code correctly, when this is called we're not supposed to
be streaming video, right?

> +	if (video->srcs[1].size)
> +		aspeed_video_free_buf(video, &video->srcs[1]);
> +
> +	if (!video->detect.size) {
> +		if (video->srcs[0].size >= VE_MAX_SRC_BUFFER_SIZE) {
> +			video->detect = video->srcs[0];
> +
> +			video->srcs[0].size = 0;
> +			video->srcs[0].dma = 0ULL;
> +			video->srcs[0].virt = NULL;
> +		} else {
> +			if (video->srcs[0].size)
> +				aspeed_video_free_buf(video, &video->srcs[0]);
> +
> +			if (!aspeed_video_alloc_buf(video, &video->detect,
> +						    VE_MAX_SRC_BUFFER_SIZE)) {
> +				dev_err(video->dev,
> +					"failed to allocate source buffers\n");
> +				return -ENOMEM;
> +			}
> +		}
> +	}
> +
> +	aspeed_video_write(video, VE_SRC0_ADDR, video->detect.dma);
> +
> +	video->timings.width = 0;
> +	video->timings.height = 0;

detected_timings

> +
> +	do {
> +		if (tries) {
> +			set_current_state(TASK_INTERRUPTIBLE);
> +			if (schedule_timeout(INVALID_RESOLUTION_DELAY))
> +				return -EINTR;
> +		}
> +
> +		aspeed_video_start_mode_detect(video);
> +
> +		rc = wait_event_interruptible_timeout(video->wait,
> +						      res_check(video),
> +						      MODE_DETECT_TIMEOUT);
> +		if (!rc) {
> +			dev_err(video->dev, "timed out on 1st mode detect\n");
> +			aspeed_video_disable_mode_detect(video);
> +			return -ETIMEDOUT;
> +		}
> +
> +		/* Disable mode detect in order to re-trigger */
> +		aspeed_video_update(video, VE_SEQ_CTRL,
> +				    VE_SEQ_CTRL_TRIG_MODE_DET, 0);
> +
> +		aspeed_video_check_polarity(video);
> +
> +		aspeed_video_start_mode_detect(video);

Just curious: why is check_polarity done before starting the mode_detect?
I would expect it to be the other way around.

As mentioned above, aspeed_video_check_polarity doesn't just check the polarity,
it also seems to set it. If this is required before you can do a mode detect,
then that should be documented and the function should be renamed to
check_and_set_polarity.

> +
> +		rc = wait_event_interruptible_timeout(video->wait,
> +						      res_check(video),
> +						      MODE_DETECT_TIMEOUT);
> +		if (!rc) {
> +			dev_err(video->dev, "timed out on 2nd mode detect\n");
> +			aspeed_video_disable_mode_detect(video);
> +			return -ETIMEDOUT;
> +		}
> +
> +		src_lr_edge = aspeed_video_read(video, VE_SRC_LR_EDGE_DET);
> +		src_tb_edge = aspeed_video_read(video, VE_SRC_TB_EDGE_DET);
> +		mds = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
> +		sync = aspeed_video_read(video, VE_SYNC_STATUS);
> +
> +		bottom = (src_tb_edge & VE_SRC_TB_EDGE_DET_BOT) >>
> +			VE_SRC_TB_EDGE_DET_BOT_SHF;
> +		top = src_tb_edge & VE_SRC_TB_EDGE_DET_TOP;
> +		video->timings.vfrontporch = top;
> +		video->timings.vbackporch = ((mds & VE_MODE_DETECT_V_LINES) >>
> +			VE_MODE_DETECT_V_LINES_SHF) - bottom;
> +		video->timings.vsync = (sync & VE_SYNC_STATUS_VSYNC) >>
> +			VE_SYNC_STATUS_VSYNC_SHF;
> +		if (top > bottom)
> +			continue;
> +
> +		right = (src_lr_edge & VE_SRC_LR_EDGE_DET_RT) >>
> +			VE_SRC_LR_EDGE_DET_RT_SHF;
> +		left = src_lr_edge & VE_SRC_LR_EDGE_DET_LEFT;
> +		video->timings.hfrontporch = left;
> +		video->timings.hbackporch = (mds & VE_MODE_DETECT_H_PIXELS) -
> +			right;
> +		video->timings.hsync = sync & VE_SYNC_STATUS_HSYNC;
> +		if (left > right)
> +			continue;
> +
> +		invalid_resolution = false;
> +	} while (invalid_resolution && (tries++ < INVALID_RESOLUTION_RETRIES));
> +
> +	if (invalid_resolution) {
> +		dev_err(video->dev, "invalid resolution detected\n");
> +		return -ERANGE;
> +	}
> +
> +	video->timings.height = (bottom - top) + 1;
> +	video->timings.width = (right - left) + 1;
> +
> +	/* Don't use direct mode below 1024 x 768 (irqs don't fire) */
> +	if (video->timings.width * video->timings.height < DIRECT_FETCH_THRESHOLD) {
> +		aspeed_video_write(video, VE_TGS_0,
> +				   FIELD_PREP(VE_TGS_FIRST, left - 1) |
> +				   FIELD_PREP(VE_TGS_LAST, right));
> +		aspeed_video_write(video, VE_TGS_1,
> +				   FIELD_PREP(VE_TGS_FIRST, top) |
> +				   FIELD_PREP(VE_TGS_LAST, bottom + 1));
> +		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
> +	} else {
> +		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
> +	}

Shouldn't this be done in set_capture_resolution?

> +
> +	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
> +			    VE_INTERRUPT_MODE_DETECT_WD);
> +	aspeed_video_update(video, VE_SEQ_CTRL, 0,
> +			    VE_SEQ_CTRL_AUTO_COMP | VE_SEQ_CTRL_EN_WATCHDOG);

What do these two lines do? Perhaps a comment would help.

> +
> +	dev_dbg(video->dev, "got resolution[%dx%d]\n", video->timings.width,
> +		video->timings.height);
> +
> +	return 0;
> +
> +}
> +
> +static int aspeed_video_set_capture_resolution(struct aspeed_video *video)
> +{
> +	unsigned int size = video->timings.width * video->timings.height;

active_timings

> +	struct aspeed_video_addr detect = video->detect;
> +
> +	video->detect.size = 0;
> +	video->detect.dma = 0ULL;
> +	video->detect.virt = NULL;
> +
> +	video->capture_width = video->timings.width;
> +	video->capture_height = video->timings.height;
> +
> +	aspeed_video_write(video, VE_CAP_WINDOW,
> +			   video->capture_width << 16 | video->capture_height);
> +	aspeed_video_write(video, VE_COMP_WINDOW,
> +			   video->capture_width << 16 | video->capture_height);
> +	aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET,
> +			   video->capture_width * 4);
> +
> +	size *= 4;
> +	if (size == detect.size / 2) {
> +		aspeed_video_write(video, VE_SRC1_ADDR, detect.dma + size);
> +		video->srcs[0] = detect;
> +	} else if (size == detect.size) {
> +		video->srcs[0] = detect;
> +
> +		if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
> +			goto err_mem;
> +
> +		aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
> +	} else {
> +		aspeed_video_free_buf(video, &detect);
> +
> +		if (!aspeed_video_alloc_buf(video, &video->srcs[0], size))
> +			goto err_mem;
> +
> +		if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
> +			goto err_mem;
> +
> +		aspeed_video_write(video, VE_SRC0_ADDR, video->srcs[0].dma);
> +		aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
> +	}
> +
> +	aspeed_video_calc_compressed_size(video);
> +
> +	dev_dbg(video->dev, "set resolution[%dx%d]\n", video->capture_width,
> +		video->capture_height);
> +
> +	return 0;
> +
> +err_mem:
> +	dev_err(video->dev, "failed to allocate source buffers\n");
> +
> +	if (video->srcs[0].size)
> +		aspeed_video_free_buf(video, &video->srcs[0]);
> +
> +	return -ENOMEM;
> +}
> +
> +static void aspeed_video_init_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 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
> +
> +	if (video->frame_rate)
> +		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
> +
> +	if (video->yuv420)
> +		seq_ctrl |= VE_SEQ_CTRL_YUV420;
> +
> +	/* Unlock VE registers */
> +	aspeed_video_write(video, VE_PROTECTION_KEY, VE_PROTECTION_KEY_UNLOCK);
> +
> +	/* Disable interrupts */
> +	aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
> +	aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
> +
> +	/* Clear the offset */
> +	aspeed_video_write(video, VE_COMP_PROC_OFFSET, 0);
> +	aspeed_video_write(video, VE_COMP_OFFSET, 0);
> +
> +	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);
> +
> +	/* Don't downscale */
> +	aspeed_video_write(video, VE_SCALING_FACTOR, 0x10001000);
> +	aspeed_video_write(video, VE_SCALING_FILTER0, 0x00200000);
> +	aspeed_video_write(video, VE_SCALING_FILTER1, 0x00200000);
> +	aspeed_video_write(video, VE_SCALING_FILTER2, 0x00200000);
> +	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
> +
> +	/* Set mode detection defaults */
> +	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
> +}
> +
> +static int aspeed_video_start(struct aspeed_video *video)
> +{
> +	int rc;
> +
> +	aspeed_video_on(video);
> +
> +	aspeed_video_init_regs(video);
> +
> +	rc = aspeed_video_get_resolution(video);

No, this shouldn't be called here. Only query_dv_timings should call this.

> +	if (rc)
> +		return rc;
> +
> +	rc = aspeed_video_set_capture_resolution(video);
> +	if (rc)
> +		return rc;
> +
> +	video->pix_fmt.width = video->timings.width;
> +	video->pix_fmt.height = video->timings.height;
> +	video->pix_fmt.sizeimage = video->max_compressed_size;

You don't set that here: otherwise opening the device node would magically
change the format. That's not what should happen.

The golden rule is that you never change timings and/or format unless
explicitly commanded by the application through S_DV_TIMINGS and S_FMT.

> +
> +	return 0;
> +}
> +
> +static void aspeed_video_stop(struct aspeed_video *video)
> +{
> +	cancel_delayed_work_sync(&video->res_work);
> +
> +	aspeed_video_off(video);
> +
> +	if (video->srcs[0].size)
> +		aspeed_video_free_buf(video, &video->srcs[0]);
> +
> +	if (video->srcs[1].size)
> +		aspeed_video_free_buf(video, &video->srcs[1]);
> +
> +	if (video->detect.size)
> +		aspeed_video_free_buf(video, &video->detect);
> +
> +	video->flags = 0;
> +}
> +
> +static int aspeed_video_querycap(struct file *file, void *fh,
> +				 struct v4l2_capability *cap)
> +{
> +	strscpy(cap->driver, DEVICE_NAME, sizeof(cap->driver));
> +	strscpy(cap->card, "Aspeed Video Engine", sizeof(cap->card));
> +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> +		 DEVICE_NAME);
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_enum_format(struct file *file, void *fh,
> +				    struct v4l2_fmtdesc *f)
> +{
> +	if (f->index)
> +		return -EINVAL;
> +
> +	f->pixelformat = V4L2_PIX_FMT_JPEG;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_get_format(struct file *file, void *fh,
> +				   struct v4l2_format *f)
> +{
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	f->fmt.pix = video->pix_fmt;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_enum_input(struct file *file, void *fh,
> +				   struct v4l2_input *inp)
> +{
> +	if (inp->index)
> +		return -EINVAL;
> +
> +	strscpy(inp->name, "Host VGA capture", sizeof(inp->name));
> +	inp->type = V4L2_INPUT_TYPE_CAMERA;
> +	inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
> +	inp->status = 0;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_get_input(struct file *file, void *fh, unsigned int *i)
> +{
> +	*i = 0;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_set_input(struct file *file, void *fh, unsigned int i)
> +{
> +	if (i)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_get_parm(struct file *file, void *fh,
> +				 struct v4l2_streamparm *a)
> +{
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> +	a->parm.capture.readbuffers = 3;
> +	a->parm.capture.timeperframe.numerator = 1;
> +	if (!video->frame_rate)
> +		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
> +	else
> +		a->parm.capture.timeperframe.denominator = video->frame_rate;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_set_parm(struct file *file, void *fh,
> +				 struct v4l2_streamparm *a)
> +{
> +	unsigned int frame_rate = 0;
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> +	a->parm.capture.readbuffers = 3;
> +
> +	if (a->parm.capture.timeperframe.numerator)
> +		frame_rate = a->parm.capture.timeperframe.denominator /
> +			a->parm.capture.timeperframe.numerator;
> +
> +	if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
> +		frame_rate = 0;
> +		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
> +		a->parm.capture.timeperframe.numerator = 1;
> +	}
> +
> +	if (video->frame_rate != frame_rate) {
> +		video->frame_rate = frame_rate;
> +		aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC,
> +				    FIELD_PREP(VE_CTRL_FRC, frame_rate));
> +	}
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_enum_framesizes(struct file *file, void *fh,
> +					struct v4l2_frmsizeenum *fsize)
> +{
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	if (fsize->index)
> +		return -EINVAL;
> +
> +	if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
> +		return -EINVAL;
> +
> +	fsize->discrete.width = video->pix_fmt.width;
> +	fsize->discrete.height = video->pix_fmt.height;
> +	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_enum_frameintervals(struct file *file, void *fh,
> +					    struct v4l2_frmivalenum *fival)
> +{
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	if (fival->index)
> +		return -EINVAL;
> +
> +	if (fival->width != video->timings.width ||
> +	    fival->height != video->timings.height)
> +		return -EINVAL;
> +
> +	if (fival->pixel_format != V4L2_PIX_FMT_JPEG)
> +		return -EINVAL;
> +
> +	fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
> +
> +	fival->stepwise.min.denominator = MAX_FRAME_RATE;
> +	fival->stepwise.min.numerator = 1;
> +	fival->stepwise.max.denominator = 1;
> +	fival->stepwise.max.numerator = 1;
> +	fival->stepwise.step = fival->stepwise.max;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_set_dv_timings(struct file *file, void *fh,
> +				       struct v4l2_dv_timings *timings)
> +{
> +	int rc;
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	if (video->capture_width == timings->bt.width &&
> +	    video->capture_height == timings->bt.height)

Unless I am mistaken, capture_width/height just duplicates
video->pix_fmt.width/height. I'd drop capture_width/height and only
use pix_fmt.

> +		return 0;
> +
> +	if (vb2_is_busy(&video->queue))
> +		return -EBUSY;
> +
> +	if (video->timings.width != timings->bt.width ||
> +	    video->timings.height != timings->bt.height)
> +		return -EINVAL;

This 'if' makes no sense. It is perfectly fine (if silly) to set
timings that do not match the current resolution. The timings can
change at any time anyway, so there is no point to check this.

> +
> +	rc = aspeed_video_set_capture_resolution(video);
> +	if (rc)
> +		return rc;
> +
> +	video->pix_fmt.width = timings->bt.width;
> +	video->pix_fmt.height = timings->bt.height;
> +	video->pix_fmt.sizeimage = video->max_compressed_size;
> +
> +	timings->type = V4L2_DV_BT_656_1120;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_get_dv_timings(struct file *file, void *fh,
> +				       struct v4l2_dv_timings *timings)
> +{
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	timings->type = V4L2_DV_BT_656_1120;
> +	timings->bt = video->timings;

Should return active_timings.

> +
> +	return 0;
> +}
> +
> +static int aspeed_video_query_dv_timings(struct file *file, void *fh,
> +					 struct v4l2_dv_timings *timings)
> +{
> +	int rc;
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	if (file->f_flags & O_NONBLOCK) {
> +		if (test_bit(VIDEO_RES_CHANGE, &video->flags))
> +			return -EAGAIN;
> +	} else {
> +		rc = wait_event_interruptible(video->wait,
> +					      !test_bit(VIDEO_RES_CHANGE,
> +							&video->flags));
> +		if (rc)
> +			return -EINTR;
> +	}
> +
> +	timings->type = V4L2_DV_BT_656_1120;
> +	timings->bt = video->timings;
> +	timings->bt.width = video->timings.width;
> +	timings->bt.height = video->timings.height;

Should return the detected_timings.

Why assign bt.width/height again? Those two lines can be dropped.

> +
> +	return 0;
> +}
> +
> +static int aspeed_video_enum_dv_timings(struct file *file, void *fh,
> +					struct v4l2_enum_dv_timings *timings)
> +{
> +	if (timings->index)
> +		return -EINVAL;
> +
> +	return aspeed_video_get_dv_timings(file, fh, &timings->timings);
> +}
> +
> +static int aspeed_video_dv_timings_cap(struct file *file, void *fh,
> +				       struct v4l2_dv_timings_cap *cap)
> +{
> +	cap->type = V4L2_DV_BT_656_1120;
> +	cap->bt.capabilities = V4L2_DV_BT_CAP_PROGRESSIVE;
> +	cap->bt.min_width = MIN_WIDTH;
> +	cap->bt.max_width = MAX_WIDTH;
> +	cap->bt.min_height = MIN_HEIGHT;
> +	cap->bt.max_height = MAX_HEIGHT;

I would create a static const struct v4l2_dv_timings_cap at the top of the source
and return that here.

You can then use the helpers v4l2_valid_dv_timings in s_dv_timings and
v4l2_enum_dv_timings_cap in enum_dv_timings.

You need to set the V4L2_DV_BT_CAP_REDUCED_BLANKING capability as well.
And fill in cap->bt.min/max_pixelclock and standards (CEA861|DMT|CVT|GTF).

> +
> +	return 0;
> +}
> +
> +static int aspeed_video_sub_event(struct v4l2_fh *fh,
> +				  const struct v4l2_event_subscription *sub)
> +{
> +	switch (sub->type) {
> +	case V4L2_EVENT_SOURCE_CHANGE:
> +		return v4l2_src_change_event_subscribe(fh, sub);
> +	}
> +
> +	return v4l2_ctrl_subscribe_event(fh, sub);
> +}
> +
> +static const struct v4l2_ioctl_ops aspeed_video_ioctl_ops = {
> +	.vidioc_querycap = aspeed_video_querycap,
> +
> +	.vidioc_enum_fmt_vid_cap = aspeed_video_enum_format,
> +	.vidioc_g_fmt_vid_cap = aspeed_video_get_format,
> +	.vidioc_s_fmt_vid_cap = aspeed_video_get_format,
> +	.vidioc_try_fmt_vid_cap = aspeed_video_get_format,
> +
> +	.vidioc_reqbufs = vb2_ioctl_reqbufs,
> +	.vidioc_querybuf = vb2_ioctl_querybuf,
> +	.vidioc_qbuf = vb2_ioctl_qbuf,
> +	.vidioc_dqbuf = vb2_ioctl_dqbuf,
> +	.vidioc_create_bufs = vb2_ioctl_create_bufs,
> +	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> +	.vidioc_streamon = vb2_ioctl_streamon,
> +	.vidioc_streamoff = vb2_ioctl_streamoff,
> +
> +	.vidioc_enum_input = aspeed_video_enum_input,
> +	.vidioc_g_input = aspeed_video_get_input,
> +	.vidioc_s_input = aspeed_video_set_input,
> +
> +	.vidioc_g_parm = aspeed_video_get_parm,
> +	.vidioc_s_parm = aspeed_video_set_parm,
> +	.vidioc_enum_framesizes = aspeed_video_enum_framesizes,
> +	.vidioc_enum_frameintervals = aspeed_video_enum_frameintervals,
> +
> +	.vidioc_s_dv_timings = aspeed_video_set_dv_timings,
> +	.vidioc_g_dv_timings = aspeed_video_get_dv_timings,
> +	.vidioc_query_dv_timings = aspeed_video_query_dv_timings,
> +	.vidioc_enum_dv_timings = aspeed_video_enum_dv_timings,
> +	.vidioc_dv_timings_cap = aspeed_video_dv_timings_cap,
> +
> +	.vidioc_subscribe_event = aspeed_video_sub_event,
> +	.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_init_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,
> +						  struct aspeed_video,
> +						  ctrl_handler);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> +		video->jpeg_quality = ctrl->val;
> +		aspeed_video_update_jpeg_quality(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);
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
> +	.s_ctrl = aspeed_video_set_ctrl,
> +};
> +
> +static void aspeed_video_resolution_work(struct work_struct *work)
> +{
> +	int rc;
> +	struct delayed_work *dwork = to_delayed_work(work);
> +	struct aspeed_video *video = container_of(dwork, struct aspeed_video,
> +						  res_work);
> +
> +	/* No clients remaining after delay */
> +	if (atomic_read(&video->clients) == 0)
> +		goto done;
> +
> +	aspeed_video_on(video);
> +
> +	aspeed_video_init_regs(video);
> +
> +	rc = aspeed_video_get_resolution(video);
> +	if (rc)
> +		dev_err(video->dev,
> +			"resolution changed; couldn't get new resolution\n");
> +
> +	if (video->timings.width != video->pix_fmt.width ||
> +	    video->timings.height != video->pix_fmt.height) {
> +		static const struct v4l2_event ev = {
> +			.type = V4L2_EVENT_SOURCE_CHANGE,
> +			.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> +		};
> +
> +		v4l2_event_queue(&video->vdev, &ev);
> +	}
> +
> +done:
> +	clear_bit(VIDEO_RES_CHANGE, &video->flags);
> +	wake_up_interruptible_all(&video->wait);
> +}
> +
> +static int aspeed_video_open(struct file *file)
> +{
> +	int rc;
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	mutex_lock(&video->video_lock);
> +
> +	if (atomic_inc_return(&video->clients) == 1) {

No need for a clients refcount. You can use the v4l2_fh_is_singular(_file) helpers.
See how other drivers do this.

> +		rc = aspeed_video_start(video);
> +		if (rc) {
> +			dev_err(video->dev, "Failed to start video engine\n");
> +			atomic_dec(&video->clients);
> +			mutex_unlock(&video->video_lock);
> +			return rc;
> +		}
> +	}
> +
> +	mutex_unlock(&video->video_lock);
> +
> +	return v4l2_fh_open(file);
> +}
> +
> +static int aspeed_video_release(struct file *file)
> +{
> +	int rc;
> +	struct aspeed_video *video = video_drvdata(file);
> +
> +	rc = vb2_fop_release(file);
> +
> +	mutex_lock(&video->video_lock);
> +
> +	if (atomic_dec_return(&video->clients) == 0)
> +		aspeed_video_stop(video);
> +
> +	mutex_unlock(&video->video_lock);
> +
> +	return rc;
> +}
> +
> +static const struct v4l2_file_operations aspeed_video_v4l2_fops = {
> +	.owner = THIS_MODULE,
> +	.read = vb2_fop_read,
> +	.poll = vb2_fop_poll,
> +	.unlocked_ioctl = video_ioctl2,
> +	.mmap = vb2_fop_mmap,
> +	.open = aspeed_video_open,
> +	.release = aspeed_video_release,
> +};
> +
> +static int aspeed_video_queue_setup(struct vb2_queue *q,
> +				    unsigned int *num_buffers,
> +				    unsigned int *num_planes,
> +				    unsigned int sizes[],
> +				    struct device *alloc_devs[])
> +{
> +	struct aspeed_video *video = vb2_get_drv_priv(q);
> +
> +	if (*num_planes) {
> +		if (sizes[0] < video->max_compressed_size)
> +			return -EINVAL;
> +
> +		return 0;
> +	}
> +
> +	*num_planes = 1;
> +	sizes[0] = video->max_compressed_size;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_buf_prepare(struct vb2_buffer *vb)
> +{
> +	struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +
> +	if (vb2_plane_size(vb, 0) < video->max_compressed_size)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_start_streaming(struct vb2_queue *q,
> +					unsigned int count)
> +{
> +	int rc;
> +	struct aspeed_video *video = vb2_get_drv_priv(q);
> +
> +	rc = aspeed_video_start_frame(video);
> +	if (rc) {
> +		aspeed_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
> +		return rc;
> +	}
> +
> +	video->sequence = 0;
> +	set_bit(VIDEO_STREAMING, &video->flags);
> +	return 0;
> +}
> +
> +static void aspeed_video_stop_streaming(struct vb2_queue *q)
> +{
> +	int rc;
> +	struct aspeed_video *video = vb2_get_drv_priv(q);
> +
> +	clear_bit(VIDEO_STREAMING, &video->flags);
> +
> +	rc = wait_event_timeout(video->wait,
> +				!test_bit(VIDEO_FRAME_INPRG, &video->flags),
> +				STOP_TIMEOUT);
> +	if (!rc) {
> +		dev_err(video->dev, "Timed out when stopping streaming\n");
> +		aspeed_video_stop(video);
> +	}
> +
> +	aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
> +}
> +
> +static void aspeed_video_buf_queue(struct vb2_buffer *vb)
> +{
> +	struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct aspeed_video_buffer *avb = to_aspeed_video_buffer(vbuf);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&video->lock, flags);
> +	list_add_tail(&avb->link, &video->buffers);
> +	spin_unlock_irqrestore(&video->lock, flags);
> +}
> +
> +static const struct vb2_ops aspeed_video_vb2_ops = {
> +	.queue_setup = aspeed_video_queue_setup,
> +	.wait_prepare = vb2_ops_wait_prepare,
> +	.wait_finish = vb2_ops_wait_finish,
> +	.buf_prepare = aspeed_video_buf_prepare,
> +	.start_streaming = aspeed_video_start_streaming,
> +	.stop_streaming = aspeed_video_stop_streaming,
> +	.buf_queue =  aspeed_video_buf_queue,
> +};
> +
> +static int aspeed_video_setup_video(struct aspeed_video *video)
> +{
> +	const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
> +			   BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_420));
> +	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
> +	struct vb2_queue *vbq = &video->queue;
> +	struct video_device *vdev = &video->vdev;
> +	int rc;
> +
> +	video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
> +	video->pix_fmt.field = V4L2_FIELD_NONE;
> +	video->pix_fmt.colorspace = V4L2_COLORSPACE_SRGB;
> +	video->pix_fmt.quantization = V4L2_QUANTIZATION_FULL_RANGE;
> +
> +	rc = v4l2_device_register(video->dev, v4l2_dev);
> +	if (rc) {
> +		dev_err(video->dev, "Failed to register v4l2 device\n");
> +		return rc;
> +	}
> +
> +	v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
> +	v4l2_ctrl_new_std(&video->ctrl_handler, &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_CID_JPEG_CHROMA_SUBSAMPLING,
> +			       V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
> +			       V4L2_JPEG_CHROMA_SUBSAMPLING_444);
> +
> +	if (video->ctrl_handler.error) {
> +		v4l2_ctrl_handler_free(&video->ctrl_handler);
> +		v4l2_device_unregister(v4l2_dev);
> +
> +		dev_err(video->dev, "Failed to init controls: %d\n",
> +			video->ctrl_handler.error);
> +		return rc;
> +	}
> +
> +	v4l2_dev->ctrl_handler = &video->ctrl_handler;
> +
> +	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
> +	vbq->dev = v4l2_dev->dev;
> +	vbq->lock = &video->video_lock;
> +	vbq->ops = &aspeed_video_vb2_ops;
> +	vbq->mem_ops = &vb2_dma_contig_memops;
> +	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;
> +
> +	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;
> +	}
> +
> +	vdev->queue = vbq;
> +	vdev->fops = &aspeed_video_v4l2_fops;
> +	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
> +		V4L2_CAP_STREAMING;
> +	vdev->v4l2_dev = v4l2_dev;
> +	strscpy(vdev->name, DEVICE_NAME, sizeof(vdev->name));
> +	vdev->vfl_type = VFL_TYPE_GRABBER;
> +	vdev->vfl_dir = VFL_DIR_RX;
> +	vdev->release = video_device_release_empty;
> +	vdev->ioctl_ops = &aspeed_video_ioctl_ops;
> +	vdev->lock = &video->video_lock;
> +
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_init(struct aspeed_video *video)
> +{
> +	int irq;
> +	int rc;
> +	struct device *dev = video->dev;
> +
> +	irq = irq_of_parse_and_map(dev->of_node, 0);
> +	if (!irq) {
> +		dev_err(dev, "Unable to find IRQ\n");
> +		return -ENODEV;
> +	}
> +
> +	rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED,
> +			      DEVICE_NAME, video);
> +	if (rc < 0) {
> +		dev_err(dev, "Unable to request IRQ %d\n", irq);
> +		return rc;
> +	}
> +
> +	video->eclk = devm_clk_get(dev, "eclk");
> +	if (IS_ERR(video->eclk)) {
> +		dev_err(dev, "Unable to get ECLK\n");
> +		return PTR_ERR(video->eclk);
> +	}
> +
> +	video->vclk = devm_clk_get(dev, "vclk");
> +	if (IS_ERR(video->vclk)) {
> +		dev_err(dev, "Unable to get VCLK\n");
> +		return PTR_ERR(video->vclk);
> +	}
> +
> +	video->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	if (IS_ERR(video->rst)) {
> +		dev_err(dev, "Unable to get VE reset\n");
> +		return PTR_ERR(video->rst);
> +	}
> +
> +	rc = of_reserved_mem_device_init(dev);
> +	if (rc) {
> +		dev_err(dev, "Unable to reserve memory\n");
> +		return rc;
> +	}
> +
> +	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +	if (rc) {
> +		dev_err(dev, "Failed to set DMA mask\n");
> +		of_reserved_mem_device_release(dev);
> +		return rc;
> +	}
> +
> +	if (!aspeed_video_alloc_buf(video, &video->jpeg,
> +				    VE_JPEG_HEADER_SIZE)) {
> +		dev_err(dev, "Failed to allocate DMA for JPEG header\n");
> +		of_reserved_mem_device_release(dev);
> +		return rc;
> +	}
> +
> +	aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_probe(struct platform_device *pdev)
> +{
> +	int rc;
> +	struct resource *res;
> +	struct aspeed_video *video = kzalloc(sizeof(*video), GFP_KERNEL);
> +
> +	if (!video)
> +		return -ENOMEM;
> +
> +	video->frame_rate = 30;
> +	video->dev = &pdev->dev;
> +	mutex_init(&video->video_lock);
> +	init_waitqueue_head(&video->wait);
> +	INIT_DELAYED_WORK(&video->res_work, aspeed_video_resolution_work);
> +	INIT_LIST_HEAD(&video->buffers);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	video->base = devm_ioremap_resource(video->dev, res);
> +
> +	if (IS_ERR(video->base))
> +		return PTR_ERR(video->base);
> +
> +	rc = aspeed_video_init(video);
> +	if (rc)
> +		return rc;
> +
> +	rc = aspeed_video_setup_video(video);
> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}
> +
> +static int aspeed_video_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
> +	struct aspeed_video *video = to_aspeed_video(v4l2_dev);
> +
> +	video_unregister_device(&video->vdev);
> +
> +	vb2_queue_release(&video->queue);
> +
> +	v4l2_ctrl_handler_free(&video->ctrl_handler);
> +
> +	v4l2_device_unregister(v4l2_dev);
> +
> +	dma_free_coherent(video->dev, VE_JPEG_HEADER_SIZE, video->jpeg.virt,
> +			  video->jpeg.dma);
> +
> +	of_reserved_mem_device_release(dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id aspeed_video_of_match[] = {
> +	{ .compatible = "aspeed,ast2400-video-engine" },
> +	{ .compatible = "aspeed,ast2500-video-engine" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_video_of_match);
> +
> +static struct platform_driver aspeed_video_driver = {
> +	.driver = {
> +		.name = DEVICE_NAME,
> +		.of_match_table = aspeed_video_of_match,
> +	},
> +	.probe = aspeed_video_probe,
> +	.remove = aspeed_video_remove,
> +};
> +
> +module_platform_driver(aspeed_video_driver);
> +
> +MODULE_DESCRIPTION("ASPEED Video Engine Driver");
> +MODULE_AUTHOR("Eddie James");
> +MODULE_LICENSE("GPL v2");
> 

Regards,

	Hans

^ permalink raw reply

* [PATCH v5 2/2] media: platform: Add Aspeed Video Engine driver
From: Eddie James @ 2018-11-12 21:00 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1542056431-18146-1-git-send-email-eajames@linux.ibm.com>

The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
can capture and compress video data from digital or analog sources. With
the Aspeed chip acting a service processor, the Video Engine can capture
the host processor graphics output.

Add a V4L2 driver to capture video data and compress it to JPEG images.
Make the video frames available through the V4L2 streaming interface.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 MAINTAINERS                           |    8 +
 drivers/media/platform/Kconfig        |    9 +
 drivers/media/platform/Makefile       |    1 +
 drivers/media/platform/aspeed-video.c | 1711 +++++++++++++++++++++++++++++++++
 4 files changed, 1729 insertions(+)
 create mode 100644 drivers/media/platform/aspeed-video.c

diff --git a/MAINTAINERS b/MAINTAINERS
index fa45ff3..f8f08a4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2414,6 +2414,14 @@ S:	Maintained
 F:	Documentation/hwmon/asc7621
 F:	drivers/hwmon/asc7621.c
 
+ASPEED VIDEO ENGINE DRIVER
+M:	Eddie James <eajames@linux.ibm.com>
+L:	linux-media at vger.kernel.org
+L:	openbmc at lists.ozlabs.org (moderated for non-subscribers)
+S:	Maintained
+F:	drivers/media/platform/aspeed-video.c
+F:	Documentation/devicetree/bindings/media/aspeed-video.txt
+
 ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
 M:	Corentin Chary <corentin.chary@gmail.com>
 L:	acpi4asus-user at lists.sourceforge.net
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 70c4f6c..ba78dd5 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig"
 
 source "drivers/media/platform/omap/Kconfig"
 
+config VIDEO_ASPEED
+	tristate "Aspeed AST2400 and AST2500 Video Engine driver"
+	depends on VIDEO_V4L2
+	select VIDEOBUF2_DMA_CONTIG
+	help
+	  Support for the Aspeed Video Engine (VE) embedded in the Aspeed
+	  AST2400 and AST2500 SOCs. The VE can capture and compress video data
+	  from digital or analog sources.
+
 config VIDEO_SH_VOU
 	tristate "SuperH VOU video output driver"
 	depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 6ab6200..2973953 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -3,6 +3,7 @@
 # Makefile for the video capture/playback device drivers.
 #
 
+obj-$(CONFIG_VIDEO_ASPEED)		+= aspeed-video.o
 obj-$(CONFIG_VIDEO_CADENCE)		+= cadence/
 obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o
 obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
new file mode 100644
index 0000000..a2fd0bf
--- /dev/null
+++ b/drivers/media/platform/aspeed-video.c
@@ -0,0 +1,1711 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/sched.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/v4l2-controls.h>
+#include <linux/videodev2.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
+#include <media/videobuf2-dma-contig.h>
+
+#define DEVICE_NAME			"aspeed-video"
+
+#define ASPEED_VIDEO_JPEG_NUM_QUALITIES	12
+#define ASPEED_VIDEO_JPEG_HEADER_SIZE	10
+#define ASPEED_VIDEO_JPEG_QUANT_SIZE	116
+#define ASPEED_VIDEO_JPEG_DCT_SIZE	34
+
+#define MAX_FRAME_RATE			60
+#define MAX_HEIGHT			1200
+#define MAX_WIDTH			1920
+#define MIN_HEIGHT			480
+#define MIN_WIDTH			640
+
+#define NUM_POLARITY_CHECKS		10
+#define INVALID_RESOLUTION_RETRIES	2
+#define INVALID_RESOLUTION_DELAY	msecs_to_jiffies(250)
+#define RESOLUTION_CHANGE_DELAY		msecs_to_jiffies(500)
+#define MODE_DETECT_TIMEOUT		msecs_to_jiffies(500)
+#define STOP_TIMEOUT			msecs_to_jiffies(250)
+#define DIRECT_FETCH_THRESHOLD		0x0c0000 /* 1024 * 768 */
+
+#define VE_MAX_SRC_BUFFER_SIZE		0x8ca000 /* 1920 * 1200, 32bpp */
+#define VE_JPEG_HEADER_SIZE		0x006000 /* 512 * 12 * 4 */
+
+#define VE_PROTECTION_KEY		0x000
+#define  VE_PROTECTION_KEY_UNLOCK	0x1a038aa8
+
+#define VE_SEQ_CTRL			0x004
+#define  VE_SEQ_CTRL_TRIG_MODE_DET	BIT(0)
+#define  VE_SEQ_CTRL_TRIG_CAPTURE	BIT(1)
+#define  VE_SEQ_CTRL_FORCE_IDLE		BIT(2)
+#define  VE_SEQ_CTRL_MULT_FRAME		BIT(3)
+#define  VE_SEQ_CTRL_TRIG_COMP		BIT(4)
+#define  VE_SEQ_CTRL_AUTO_COMP		BIT(5)
+#define  VE_SEQ_CTRL_EN_WATCHDOG	BIT(7)
+#define  VE_SEQ_CTRL_YUV420		BIT(10)
+#define  VE_SEQ_CTRL_COMP_FMT		GENMASK(11, 10)
+#define  VE_SEQ_CTRL_HALT		BIT(12)
+#define  VE_SEQ_CTRL_EN_WATCHDOG_COMP	BIT(14)
+#define  VE_SEQ_CTRL_TRIG_JPG		BIT(15)
+#define  VE_SEQ_CTRL_CAP_BUSY		BIT(16)
+#define  VE_SEQ_CTRL_COMP_BUSY		BIT(18)
+
+#ifdef CONFIG_MACH_ASPEED_G5
+#define  VE_SEQ_CTRL_JPEG_MODE		BIT(13)	/* AST2500 */
+#else
+#define  VE_SEQ_CTRL_JPEG_MODE		BIT(8)	/* AST2400 */
+#endif /* CONFIG_MACH_ASPEED_G5 */
+
+#define VE_CTRL				0x008
+#define  VE_CTRL_HSYNC_POL		BIT(0)
+#define  VE_CTRL_VSYNC_POL		BIT(1)
+#define  VE_CTRL_SOURCE			BIT(2)
+#define  VE_CTRL_INT_DE			BIT(4)
+#define  VE_CTRL_DIRECT_FETCH		BIT(5)
+#define  VE_CTRL_YUV			BIT(6)
+#define  VE_CTRL_RGB			BIT(7)
+#define  VE_CTRL_CAPTURE_FMT		GENMASK(7, 6)
+#define  VE_CTRL_AUTO_OR_CURSOR		BIT(8)
+#define  VE_CTRL_CLK_INVERSE		BIT(11)
+#define  VE_CTRL_CLK_DELAY		GENMASK(11, 9)
+#define  VE_CTRL_INTERLACE		BIT(14)
+#define  VE_CTRL_HSYNC_POL_CTRL		BIT(15)
+#define  VE_CTRL_FRC			GENMASK(23, 16)
+
+#define VE_TGS_0			0x00c
+#define VE_TGS_1			0x010
+#define  VE_TGS_FIRST			GENMASK(28, 16)
+#define  VE_TGS_LAST			GENMASK(12, 0)
+
+#define VE_SCALING_FACTOR		0x014
+#define VE_SCALING_FILTER0		0x018
+#define VE_SCALING_FILTER1		0x01c
+#define VE_SCALING_FILTER2		0x020
+#define VE_SCALING_FILTER3		0x024
+
+#define VE_CAP_WINDOW			0x030
+#define VE_COMP_WINDOW			0x034
+#define VE_COMP_PROC_OFFSET		0x038
+#define VE_COMP_OFFSET			0x03c
+#define VE_JPEG_ADDR			0x040
+#define VE_SRC0_ADDR			0x044
+#define VE_SRC_SCANLINE_OFFSET		0x048
+#define VE_SRC1_ADDR			0x04c
+#define VE_COMP_ADDR			0x054
+
+#define VE_STREAM_BUF_SIZE		0x058
+#define  VE_STREAM_BUF_SIZE_N_PACKETS	GENMASK(5, 3)
+#define  VE_STREAM_BUF_SIZE_P_SIZE	GENMASK(2, 0)
+
+#define VE_COMP_CTRL			0x060
+#define  VE_COMP_CTRL_VQ_DCT_ONLY	BIT(0)
+#define  VE_COMP_CTRL_VQ_4COLOR		BIT(1)
+#define  VE_COMP_CTRL_QUANTIZE		BIT(2)
+#define  VE_COMP_CTRL_EN_BQ		BIT(4)
+#define  VE_COMP_CTRL_EN_CRYPTO		BIT(5)
+#define  VE_COMP_CTRL_DCT_CHR		GENMASK(10, 6)
+#define  VE_COMP_CTRL_DCT_LUM		GENMASK(15, 11)
+#define  VE_COMP_CTRL_EN_HQ		BIT(16)
+#define  VE_COMP_CTRL_RSVD		BIT(19)
+#define  VE_COMP_CTRL_ENCODE		GENMASK(21, 20)
+#define  VE_COMP_CTRL_HQ_DCT_CHR	GENMASK(26, 22)
+#define  VE_COMP_CTRL_HQ_DCT_LUM	GENMASK(31, 27)
+
+#define VE_OFFSET_COMP_STREAM		0x078
+
+#define VE_SRC_LR_EDGE_DET		0x090
+#define  VE_SRC_LR_EDGE_DET_LEFT	GENMASK(11, 0)
+#define  VE_SRC_LR_EDGE_DET_NO_V	BIT(12)
+#define  VE_SRC_LR_EDGE_DET_NO_H	BIT(13)
+#define  VE_SRC_LR_EDGE_DET_NO_DISP	BIT(14)
+#define  VE_SRC_LR_EDGE_DET_NO_CLK	BIT(15)
+#define  VE_SRC_LR_EDGE_DET_RT_SHF	16
+#define  VE_SRC_LR_EDGE_DET_RT		GENMASK(27, VE_SRC_LR_EDGE_DET_RT_SHF)
+#define  VE_SRC_LR_EDGE_DET_INTERLACE	BIT(31)
+
+#define VE_SRC_TB_EDGE_DET		0x094
+#define  VE_SRC_TB_EDGE_DET_TOP		GENMASK(12, 0)
+#define  VE_SRC_TB_EDGE_DET_BOT_SHF	16
+#define  VE_SRC_TB_EDGE_DET_BOT		GENMASK(28, VE_SRC_TB_EDGE_DET_BOT_SHF)
+
+#define VE_MODE_DETECT_STATUS		0x098
+#define  VE_MODE_DETECT_H_PIXELS	GENMASK(11, 0)
+#define  VE_MODE_DETECT_V_LINES_SHF	16
+#define  VE_MODE_DETECT_V_LINES		GENMASK(27, VE_MODE_DETECT_V_LINES_SHF)
+#define  VE_MODE_DETECT_STATUS_VSYNC	BIT(28)
+#define  VE_MODE_DETECT_STATUS_HSYNC	BIT(29)
+
+#define VE_SYNC_STATUS			0x09c
+#define  VE_SYNC_STATUS_HSYNC		GENMASK(11, 0)
+#define  VE_SYNC_STATUS_VSYNC_SHF	16
+#define  VE_SYNC_STATUS_VSYNC		GENMASK(27, VE_SYNC_STATUS_VSYNC_SHF)
+
+#define VE_INTERRUPT_CTRL		0x304
+#define VE_INTERRUPT_STATUS		0x308
+#define  VE_INTERRUPT_MODE_DETECT_WD	BIT(0)
+#define  VE_INTERRUPT_CAPTURE_COMPLETE	BIT(1)
+#define  VE_INTERRUPT_COMP_READY	BIT(2)
+#define  VE_INTERRUPT_COMP_COMPLETE	BIT(3)
+#define  VE_INTERRUPT_MODE_DETECT	BIT(4)
+#define  VE_INTERRUPT_FRAME_COMPLETE	BIT(5)
+#define  VE_INTERRUPT_DECODE_ERR	BIT(6)
+#define  VE_INTERRUPT_HALT_READY	BIT(8)
+#define  VE_INTERRUPT_HANG_WD		BIT(9)
+#define  VE_INTERRUPT_STREAM_DESC	BIT(10)
+#define  VE_INTERRUPT_VSYNC_DESC	BIT(11)
+
+#define VE_MODE_DETECT			0x30c
+#define VE_MEM_RESTRICT_START		0x310
+#define VE_MEM_RESTRICT_END		0x314
+
+enum {
+	VIDEO_MODE_DETECT_DONE,
+	VIDEO_RES_CHANGE,
+	VIDEO_STREAMING,
+	VIDEO_FRAME_INPRG,
+};
+
+struct aspeed_video_addr {
+	unsigned int size;
+	dma_addr_t dma;
+	void *virt;
+};
+
+struct aspeed_video_buffer {
+	struct vb2_v4l2_buffer vb;
+	struct list_head link;
+};
+
+#define to_aspeed_video_buffer(x) \
+	container_of((x), struct aspeed_video_buffer, vb)
+
+struct aspeed_video {
+	void __iomem *base;
+	struct clk *eclk;
+	struct clk *vclk;
+	struct reset_control *rst;
+
+	struct device *dev;
+	struct v4l2_ctrl_handler ctrl_handler;
+	struct v4l2_device v4l2_dev;
+	struct v4l2_pix_format pix_fmt;
+	struct v4l2_bt_timings timings;
+	struct vb2_queue queue;
+	struct video_device vdev;
+	struct mutex video_lock;
+
+	atomic_t clients;
+	wait_queue_head_t wait;
+	spinlock_t lock;
+	struct delayed_work res_work;
+	struct list_head buffers;
+	unsigned long flags;
+	unsigned int sequence;
+
+	unsigned int max_compressed_size;
+	struct aspeed_video_addr srcs[2];
+	struct aspeed_video_addr jpeg;
+	struct aspeed_video_addr detect;
+
+	bool yuv420;
+	unsigned int frame_rate;
+	unsigned int jpeg_quality;
+	unsigned int capture_height;
+	unsigned int capture_width;
+};
+
+#define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
+
+static const u32 aspeed_video_jpeg_header[ASPEED_VIDEO_JPEG_HEADER_SIZE] = {
+	0xe0ffd8ff, 0x464a1000, 0x01004649, 0x60000101, 0x00006000, 0x0f00feff,
+	0x00002d05, 0x00000000, 0x00000000, 0x00dbff00
+};
+
+static const u32 aspeed_video_jpeg_quant[ASPEED_VIDEO_JPEG_QUANT_SIZE] = {
+	0x081100c0, 0x00000000, 0x00110103, 0x03011102, 0xc4ff0111, 0x00001f00,
+	0x01010501, 0x01010101, 0x00000000, 0x00000000, 0x04030201, 0x08070605,
+	0xff0b0a09, 0x10b500c4, 0x03010200, 0x03040203, 0x04040505, 0x7d010000,
+	0x00030201, 0x12051104, 0x06413121, 0x07615113, 0x32147122, 0x08a19181,
+	0xc1b14223, 0xf0d15215, 0x72623324, 0x160a0982, 0x1a191817, 0x28272625,
+	0x35342a29, 0x39383736, 0x4544433a, 0x49484746, 0x5554534a, 0x59585756,
+	0x6564635a, 0x69686766, 0x7574736a, 0x79787776, 0x8584837a, 0x89888786,
+	0x9493928a, 0x98979695, 0xa3a29a99, 0xa7a6a5a4, 0xb2aaa9a8, 0xb6b5b4b3,
+	0xbab9b8b7, 0xc5c4c3c2, 0xc9c8c7c6, 0xd4d3d2ca, 0xd8d7d6d5, 0xe2e1dad9,
+	0xe6e5e4e3, 0xeae9e8e7, 0xf4f3f2f1, 0xf8f7f6f5, 0xc4fffaf9, 0x00011f00,
+	0x01010103, 0x01010101, 0x00000101, 0x00000000, 0x04030201, 0x08070605,
+	0xff0b0a09, 0x11b500c4, 0x02010200, 0x04030404, 0x04040507, 0x77020100,
+	0x03020100, 0x21050411, 0x41120631, 0x71610751, 0x81322213, 0x91421408,
+	0x09c1b1a1, 0xf0523323, 0xd1726215, 0x3424160a, 0x17f125e1, 0x261a1918,
+	0x2a292827, 0x38373635, 0x44433a39, 0x48474645, 0x54534a49, 0x58575655,
+	0x64635a59, 0x68676665, 0x74736a69, 0x78777675, 0x83827a79, 0x87868584,
+	0x928a8988, 0x96959493, 0x9a999897, 0xa5a4a3a2, 0xa9a8a7a6, 0xb4b3b2aa,
+	0xb8b7b6b5, 0xc3c2bab9, 0xc7c6c5c4, 0xd2cac9c8, 0xd6d5d4d3, 0xdad9d8d7,
+	0xe5e4e3e2, 0xe9e8e7e6, 0xf4f3f2ea, 0xf8f7f6f5, 0xdafffaf9, 0x01030c00,
+	0x03110200, 0x003f0011
+};
+
+static const u32 aspeed_video_jpeg_dct[ASPEED_VIDEO_JPEG_NUM_QUALITIES]
+				      [ASPEED_VIDEO_JPEG_DCT_SIZE] = {
+	{ 0x0d140043, 0x0c0f110f, 0x11101114, 0x17141516, 0x1e20321e,
+	  0x3d1e1b1b, 0x32242e2b, 0x4b4c3f48, 0x44463f47, 0x61735a50,
+	  0x566c5550, 0x88644644, 0x7a766c65, 0x4d808280, 0x8c978d60,
+	  0x7e73967d, 0xdbff7b80, 0x1f014300, 0x272d2121, 0x3030582d,
+	  0x697bb958, 0xb8b9b97b, 0xb9b8a6a6, 0xb9b9b9b9, 0xb9b9b9b9,
+	  0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9,
+	  0xb9b9b9b9, 0xb9b9b9b9, 0xb9b9b9b9, 0xffb9b9b9 },
+	{ 0x0c110043, 0x0a0d0f0d, 0x0f0e0f11, 0x14111213, 0x1a1c2b1a,
+	  0x351a1818, 0x2b1f2826, 0x4142373f, 0x3c3d373e, 0x55644e46,
+	  0x4b5f4a46, 0x77573d3c, 0x6b675f58, 0x43707170, 0x7a847b54,
+	  0x6e64836d, 0xdbff6c70, 0x1b014300, 0x22271d1d, 0x2a2a4c27,
+	  0x5b6ba04c, 0xa0a0a06b, 0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0,
+	  0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0,
+	  0xa0a0a0a0, 0xa0a0a0a0, 0xa0a0a0a0, 0xffa0a0a0 },
+	{ 0x090e0043, 0x090a0c0a, 0x0c0b0c0e, 0x110e0f10, 0x15172415,
+	  0x2c151313, 0x241a211f, 0x36372e34, 0x31322e33, 0x4653413a,
+	  0x3e4e3d3a, 0x62483231, 0x58564e49, 0x385d5e5d, 0x656d6645,
+	  0x5b536c5a, 0xdbff595d, 0x16014300, 0x1c201818, 0x22223f20,
+	  0x4b58853f, 0x85858558, 0x85858585, 0x85858585, 0x85858585,
+	  0x85858585, 0x85858585, 0x85858585, 0x85858585, 0x85858585,
+	  0x85858585, 0x85858585, 0x85858585, 0xff858585 },
+	{ 0x070b0043, 0x07080a08, 0x0a090a0b, 0x0d0b0c0c, 0x11121c11,
+	  0x23110f0f, 0x1c141a19, 0x2b2b2429, 0x27282428, 0x3842332e,
+	  0x313e302e, 0x4e392827, 0x46443e3a, 0x2c4a4a4a, 0x50565137,
+	  0x48425647, 0xdbff474a, 0x12014300, 0x161a1313, 0x1c1c331a,
+	  0x3d486c33, 0x6c6c6c48, 0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c,
+	  0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c,
+	  0x6c6c6c6c, 0x6c6c6c6c, 0x6c6c6c6c, 0xff6c6c6c },
+	{ 0x06090043, 0x05060706, 0x07070709, 0x0a09090a, 0x0d0e160d,
+	  0x1b0d0c0c, 0x16101413, 0x21221c20, 0x1e1f1c20, 0x2b332824,
+	  0x26302624, 0x3d2d1f1e, 0x3735302d, 0x22393a39, 0x3f443f2b,
+	  0x38334338, 0xdbff3739, 0x0d014300, 0x11130e0e, 0x15152613,
+	  0x2d355026, 0x50505035, 0x50505050, 0x50505050, 0x50505050,
+	  0x50505050, 0x50505050, 0x50505050, 0x50505050, 0x50505050,
+	  0x50505050, 0x50505050, 0x50505050, 0xff505050 },
+	{ 0x04060043, 0x03040504, 0x05040506, 0x07060606, 0x09090f09,
+	  0x12090808, 0x0f0a0d0d, 0x16161315, 0x14151315, 0x1d221b18,
+	  0x19201918, 0x281e1514, 0x2423201e, 0x17262726, 0x2a2d2a1c,
+	  0x25222d25, 0xdbff2526, 0x09014300, 0x0b0d0a0a, 0x0e0e1a0d,
+	  0x1f25371a, 0x37373725, 0x37373737, 0x37373737, 0x37373737,
+	  0x37373737, 0x37373737, 0x37373737, 0x37373737, 0x37373737,
+	  0x37373737, 0x37373737, 0x37373737, 0xff373737 },
+	{ 0x02030043, 0x01020202, 0x02020203, 0x03030303, 0x04040704,
+	  0x09040404, 0x07050606, 0x0b0b090a, 0x0a0a090a, 0x0e110d0c,
+	  0x0c100c0c, 0x140f0a0a, 0x1211100f, 0x0b131313, 0x1516150e,
+	  0x12111612, 0xdbff1213, 0x04014300, 0x05060505, 0x07070d06,
+	  0x0f121b0d, 0x1b1b1b12, 0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b,
+	  0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b,
+	  0x1b1b1b1b, 0x1b1b1b1b, 0x1b1b1b1b, 0xff1b1b1b },
+	{ 0x01020043, 0x01010101, 0x01010102, 0x02020202, 0x03030503,
+	  0x06030202, 0x05030404, 0x07070607, 0x06070607, 0x090b0908,
+	  0x080a0808, 0x0d0a0706, 0x0c0b0a0a, 0x070c0d0c, 0x0e0f0e09,
+	  0x0c0b0f0c, 0xdbff0c0c, 0x03014300, 0x03040303, 0x04040804,
+	  0x0a0c1208, 0x1212120c, 0x12121212, 0x12121212, 0x12121212,
+	  0x12121212, 0x12121212, 0x12121212, 0x12121212, 0x12121212,
+	  0x12121212, 0x12121212, 0x12121212, 0xff121212 },
+	{ 0x01020043, 0x01010101, 0x01010102, 0x02020202, 0x03030503,
+	  0x06030202, 0x05030404, 0x07070607, 0x06070607, 0x090b0908,
+	  0x080a0808, 0x0d0a0706, 0x0c0b0a0a, 0x070c0d0c, 0x0e0f0e09,
+	  0x0c0b0f0c, 0xdbff0c0c, 0x02014300, 0x03030202, 0x04040703,
+	  0x080a0f07, 0x0f0f0f0a, 0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f,
+	  0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f,
+	  0x0f0f0f0f, 0x0f0f0f0f, 0x0f0f0f0f, 0xff0f0f0f },
+	{ 0x01010043, 0x01010101, 0x01010101, 0x01010101, 0x02020302,
+	  0x04020202, 0x03020303, 0x05050405, 0x05050405, 0x07080606,
+	  0x06080606, 0x0a070505, 0x09080807, 0x05090909, 0x0a0b0a07,
+	  0x09080b09, 0xdbff0909, 0x02014300, 0x02030202, 0x03030503,
+	  0x07080c05, 0x0c0c0c08, 0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c,
+	  0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c,
+	  0x0c0c0c0c, 0x0c0c0c0c, 0x0c0c0c0c, 0xff0c0c0c },
+	{ 0x01010043, 0x01010101, 0x01010101, 0x01010101, 0x01010201,
+	  0x03010101, 0x02010202, 0x03030303, 0x03030303, 0x04050404,
+	  0x04050404, 0x06050303, 0x06050505, 0x03060606, 0x07070704,
+	  0x06050706, 0xdbff0606, 0x01014300, 0x01020101, 0x02020402,
+	  0x05060904, 0x09090906, 0x09090909, 0x09090909, 0x09090909,
+	  0x09090909, 0x09090909, 0x09090909, 0x09090909, 0x09090909,
+	  0x09090909, 0x09090909, 0x09090909, 0xff090909 },
+	{ 0x01010043, 0x01010101, 0x01010101, 0x01010101, 0x01010101,
+	  0x01010101, 0x01010101, 0x01010101, 0x01010101, 0x02020202,
+	  0x02020202, 0x03020101, 0x03020202, 0x01030303, 0x03030302,
+	  0x03020303, 0xdbff0403, 0x01014300, 0x01010101, 0x01010201,
+	  0x03040602, 0x06060604, 0x06060606, 0x06060606, 0x06060606,
+	  0x06060606, 0x06060606, 0x06060606, 0x06060606, 0x06060606,
+	  0x06060606, 0x06060606, 0x06060606, 0xff060606 }
+};
+
+static void aspeed_video_init_jpeg_table(u32 *table, bool yuv420)
+{
+	int i;
+	unsigned int base;
+
+	for (i = 0; i < ASPEED_VIDEO_JPEG_NUM_QUALITIES; i++) {
+		base = 256 * i;	/* AST HW requires this header spacing */
+		memcpy(&table[base], aspeed_video_jpeg_header,
+		       sizeof(aspeed_video_jpeg_header));
+
+		base += ASPEED_VIDEO_JPEG_HEADER_SIZE;
+		memcpy(&table[base], aspeed_video_jpeg_dct[i],
+		       sizeof(aspeed_video_jpeg_dct[i]));
+
+		base += ASPEED_VIDEO_JPEG_DCT_SIZE;
+		memcpy(&table[base], aspeed_video_jpeg_quant,
+		       sizeof(aspeed_video_jpeg_quant));
+
+		if (yuv420)
+			table[base + 2] = 0x00220103;
+	}
+}
+
+static void aspeed_video_update(struct aspeed_video *video, u32 reg, u32 clear,
+				u32 bits)
+{
+	u32 t = readl(video->base + reg);
+	u32 before = t;
+
+	t &= ~clear;
+	t |= bits;
+	writel(t, video->base + reg);
+	dev_dbg(video->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);
+	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));
+}
+
+static bool aspeed_video_engine_busy(struct aspeed_video *video)
+{
+	u32 seq_ctrl = aspeed_video_read(video, VE_SEQ_CTRL);
+
+	if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
+	    !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
+		dev_err(video->dev, "video engine busy\n");
+		return true;
+	}
+
+	return false;
+}
+
+static int aspeed_video_start_frame(struct aspeed_video *video)
+{
+	dma_addr_t addr;
+	unsigned long flags;
+	struct aspeed_video_buffer *buf;
+
+	if (aspeed_video_engine_busy(video))
+		return -EBUSY;
+
+	spin_lock_irqsave(&video->lock, flags);
+	buf = list_first_entry_or_null(&video->buffers,
+				       struct aspeed_video_buffer, link);
+	if (!buf) {
+		spin_unlock_irqrestore(&video->lock, flags);
+		return -EPROTO;
+	}
+
+	set_bit(VIDEO_FRAME_INPRG, &video->flags);
+	addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
+	spin_unlock_irqrestore(&video->lock, flags);
+
+	aspeed_video_write(video, VE_COMP_PROC_OFFSET, 0);
+	aspeed_video_write(video, VE_COMP_OFFSET, 0);
+	aspeed_video_write(video, VE_COMP_ADDR, addr);
+
+	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
+			    VE_INTERRUPT_COMP_COMPLETE |
+			    VE_INTERRUPT_CAPTURE_COMPLETE);
+
+	aspeed_video_update(video, VE_SEQ_CTRL, 0,
+			    VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
+
+	return 0;
+}
+
+static void aspeed_video_start_mode_detect(struct aspeed_video *video)
+{
+	/* Enable mode detect interrupts */
+	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
+			    VE_INTERRUPT_MODE_DETECT);
+
+	/* Trigger mode detect */
+	aspeed_video_update(video, VE_SEQ_CTRL, 0, VE_SEQ_CTRL_TRIG_MODE_DET);
+}
+
+static void aspeed_video_disable_mode_detect(struct aspeed_video *video)
+{
+	/* Disable mode detect interrupts */
+	aspeed_video_update(video, VE_INTERRUPT_CTRL,
+			    VE_INTERRUPT_MODE_DETECT, 0);
+
+	/* Disable mode detect */
+	aspeed_video_update(video, VE_SEQ_CTRL, VE_SEQ_CTRL_TRIG_MODE_DET, 0);
+}
+
+static void aspeed_video_off(struct aspeed_video *video)
+{
+	/* Reset the engine */
+	reset_control_assert(video->rst);
+	udelay(100);
+	reset_control_deassert(video->rst);
+
+	/* Turn off the relevant clocks */
+	clk_disable_unprepare(video->vclk);
+	clk_disable_unprepare(video->eclk);
+}
+
+static void aspeed_video_on(struct aspeed_video *video)
+{
+	/* Turn on the relevant clocks */
+	clk_prepare_enable(video->eclk);
+	clk_prepare_enable(video->vclk);
+
+	/* Reset the engine */
+	reset_control_assert(video->rst);
+	udelay(100);
+	reset_control_deassert(video->rst);
+}
+
+static void aspeed_video_bufs_done(struct aspeed_video *video,
+				   enum vb2_buffer_state state)
+{
+	unsigned long flags;
+	struct aspeed_video_buffer *buf;
+
+	spin_lock_irqsave(&video->lock, flags);
+	list_for_each_entry(buf, &video->buffers, link) {
+		if (list_is_last(&buf->link, &video->buffers))
+			buf->vb.flags |= V4L2_BUF_FLAG_LAST;
+		vb2_buffer_done(&buf->vb.vb2_buf, state);
+	}
+	INIT_LIST_HEAD(&video->buffers);
+	spin_unlock_irqrestore(&video->lock, flags);
+}
+
+static irqreturn_t aspeed_video_irq(int irq, void *arg)
+{
+	struct aspeed_video *video = arg;
+	u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS);
+
+	if (atomic_read(&video->clients) == 0) {
+		dev_info(video->dev, "irq with no client; disabling irqs\n");
+
+		aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
+		aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
+		return IRQ_HANDLED;
+	}
+
+	/* Resolution changed; reset entire engine and reinitialize */
+	if (sts & VE_INTERRUPT_MODE_DETECT_WD) {
+		dev_info(video->dev, "resolution changed; resetting\n");
+		set_bit(VIDEO_RES_CHANGE, &video->flags);
+		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+		clear_bit(VIDEO_STREAMING, &video->flags);
+
+		aspeed_video_off(video);
+		aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
+
+		schedule_delayed_work(&video->res_work,
+				      RESOLUTION_CHANGE_DELAY);
+		return IRQ_HANDLED;
+	}
+
+	if (sts & VE_INTERRUPT_MODE_DETECT) {
+		aspeed_video_update(video, VE_INTERRUPT_CTRL,
+				    VE_INTERRUPT_MODE_DETECT, 0);
+		aspeed_video_write(video, VE_INTERRUPT_STATUS,
+				   VE_INTERRUPT_MODE_DETECT);
+
+		set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
+		wake_up_interruptible_all(&video->wait);
+	}
+
+	if ((sts & VE_INTERRUPT_COMP_COMPLETE) &&
+	    (sts & VE_INTERRUPT_CAPTURE_COMPLETE)) {
+		struct aspeed_video_buffer *buf;
+		u32 frame_size = aspeed_video_read(video,
+						   VE_OFFSET_COMP_STREAM);
+
+		spin_lock(&video->lock);
+		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+		buf = list_first_entry_or_null(&video->buffers,
+					       struct aspeed_video_buffer,
+					       link);
+		if (buf) {
+			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size);
+
+			if (!list_is_last(&buf->link, &video->buffers)) {
+				buf->vb.vb2_buf.timestamp = ktime_get_ns();
+				buf->vb.sequence = video->sequence++;
+				buf->vb.field = V4L2_FIELD_NONE;
+				vb2_buffer_done(&buf->vb.vb2_buf,
+						VB2_BUF_STATE_DONE);
+				list_del(&buf->link);
+			}
+		}
+		spin_unlock(&video->lock);
+
+		aspeed_video_update(video, VE_SEQ_CTRL,
+				    VE_SEQ_CTRL_TRIG_CAPTURE |
+				    VE_SEQ_CTRL_FORCE_IDLE |
+				    VE_SEQ_CTRL_TRIG_COMP, 0);
+		aspeed_video_update(video, VE_INTERRUPT_CTRL,
+				    VE_INTERRUPT_COMP_COMPLETE |
+				    VE_INTERRUPT_CAPTURE_COMPLETE, 0);
+		aspeed_video_write(video, VE_INTERRUPT_STATUS,
+				   VE_INTERRUPT_COMP_COMPLETE |
+				   VE_INTERRUPT_CAPTURE_COMPLETE);
+
+		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
+			aspeed_video_start_frame(video);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void aspeed_video_check_polarity(struct aspeed_video *video)
+{
+	int i;
+	int hsync_counter = 0;
+	int vsync_counter = 0;
+	u32 sts;
+
+	for (i = 0; i < NUM_POLARITY_CHECKS; ++i) {
+		sts = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
+		if (sts & VE_MODE_DETECT_STATUS_VSYNC)
+			vsync_counter--;
+		else
+			vsync_counter++;
+
+		if (sts & VE_MODE_DETECT_STATUS_HSYNC)
+			hsync_counter--;
+		else
+			hsync_counter++;
+	}
+
+	if (hsync_counter < 0 || vsync_counter < 0) {
+		u32 ctrl;
+
+		if (hsync_counter < 0)
+			ctrl = VE_CTRL_HSYNC_POL;
+		else
+			video->timings.polarities |= V4L2_DV_HSYNC_POS_POL;
+
+		if (vsync_counter < 0)
+			ctrl = VE_CTRL_VSYNC_POL;
+		else
+			video->timings.polarities |= V4L2_DV_VSYNC_POS_POL;
+
+		aspeed_video_update(video, VE_CTRL, 0, ctrl);
+	}
+}
+
+static bool aspeed_video_alloc_buf(struct aspeed_video *video,
+				   struct aspeed_video_addr *addr,
+				   unsigned int size)
+{
+	addr->virt = dma_alloc_coherent(video->dev, size, &addr->dma,
+					GFP_KERNEL);
+	if (!addr->virt)
+		return false;
+
+	addr->size = size;
+	return true;
+}
+
+static void aspeed_video_free_buf(struct aspeed_video *video,
+				  struct aspeed_video_addr *addr)
+{
+	dma_free_coherent(video->dev, addr->size, addr->virt, addr->dma);
+	addr->size = 0;
+	addr->dma = 0ULL;
+	addr->virt = NULL;
+}
+
+/*
+ * Get the minimum HW-supported compression buffer size for the frame size.
+ * Assume worst-case JPEG compression size is 1/8 raw size. This should be
+ * plenty even for maximum quality; any worse and the engine will simply return
+ * incomplete JPEGs.
+ */
+static void aspeed_video_calc_compressed_size(struct aspeed_video *video)
+{
+	int i, j;
+	u32 compression_buffer_size_reg = 0;
+	unsigned int size;
+	const unsigned int num_compression_packets = 4;
+	const unsigned int compression_packet_size = 1024;
+	const unsigned int max_compressed_size =
+		video->capture_width * video->capture_height / 2;
+
+	video->max_compressed_size = UINT_MAX;
+
+	for (i = 0; i < 6; ++i) {
+		for (j = 0; j < 8; ++j) {
+			size = (num_compression_packets << i) *
+				(compression_packet_size << j);
+			if (size < max_compressed_size)
+				continue;
+
+			if (size < video->max_compressed_size) {
+				compression_buffer_size_reg = (i << 3) | j;
+				video->max_compressed_size = size;
+			}
+		}
+	}
+
+	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);
+}
+
+#define res_check(v) test_and_clear_bit(VIDEO_MODE_DETECT_DONE, &(v)->flags)
+
+static int aspeed_video_get_resolution(struct aspeed_video *video)
+{
+	bool invalid_resolution = true;
+	int rc;
+	int tries = 0;
+	unsigned int bottom;
+	unsigned int left;
+	unsigned int right;
+	unsigned int top;
+	u32 mds;
+	u32 src_lr_edge;
+	u32 src_tb_edge;
+	u32 sync;
+
+	if (video->srcs[1].size)
+		aspeed_video_free_buf(video, &video->srcs[1]);
+
+	if (!video->detect.size) {
+		if (video->srcs[0].size >= VE_MAX_SRC_BUFFER_SIZE) {
+			video->detect = video->srcs[0];
+
+			video->srcs[0].size = 0;
+			video->srcs[0].dma = 0ULL;
+			video->srcs[0].virt = NULL;
+		} else {
+			if (video->srcs[0].size)
+				aspeed_video_free_buf(video, &video->srcs[0]);
+
+			if (!aspeed_video_alloc_buf(video, &video->detect,
+						    VE_MAX_SRC_BUFFER_SIZE)) {
+				dev_err(video->dev,
+					"failed to allocate source buffers\n");
+				return -ENOMEM;
+			}
+		}
+	}
+
+	aspeed_video_write(video, VE_SRC0_ADDR, video->detect.dma);
+
+	video->timings.width = 0;
+	video->timings.height = 0;
+
+	do {
+		if (tries) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			if (schedule_timeout(INVALID_RESOLUTION_DELAY))
+				return -EINTR;
+		}
+
+		aspeed_video_start_mode_detect(video);
+
+		rc = wait_event_interruptible_timeout(video->wait,
+						      res_check(video),
+						      MODE_DETECT_TIMEOUT);
+		if (!rc) {
+			dev_err(video->dev, "timed out on 1st mode detect\n");
+			aspeed_video_disable_mode_detect(video);
+			return -ETIMEDOUT;
+		}
+
+		/* Disable mode detect in order to re-trigger */
+		aspeed_video_update(video, VE_SEQ_CTRL,
+				    VE_SEQ_CTRL_TRIG_MODE_DET, 0);
+
+		aspeed_video_check_polarity(video);
+
+		aspeed_video_start_mode_detect(video);
+
+		rc = wait_event_interruptible_timeout(video->wait,
+						      res_check(video),
+						      MODE_DETECT_TIMEOUT);
+		if (!rc) {
+			dev_err(video->dev, "timed out on 2nd mode detect\n");
+			aspeed_video_disable_mode_detect(video);
+			return -ETIMEDOUT;
+		}
+
+		src_lr_edge = aspeed_video_read(video, VE_SRC_LR_EDGE_DET);
+		src_tb_edge = aspeed_video_read(video, VE_SRC_TB_EDGE_DET);
+		mds = aspeed_video_read(video, VE_MODE_DETECT_STATUS);
+		sync = aspeed_video_read(video, VE_SYNC_STATUS);
+
+		bottom = (src_tb_edge & VE_SRC_TB_EDGE_DET_BOT) >>
+			VE_SRC_TB_EDGE_DET_BOT_SHF;
+		top = src_tb_edge & VE_SRC_TB_EDGE_DET_TOP;
+		video->timings.vfrontporch = top;
+		video->timings.vbackporch = ((mds & VE_MODE_DETECT_V_LINES) >>
+			VE_MODE_DETECT_V_LINES_SHF) - bottom;
+		video->timings.vsync = (sync & VE_SYNC_STATUS_VSYNC) >>
+			VE_SYNC_STATUS_VSYNC_SHF;
+		if (top > bottom)
+			continue;
+
+		right = (src_lr_edge & VE_SRC_LR_EDGE_DET_RT) >>
+			VE_SRC_LR_EDGE_DET_RT_SHF;
+		left = src_lr_edge & VE_SRC_LR_EDGE_DET_LEFT;
+		video->timings.hfrontporch = left;
+		video->timings.hbackporch = (mds & VE_MODE_DETECT_H_PIXELS) -
+			right;
+		video->timings.hsync = sync & VE_SYNC_STATUS_HSYNC;
+		if (left > right)
+			continue;
+
+		invalid_resolution = false;
+	} while (invalid_resolution && (tries++ < INVALID_RESOLUTION_RETRIES));
+
+	if (invalid_resolution) {
+		dev_err(video->dev, "invalid resolution detected\n");
+		return -ERANGE;
+	}
+
+	video->timings.height = (bottom - top) + 1;
+	video->timings.width = (right - left) + 1;
+
+	/* Don't use direct mode below 1024 x 768 (irqs don't fire) */
+	if (video->timings.width * video->timings.height < DIRECT_FETCH_THRESHOLD) {
+		aspeed_video_write(video, VE_TGS_0,
+				   FIELD_PREP(VE_TGS_FIRST, left - 1) |
+				   FIELD_PREP(VE_TGS_LAST, right));
+		aspeed_video_write(video, VE_TGS_1,
+				   FIELD_PREP(VE_TGS_FIRST, top) |
+				   FIELD_PREP(VE_TGS_LAST, bottom + 1));
+		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_INT_DE);
+	} else {
+		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
+	}
+
+	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
+			    VE_INTERRUPT_MODE_DETECT_WD);
+	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", video->timings.width,
+		video->timings.height);
+
+	return 0;
+
+}
+
+static int aspeed_video_set_capture_resolution(struct aspeed_video *video)
+{
+	unsigned int size = video->timings.width * video->timings.height;
+	struct aspeed_video_addr detect = video->detect;
+
+	video->detect.size = 0;
+	video->detect.dma = 0ULL;
+	video->detect.virt = NULL;
+
+	video->capture_width = video->timings.width;
+	video->capture_height = video->timings.height;
+
+	aspeed_video_write(video, VE_CAP_WINDOW,
+			   video->capture_width << 16 | video->capture_height);
+	aspeed_video_write(video, VE_COMP_WINDOW,
+			   video->capture_width << 16 | video->capture_height);
+	aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET,
+			   video->capture_width * 4);
+
+	size *= 4;
+	if (size == detect.size / 2) {
+		aspeed_video_write(video, VE_SRC1_ADDR, detect.dma + size);
+		video->srcs[0] = detect;
+	} else if (size == detect.size) {
+		video->srcs[0] = detect;
+
+		if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
+			goto err_mem;
+
+		aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
+	} else {
+		aspeed_video_free_buf(video, &detect);
+
+		if (!aspeed_video_alloc_buf(video, &video->srcs[0], size))
+			goto err_mem;
+
+		if (!aspeed_video_alloc_buf(video, &video->srcs[1], size))
+			goto err_mem;
+
+		aspeed_video_write(video, VE_SRC0_ADDR, video->srcs[0].dma);
+		aspeed_video_write(video, VE_SRC1_ADDR, video->srcs[1].dma);
+	}
+
+	aspeed_video_calc_compressed_size(video);
+
+	dev_dbg(video->dev, "set resolution[%dx%d]\n", video->capture_width,
+		video->capture_height);
+
+	return 0;
+
+err_mem:
+	dev_err(video->dev, "failed to allocate source buffers\n");
+
+	if (video->srcs[0].size)
+		aspeed_video_free_buf(video, &video->srcs[0]);
+
+	return -ENOMEM;
+}
+
+static void aspeed_video_init_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 seq_ctrl = VE_SEQ_CTRL_JPEG_MODE;
+
+	if (video->frame_rate)
+		ctrl |= FIELD_PREP(VE_CTRL_FRC, video->frame_rate);
+
+	if (video->yuv420)
+		seq_ctrl |= VE_SEQ_CTRL_YUV420;
+
+	/* Unlock VE registers */
+	aspeed_video_write(video, VE_PROTECTION_KEY, VE_PROTECTION_KEY_UNLOCK);
+
+	/* Disable interrupts */
+	aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
+	aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
+
+	/* Clear the offset */
+	aspeed_video_write(video, VE_COMP_PROC_OFFSET, 0);
+	aspeed_video_write(video, VE_COMP_OFFSET, 0);
+
+	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);
+
+	/* Don't downscale */
+	aspeed_video_write(video, VE_SCALING_FACTOR, 0x10001000);
+	aspeed_video_write(video, VE_SCALING_FILTER0, 0x00200000);
+	aspeed_video_write(video, VE_SCALING_FILTER1, 0x00200000);
+	aspeed_video_write(video, VE_SCALING_FILTER2, 0x00200000);
+	aspeed_video_write(video, VE_SCALING_FILTER3, 0x00200000);
+
+	/* Set mode detection defaults */
+	aspeed_video_write(video, VE_MODE_DETECT, 0x22666500);
+}
+
+static int aspeed_video_start(struct aspeed_video *video)
+{
+	int rc;
+
+	aspeed_video_on(video);
+
+	aspeed_video_init_regs(video);
+
+	rc = aspeed_video_get_resolution(video);
+	if (rc)
+		return rc;
+
+	rc = aspeed_video_set_capture_resolution(video);
+	if (rc)
+		return rc;
+
+	video->pix_fmt.width = video->timings.width;
+	video->pix_fmt.height = video->timings.height;
+	video->pix_fmt.sizeimage = video->max_compressed_size;
+
+	return 0;
+}
+
+static void aspeed_video_stop(struct aspeed_video *video)
+{
+	cancel_delayed_work_sync(&video->res_work);
+
+	aspeed_video_off(video);
+
+	if (video->srcs[0].size)
+		aspeed_video_free_buf(video, &video->srcs[0]);
+
+	if (video->srcs[1].size)
+		aspeed_video_free_buf(video, &video->srcs[1]);
+
+	if (video->detect.size)
+		aspeed_video_free_buf(video, &video->detect);
+
+	video->flags = 0;
+}
+
+static int aspeed_video_querycap(struct file *file, void *fh,
+				 struct v4l2_capability *cap)
+{
+	strscpy(cap->driver, DEVICE_NAME, sizeof(cap->driver));
+	strscpy(cap->card, "Aspeed Video Engine", sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
+		 DEVICE_NAME);
+
+	return 0;
+}
+
+static int aspeed_video_enum_format(struct file *file, void *fh,
+				    struct v4l2_fmtdesc *f)
+{
+	if (f->index)
+		return -EINVAL;
+
+	f->pixelformat = V4L2_PIX_FMT_JPEG;
+
+	return 0;
+}
+
+static int aspeed_video_get_format(struct file *file, void *fh,
+				   struct v4l2_format *f)
+{
+	struct aspeed_video *video = video_drvdata(file);
+
+	f->fmt.pix = video->pix_fmt;
+
+	return 0;
+}
+
+static int aspeed_video_enum_input(struct file *file, void *fh,
+				   struct v4l2_input *inp)
+{
+	if (inp->index)
+		return -EINVAL;
+
+	strscpy(inp->name, "Host VGA capture", sizeof(inp->name));
+	inp->type = V4L2_INPUT_TYPE_CAMERA;
+	inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
+	inp->status = 0;
+
+	return 0;
+}
+
+static int aspeed_video_get_input(struct file *file, void *fh, unsigned int *i)
+{
+	*i = 0;
+
+	return 0;
+}
+
+static int aspeed_video_set_input(struct file *file, void *fh, unsigned int i)
+{
+	if (i)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int aspeed_video_get_parm(struct file *file, void *fh,
+				 struct v4l2_streamparm *a)
+{
+	struct aspeed_video *video = video_drvdata(file);
+
+	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+	a->parm.capture.readbuffers = 3;
+	a->parm.capture.timeperframe.numerator = 1;
+	if (!video->frame_rate)
+		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
+	else
+		a->parm.capture.timeperframe.denominator = video->frame_rate;
+
+	return 0;
+}
+
+static int aspeed_video_set_parm(struct file *file, void *fh,
+				 struct v4l2_streamparm *a)
+{
+	unsigned int frame_rate = 0;
+	struct aspeed_video *video = video_drvdata(file);
+
+	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+	a->parm.capture.readbuffers = 3;
+
+	if (a->parm.capture.timeperframe.numerator)
+		frame_rate = a->parm.capture.timeperframe.denominator /
+			a->parm.capture.timeperframe.numerator;
+
+	if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
+		frame_rate = 0;
+		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
+		a->parm.capture.timeperframe.numerator = 1;
+	}
+
+	if (video->frame_rate != frame_rate) {
+		video->frame_rate = frame_rate;
+		aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC,
+				    FIELD_PREP(VE_CTRL_FRC, frame_rate));
+	}
+
+	return 0;
+}
+
+static int aspeed_video_enum_framesizes(struct file *file, void *fh,
+					struct v4l2_frmsizeenum *fsize)
+{
+	struct aspeed_video *video = video_drvdata(file);
+
+	if (fsize->index)
+		return -EINVAL;
+
+	if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
+		return -EINVAL;
+
+	fsize->discrete.width = video->pix_fmt.width;
+	fsize->discrete.height = video->pix_fmt.height;
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+
+	return 0;
+}
+
+static int aspeed_video_enum_frameintervals(struct file *file, void *fh,
+					    struct v4l2_frmivalenum *fival)
+{
+	struct aspeed_video *video = video_drvdata(file);
+
+	if (fival->index)
+		return -EINVAL;
+
+	if (fival->width != video->timings.width ||
+	    fival->height != video->timings.height)
+		return -EINVAL;
+
+	if (fival->pixel_format != V4L2_PIX_FMT_JPEG)
+		return -EINVAL;
+
+	fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
+
+	fival->stepwise.min.denominator = MAX_FRAME_RATE;
+	fival->stepwise.min.numerator = 1;
+	fival->stepwise.max.denominator = 1;
+	fival->stepwise.max.numerator = 1;
+	fival->stepwise.step = fival->stepwise.max;
+
+	return 0;
+}
+
+static int aspeed_video_set_dv_timings(struct file *file, void *fh,
+				       struct v4l2_dv_timings *timings)
+{
+	int rc;
+	struct aspeed_video *video = video_drvdata(file);
+
+	if (video->capture_width == timings->bt.width &&
+	    video->capture_height == timings->bt.height)
+		return 0;
+
+	if (vb2_is_busy(&video->queue))
+		return -EBUSY;
+
+	if (video->timings.width != timings->bt.width ||
+	    video->timings.height != timings->bt.height)
+		return -EINVAL;
+
+	rc = aspeed_video_set_capture_resolution(video);
+	if (rc)
+		return rc;
+
+	video->pix_fmt.width = timings->bt.width;
+	video->pix_fmt.height = timings->bt.height;
+	video->pix_fmt.sizeimage = video->max_compressed_size;
+
+	timings->type = V4L2_DV_BT_656_1120;
+
+	return 0;
+}
+
+static int aspeed_video_get_dv_timings(struct file *file, void *fh,
+				       struct v4l2_dv_timings *timings)
+{
+	struct aspeed_video *video = video_drvdata(file);
+
+	timings->type = V4L2_DV_BT_656_1120;
+	timings->bt = video->timings;
+
+	return 0;
+}
+
+static int aspeed_video_query_dv_timings(struct file *file, void *fh,
+					 struct v4l2_dv_timings *timings)
+{
+	int rc;
+	struct aspeed_video *video = video_drvdata(file);
+
+	if (file->f_flags & O_NONBLOCK) {
+		if (test_bit(VIDEO_RES_CHANGE, &video->flags))
+			return -EAGAIN;
+	} else {
+		rc = wait_event_interruptible(video->wait,
+					      !test_bit(VIDEO_RES_CHANGE,
+							&video->flags));
+		if (rc)
+			return -EINTR;
+	}
+
+	timings->type = V4L2_DV_BT_656_1120;
+	timings->bt = video->timings;
+	timings->bt.width = video->timings.width;
+	timings->bt.height = video->timings.height;
+
+	return 0;
+}
+
+static int aspeed_video_enum_dv_timings(struct file *file, void *fh,
+					struct v4l2_enum_dv_timings *timings)
+{
+	if (timings->index)
+		return -EINVAL;
+
+	return aspeed_video_get_dv_timings(file, fh, &timings->timings);
+}
+
+static int aspeed_video_dv_timings_cap(struct file *file, void *fh,
+				       struct v4l2_dv_timings_cap *cap)
+{
+	cap->type = V4L2_DV_BT_656_1120;
+	cap->bt.capabilities = V4L2_DV_BT_CAP_PROGRESSIVE;
+	cap->bt.min_width = MIN_WIDTH;
+	cap->bt.max_width = MAX_WIDTH;
+	cap->bt.min_height = MIN_HEIGHT;
+	cap->bt.max_height = MAX_HEIGHT;
+
+	return 0;
+}
+
+static int aspeed_video_sub_event(struct v4l2_fh *fh,
+				  const struct v4l2_event_subscription *sub)
+{
+	switch (sub->type) {
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_src_change_event_subscribe(fh, sub);
+	}
+
+	return v4l2_ctrl_subscribe_event(fh, sub);
+}
+
+static const struct v4l2_ioctl_ops aspeed_video_ioctl_ops = {
+	.vidioc_querycap = aspeed_video_querycap,
+
+	.vidioc_enum_fmt_vid_cap = aspeed_video_enum_format,
+	.vidioc_g_fmt_vid_cap = aspeed_video_get_format,
+	.vidioc_s_fmt_vid_cap = aspeed_video_get_format,
+	.vidioc_try_fmt_vid_cap = aspeed_video_get_format,
+
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_qbuf = vb2_ioctl_qbuf,
+	.vidioc_dqbuf = vb2_ioctl_dqbuf,
+	.vidioc_create_bufs = vb2_ioctl_create_bufs,
+	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
+
+	.vidioc_enum_input = aspeed_video_enum_input,
+	.vidioc_g_input = aspeed_video_get_input,
+	.vidioc_s_input = aspeed_video_set_input,
+
+	.vidioc_g_parm = aspeed_video_get_parm,
+	.vidioc_s_parm = aspeed_video_set_parm,
+	.vidioc_enum_framesizes = aspeed_video_enum_framesizes,
+	.vidioc_enum_frameintervals = aspeed_video_enum_frameintervals,
+
+	.vidioc_s_dv_timings = aspeed_video_set_dv_timings,
+	.vidioc_g_dv_timings = aspeed_video_get_dv_timings,
+	.vidioc_query_dv_timings = aspeed_video_query_dv_timings,
+	.vidioc_enum_dv_timings = aspeed_video_enum_dv_timings,
+	.vidioc_dv_timings_cap = aspeed_video_dv_timings_cap,
+
+	.vidioc_subscribe_event = aspeed_video_sub_event,
+	.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_init_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,
+						  struct aspeed_video,
+						  ctrl_handler);
+
+	switch (ctrl->id) {
+	case V4L2_CID_JPEG_COMPRESSION_QUALITY:
+		video->jpeg_quality = ctrl->val;
+		aspeed_video_update_jpeg_quality(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);
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
+	.s_ctrl = aspeed_video_set_ctrl,
+};
+
+static void aspeed_video_resolution_work(struct work_struct *work)
+{
+	int rc;
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct aspeed_video *video = container_of(dwork, struct aspeed_video,
+						  res_work);
+
+	/* No clients remaining after delay */
+	if (atomic_read(&video->clients) == 0)
+		goto done;
+
+	aspeed_video_on(video);
+
+	aspeed_video_init_regs(video);
+
+	rc = aspeed_video_get_resolution(video);
+	if (rc)
+		dev_err(video->dev,
+			"resolution changed; couldn't get new resolution\n");
+
+	if (video->timings.width != video->pix_fmt.width ||
+	    video->timings.height != video->pix_fmt.height) {
+		static const struct v4l2_event ev = {
+			.type = V4L2_EVENT_SOURCE_CHANGE,
+			.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
+		};
+
+		v4l2_event_queue(&video->vdev, &ev);
+	}
+
+done:
+	clear_bit(VIDEO_RES_CHANGE, &video->flags);
+	wake_up_interruptible_all(&video->wait);
+}
+
+static int aspeed_video_open(struct file *file)
+{
+	int rc;
+	struct aspeed_video *video = video_drvdata(file);
+
+	mutex_lock(&video->video_lock);
+
+	if (atomic_inc_return(&video->clients) == 1) {
+		rc = aspeed_video_start(video);
+		if (rc) {
+			dev_err(video->dev, "Failed to start video engine\n");
+			atomic_dec(&video->clients);
+			mutex_unlock(&video->video_lock);
+			return rc;
+		}
+	}
+
+	mutex_unlock(&video->video_lock);
+
+	return v4l2_fh_open(file);
+}
+
+static int aspeed_video_release(struct file *file)
+{
+	int rc;
+	struct aspeed_video *video = video_drvdata(file);
+
+	rc = vb2_fop_release(file);
+
+	mutex_lock(&video->video_lock);
+
+	if (atomic_dec_return(&video->clients) == 0)
+		aspeed_video_stop(video);
+
+	mutex_unlock(&video->video_lock);
+
+	return rc;
+}
+
+static const struct v4l2_file_operations aspeed_video_v4l2_fops = {
+	.owner = THIS_MODULE,
+	.read = vb2_fop_read,
+	.poll = vb2_fop_poll,
+	.unlocked_ioctl = video_ioctl2,
+	.mmap = vb2_fop_mmap,
+	.open = aspeed_video_open,
+	.release = aspeed_video_release,
+};
+
+static int aspeed_video_queue_setup(struct vb2_queue *q,
+				    unsigned int *num_buffers,
+				    unsigned int *num_planes,
+				    unsigned int sizes[],
+				    struct device *alloc_devs[])
+{
+	struct aspeed_video *video = vb2_get_drv_priv(q);
+
+	if (*num_planes) {
+		if (sizes[0] < video->max_compressed_size)
+			return -EINVAL;
+
+		return 0;
+	}
+
+	*num_planes = 1;
+	sizes[0] = video->max_compressed_size;
+
+	return 0;
+}
+
+static int aspeed_video_buf_prepare(struct vb2_buffer *vb)
+{
+	struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
+
+	if (vb2_plane_size(vb, 0) < video->max_compressed_size)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int aspeed_video_start_streaming(struct vb2_queue *q,
+					unsigned int count)
+{
+	int rc;
+	struct aspeed_video *video = vb2_get_drv_priv(q);
+
+	rc = aspeed_video_start_frame(video);
+	if (rc) {
+		aspeed_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
+		return rc;
+	}
+
+	video->sequence = 0;
+	set_bit(VIDEO_STREAMING, &video->flags);
+	return 0;
+}
+
+static void aspeed_video_stop_streaming(struct vb2_queue *q)
+{
+	int rc;
+	struct aspeed_video *video = vb2_get_drv_priv(q);
+
+	clear_bit(VIDEO_STREAMING, &video->flags);
+
+	rc = wait_event_timeout(video->wait,
+				!test_bit(VIDEO_FRAME_INPRG, &video->flags),
+				STOP_TIMEOUT);
+	if (!rc) {
+		dev_err(video->dev, "Timed out when stopping streaming\n");
+		aspeed_video_stop(video);
+	}
+
+	aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
+}
+
+static void aspeed_video_buf_queue(struct vb2_buffer *vb)
+{
+	struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct aspeed_video_buffer *avb = to_aspeed_video_buffer(vbuf);
+	unsigned long flags;
+
+	spin_lock_irqsave(&video->lock, flags);
+	list_add_tail(&avb->link, &video->buffers);
+	spin_unlock_irqrestore(&video->lock, flags);
+}
+
+static const struct vb2_ops aspeed_video_vb2_ops = {
+	.queue_setup = aspeed_video_queue_setup,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
+	.buf_prepare = aspeed_video_buf_prepare,
+	.start_streaming = aspeed_video_start_streaming,
+	.stop_streaming = aspeed_video_stop_streaming,
+	.buf_queue =  aspeed_video_buf_queue,
+};
+
+static int aspeed_video_setup_video(struct aspeed_video *video)
+{
+	const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
+			   BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_420));
+	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
+	struct vb2_queue *vbq = &video->queue;
+	struct video_device *vdev = &video->vdev;
+	int rc;
+
+	video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
+	video->pix_fmt.field = V4L2_FIELD_NONE;
+	video->pix_fmt.colorspace = V4L2_COLORSPACE_SRGB;
+	video->pix_fmt.quantization = V4L2_QUANTIZATION_FULL_RANGE;
+
+	rc = v4l2_device_register(video->dev, v4l2_dev);
+	if (rc) {
+		dev_err(video->dev, "Failed to register v4l2 device\n");
+		return rc;
+	}
+
+	v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
+	v4l2_ctrl_new_std(&video->ctrl_handler, &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_CID_JPEG_CHROMA_SUBSAMPLING,
+			       V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
+			       V4L2_JPEG_CHROMA_SUBSAMPLING_444);
+
+	if (video->ctrl_handler.error) {
+		v4l2_ctrl_handler_free(&video->ctrl_handler);
+		v4l2_device_unregister(v4l2_dev);
+
+		dev_err(video->dev, "Failed to init controls: %d\n",
+			video->ctrl_handler.error);
+		return rc;
+	}
+
+	v4l2_dev->ctrl_handler = &video->ctrl_handler;
+
+	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
+	vbq->dev = v4l2_dev->dev;
+	vbq->lock = &video->video_lock;
+	vbq->ops = &aspeed_video_vb2_ops;
+	vbq->mem_ops = &vb2_dma_contig_memops;
+	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;
+
+	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;
+	}
+
+	vdev->queue = vbq;
+	vdev->fops = &aspeed_video_v4l2_fops;
+	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
+		V4L2_CAP_STREAMING;
+	vdev->v4l2_dev = v4l2_dev;
+	strscpy(vdev->name, DEVICE_NAME, sizeof(vdev->name));
+	vdev->vfl_type = VFL_TYPE_GRABBER;
+	vdev->vfl_dir = VFL_DIR_RX;
+	vdev->release = video_device_release_empty;
+	vdev->ioctl_ops = &aspeed_video_ioctl_ops;
+	vdev->lock = &video->video_lock;
+
+	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;
+	}
+
+	return 0;
+}
+
+static int aspeed_video_init(struct aspeed_video *video)
+{
+	int irq;
+	int rc;
+	struct device *dev = video->dev;
+
+	irq = irq_of_parse_and_map(dev->of_node, 0);
+	if (!irq) {
+		dev_err(dev, "Unable to find IRQ\n");
+		return -ENODEV;
+	}
+
+	rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED,
+			      DEVICE_NAME, video);
+	if (rc < 0) {
+		dev_err(dev, "Unable to request IRQ %d\n", irq);
+		return rc;
+	}
+
+	video->eclk = devm_clk_get(dev, "eclk");
+	if (IS_ERR(video->eclk)) {
+		dev_err(dev, "Unable to get ECLK\n");
+		return PTR_ERR(video->eclk);
+	}
+
+	video->vclk = devm_clk_get(dev, "vclk");
+	if (IS_ERR(video->vclk)) {
+		dev_err(dev, "Unable to get VCLK\n");
+		return PTR_ERR(video->vclk);
+	}
+
+	video->rst = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(video->rst)) {
+		dev_err(dev, "Unable to get VE reset\n");
+		return PTR_ERR(video->rst);
+	}
+
+	rc = of_reserved_mem_device_init(dev);
+	if (rc) {
+		dev_err(dev, "Unable to reserve memory\n");
+		return rc;
+	}
+
+	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	if (rc) {
+		dev_err(dev, "Failed to set DMA mask\n");
+		of_reserved_mem_device_release(dev);
+		return rc;
+	}
+
+	if (!aspeed_video_alloc_buf(video, &video->jpeg,
+				    VE_JPEG_HEADER_SIZE)) {
+		dev_err(dev, "Failed to allocate DMA for JPEG header\n");
+		of_reserved_mem_device_release(dev);
+		return rc;
+	}
+
+	aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
+
+	return 0;
+}
+
+static int aspeed_video_probe(struct platform_device *pdev)
+{
+	int rc;
+	struct resource *res;
+	struct aspeed_video *video = kzalloc(sizeof(*video), GFP_KERNEL);
+
+	if (!video)
+		return -ENOMEM;
+
+	video->frame_rate = 30;
+	video->dev = &pdev->dev;
+	mutex_init(&video->video_lock);
+	init_waitqueue_head(&video->wait);
+	INIT_DELAYED_WORK(&video->res_work, aspeed_video_resolution_work);
+	INIT_LIST_HEAD(&video->buffers);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	video->base = devm_ioremap_resource(video->dev, res);
+
+	if (IS_ERR(video->base))
+		return PTR_ERR(video->base);
+
+	rc = aspeed_video_init(video);
+	if (rc)
+		return rc;
+
+	rc = aspeed_video_setup_video(video);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int aspeed_video_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
+	struct aspeed_video *video = to_aspeed_video(v4l2_dev);
+
+	video_unregister_device(&video->vdev);
+
+	vb2_queue_release(&video->queue);
+
+	v4l2_ctrl_handler_free(&video->ctrl_handler);
+
+	v4l2_device_unregister(v4l2_dev);
+
+	dma_free_coherent(video->dev, VE_JPEG_HEADER_SIZE, video->jpeg.virt,
+			  video->jpeg.dma);
+
+	of_reserved_mem_device_release(dev);
+
+	return 0;
+}
+
+static const struct of_device_id aspeed_video_of_match[] = {
+	{ .compatible = "aspeed,ast2400-video-engine" },
+	{ .compatible = "aspeed,ast2500-video-engine" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, aspeed_video_of_match);
+
+static struct platform_driver aspeed_video_driver = {
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = aspeed_video_of_match,
+	},
+	.probe = aspeed_video_probe,
+	.remove = aspeed_video_remove,
+};
+
+module_platform_driver(aspeed_video_driver);
+
+MODULE_DESCRIPTION("ASPEED Video Engine Driver");
+MODULE_AUTHOR("Eddie James");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v5 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation
From: Eddie James @ 2018-11-12 21:00 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1542056431-18146-1-git-send-email-eajames@linux.ibm.com>

Document the bindings.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/media/aspeed-video.txt     | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt

diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt
new file mode 100644
index 0000000..78b464a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/aspeed-video.txt
@@ -0,0 +1,26 @@
+* Device tree bindings for Aspeed Video Engine
+
+The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can
+capture and compress video data from digital or analog sources.
+
+Required properties:
+ - compatible:		"aspeed,ast2400-video-engine" or
+			"aspeed,ast2500-video-engine"
+ - reg:			contains the offset and length of the VE memory region
+ - clocks:		clock specifiers for the syscon clocks associated with
+			the VE (ordering must match the clock-names property)
+ - clock-names:		"vclk" and "eclk"
+ - resets:		reset specifier for the syscon reset associated with
+			the VE
+ - interrupts:		the interrupt associated with the VE on this platform
+
+Example:
+
+video-engine at 1e700000 {
+    compatible = "aspeed,ast2500-video-engine";
+    reg = <0x1e700000 0x20000>;
+    clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>;
+    clock-names = "vclk", "eclk";
+    resets = <&syscon ASPEED_RESET_VIDEO>;
+    interrupts = <7>;
+};
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v5 0/2] media: platform: Add Aspeed Video Engine Driver
From: Eddie James @ 2018-11-12 21:00 UTC (permalink / raw)
  To: linux-aspeed

From: Eddie James <eajames@linux.vnet.ibm.com>

The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
can capture and compress video data from digital or analog sources. With
the Aspeed chip acting as a service processor, the Video Engine can
capture the host processor graphics output.

This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming
interface by way of videobuf2. Each frame, the driver triggers the hardware to
capture the host graphics output and compress it to JPEG format.

v4l-utils HEAD dd3ff81f58c4e1e6f33765dc61ad33c48ae6bb07

v4l2-compliance SHA: not available, 32 bits

Compliance test for device /dev/video0:

Driver Info:
	Driver name      : aspeed-video
	Card type        : Aspeed Video Engine
	Bus info         : platform:aspeed-video
	Driver version   : 4.18.12
	Capabilities     : 0x85200001
		Video Capture
		Read/Write
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x05200001
		Video Capture
		Read/Write
		Streaming
		Extended Pix Format

Required ioctls:
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/video0 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
	test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK
	test VIDIOC_DV_TIMINGS_CAP: OK
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
		fail: v4l2-test-controls.cpp(823): failed to find event for control 'Chroma Subsampling'
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 3 Private Controls: 0

Format ioctls (Input 0):
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK (Not Supported)

Codec ioctls (Input 0):
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
		fail: v4l2-test-buffers.cpp(422): dmabuf_valid ^ !!(caps & V4L2_BUF_CAP_SUPPORTS_DMABUF)
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
	test VIDIOC_EXPBUF: OK (Not Supported)

Test input 0:

Streaming ioctls:
	test read/write: OK
	test blocking wait: OK
	test MMAP: OK                                     
	test USERPTR: OK (Not Supported)
		fail: v4l2-test-buffers.cpp(1102): exp_q.reqbufs(expbuf_node, q.g_buffers())
		fail: v4l2-test-buffers.cpp(1192): setupDmaBuf(expbuf_node, node, q, exp_q)
	test DMABUF: FAIL

Total: 48, Succeeded: 45, Failed: 3, Warnings: 0

The apparent rate of change to the v4l2/vb2 API makes it difficult to pass
these tests. None of the failing tests even ran last time I submitted my
series. And V4L2_BUF_CAP_SUPPORTS_DMABUF is undefined in 4.19.

Changes since v4:
 - Set real min and max resolution in enum_dv_timings
 - Add check for vb2_is_busy before settings the timings
 - Set max frame rate to the actual max rather than max + 1
 - Correct the input status to 0.
 - Rework resolution change to only set the relevant h/w regs during startup or
   when set_timings is called.

Changes since v3:
 - Switch update reg function to use u32 clear rather than unsigned long mask
 - Add timing information from host VGA signal
 - Fix binding documentation mispelling
 - Fix upper case hex values
 - Add wait_prepare and wait_finish
 - Set buffer state to queued (rather than error) if streaming fails to start
 - Switch engine busy print statement to error
 - Removed a couple unecessary type assignments in v4l2 ioctls
 - Added query_dv_timings, fixed get_dv_timings
 - Corrected source change event to fire if and only if size actually changes
 - Locked open and release

Changes since v2:
 - Switch to streaming interface. This involved a lot of changes.
 - Rework memory allocation due to using videobuf2 buffers, but also only
   allocate the necessary size of source buffer rather than the max size

Changes since v1:
 - Removed le32_to_cpu calls for JPEG header data
 - Reworked v4l2 ioctls to be compliant.
 - Added JPEG controls
 - Updated devicetree docs according to Rob's suggestions.
 - Added myself to MAINTAINERS

Eddie James (2):
  dt-bindings: media: Add Aspeed Video Engine binding documentation
  media: platform: Add Aspeed Video Engine driver

 .../devicetree/bindings/media/aspeed-video.txt     |   26 +
 MAINTAINERS                                        |    8 +
 drivers/media/platform/Kconfig                     |    9 +
 drivers/media/platform/Makefile                    |    1 +
 drivers/media/platform/aspeed-video.c              | 1711 ++++++++++++++++++++
 5 files changed, 1755 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt
 create mode 100644 drivers/media/platform/aspeed-video.c

-- 
1.8.3.1


^ permalink raw reply

* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Greg Kroah-Hartman @ 2018-11-09 17:07 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>

On Tue, Oct 30, 2018 at 03:11:02PM -0700, Douglas Anderson wrote:
> I started out this series trying to make sysrq work over the serial
> console on qcom_geni_serial, then fell into a rat's nest.
> 
> To solve the deadlock I faced when enabling sysrq I tried to borrow
> code from '8250_port.c' which avoided grabbing the port lock in
> console_write().  ...but since these days I try to run with lockdep on
> all the time, I found it caused an annoying lockdep splat (which I
> also reproduced on my rk3399 board).  ...so I instead changed my
> qcom_geni_serial solution to borrow code from 'msm_serial.c'
> 
> I wasn't super happy with the solution in 'msm_serial.c' though.  I
> don't like releasing the spinlock there.  Not only is it ugly but it
> means we are unlocking / re-locking _all the time_ even though sysrq
> characters are rare.  ...so I came up with what I think is a better
> solution and then implemented it for qcom_geni_serial.
> 
> Since I had a good way to test 8250-based UARTs, I also fixed that
> driver to use my new method.  When doing so, I ran into a missing
> include in serial_core.h.  NOTE: I didn't have a way to test
> msm_serial.c at all, so I didn't switch that (or all other serial
> drivers for that matter) to the new method.
> 
> NOTE: from a serial point of view v2 is the same as v1 but I've
> removed the extra kgdb-related patches and made it obvious that this
> is really for all sysrq, not just kgdb.  I've also generally tried to
> curate the CCs more properly.

Looks good, thanks for cleaning this up.  All now queued up.

greg k-h

^ permalink raw reply

* [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
From: Hans Verkuil @ 2018-11-09 10:54 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <7027cd38-b8f5-08b2-0536-eed1c6a0516b@linux.vnet.ibm.com>

Hi Eddie,

Sorry for the delay, I've been away for the past two weeks and I've been
catching up ever since I came back.

On 11/08/18 16:48, Eddie James wrote:
> 
> 
> On 10/19/2018 03:26 PM, Eddie James wrote:
>>
>>
>> On 10/12/2018 07:22 AM, Hans Verkuil wrote:
>>> On 10/05/2018 09:57 PM, Eddie James wrote:
>>>> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
>>>> can capture and compress video data from digital or analog sources. 
>>>> With
>>>> the Aspeed chip acting a service processor, the Video Engine can 
>>>> capture
>>>> the host processor graphics output.
>>>>
>>>> Add a V4L2 driver to capture video data and compress it to JPEG images.
>>>> Make the video frames available through the V4L2 streaming interface.
>>>>
>>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>>> ---
>>>>   MAINTAINERS                           |    8 +
>>>>   drivers/media/platform/Kconfig        |    8 +
>>>>   drivers/media/platform/Makefile       |    1 +
>>>>   drivers/media/platform/aspeed-video.c | 1674 
>>>> +++++++++++++++++++++++++++++++++
>>>>   4 files changed, 1691 insertions(+)
>>>>   create mode 100644 drivers/media/platform/aspeed-video.c
>>>>
>>> <snip>
>>>
>>>> +static int aspeed_video_enum_input(struct file *file, void *fh,
>>>> +                   struct v4l2_input *inp)
>>>> +{
>>>> +    if (inp->index)
>>>> +        return -EINVAL;
>>>> +
>>>> +    strscpy(inp->name, "Host VGA capture", sizeof(inp->name));
>>>> +    inp->type = V4L2_INPUT_TYPE_CAMERA;
>>>> +    inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
>>>> +    inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC;
>>> This can't be right. If there is a valid signal, then status should 
>>> be 0.
>>> And ideally you can tell the difference between no signal and no sync
>>> as well.
>>>
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int aspeed_video_get_input(struct file *file, void *fh, 
>>>> unsigned int *i)
>>>> +{
>>>> +    *i = 0;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int aspeed_video_set_input(struct file *file, void *fh, 
>>>> unsigned int i)
>>>> +{
>>>> +    if (i)
>>>> +        return -EINVAL;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int aspeed_video_get_parm(struct file *file, void *fh,
>>>> +                 struct v4l2_streamparm *a)
>>>> +{
>>>> +    struct aspeed_video *video = video_drvdata(file);
>>>> +
>>>> +    a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
>>>> +    a->parm.capture.readbuffers = 3;
>>>> +    a->parm.capture.timeperframe.numerator = 1;
>>>> +    if (!video->frame_rate)
>>>> +        a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
>>>> +    else
>>>> +        a->parm.capture.timeperframe.denominator = video->frame_rate;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int aspeed_video_set_parm(struct file *file, void *fh,
>>>> +                 struct v4l2_streamparm *a)
>>>> +{
>>>> +    unsigned int frame_rate = 0;
>>>> +    struct aspeed_video *video = video_drvdata(file);
>>>> +
>>>> +    a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
>>>> +    a->parm.capture.readbuffers = 3;
>>>> +
>>>> +    if (a->parm.capture.timeperframe.numerator)
>>>> +        frame_rate = a->parm.capture.timeperframe.denominator /
>>>> +            a->parm.capture.timeperframe.numerator;
>>>> +
>>>> +    if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
>>>> +        frame_rate = 0;
>>>> +
>>>> +        /*
>>>> +         * Set to max + 1 to differentiate between max and 0, which
>>>> +         * means "don't care".
>>> But what does "don't care" mean in practice? It's still not clear to 
>>> me how this
>>> is supposed to work.
>>>
>>>> +         */
>>>> +        a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
>>> And regardless of anything else this timeperframe is out of the range 
>>> that
>>> aspeed_video_enum_frameintervals() returns.
>>>
>>>> + a->parm.capture.timeperframe.numerator = 1;
>>>> +    }
>>>> +
>>>> +    if (video->frame_rate != frame_rate) {
>>>> +        video->frame_rate = frame_rate;
>>>> +        aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC,
>>>> +                    FIELD_PREP(VE_CTRL_FRC, frame_rate));
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int aspeed_video_enum_framesizes(struct file *file, void *fh,
>>>> +                    struct v4l2_frmsizeenum *fsize)
>>>> +{
>>>> +    struct aspeed_video *video = video_drvdata(file);
>>>> +
>>>> +    if (fsize->index)
>>>> +        return -EINVAL;
>>>> +
>>>> +    if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
>>>> +        return -EINVAL;
>>>> +
>>>> +    fsize->discrete.width = video->pix_fmt.width;
>>>> +    fsize->discrete.height = video->pix_fmt.height;
>>>> +    fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int aspeed_video_enum_frameintervals(struct file *file, void 
>>>> *fh,
>>>> +                        struct v4l2_frmivalenum *fival)
>>>> +{
>>>> +    struct aspeed_video *video = video_drvdata(file);
>>>> +
>>>> +    if (fival->index)
>>>> +        return -EINVAL;
>>>> +
>>>> +    if (fival->width != video->width || fival->height != 
>>>> video->height)
>>>> +        return -EINVAL;
>>>> +
>>>> +    if (fival->pixel_format != V4L2_PIX_FMT_JPEG)
>>>> +        return -EINVAL;
>>>> +
>>>> +    fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
>>>> +
>>>> +    fival->stepwise.min.denominator = MAX_FRAME_RATE;
>>>> +    fival->stepwise.min.numerator = 1;
>>>> +    fival->stepwise.max.denominator = 1;
>>>> +    fival->stepwise.max.numerator = 1;
>>>> +    fival->stepwise.step = fival->stepwise.max;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int aspeed_video_set_dv_timings(struct file *file, void *fh,
>>>> +                       struct v4l2_dv_timings *timings)
>>>> +{
>>>> +    struct aspeed_video *video = video_drvdata(file);
>>>> +
>>> If vb2_is_busy() returns true, then return -EBUSY here. It is not 
>>> allowed to
>>> set the timings while vb2 is busy.
>>
>> Buffer ioctls (Input 0):
>>         fail: v4l2-test-buffers.cpp(344): node->s_dv_timings(timings)
>>         fail: v4l2-test-buffers.cpp(452): testCanSetSameTimings(node)
>>     test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
>>
>> Streaming ioctls:
>>     test read/write: OK
>>     test blocking wait: OK
>>         fail: v4l2-test-buffers.cpp(344): node->s_dv_timings(timings)
>>         fail: v4l2-test-buffers.cpp(637): testCanSetSameTimings(node)
>>         fail: v4l2-test-buffers.cpp(952): captureBufs(node, q, m2m_q, 
>> frame_count, false)
>>     test MMAP: FAIL
>>
>> Built from v4l-utils c36dbbdfa8b30b2badd4f893b59d0bd4f0bd12aa
>>
>> Adding this causes some of the v4l2-compliance streaming tests to 
>> fail, and prevents my own application from being able to call 
>> S_DV_TIMINGS after detecting a resolution change, despite calling 
>> streamoff and unmapping all the buffers first.
> 
> Any thoughts on this Hans?

The compliance fail is because there is a requirement that if you set
new timings that are equal to the existing timings, then that should
work (and is effectively a NOP operation).

So in the code you can do something similar to what vivid does:

        if (v4l2_match_dv_timings(timings, &dev->dv_timings_cap, 0, false))
                return 0;
        if (vb2_is_busy(&dev->vb_vid_cap_q))
                return -EBUSY;

You can probably get away with just checking width, height and interlaced
in this driver, rather than calling v4l2_match_dv_timings.

If the resolution changes, however, then you need to completely free all
buffers (i.e. close the fd or call VIDIOC_REQBUFS with count = 0) before
you can set new timings. So streamoff + unmap is not enough.

Streamoff only returns the buffers to userspace, it doesn't free them.

Regards,

	Hans

^ permalink raw reply

* [PATCH v3 2/2] ARM: dts: aspeed: Add Facebook Backpack-CMM BMC
From: Tao Ren @ 2018-11-09  0:50 UTC (permalink / raw)
  To: linux-aspeed

Add initial version of device tree file for Facebook Backpack CMM
(Chasis Management Module) ast2500 BMC.

Signed-off-by: Tao Ren <taoren@fb.com>
---
 Changes in v3:
   - including pin configurations in UARTs 1-4 device nodes explicitly.
   - including external pinctrl controller nodes gfx and lhc explicitly.
   - deleting misleading i2c-related statements from commit messages, and
     adding comments for i2c buses in the board device tree file.
 Changes in v2:
   - including "aspeed-bmc-facebook-cmm.dtb" in Makefile.

 arch/arm/boot/dts/Makefile                    |   1 +
 arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts | 368 ++++++++++++++++++
 2 files changed, 369 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index a7c313bfe490..2239e6a30d7a 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1200,6 +1200,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
 	aspeed-ast2500-evb.dtb \
 	aspeed-bmc-arm-centriq2400-rep.dtb \
 	aspeed-bmc-arm-stardragon4800-rep2.dtb \
+	aspeed-bmc-facebook-cmm.dtb \
 	aspeed-bmc-facebook-tiogapass.dtb \
 	aspeed-bmc-intel-s2600wf.dtb \
 	aspeed-bmc-opp-lanyang.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
new file mode 100644
index 000000000000..9f194b5eeba4
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2018 Facebook Inc.
+/dts-v1/;
+
+#include "aspeed-g5.dtsi"
+
+/ {
+	model = "Facebook Backpack CMM BMC";
+	compatible = "facebook,cmm-bmc", "aspeed,ast2500";
+
+	aliases {
+		/*
+		 * Override the default uart aliases to avoid breaking
+		 * the legacy applications.
+		 */
+		serial0 = &uart5;
+		serial1 = &uart1;
+		serial2 = &uart3;
+		serial3 = &uart4;
+
+		/*
+		 * Hardcode the bus number of i2c switches' channels to
+		 * avoid breaking the legacy applications.
+		 */
+		i2c16 = &imux16;
+		i2c17 = &imux17;
+		i2c18 = &imux18;
+		i2c19 = &imux19;
+		i2c20 = &imux20;
+		i2c21 = &imux21;
+		i2c22 = &imux22;
+		i2c23 = &imux23;
+		i2c24 = &imux24;
+		i2c25 = &imux25;
+		i2c26 = &imux26;
+		i2c27 = &imux27;
+		i2c28 = &imux28;
+		i2c29 = &imux29;
+		i2c30 = &imux30;
+		i2c31 = &imux31;
+		i2c32 = &imux32;
+		i2c33 = &imux33;
+		i2c34 = &imux34;
+		i2c35 = &imux35;
+		i2c36 = &imux36;
+		i2c37 = &imux37;
+		i2c38 = &imux38;
+		i2c39 = &imux39;
+	};
+
+	chosen {
+		stdout-path = &uart1;
+		bootargs = "console=ttyS1,9600n8 root=/dev/ram rw earlyprintk";
+	};
+
+	memory at 80000000 {
+		reg = <0x80000000 0x20000000>;
+	};
+};
+
+&pinctrl {
+	aspeed,external-nodes = <&gfx &lhc>;
+};
+
+/*
+ * Update reset type to "system" (full chip) to fix warm reboot hang issue
+ * when reset type is set to default ("soc", gated by reset mask registers).
+ */
+&wdt1 {
+	status = "okay";
+	aspeed,reset-type = "system";
+};
+
+/*
+ * wdt2 is not used by Backpack CMM.
+ */
+&wdt2 {
+	status = "disabled";
+};
+
+&fmc {
+	status = "okay";
+	flash at 0 {
+		status = "okay";
+		m25p,fast-read;
+		label = "bmc";
+#include "facebook-bmc-flash-layout.dtsi"
+	};
+};
+
+&uart1 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_txd1_default
+		     &pinctrl_rxd1_default
+		     &pinctrl_ncts1_default
+		     &pinctrl_ndcd1_default
+		     &pinctrl_ndsr1_default
+		     &pinctrl_ndtr1_default
+		     &pinctrl_nrts1_default>;
+};
+
+&uart3 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_txd3_default
+		     &pinctrl_rxd3_default
+		     &pinctrl_ncts3_default
+		     &pinctrl_ndcd3_default
+		     &pinctrl_nri3_default>;
+};
+
+&uart4 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_txd4_default
+		     &pinctrl_rxd4_default>;
+};
+
+&uart5 {
+	status = "okay";
+};
+
+&mac1 {
+	status = "okay";
+	no-hw-checksum;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
+};
+
+/*
+ * I2C bus reserved for communication with COM-E.
+ */
+&i2c0 {
+	status = "okay";
+};
+
+/*
+ * I2C bus to Line Cards and Fabric Cards.
+ */
+&i2c1 {
+	status = "okay";
+
+	i2c-switch at 77 {
+		compatible = "nxp,pca9548";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x77>;
+
+		imux16: i2c at 0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+		};
+
+		imux17: i2c at 1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+		};
+
+		imux18: i2c at 2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <2>;
+		};
+
+		imux19: i2c at 3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <3>;
+		};
+
+		imux20: i2c at 4 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <4>;
+		};
+
+		imux21: i2c at 5 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <5>;
+		};
+
+		imux22: i2c at 6 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <6>;
+		};
+
+		imux23: i2c at 7 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <7>;
+		};
+	};
+};
+
+/*
+ * I2C bus to Power Distribution Board.
+ */
+&i2c2 {
+	status = "okay";
+
+	i2c-switch at 71 {
+		compatible = "nxp,pca9548";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x71>;
+
+		imux24: i2c at 0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+		};
+
+		imux25: i2c at 1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+		};
+
+		imux26: i2c at 2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <2>;
+		};
+
+		imux27: i2c at 3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <3>;
+		};
+
+		imux28: i2c at 4 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <4>;
+		};
+
+		imux29: i2c at 5 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <5>;
+		};
+
+		imux30: i2c at 6 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <6>;
+		};
+
+		imux31: i2c at 7 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <7>;
+		};
+	};
+};
+
+/*
+ * I2c bus connected with temperature sensors on CMM.
+ */
+&i2c3 {
+	status = "okay";
+};
+
+/*
+ * I2C bus reserved for communication with COM-E.
+ */
+&i2c4 {
+	status = "okay";
+};
+
+/*
+ * I2c bus connected with ADM1278.
+ */
+&i2c5 {
+	status = "okay";
+};
+
+/*
+ * I2c bus connected with I/O Expander.
+ */
+&i2c6 {
+	status = "okay";
+};
+
+/*
+ * I2c bus connected with I/O Expander and EPROMs.
+ */
+&i2c7 {
+	status = "okay";
+};
+
+/*
+ * I2C bus to Fan Control Board.
+ */
+&i2c8 {
+	status = "okay";
+
+	i2c-switch at 77 {
+		compatible = "nxp,pca9548";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x77>;
+
+		imux32: i2c at 0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+		};
+
+		imux33: i2c at 1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+		};
+
+		imux34: i2c at 2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <2>;
+		};
+
+		imux35: i2c at 3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <3>;
+		};
+
+		imux36: i2c at 4 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <4>;
+		};
+
+		imux37: i2c at 5 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <5>;
+		};
+
+		imux38: i2c at 6 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <6>;
+		};
+
+		imux39: i2c at 7 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <7>;
+		};
+	};
+};
+
+/*
+ * I2C bus to CMM CPLD.
+ */
+&i2c13 {
+	status = "okay";
+};
+
+&adc {
+	status = "okay";
+};
-- 
2.17.1


^ permalink raw reply related

* [PATCH v3 1/2] ARM: dts: Add Facebook BMC flash layout
From: Tao Ren @ 2018-11-09  0:50 UTC (permalink / raw)
  To: linux-aspeed

This is the layout used by Facebook BMC systems. It describes the fixed
flash layout of a 32MB mtd device.

Signed-off-by: Tao Ren <taoren@fb.com>
---
 Changes in v3:
   - adding comments explaining why "data0" and "flash0" partitions are
     needed.
 Changes in v2:
   - adding facebook copyright.

 .../boot/dts/facebook-bmc-flash-layout.dtsi   | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi

diff --git a/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi
new file mode 100644
index 000000000000..87bb8b576250
--- /dev/null
+++ b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2018 Facebook Inc.
+
+partitions {
+	compatible = "fixed-partitions";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	u-boot at 0 {
+		reg = <0x0 0x60000>;
+		label = "u-boot";
+	};
+
+	u-boot-env at 60000 {
+		reg = <0x60000 0x20000>;
+		label = "env";
+	};
+
+	fit at 80000 {
+		reg = <0x80000 0x1b80000>;
+		label = "fit";
+	};
+
+	/*
+	 * "data0" partition is used by several Facebook BMC platforms
+	 * as persistent data store.
+	 */
+	data0 at 1c00000 {
+		reg = <0x1c00000 0x400000>;
+		label = "data0";
+	};
+
+	/*
+	 * Although the master partition can be created by enabling
+	 * MTD_PARTITIONED_MASTER option, below "flash0" partition is
+	 * explicitly created to avoid breaking legacy applications.
+	 */
+	flash0 at 0 {
+		reg = <0x0 0x2000000>;
+		label = "flash0";
+	};
+};
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
From: Eddie James @ 2018-11-08 15:48 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <b64d0a4b-f74a-e887-366d-c242ac3f0d1c@linux.vnet.ibm.com>



On 10/19/2018 03:26 PM, Eddie James wrote:
>
>
> On 10/12/2018 07:22 AM, Hans Verkuil wrote:
>> On 10/05/2018 09:57 PM, Eddie James wrote:
>>> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
>>> can capture and compress video data from digital or analog sources. 
>>> With
>>> the Aspeed chip acting a service processor, the Video Engine can 
>>> capture
>>> the host processor graphics output.
>>>
>>> Add a V4L2 driver to capture video data and compress it to JPEG images.
>>> Make the video frames available through the V4L2 streaming interface.
>>>
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>> ---
>>> ? MAINTAINERS?????????????????????????? |??? 8 +
>>> ? drivers/media/platform/Kconfig??????? |??? 8 +
>>> ? drivers/media/platform/Makefile?????? |??? 1 +
>>> ? drivers/media/platform/aspeed-video.c | 1674 
>>> +++++++++++++++++++++++++++++++++
>>> ? 4 files changed, 1691 insertions(+)
>>> ? create mode 100644 drivers/media/platform/aspeed-video.c
>>>
>> <snip>
>>
>>> +static int aspeed_video_enum_input(struct file *file, void *fh,
>>> +?????????????????? struct v4l2_input *inp)
>>> +{
>>> +??? if (inp->index)
>>> +??????? return -EINVAL;
>>> +
>>> +??? strscpy(inp->name, "Host VGA capture", sizeof(inp->name));
>>> +??? inp->type = V4L2_INPUT_TYPE_CAMERA;
>>> +??? inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
>>> +??? inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC;
>> This can't be right. If there is a valid signal, then status should 
>> be 0.
>> And ideally you can tell the difference between no signal and no sync
>> as well.
>>
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_get_input(struct file *file, void *fh, 
>>> unsigned int *i)
>>> +{
>>> +??? *i = 0;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_set_input(struct file *file, void *fh, 
>>> unsigned int i)
>>> +{
>>> +??? if (i)
>>> +??????? return -EINVAL;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_get_parm(struct file *file, void *fh,
>>> +???????????????? struct v4l2_streamparm *a)
>>> +{
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
>>> +??? a->parm.capture.readbuffers = 3;
>>> +??? a->parm.capture.timeperframe.numerator = 1;
>>> +??? if (!video->frame_rate)
>>> +??????? a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
>>> +??? else
>>> +??????? a->parm.capture.timeperframe.denominator = video->frame_rate;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_set_parm(struct file *file, void *fh,
>>> +???????????????? struct v4l2_streamparm *a)
>>> +{
>>> +??? unsigned int frame_rate = 0;
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
>>> +??? a->parm.capture.readbuffers = 3;
>>> +
>>> +??? if (a->parm.capture.timeperframe.numerator)
>>> +??????? frame_rate = a->parm.capture.timeperframe.denominator /
>>> +??????????? a->parm.capture.timeperframe.numerator;
>>> +
>>> +??? if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
>>> +??????? frame_rate = 0;
>>> +
>>> +??????? /*
>>> +???????? * Set to max + 1 to differentiate between max and 0, which
>>> +???????? * means "don't care".
>> But what does "don't care" mean in practice? It's still not clear to 
>> me how this
>> is supposed to work.
>>
>>> +???????? */
>>> +??????? a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
>> And regardless of anything else this timeperframe is out of the range 
>> that
>> aspeed_video_enum_frameintervals() returns.
>>
>>> + a->parm.capture.timeperframe.numerator = 1;
>>> +??? }
>>> +
>>> +??? if (video->frame_rate != frame_rate) {
>>> +??????? video->frame_rate = frame_rate;
>>> +??????? aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC,
>>> +??????????????????? FIELD_PREP(VE_CTRL_FRC, frame_rate));
>>> +??? }
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_enum_framesizes(struct file *file, void *fh,
>>> +??????????????????? struct v4l2_frmsizeenum *fsize)
>>> +{
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? if (fsize->index)
>>> +??????? return -EINVAL;
>>> +
>>> +??? if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
>>> +??????? return -EINVAL;
>>> +
>>> +??? fsize->discrete.width = video->pix_fmt.width;
>>> +??? fsize->discrete.height = video->pix_fmt.height;
>>> +??? fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_enum_frameintervals(struct file *file, void 
>>> *fh,
>>> +??????????????????????? struct v4l2_frmivalenum *fival)
>>> +{
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? if (fival->index)
>>> +??????? return -EINVAL;
>>> +
>>> +??? if (fival->width != video->width || fival->height != 
>>> video->height)
>>> +??????? return -EINVAL;
>>> +
>>> +??? if (fival->pixel_format != V4L2_PIX_FMT_JPEG)
>>> +??????? return -EINVAL;
>>> +
>>> +??? fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
>>> +
>>> +??? fival->stepwise.min.denominator = MAX_FRAME_RATE;
>>> +??? fival->stepwise.min.numerator = 1;
>>> +??? fival->stepwise.max.denominator = 1;
>>> +??? fival->stepwise.max.numerator = 1;
>>> +??? fival->stepwise.step = fival->stepwise.max;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_set_dv_timings(struct file *file, void *fh,
>>> +?????????????????????? struct v4l2_dv_timings *timings)
>>> +{
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>> If vb2_is_busy() returns true, then return -EBUSY here. It is not 
>> allowed to
>> set the timings while vb2 is busy.
>
> Buffer ioctls (Input 0):
> ??? ??? fail: v4l2-test-buffers.cpp(344): node->s_dv_timings(timings)
> ??? ??? fail: v4l2-test-buffers.cpp(452): testCanSetSameTimings(node)
> ??? test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
>
> Streaming ioctls:
> ??? test read/write: OK
> ??? test blocking wait: OK
> ??? ??? fail: v4l2-test-buffers.cpp(344): node->s_dv_timings(timings)
> ??? ??? fail: v4l2-test-buffers.cpp(637): testCanSetSameTimings(node)
> ??? ??? fail: v4l2-test-buffers.cpp(952): captureBufs(node, q, m2m_q, 
> frame_count, false)
> ??? test MMAP: FAIL
>
> Built from v4l-utils c36dbbdfa8b30b2badd4f893b59d0bd4f0bd12aa
>
> Adding this causes some of the v4l2-compliance streaming tests to 
> fail, and prevents my own application from being able to call 
> S_DV_TIMINGS after detecting a resolution change, despite calling 
> streamoff and unmapping all the buffers first.

Any thoughts on this Hans?

Thanks,
Eddie

>
> Thanks,
> Eddie
>
>>
>>> +??? if (video->width != timings->bt.width ||
>>> +??????? video->height != timings->bt.height)
>>> +??????? return -EINVAL;
>>> +
>>> +??? video->pix_fmt.width = timings->bt.width;
>>> +??? video->pix_fmt.height = timings->bt.height;
>>> +??? video->pix_fmt.sizeimage = video->max_compressed_size;
>>> +??? video->timings.width = timings->bt.width;
>>> +??? video->timings.height = timings->bt.height;
>>> +
>>> +??? timings->type = V4L2_DV_BT_656_1120;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_get_dv_timings(struct file *file, void *fh,
>>> +?????????????????????? struct v4l2_dv_timings *timings)
>>> +{
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? timings->type = V4L2_DV_BT_656_1120;
>>> +??? timings->bt = video->timings;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_query_dv_timings(struct file *file, void *fh,
>>> +???????????????????? struct v4l2_dv_timings *timings)
>>> +{
>>> +??? int rc;
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? if (file->f_flags & O_NONBLOCK) {
>>> +??????? if (test_bit(VIDEO_RES_CHANGE, &video->flags))
>>> +??????????? return -EAGAIN;
>>> +??? } else {
>>> +??????? rc = wait_event_interruptible(video->wait,
>>> +????????????????????????? !test_bit(VIDEO_RES_CHANGE,
>>> +??????????????????????????? &video->flags));
>>> +??????? if (rc)
>>> +??????????? return -EINTR;
>>> +??? }
>>> +
>>> +??? timings->type = V4L2_DV_BT_656_1120;
>>> +??? timings->bt = video->timings;
>>> +??? timings->bt.width = video->width;
>>> +??? timings->bt.height = video->height;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_enum_dv_timings(struct file *file, void *fh,
>>> +??????????????????? struct v4l2_enum_dv_timings *timings)
>>> +{
>>> +??? if (timings->index)
>>> +??????? return -EINVAL;
>>> +
>>> +??? return aspeed_video_get_dv_timings(file, fh, &timings->timings);
>>> +}
>>> +
>>> +static int aspeed_video_dv_timings_cap(struct file *file, void *fh,
>>> +?????????????????????? struct v4l2_dv_timings_cap *cap)
>>> +{
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? cap->type = V4L2_DV_BT_656_1120;
>>> +??? cap->bt.capabilities = V4L2_DV_BT_CAP_PROGRESSIVE;
>>> +??? cap->bt.min_width = video->width;
>>> +??? cap->bt.max_width = video->width;
>>> +??? cap->bt.min_height = video->height;
>>> +??? cap->bt.max_height = video->height;
>> This should return the capabilities of the aspeed. In this case I'd
>> guess that the max width/height is 1920x1080 (or perhaps 1200).
>>
>> The minimum is probably the VGA resolution.
>>
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_sub_event(struct v4l2_fh *fh,
>>> +????????????????? const struct v4l2_event_subscription *sub)
>>> +{
>>> +??? switch (sub->type) {
>>> +??? case V4L2_EVENT_SOURCE_CHANGE:
>>> +??????? return v4l2_src_change_event_subscribe(fh, sub);
>>> +??? }
>>> +
>>> +??? return v4l2_ctrl_subscribe_event(fh, sub);
>>> +}
>>> +
>>> +static const struct v4l2_ioctl_ops aspeed_video_ioctl_ops = {
>>> +??? .vidioc_querycap = aspeed_video_querycap,
>>> +
>>> +??? .vidioc_enum_fmt_vid_cap = aspeed_video_enum_format,
>>> +??? .vidioc_g_fmt_vid_cap = aspeed_video_get_format,
>>> +??? .vidioc_s_fmt_vid_cap = aspeed_video_get_format,
>>> +??? .vidioc_try_fmt_vid_cap = aspeed_video_get_format,
>>> +
>>> +??? .vidioc_reqbufs = vb2_ioctl_reqbufs,
>>> +??? .vidioc_querybuf = vb2_ioctl_querybuf,
>>> +??? .vidioc_qbuf = vb2_ioctl_qbuf,
>>> +??? .vidioc_dqbuf = vb2_ioctl_dqbuf,
>>> +??? .vidioc_create_bufs = vb2_ioctl_create_bufs,
>>> +??? .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
>>> +??? .vidioc_streamon = vb2_ioctl_streamon,
>>> +??? .vidioc_streamoff = vb2_ioctl_streamoff,
>>> +
>>> +??? .vidioc_enum_input = aspeed_video_enum_input,
>>> +??? .vidioc_g_input = aspeed_video_get_input,
>>> +??? .vidioc_s_input = aspeed_video_set_input,
>>> +
>>> +??? .vidioc_g_parm = aspeed_video_get_parm,
>>> +??? .vidioc_s_parm = aspeed_video_set_parm,
>>> +??? .vidioc_enum_framesizes = aspeed_video_enum_framesizes,
>>> +??? .vidioc_enum_frameintervals = aspeed_video_enum_frameintervals,
>>> +
>>> +??? .vidioc_s_dv_timings = aspeed_video_set_dv_timings,
>>> +??? .vidioc_g_dv_timings = aspeed_video_get_dv_timings,
>>> +??? .vidioc_query_dv_timings = aspeed_video_query_dv_timings,
>>> +??? .vidioc_enum_dv_timings = aspeed_video_enum_dv_timings,
>>> +??? .vidioc_dv_timings_cap = aspeed_video_dv_timings_cap,
>>> +
>>> +??? .vidioc_subscribe_event = aspeed_video_sub_event,
>>> +??? .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_init_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,
>>> +????????????????????????? struct aspeed_video,
>>> +????????????????????????? ctrl_handler);
>>> +
>>> +??? switch (ctrl->id) {
>>> +??? case V4L2_CID_JPEG_COMPRESSION_QUALITY:
>>> +??????? video->jpeg_quality = ctrl->val;
>>> +??????? aspeed_video_update_jpeg_quality(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);
>>> +??????? }
>>> +??????? break;
>>> +??? default:
>>> +??????? return -EINVAL;
>>> +??? }
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
>>> +??? .s_ctrl = aspeed_video_set_ctrl,
>>> +};
>>> +
>>> +static void aspeed_video_resolution_work(struct work_struct *work)
>>> +{
>>> +??? int rc;
>>> +??? struct delayed_work *dwork = to_delayed_work(work);
>>> +??? struct aspeed_video *video = container_of(dwork, struct 
>>> aspeed_video,
>>> +????????????????????????? res_work);
>>> +
>>> +??? /* No clients remaining after delay */
>>> +??? if (atomic_read(&video->clients) == 0)
>>> +??????? goto done;
>>> +
>>> +??? aspeed_video_on(video);
>>> +
>>> +??? aspeed_video_init_regs(video);
>>> +
>>> +??? rc = aspeed_video_get_resolution(video);
>>> +??? if (rc)
>>> +??????? dev_err(video->dev,
>>> +??????????? "resolution changed; couldn't get new resolution\n");
>>> +??? else if (test_bit(VIDEO_STREAMING, &video->flags))
>>> +??????? aspeed_video_start_frame(video);
>>> +
>>> +??? if (video->width != video->pix_fmt.width ||
>>> +??????? video->height != video->pix_fmt.height) {
>>> +??????? static const struct v4l2_event ev = {
>>> +??????????? .type = V4L2_EVENT_SOURCE_CHANGE,
>>> +??????????? .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
>>> +??????? };
>>> +
>>> +??????? v4l2_event_queue(&video->vdev, &ev);
>>> +??? }
>>> +
>>> +done:
>>> +??? clear_bit(VIDEO_RES_CHANGE, &video->flags);
>>> +??? wake_up_interruptible_all(&video->wait);
>>> +}
>>> +
>>> +static int aspeed_video_open(struct file *file)
>>> +{
>>> +??? int rc;
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? mutex_lock(&video->video_lock);
>>> +
>>> +??? if (atomic_inc_return(&video->clients) == 1) {
>>> +??????? rc = aspeed_video_start(video);
>>> +??????? if (rc) {
>>> +??????????? dev_err(video->dev, "Failed to start video engine\n");
>>> +??????????? atomic_dec(&video->clients);
>>> +??????????? mutex_unlock(&video->video_lock);
>>> +??????????? return rc;
>>> +??????? }
>>> +??? }
>>> +
>>> +??? mutex_unlock(&video->video_lock);
>>> +
>>> +??? return v4l2_fh_open(file);
>>> +}
>>> +
>>> +static int aspeed_video_release(struct file *file)
>>> +{
>>> +??? int rc;
>>> +??? struct aspeed_video *video = video_drvdata(file);
>>> +
>>> +??? rc = vb2_fop_release(file);
>>> +
>>> +??? mutex_lock(&video->video_lock);
>>> +
>>> +??? if (atomic_dec_return(&video->clients) == 0)
>>> +??????? aspeed_video_stop(video);
>>> +
>>> +??? mutex_unlock(&video->video_lock);
>>> +
>>> +??? return rc;
>>> +}
>>> +
>>> +static const struct v4l2_file_operations aspeed_video_v4l2_fops = {
>>> +??? .owner = THIS_MODULE,
>>> +??? .read = vb2_fop_read,
>>> +??? .poll = vb2_fop_poll,
>>> +??? .unlocked_ioctl = video_ioctl2,
>>> +??? .mmap = vb2_fop_mmap,
>>> +??? .open = aspeed_video_open,
>>> +??? .release = aspeed_video_release,
>>> +};
>>> +
>>> +static int aspeed_video_queue_setup(struct vb2_queue *q,
>>> +??????????????????? unsigned int *num_buffers,
>>> +??????????????????? unsigned int *num_planes,
>>> +??????????????????? unsigned int sizes[],
>>> +??????????????????? struct device *alloc_devs[])
>>> +{
>>> +??? struct aspeed_video *video = vb2_get_drv_priv(q);
>>> +
>>> +??? if (*num_planes) {
>>> +??????? if (sizes[0] < video->max_compressed_size)
>>> +??????????? return -EINVAL;
>>> +
>>> +??????? return 0;
>>> +??? }
>>> +
>>> +??? *num_planes = 1;
>>> +??? sizes[0] = video->max_compressed_size;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_buf_prepare(struct vb2_buffer *vb)
>>> +{
>>> +??? struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
>>> +
>>> +??? if (vb2_plane_size(vb, 0) < video->max_compressed_size)
>>> +??????? return -EINVAL;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_start_streaming(struct vb2_queue *q,
>>> +??????????????????? unsigned int count)
>>> +{
>>> +??? int rc;
>>> +??? struct aspeed_video *video = vb2_get_drv_priv(q);
>>> +
>>> +??? rc = aspeed_video_start_frame(video);
>>> +??? if (rc) {
>>> +??????? aspeed_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
>>> +??????? return rc;
>>> +??? }
>>> +
>>> +??? video->sequence = 0;
>>> +??? set_bit(VIDEO_STREAMING, &video->flags);
>>> +??? return 0;
>>> +}
>>> +
>>> +static void aspeed_video_stop_streaming(struct vb2_queue *q)
>>> +{
>>> +??? int rc;
>>> +??? struct aspeed_video *video = vb2_get_drv_priv(q);
>>> +
>>> +??? clear_bit(VIDEO_STREAMING, &video->flags);
>>> +
>>> +??? rc = wait_event_timeout(video->wait,
>>> +??????????????? !test_bit(VIDEO_FRAME_INPRG, &video->flags),
>>> +??????????????? STOP_TIMEOUT);
>>> +??? if (!rc) {
>>> +??????? dev_err(video->dev, "Timed out when stopping streaming\n");
>>> +??????? aspeed_video_stop(video);
>>> +??? }
>>> +
>>> +??? aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
>>> +}
>>> +
>>> +static void aspeed_video_buf_queue(struct vb2_buffer *vb)
>>> +{
>>> +??? struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
>>> +??? struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>>> +??? struct aspeed_video_buffer *avb = to_aspeed_video_buffer(vbuf);
>>> +??? unsigned long flags;
>>> +
>>> +??? spin_lock_irqsave(&video->lock, flags);
>>> +??? list_add_tail(&avb->link, &video->buffers);
>>> +??? spin_unlock_irqrestore(&video->lock, flags);
>>> +}
>>> +
>>> +static const struct vb2_ops aspeed_video_vb2_ops = {
>>> +??? .queue_setup = aspeed_video_queue_setup,
>>> +??? .wait_prepare = vb2_ops_wait_prepare,
>>> +??? .wait_finish = vb2_ops_wait_finish,
>>> +??? .buf_prepare = aspeed_video_buf_prepare,
>>> +??? .start_streaming = aspeed_video_start_streaming,
>>> +??? .stop_streaming = aspeed_video_stop_streaming,
>>> +??? .buf_queue =? aspeed_video_buf_queue,
>>> +};
>>> +
>>> +static int aspeed_video_setup_video(struct aspeed_video *video)
>>> +{
>>> +??? const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
>>> +?????????????? BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_420));
>>> +??? struct v4l2_device *v4l2_dev = &video->v4l2_dev;
>>> +??? struct vb2_queue *vbq = &video->queue;
>>> +??? struct video_device *vdev = &video->vdev;
>>> +??? int rc;
>>> +
>>> +??? video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
>>> +??? video->pix_fmt.field = V4L2_FIELD_NONE;
>>> +??? video->pix_fmt.colorspace = V4L2_COLORSPACE_SRGB;
>>> +??? video->pix_fmt.quantization = V4L2_QUANTIZATION_FULL_RANGE;
>>> +
>>> +??? rc = v4l2_device_register(video->dev, v4l2_dev);
>>> +??? if (rc) {
>>> +??????? dev_err(video->dev, "Failed to register v4l2 device\n");
>>> +??????? return rc;
>>> +??? }
>>> +
>>> +??? v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
>>> +??? v4l2_ctrl_new_std(&video->ctrl_handler, &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_CID_JPEG_CHROMA_SUBSAMPLING,
>>> +?????????????????? V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
>>> +?????????????????? V4L2_JPEG_CHROMA_SUBSAMPLING_444);
>>> +
>>> +??? if (video->ctrl_handler.error) {
>>> +??????? v4l2_ctrl_handler_free(&video->ctrl_handler);
>>> +??????? v4l2_device_unregister(v4l2_dev);
>>> +
>>> +??????? dev_err(video->dev, "Failed to init controls: %d\n",
>>> +??????????? video->ctrl_handler.error);
>>> +??????? return rc;
>>> +??? }
>>> +
>>> +??? v4l2_dev->ctrl_handler = &video->ctrl_handler;
>>> +
>>> +??? vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>> +??? vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
>>> +??? vbq->dev = v4l2_dev->dev;
>>> +??? vbq->lock = &video->video_lock;
>>> +??? vbq->ops = &aspeed_video_vb2_ops;
>>> +??? vbq->mem_ops = &vb2_dma_contig_memops;
>>> +??? 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;
>>> +
>>> +??? 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;
>>> +??? }
>>> +
>>> +??? vdev->queue = vbq;
>>> +??? vdev->fops = &aspeed_video_v4l2_fops;
>>> +??? vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
>> READWRITE doesn't work for JPEG since there is no clear end of a 
>> frame. Just drop
>> this and the read op in aspeed_video_v4l2_fops.
>>
>>> +??????? V4L2_CAP_STREAMING;
>>> +??? vdev->v4l2_dev = v4l2_dev;
>>> +??? strscpy(vdev->name, DEVICE_NAME, sizeof(vdev->name));
>>> +??? vdev->vfl_type = VFL_TYPE_GRABBER;
>>> +??? vdev->vfl_dir = VFL_DIR_RX;
>>> +??? vdev->release = video_device_release_empty;
>>> +??? vdev->ioctl_ops = &aspeed_video_ioctl_ops;
>>> +??? vdev->lock = &video->video_lock;
>>> +
>>> +??? 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;
>>> +??? }
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_init(struct aspeed_video *video)
>>> +{
>>> +??? int irq;
>>> +??? int rc;
>>> +??? struct device *dev = video->dev;
>>> +
>>> +??? irq = irq_of_parse_and_map(dev->of_node, 0);
>>> +??? if (!irq) {
>>> +??????? dev_err(dev, "Unable to find IRQ\n");
>>> +??????? return -ENODEV;
>>> +??? }
>>> +
>>> +??? rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED,
>>> +????????????????? DEVICE_NAME, video);
>>> +??? if (rc < 0) {
>>> +??????? dev_err(dev, "Unable to request IRQ %d\n", irq);
>>> +??????? return rc;
>>> +??? }
>>> +
>>> +??? video->eclk = devm_clk_get(dev, "eclk");
>>> +??? if (IS_ERR(video->eclk)) {
>>> +??????? dev_err(dev, "Unable to get ECLK\n");
>>> +??????? return PTR_ERR(video->eclk);
>>> +??? }
>>> +
>>> +??? video->vclk = devm_clk_get(dev, "vclk");
>>> +??? if (IS_ERR(video->vclk)) {
>>> +??????? dev_err(dev, "Unable to get VCLK\n");
>>> +??????? return PTR_ERR(video->vclk);
>>> +??? }
>>> +
>>> +??? video->rst = devm_reset_control_get_exclusive(dev, NULL);
>>> +??? if (IS_ERR(video->rst)) {
>>> +??????? dev_err(dev, "Unable to get VE reset\n");
>>> +??????? return PTR_ERR(video->rst);
>>> +??? }
>>> +
>>> +??? rc = of_reserved_mem_device_init(dev);
>>> +??? if (rc) {
>>> +??????? dev_err(dev, "Unable to reserve memory\n");
>>> +??????? return rc;
>>> +??? }
>>> +
>>> +??? rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>>> +??? if (rc) {
>>> +??????? dev_err(dev, "Failed to set DMA mask\n");
>>> +??????? of_reserved_mem_device_release(dev);
>>> +??????? return rc;
>>> +??? }
>>> +
>>> +??? if (!aspeed_video_alloc_buf(video, &video->jpeg,
>>> +??????????????????? VE_JPEG_HEADER_SIZE)) {
>>> +??????? dev_err(dev, "Failed to allocate DMA for JPEG header\n");
>>> +??????? of_reserved_mem_device_release(dev);
>>> +??????? return rc;
>>> +??? }
>>> +
>>> +??? aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_probe(struct platform_device *pdev)
>>> +{
>>> +??? int rc;
>>> +??? struct resource *res;
>>> +??? struct aspeed_video *video = kzalloc(sizeof(*video), GFP_KERNEL);
>>> +
>>> +??? if (!video)
>>> +??????? return -ENOMEM;
>>> +
>>> +??? video->frame_rate = 30;
>>> +??? video->dev = &pdev->dev;
>>> +??? mutex_init(&video->video_lock);
>>> +??? init_waitqueue_head(&video->wait);
>>> +??? INIT_DELAYED_WORK(&video->res_work, aspeed_video_resolution_work);
>>> +??? INIT_LIST_HEAD(&video->buffers);
>>> +
>>> +??? res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +
>>> +??? video->base = devm_ioremap_resource(video->dev, res);
>>> +
>>> +??? if (IS_ERR(video->base))
>>> +??????? return PTR_ERR(video->base);
>>> +
>>> +??? rc = aspeed_video_init(video);
>>> +??? if (rc)
>>> +??????? return rc;
>>> +
>>> +??? rc = aspeed_video_setup_video(video);
>>> +??? if (rc)
>>> +??????? return rc;
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static int aspeed_video_remove(struct platform_device *pdev)
>>> +{
>>> +??? struct device *dev = &pdev->dev;
>>> +??? struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
>>> +??? struct aspeed_video *video = to_aspeed_video(v4l2_dev);
>>> +
>>> +??? video_unregister_device(&video->vdev);
>>> +
>>> +??? vb2_queue_release(&video->queue);
>>> +
>>> +??? v4l2_ctrl_handler_free(&video->ctrl_handler);
>>> +
>>> +??? v4l2_device_unregister(v4l2_dev);
>>> +
>>> +??? dma_free_coherent(video->dev, VE_JPEG_HEADER_SIZE, 
>>> video->jpeg.virt,
>>> +????????????? video->jpeg.dma);
>>> +
>>> +??? of_reserved_mem_device_release(dev);
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> +static const struct of_device_id aspeed_video_of_match[] = {
>>> +??? { .compatible = "aspeed,ast2400-video-engine" },
>>> +??? { .compatible = "aspeed,ast2500-video-engine" },
>>> +??? {}
>>> +};
>>> +MODULE_DEVICE_TABLE(of, aspeed_video_of_match);
>>> +
>>> +static struct platform_driver aspeed_video_driver = {
>>> +??? .driver = {
>>> +??????? .name = DEVICE_NAME,
>>> +??????? .of_match_table = aspeed_video_of_match,
>>> +??? },
>>> +??? .probe = aspeed_video_probe,
>>> +??? .remove = aspeed_video_remove,
>>> +};
>>> +
>>> +module_platform_driver(aspeed_video_driver);
>>> +
>>> +MODULE_DESCRIPTION("ASPEED Video Engine Driver");
>>> +MODULE_AUTHOR("Eddie James");
>>> +MODULE_LICENSE("GPL v2");
>>>
>> Regards,
>>
>> ????Hans
>>
>


^ permalink raw reply

* [PATCH] usb: gadget: fix spelling mistakeis "[En]queing" -> "[En]queuing"
From: David Laight @ 2018-11-08 14:19 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1541642122.1498358.1569514624.6EA70C69@webmail.messagingengine.com>

From: Andrew Jeffery
> Sent: 08 November 2018 01:55
> > -		EPDBG(ep,"Enqueing request on wrong or disabled EP\n");
> > +		EPDBG(ep, "Enqueuing request on wrong or disabled EP\n");

Shouldn't it be Enqueueing ?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Petr Mladek @ 2018-11-08  9:41 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAD=FV=Xyra4+cLB0J64+SWqCF=YKdEJpAuybQboYxOcfrdTeQg@mail.gmail.com>

On Wed 2018-11-07 11:26:56, Doug Anderson wrote:
> On Wed, Nov 7, 2018 at 10:23 AM Andy Shevchenko
> Ah!  Based on who you added to the CC list I guess you meant to CC
> "printk" folks?
> 
> PRINTK
> M:      Petr Mladek <pmladek@suse.com>
> M:      Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> R:      Steven Rostedt <rostedt@goodmis.org>
> S:      Maintained
> F:      kernel/printk/
> F:      include/linux/printk.h
> 
> 
> I'd be happy to CC those folks on future spins (if there are any).
> I'm not convinced that these patches are directly relevant to the
> printk subsystem, but I'm always happy for more people to have a
> chance to review patches.

I, as a printk maintainer, am not completely familiar with all
the console driver problems. But we, printk maintainers, come
to similar deadlocks from the printk side, so we are definitely
interested into this kind of patches.

BTW: There was an attempt to avoid the console_unlock() related
deadlocks a more generic way, see
https://lore.kernel.org/lkml/20181016050428.17966-1-sergey.senozhatsky at gmail.com
Unfortunately, there is some push back against introducing a new
printk-related-locking API.


> Hopefully anyone who needs this patch can find it on one of the
> relevant mailing lists.  I screwed up and missed LKML this time
> around, but there are plenty of other mailing lists here that it could
> be found on.  If requested I'm also happy to re-post the same series
> adding those 3 people if that's what everyone wants.

I have glanced over the patches via
https://www.spinics.net/lists/linux-arm-msm/msg44083.html
I still have to think about it. I will be traveling next week
so it might take some time.

Anyway, please CC printk people into v2 if any.

Best Reagards,
Petr

^ permalink raw reply

* [PATCH v2 1/2] ARM: dts: Add Facebook BMC flash layout
From: Tao Ren @ 2018-11-08  2:31 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1541641556.1495362.1569509568.7BE039CA@webmail.messagingengine.com>

On 11/7/18 5:45 PM, Andrew Jeffery wrote:

>>> Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device?
>>
>> Hi Andrew,
>>
>> Thank you for the review! The new layout file is needed mainly because 
>> of "data0" partition: several facebook platforms use the partition as 
>> "persistent" storage.
>>
>> As for "flash0", technically it's not needed (as you pointed out, /dev/
>> mtd0 covers the entire flash if master_partition is enabled). It's still 
>> here to avoid breaking some legacy applications.
> 
> This is what I expected. I think it might be worth adding a comment, given
> you are respinning the series to address my comments on the board
> devicetree patch.
> 
> Anyway, thanks for the clarification.
> 
> Andrew

Sure Andrew. I will add some comments and send out 2 updated patches together (most likely sometime tomorrow).

Thanks,
Tao Ren

^ permalink raw reply

* [PATCH] usb: gadget: fix spelling mistakeis "[En]queing" -> "[En]queuing"
From: Andrew Jeffery @ 2018-11-08  1:55 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20180929114313.10557-1-colin.king@canonical.com>

On Sat, 29 Sep 2018, at 22:13, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Trivial fix to spelling mistakes in debug warning messages
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  drivers/usb/gadget/udc/aspeed-vhub/epn.c | 2 +-
>  drivers/usb/gadget/udc/udc-xilinx.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c b/drivers/usb/
> gadget/udc/aspeed-vhub/epn.c
> index 5939eb1e97f2..e9ee2b72af19 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> @@ -353,7 +353,7 @@ static int ast_vhub_epn_queue(struct usb_ep* u_ep, 
> struct usb_request *u_req,
>  	/* Endpoint enabled ? */
>  	if (!ep->epn.enabled || !u_ep->desc || !ep->dev || !ep->d_idx ||
>  	    !ep->dev->enabled || ep->dev->suspended) {
> -		EPDBG(ep,"Enqueing request on wrong or disabled EP\n");
> +		EPDBG(ep, "Enqueuing request on wrong or disabled EP\n");
>  		return -ESHUTDOWN;
>  	}
>  
> diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/
> udc/udc-xilinx.c
> index 6407e433bc78..b1f4104d1283 100644
> --- a/drivers/usb/gadget/udc/udc-xilinx.c
> +++ b/drivers/usb/gadget/udc/udc-xilinx.c
> @@ -1078,7 +1078,7 @@ static int xudc_ep_queue(struct usb_ep *_ep, 
> struct usb_request *_req,
>  	unsigned long flags;
>  
>  	if (!ep->desc) {
> -		dev_dbg(udc->dev, "%s:queing request to disabled %s\n",
> +		dev_dbg(udc->dev, "%s: queuing request to disabled %s\n",
>  			__func__, ep->name);
>  		return -ESHUTDOWN;
>  	}
> -- 
> 2.17.1
> 

^ permalink raw reply

* [PATCH v2 1/2] ARM: dts: Add Facebook BMC flash layout
From: Andrew Jeffery @ 2018-11-08  1:45 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <c5e56fc4-397f-a38b-6378-99eecce1180b@fb.com>

On Thu, 8 Nov 2018, at 10:48, Tao Ren wrote:
> On 11/7/18 4:02 PM, Andrew Jeffery wrote:
> >> +partitions {
> >> +	compatible = "fixed-partitions";
> >> +	#address-cells = <1>;
> >> +	#size-cells = <1>;
> >> +
> >> +	u-boot at 0 {
> >> +		reg = <0x0 0x60000>;
> >> +		label = "u-boot";
> >> +	};
> >> +
> >> +	u-boot-env at 60000 {
> >> +		reg = <0x60000 0x20000>;
> >> +		label = "env";
> >> +	};
> >> +
> >> +	fit at 80000 {
> >> +		reg = <0x80000 0x1b80000>;
> >> +		label = "fit";
> >> +	};
> >> +
> >> +	data0 at 1c00000 {
> >> +		reg = <0x1c00000 0x400000>;
> >> +		label = "data0";
> >> +	};
> >> +
> >> +	flash0 at 0 {
> >> +		reg = <0x0 0x2000000>;
> >> +		label = "flash0";
> >> +	};
> > 
> > Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device?
> 
> Hi Andrew,
> 
> Thank you for the review! The new layout file is needed mainly because 
> of "data0" partition: several facebook platforms use the partition as 
> "persistent" storage.
> 
> As for "flash0", technically it's not needed (as you pointed out, /dev/
> mtd0 covers the entire flash if master_partition is enabled). It's still 
> here to avoid breaking some legacy applications.

This is what I expected. I think it might be worth adding a comment, given
you are respinning the series to address my comments on the board
devicetree patch.

Anyway, thanks for the clarification.

Andrew

^ permalink raw reply

* [PATCH] usb: gadget: aspeed-vhub: constify usb_gadget_ops structure
From: Andrew Jeffery @ 2018-11-08  0:35 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1540916399-22426-1-git-send-email-Julia.Lawall@lip6.fr>

On Wed, 31 Oct 2018, at 02:49, Julia Lawall wrote:
> The usb_gadget_ops structure can be const as it is only stored in
> the ops field of a usb_gadget structure and this field is const.
> 
> Done with the help of Coccinelle.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> 
> ---
>  drivers/usb/gadget/udc/aspeed-vhub/dev.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c b/drivers/usb/
> gadget/udc/aspeed-vhub/dev.c
> index f0233912bace..6b1b16b17d7d 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> @@ -438,7 +438,7 @@ static int ast_vhub_udc_stop(struct usb_gadget 
> *gadget)
>  	return 0;
>  }
>  
> -static struct usb_gadget_ops ast_vhub_udc_ops = {
> +static const struct usb_gadget_ops ast_vhub_udc_ops = {
>  	.get_frame	= ast_vhub_udc_get_frame,
>  	.wakeup		= ast_vhub_udc_wakeup,
>  	.pullup		= ast_vhub_udc_pullup,
> 

^ permalink raw reply

* [PATCH v2 2/2] ARM: dts: aspeed: Add Facebook Backpack-CMM BMC
From: Andrew Jeffery @ 2018-11-08  0:24 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181106042842.2771782-1-taoren@fb.com>

Hi Tao,

On Tue, 6 Nov 2018, at 14:58, Tao Ren wrote:
> Add initial version of device tree file for Facebook Backpack CMM
> (Chasis Management Module) ast2500 BMC.
> 
> Note: I2C devices on Backpack Line Cards and Fabric Cards are not
> listed in the device tree file because Line/Fabric Cards may be
> unplugged.
> 
> Signed-off-by: Tao Ren <taoren@fb.com>
> ---
>  Changes since v1:
>   - including "aspeed-bmc-facebook-cmm.dtb" in Makefile.
> 
>  arch/arm/boot/dts/Makefile                    |   1 +
>  arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts | 317 ++++++++++++++++++
>  2 files changed, 318 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index a7c313bfe490..2239e6a30d7a 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1200,6 +1200,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	aspeed-ast2500-evb.dtb \
>  	aspeed-bmc-arm-centriq2400-rep.dtb \
>  	aspeed-bmc-arm-stardragon4800-rep2.dtb \
> +	aspeed-bmc-facebook-cmm.dtb \
>  	aspeed-bmc-facebook-tiogapass.dtb \
>  	aspeed-bmc-intel-s2600wf.dtb \
>  	aspeed-bmc-opp-lanyang.dtb \
> diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts b/arch/arm/
> boot/dts/aspeed-bmc-facebook-cmm.dts
> new file mode 100644
> index 000000000000..a259d8a2426a
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
> @@ -0,0 +1,317 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (c) 2018 Facebook Inc.
> +/dts-v1/;
> +
> +#include "aspeed-g5.dtsi"
> +
> +/ {
> +	model = "Facebook Backpack CMM BMC";
> +	compatible = "facebook,cmm-bmc", "aspeed,ast2500";
> +
> +	aliases {
> +		/*
> +		 * Override the default uart aliases to avoid breaking
> +		 * the legacy applications.
> +		 */
> +		serial0 = &uart5;
> +		serial1 = &uart1;
> +		serial2 = &uart3;
> +		serial3 = &uart4;
> +
> +		/*
> +		 * Hardcode the bus number of i2c switches' channels to
> +		 * avoid breaking the legacy applications.
> +		 */
> +		i2c16 = &imux16;
> +		i2c17 = &imux17;
> +		i2c18 = &imux18;
> +		i2c19 = &imux19;
> +		i2c20 = &imux20;
> +		i2c21 = &imux21;
> +		i2c22 = &imux22;
> +		i2c23 = &imux23;
> +		i2c24 = &imux24;
> +		i2c25 = &imux25;
> +		i2c26 = &imux26;
> +		i2c27 = &imux27;
> +		i2c28 = &imux28;
> +		i2c29 = &imux29;
> +		i2c30 = &imux30;
> +		i2c31 = &imux31;
> +		i2c32 = &imux32;
> +		i2c33 = &imux33;
> +		i2c34 = &imux34;
> +		i2c35 = &imux35;
> +		i2c36 = &imux36;
> +		i2c37 = &imux37;
> +		i2c38 = &imux38;
> +		i2c39 = &imux39;
> +	};
> +
> +	chosen {
> +		stdout-path = &uart1;
> +		bootargs = "console=ttyS1,9600n8 root=/dev/ram rw earlyprintk";
> +	};
> +
> +	memory at 80000000 {
> +		reg = <0x80000000 0x20000000>;
> +	};
> +};
> +
> +/*
> + * Update reset type to "system" (full chip) to fix warm reboot hang 
> issue
> + * when reset type is set to default ("soc", gated by reset mask 
> registers).
> + */
> +&wdt1 {
> +	status = "okay";
> +	aspeed,reset-type = "system";
> +};
> +
> +/*
> + * wdt2 is not used by Backpack CMM.
> + */
> +&wdt2 {
> +	status = "disabled";
> +};
> +
> +&fmc {
> +	status = "okay";
> +	flash at 0 {
> +		status = "okay";
> +		m25p,fast-read;
> +		label = "bmc";
> +#include "facebook-bmc-flash-layout.dtsi"
> +	};
> +};
> +
> +&uart1 {
> +	status = "okay";
> +};
> +
> +&uart3 {
> +	status = "okay";
> +};
> +
> +&uart4 {
> +	status = "okay";
> +};

UARTs 1-4 should be explicitly pinmuxed. It's a bit fiddly as each of the UART lines has its own mux configuration, but it at least allows some flexibility (i.e. whether you just want Rx/Tx, or something more). See e.g. the uart1 node in arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts.

Speaking of pinctrl, as it's an AST2500 system you'll probably want to add the following to ensure you have full control of the pinmux configuration:

&pinctrl {
	aspeed,external-nodes = <&gfx &lhc>;
};

> +
> +&uart5 {
> +	status = "okay";
> +};

UART5 has fixed pins, so no pinctrl necessary here.

> +
> +&mac1 {
> +	status = "okay";
> +	no-hw-checksum;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	status = "okay";
> +
> +	i2c-switch at 77 {
> +		compatible = "nxp,pca9548";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x77>;
> +
> +		imux16: i2c at 0 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0>;
> +		};
> +
> +		imux17: i2c at 1 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <1>;
> +		};
> +
> +		imux18: i2c at 2 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <2>;
> +		};
> +
> +		imux19: i2c at 3 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <3>;
> +		};
> +
> +		imux20: i2c at 4 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <4>;
> +		};
> +
> +		imux21: i2c at 5 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <5>;
> +		};
> +
> +		imux22: i2c at 6 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <6>;
> +		};
> +
> +		imux23: i2c at 7 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <7>;
> +		};
> +	};
> +};
> +
> +&i2c2 {
> +	status = "okay";
> +
> +	i2c-switch at 71 {
> +		compatible = "nxp,pca9548";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x71>;
> +
> +		imux24: i2c at 0 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0>;
> +		};
> +
> +		imux25: i2c at 1 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <1>;
> +		};
> +
> +		imux26: i2c at 2 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <2>;
> +		};
> +
> +		imux27: i2c at 3 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <3>;
> +		};
> +
> +		imux28: i2c at 4 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <4>;
> +		};
> +
> +		imux29: i2c at 5 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <5>;
> +		};
> +
> +		imux30: i2c at 6 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <6>;
> +		};
> +
> +		imux31: i2c at 7 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <7>;
> +		};
> +	};
> +};
> +
> +&i2c3 {
> +	status = "okay";
> +};
> +
> +&i2c4 {
> +	status = "okay";
> +};
> +
> +&i2c5 {
> +	status = "okay";
> +};
> +
> +&i2c6 {
> +	status = "okay";
> +};
> +
> +&i2c7 {
> +	status = "okay";
> +};
> +
> +&i2c8 {
> +	status = "okay";
> +
> +	i2c-switch at 77 {
> +		compatible = "nxp,pca9548";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x77>;
> +
> +		imux32: i2c at 0 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0>;
> +		};
> +
> +		imux33: i2c at 1 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <1>;
> +		};
> +
> +		imux34: i2c at 2 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <2>;
> +		};
> +
> +		imux35: i2c at 3 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <3>;
> +		};
> +
> +		imux36: i2c at 4 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <4>;
> +		};
> +
> +		imux37: i2c at 5 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <5>;
> +		};
> +
> +		imux38: i2c at 6 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <6>;
> +		};
> +
> +		imux39: i2c at 7 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <7>;
> +		};
> +	};
> +};
> +
> +&i2c13 {
> +	status = "okay";
> +};

You said this in the commit message:

> Note: I2C devices on Backpack Line Cards and Fabric Cards are not
> listed in the device tree file because Line/Fabric Cards may be
> unplugged.

It's not clear to me what this actually means, and I'm trying to reconcile it with enabling all of the i2c buses as I'm curious as to whether that's necessary.

Cheers,

Andrew

> +
> +&adc {
> +	status = "okay";
> +};
> -- 
> 2.17.1
> 

^ permalink raw reply

* [PATCH v2 1/2] ARM: dts: Add Facebook BMC flash layout
From: Tao Ren @ 2018-11-08  0:18 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1541635320.1462459.1569430616.0F5E4C4D@webmail.messagingengine.com>

On 11/7/18 4:02 PM, Andrew Jeffery wrote:
>> +partitions {
>> +	compatible = "fixed-partitions";
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +
>> +	u-boot at 0 {
>> +		reg = <0x0 0x60000>;
>> +		label = "u-boot";
>> +	};
>> +
>> +	u-boot-env at 60000 {
>> +		reg = <0x60000 0x20000>;
>> +		label = "env";
>> +	};
>> +
>> +	fit at 80000 {
>> +		reg = <0x80000 0x1b80000>;
>> +		label = "fit";
>> +	};
>> +
>> +	data0 at 1c00000 {
>> +		reg = <0x1c00000 0x400000>;
>> +		label = "data0";
>> +	};
>> +
>> +	flash0 at 0 {
>> +		reg = <0x0 0x2000000>;
>> +		label = "flash0";
>> +	};
> 
> Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device?

Hi Andrew,

Thank you for the review! The new layout file is needed mainly because of "data0" partition: several facebook platforms use the partition as "persistent" storage.

As for "flash0", technically it's not needed (as you pointed out, /dev/mtd0 covers the entire flash if master_partition is enabled). It's still here to avoid breaking some legacy applications.


Thanks,
Tao Ren

^ permalink raw reply

* [PATCH v2 1/2] ARM: dts: Add Facebook BMC flash layout
From: Andrew Jeffery @ 2018-11-08  0:02 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181106042829.2771226-1-taoren@fb.com>

Hi Tao,

On Tue, 6 Nov 2018, at 14:58, Tao Ren wrote:
> This is the layout used by Facebook BMC systems. It describes the fixed
> flash layout of a 32MB mtd device.
> 
> Signed-off-by: Tao Ren <taoren@fb.com>
> ---
>  Changes since v1:
>   - adding facebook copyright.
> 
>  .../boot/dts/facebook-bmc-flash-layout.dtsi   | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi
> 
> diff --git a/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi b/arch/
> arm/boot/dts/facebook-bmc-flash-layout.dtsi
> new file mode 100644
> index 000000000000..94039d7b7de5
> --- /dev/null
> +++ b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (c) 2018 Facebook Inc.
> +
> +partitions {
> +	compatible = "fixed-partitions";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	u-boot at 0 {
> +		reg = <0x0 0x60000>;
> +		label = "u-boot";
> +	};
> +
> +	u-boot-env at 60000 {
> +		reg = <0x60000 0x20000>;
> +		label = "env";
> +	};
> +
> +	fit at 80000 {
> +		reg = <0x80000 0x1b80000>;
> +		label = "fit";
> +	};
> +
> +	data0 at 1c00000 {
> +		reg = <0x1c00000 0x400000>;
> +		label = "data0";
> +	};
> +
> +	flash0 at 0 {
> +		reg = <0x0 0x2000000>;
> +		label = "flash0";
> +	};

Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device?

Cheers,

Andrew

> +};
> -- 
> 2.17.1
> 

^ permalink raw reply

* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Andy Shevchenko @ 2018-11-07 19:54 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAD=FV=Xyra4+cLB0J64+SWqCF=YKdEJpAuybQboYxOcfrdTeQg@mail.gmail.com>

On Wed, Nov 07, 2018 at 11:26:56AM -0800, Doug Anderson wrote:
> Hi,
> 
> On Wed, Nov 7, 2018 at 10:23 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Tue, Oct 30, 2018 at 03:11:02PM -0700, Douglas Anderson wrote:
> > > I started out this series trying to make sysrq work over the serial
> > > console on qcom_geni_serial, then fell into a rat's nest.
> > >
> > > To solve the deadlock I faced when enabling sysrq I tried to borrow
> > > code from '8250_port.c' which avoided grabbing the port lock in
> > > console_write().  ...but since these days I try to run with lockdep on
> > > all the time, I found it caused an annoying lockdep splat (which I
> > > also reproduced on my rk3399 board).  ...so I instead changed my
> > > qcom_geni_serial solution to borrow code from 'msm_serial.c'
> > >
> > > I wasn't super happy with the solution in 'msm_serial.c' though.  I
> > > don't like releasing the spinlock there.  Not only is it ugly but it
> > > means we are unlocking / re-locking _all the time_ even though sysrq
> > > characters are rare.  ...so I came up with what I think is a better
> > > solution and then implemented it for qcom_geni_serial.
> > >
> > > Since I had a good way to test 8250-based UARTs, I also fixed that
> > > driver to use my new method.  When doing so, I ran into a missing
> > > include in serial_core.h.  NOTE: I didn't have a way to test
> > > msm_serial.c at all, so I didn't switch that (or all other serial
> > > drivers for that matter) to the new method.
> > >
> > > NOTE: from a serial point of view v2 is the same as v1 but I've
> > > removed the extra kgdb-related patches and made it obvious that this
> > > is really for all sysrq, not just kgdb.  I've also generally tried to
> > > curate the CCs more properly.
> >
> > It seems your forgot console people to Cc.
> 
> Can you be more specific, please?  Which section of the "MAINTAINERS"
> file should I be looking at for the "console" you are thinking of?

I have added them to Cc list: Petr, Sergey, and Steven.

> Certainly there are lots of hits for "console" in MAINTAINERS but I
> don't think I see any that are relevant that I missed.  Grepping:

> Ah!  Based on who you added to the CC list I guess you meant to CC
> "printk" folks?

Correct.

> PRINTK
> M:      Petr Mladek <pmladek@suse.com>
> M:      Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> R:      Steven Rostedt <rostedt@goodmis.org>
> S:      Maintained
> F:      kernel/printk/
> F:      include/linux/printk.h


> I'd be happy to CC those folks on future spins (if there are any).
> I'm not convinced that these patches are directly relevant to the
> printk subsystem, but I'm always happy for more people to have a
> chance to review patches.

If you look retrospectively in the mailing lists, you can find that they are
doing most of the console core work, which I believe includes SysRq behaviour.
So, don't be confused their names are listed under PRINTK.

> Hopefully anyone who needs this patch can find it on one of the
> relevant mailing lists.  I screwed up and missed LKML this time
> around, but there are plenty of other mailing lists here that it could
> be found on.  If requested I'm also happy to re-post the same series
> adding those 3 people if that's what everyone wants.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Doug Anderson @ 2018-11-07 19:26 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181107182349.GP10650@smile.fi.intel.com>

Hi,

On Wed, Nov 7, 2018 at 10:23 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Oct 30, 2018 at 03:11:02PM -0700, Douglas Anderson wrote:
> > I started out this series trying to make sysrq work over the serial
> > console on qcom_geni_serial, then fell into a rat's nest.
> >
> > To solve the deadlock I faced when enabling sysrq I tried to borrow
> > code from '8250_port.c' which avoided grabbing the port lock in
> > console_write().  ...but since these days I try to run with lockdep on
> > all the time, I found it caused an annoying lockdep splat (which I
> > also reproduced on my rk3399 board).  ...so I instead changed my
> > qcom_geni_serial solution to borrow code from 'msm_serial.c'
> >
> > I wasn't super happy with the solution in 'msm_serial.c' though.  I
> > don't like releasing the spinlock there.  Not only is it ugly but it
> > means we are unlocking / re-locking _all the time_ even though sysrq
> > characters are rare.  ...so I came up with what I think is a better
> > solution and then implemented it for qcom_geni_serial.
> >
> > Since I had a good way to test 8250-based UARTs, I also fixed that
> > driver to use my new method.  When doing so, I ran into a missing
> > include in serial_core.h.  NOTE: I didn't have a way to test
> > msm_serial.c at all, so I didn't switch that (or all other serial
> > drivers for that matter) to the new method.
> >
> > NOTE: from a serial point of view v2 is the same as v1 but I've
> > removed the extra kgdb-related patches and made it obvious that this
> > is really for all sysrq, not just kgdb.  I've also generally tried to
> > curate the CCs more properly.
>
> It seems your forgot console people to Cc.

Can you be more specific, please?  Which section of the "MAINTAINERS"
file should I be looking at for the "console" you are thinking of?
Certainly there are lots of hits for "console" in MAINTAINERS but I
don't think I see any that are relevant that I missed.  Grepping:

$ grep  -i console MAINTAINERS
HYPERVISOR VIRTUAL CONSOLE DRIVER
F:      drivers/video/console/sti*
PCDP - PRIMARY CONSOLE AND DEBUG PORT
SGI SN-IA64 (Altix) SERIAL CONSOLE DRIVER
STAGING - SPEAKUP CONSOLE SPEECH DRIVER
VIRTIO CONSOLE DRIVER
F:      drivers/char/virtio_console.c
F:      include/linux/virtio_console.h
F:      include/uapi/linux/virtio_console.h

...none of those seem relevant upon first glance but I'm happy to
stand corrected.

Ah!  Based on who you added to the CC list I guess you meant to CC
"printk" folks?

PRINTK
M:      Petr Mladek <pmladek@suse.com>
M:      Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
R:      Steven Rostedt <rostedt@goodmis.org>
S:      Maintained
F:      kernel/printk/
F:      include/linux/printk.h


I'd be happy to CC those folks on future spins (if there are any).
I'm not convinced that these patches are directly relevant to the
printk subsystem, but I'm always happy for more people to have a
chance to review patches.

Hopefully anyone who needs this patch can find it on one of the
relevant mailing lists.  I screwed up and missed LKML this time
around, but there are plenty of other mailing lists here that it could
be found on.  If requested I'm also happy to re-post the same series
adding those 3 people if that's what everyone wants.

-Doug

^ permalink raw reply

* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Andy Shevchenko @ 2018-11-07 18:23 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>

On Tue, Oct 30, 2018 at 03:11:02PM -0700, Douglas Anderson wrote:
> I started out this series trying to make sysrq work over the serial
> console on qcom_geni_serial, then fell into a rat's nest.
> 
> To solve the deadlock I faced when enabling sysrq I tried to borrow
> code from '8250_port.c' which avoided grabbing the port lock in
> console_write().  ...but since these days I try to run with lockdep on
> all the time, I found it caused an annoying lockdep splat (which I
> also reproduced on my rk3399 board).  ...so I instead changed my
> qcom_geni_serial solution to borrow code from 'msm_serial.c'
> 
> I wasn't super happy with the solution in 'msm_serial.c' though.  I
> don't like releasing the spinlock there.  Not only is it ugly but it
> means we are unlocking / re-locking _all the time_ even though sysrq
> characters are rare.  ...so I came up with what I think is a better
> solution and then implemented it for qcom_geni_serial.
> 
> Since I had a good way to test 8250-based UARTs, I also fixed that
> driver to use my new method.  When doing so, I ran into a missing
> include in serial_core.h.  NOTE: I didn't have a way to test
> msm_serial.c at all, so I didn't switch that (or all other serial
> drivers for that matter) to the new method.
> 
> NOTE: from a serial point of view v2 is the same as v1 but I've
> removed the extra kgdb-related patches and made it obvious that this
> is really for all sysrq, not just kgdb.  I've also generally tried to
> curate the CCs more properly.

It seems your forgot console people to Cc.

> 
> 
> Douglas Anderson (5):
>   serial: qcom_geni_serial: Finish supporting sysrq
>   serial: core: Allow processing sysrq at port unlock time
>   serial: qcom_geni_serial: Process sysrq at port unlock time
>   serial: core: Include console.h from serial_core.h
>   serial: 8250: Process sysrq at port unlock time
> 
>  drivers/tty/serial/8250/8250_aspeed_vuart.c |  6 +++-
>  drivers/tty/serial/8250/8250_fsl.c          |  6 +++-
>  drivers/tty/serial/8250/8250_omap.c         |  6 +++-
>  drivers/tty/serial/8250/8250_port.c         |  8 ++---
>  drivers/tty/serial/qcom_geni_serial.c       | 10 ++++--
>  include/linux/serial_core.h                 | 38 ++++++++++++++++++++-
>  6 files changed, 63 insertions(+), 11 deletions(-)
> 
> -- 
> 2.19.1.568.g152ad8e336-goog
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox