| Message ID | 20221110095611.522-1-jammy_huang@aspeedtech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: aspeed: Use v4l2_dbg to replace v4l2_warn to avoid log spam | expand |
On Thu, Nov 10, 2022 at 01:56:11AM PST, Jammy Huang wrote: >If the host is powered off, there will be many warning log. To avoid the >log spam in this condition, replace v4l2_warn with v4l2_dbg. > >Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com> >--- > drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > Thanks Jammy -- after dropping the one hunk that wasn't applicable (due to not having all the prerequisite patches) this definitely fixed the log noise problems I'd been having. I'm assuming the prerequisite patches are from your aspeed-jpeg support patch series? If so, since this is a smaller/simpler patch, I think it might make sense to reorder things to try to get this merged first and then fixing up that patchset to use v4l2_dbg() instead of v4l2_warn(), rather than introducing a new instance of it and then patching it back out later with this one. Thanks, Zev
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
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
Hi Zev, On 2022/11/23 上午 07:12, Zev Weiss wrote: > On Thu, Nov 10, 2022 at 01:56:11AM PST, Jammy Huang wrote: >> If the host is powered off, there will be many warning log. To avoid the >> log spam in this condition, replace v4l2_warn with v4l2_dbg. >> >> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com> >> --- >> drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> > Thanks Jammy -- after dropping the one hunk that wasn't applicable (due > to not having all the prerequisite patches) this definitely fixed the > log noise problems I'd been having. > > I'm assuming the prerequisite patches are from your aspeed-jpeg support > patch series? If so, since this is a smaller/simpler patch, I think it > might make sense to reorder things to try to get this merged first and > then fixing up that patchset to use v4l2_dbg() instead of v4l2_warn(), > rather than introducing a new instance of it and then patching it back > out later with this one. Yes, the prerequisite patches are 'aspeed-jpeg support' patch series. And that patch series has been accepted and pull for v6.2. Your suggestion is better to decouple these patches. I would pay attention to the problem like this to avoid this condition in the future. Many thanks for your kindly help. > > > Thanks, > Zev
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
If the host is powered off, there will be many warning log. To avoid the log spam in this condition, replace v4l2_warn with v4l2_dbg. Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com> --- drivers/media/platform/aspeed/aspeed-video.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) base-commit: aae703b02f92bde9264366c545e87cec451de471 prerequisite-patch-id: bf47e8ab2998acfbc32be5a4b7b5ae8a3ae4218b prerequisite-patch-id: bf82715983e08f2e810ff1a82ce644f5f9006cd9 prerequisite-patch-id: 28a2040ef0235e5765f05d2fc5529bce2a0f4c6f prerequisite-patch-id: 7e761c779730536db8baf50db5fc8caf058e95af prerequisite-patch-id: c48ea20973fa35938a7d33a0e20d2900df48755f