2022-11-16 13:20:27

by Usyskin, Alexander

[permalink] [raw]
Subject: [PATCH v3 0/2] mei: add timeout to send

When driver wakes up the firmware from the low power state,
it is sending a memory ready message.
The send is done via synchronous/blocking function to ensure
that firmware is in ready state. However, in case of firmware
undergoing reset send might be block forever.
To address this issue a timeout is added to blocking
write command on the internal bus.

Introduce the __mei_cl_send_timeout function to use instead of
__mei_cl_send in cases where timeout is required.
The mei_cl_write has only two callers and there is no need to split
it into two functions.

V2: address review comments:
- split __mei_cl_send and __mei_cl_send_timeout
- add units to timeout KDoc
- use MAX_SCHEDULE_TIMEOUT to squash wait to one macro

V3: - split the state fix into separate patch
- document define unit
- expand timeout KDoc

Alexander Usyskin (2):
mei: add timeout to send
mei: bus-fixup: change pxp mode only if message was sent

drivers/misc/mei/bus-fixup.c | 14 +++++++++-----
drivers/misc/mei/bus.c | 22 +++++++++++++++++++++-
drivers/misc/mei/client.c | 20 ++++++++++++++++----
drivers/misc/mei/client.h | 2 +-
drivers/misc/mei/main.c | 2 +-
drivers/misc/mei/mei_dev.h | 2 ++
6 files changed, 50 insertions(+), 12 deletions(-)

--
2.34.1



2022-11-16 13:42:56

by Usyskin, Alexander

[permalink] [raw]
Subject: [PATCH v3 2/2] mei: bus-fixup: change pxp mode only if message was sent

Move PXP mode state machine to SETUP mode only if
memory ready message sent successfully to the firmware.
Leave it in INIT mode otherwise to allow try to send message later.

Signed-off-by: Alexander Usyskin <[email protected]>
---
drivers/misc/mei/bus-fixup.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index 90023c34666e..6df7679d9739 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -266,12 +266,13 @@ static void mei_gsc_mkhi_fix_ver(struct mei_cl_device *cldev)

if (cldev->bus->pxp_mode == MEI_DEV_PXP_INIT) {
ret = mei_gfx_memory_ready(cldev);
- if (ret < 0)
+ if (ret < 0) {
dev_err(&cldev->dev, "memory ready command failed %d\n", ret);
- else
+ } else {
dev_dbg(&cldev->dev, "memory ready command sent\n");
+ cldev->bus->pxp_mode = MEI_DEV_PXP_SETUP;
+ }
/* we go to reset after that */
- cldev->bus->pxp_mode = MEI_DEV_PXP_SETUP;
goto out;
}

--
2.34.1


2022-11-30 14:34:15

by Rodrigo Vivi

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH v3 0/2] mei: add timeout to send

On Wed, Nov 16, 2022 at 02:47:33PM +0200, Alexander Usyskin wrote:
> When driver wakes up the firmware from the low power state,
> it is sending a memory ready message.
> The send is done via synchronous/blocking function to ensure
> that firmware is in ready state. However, in case of firmware
> undergoing reset send might be block forever.
> To address this issue a timeout is added to blocking
> write command on the internal bus.
>
> Introduce the __mei_cl_send_timeout function to use instead of
> __mei_cl_send in cases where timeout is required.
> The mei_cl_write has only two callers and there is no need to split
> it into two functions.
>
> V2: address review comments:
> - split __mei_cl_send and __mei_cl_send_timeout
> - add units to timeout KDoc
> - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
>
> V3: - split the state fix into separate patch
> - document define unit
> - expand timeout KDoc

These 2 patches looks good to me now.

Greg, whenever you review it, please let me know if it is
okay to me to push these through the drm-fixes, or if you
prefer these to the mei branches.

Btw, Alexander, would we have a good "Fixes:" tag for these
patches?

Thanks,
Rodrigo.

>
> Alexander Usyskin (2):
> mei: add timeout to send
> mei: bus-fixup: change pxp mode only if message was sent
>
> drivers/misc/mei/bus-fixup.c | 14 +++++++++-----
> drivers/misc/mei/bus.c | 22 +++++++++++++++++++++-
> drivers/misc/mei/client.c | 20 ++++++++++++++++----
> drivers/misc/mei/client.h | 2 +-
> drivers/misc/mei/main.c | 2 +-
> drivers/misc/mei/mei_dev.h | 2 ++
> 6 files changed, 50 insertions(+), 12 deletions(-)
>
> --
> 2.34.1
>

2022-11-30 18:08:52

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH v3 0/2] mei: add timeout to send

On Wed, Nov 30, 2022 at 09:20:28AM -0500, Rodrigo Vivi wrote:
> On Wed, Nov 16, 2022 at 02:47:33PM +0200, Alexander Usyskin wrote:
> > When driver wakes up the firmware from the low power state,
> > it is sending a memory ready message.
> > The send is done via synchronous/blocking function to ensure
> > that firmware is in ready state. However, in case of firmware
> > undergoing reset send might be block forever.
> > To address this issue a timeout is added to blocking
> > write command on the internal bus.
> >
> > Introduce the __mei_cl_send_timeout function to use instead of
> > __mei_cl_send in cases where timeout is required.
> > The mei_cl_write has only two callers and there is no need to split
> > it into two functions.
> >
> > V2: address review comments:
> > - split __mei_cl_send and __mei_cl_send_timeout
> > - add units to timeout KDoc
> > - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
> >
> > V3: - split the state fix into separate patch
> > - document define unit
> > - expand timeout KDoc
>
> These 2 patches looks good to me now.
>
> Greg, whenever you review it, please let me know if it is
> okay to me to push these through the drm-fixes, or if you
> prefer these to the mei branches.

These have been in my tree for over a week now, sorry. No one told me
not to take them...

{sigh}

greg k-h

2022-11-30 19:16:35

by Rodrigo Vivi

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH v3 0/2] mei: add timeout to send

On Wed, 2022-11-30 at 18:43 +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 30, 2022 at 09:20:28AM -0500, Rodrigo Vivi wrote:
> > On Wed, Nov 16, 2022 at 02:47:33PM +0200, Alexander Usyskin wrote:
> > > When driver wakes up the firmware from the low power state,
> > > it is sending a memory ready message.
> > > The send is done via synchronous/blocking function to ensure
> > > that firmware is in ready state. However, in case of firmware
> > > undergoing reset send might be block forever.
> > > To address this issue a timeout is added to blocking
> > > write command on the internal bus.
> > >
> > > Introduce the __mei_cl_send_timeout function to use instead of
> > > __mei_cl_send in cases where timeout is required.
> > > The mei_cl_write has only two callers and there is no need to
> > > split
> > > it into two functions.
> > >
> > > V2: address review comments:
> > >  - split __mei_cl_send and __mei_cl_send_timeout
> > >  - add units to timeout KDoc
> > >  - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
> > >
> > > V3: - split the state fix into separate patch
> > >     - document define unit
> > >     - expand timeout KDoc
> >
> > These 2 patches looks good to me now.
> >
> > Greg, whenever you review it, please let me know if it is
> > okay to me to push these through the drm-fixes, or if you
> > prefer these to the mei branches.
>
> These have been in my tree for over a week now, sorry.  No one told
> me
> not to take them...
>
> {sigh}

no worries at all. The important thing is that the users will get the
fix.
We can keep them in our topic/core-for-CI while our branches are out-
of-sync.

Thanks a lot,
Rodrigo.

>
> greg k-h