[Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Aug 21 08:14:19 PDT 2015
On Tue, Aug 18, 2015 at 02:51:52PM +0800, libin.yang at intel.com wrote:
> From: Libin Yang <libin.yang at intel.com>
>
> HDMI audio may not work at some frequencies
> with the HW provided N/CTS.
>
> This patch sets the proper N value for the
> given audio sample rate at the impacted frequencies.
> At other frequencies, it will use the N/CTS value
> which HW provides.
>
> Signed-off-by: Libin Yang <libin.yang at intel.com>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 119 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 119 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index dc32cf4..96b97be 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -68,6 +68,31 @@ static const struct {
> { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> };
>
> +/* HDMI N/CTS table */
> +#define TMDS_297M 297000
> +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
I don't really like the defines.
> +static const struct {
> + int sample_rate;
> + int clock;
> + int n;
> + int cts;
> +} aud_ncts[] = {
> + { 44100, TMDS_296M, 4459, 234375 },
> + { 44100, TMDS_297M, 4704, 247500 },
> + { 48000, TMDS_296M, 5824, 281250 },
> + { 48000, TMDS_297M, 5120, 247500 },
> + { 32000, TMDS_296M, 5824, 421875 },
> + { 32000, TMDS_297M, 3072, 222750 },
> + { 88200, TMDS_296M, 8918, 234375 },
> + { 88200, TMDS_297M, 9408, 247500 },
> + { 96000, TMDS_296M, 11648, 281250 },
> + { 96000, TMDS_297M, 10240, 247500 },
> + { 176400, TMDS_296M, 17836, 234375 },
> + { 176400, TMDS_297M, 18816, 247500 },
> + { 44100, TMDS_296M, 23296, 281250 },
> + { 44100, TMDS_297M, 20480, 247500 },
> +};
Last two should be 192 kHz. All the other values seem to match the spec.
These do assume 8bpc, but as the spec doesn't even specify N/CTS
values for deep color modes @ 297MHz, so I suppose the expectations is
that the max TMDS clock will never be so high as to allow them.
If/when we start using manual programming for other TMDS clock rates
we'll need to consider 12bpc as well.
> +
> /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
> {
> @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
> return hdmi_audio_clock[i].config;
> }
>
> +static int audio_config_get_n(struct drm_display_mode *mode, int rate)
mode can be const.
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> + if ((rate == aud_ncts[i].sample_rate) &&
> + (mode->clock == aud_ncts[i].clock)) {
> + return aud_ncts[i].n;
> + }
> + }
> + return 0;
> +}
> +
> +/* check whether N/CTS/M need be set manually */
> +static bool audio_rate_need_prog(struct intel_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + if (((mode->clock == TMDS_297M) ||
> + (mode->clock == TMDS_296M)) &&
> + intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> + return true;
> + else
> + return false;
> +}
> +
> static bool intel_eld_uptodate(struct drm_connector *connector,
> int reg_eldv, uint32_t bits_eldv,
> int reg_elda, uint32_t bits_elda,
> @@ -514,12 +564,81 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
> return ret;
> }
>
> +static int i915_audio_component_sync_audio_rate(struct device *dev,
> + int port, int rate)
> +{
> + struct drm_i915_private *dev_priv = dev_to_i915(dev);
> + struct drm_device *drm_dev = dev_priv->dev;
> + struct intel_encoder *intel_encoder;
> + struct intel_digital_port *intel_dig_port;
> + struct intel_crtc *crtc;
> + struct drm_display_mode *mode;
> + enum pipe pipe = -1;
> + u32 tmp;
> + int n_low, n_up, n;
> +
> + /* 1. get the pipe */
> + for_each_intel_encoder(drm_dev, intel_encoder) {
> + if (intel_encoder->type != INTEL_OUTPUT_HDMI)
> + continue;
> + intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> + if (port == intel_dig_port->port) {
> + crtc = to_intel_crtc(intel_encoder->base.crtc);
This sort of thing would need some locking to safely start digging at
the modeset state.
I would probably not do that, and instead add some new lock(s) for this.
The modeset code would then fill out some relevant information protected
by that same lock, and this function could then go look at it without
having to worry about the rest of modeset locking/state.
> + if (!crtc) {
> + DRM_DEBUG_KMS("%s: crtc is NULL\n", __func__);
> + continue;
> + }
> + pipe = crtc->pipe;
> + break;
> + }
> + }
> +
> + if (pipe == INVALID_PIPE) {
> + DRM_DEBUG_KMS("no pipe for the port %c\n", port_name(port));
> + return -ENODEV;
> + }
> + DRM_DEBUG_KMS("pipe %c connects port %c\n",
> + pipe_name(pipe), port_name(port));
> + mode = &crtc->config->base.adjusted_mode;
> +
> + /* 2. check whether to set the N/CTS/M manually or not */
> + if (!audio_rate_need_prog(crtc, mode)) {
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
This stuff would need to be abstracted to work on pre-HSW too.
> + return 0;
> + }
> +
> + n = audio_config_get_n(mode, rate);
> + if (n == 0) {
> + DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
> + port_name(port));
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> + return 0;
> + }
> + n_low = n & 0xfff;
> + n_up = (n >> 12) & 0xff;
> +
> + /* 4. set the N/CTS/M */
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
> + tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> + (n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> + AUD_CONFIG_N_PROG_ENABLE);
> + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +
> + return 0;
> +}
> +
> static const struct i915_audio_component_ops i915_audio_component_ops = {
> .owner = THIS_MODULE,
> .get_power = i915_audio_component_get_power,
> .put_power = i915_audio_component_put_power,
> .codec_wake_override = i915_audio_component_codec_wake_override,
> .get_cdclk_freq = i915_audio_component_get_cdclk_freq,
> + .sync_audio_rate = i915_audio_component_sync_audio_rate,
> };
>
> static int i915_audio_component_bind(struct device *i915_dev,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list