Linux-Rockchip Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Neil Armstrong <narmstrong@baylibre.com>
To: Romain Perier <romain.perier@collabora.com>,
	Jose Abreu <Jose.Abreu@synopsys.com>,
	Russell King <linux@armlinux.org.uk>
Cc: 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
Date: Mon, 13 Mar 2017 10:27:08 +0100	[thread overview]
Message-ID: <25ad96a7-d907-2bcb-3a96-15a2956e7652@baylibre.com> (raw)
In-Reply-To: <20170310093509.19044-1-romain.perier@collabora.com>

On 03/10/2017 10:35 AM, Romain Perier wrote:
> Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
> step E. and is kept enabled for later use. This clock should be enabled
> and disabled along with the actual audio stream and not always on (that
> is bad for PM). Futhermore, this might cause sound glitches with some
> HDMI devices, as the CTS+N is forced to zero when the stream is disabled
> while the audio clock is still running.
> 
> This commit adds a parameter to hdmi_audio_enable_clk() that controls
> when the audio sample clock must be enabled or disabled. Then, it moves
> the call to this function into dw_hdmi_audio_enable() and
> dw_hdmi_audio_disable().
> 
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 

Hi Romain, Russell, Jose,

This is a little out of scope, but I was wondering why the CTS calculation was not left
in AUTO mode in the dw-hdmi driver ?

In the Amlogic Vendor tree, they don't set the HDMI_AUD_CTS3_CTS_MANUAL bit and leave
the CTS value to 0.

I am wondering if the AUTO feature is specific to newer IP version, or it was disabled
in the iMX.6 IP configuration ? or simply tied to the AHB audio specificity.

It would be simpler and smarter to leave the AUTO feature enabled for I2S and S/PDIF
input when the IP supports it.

Jose, is there a config bit for this feature, or a specific IP version where it landed ?

Anyway, we should really differentiate the AHB audio and I2S/SPDIF audio in the CTS/N calculation,
since the I2S/SPDIF depends on an different clock for audio input and the AUTO CTS calculation
feature will certainly offer a better clock synchronization.

Thanks,
Neil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2017-03-13  9:27 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-10  9:35 [PATCH] drm: dw_hdmi: Gate audio sampler clock from the enablement functions Romain Perier
     [not found] ` <20170310093509.19044-1-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
2017-03-10  9:46   ` Russell King - ARM Linux
2017-03-10 10:21     ` Romain Perier
2017-03-10 10:27       ` Russell King - ARM Linux
2017-03-10 10:58         ` Romain Perier
2017-03-10 11:15           ` Russell King - ARM Linux
2017-03-10 12:58             ` Romain Perier
2017-03-13  9:27 ` Neil Armstrong [this message]
2017-03-13  9:36   ` Russell King - ARM Linux
2017-03-13 12:55     ` Jose Abreu
2017-03-13 13:10       ` Russell King - ARM Linux
2017-03-13 18:49         ` Jose Abreu
2017-03-14  7:53           ` Romain Perier
2017-03-14  8:13             ` Neil Armstrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=25ad96a7-d907-2bcb-3a96-15a2956e7652@baylibre.com \
    --to=narmstrong@baylibre.com \
    --cc=Jose.Abreu@synopsys.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=romain.perier@collabora.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox