[PATCH] media: venus: Fix reported frame intervals

[PATCH] media: venus: Fix reported frame intervals

[Loic Poulain]

On dragonboard-410c (apq8016) with HFI_VERSION_1XX, the reported
framerate is in unit of 1/65535 fps (for fine grained control).
So the current reported supported frame intervals is wrong (max
is 1/65535 fps), leading to encoding issues or format negotiation
failures with gstreamer.

Fix that by setting the framerate numerator according the framerate
factor (65535).

The factor is not always the same, e.g. with db820c (apq8096) HFI
reports framerate in fps unit. So only apply that for HFI_VERSION_1XX.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/media/platform/qcom/venus/venc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 9981a2a..654bbaf 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -575,7 +575,9 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
 				    struct v4l2_frmivalenum *fival)
 {
 	struct venus_inst *inst = to_inst(file);
+	enum hfi_version ver = inst->core->res->hfi_version;
 	const struct venus_format *fmt;
+	unsigned int framerate_factor = 1;
 
 	fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
 
@@ -600,11 +602,16 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
 	    fival->height < frame_height_min(inst))
 		return -EINVAL;
 
-	fival->stepwise.min.numerator = 1;
+	if (ver == HFI_VERSION_1XX) {
+		/* framerate is reported in 1/65535 fps unit */
+		framerate_factor = (1 << 16);
+	}
+
+	fival->stepwise.min.numerator = framerate_factor;
 	fival->stepwise.min.denominator = frate_max(inst);
-	fival->stepwise.max.numerator = 1;
+	fival->stepwise.max.numerator = framerate_factor;
 	fival->stepwise.max.denominator = frate_min(inst);
-	fival->stepwise.step.numerator = 1;
+	fival->stepwise.step.numerator = framerate_factor;
 	fival->stepwise.step.denominator = frate_max(inst);
 
 	return 0;
-- 
2.7.4

Re: [PATCH] media: venus: Fix reported frame intervals

[Stanimir Varbanov]

Hi Loic,

On 8/3/20 5:17 PM, Loic Poulain wrote:
> On dragonboard-410c (apq8016) with HFI_VERSION_1XX, the reported
> framerate is in unit of 1/65535 fps (for fine grained control).
> So the current reported supported frame intervals is wrong (max
> is 1/65535 fps), leading to encoding issues or format negotiation
> failures with gstreamer.
> 
> Fix that by setting the framerate numerator according the framerate
> factor (65535).
> 
> The factor is not always the same, e.g. with db820c (apq8096) HFI
> reports framerate in fps unit. So only apply that for HFI_VERSION_1XX.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/venc.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 9981a2a..654bbaf 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -575,7 +575,9 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
>  				    struct v4l2_frmivalenum *fival)
>  {
>  	struct venus_inst *inst = to_inst(file);
> +	enum hfi_version ver = inst->core->res->hfi_version;
>  	const struct venus_format *fmt;
> +	unsigned int framerate_factor = 1;
>  
>  	fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
>  
> @@ -600,11 +602,16 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
>  	    fival->height < frame_height_min(inst))
>  		return -EINVAL;
>  
> -	fival->stepwise.min.numerator = 1;
> +	if (ver == HFI_VERSION_1XX) {

Could you use IS_V1(inst->core) macro instead.

> +		/* framerate is reported in 1/65535 fps unit */

I wonder isn't better to be consistent for all venus versions and return
1/1 to 1/120 instead of 65535/65535 to 65535/7864320?

> +		framerate_factor = (1 << 16);
> +	}
> +
> +	fival->stepwise.min.numerator = framerate_factor;
>  	fival->stepwise.min.denominator = frate_max(inst);
> -	fival->stepwise.max.numerator = 1;
> +	fival->stepwise.max.numerator = framerate_factor;
>  	fival->stepwise.max.denominator = frate_min(inst);
> -	fival->stepwise.step.numerator = 1;
> +	fival->stepwise.step.numerator = framerate_factor;
>  	fival->stepwise.step.denominator = frate_max(inst);
>  
>  	return 0;
> 

-- 
regards,
Stan