From patchwork Fri Mar 29 00:05:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shuah X-Patchwork-Id: 10876335 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 088C915AC for ; Fri, 29 Mar 2019 03:46:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D813B28F5F for ; Fri, 29 Mar 2019 03:46:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C842B290A9; Fri, 29 Mar 2019 03:46:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 986CC28F5F for ; Fri, 29 Mar 2019 03:46:40 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 7973F887; Fri, 29 Mar 2019 01:09:13 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 7973F887 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1553818203; bh=HNYXkSZBjNbsdP4ZSh4KdIXOQENxigtOJIl3FgIrZLI=; h=From:To:Date:In-Reply-To:References:Cc:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=EnES7CvPtQ8QJZlfc4aIKPcq9N5HKBD6YOVAY/SN0acC6upeh1/2fhm/AlDxK7Ti8 UwaO7xT1TLMkqKtMmlVFuNn/4TytMyK0Zpsye1ZSG+ahPg0Pj4rOqtZEN4ZdU9Gw1m LzevVMfla8NLftaWqwe31uRWKE8adekODwOse8rY= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 6A836F89731; Fri, 29 Mar 2019 01:05:24 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa1.perex.cz (Postfix, from userid 50401) id 222B0F8971E; Fri, 29 Mar 2019 01:05:19 +0100 (CET) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id A3A04F89708 for ; Fri, 29 Mar 2019 01:05:11 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz A3A04F89708 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="xAWacscw" Received: from shuah-t480s.internal (c-24-9-64-241.hsd1.co.comcast.net [24.9.64.241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 963C7218FD; Fri, 29 Mar 2019 00:05:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1553817910; bh=mC9ZkBI1j4g2UcrzpSbUy4xPO3jLn4BSD+4RRtirvjY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=xAWacscwvOeFsgOU4yEM6RG/KH458SibI9WAI6t+0K4xM9XIsQqjxVSncdDopXbHg s3XLTkMKYjGnw+QxIrJfFlqJnM6WhzcdeA7t8dx/Mlg5nsFttHJ53n+l/I8H8ZN7zM OZ+soKlT3RU7W1G28BWrB5kxD1czIgJ/CC6hUcck= From: Shuah Khan To: mchehab@kernel.org, perex@perex.cz, tiwai@suse.com, hverkuil@xs4all.nl Date: Thu, 28 Mar 2019 18:05:00 -0600 Message-Id: <9135b2b177bad233e2676641e26003cddf667c67.1553814518.git.shuah@kernel.org> X-Mailer: git-send-email 2.19.1 In-Reply-To: References: MIME-Version: 1.0 Cc: alsa-devel@alsa-project.org, Shuah Khan , linux-kernel@vger.kernel.org, linux-media@vger.kernel.org Subject: [alsa-devel] [PATCH v13 5/6] au0828: fix enable and disable source audio and video inconsistencies X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" X-Virus-Scanned: ClamAV using ClamSMTP Enable and disable source interfaces aren't consistent in enforcing how video and audio share the tuner resource. Fix these issues to enforce the following rules and allow sharing between audio and video applications. - When DVB is streaming, audio/video/vbi/s-video/composite should find the resource busy. DVB holds the tuner in exclusive mode. - When video is streaming, audio can share the tuner and vice versa. - v4l2 allows multiple applications to open video device. - Video applications call enable source multiple times during their run-time. Resource should stay locked until the last application releases it. - A shared resource should stay in shared state and locked when it is in use by audio and video. More than one video application is allowed to use the tuner as long as video streaming protocol allows such usage. Resource is released when the last video/audio application releases it. - S-Video and Composite hold the resource in exclusive mode. - VBI allows more than vbi applications sharing and will not share with another type. When resource is locked by VBI and in use by multiple VBI applications, it should stay locked until the last application disables it. Signed-off-by: Shuah Khan --- drivers/media/usb/au0828/au0828-core.c | 182 +++++++++++++++++++------ drivers/media/usb/au0828/au0828.h | 5 +- 2 files changed, 148 insertions(+), 39 deletions(-) diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index 4f8ba6f64d3c..b8bcab2fa1dc 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -272,6 +272,27 @@ static void au0828_media_graph_notify(struct media_entity *new, } } +static bool au0828_is_link_sharable(struct media_entity *owner, + struct media_entity *entity) +{ + bool sharable = false; + + /* Tuner link can be shared by audio and video */ + switch (owner->function) { + case MEDIA_ENT_F_IO_V4L: + case MEDIA_ENT_F_AUDIO_CAPTURE: + if (entity->function == MEDIA_ENT_F_IO_V4L || + entity->function == MEDIA_ENT_F_AUDIO_CAPTURE) + sharable = true; + break; + case MEDIA_ENT_F_DTV_DEMOD: + case MEDIA_ENT_F_IO_VBI: + default: + break; + } + return sharable; +} + /* Callers should hold graph_mutex */ static int au0828_enable_source(struct media_entity *entity, struct media_pipeline *pipe) @@ -314,18 +335,20 @@ static int au0828_enable_source(struct media_entity *entity, /* * Default input is tuner and default input_type * is AU0828_VMUX_TELEVISION. - * FIXME: + * * There is a problem when s_input is called to * change the default input. s_input will try to * enable_source before attempting to change the * input on the device, and will end up enabling * default source which is tuner. * - * Additional logic is necessary in au0828 - * to detect that the input has changed and - * enable the right source. + * Additional logic is necessary in au0828 to detect + * that the input has changed and enable the right + * source. au0828 handles this case in its s_input. + * It will disable the old source and enable the new + * source. + * */ - if (dev->input_type == AU0828_VMUX_TELEVISION) find_source = dev->tuner; else if (dev->input_type == AU0828_VMUX_SVIDEO || @@ -338,27 +361,33 @@ static int au0828_enable_source(struct media_entity *entity, } } - /* Is an active link between sink and source */ + /* Is there an active link between sink and source */ if (dev->active_link) { - /* - * If DVB is using the tuner and calling entity is - * audio/video, the following check will be false, - * since sink is different. Result is Busy. - */ - if (dev->active_link->sink->entity == sink && - dev->active_link->source->entity == find_source) { - /* - * Either ALSA or Video own tuner. sink is - * the same for both. Prevent Video stepping - * on ALSA when ALSA owns the source. + if (dev->active_link_owner == entity) { + /* This check is necessary to handle multiple + * enable_source calls from v4l_ioctls during + * the course of video/vbi application run-time. */ - if (dev->active_link_owner != entity && - dev->active_link_owner->function == - MEDIA_ENT_F_AUDIO_CAPTURE) { - pr_debug("ALSA has the tuner\n"); - ret = -EBUSY; - goto end; - } + pr_debug("%s already owns the tuner\n", entity->name); + ret = 0; + goto end; + } else if (au0828_is_link_sharable(dev->active_link_owner, + entity)) { + /* Either ALSA or Video own tuner. Sink is the same + * for both. Allow sharing the active link between + * their common source (tuner) and sink (decoder). + * Starting pipeline between sharing entity and sink + * will fail with pipe mismatch, while owner has an + * active pipeline. Switch pipeline ownership from + * user to owner when owner disables the source. + */ + dev->active_link_shared = true; + /* save the user info to use from disable */ + dev->active_link_user = entity; + dev->active_link_user_pipe = pipe; + pr_debug("%s owns the tuner %s can share!\n", + dev->active_link_owner->name, + entity->name); ret = 0; goto end; } else { @@ -385,7 +414,7 @@ static int au0828_enable_source(struct media_entity *entity, source = found_link->source->entity; ret = __media_entity_setup_link(found_link, MEDIA_LNK_FL_ENABLED); if (ret) { - pr_err("Activate tuner link %s->%s. Error %d\n", + pr_err("Activate link from %s->%s. Error %d\n", source->name, sink->name, ret); goto end; } @@ -395,25 +424,26 @@ static int au0828_enable_source(struct media_entity *entity, pr_err("Start Pipeline: %s->%s Error %d\n", source->name, entity->name, ret); ret = __media_entity_setup_link(found_link, 0); - pr_err("Deactivate link Error %d\n", ret); + if (ret) + pr_err("Deactivate link Error %d\n", ret); goto end; } - /* - * save active link and active link owner to avoid audio - * deactivating video owned link from disable_source and - * vice versa + + /* save link state to allow audio and video share the link + * and not disable the link while the other is using it. + * active_link_owner is used to deactivate the link. */ dev->active_link = found_link; dev->active_link_owner = entity; dev->active_source = source; dev->active_sink = sink; - pr_debug("Enabled Source: %s->%s->%s Ret %d\n", + pr_info("Enabled Source: %s->%s->%s Ret %d\n", dev->active_source->name, dev->active_sink->name, dev->active_link_owner->name, ret); end: - pr_debug("au0828_enable_source() end %s %d %d\n", - entity->name, entity->function, ret); + pr_debug("%s end: ent:%s fnc:%d ret %d\n", + __func__, entity->name, entity->function, ret); return ret; } @@ -432,21 +462,95 @@ static void au0828_disable_source(struct media_entity *entity) if (!dev->active_link) return; - /* link is active - stop pipeline from source (tuner) */ + /* link is active - stop pipeline from source + * (tuner/s-video/Composite) to the entity + * When DVB/s-video/Composite owns tuner, it won't be in + * shared state. + */ if (dev->active_link->sink->entity == dev->active_sink && dev->active_link->source->entity == dev->active_source) { /* - * prevent video from deactivating link when audio - * has active pipeline + * Prevent video from deactivating link when audio + * has active pipeline and vice versa. In addition + * handle the case when more than one video/vbi + * application is sharing the link. */ + bool owner_is_audio = false; + + if (dev->active_link_owner->function == + MEDIA_ENT_F_AUDIO_CAPTURE) + owner_is_audio = true; + + if (dev->active_link_shared) { + pr_debug("Shared link owner %s user %s %d\n", + dev->active_link_owner->name, + entity->name, dev->users); + + /* Handle video device users > 1 + * When audio owns the shared link with + * more than one video users, avoid + * disabling the source and/or switching + * the owner until the last disable_source + * call from video _close(). Use dev->users to + * determine when to switch/disable. + */ + if (dev->active_link_owner != entity) { + /* video device has users > 1 */ + if (owner_is_audio && dev->users > 1) + return; + + dev->active_link_user = NULL; + dev->active_link_user_pipe = NULL; + dev->active_link_shared = false; + return; + } + + /* video owns the link and has users > 1 */ + if (!owner_is_audio && dev->users > 1) + return; + + /* stop pipeline */ + __media_pipeline_stop(dev->active_link_owner); + pr_debug("Pipeline stop for %s\n", + dev->active_link_owner->name); + + ret = __media_pipeline_start( + dev->active_link_user, + dev->active_link_user_pipe); + if (ret) { + pr_err("Start Pipeline: %s->%s %d\n", + dev->active_source->name, + dev->active_link_user->name, + ret); + goto deactivate_link; + } + /* link user is now the owner */ + dev->active_link_owner = dev->active_link_user; + dev->active_link_user = NULL; + dev->active_link_user_pipe = NULL; + dev->active_link_shared = false; + + pr_debug("Pipeline started for %s\n", + dev->active_link_owner->name); + return; + } else if (!owner_is_audio && dev->users > 1) + /* video/vbi owns the link and has users > 1 */ + return; + if (dev->active_link_owner != entity) return; - __media_pipeline_stop(entity); + + /* stop pipeline */ + __media_pipeline_stop(dev->active_link_owner); + pr_debug("Pipeline stop for %s\n", + dev->active_link_owner->name); + +deactivate_link: ret = __media_entity_setup_link(dev->active_link, 0); if (ret) pr_err("Deactivate link Error %d\n", ret); - pr_debug("Disabled Source: %s->%s->%s Ret %d\n", + pr_info("Disabled Source: %s->%s->%s Ret %d\n", dev->active_source->name, dev->active_sink->name, dev->active_link_owner->name, ret); @@ -454,6 +558,8 @@ static void au0828_disable_source(struct media_entity *entity) dev->active_link_owner = NULL; dev->active_source = NULL; dev->active_sink = NULL; + dev->active_link_shared = false; + dev->active_link_user = NULL; } } #endif diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h index 57b00de8d3f2..b47ecc9affd8 100644 --- a/drivers/media/usb/au0828/au0828.h +++ b/drivers/media/usb/au0828/au0828.h @@ -284,9 +284,12 @@ struct au0828_dev { struct media_entity_notify entity_notify; struct media_entity *tuner; struct media_link *active_link; - struct media_entity *active_link_owner; struct media_entity *active_source; struct media_entity *active_sink; + struct media_entity *active_link_owner; + struct media_entity *active_link_user; + struct media_pipeline *active_link_user_pipe; + bool active_link_shared; #endif };