> -----Original Message-----
> From: Takashi Iwai [mailto:ti...@suse.de]
> Sent: Monday, August 10, 2015 4:04 PM
> To: Yang, Libin
> Cc: alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org;
> daniel.vet...@ffwll.ch
> Subject: Re: [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
> 
> On Mon, 10 Aug 2015 09:32:11 +0200,
> libin.y...@intel.com wrote:
> >
> > From: Libin Yang <libin.y...@intel.com>
> >
> > When modeset occurs and the TMDS frequency is set to some
> > speical value, the N/CTS need to be set manually if audio
> > is playing.
> >
> > Signed-off-by: Libin Yang <libin.y...@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  6 ++++++
> >  drivers/gpu/drm/i915/intel_audio.c | 42
> ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 48 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index da2d128..85f3beb 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7030,6 +7030,12 @@ enum skl_disp_power_wells {
> >                                     _HSW_AUD_DIG_CNVT_2)
> >  #define DIP_PORT_SEL_MASK          0x3
> >
> > +#define _HSW_AUD_STR_DESC_1                0x65084
> > +#define _HSW_AUD_STR_DESC_2                0x65184
> > +#define AUD_STR_DESC(pipe) _PIPE(pipe, \
> > +                                    _HSW_AUD_STR_DESC_1,
>       \
> > +                                    _HSW_AUD_STR_DESC_2)
> > +
> >  #define _HSW_AUD_EDID_DATA_A               0x65050
> >  #define _HSW_AUD_EDID_DATA_B               0x65150
> >  #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index eddf37f..082f96d 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -235,6 +235,9 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> >     const uint8_t *eld = connector->eld;
> >     uint32_t tmp;
> >     int len, i;
> > +   int cvt_idx;
> > +   int n_low, n_up, n;
> > +   int base_rate, mul, div, rate;
> >
> >     DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes
> ELD\n",
> >                   pipe_name(pipe), drm_eld_size(eld));
> > @@ -267,6 +270,21 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> >     tmp |= AUDIO_ELD_VALID(pipe);
> >     I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> >
> > +   if ((mode->clock == 297000) ||
> > +           (mode->clock == DIV_ROUND_UP(297000 * 1000, 1001)))
> {
> 
> TMDS_297M and TMDS_296M can be used here?

Oh, sorry, I forgot to use it here.

> 
> 
> > +           tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
> > +           cvt_idx = (tmp >> pipe*8) & 0xff;
> > +           tmp = I915_READ(AUD_STR_DESC(cvt_idx));
> > +           base_rate = tmp & (1 << 14);
> > +           if (base_rate == 0)
> > +                   rate = 48000;
> > +           else
> > +                   rate = 44100;
> > +           mul = (tmp & (0x7 << 11)) + 1;
> > +           div = (tmp & (0x7 << 8)) + 1;
> > +           rate = rate * mul / div;
> > +   }
> > +
> >     /* Enable timestamps */
> >     tmp = I915_READ(HSW_AUD_CFG(pipe));
> >     tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > @@ -276,6 +294,30 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> >             tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >     else
> >             tmp |= audio_config_hdmi_pixel_clock(mode);
> > +
> > +   if ((mode->clock != 297000) &&
> > +           (mode->clock != DIV_ROUND_UP(297000 * 1000, 1001)))
> {
> > +           tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > +   } else {
> > +           for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> > +                   if ((rate == aud_ncts[i].sample_rate) &&
> > +                           (mode->clock == aud_ncts[i].clock)) {
> > +                           n = aud_ncts[i].n;
> > +                           break;
> > +                   }
> > +           }
> 
> Missing initialization of n?

Oh, yes, I will fix it.

> 
> > +           if (n != 0) {
> > +                   tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > +                   n_low = n & 0xfff;
> > +                   n_up = (n >> 12) & 0xff;
> > +                   tmp |= AUD_CONFIG_N_PROG_ENABLE;
> 
> You don't need to set it twice.

Yes.

> 
> > +                   tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > +                   tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
> > +                   tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > +                   tmp |= (n_low <<
> AUD_CONFIG_LOWER_N_SHIFT);
> > +           }
> > +   }
> > +
> >     I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  }
> >
> > --
> > 1.9.1
> >
> 
> 
> thanks,
> 
> Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to