From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D707EC433F5 for ; Fri, 6 May 2022 15:48:15 +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 59E83182F; Fri, 6 May 2022 17:47:23 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 59E83182F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1651852093; bh=m6NchDWC26O6ucu/OhX53UgFgZQ5Nzjejk20v+5/WOo=; h=Date:Subject:To:References:From:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=SFbMFsLJn94hKVV4IskUxHyymeov1cB3QLtyiqVkpmITsf7mNfsH3YZTX+CYS6znb GQtRwezrIYoeayD3YLzdVRbyv/vPr0B565RSYoDNiuJsHDlkJfmUro83rwj4D4Wlwg E9nvR/1RJzM8x43njs0VquIl+Xemwof9HhzMG630= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id E63D5F800F0; Fri, 6 May 2022 17:47:22 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id B10DDF8014B; Fri, 6 May 2022 17:47:20 +0200 (CEST) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 6A108F800F0 for ; Fri, 6 May 2022 17:47:16 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 6A108F800F0 Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="E8lIWnzV" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1651852038; x=1683388038; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=m6NchDWC26O6ucu/OhX53UgFgZQ5Nzjejk20v+5/WOo=; b=E8lIWnzVIgu4U029pG+ZuSLaF2TIXJC6vh4jT8LjdwjsHOV4MJQ6Pnt3 tsqX5Jf9nAuMr4VIeDYQciJY+D8bIoOMRpilVn5hTUOBRRpHTa+epi6Yk azsd7rGU/IA9QI3JXFAxuTkdVWWSzo0Wk8kayx2sJIYiavCuCyHvrcbvQ vgG/nVqr6xpL+rAKYeDpSTOgM0UicfjfQeyd9lQlG6F0h4j9xC8OeCFz6 orcfDGNdTcdDA2FzAztGICViqSmYReKcoVezaPw70gOpNjNKNvIXQucV3 np1mw0zyCQZ7lEd5ofNB5Q2Pk0dfZjdkHbPQam+9ftIHTlFpanrt4kZSL Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10339"; a="329046590" X-IronPort-AV: E=Sophos;i="5.91,203,1647327600"; d="scan'208";a="329046590" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2022 08:47:08 -0700 X-IronPort-AV: E=Sophos;i="5.91,203,1647327600"; d="scan'208";a="654744694" Received: from ysomasun-mobl1.amr.corp.intel.com (HELO [10.209.0.67]) ([10.209.0.67]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2022 08:47:07 -0700 Message-ID: Date: Fri, 6 May 2022 10:47:06 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0 Thunderbird/91.5.0 Subject: Re: [PATCH 01/14] ASoC: Intel: avs: Account for libraries when booting basefw Content-Language: en-US To: Piotr Maziarz , Cezary Rojewski , alsa-devel@alsa-project.org, broonie@kernel.org References: <20220426172346.3508411-1-cezary.rojewski@intel.com> <20220426172346.3508411-2-cezary.rojewski@intel.com> <9854d2e1-63da-2377-3fd1-120adfb4d381@linux.intel.com> <4ba8b812-2b67-5dd4-2774-f7a94e2d3cc1@intel.com> From: Pierre-Louis Bossart In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: upstream@semihalf.com, harshapriya.n@intel.com, rad@semihalf.com, tiwai@suse.com, hdegoede@redhat.com, amadeuszx.slawinski@linux.intel.com, cujomalainey@chromium.org, lma@semihalf.com 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" >>>>   +int avs_dsp_load_libraries(struct avs_dev *adev, struct >>>> avs_tplg_library *libs, u32 num_libs) >>>> +{ >>>> +    int start, id, i = 0; >>>> +    int ret; >>>> + >>>> +    /* Calculate the id to assign for the next lib. */ >>>> +    for (id = 0; id < adev->fw_cfg.max_libs_count; id++) >>>> +        if (adev->lib_names[id][0] == '\0') >>>> +            break; >>>> +    if (id + num_libs >= adev->fw_cfg.max_libs_count) >>>> +        return -EINVAL; >>> >>> use ida_alloc_max() ? >> >> >> After reading this one couple of times I'm keen to agree that IDA >> should have been used for library ID allocation and a at the same >> time, surprised it has't done that already. Till now we used IDA >> 'only' when allocating pipeline IDs and module instance IDs. Pipeline >> allocation is good comparison here - makes use of ida_alloc_max() >> already - library one should follow. >> >> This finding is much appreciated, Pierre. > > I think that using ida here is a bit of an overkill. Ida works fine when > there can be both id allocation and freeing and that's how it work with > pipelines and modules IDs in avs. However there is no mechanism for > unloading libraries in cAVS firmware, therefore ida would be used here > only to increase the ID, so it needlessly complicates the code while not > giving much of a benefit. Also our approach to check if we can load all > libraries before the loop makes it problematic with ida because we would > need to allocate an id at start and calculate if all libs would fit and > then either free it instantly or complicate the loop to use id allocated > before. Therefore I suggest to leave this code unchanged. I've synced > with Cezary on this and provided explanation convinced him too. That's fine, you should however capture this design decision with a comment or a clarification in the commit message. "libraries" mean different things to different people, and it's hard to review without context.