[PATCH v2 0/4] Follow-up patches for Cedrus v9

[PATCH v2 0/4] Follow-up patches for Cedrus v9

[Paul Kocialkowski]

This brings the requested modifications on top of version 9 of the
Cedrus VPU driver, that implements stateless video decoding using the
Request API.

Changes since v1:
* Added two more commits to fix build issues with non-sunxi configs.

In order to build correctly on non-sunxi platforms, the following commit
is also required to select the sunxi SRAM driver:
* drivers: soc: Allow building the sunxi driver without ARCH_SUNXI

Paul Kocialkowski (4):
  media: cedrus: Fix error reporting in request validation
  media: cedrus: Add TODO file with tasks to complete before unstaging
  media: cedrus: Wrap PHYS_PFN_OFFSET with ifdef and add dedicated
    comment
  media: cedrus: Select the sunxi SRAM driver in Kconfig

 drivers/staging/media/sunxi/cedrus/Kconfig     |  1 +
 drivers/staging/media/sunxi/cedrus/TODO        |  7 +++++++
 drivers/staging/media/sunxi/cedrus/cedrus.c    | 15 ++++++++++++---
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c |  5 +++++
 4 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 drivers/staging/media/sunxi/cedrus/TODO

-- 
2.18.0

[PATCH v2 0/4] Follow-up patches for Cedrus v9

[Paul Kocialkowski]

This brings the requested modifications on top of version 9 of the
Cedrus VPU driver, that implements stateless video decoding using the
Request API.

Changes since v1:
* Added two more commits to fix build issues with non-sunxi configs.

In order to build correctly on non-sunxi platforms, the following commit
is also required to select the sunxi SRAM driver:
* drivers: soc: Allow building the sunxi driver without ARCH_SUNXI

Paul Kocialkowski (4):
  media: cedrus: Fix error reporting in request validation
  media: cedrus: Add TODO file with tasks to complete before unstaging
  media: cedrus: Wrap PHYS_PFN_OFFSET with ifdef and add dedicated
    comment
  media: cedrus: Select the sunxi SRAM driver in Kconfig

 drivers/staging/media/sunxi/cedrus/Kconfig     |  1 +
 drivers/staging/media/sunxi/cedrus/TODO        |  7 +++++++
 drivers/staging/media/sunxi/cedrus/cedrus.c    | 15 ++++++++++++---
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c |  5 +++++
 4 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 drivers/staging/media/sunxi/cedrus/TODO

-- 
2.18.0

[PATCH v2 1/4] media: cedrus: Fix error reporting in request validation

[Paul Kocialkowski]

This fixes error reporting by using the appropriate logging helpers and
return codes, while introducing new messages when there are not enough
or too many buffers associated with the request.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 09ab1b732c31..0a9363c7db06 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -105,10 +105,19 @@ static int cedrus_request_validate(struct media_request *req)
 	struct v4l2_ctrl_handler *parent_hdl, *hdl;
 	struct cedrus_ctx *ctx = NULL;
 	struct v4l2_ctrl *ctrl_test;
+	unsigned int count;
 	unsigned int i;
 
-	if (vb2_request_buffer_cnt(req) != 1)
+	count = vb2_request_buffer_cnt(req);
+	if (!count) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "No buffer was provided with the request\n");
 		return -ENOENT;
+	} else if (count > 1) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "More than one buffer was provided with the request\n");
+		return -EINVAL;
+	}
 
 	list_for_each_entry(obj, &req->objects, list) {
 		struct vb2_buffer *vb;
@@ -128,7 +137,7 @@ static int cedrus_request_validate(struct media_request *req)
 
 	hdl = v4l2_ctrl_request_hdl_find(req, parent_hdl);
 	if (!hdl) {
-		v4l2_err(&ctx->dev->v4l2_dev, "Missing codec control(s)\n");
+		v4l2_info(&ctx->dev->v4l2_dev, "Missing codec control(s)\n");
 		return -ENOENT;
 	}
 
@@ -140,7 +149,7 @@ static int cedrus_request_validate(struct media_request *req)
 		ctrl_test = v4l2_ctrl_request_hdl_ctrl_find(hdl,
 			cedrus_controls[i].id);
 		if (!ctrl_test) {
-			v4l2_err(&ctx->dev->v4l2_dev,
+			v4l2_info(&ctx->dev->v4l2_dev,
 				 "Missing required codec control\n");
 			return -ENOENT;
 		}
-- 
2.18.0

[PATCH v2 1/4] media: cedrus: Fix error reporting in request validation

[Paul Kocialkowski]

This fixes error reporting by using the appropriate logging helpers and
return codes, while introducing new messages when there are not enough
or too many buffers associated with the request.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 09ab1b732c31..0a9363c7db06 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -105,10 +105,19 @@ static int cedrus_request_validate(struct media_request *req)
 	struct v4l2_ctrl_handler *parent_hdl, *hdl;
 	struct cedrus_ctx *ctx = NULL;
 	struct v4l2_ctrl *ctrl_test;
+	unsigned int count;
 	unsigned int i;
 
-	if (vb2_request_buffer_cnt(req) != 1)
+	count = vb2_request_buffer_cnt(req);
+	if (!count) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "No buffer was provided with the request\n");
 		return -ENOENT;
+	} else if (count > 1) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "More than one buffer was provided with the request\n");
+		return -EINVAL;
+	}
 
 	list_for_each_entry(obj, &req->objects, list) {
 		struct vb2_buffer *vb;
@@ -128,7 +137,7 @@ static int cedrus_request_validate(struct media_request *req)
 
 	hdl = v4l2_ctrl_request_hdl_find(req, parent_hdl);
 	if (!hdl) {
-		v4l2_err(&ctx->dev->v4l2_dev, "Missing codec control(s)\n");
+		v4l2_info(&ctx->dev->v4l2_dev, "Missing codec control(s)\n");
 		return -ENOENT;
 	}
 
@@ -140,7 +149,7 @@ static int cedrus_request_validate(struct media_request *req)
 		ctrl_test = v4l2_ctrl_request_hdl_ctrl_find(hdl,
 			cedrus_controls[i].id);
 		if (!ctrl_test) {
-			v4l2_err(&ctx->dev->v4l2_dev,
+			v4l2_info(&ctx->dev->v4l2_dev,
 				 "Missing required codec control\n");
 			return -ENOENT;
 		}
-- 
2.18.0

[PATCH v2 2/4] media: cedrus: Add TODO file with tasks to complete before unstaging

[Paul Kocialkowski]

When the elements listed are complete, the Cedrus driver will be ready
to move out of the staging area of the kernel.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/TODO | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 drivers/staging/media/sunxi/cedrus/TODO

diff --git a/drivers/staging/media/sunxi/cedrus/TODO b/drivers/staging/media/sunxi/cedrus/TODO
new file mode 100644
index 000000000000..ec277ece47af
--- /dev/null
+++ b/drivers/staging/media/sunxi/cedrus/TODO
@@ -0,0 +1,7 @@
+Before this stateless decoder driver can leave the staging area:
+* The Request API needs to be stabilized;
+* The codec-specific controls need to be thoroughly reviewed to ensure they
+  cover all intended uses cases;
+* Userspace support for the Request API needs to be reviewed;
+* Another stateless decoder driver should be submitted;
+* At least one stateless encoder driver should be submitted.
-- 
2.18.0

[PATCH v2 2/4] media: cedrus: Add TODO file with tasks to complete before unstaging

[Paul Kocialkowski]

When the elements listed are complete, the Cedrus driver will be ready
to move out of the staging area of the kernel.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/TODO | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 drivers/staging/media/sunxi/cedrus/TODO

diff --git a/drivers/staging/media/sunxi/cedrus/TODO b/drivers/staging/media/sunxi/cedrus/TODO
new file mode 100644
index 000000000000..ec277ece47af
--- /dev/null
+++ b/drivers/staging/media/sunxi/cedrus/TODO
@@ -0,0 +1,7 @@
+Before this stateless decoder driver can leave the staging area:
+* The Request API needs to be stabilized;
+* The codec-specific controls need to be thoroughly reviewed to ensure they
+  cover all intended uses cases;
+* Userspace support for the Request API needs to be reviewed;
+* Another stateless decoder driver should be submitted;
+* At least one stateless encoder driver should be submitted.
-- 
2.18.0

[PATCH v2 3/4] media: cedrus: Wrap PHYS_PFN_OFFSET with ifdef and add dedicated comment

[Paul Kocialkowski]

Since PHYS_PFN_OFFSET is not defined for all architectures, it is
requried to wrap it with ifdef so that it can be built on all
architectures.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index f4307e8f7908..32adbcbe6175 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -199,8 +199,13 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
 	/*
 	 * The VPU is only able to handle bus addresses so we have to subtract
 	 * the RAM offset to the physcal addresses.
+	 *
+	 * This information will eventually be obtained from device-tree.
 	 */
+
+#ifdef PHYS_PFN_OFFSET
 	dev->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
+#endif
 
 	ret = of_reserved_mem_device_init(dev->dev);
 	if (ret && ret != -ENODEV) {
-- 
2.18.0

[PATCH v2 3/4] media: cedrus: Wrap PHYS_PFN_OFFSET with ifdef and add dedicated comment

[Paul Kocialkowski]

Since PHYS_PFN_OFFSET is not defined for all architectures, it is
requried to wrap it with ifdef so that it can be built on all
architectures.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index f4307e8f7908..32adbcbe6175 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -199,8 +199,13 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
 	/*
 	 * The VPU is only able to handle bus addresses so we have to subtract
 	 * the RAM offset to the physcal addresses.
+	 *
+	 * This information will eventually be obtained from device-tree.
 	 */
+
+#ifdef PHYS_PFN_OFFSET
 	dev->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
+#endif
 
 	ret = of_reserved_mem_device_init(dev->dev);
 	if (ret && ret != -ENODEV) {
-- 
2.18.0

[PATCH v2 4/4] media: cedrus: Select the sunxi SRAM driver in Kconfig

[Paul Kocialkowski]

Since the sunxi SRAM driver is required to build the Cedrus driver,
select it in Kconfig.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
index afd7d7ee0388..3b06283e4bf3 100644
--- a/drivers/staging/media/sunxi/cedrus/Kconfig
+++ b/drivers/staging/media/sunxi/cedrus/Kconfig
@@ -3,6 +3,7 @@ config VIDEO_SUNXI_CEDRUS
 	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
 	depends on HAS_DMA
 	depends on OF
+	select SUNXI_SRAM
 	select VIDEOBUF2_DMA_CONTIG
 	select MEDIA_REQUEST_API
 	select V4L2_MEM2MEM_DEV
-- 
2.18.0

[PATCH v2 4/4] media: cedrus: Select the sunxi SRAM driver in Kconfig

[Paul Kocialkowski]

Since the sunxi SRAM driver is required to build the Cedrus driver,
select it in Kconfig.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/staging/media/sunxi/cedrus/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
index afd7d7ee0388..3b06283e4bf3 100644
--- a/drivers/staging/media/sunxi/cedrus/Kconfig
+++ b/drivers/staging/media/sunxi/cedrus/Kconfig
@@ -3,6 +3,7 @@ config VIDEO_SUNXI_CEDRUS
 	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
 	depends on HAS_DMA
 	depends on OF
+	select SUNXI_SRAM
 	select VIDEOBUF2_DMA_CONTIG
 	select MEDIA_REQUEST_API
 	select V4L2_MEM2MEM_DEV
-- 
2.18.0

Re: [PATCH v2 4/4] media: cedrus: Select the sunxi SRAM driver in Kconfig

[Hans Verkuil]

On 09/09/2018 09:10 PM, Paul Kocialkowski wrote:
> Since the sunxi SRAM driver is required to build the Cedrus driver,
> select it in Kconfig.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  drivers/staging/media/sunxi/cedrus/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
> index afd7d7ee0388..3b06283e4bf3 100644
> --- a/drivers/staging/media/sunxi/cedrus/Kconfig
> +++ b/drivers/staging/media/sunxi/cedrus/Kconfig
> @@ -3,6 +3,7 @@ config VIDEO_SUNXI_CEDRUS
>  	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
>  	depends on HAS_DMA
>  	depends on OF
> +	select SUNXI_SRAM
>  	select VIDEOBUF2_DMA_CONTIG
>  	select MEDIA_REQUEST_API

I noticed this select, but CONFIG_MEDIA_REQUEST_API doesn't exist. It's
probably from an old version and I have dropped it from the patch that
added this.

No need for you to do anything.

>  	select V4L2_MEM2MEM_DEV
> 

Regards,

	Hans

[PATCH v2 4/4] media: cedrus: Select the sunxi SRAM driver in Kconfig

[Hans Verkuil]

On 09/09/2018 09:10 PM, Paul Kocialkowski wrote:
> Since the sunxi SRAM driver is required to build the Cedrus driver,
> select it in Kconfig.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  drivers/staging/media/sunxi/cedrus/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
> index afd7d7ee0388..3b06283e4bf3 100644
> --- a/drivers/staging/media/sunxi/cedrus/Kconfig
> +++ b/drivers/staging/media/sunxi/cedrus/Kconfig
> @@ -3,6 +3,7 @@ config VIDEO_SUNXI_CEDRUS
>  	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
>  	depends on HAS_DMA
>  	depends on OF
> +	select SUNXI_SRAM
>  	select VIDEOBUF2_DMA_CONTIG
>  	select MEDIA_REQUEST_API

I noticed this select, but CONFIG_MEDIA_REQUEST_API doesn't exist. It's
probably from an old version and I have dropped it from the patch that
added this.

No need for you to do anything.

>  	select V4L2_MEM2MEM_DEV
> 

Regards,

	Hans