Skip to content

Conversation

@tlauda
Copy link
Contributor

@tlauda tlauda commented Feb 26, 2020

Adds notification to the beginning of the list if there is some
other IPC notification being sent.

Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com

@tlauda
Copy link
Contributor Author

tlauda commented Feb 26, 2020

@lgirdwood This should fix internal CI, however some major refactor of IPC notifications is needed. I'll try to take care of it.

Copy link
Collaborator

@paulstelian97 paulstelian97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the other platform specific IPC drivers have to adjust? I see no adjustment in the i.MX specific IPC implementation and I think there are others as well.

Adds notification to the beginning of the list if there is some
other IPC notification being sent.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
@tlauda
Copy link
Contributor Author

tlauda commented Feb 26, 2020

Do the other platform specific IPC drivers have to adjust? I see no adjustment in the i.MX specific IPC implementation and I think there are others as well.

With this change no.

@paulstelian97
Copy link
Collaborator

Do the other platform specific IPC drivers have to adjust? I see no adjustment in the i.MX specific IPC implementation and I think there are others as well.

With this change no.

I suspected it because you have modified that platform specific function related to IPC, both change of signature and usage.

Build failures can be because of conflicting prototype and definition of this function on the non-CAVS platforms?

Copy link
Collaborator

@paulstelian97 paulstelian97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking until the implementation is adjusted on all platforms. As of this review, non-CAVS platforms fail the build.

@tlauda tlauda force-pushed the topic/detect-notify-improvements branch from 9811df6 to be28b88 Compare February 26, 2020 13:06
@tlauda tlauda requested a review from RanderWang as a code owner February 26, 2020 13:06
@paulstelian97 paulstelian97 dismissed their stale review February 26, 2020 13:07

Dismiss as issue is resolved, I will re-review now.

Copy link
Collaborator

@paulstelian97 paulstelian97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now.

ipc_platform_send_msg(&cd->msg);
if (ipc_platform_send_msg(&cd->msg) < 0)
/* Other notification in progress, so just add to the list */
list_item_prepend(&cd->msg.list, &ipc_get()->msg_list);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prepending should be in the IPC core. i.e. we should have a new IPC API

int ipc_platform_send_msg_queue(msg, priority, remove_duplicates);

Where priority HIGH would mean we insert new message at list head, otherwise it's list tail. The remove duplicates bool means to remove any older messages from this sender and use this message. Handy if we are sending any high frequency updates and want to remove stale or older updates.

This should be called directly instead of existing method (and will acquire any locks etc)..

@tlauda tlauda closed this Feb 27, 2020
@lgirdwood
Copy link
Member

@tlauda any reason to close ? Is there another fix ?

@tlauda
Copy link
Contributor Author

tlauda commented Feb 27, 2020

@tlauda any reason to close ? Is there another fix ?

#2454

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants