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 X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3250FC43463 for ; Fri, 18 Sep 2020 13:59:18 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 4DC7120878 for ; Fri, 18 Sep 2020 13:59:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="q2aV0baQ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4DC7120878 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org 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 7628F16B9; Fri, 18 Sep 2020 15:58:25 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 7628F16B9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1600437555; bh=LDHaEVsyhIG/cMAiqgbkNxEoHFVOytcW31eA1s45lbY=; h=Date:From:To:Subject:References:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=q2aV0baQVnWh6richc82gDoe5TWbsZNSHTkIeuazRa/rqPOamnXTLg2CsPjklseKG yZsX38Jq2BqFTXBnA1rm2mqRyemaMNQvvzQ1qd3r5+oc+aQxNkIUBWby4ROM3UzLax RjlFEfs3+qU+o4eaDDaaN/pQc9iOmF/MqvZQcvkc= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 055E2F80150; Fri, 18 Sep 2020 15:58:25 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 1C8CDF800E8; Fri, 18 Sep 2020 15:58:23 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (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 79EBDF800E8 for ; Fri, 18 Sep 2020 15:58:15 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 79EBDF800E8 IronPort-SDR: 6CYrldjbaYPvF385G1Pz5EqK9JL+RLqRWVqkOHY3HmqgEG7Ins9UIaL+vyf72rXDx69bu8H9lj MadEmnA+8Pwg== X-IronPort-AV: E=McAfee;i="6000,8403,9747"; a="147678957" X-IronPort-AV: E=Sophos;i="5.77,274,1596524400"; d="scan'208";a="147678957" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Sep 2020 06:58:14 -0700 IronPort-SDR: fk9+0WrxQep8vMFPHxyIeXp3i4hvNc1YCr3WHVfIr53GEKU7mR88AkxO6MLQH7fO/MbjVIlJBa zIWrAFB2YPOA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,274,1596524400"; d="scan'208";a="336812153" Received: from smile.fi.intel.com (HELO smile) ([10.237.68.40]) by orsmga008.jf.intel.com with ESMTP; 18 Sep 2020 06:58:00 -0700 Received: from andy by smile with local (Exim 4.94) (envelope-from ) id 1kJGra-0000F2-Bm; Fri, 18 Sep 2020 16:55:46 +0300 Date: Fri, 18 Sep 2020 16:55:46 +0300 From: Andy Shevchenko To: Cezary Rojewski Subject: Re: [PATCH v6 00/14] ASoC: Intel: Catpt - Lynx and Wildcat point Message-ID: <20200918135546.GE3956970@smile.fi.intel.com> References: <20200917141242.9081-1-cezary.rojewski@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200917141242.9081-1-cezary.rojewski@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Cc: pierre-louis.bossart@linux.intel.com, alsa-devel@alsa-project.org, filip.kaczmarski@intel.com, harshapriya.n@intel.com, marcin.barlik@intel.com, zwisler@google.com, lgirdwood@gmail.com, tiwai@suse.com, filip.proborszcz@intel.com, broonie@kernel.org, amadeuszx.slawinski@linux.intel.com, michal.wasko@intel.com, cujomalainey@chromium.org, krzysztof.hejmowski@intel.com, ppapierkowski@habana.ai, vamshi.krishna.gopal@intel.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" On Thu, Sep 17, 2020 at 04:12:28PM +0200, Cezary Rojewski wrote: > Implement support for Lynxpoint and Wildcat Point AudioDSP. Catpt > solution deprecates existing sound/soc/intel/haswell which is removed in > the following series. This cover-letter is followed by 'Developer's deep > dive' message schedding light on catpt's key concepts and areas > addressed. > > Due to high range of errors and desynchronization from recommendations > set by Windows solution, re-write came as a lower-cost solution compared > to refactoring /haswell/ with several series of patches. > > Series is dependent on linux-spi change: > spi: pxa2xx: Add SSC2 and SSPSP2 SSP registers > https://www.spinics.net/lists/linux-spi/msg23885.html > which has been already merged and is now part of linux-spi tree. > > Bulk of series content is device driver core code - everything up to > patch 7/14 - with fs entries and trace macros introduced right after. > While each core patch is shaped in such a way that no unavailable > members are ever called, until patch 10/14 is applied, no code > compilation can occur as no Makefile is present. Once said patch is > added, Makefile and Kconfig are implemented and driver module compiles > as expected. > > > Special thanks go to Marcin Barlik and Piotr Papierkowski for sharing > their LPT/WPT AudioDSP architecture expertise as well as helping > backtrack its historical background. > My thanks go to Amadeusz Slawinski for reviews and improvements proposed > on and off the internal list. Most of internal diff below is his > contribution. > Krzysztof Hejmowski helped me setup my own Xtensa environment and > recompile LPT/WPT FW binary sources what sped up the development greatly. > > This would not have been possible without help from these champions, > especially considering how quickly the catpt was written: 2 weeks > features, 3 weeks optimizations. Thank you. > > Userspace-exposed members are compatible with what is exposed by > deprecated solution as well as FW binary being re-used thus no harm is > done. The only visible differences are: the newly added 'Loopback Mute' > kcontrol and volume support extending to quad from stereo. > > On top of fixing erros and design flows, catpt also adds module reload, > dynamic SRAM memory allocation during PCM runtime and exposes missing > userspace API: 'Loopback Mute' kcontrol, quad volume controls and sysfs > fw-version entries. Event tracing is provided to easy solution > debugging. > > Following are not included in this update and are scheduled as later > addition: > - fw logging > - module (library) support > > Note: LPT power up/down sequences might get aligned with WPT once enough > testing is done as capabilities are shared for both DSPs. > Note #2: Both LPT and WPT power up/down sequences may get optimized in > future updates as thanks to help from the Windows team, most of nuances > behind why/what/when in regard to hw registers have been backtracked and > reviewed again. > > Link to developer's deep dive message: > https://www.spinics.net/lists/alsa-devel/msg113563.html Okay, I don't see any reason to resend a series right now and all my comments that left are rather nit-picks, so Reviewed-by: Andy Shevchenko > Changes in v6: > - reordered and reorganized code for patches 1/13 - 8/13 of v5, so each > patches makes use of no member or function which is unavailable to it. > Series size increased from 13 to 14 patches: addition of base members > e.g.: registers has been split from addition of device.c file which > describes acpi device behavior > > > Changes in v5: > https://www.spinics.net/lists/alsa-devel/msg115621.html > Basically everything below is result of Andy's review. Thank you Andy > for taking time into this detailed review > > - catpt now makes use of common linux/pxa2xx_ssp.h header file, removing > redundant SSP register declarations in the process. As stated in the > opening, this is dependent upon linux-spi change: > spi: pxa2xx: Add SSC2 and SSPSP2 SSP registers > > - updated Kconfig by removing DMADEVICES and adding COMPILE_TEST > as optional depends-on > - updated all register macros definitions to be more safe against common > arithmetics when specifying macro's parameters > - removed CONFIG_PM and CONFIG_PM_SLEEP usage in favor of __maybe_unused > - all 'if (ret < 0)' converted to simple 'if (ret)' whenever possible > - fixed erroneous check for platform_device_register_data within > catpt_register_board() > - _SLAVE/_MASTER replaced with more inclusive _CONSUMER/_PROVIDER for > enum catpt_ssp_mode > - catpt_acpi_probe() is now making use of high-level wrappers for > ioremapping and resource assignment, reducing function's code size > - due to improved catpt_acpi_probe() behavior, catpt_acpi_remove() needs > not to cast dma_free_coherent() any longer > - DMA source and destrination maxburst now of value 16, see: > https://www.spinics.net/lists/alsa-devel/msg114394.html > > - simplified catpt_dsp_update_lpclock() as list_for_each_entry() is > empty-safe by default > - dropped '_SSP_' from all names of all CATPT_SSP_SSXXX_DEFAULT macros > - catpt_updatel_pci now makes use of linux/pci.h and uapi/linux/pci.h > constants such as: PCI_PM_CTRL_STATE_MASK and PCI_D3hot > > > Changes in v4: > https://www.spinics.net/lists/alsa-devel/msg113762.html > - fixed compilation with i386 kconfig (conflicting names) > - streamlined naming for SHIM and PCI registers to match SSP ones > (SHIM_REG -> SHIM) > - catpt_component_probe removed and kcontrols again initializzed > statically via snd_kcontrol_new array: this is to remove > kctl->id.device shenanigans > - renamed catpt_set_ctlvol to catpt_set_dspvol - function name wasn't > matching its purpose > > > Changes in v3: > - fixed IRAM mask usage in lpt_dsp_power_up (dsp.c) > - updated dbg message formatting in catpt_restore_fwimage as suggested > by Andy > - fixed alignment for struct catpt_ssp_device_format > - catpt_set_ctlvol now verifies all-equal scenario based on all > channels rather than just first two as requested by Amadeo > - fixed SPDX for registers.h > > > Changes in v2: > https://www.spinics.net/lists/alsa-devel/msg113660.html > - fixed SPDX formatting for all header files as well as pcm.c > - fixed size provided to memcpy() in fw_build_read() as reported by Mark > - renamed struct catpt_pdata to struct catpt_spec (cosmetic) > - fixed erroneous path in catpt_load_block: region is properly released > - trace.h events for updating registers have been removed and usages > replaced by dev_dbg (SRAMPGE/ LPCS) > > - as requested by Andy, struct resource has replaced struct catpt_mbank > and struct catpt_mregion. This change cascaded into: > > - catpt_mbank_size and catpt_mregion_size replaced by resource_size > - catpt_mregion_intersects replaced by resource_overlaps > - all catpt_mbank_ and catpt_mregion_ handlers found in loader.c > (_request, _reserve, _release, _extract, _split, _join) have been > removed > - __request_region and __release_region have been enlisted in their > place > - catpt_mregion_intersecting renamed to catpt_resource_overlapping > - catpt_request_region helper has been provided to deal with -size > based requests > o haven't found direct replacements in resource.c/ ioport.h for > both functions > > - catpt_mbank_create and catpt_mbank_remove renamed to catpt_sram_init > and catpt_sram_free respectively > - catpt_sram_init now returns void instead of int and has been > converted to simple initialized. This change ultimately cascaded > into: > o both SRAM banks initialization being moved to catpt_dev_init > from catpt_acpi_probe (device.c) > o catpt_dev::spec is now initialized first, with catpt_dev_init > following it soon after > o catpt_acpi_probe erroneous path has been simplified as SRAM > banks no longer need to be freed > > - catpt_sram_free now frees all resources via child -> sibling > enumeration rather than region_list iteration > - catpt_dsp_update_srampge and catpt_dsp_set_srampge now accept new > argument: unsigned long mask. Caused by removal of catpt_mbank - > mask is taken directly from catpt_dev::spec::d/iram_mask > - trace.h events for catpt_mbank and catpt_mregion have been removed > > > Diff against last drop on internal list: > https://www.spinics.net/lists/alsa-devel/msg113549.html > - replaced spinlock with mutex for mregion allocation and release to > address sleeping in atomic context warnings > - fixed coredump fw_hash dumping > - kcontrol values are now always stored regardless of stream of interest > is running or not > - kcontrol values are now applied after stream is prepared instead of > ignoring what has been set by user initially > - catpt_pdata instances have been renamed from hsw_ and bdw_ to lpt_ and > wpt_ respectively > - reordered Makefile .o(s) (cosmetic) > > Cezary Rojewski (14): > ASoC: Intel: Add catpt base members > ASoC: Intel: catpt: Implement IPC protocol > ASoC: Intel: catpt: Add IPC message handlers > ASoC: Intel: catpt: Define DSP operations > ASoC: Intel: catpt: Firmware loading and context restore > ASoC: Intel: catpt: PCM operations > ASoC: Intel: catpt: Device driver lifecycle > ASoC: Intel: catpt: Event tracing > ASoC: Intel: catpt: Simple sysfs attributes > ASoC: Intel: Select catpt and deprecate haswell > ASoC: Intel: haswell: Remove haswell-solution specific code > ASoC: Intel: broadwell: Remove haswell-solution specific code > ASoC: Intel: bdw-5650: Remove haswell-solution specific code > ASoC: Intel: bdw-5677: Remove haswell-solution specific code > > sound/soc/intel/Kconfig | 24 +- > sound/soc/intel/Makefile | 2 +- > sound/soc/intel/boards/Kconfig | 8 +- > sound/soc/intel/boards/bdw-rt5650.c | 36 - > sound/soc/intel/boards/bdw-rt5677.c | 33 - > sound/soc/intel/boards/broadwell.c | 33 - > sound/soc/intel/boards/haswell.c | 28 +- > sound/soc/intel/catpt/Makefile | 6 + > sound/soc/intel/catpt/core.h | 189 +++++ > sound/soc/intel/catpt/device.c | 354 ++++++++ > sound/soc/intel/catpt/dsp.c | 572 +++++++++++++ > sound/soc/intel/catpt/fs.c | 79 ++ > sound/soc/intel/catpt/ipc.c | 298 +++++++ > sound/soc/intel/catpt/loader.c | 671 +++++++++++++++ > sound/soc/intel/catpt/messages.c | 313 +++++++ > sound/soc/intel/catpt/messages.h | 401 +++++++++ > sound/soc/intel/catpt/pcm.c | 1212 +++++++++++++++++++++++++++ > sound/soc/intel/catpt/registers.h | 178 ++++ > sound/soc/intel/catpt/trace.h | 83 ++ > 19 files changed, 4377 insertions(+), 143 deletions(-) > create mode 100644 sound/soc/intel/catpt/Makefile > create mode 100644 sound/soc/intel/catpt/core.h > create mode 100644 sound/soc/intel/catpt/device.c > create mode 100644 sound/soc/intel/catpt/dsp.c > create mode 100644 sound/soc/intel/catpt/fs.c > create mode 100644 sound/soc/intel/catpt/ipc.c > create mode 100644 sound/soc/intel/catpt/loader.c > create mode 100644 sound/soc/intel/catpt/messages.c > create mode 100644 sound/soc/intel/catpt/messages.h > create mode 100644 sound/soc/intel/catpt/pcm.c > create mode 100644 sound/soc/intel/catpt/registers.h > create mode 100644 sound/soc/intel/catpt/trace.h > > -- > 2.17.1 > -- With Best Regards, Andy Shevchenko