[Intel-gfx] [PATCH v3 2/4] drm/i915: implement sync_audio_rate callback
Yang, Libin
libin.yang at intel.com
Wed Aug 26 01:07:53 PDT 2015
Hi Daniel,
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Wednesday, August 26, 2015 3:53 PM
> To: Jani Nikula
> Cc: Yang, Libin; alsa-devel at alsa-project.org; tiwai at suse.de; intel-
> gfx at lists.freedesktop.org; daniel.vetter at ffwll.ch
> Subject: Re: [Intel-gfx] [PATCH v3 2/4] drm/i915: implement
> sync_audio_rate callback
>
> On Mon, Aug 17, 2015 at 03:20:38PM +0300, Jani Nikula wrote:
> > On Mon, 17 Aug 2015, 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 | 117
> +++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 117 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > > index dc32cf4..27add6a 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)
> > > +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 },
> > > +};
> > > +
> > > /* 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)
> > > +{
> > > + 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,79 @@ 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) {
> > > + intel_dig_port = enc_to_dig_port(&intel_encoder-
> >base);
> >
> > Mmmh, not all encoders are digital ports. enc_to_dig_port does a
> > container_of, so you can't just check for NULL either. IOW, you need
> to
> > check the encoder->type (and you only care about HDMI here).
>
> encoder->type is changed at runtime thanks to ddi encoders. Also
> shouldn't
> we care about dp audio too or am I confused?
Do you have suggestion how to handle it as it is changed
at runtime? Need a lock or not?
For DP, because the 4K DP display is not stable, so we didn't
do the full test on DP and no patch is ready for DP currently.
Based on the very simple test on DP, it seems DP audio is
OK. When DP is stable, we will do the full test and decide
to make the patch or not based on the test result.
Regards,
Libin
> -Daniel
>
> >
> > > + if (port == intel_dig_port->port) {
> > > + crtc = to_intel_crtc(intel_encoder->base.crtc);
> > > + 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);
> > > + 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
> > >
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
More information about the Intel-gfx
mailing list