linux-kernel - RE: [PATCH] media: aspeed: Fix dram hang at res-change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <TYZPR06MB656876892F6046F22AD87073F1DBA@TYZPR06MB6568.apcprd06.prod.outlook.com>
Date: Mon, 1 Dec 2025 01:34:25 +0000
From: Jammy Huang <jammy_huang@...eedtech.com>
To: 王敏 <wangmin@...tium.com.cn>
CC: Eddie James <eajames@...ux.ibm.com>, Mauro Carvalho Chehab
	<mchehab@...nel.org>, Joel Stanley <joel@....id.au>, Andrew Jeffery
	<andrew@...econstruct.com.au>, Philipp Zabel <p.zabel@...gutronix.de>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-aspeed@...ts.ozlabs.org"
	<linux-aspeed@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, 舒奕棋
	<shuyiqi@...tium.com.cn>
Subject: RE: [PATCH] media: aspeed: Fix dram hang at res-change

Hi Wang,

Thanks for your feedback.

Regards,
Jammy Huang

> >
> > Dram hang could happen in the steps below:
> > 1. start capture/compression
> > 2. out-of-lock watchdog raise irq because of res-change.
> > 3. aspeed_video_irq_res_change do clk-off
> >
> > At step3, capture/compression could be not accomplished yet. If
> > clk-off in the middle of video operation, dram controller could hang at
> ast2500.
> >
> > Use reset rather than clk-off/on to avoid this problem.
> >
> > Signed-off-by: Jammy Huang <jammy_huang@...eedtech.com>
> > ---
> > On Aspeed KVM testing, we found it could lead to dram-hang if
> > res-change. Although the issue rarely happens, the impact is serious.
> 
> Capturing and compressing the video stream takes longer than the video
> engine’s idle period.
Sorry, but this is not what I mean. The issue happens because video engine's clk
is turned off during capture/compression.

> If this is not the intended behavior, please increase the frame rate. This makes
> resolution switches more prone to happen when the video engine is working.
> However, according to your email, this issue rarely occurs. Is there a similar
> issue on the AST2600 SoC?
Increase frame rate would not helpful. This is a video compression engine. The time taken
for each frame's capture/compression is the same. The way to reproduce this issue
we did is continuously resolution-switch.

> 
> >
> > To avoid this issue, we use reset only rathar than clk-off/on in
> > res-change to avoid this issue.
> > ---
> >  drivers/media/platform/aspeed/aspeed-video.c | 22
> > +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/aspeed/aspeed-video.c
> > b/drivers/media/platform/aspeed/aspeed-video.c
> > index b83e432452..41cb96f601 100644
> > --- a/drivers/media/platform/aspeed/aspeed-video.c
> > +++ b/drivers/media/platform/aspeed/aspeed-video.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/workqueue.h>
> >  #include <linux/debugfs.h>
> >  #include <linux/ktime.h>
> > +#include <linux/reset.h>
> >  #include <linux/regmap.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <media/v4l2-ctrls.h>
> > @@ -310,6 +311,7 @@ struct aspeed_video {
> >  	void __iomem *base;
> >  	struct clk *eclk;
> >  	struct clk *vclk;
> > +	struct reset_control *reset;
> >
> >  	struct device *dev;
> >  	struct v4l2_ctrl_handler ctrl_handler; @@ -720,6 +722,13 @@ static
> > void aspeed_video_on(struct aspeed_video *video)
> >  	set_bit(VIDEO_CLOCKS_ON, &video->flags);  }
> >
> > +static void aspeed_video_reset(struct aspeed_video *v) {
> > +	reset_control_assert(v->reset);
> > +	usleep_range(100, 150);
> > +	reset_control_deassert(v->reset);
> > +}
> > +
> >  static void aspeed_video_bufs_done(struct aspeed_video *video,
> >  				   enum vb2_buffer_state state)
> >  {
> > @@ -742,7 +751,9 @@ static void aspeed_video_irq_res_change(struct
> > aspeed_video *video, ulong delay)
> >
> >  	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
> >
> > -	aspeed_video_off(video);
> > +	aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
> > +	aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
> > +	aspeed_video_reset(video);
> >  	aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
> >
> >  	schedule_delayed_work(&video->res_work, delay); @@ -1984,8 +1995,7
> > @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
> >  		 * Need to force stop any DMA and try and get HW into a good
> >  		 * state for future calls to start streaming again.
> >  		 */
> > -		aspeed_video_off(video);
> > -		aspeed_video_on(video);
> > +		aspeed_video_reset(video);
> >
> >  		aspeed_video_init_regs(video);
> >
> > @@ -2230,6 +2240,12 @@ static int aspeed_video_init(struct aspeed_video
> *video)
> >  	}
> >  	dev_info(video->dev, "irq %d\n", irq);
> >
> > +	video->reset = devm_reset_control_get(dev, NULL);
> > +	if (IS_ERR(video->reset)) {
> > +		dev_err(dev, "Unable to get reset\n");
> > +		return PTR_ERR(video->reset);
> > +	}
> > +
> >  	video->eclk = devm_clk_get(dev, "eclk");
> >  	if (IS_ERR(video->eclk)) {
> >  		dev_err(dev, "Unable to get ECLK\n");
> >
> > ---
> > base-commit: ac3fd01e4c1efce8f2c054cdeb2ddd2fc0fb150d
> > change-id: 20251124-video_dram_reset-c531f6ba573f
> >
> > Best regards,
> > --
> > Jammy Huang <jammy_huang@...eedtech.com>
> >
> 
> 
> 
> 
> 信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对
> 该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何
> 第三方组织和个人透露本邮件所含信息。
> Information Security Notice: The information contained in this mail is solely
> property of the sender's organization.This mail communication is
> confidential.Recipients named above are obligated to maintain secrecy and are
> not permitted to disclose the contents of this communication to others.