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

[王敏 <wangmin@phytium.com.cn>]

> -----原始邮件-----
> 发件人: "Jammy Huang" <jammy_huang@aspeedtech.com>
> 发送时间:2025-12-01 09:34:25 (星期一)
> 收件人: 王敏 <wangmin@phytium.com.cn>
> 抄送: "Eddie James" <eajames@linux.ibm.com>, "Mauro Carvalho Chehab" <mchehab@kernel.org>, "Joel Stanley" <joel@jms.id.au>, "Andrew Jeffery" <andrew@codeconstruct.com.au>, "Philipp Zabel" <p.zabel@pengutronix.de>, "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>, "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 舒奕棋 <shuyiqi@phytium.com.cn>
> 主题: 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@aspeedtech.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.
>
Thank you for the clarification.

I am encountering another issue related to resolution switching on the AST2500 SoC. 
When repeatedly switching from other resolutions to 1680x1050, or from 1680x1050 to 
other resolutions, there is a high likelihood that either the BMC OS will hang or 
the KVM screen will experience tearing. Could you please attempt to reproduce this 
issue and provide a resolution?

> > 
> > >
> > > 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@aspeedtech.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.