| 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
| ||
|
Message-ID:
<AS8PR04MB90802170205D9EA2AA3C1098FA34A@AS8PR04MB9080.eurprd04.prod.outlook.com>
Date: Fri, 15 Aug 2025 08:50:21 +0000
From: "G.N. Zhou (OSS)" <guoniu.zhou@....nxp.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>, "G.N. Zhou (OSS)"
<guoniu.zhou@....nxp.com>
CC: "rmfrfs@...il.com" <rmfrfs@...il.com>, "martink@...teo.de"
<martink@...teo.de>, "kernel@...i.sm" <kernel@...i.sm>, "mchehab@...nel.org"
<mchehab@...nel.org>, "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>, "festevam@...il.com"
<festevam@...il.com>, "linux-media@...r.kernel.org"
<linux-media@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] media: nxp: imx8-isi: Add suspend/resume support for ISI
mem2mem
Hi Laurent,
Thanks for your reviewing.
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Sent: Thursday, August 14, 2025 6:39 AM
> To: G.N. Zhou (OSS) <guoniu.zhou@....nxp.com>
> Cc: rmfrfs@...il.com; martink@...teo.de; kernel@...i.sm;
> mchehab@...nel.org; s.hauer@...gutronix.de; kernel@...gutronix.de;
> festevam@...il.com; linux-media@...r.kernel.org; imx@...ts.linux.dev; linux-
> arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH] media: nxp: imx8-isi: Add suspend/resume support for ISI
> mem2mem
>
> Hi Guoniu,
>
> Thank you for the patch.
>
> On Thu, Jul 17, 2025 at 10:43:54AM +0800, guoniu.zhou@....nxp.com wrote:
> > From: Guoniu Zhou <guoniu.zhou@....com>
> >
> > Add suspend/resume support for ISI when work at memory to memory mode.
> >
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@....com>
> > ---
> > .../platform/nxp/imx8-isi/imx8-isi-core.c | 8 ++++
> > .../platform/nxp/imx8-isi/imx8-isi-core.h | 11 +++++
> > .../platform/nxp/imx8-isi/imx8-isi-m2m.c | 48 +++++++++++++++++++
> > 3 files changed, 67 insertions(+)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > index 981648a03113..6eef45302e6c 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -372,6 +372,7 @@ static int mxc_isi_pm_suspend(struct device *dev)
> > struct mxc_isi_pipe *pipe = &isi->pipes[i];
> >
> > mxc_isi_video_suspend(pipe);
> > + mxc_isi_m2m_suspend(pipe);
> > }
>
> Given that only the first pipe can be used for M2M, I would move the
> mxc_isi_m2m_suspend() call after the loop, and pass it a &isi->m2m argument.
[G.N] Ok, will address in next version
>
> >
> > return pm_runtime_force_suspend(dev); @@ -401,6 +402,13 @@
> static
> > int mxc_isi_pm_resume(struct device *dev)
> > */
> > err = ret;
> > }
> > +
> > + ret = mxc_isi_m2m_resume(pipe);
> > + if (ret) {
> > + dev_err(dev, "Failed to resume ISI%u (%d) for m2m\n",
> i,
> > + ret);
> > + err = ret;
> > + }
>
> Same here, that can be moved after the loop.
>
> > }
> >
> > return err;
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > index 206995bedca4..b979b79b5525 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > @@ -343,6 +343,8 @@ int mxc_isi_video_buffer_prepare(struct
> > mxc_isi_dev *isi, struct vb2_buffer *vb2 #ifdef
> > CONFIG_VIDEO_IMX8_ISI_M2M int mxc_isi_m2m_register(struct
> mxc_isi_dev
> > *isi, struct v4l2_device *v4l2_dev); int
> > mxc_isi_m2m_unregister(struct mxc_isi_dev *isi);
> > +void mxc_isi_m2m_suspend(struct mxc_isi_pipe *pipe); int
> > +mxc_isi_m2m_resume(struct mxc_isi_pipe *pipe);
> > #else
> > static inline int mxc_isi_m2m_register(struct mxc_isi_dev *isi,
> > struct v4l2_device *v4l2_dev) @@ -353,6
> +355,15 @@ static
> > inline int mxc_isi_m2m_unregister(struct mxc_isi_dev *isi) {
> > return 0;
> > }
> > +
> > +static inline void mxc_isi_m2m_suspend(struct mxc_isi_pipe *pipe) { }
> > +
> > +static inline int mxc_isi_m2m_resume(struct mxc_isi_pipe *pipe) {
> > + return 0;
> > +}
> > #endif
> >
> > int mxc_isi_channel_acquire(struct mxc_isi_pipe *pipe, diff --git
> > a/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
> > index 22e49d3a1287..fe9d85335b6c 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
> > @@ -732,6 +732,54 @@ static const struct v4l2_file_operations
> mxc_isi_m2m_fops = {
> > .mmap = v4l2_m2m_fop_mmap,
> > };
> >
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * Suspend & resume
> > + */
> > +
> > +void mxc_isi_m2m_suspend(struct mxc_isi_pipe *pipe) {
> > + struct mxc_isi_m2m *m2m = &pipe->isi->m2m;
> > + struct mxc_isi_m2m_ctx *ctx = m2m->last_ctx;
> > +
> > + /*
> > + * Check pipe for ISI memory to memory since only
> > + * channel 0 support this feature.
> > + */
> > + if (m2m->pipe != pipe || m2m->usage_count == 0)
>
> If we pass the mxc_isi_m2m pointer to the function, you can drop the comment
> and the first part of the condition. Same in mxc_isi_m2m_resume().
[G.N] Ok, will address in next version.
>
> > + return;
> > +
> > + v4l2_m2m_suspend(m2m->m2m_dev);
> > +
> > + if (ctx->chained)
>
> Shouldn't you check m2m->chained_count instead ? Same in
> mxc_isi_m2m_resume().
[G.N] Agree with you.
>
> > + mxc_isi_channel_unchain(pipe);
> > +
> > + mxc_isi_channel_disable(pipe);
> > + mxc_isi_channel_put(pipe);
> > +}
> > +
> > +int mxc_isi_m2m_resume(struct mxc_isi_pipe *pipe) {
> > + struct mxc_isi_m2m *m2m = &pipe->isi->m2m;
> > + struct mxc_isi_m2m_ctx *ctx = m2m->last_ctx;
> > +
> > + /*
> > + * Check pipe for ISI memory to memory since only
> > + * channel 0 support this feature.
> > + */
> > + if (m2m->pipe != pipe || m2m->usage_count == 0)
> > + return 0;
> > +
> > + mxc_isi_channel_get(pipe);
> > +
> > + if (ctx->chained)
> > + mxc_isi_channel_chain(pipe, false);
> > +
> > + m2m->last_ctx = NULL;
> > + v4l2_m2m_resume(m2m->m2m_dev);
> > +
> > + return 0;
> > +}
> > +
> > /* -----------------------------------------------------------------------------
> > * Registration
> > */
>
> --
> Regards,
>
> Laurent Pinchart
Powered by blists - more mailing lists