* Re: [PATCH] media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam
@ 2022-11-23 8:08 ` Paul Menzel
0 siblings, 0 replies; 19+ messages in thread
From: Paul Menzel @ 2022-11-23 8:08 UTC (permalink / raw)
To: Jammy Huang
Cc: eajames, mchehab, joel, andrew, hverkuil-cisco, zev, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
Dear Jammy,
Am 10.11.22 um 10:56 schrieb Jammy Huang:
> If the host is powered off, there will be many warning log. To avoid the
…, many warnings are logged.
Also, please paste one example message.
Are the messages really just debug messages, or only in the one
condition? If the latter, another solution should be found, like
checking if the host is powered off, or rate limiting the message.
> log spam in this condition, replace v4l2_warn with v4l2_dbg.
Please add a reference, to Zev’s report on the mailing list.
Link: …
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
> drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed/aspeed-video.c b/drivers/media/platform/aspeed/aspeed-video.c
> index cf76aeee8cb6..662465d13a0e 100644
> --- a/drivers/media/platform/aspeed/aspeed-video.c
> +++ b/drivers/media/platform/aspeed/aspeed-video.c
> @@ -586,13 +586,13 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
> bool bcd_buf_need = (video->format != VIDEO_FMT_STANDARD);
>
> if (video->v4l2_input_status) {
> - v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "No signal; don't start frame\n");
> return 0;
> }
>
> if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
> !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
> - v4l2_warn(&video->v4l2_dev, "Engine busy; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Engine busy; don't start frame\n");
> return -EBUSY;
> }
>
> @@ -615,7 +615,7 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
> struct aspeed_video_buffer, link);
> if (!buf) {
> spin_unlock_irqrestore(&video->lock, flags);
> - v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "No buffers; don't start frame\n");
> return -EPROTO;
> }
>
> @@ -796,7 +796,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
> if (video->format == VIDEO_FMT_STANDARD &&
> list_is_last(&buf->link, &video->buffers)) {
> empty = false;
> - v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "skip to keep last frame updated\n");
> } else {
> buf->vb.vb2_buf.timestamp = ktime_get_ns();
> buf->vb.sequence = video->sequence++;
> @@ -1060,7 +1060,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> res_check(video),
> MODE_DETECT_TIMEOUT);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out; first mode detect\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; first mode detect\n");
> clear_bit(VIDEO_RES_DETECT, &video->flags);
> return;
> }
> @@ -1081,7 +1081,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> MODE_DETECT_TIMEOUT);
> clear_bit(VIDEO_RES_DETECT, &video->flags);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out; second mode detect\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; second mode detect\n");
> return;
> }
>
> @@ -1104,7 +1104,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> } while (invalid_resolution && (tries++ < INVALID_RESOLUTION_RETRIES));
>
> if (invalid_resolution) {
> - v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Invalid resolution detected\n");
> return;
> }
>
> @@ -1856,7 +1856,7 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
> !test_bit(VIDEO_FRAME_INPRG, &video->flags),
> STOP_TIMEOUT);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out when stopping streaming\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out when stopping streaming\n");
>
> /*
> * Need to force stop any DMA and try and get HW into a good
>
> base-commit: aae703b02f92bde9264366c545e87cec451de471
> prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b
> prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9
> prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f
> prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af
> prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam
@ 2022-11-23 8:08 ` Paul Menzel
0 siblings, 0 replies; 19+ messages in thread
From: Paul Menzel @ 2022-11-23 8:08 UTC (permalink / raw)
To: Jammy Huang
Cc: eajames, mchehab, joel, andrew, hverkuil-cisco, zev, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
Dear Jammy,
Am 10.11.22 um 10:56 schrieb Jammy Huang:
> If the host is powered off, there will be many warning log. To avoid the
…, many warnings are logged.
Also, please paste one example message.
Are the messages really just debug messages, or only in the one
condition? If the latter, another solution should be found, like
checking if the host is powered off, or rate limiting the message.
> log spam in this condition, replace v4l2_warn with v4l2_dbg.
Please add a reference, to Zev’s report on the mailing list.
Link: …
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
> drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed/aspeed-video.c b/drivers/media/platform/aspeed/aspeed-video.c
> index cf76aeee8cb6..662465d13a0e 100644
> --- a/drivers/media/platform/aspeed/aspeed-video.c
> +++ b/drivers/media/platform/aspeed/aspeed-video.c
> @@ -586,13 +586,13 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
> bool bcd_buf_need = (video->format != VIDEO_FMT_STANDARD);
>
> if (video->v4l2_input_status) {
> - v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "No signal; don't start frame\n");
> return 0;
> }
>
> if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
> !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
> - v4l2_warn(&video->v4l2_dev, "Engine busy; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Engine busy; don't start frame\n");
> return -EBUSY;
> }
>
> @@ -615,7 +615,7 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
> struct aspeed_video_buffer, link);
> if (!buf) {
> spin_unlock_irqrestore(&video->lock, flags);
> - v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "No buffers; don't start frame\n");
> return -EPROTO;
> }
>
> @@ -796,7 +796,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
> if (video->format == VIDEO_FMT_STANDARD &&
> list_is_last(&buf->link, &video->buffers)) {
> empty = false;
> - v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "skip to keep last frame updated\n");
> } else {
> buf->vb.vb2_buf.timestamp = ktime_get_ns();
> buf->vb.sequence = video->sequence++;
> @@ -1060,7 +1060,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> res_check(video),
> MODE_DETECT_TIMEOUT);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out; first mode detect\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; first mode detect\n");
> clear_bit(VIDEO_RES_DETECT, &video->flags);
> return;
> }
> @@ -1081,7 +1081,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> MODE_DETECT_TIMEOUT);
> clear_bit(VIDEO_RES_DETECT, &video->flags);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out; second mode detect\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; second mode detect\n");
> return;
> }
>
> @@ -1104,7 +1104,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> } while (invalid_resolution && (tries++ < INVALID_RESOLUTION_RETRIES));
>
> if (invalid_resolution) {
> - v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Invalid resolution detected\n");
> return;
> }
>
> @@ -1856,7 +1856,7 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
> !test_bit(VIDEO_FRAME_INPRG, &video->flags),
> STOP_TIMEOUT);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out when stopping streaming\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out when stopping streaming\n");
>
> /*
> * Need to force stop any DMA and try and get HW into a good
>
> base-commit: aae703b02f92bde9264366c545e87cec451de471
> prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b
> prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9
> prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f
> prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af
> prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam
@ 2022-11-23 8:08 ` Paul Menzel
0 siblings, 0 replies; 19+ messages in thread
From: Paul Menzel @ 2022-11-23 8:08 UTC (permalink / raw)
To: Jammy Huang
Cc: linux-aspeed, zev, andrew, openbmc, eajames, linux-kernel, joel,
hverkuil-cisco, mchehab, linux-arm-kernel, linux-media
Dear Jammy,
Am 10.11.22 um 10:56 schrieb Jammy Huang:
> If the host is powered off, there will be many warning log. To avoid the
…, many warnings are logged.
Also, please paste one example message.
Are the messages really just debug messages, or only in the one
condition? If the latter, another solution should be found, like
checking if the host is powered off, or rate limiting the message.
> log spam in this condition, replace v4l2_warn with v4l2_dbg.
Please add a reference, to Zev’s report on the mailing list.
Link: …
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
> drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed/aspeed-video.c b/drivers/media/platform/aspeed/aspeed-video.c
> index cf76aeee8cb6..662465d13a0e 100644
> --- a/drivers/media/platform/aspeed/aspeed-video.c
> +++ b/drivers/media/platform/aspeed/aspeed-video.c
> @@ -586,13 +586,13 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
> bool bcd_buf_need = (video->format != VIDEO_FMT_STANDARD);
>
> if (video->v4l2_input_status) {
> - v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "No signal; don't start frame\n");
> return 0;
> }
>
> if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
> !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
> - v4l2_warn(&video->v4l2_dev, "Engine busy; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Engine busy; don't start frame\n");
> return -EBUSY;
> }
>
> @@ -615,7 +615,7 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
> struct aspeed_video_buffer, link);
> if (!buf) {
> spin_unlock_irqrestore(&video->lock, flags);
> - v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "No buffers; don't start frame\n");
> return -EPROTO;
> }
>
> @@ -796,7 +796,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
> if (video->format == VIDEO_FMT_STANDARD &&
> list_is_last(&buf->link, &video->buffers)) {
> empty = false;
> - v4l2_warn(&video->v4l2_dev, "skip to keep last frame updated\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "skip to keep last frame updated\n");
> } else {
> buf->vb.vb2_buf.timestamp = ktime_get_ns();
> buf->vb.sequence = video->sequence++;
> @@ -1060,7 +1060,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> res_check(video),
> MODE_DETECT_TIMEOUT);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out; first mode detect\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; first mode detect\n");
> clear_bit(VIDEO_RES_DETECT, &video->flags);
> return;
> }
> @@ -1081,7 +1081,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> MODE_DETECT_TIMEOUT);
> clear_bit(VIDEO_RES_DETECT, &video->flags);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out; second mode detect\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; second mode detect\n");
> return;
> }
>
> @@ -1104,7 +1104,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
> } while (invalid_resolution && (tries++ < INVALID_RESOLUTION_RETRIES));
>
> if (invalid_resolution) {
> - v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Invalid resolution detected\n");
> return;
> }
>
> @@ -1856,7 +1856,7 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
> !test_bit(VIDEO_FRAME_INPRG, &video->flags),
> STOP_TIMEOUT);
> if (!rc) {
> - v4l2_warn(&video->v4l2_dev, "Timed out when stopping streaming\n");
> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out when stopping streaming\n");
>
> /*
> * Need to force stop any DMA and try and get HW into a good
>
> base-commit: aae703b02f92bde9264366c545e87cec451de471
> prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b
> prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9
> prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f
> prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af
> prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH] media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam
2022-11-23 8:08 ` Paul Menzel
(?)
(?)
@ 2022-11-25 3:23 ` Jammy Huang
-1 siblings, 0 replies; 19+ messages in thread
From: Jammy Huang @ 2022-11-25 3:23 UTC (permalink / raw)
To: linux-aspeed
Hi Paul,
On 2022/11/23 ?? 04:08, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 10.11.22 um 10:56 schrieb Jammy Huang:
>> If the host is powered off, there will be many warning log. To avoid the
>
> ?, many warnings are logged.
>
> Also, please paste one example message.
OK
>
> Are the messages really just debug messages, or only in the one
> condition? If the latter, another solution should be found, like
> checking if the host is powered off, or rate limiting the message.
In my opinion, these logs are the former one.
video: Timed out; first mode detect
=> This would happen if timing detection failed.
No signal; don't start frame
=> This would happen when timing-detection is not finished with valid
resolution detected.
>
>> log spam in this condition, replace v4l2_warn with v4l2_dbg.
>
> Please add a reference, to Zev?s report on the mailing list.
>
> Link: ?
OK, thanks.
>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>> ? drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++--------
>> ? 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed/aspeed-video.c
>> b/drivers/media/platform/aspeed/aspeed-video.c
>> index cf76aeee8cb6..662465d13a0e 100644
>> --- a/drivers/media/platform/aspeed/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed/aspeed-video.c
>> @@ -586,13 +586,13 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> ????? bool bcd_buf_need = (video->format != VIDEO_FMT_STANDARD);
>> ? ????? if (video->v4l2_input_status) {
>> -??????? v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
>> +??????? v4l2_dbg(1, debug, &video->v4l2_dev, "No signal; don't start
>> frame\n");
>> ????????? return 0;
>> ????? }
>> ? ????? if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
>> ????????? !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
>> -??????? v4l2_warn(&video->v4l2_dev, "Engine busy; don't start
>> frame\n");
>> +??????? v4l2_dbg(1, debug, &video->v4l2_dev, "Engine busy; don't
>> start frame\n");
>> ????????? return -EBUSY;
>> ????? }
>> ? @@ -615,7 +615,7 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> ???????????????????????? struct aspeed_video_buffer, link);
>> ????? if (!buf) {
>> ????????? spin_unlock_irqrestore(&video->lock, flags);
>> -??????? v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
>> +??????? v4l2_dbg(1, debug, &video->v4l2_dev, "No buffers; don't
>> start frame\n");
>> ????????? return -EPROTO;
>> ????? }
>> ? @@ -796,7 +796,7 @@ static irqreturn_t aspeed_video_irq(int irq,
>> void *arg)
>> ????????????? if (video->format == VIDEO_FMT_STANDARD &&
>> ????????????????? list_is_last(&buf->link, &video->buffers)) {
>> ????????????????? empty = false;
>> -??????????????? v4l2_warn(&video->v4l2_dev, "skip to keep last frame
>> updated\n");
>> +??????????????? v4l2_dbg(1, debug, &video->v4l2_dev, "skip to keep
>> last frame updated\n");
>> ????????????? } else {
>> ????????????????? buf->vb.vb2_buf.timestamp = ktime_get_ns();
>> ????????????????? buf->vb.sequence = video->sequence++;
>> @@ -1060,7 +1060,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> ??????????????????????????????? res_check(video),
>> ??????????????????????????????? MODE_DETECT_TIMEOUT);
>> ????????? if (!rc) {
>> -??????????? v4l2_warn(&video->v4l2_dev, "Timed out; first mode
>> detect\n");
>> +??????????? v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; first
>> mode detect\n");
>> ????????????? clear_bit(VIDEO_RES_DETECT, &video->flags);
>> ????????????? return;
>> ????????? }
>> @@ -1081,7 +1081,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> ??????????????????????????????? MODE_DETECT_TIMEOUT);
>> ????????? clear_bit(VIDEO_RES_DETECT, &video->flags);
>> ????????? if (!rc) {
>> -??????????? v4l2_warn(&video->v4l2_dev, "Timed out; second mode
>> detect\n");
>> +??????????? v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; second
>> mode detect\n");
>> ????????????? return;
>> ????????? }
>> ? @@ -1104,7 +1104,7 @@ static void
>> aspeed_video_get_resolution(struct aspeed_video *video)
>> ????? } while (invalid_resolution && (tries++ <
>> INVALID_RESOLUTION_RETRIES));
>> ? ????? if (invalid_resolution) {
>> -??????? v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
>> +??????? v4l2_dbg(1, debug, &video->v4l2_dev, "Invalid resolution
>> detected\n");
>> ????????? return;
>> ????? }
>> ? @@ -1856,7 +1856,7 @@ static void
>> aspeed_video_stop_streaming(struct vb2_queue *q)
>> ????????????????? !test_bit(VIDEO_FRAME_INPRG, &video->flags),
>> ????????????????? STOP_TIMEOUT);
>> ????? if (!rc) {
>> -??????? v4l2_warn(&video->v4l2_dev, "Timed out when stopping
>> streaming\n");
>> +??????? v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out when
>> stopping streaming\n");
>> ? ????????? /*
>> ?????????? * Need to force stop any DMA and try and get HW into a good
>>
>> base-commit: aae703b02f92bde9264366c545e87cec451de471
>> prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b
>> prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9
>> prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f
>> prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af
>> prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f
--
Best Regards
Jammy
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam
@ 2022-11-25 3:23 ` Jammy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Jammy Huang @ 2022-11-25 3:23 UTC (permalink / raw)
To: Paul Menzel
Cc: eajames, mchehab, joel, andrew, hverkuil-cisco, zev, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
Hi Paul,
On 2022/11/23 下午 04:08, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 10.11.22 um 10:56 schrieb Jammy Huang:
>> If the host is powered off, there will be many warning log. To avoid the
>
> …, many warnings are logged.
>
> Also, please paste one example message.
OK
>
> Are the messages really just debug messages, or only in the one
> condition? If the latter, another solution should be found, like
> checking if the host is powered off, or rate limiting the message.
In my opinion, these logs are the former one.
video: Timed out; first mode detect
=> This would happen if timing detection failed.
No signal; don't start frame
=> This would happen when timing-detection is not finished with valid
resolution detected.
>
>> log spam in this condition, replace v4l2_warn with v4l2_dbg.
>
> Please add a reference, to Zev’s report on the mailing list.
>
> Link: …
OK, thanks.
>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>> drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed/aspeed-video.c
>> b/drivers/media/platform/aspeed/aspeed-video.c
>> index cf76aeee8cb6..662465d13a0e 100644
>> --- a/drivers/media/platform/aspeed/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed/aspeed-video.c
>> @@ -586,13 +586,13 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> bool bcd_buf_need = (video->format != VIDEO_FMT_STANDARD);
>> if (video->v4l2_input_status) {
>> - v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "No signal; don't start
>> frame\n");
>> return 0;
>> }
>> if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
>> !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
>> - v4l2_warn(&video->v4l2_dev, "Engine busy; don't start
>> frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Engine busy; don't
>> start frame\n");
>> return -EBUSY;
>> }
>> @@ -615,7 +615,7 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> struct aspeed_video_buffer, link);
>> if (!buf) {
>> spin_unlock_irqrestore(&video->lock, flags);
>> - v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "No buffers; don't
>> start frame\n");
>> return -EPROTO;
>> }
>> @@ -796,7 +796,7 @@ static irqreturn_t aspeed_video_irq(int irq,
>> void *arg)
>> if (video->format == VIDEO_FMT_STANDARD &&
>> list_is_last(&buf->link, &video->buffers)) {
>> empty = false;
>> - v4l2_warn(&video->v4l2_dev, "skip to keep last frame
>> updated\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "skip to keep
>> last frame updated\n");
>> } else {
>> buf->vb.vb2_buf.timestamp = ktime_get_ns();
>> buf->vb.sequence = video->sequence++;
>> @@ -1060,7 +1060,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> res_check(video),
>> MODE_DETECT_TIMEOUT);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out; first mode
>> detect\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; first
>> mode detect\n");
>> clear_bit(VIDEO_RES_DETECT, &video->flags);
>> return;
>> }
>> @@ -1081,7 +1081,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> MODE_DETECT_TIMEOUT);
>> clear_bit(VIDEO_RES_DETECT, &video->flags);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out; second mode
>> detect\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; second
>> mode detect\n");
>> return;
>> }
>> @@ -1104,7 +1104,7 @@ static void
>> aspeed_video_get_resolution(struct aspeed_video *video)
>> } while (invalid_resolution && (tries++ <
>> INVALID_RESOLUTION_RETRIES));
>> if (invalid_resolution) {
>> - v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Invalid resolution
>> detected\n");
>> return;
>> }
>> @@ -1856,7 +1856,7 @@ static void
>> aspeed_video_stop_streaming(struct vb2_queue *q)
>> !test_bit(VIDEO_FRAME_INPRG, &video->flags),
>> STOP_TIMEOUT);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out when stopping
>> streaming\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out when
>> stopping streaming\n");
>> /*
>> * Need to force stop any DMA and try and get HW into a good
>>
>> base-commit: aae703b02f92bde9264366c545e87cec451de471
>> prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b
>> prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9
>> prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f
>> prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af
>> prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f
--
Best Regards
Jammy
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam
@ 2022-11-25 3:23 ` Jammy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Jammy Huang @ 2022-11-25 3:23 UTC (permalink / raw)
To: Paul Menzel
Cc: eajames, mchehab, joel, andrew, hverkuil-cisco, zev, linux-media,
openbmc, linux-arm-kernel, linux-aspeed, linux-kernel
Hi Paul,
On 2022/11/23 下午 04:08, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 10.11.22 um 10:56 schrieb Jammy Huang:
>> If the host is powered off, there will be many warning log. To avoid the
>
> …, many warnings are logged.
>
> Also, please paste one example message.
OK
>
> Are the messages really just debug messages, or only in the one
> condition? If the latter, another solution should be found, like
> checking if the host is powered off, or rate limiting the message.
In my opinion, these logs are the former one.
video: Timed out; first mode detect
=> This would happen if timing detection failed.
No signal; don't start frame
=> This would happen when timing-detection is not finished with valid
resolution detected.
>
>> log spam in this condition, replace v4l2_warn with v4l2_dbg.
>
> Please add a reference, to Zev’s report on the mailing list.
>
> Link: …
OK, thanks.
>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>> drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed/aspeed-video.c
>> b/drivers/media/platform/aspeed/aspeed-video.c
>> index cf76aeee8cb6..662465d13a0e 100644
>> --- a/drivers/media/platform/aspeed/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed/aspeed-video.c
>> @@ -586,13 +586,13 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> bool bcd_buf_need = (video->format != VIDEO_FMT_STANDARD);
>> if (video->v4l2_input_status) {
>> - v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "No signal; don't start
>> frame\n");
>> return 0;
>> }
>> if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
>> !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
>> - v4l2_warn(&video->v4l2_dev, "Engine busy; don't start
>> frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Engine busy; don't
>> start frame\n");
>> return -EBUSY;
>> }
>> @@ -615,7 +615,7 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> struct aspeed_video_buffer, link);
>> if (!buf) {
>> spin_unlock_irqrestore(&video->lock, flags);
>> - v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "No buffers; don't
>> start frame\n");
>> return -EPROTO;
>> }
>> @@ -796,7 +796,7 @@ static irqreturn_t aspeed_video_irq(int irq,
>> void *arg)
>> if (video->format == VIDEO_FMT_STANDARD &&
>> list_is_last(&buf->link, &video->buffers)) {
>> empty = false;
>> - v4l2_warn(&video->v4l2_dev, "skip to keep last frame
>> updated\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "skip to keep
>> last frame updated\n");
>> } else {
>> buf->vb.vb2_buf.timestamp = ktime_get_ns();
>> buf->vb.sequence = video->sequence++;
>> @@ -1060,7 +1060,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> res_check(video),
>> MODE_DETECT_TIMEOUT);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out; first mode
>> detect\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; first
>> mode detect\n");
>> clear_bit(VIDEO_RES_DETECT, &video->flags);
>> return;
>> }
>> @@ -1081,7 +1081,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> MODE_DETECT_TIMEOUT);
>> clear_bit(VIDEO_RES_DETECT, &video->flags);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out; second mode
>> detect\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; second
>> mode detect\n");
>> return;
>> }
>> @@ -1104,7 +1104,7 @@ static void
>> aspeed_video_get_resolution(struct aspeed_video *video)
>> } while (invalid_resolution && (tries++ <
>> INVALID_RESOLUTION_RETRIES));
>> if (invalid_resolution) {
>> - v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Invalid resolution
>> detected\n");
>> return;
>> }
>> @@ -1856,7 +1856,7 @@ static void
>> aspeed_video_stop_streaming(struct vb2_queue *q)
>> !test_bit(VIDEO_FRAME_INPRG, &video->flags),
>> STOP_TIMEOUT);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out when stopping
>> streaming\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out when
>> stopping streaming\n");
>> /*
>> * Need to force stop any DMA and try and get HW into a good
>>
>> base-commit: aae703b02f92bde9264366c545e87cec451de471
>> prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b
>> prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9
>> prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f
>> prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af
>> prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f
--
Best Regards
Jammy
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam
@ 2022-11-25 3:23 ` Jammy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Jammy Huang @ 2022-11-25 3:23 UTC (permalink / raw)
To: Paul Menzel
Cc: linux-aspeed, zev, andrew, openbmc, eajames, linux-kernel, joel,
hverkuil-cisco, mchehab, linux-arm-kernel, linux-media
Hi Paul,
On 2022/11/23 下午 04:08, Paul Menzel wrote:
> Dear Jammy,
>
>
> Am 10.11.22 um 10:56 schrieb Jammy Huang:
>> If the host is powered off, there will be many warning log. To avoid the
>
> …, many warnings are logged.
>
> Also, please paste one example message.
OK
>
> Are the messages really just debug messages, or only in the one
> condition? If the latter, another solution should be found, like
> checking if the host is powered off, or rate limiting the message.
In my opinion, these logs are the former one.
video: Timed out; first mode detect
=> This would happen if timing detection failed.
No signal; don't start frame
=> This would happen when timing-detection is not finished with valid
resolution detected.
>
>> log spam in this condition, replace v4l2_warn with v4l2_dbg.
>
> Please add a reference, to Zev’s report on the mailing list.
>
> Link: …
OK, thanks.
>
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>> drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed/aspeed-video.c
>> b/drivers/media/platform/aspeed/aspeed-video.c
>> index cf76aeee8cb6..662465d13a0e 100644
>> --- a/drivers/media/platform/aspeed/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed/aspeed-video.c
>> @@ -586,13 +586,13 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> bool bcd_buf_need = (video->format != VIDEO_FMT_STANDARD);
>> if (video->v4l2_input_status) {
>> - v4l2_warn(&video->v4l2_dev, "No signal; don't start frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "No signal; don't start
>> frame\n");
>> return 0;
>> }
>> if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
>> !(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
>> - v4l2_warn(&video->v4l2_dev, "Engine busy; don't start
>> frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Engine busy; don't
>> start frame\n");
>> return -EBUSY;
>> }
>> @@ -615,7 +615,7 @@ static int aspeed_video_start_frame(struct
>> aspeed_video *video)
>> struct aspeed_video_buffer, link);
>> if (!buf) {
>> spin_unlock_irqrestore(&video->lock, flags);
>> - v4l2_warn(&video->v4l2_dev, "No buffers; don't start frame\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "No buffers; don't
>> start frame\n");
>> return -EPROTO;
>> }
>> @@ -796,7 +796,7 @@ static irqreturn_t aspeed_video_irq(int irq,
>> void *arg)
>> if (video->format == VIDEO_FMT_STANDARD &&
>> list_is_last(&buf->link, &video->buffers)) {
>> empty = false;
>> - v4l2_warn(&video->v4l2_dev, "skip to keep last frame
>> updated\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "skip to keep
>> last frame updated\n");
>> } else {
>> buf->vb.vb2_buf.timestamp = ktime_get_ns();
>> buf->vb.sequence = video->sequence++;
>> @@ -1060,7 +1060,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> res_check(video),
>> MODE_DETECT_TIMEOUT);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out; first mode
>> detect\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; first
>> mode detect\n");
>> clear_bit(VIDEO_RES_DETECT, &video->flags);
>> return;
>> }
>> @@ -1081,7 +1081,7 @@ static void aspeed_video_get_resolution(struct
>> aspeed_video *video)
>> MODE_DETECT_TIMEOUT);
>> clear_bit(VIDEO_RES_DETECT, &video->flags);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out; second mode
>> detect\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out; second
>> mode detect\n");
>> return;
>> }
>> @@ -1104,7 +1104,7 @@ static void
>> aspeed_video_get_resolution(struct aspeed_video *video)
>> } while (invalid_resolution && (tries++ <
>> INVALID_RESOLUTION_RETRIES));
>> if (invalid_resolution) {
>> - v4l2_warn(&video->v4l2_dev, "Invalid resolution detected\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Invalid resolution
>> detected\n");
>> return;
>> }
>> @@ -1856,7 +1856,7 @@ static void
>> aspeed_video_stop_streaming(struct vb2_queue *q)
>> !test_bit(VIDEO_FRAME_INPRG, &video->flags),
>> STOP_TIMEOUT);
>> if (!rc) {
>> - v4l2_warn(&video->v4l2_dev, "Timed out when stopping
>> streaming\n");
>> + v4l2_dbg(1, debug, &video->v4l2_dev, "Timed out when
>> stopping streaming\n");
>> /*
>> * Need to force stop any DMA and try and get HW into a good
>>
>> base-commit: aae703b02f92bde9264366c545e87cec451de471
>> prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b
>> prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9
>> prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f
>> prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af
>> prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f
--
Best Regards
Jammy
^ permalink raw reply [flat|nested] 19+ messages in thread