Media Controller - graph_mutex

Media Controller - graph_mutex

[Shuah Khan]

All,

ALSA has to make media_entity_pipeline_start() call in irq
path. I am seeing warnings that the graph_mutex is unsafe irq
lock as expected. We have to update MC start/stop pipeline
to be irq safe for ALSA. Maybe there are other MC interfaces
that need to be irq safe, but I haven't seen any problems with
my limited testing.

So as per options, graph_mutex could be changed to a spinlock.
It looks like drivers hold this lock and it isn't abstracted to
MC API. Unfortunate, this would require changes to drivers that
directly hold the lock for graph walks if this mutex is changed
to spinlock.

e.g: drivers/media/platform/exynos4-is/fimc-isp-video.c

Changes aren't complex, just that the scope isn't limited
to MC API.

Other ideas??

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: Media Controller - graph_mutex

[Sakari Ailus]

Hi Shuah,

Shuah Khan wrote:
> All,
> 
> ALSA has to make media_entity_pipeline_start() call in irq
> path. I am seeing warnings that the graph_mutex is unsafe irq
> lock as expected. We have to update MC start/stop pipeline
> to be irq safe for ALSA. Maybe there are other MC interfaces
> that need to be irq safe, but I haven't seen any problems with
> my limited testing.
> 
> So as per options, graph_mutex could be changed to a spinlock.
> It looks like drivers hold this lock and it isn't abstracted to
> MC API. Unfortunate, this would require changes to drivers that
> directly hold the lock for graph walks if this mutex is changed
> to spinlock.
> 
> e.g: drivers/media/platform/exynos4-is/fimc-isp-video.c
> 
> Changes aren't complex, just that the scope isn't limited
> to MC API.
> 
> Other ideas??

Do you have (preliminary?) patches and / or more information? I'd like
to understand why would ALSA need this.

The graph_mutex is currently also taken when the link state is modified,
and the callbacks to the drivers may involve power state changes and
device initialisation. On some busses such as I2C these are blocking
operations.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: Media Controller - graph_mutex

[Shuah Khan]

On 07/14/2015 08:06 AM, Sakari Ailus wrote:
> Hi Shuah,
> 
> Shuah Khan wrote:
>> All,
>>
>> ALSA has to make media_entity_pipeline_start() call in irq
>> path. I am seeing warnings that the graph_mutex is unsafe irq
>> lock as expected. We have to update MC start/stop pipeline
>> to be irq safe for ALSA. Maybe there are other MC interfaces
>> that need to be irq safe, but I haven't seen any problems with
>> my limited testing.
>>
>> So as per options, graph_mutex could be changed to a spinlock.
>> It looks like drivers hold this lock and it isn't abstracted to
>> MC API. Unfortunate, this would require changes to drivers that
>> directly hold the lock for graph walks if this mutex is changed
>> to spinlock.
>>
>> e.g: drivers/media/platform/exynos4-is/fimc-isp-video.c
>>
>> Changes aren't complex, just that the scope isn't limited
>> to MC API.
>>
>> Other ideas??
> 
> Do you have (preliminary?) patches and / or more information? I'd like
> to understand why would ALSA need this.
> 
> The graph_mutex is currently also taken when the link state is modified,
> and the callbacks to the drivers may involve power state changes and
> device initialisation. On some busses such as I2C these are blocking
> operations.
> 

I am getting close to sending the RFC patches for review. I am looking
at one last issue in addition to this mutex problem. I have dmesg logs
to send and I will send them when I send the patches, so you have code
to look at as well as the dmesg.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978