[v2,6/9] pipeline: rkisp1: Add more info when parameter queue fails

23163 diff mbox series

When the rkisp1 driver of the running kernel supports less parameter
types than the libcamera build and such a parameter is used, queuing of
the parameter buffer fails with -EINVAL. For the time being we have no
way to query the kernel for supported parameter types at runtime. So at
least print a error message that points the user in that direction.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

Changes in v2:
- Also print the error code in case of failure
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Quoting Stefan Klug (2025-04-11 13:36:34)
> When the rkisp1 driver of the running kernel supports less parameter
> types than the libcamera build and such a parameter is used, queuing of
> the parameter buffer fails with -EINVAL. For the time being we have no
> way to query the kernel for supported parameter types at runtime. So at
> least print a error message that points the user in that direction.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Also print the error code in case of failure
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 52633fe3cb85..9d7a3bd57ada 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -409,7 +409,15 @@ void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bytesused
>                 return;
>  
>         info->paramBuffer->_d()->metadata().planes()[0].bytesused = bytesused;
> -       pipe->param_->queueBuffer(info->paramBuffer);
> +
> +       int ret = pipe->param_->queueBuffer(info->paramBuffer);
> +       if (ret < 0) {
> +               LOG(RkISP1, Error) << "Failed to queue parameter buffer: " << ret

strerror(-ret)? although not required.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +                                  << " Maybe the kernel doesn't support the"
> +                                     " required parameter types.";
> +               return;
> +       }
> +
>         pipe->stat_->queueBuffer(info->statBuffer);
>  
>         if (info->mainPathBuffer)
> -- 
> 2.43.0
>

Quoting Stefan Klug (2025-04-11 14:36:34)
> When the rkisp1 driver of the running kernel supports less parameter
> types than the libcamera build and such a parameter is used, queuing of
> the parameter buffer fails with -EINVAL. For the time being we have no
> way to query the kernel for supported parameter types at runtime. So at
> least print a error message that points the user in that direction.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> 
> ---
> 
> Changes in v2:
> - Also print the error code in case of failure
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 52633fe3cb85..9d7a3bd57ada 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -409,7 +409,15 @@ void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bytesused
>                 return;
>  
>         info->paramBuffer->_d()->metadata().planes()[0].bytesused = bytesused;
> -       pipe->param_->queueBuffer(info->paramBuffer);
> +
> +       int ret = pipe->param_->queueBuffer(info->paramBuffer);
> +       if (ret < 0) {
> +               LOG(RkISP1, Error) << "Failed to queue parameter buffer: " << ret
> +                                  << " Maybe the kernel doesn't support the"
> +                                     " required parameter types.";
> +               return;
> +       }
> +
>         pipe->stat_->queueBuffer(info->statBuffer);
>  
>         if (info->mainPathBuffer)
> -- 
> 2.43.0
>

On Fri, Apr 11, 2025 at 02:36:34PM +0200, Stefan Klug wrote:
> When the rkisp1 driver of the running kernel supports less parameter
> types than the libcamera build and such a parameter is used, queuing of
> the parameter buffer fails with -EINVAL. For the time being we have no
> way to query the kernel for supported parameter types at runtime. So at
> least print a error message that points the user in that direction.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Not blocking this patch, but would you have time to extend the rkisp1
driver with a V4L2_CTRL_TYPE_BITMASK read-only control that returns the
list of supported blocks ?

Actually, I'm thinking we should have that control before we merge WDR
support in rkisp1, in which case this patch wouldn't be needed.

> ---
> 
> Changes in v2:
> - Also print the error code in case of failure
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 52633fe3cb85..9d7a3bd57ada 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -409,7 +409,15 @@ void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bytesused
>  		return;
>  
>  	info->paramBuffer->_d()->metadata().planes()[0].bytesused = bytesused;
> -	pipe->param_->queueBuffer(info->paramBuffer);
> +
> +	int ret = pipe->param_->queueBuffer(info->paramBuffer);
> +	if (ret < 0) {
> +		LOG(RkISP1, Error) << "Failed to queue parameter buffer: " << ret
> +				   << " Maybe the kernel doesn't support the"
> +				      " required parameter types.";
> +		return;
> +	}
> +
>  	pipe->stat_->queueBuffer(info->statBuffer);
>  
>  	if (info->mainPathBuffer)