From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934608AbdCJLPZ (ORCPT ); Fri, 10 Mar 2017 06:15:25 -0500 Received: from pandora.armlinux.org.uk ([78.32.30.218]:35904 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933945AbdCJLPW (ORCPT ); Fri, 10 Mar 2017 06:15:22 -0500 Date: Fri, 10 Mar 2017 11:15:04 +0000 From: Russell King - ARM Linux To: Romain Perier Cc: Archit Taneja , David Airlie , Heiko Stuebner , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] drm: dw_hdmi: Gate audio sampler clock from the enablement functions Message-ID: <20170310111504.GT21222@n2100.armlinux.org.uk> References: <20170310093509.19044-1-romain.perier@collabora.com> <20170310094613.GQ21222@n2100.armlinux.org.uk> <7888eb1f-2781-d4ed-6f26-cb76e1ead4d2@collabora.com> <20170310102753.GR21222@n2100.armlinux.org.uk> <202cd871-eb1c-d8f4-62d6-bed479f0eece@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <202cd871-eb1c-d8f4-62d6-bed479f0eece@collabora.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 10, 2017 at 11:58:19AM +0100, Romain Perier wrote: > Hello, > > Le 10/03/2017 à 11:27, Russell King - ARM Linux a écrit : > > I also would not think that it's platform specific - remember that > > this is Designware IP, and it's likely that other platforms with > > exactly the same IP would suffer from the same problem. It's > > probably revision specific, but we need Synopsis' input on that. > > > > However, I do believe that your patch probably doesn't have much > > merit in any case: on a mode set, you enable the audio clock, and > > it will remain enabled until the audio device is first opened and > > then closed. If another mode set comes along, then exactly the > > same situation happens again. So, really it isn't achieving all > > that much. > > The fact is we still have sound glitches caused by this workaround on > Rockchip, we probably have a revision > of the IP without this issue. If CTS+N is not forced to zero we no > longer have the glitches :/ (with or without touching the clock) Are you sure that removing the workaround just doesn't result in the same bug as on iMX6 appearing? The bug concerned is the ordering of the channels, so unless you're (eg0 monitoring left/right separately or directing audio to just one channel, you may not (with modern TVs) realise that the channels are swapped. I'll include the errata description and impact below. There are occasional issues on iMX6 as well despite this work-around, but I don't clearly remember what devmem2 tweaks I've done in the past to get it to resolve itself, nor could I describe them from memory any better than "burbling audio". When the AHB Audio DMA is started, by setting to 1'b1 for the first time the register field AHB_DMA_START.data_buffer_ready, the AHB Audio DMA will request data from the AHB bus to fill its internal AHB DMA FIFO. It is possible that a AHB DMA FIFO read action occurs during the time window between the first sample stored on the AHB DMA FIFO and when the AHB DMA FIFO has stored, at least, the number of configured audio channels in samples. If this happens, the AHB DMA FIFO will reply with samples that are currently on the AHB Audio FIFO and will repeat the last sample after the empty condition is reached. Projected Impact: This will miss-align the audio stream, causing a shift in the distribution of audio on the channels on the HDMI sink side, with no knowledge of the DWC_hdmi_tx enabled system. If you know that this definitely does not apply to your version, then we need to split the audio enable/disable functions between the AHB and I2S variants. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: Fri, 10 Mar 2017 11:15:04 +0000 Subject: [PATCH] drm: dw_hdmi: Gate audio sampler clock from the enablement functions In-Reply-To: <202cd871-eb1c-d8f4-62d6-bed479f0eece@collabora.com> References: <20170310093509.19044-1-romain.perier@collabora.com> <20170310094613.GQ21222@n2100.armlinux.org.uk> <7888eb1f-2781-d4ed-6f26-cb76e1ead4d2@collabora.com> <20170310102753.GR21222@n2100.armlinux.org.uk> <202cd871-eb1c-d8f4-62d6-bed479f0eece@collabora.com> Message-ID: <20170310111504.GT21222@n2100.armlinux.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Mar 10, 2017 at 11:58:19AM +0100, Romain Perier wrote: > Hello, > > Le 10/03/2017 ? 11:27, Russell King - ARM Linux a ?crit : > > I also would not think that it's platform specific - remember that > > this is Designware IP, and it's likely that other platforms with > > exactly the same IP would suffer from the same problem. It's > > probably revision specific, but we need Synopsis' input on that. > > > > However, I do believe that your patch probably doesn't have much > > merit in any case: on a mode set, you enable the audio clock, and > > it will remain enabled until the audio device is first opened and > > then closed. If another mode set comes along, then exactly the > > same situation happens again. So, really it isn't achieving all > > that much. > > The fact is we still have sound glitches caused by this workaround on > Rockchip, we probably have a revision > of the IP without this issue. If CTS+N is not forced to zero we no > longer have the glitches :/ (with or without touching the clock) Are you sure that removing the workaround just doesn't result in the same bug as on iMX6 appearing? The bug concerned is the ordering of the channels, so unless you're (eg0 monitoring left/right separately or directing audio to just one channel, you may not (with modern TVs) realise that the channels are swapped. I'll include the errata description and impact below. There are occasional issues on iMX6 as well despite this work-around, but I don't clearly remember what devmem2 tweaks I've done in the past to get it to resolve itself, nor could I describe them from memory any better than "burbling audio". When the AHB Audio DMA is started, by setting to 1'b1 for the first time the register field AHB_DMA_START.data_buffer_ready, the AHB Audio DMA will request data from the AHB bus to fill its internal AHB DMA FIFO. It is possible that a AHB DMA FIFO read action occurs during the time window between the first sample stored on the AHB DMA FIFO and when the AHB DMA FIFO has stored, at least, the number of configured audio channels in samples. If this happens, the AHB DMA FIFO will reply with samples that are currently on the AHB Audio FIFO and will repeat the last sample after the empty condition is reached. Projected Impact: This will miss-align the audio stream, causing a shift in the distribution of audio on the channels on the HDMI sink side, with no knowledge of the DWC_hdmi_tx enabled system. If you know that this definitely does not apply to your version, then we need to split the audio enable/disable functions between the AHB and I2S variants. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.