diff mbox

[media] exynos-gsc: Avoid spamming the log on VIDIOC_TRY_FMT

Message ID 1485294146-7467-1-git-send-email-javier@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Jan. 24, 2017, 9:42 p.m. UTC
There isn't an ioctl to enum the supported field orders, so a user-space
application can call VIDIOC_TRY_FMT using different field orders to know
if one is supported. For example, GStreamer does this so during playback
dozens of the following messages appear in the kernel log buffer:

[ 442.143393] Not supported field order(4)

Instead of printing this as an error, just keep it as debug information.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 drivers/media/platform/exynos-gsc/gsc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Shuah Khan Feb. 2, 2017, 2:25 a.m. UTC | #1
On 01/24/2017 02:42 PM, Javier Martinez Canillas wrote:
> There isn't an ioctl to enum the supported field orders, so a user-space
> application can call VIDIOC_TRY_FMT using different field orders to know
> if one is supported. For example, GStreamer does this so during playback
> dozens of the following messages appear in the kernel log buffer:
> 
> [ 442.143393] Not supported field order(4)
> 
> Instead of printing this as an error, just keep it as debug information.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
>  drivers/media/platform/exynos-gsc/gsc-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 8524fe15fa80..678b600f0500 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -408,7 +408,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>  	if (pix_mp->field == V4L2_FIELD_ANY)
>  		pix_mp->field = V4L2_FIELD_NONE;
>  	else if (pix_mp->field != V4L2_FIELD_NONE) {
> -		pr_err("Not supported field order(%d)\n", pix_mp->field);
> +		pr_debug("Not supported field order(%d)\n", pix_mp->field);

It make sense to leave it as an error, but print only once perhaps.
The down side to making this debug is that it becomes harder to
figure out when we run into this case.

thanks,
-- Shuah

>  		return -EINVAL;
>  	}
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Feb. 2, 2017, 12:21 p.m. UTC | #2
Hello Shuah,

On 02/01/2017 11:25 PM, Shuah Khan wrote:
> On 01/24/2017 02:42 PM, Javier Martinez Canillas wrote:
>> There isn't an ioctl to enum the supported field orders, so a user-space
>> application can call VIDIOC_TRY_FMT using different field orders to know
>> if one is supported. For example, GStreamer does this so during playback
>> dozens of the following messages appear in the kernel log buffer:
>>
>> [ 442.143393] Not supported field order(4)
>>
>> Instead of printing this as an error, just keep it as debug information.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> ---
>>
>>  drivers/media/platform/exynos-gsc/gsc-core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
>> index 8524fe15fa80..678b600f0500 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
>> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
>> @@ -408,7 +408,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>>  	if (pix_mp->field == V4L2_FIELD_ANY)
>>  		pix_mp->field = V4L2_FIELD_NONE;
>>  	else if (pix_mp->field != V4L2_FIELD_NONE) {
>> -		pr_err("Not supported field order(%d)\n", pix_mp->field);
>> +		pr_debug("Not supported field order(%d)\n", pix_mp->field);
> 
> It make sense to leave it as an error, but print only once perhaps.
> The down side to making this debug is that it becomes harder to
> figure out when we run into this case.
>

I disagree. User-space trying different field orders doesn't sound like an
error to me since as mentioned there isn't an ioctl to enum supported ones.

In fact other drivers don't print anything in their try_fmt handler for the
same case, for example [0] and [1].

> thanks,
> -- Shuah
> 

[0]: http://lxr.free-electrons.com/source/drivers/media/platform/mx2_emmaprp.c#L496
[1]: http://lxr.free-electrons.com/source/drivers/media/platform/exynos4-is/fimc-m2m.c#L297

Best regards,
diff mbox

Patch

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 8524fe15fa80..678b600f0500 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -408,7 +408,7 @@  int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
 	if (pix_mp->field == V4L2_FIELD_ANY)
 		pix_mp->field = V4L2_FIELD_NONE;
 	else if (pix_mp->field != V4L2_FIELD_NONE) {
-		pr_err("Not supported field order(%d)\n", pix_mp->field);
+		pr_debug("Not supported field order(%d)\n", pix_mp->field);
 		return -EINVAL;
 	}