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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 2E36FC433EF for ; Mon, 6 Dec 2021 19:43:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:In-reply-to: Date:Subject:Cc:To:From:References:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=E6TMzzFIQ5tEjo4g8+IoaG7OT0ykVlKQbJInPa400Xc=; b=qP9yvvSkb0jO+a ZTzPKuhLo24tuDeMtMEpW3OvJB7e8mqeNkGisEI9xVi3AHdqg5B6+KJ/dhjJ2pCAuXhIcFdM53LLc BXKew6cC3q8y1KE8sWZkQx6wwa8GP2s6xBce/KROac7v58F47CSVqlmhxorJ0d0SDvdJGu3+YzG/K vRX/leEaF8VLbGUpAlVKIo4+W9UsVouXLyL/VorO02FXJOK3q2iqnxH41d+D7TTwjK36XTPjlJmUc 1HUnt+rqATkSSPSylcv7s6o6p7QCtCupe6JYUK/ot2BSbuYTMGZMzkZdsaTCszBxri551fv0LP91d RqxI0aUP3VLv0/eWLIfA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1muJsM-005Wel-S7; Mon, 06 Dec 2021 19:42:15 +0000 Received: from mail-wr1-x429.google.com ([2a00:1450:4864:20::429]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1muJsF-005WdR-Nl for linux-arm-kernel@lists.infradead.org; Mon, 06 Dec 2021 19:42:11 +0000 Received: by mail-wr1-x429.google.com with SMTP id a18so24723760wrn.6 for ; Mon, 06 Dec 2021 11:42:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=references:user-agent:from:to:cc:subject:date:in-reply-to :message-id:mime-version; bh=XoyDxWHFfkkShprB5fi0ZtCpkIroCq2M/CcUOnfbOhA=; b=w8sZ41kEdX4SRoPkzKCBKIOpzSsDrjyDTmuhgrbbcp5zpmFSvfGY7gNpZK0p0dMxAa uWrDVGdLjErc8kI0vHKRLdvR+xas3quAw8WLQmicLbCVcrujBGlSxKW5v4tNRzh5Rsld PbPH7ba+Ou0DcR+MLaSif954Gu9KWrAzQrbAJ9dnM20t0XW2ozYroSORAQOs3yEGfsWc 0b8NifhWMgxwmzcAJ8vTR/83pq61hWtl1y1Me57+Y0zioTVaZzNksWyql3KdwajCCzjI pEOgp1cAfmONXrGWtiC6RzGuyMrHDMhZRencZZIde+pIeMZPjzNdS45FL5neth6TXFke HUeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:references:user-agent:from:to:cc:subject:date :in-reply-to:message-id:mime-version; bh=XoyDxWHFfkkShprB5fi0ZtCpkIroCq2M/CcUOnfbOhA=; b=mhdfVYP5T1jEo2W+sHzRZEGG3RiQHL4W8UTEyG0v97aQt/9AsrrwhsC+vOL4uv9gLs dZZg297xxGLru7nDaBBBCtjePhLkI6UN9ZbhzyYXoskUezD/fvfWVJMIGG/JOUy6DXyH EsWzbrx4b4/F6M9LO4Hx2OyI2DWksNAFnTYBVEeyqN3nZQ9X5EC8H0Q0zyigZsqd8d4y Bi+SKx2w6F6IgFaqA90gfarG+SHW+xrEhwylq4aqWOPQiO+c1dRlWdUzp6G1UdXC2Yti zaAiGGDrpCX1FRJU5FX8kZ3Qie4XSEo+p6CvgO5ju0qHmynqrIqNsHG2s0qDBurGozQ3 pVQw== X-Gm-Message-State: AOAM530uw2VvLL0dyZxrvurTpptY0BWw6G/fh7gzS/U3hZLbc5FIh+xO bJZ1e+fKCxFZIyRcoCseBrh5zg== X-Google-Smtp-Source: ABdhPJx8pLA553HMWI1HuMsyxL4boJMISHN0MO4UL5wXFAniqnUsOICkxQ/hPb/2AWWD4Mll3NrfjA== X-Received: by 2002:a5d:460d:: with SMTP id t13mr44894938wrq.44.1638819726062; Mon, 06 Dec 2021 11:42:06 -0800 (PST) Received: from localhost (82-65-169-74.subs.proxad.net. [82.65.169.74]) by smtp.gmail.com with ESMTPSA id o9sm12332737wrs.4.2021.12.06.11.42.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Dec 2021 11:42:05 -0800 (PST) References: <20211205180816.2083864-1-martin.blumenstingl@googlemail.com> <20211205180816.2083864-3-martin.blumenstingl@googlemail.com> <1jfsr659v1.fsf@starbuckisacylon.baylibre.com> User-agent: mu4e 1.6.10; emacs 27.1 From: Jerome Brunet To: Martin Blumenstingl Cc: linux-amlogic@lists.infradead.org, alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Christian Hewitt , Geraldo Nascimento Subject: Re: [PATCH RFC v1 2/2] ASoC: meson: aiu: Move AIU_I2S_MISC hold setting to aiu-fifo-i2s Date: Mon, 06 Dec 2021 20:36:45 +0100 In-reply-to: Message-ID: <1jpmq9ed0z.fsf@starbuckisacylon.baylibre.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211206_114207_869797_0A15A1BC X-CRM114-Status: GOOD ( 34.45 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon 06 Dec 2021 at 18:28, Martin Blumenstingl wrote: > Hi Jerome, > > On Mon, Dec 6, 2021 at 11:02 AM Jerome Brunet wrote: >> >> >> On Sun 05 Dec 2021 at 19:08, Martin Blumenstingl wrote: >> >> > The out-of-tree vendor driver uses the following approach to set the >> > AIU_I2S_MISC register: >> > 1) write AIU_MEM_I2S_START_PTR and AIU_MEM_I2S_RD_PTR >> > 2) configure AIU_I2S_MUTE_SWAP[15:0] >> > 3) write AIU_MEM_I2S_END_PTR >> > 4) set AIU_I2S_MISC[2] to 1 (documented as: "put I2S interface in hold >> > mode") >> > 5) set AIU_I2S_MISC[4] to 1 (depending on the driver revision it always >> > stays at 1 while for older drivers this bit is unset in step 4) >> > 6) set AIU_I2S_MISC[2] to 0 >> > 7) write AIU_MEM_I2S_MASKS >> > 8) toggle AIU_MEM_I2S_CONTROL[0] >> > 9) toggle AIU_MEM_I2S_BUF_CNTL[0] >> > >> > Additional testing shows that when AIU_I2S_MISC[2] is set to 1 then no >> > interrupts are generated anymore. The way this bit is managed by the >> > vendor driver as well as not getting any interrupts can mean that it's >> > related to the FIFO and not the encoder. >> >> Not necessarily. If the encoder stops pulling data, the FIFO will going >> over the DDR. Since it generates an IRQ after reading N bytes, IRQ would >> stop too. AFAIK, if the pipeline is stalled, the IRQ stops anyway > ah, right. so I think you're right: it can be either way > >> ... but this is not really important > I'll remove that section from the description in v2 > >> > >> > Move setting the AIU_I2S_MISC[2] bit to aiu_fifo_i2s_hw_params() so it >> > closer resembles the flow in the vendor kernel. While here also >> > configure AIU_I2S_MISC[4] (documented as: "force each audio data to >> > left or right according to the bit attached with the audio data") >> > similar to how the vendor driver does this. >> >> I understand the part of HOLD, not about FORCE_LR. >> Is it necessary to fix the problem ? Have you tested without this change >> ? > On my Le Potato board (GXL / S905X) FORCE_LR is either enabled by the > bootloader or being enabled is the default value. > All versions of the vendor driver are also setting it in some way, +1 Would you mind adding a comment in the code saying just that - so we know why it's there ? > including the latest one that I have access to [0]. > I prefer to keep this explicit write in for two reasons: > - we're not hit by surprise if some SoC/bootloaders don't set this bit > by default > - the code in the mainline does not skip anything that the vendor > driver does You can bet I've skipped a fair share of what the vendor driver does, on purpose. > > To specifically answer your question: I have not tried whether this > bit needs to be set and if unsetting it manually breaks anything. > >> > This fixes the infamous and >> > long-standing "machine gun noise" issue (a buffer underrun issue). >> >> Well done ! It took us a while to nail it, Thanks a lot !! > Thanks - this took a while to figure out but it's great to finally > have it solved! > > > Best regards, > Martin > > > [0] https://github.com/khadas/linux/blob/khadas-vims-4.9.y/sound/soc/amlogic/meson/audio_hw.c#L194:L202 With this, feel free to repost without the RFC tag and with my Acked-by: Jerome Brunet _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel