Re: [PATCH] Revert "[media] au0828: use v4l2_mc_create_media_graph()"

[Shuah Khan]

On 03/10/2016 11:05 AM, Shuah Khan wrote:
> On 03/10/2016 10:53 AM, Mauro Carvalho Chehab wrote:
>> Em Thu, 10 Mar 2016 09:16:30 -0700
>> Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> escreveu:
>>
>>> On 03/08/2016 08:26 PM, Shuah Khan wrote:
>>>> This reverts commit 9822f4173f84cb7c592edb5e1478b7903f69d018.
>>>> This commit breaks au0828_enable_handler() logic to find the tuner.
>>>> Audio, Video, and Digital applications are broken and fail to start
>>>> streaming with tuner busy error even when tuner is free.
>>>>
>>>> Signed-off-by: Shuah Khan <shuahkh@xxxxxxxxxxxxxxx>
>>>> ---
>>>>  drivers/media/usb/au0828/au0828-video.c | 103 ++++++++++++++++++++++++++++++--
>>>>  drivers/media/v4l2-core/v4l2-mc.c       |  21 +------
>>>>  2 files changed, 99 insertions(+), 25 deletions(-)
>>>>   
>>>
>>> Hi Mauro,
>>>
>>> Please pull this revert in as soon as possible. Without
>>> the revert, auido, video, and digital applications won't
>>> start even. There is a bug in the common routine introduced
>>> in the commit 9822f4173f84cb7c592edb5e1478b7903f69d018 which
>>> causes the link between source and sink to be not found.
>>> I am testing on WIn-TV HVR 950Q
>>
>> No, this patch didn't seem to have broken anything. The problems
>> you're reporting seem to be related, instead, to this patch:
>>
>> 	https://patchwork.linuxtv.org/patch/33422/
>>
>> I rebased it on the top of the master tree (without reverting this
>> patch).
> 
> I don't think so. I sent https://patchwork.linuxtv.org/patch/33422/
> after I did the revert. I tested with linux_media branch with this
> top commit:
> 
> commit de08b5a8be0df1eb7c796b0fe6b30cf1d03d14a6
> Author: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> Date:   Tue Mar 1 06:31:53 2016 -0300
> 
> when I found the problem and reverting the commit 
> 9822f4173f84cb7c592edb5e1478b7903f69d018 - solved the proble,
> 
> Could you please test with and without the revert.

ok now that we determined in our offline discussions that
the revert does solve the problem and that there is an issue
with the core routine, we can try to debug the core common
routine with the revert in place.

> 
>>
>> Please check if it solved for you.
>>
>> Yet, I'm seeing several troubles with au0828 after your patch series:
>>
>> 1) when both snd-usb-audio and au0828 are compiled as module and not
>> blacklisted, I'm getting some errors:
>> 	http://pastebin.com/nMzr3ueM

I am looking into this problem. I can't reproduce this
on my system. It would be good know the sequence that
led to this. 

I see in the above pastebin:

BUG: sleeping function called from invalid context at mm/slub.c:1289

Resulting from the kzalloc(sizeof(*link), GFP_KERNEL);

In this case media_add_link() is trying to allocate memory
within  spinlock context. media_device_register_entity()
holds mdev->lock.

I think we added this spinlock for a reason. How do we 
handle this case? Can we use GFP_ATOMIC here conditionally?

thanks,
-- Shuah


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