Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani ; libin.y...@linux.intel.com;
> Pandiyan, Dhinakaran
> Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
>
> When modeset occurs and the LS_CLK is set to some special values in DP
> mode, the N/M need to be set manually if audio is playing. Otherwise the
> first several seconds may be silent in audio playback.
>
> The relationship of Maud and Naud is expressed in the following equation:
> Maud/Naud = 512 * fs / f_LS_Clk
>
> Please refer VESA DisplayPort Standard spec for details.
>
> Signed-off-by: Libin Yang
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_reg.h| 7 +++
> drivers/gpu/drm/i915/intel_audio.c | 100
> -
> 2 files changed, 105 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7359,6 +7359,13 @@ enum {
> #define _HSW_AUD_MISC_CTRL_B 0x65110
> #define HSW_AUD_MISC_CTRL(pipe) _MMIO_PIPE(pipe,
> _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
>
> +#define _HSW_AUD_M_CTS_ENABLE_A 0x65028
> +#define _HSW_AUD_M_CTS_ENABLE_B 0x65128
> +#define HSW_AUD_M_CTS_ENABLE(pipe) _MMIO_PIPE(pipe,
> _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
> +#define AUD_M_CTS_M_VALUE_INDEX(1 << 21)
> +#define AUD_M_CTS_M_PROG_ENABLE(1 << 20)
> +#define AUD_CONFIG_M_MASK 0xf
The last line cause misalignment after applying the patch.
> +
> #define _HSW_AUD_DIP_ELD_CTRL_ST_A 0x650b4
> #define _HSW_AUD_DIP_ELD_CTRL_ST_B 0x651b4
> #define HSW_AUD_DIP_ELD_CTRL(pipe) _MMIO_PIPE(pipe,
> _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 81df29ca4947..0bc2701b6c9c 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -57,6 +57,70 @@
> * struct &i915_audio_component_audio_ops @audio_ops is called from
> i915 driver.
> */
>
> +/* DP N/M table */
> +#define LC_540M 54
> +#define LC_270M 27
> +#define LC_162M 162000
> +
> +struct dp_aud_n_m {
> + int sample_rate;
> + int clock;
> + u16 n;
> + u16 m;
> +};
> +
> +static const struct dp_aud_n_m dp_aud_n_m[] = {
> + { 192000, LC_540M, 5625, 1024 },
> + { 176400, LC_540M, 9375, 1568 },
> + { 96000, LC_540M, 5625, 512 },
> + { 88200, LC_540M, 9375, 784 },
> + { 48000, LC_540M, 5625, 256 },
> + { 44100, LC_540M, 9375, 392 },
> + { 32000, LC_540M, 16875, 512 },
> + { 192000, LC_270M, 5625, 2048 },
> + { 176400, LC_270M, 9375, 3136 },
> + { 96000, LC_270M, 5625, 1024 },
> + { 88200, LC_270M, 9375, 1568 },
> + { 48000, LC_270M, 5625, 512 },
> + { 44100, LC_270M, 9375, 784 },
> + { 32000, LC_270M, 16875, 1024 },
> + { 192000, LC_162M, 3375, 2048 },
> + { 176400, LC_162M, 5625, 3136 },
> + { 96000, LC_162M, 3375, 1024 },
> + { 88200, LC_162M, 5625, 1568 },
> + { 48000, LC_162M, 3375, 512 },
> + { 44100, LC_162M, 5625, 784 },
> + { 32000, LC_162M, 10125, 1024 },
> +};
> +
> +static const struct dp_aud_n_m *
> +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) {
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> + if (rate == dp_aud_n_m[i].sample_rate &&
> + intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
> + return &dp_aud_n_m[i];
> + }
> +
> + return NULL;
> +}
> +
> +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int
> +rate) {
> + const struct dp_aud_n_m *nm =
> audio_config_dp_get_n_m(intel_crtc,
> +rate);
> +
> + return nm ? nm->m : 0;
> +}
> +
> +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int
> +rate) {
> + const struct dp_aud_n_m *nm =
> audio_config_dp_get_n_m(intel_crtc,
> +rate);
> +
> + return nm ? nm->n : 0;
> +}
> +
> static const struct {
> int clock;
> u32 config;
> @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
> const struct drm_display_mode *adjusted_mode) {
> struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> + struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> + int rate = acomp ? acomp->aud_sample_rate[port] : 0;
> enum pipe pipe = intel_crtc->pipe;
> - u32 tmp;
> + u32 tmp, n, m;
>
> tmp = I915_READ(HSW_AUD_CFG(pipe));
> tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> @@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel
Re: [Intel-gfx] [PATCH] drm/i915: Retry port as HDMI if dp_is_edp turns out to be false
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Lukas Wunner
> Hi,
>
> On Sun, Aug 09, 2015 at 01:12:53PM +0100, Chris Wilson wrote:
> > We follow the VBT as to whether a DDI port is used for eDP and if so,
> > do not attach a HDMI encoder to it. However there are machines for
> > which the VBT eDP flag is a lie (shocking!) and we fail to detect a eDP
> > link.
> > Furthermore, on those machines the HDMI is connected to that DDI port
> > but we ignore it.
> >
> > If we reorder our port initialisation to try the eDP setup first and
> > if that fails we can treat it as a normal DP port along with a HDMI
> > output. To accomplish this, we have to delay registering the DP
> > connector/encoder until after we establish its final type.
Sorry to jump in. Could this help another use case as below ?
We have some Bytrail machine (Bayley Bay), we applied HW rework to disable eDP
connectivity to DDI1 and enable DP on DDI 1.
But we found the i915 driver still take this DDI as eDP, not DP. we suspect
it's because VBT still takes it as DP and so i915 driver just follows.
If we don't want to revise VBT in BIOS after every rework, is there any other
way to let i915 detect this is a DP link?
Thanks
Mengdong
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88331
>
> The existing code seems very carefully crafted, taking into account the
> differences betweem various GPU generations etc, shuffling that around
> might risk breakage. FWIW, I'm wondering if just adding a quirk for this
> single
> Dell workstation might be justified?
>
> Best regards,
>
> Lukas
>
>
> > Signed-off-by: Chris Wilson
> > Cc: Jesse Barnes
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 27
> > drivers/gpu/drm/i915/intel_dp.c | 127
> +--
> > drivers/gpu/drm/i915/intel_drv.h | 6 +-
> > 3 files changed, 79 insertions(+), 81 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 0bcd1b1aba4f..aab8dfd1f8a5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13941,7 +13941,6 @@ static void intel_setup_outputs(struct
> > drm_device *dev) {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_encoder *encoder;
> > - bool dpd_is_edp = false;
> >
> > intel_lvds_init(dev);
> >
> > @@ -13983,31 +13982,33 @@ static void intel_setup_outputs(struct
> drm_device *dev)
> > intel_ddi_init(dev, PORT_D);
> > } else if (HAS_PCH_SPLIT(dev)) {
> > int found;
> > - dpd_is_edp = intel_dp_is_edp(dev, PORT_D);
> >
> > if (has_edp_a(dev))
> > intel_dp_init(dev, DP_A, PORT_A);
> >
> > + found = 0;
> > + /* PCH SDVOB multiplex with HDMIB */
> > if (I915_READ(PCH_HDMIB) & SDVO_DETECTED) {
> > - /* PCH SDVOB multiplex with HDMIB */
> > found = intel_sdvo_init(dev, PCH_SDVOB, true);
> > if (!found)
> > intel_hdmi_init(dev, PCH_HDMIB, PORT_B);
> > - if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
> > - intel_dp_init(dev, PCH_DP_B, PORT_B);
> > }
> > + if (!found && I915_READ(PCH_DP_B) & DP_DETECTED)
> > + intel_dp_init(dev, PCH_DP_B, PORT_B);
> >
> > - if (I915_READ(PCH_HDMIC) & SDVO_DETECTED)
> > - intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
> > -
> > - if (!dpd_is_edp && I915_READ(PCH_HDMID) & SDVO_DETECTED)
> > - intel_hdmi_init(dev, PCH_HDMID, PORT_D);
> > -
> > + found = 0;
> > if (I915_READ(PCH_DP_C) & DP_DETECTED)
> > - intel_dp_init(dev, PCH_DP_C, PORT_C);
> > + found = intel_dp_init(dev, PCH_DP_C, PORT_C);
> > + if (found != DRM_MODE_CONNECTOR_eDP &&
> > + I915_READ(PCH_HDMIC) & SDVO_DETECTED)
> > + intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
> >
> > + found = 0;
> > if (I915_READ(PCH_DP_D) & DP_DETECTED)
> > - intel_dp_init(dev, PCH_DP_D, PORT_D);
> > + found = intel_dp_init(dev, PCH_DP_D, PORT_D);
> > + if (found != DRM_MODE_CONNECTOR_eDP &&
> > + I915_READ(PCH_HDMID) & SDVO_DETECTED)
> > + intel_hdmi_init(dev, PCH_HDMID, PORT_D);
> > } else if (IS_VALLEYVIEW(dev)) {
> > /*
> > * The DP_DETECTED bit is the latched state of the DDC diff
> > --git
> > a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 1e64a8c1e7cb..8adf500f3989 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1031,14 +1031,13 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux,
Re: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
Hi Takashi/Dave,
Shall we move or cc this discussion on audio driver side to ALSA ML?
I think we also need to decide how to manage PCM devices for DP MST.
Now the HD-A driver create a PCM device for each pin, and the substream
number is 1 for each PCM. Now with DP MST enabled, each pin can support
multiple streams (e.g. 3 on Intel HSW/BDW/SKL).
There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number
of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other
vendors can also specify a value according to actual HW capabilities.
So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors
(for 3 display pipelines), so we can open up to 3 substreams at the same
time. When the audio driver finds all 3 convertors are used when opening
a new substream, it will fail.
- #2: Create PCM device dynamically. Only create a PCM devices for a device
entry which connects to monitor with audio support. When the monitor
is removed, the PCM device will be disconnected, closed and removed,
similar to the USB case.
This will change ALSA core. But there will be less PCM devices and
substreams, since the number of connected monitors Is decided by the
actual GPU display pipeline.
Could you share your preference on these 2 options or other suggestions?
Thanks
Mengdong
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Dave Airlie
> Sent: Wednesday, June 17, 2015 12:02 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream
> to hda codec.
>
> From: Dave Airlie
>
> Add new verbs GET_DP_STREAM_ID and SET_DP_STREAM_ID from Intel docs.
> get stream id and print in proc
> split ELD to be per device not per pin
> handle pd/eldv per device not per pin
> setup codec->dp_mst earlier.
>
> Signed-off-by: Dave Airlie
> ---
> include/sound/hda_verbs.h | 2 +
> sound/pci/hda/hda_codec.c | 1 +
> sound/pci/hda/hda_proc.c | 5 +-
> sound/pci/hda/patch_hdmi.c | 181
> +++--
> 4 files changed, 131 insertions(+), 58 deletions(-)
>
> diff --git a/include/sound/hda_verbs.h b/include/sound/hda_verbs.h index
> d0509db..3b62ac5 100644
> --- a/include/sound/hda_verbs.h
> +++ b/include/sound/hda_verbs.h
> @@ -75,6 +75,7 @@ enum {
> #define AC_VERB_GET_HDMI_CHAN_SLOT 0x0f34
> #define AC_VERB_GET_DEVICE_SEL 0xf35
> #define AC_VERB_GET_DEVICE_LIST 0xf36
> +#define AC_VERB_GET_DP_STREAM_ID 0xf3c
>
> /*
> * SET verbs
> @@ -115,6 +116,7 @@ enum {
> #define AC_VERB_SET_HDMI_CP_CTRL 0x733
> #define AC_VERB_SET_HDMI_CHAN_SLOT 0x734
> #define AC_VERB_SET_DEVICE_SEL 0x735
> +#define AC_VERB_SET_DP_STREAM_ID 0x73C
>
> /*
> * Parameter IDs
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index
> 5645481..3981c63 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -482,6 +482,7 @@ int snd_hda_get_devices(struct hda_codec *codec,
> hda_nid_t nid,
> }
> return devices;
> }
> +EXPORT_SYMBOL_GPL(snd_hda_get_devices);
>
> /*
> * destructor
> diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index
> baaf7ed0..39fac53 100644
> --- a/sound/pci/hda/hda_proc.c
> +++ b/sound/pci/hda/hda_proc.c
> @@ -644,10 +644,13 @@ static void print_device_list(struct snd_info_buffer
> *buffer,
> int i, curr = -1;
> u8 dev_list[AC_MAX_DEV_LIST_LEN];
> int devlist_len;
> + int dp_s_id;
>
> + dp_s_id = snd_hda_codec_read(codec, nid, 0,
> + AC_VERB_GET_DP_STREAM_ID, 0);
> devlist_len = snd_hda_get_devices(codec, nid, dev_list,
> AC_MAX_DEV_LIST_LEN);
> - snd_iprintf(buffer, " Devices: %d\n", devlist_len);
> + snd_iprintf(buffer, " Devices: %d: 0x%x\n", devlist_len, dp_s_id);
> if (devlist_len <= 0)
> return;
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index
> 5f44f60..8272656 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -68,6 +68,17 @@ struct hdmi_spec_per_cvt {
> /* max. connections to a widget */
> #define HDA_MAX_CONNECTIONS 32
>
> +struct hdmi_spec_per_pin;
> +#define HDA_MAX_DEVICES 3
> +struct hdmi_spec_per_device {
> + struct hdmi_spec_per_pin *pin;
> + int device_idx;
> + struct hdmi_eld sink_eld;
> +#ifdef CONFIG_PROC_FS
> + struct snd_info_entry *proc_entry;
> +#endif
> +};
> +
> struct hdmi_spec_per_pin {
> hda_nid_t pin_nid;
> int num_mux_nids;
> @@ -76,7 +87,11 @@ struct hdmi_spec_per_pin {
> hda_nid_t cvt_nid;
>
> struct hda_codec *codec;
> - struct hdmi_eld sink_eld;
> +
> + int num_devices;
> + u8 dev_list[AC_MAX_DEV_LIST_LEN];
> + struct hdmi_spec_per_device devices[HD
Re: [Intel-gfx] [PATCH 2/4] i915: add support for GPU side of MST audio
> -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of > Jani Nikula > On Wed, 17 Jun 2015, Chris Wilson wrote: > > On Wed, Jun 17, 2015 at 02:01:57PM +1000, Dave Airlie wrote: > >> From: Dave Airlie > >> > >> This just adds enables for the codecs and debugfs support for mst > >> connectors to print the audio info. > >> > >> This relies on patches to the audio code to do anything more useful. > >> > >> Signed-off-by: Dave Airlie > >> --- > >> @@ -231,6 +231,7 @@ static void hsw_audio_codec_enable(struct > drm_connector *connector, > >>/* Reset ELD write address */ > >>tmp = I915_READ(HSW_AUD_DIP_ELD_CTRL(pipe)); > >>tmp &= ~IBX_ELD_ADDRESS_MASK; > >> + tmp |= ((pipe + 1) << 29); I think we should not add this line. DIP Port Select [Bit 30:29 ]of register AUD_DIP_ELD_CTRL_ST is ReadOnly. For DP MST, I think the device select happens in both i915 driver and HD-Audio driver: - For i915 driver, it should select which transcoder/pipe is connected to a device on a port. MST allows 3 devices on a port. Could someone guide me to the code? The Bit 30:29 just reflect the selection, although I think this is not enough because it only tell the port number, but now we hope know the device entry on a port. - For audio driver, it should select which convertor is connected to a device of a codec pin (pin is a mapping for a DDI port). We need to add the support in HD-A driver. The default device entry is 0 for a pin. Current HD-A driver does not handle other 2 device entries on a pin. Thanks Mengdong > >>I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp); > > > > This is not MST specific. Is this a bug fix we want ASAP? > > > """ > This read-only bit reflects which port is used to transmit the DIP data. This > can > only change when DIP is disabled. If one or more audio-related DIP packets is > enabled and audio is enabled on a digital port, these bits will reflect the > digital > port to which audio is directed. > > For DP MST, this is the device select/pipe select. > """ > > We shouldn't mess with the field if it's regular DP. Also "pipe + 1" is too > magic; > unfortunately I don't know what it should be. :( > > BR, > Jani. > > > > > > > > > -Chris > > > > -- > > Chris Wilson, Intel Open Source Technology Centre > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] DP MST audio support
> -Original Message- > From: Takashi Iwai [mailto:ti...@suse.de] > Sent: Monday, May 18, 2015 5:21 PM > At Thu, 14 May 2015 09:10:33 +1000, > Dave Airlie wrote: > > > > On 12 May 2015 at 13:27, Dave Airlie wrote: > > > On 12 May 2015 at 11:50, Dave Airlie wrote: > > >> Hi, > > >> > > >> So I have a branch that makes no sound, > > >> http://cgit.freedesktop.org/~airlied/linux/log/?h=dp-mst-audio > > >> > > >> and I'm not sure where I need to turn to next, > > >> > > >> The Intel docs I've read are kinda vague, assuming you know lots of > > >> things I clearly don't. > > >> > > >> so in theory my branch, sets up the SDP stream to the monitor in > > >> the payload creation, enables the codec in the intel GPU driver, > > >> and passes the ELD to the audio driver. > > >> > > >> The audio driver uses the device list to get the presence/valid > > >> bits per device, and manages to retrieve the ELD. I even create ELD > > >> files in /proc/asound/HDMI/ that have sensible values in them > > >> > > >> So it looks like I'm just missing some routing somewhere, most > > >> likely in the audio driver, then again I could be missing a lot > > >> more than that. > > >> > > >> Just looking for any ideas or knowledge people may have locked in > > >> their brains or inside their firewalls. > > > > > > Okay the branch now has audio on my test setup, > > > > > > I've had to hack out the intel_not_share_assigned_cvt function it > > > appears to do bad things, I set pin 6 to connection 0 (pin 2), the > > > later sets on pin 5/7 to connection 1 by that function seems to reprogram > pin 6. > > > > > > I'm guessing the connection is assigned to a device not a pin in the > > > new hw, and the same device is routed via pin 5/7 so I end up trashing it. > > > > > > my test setup is a Haswell Lenovo t440s + docking station + Dell U2410. > > ping audio guys? > > > > can someone from alsa please take a look or some interest in this? > > Sorry, I've been on vacation for last two weeks. Now still swimming in the > flood of backlogs... Hi Artie, Sorry for the late reply. We don't have Haswell Lenovo t440s atm, so could you share more info? - Dell U2410 should support both HDMI and DP input. But I guess it cannot support DP MST, right? - Are you connecting this monitor a DP cable? Which DDI port is used? DDI B, C or D? - Does audio fail after i915 enables DP MST? - Is patch "snd/hdmi: hack out haswell codec workaround" the only change on audio driver side? > > The graphics side patches are fairly trivial, also it would be good to > > get a good explaination of how the hw works, > > > > from what I can see devices get connections not pins on this hw, and I > > notice that I don't always get 3 devices, so I'm not sure if devices > > are a dynamic thing we should be reprobing on some signal. Do you mean 3 PCM devices here, like pcmC0D3p, pcmC0D7p, pcmC0D8p? Now the devices are not dynamic, a PCM device is created on each pin. It seems we need to revise this for DP MST, since a pin can be used to send up to 3 independent streams on Intel GPU which has 3 display pipelines. > > The intel_not_share_assigned_cvt() was needed for Haswell HDMI/DP as there > was static routing between the pin and the converter widgets although the > codec graph shows it's selectable. We need to check why this fails. Even if MST is enabled, the convertors should also be selectable. > Was the pin default config of pin 6 enabled by BIOS properly? > > In anyway, Intel people should have a better clue about this; it's been > always a > strange behavior that is tied with the graphics... > > > thanks, > > Takashi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: provide interface for audio driver to query cdclk
> -Original Message- > From: Lespiau, Damien > Sent: Thursday, July 03, 2014 7:19 PM > To: Nikula, Jani > Cc: Lin, Mengdong; alsa-de...@alsa-project.org; ti...@suse.de; > intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: provide interface for audio > driver to query cdclk > > On Thu, Jul 03, 2014 at 10:32:38AM +0300, Jani Nikula wrote: > > On Thu, 03 Jul 2014, mengdong@intel.com wrote: > > > From: Jani Nikula > > > > I wrote this as a quick hack patch to try as an alternative to [1] > > which ended up not working on Haswell. Please reassure me that this is > > going to be a temporary solution until we get a more generic interface > > between the audio and display drivers. I don't much like this, but at > > least it's isolated and small. > > > > I'd like the commit message amended with something like: > > > > """ > > If the display power well has been disabled, the display audio > > controller divider values EM4 MVALUE and EM5 NVALUE will have been > > lost. The CDCLK frequency is required for reprogramming them. Provide > > a private interface for the audio driver to query CDCLK. > > > > This is a stopgap solution until a more generic interface between > > audio and display drivers has been implemented. > > """ > > > > I'd also like to have an additional Reviewed-by from the i915 side. > > After that, I'm fine with merging this through alsa. > > Sure. > > Reviewed-by: Damien Lespiau > > -- > Damien Thank you, Jani and Damien! I'll add the comment message and submit the revised patch tomorrow. Regards Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: provide interface for audio driver to query cdclk
Hi Jani, > -Original Message- > From: Nikula, Jani > Sent: Thursday, July 03, 2014 3:33 PM > I wrote this as a quick hack patch to try as an alternative to [1] which > ended up not working on Haswell. Please reassure me that this is going to > be a temporary solution until we get a more generic interface between the > audio and display drivers. I don't much like this, but at least it's isolated > and small. Sure. We'll clean up code when the generic interface is ready. This is only a temporary solution, safer than the BIOS notifications. And would you like to further revise this patch? But please keep this API not changed at the moment since the audio driver already queries the symbol name. Thanks Mengdong > I'd like the commit message amended with something like: > > """ > If the display power well has been disabled, the display audio controller > divider values EM4 MVALUE and EM5 NVALUE will have been lost. The > CDCLK frequency is required for reprogramming them. Provide a private > interface for the audio driver to query CDCLK. > > This is a stopgap solution until a more generic interface between audio > and display drivers has been implemented. > """ > > I'd also like to have an additional Reviewed-by from the i915 side. After > that, I'm fine with merging this through alsa. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [RFC] set up an sync channel between audio and display driver (i.e. ALSA and DRM)
> -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] > > Hi Daniel, > > > > Would you please share more info about your idea? > > > > - What would be an avsink device represent here? > > E.g. on Intel platforms, will the whole display device have a child > > avsink device or multiple avsink devices for each DDI port? > > My idea would be to have one for each output pipe (i.e. the link between > audio and gfx), not one per ddi. Gfx driver would then let audio know > when a screen is connected and which one (e.g. exact model serial from > edid). > This is somewhat important for dp mst where there's no longer a fixed > relationship between audio pin and screen Thanks. But if we use avsink device, I prefer to have an avsink device per DDI or several avsink devices per DDI, It's because 1. Without DP MST, there is a fixed mapping between each audio codec pin and DDI; 2. With DP MST, the above pin: DDI mapping is still valid (at least on Intel platforms), and there is also a fixed mapping between each device (screen) connected to a pin/DDI. 3. HD-Audio driver creates a PCM (audio stream) devices for each pin. Keeping this behavior can make audio driver works on platforms without implementing the sound/gfx sync channel. And I guess in the future the audio driver will creates more than one PCM devices for a DP MST-capable pin, according how many devices a DDI can support. 4. Display mode change can change the pipe connected to a DDI even if the monitor stays on the same DDI, If we have an avsink device per pipe, the audio driver will have to check another avsink device for this case. It seems not convenient. > > - And for the relationship between audio driver and the avsink device, > > which would be the master and which would be the component? > > 1:1 for avsink:alsa pin (iirc it's called a pin, not sure about the name). > That way the audio driver has a clear point for getting at the eld and > similar information. Since the audio driver usually already binds to some device (PCI or platform device), I think the audio driver cannot bind to the new avsink devices created by display driver, and we need a new driver to handle these device and communication. While the display driver creates the new endpoint "avsink" devices, the audio driver can also create the same number of audio endpoint devices. And we could let the audio endpoint device be the master and its peer display endpoint device be the component. Thus the master/component framework can help us to bind/unbind each pair of display/audio endpoint devices. Is it doable? If okay, I'll modify the RFC and see if there are other gaps. > > In addition, the component framework does not touch PM now. > > And introducing PM to the component framework seems not easy since > > there can be potential conflict caused by parent-child relationship of > > the involved devices. > > Yeah, the entire PM situation seems to be a bit bad. It also looks like on > resume/suspend we still have problems, at least on the audio side since > we need to coordinate between 2 completel different underlying devices. > But at least with the parent->child relationship we have a guranatee that > the avsink won't be suspended after the gfx device is already off. > -Daniel Yes. You're right. And we could find a way to hide the Intel-specific display "power well" from the audio driver by using runtime PM API on these devices. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [RFC] set up an sync channel between audio and display driver (i.e. ALSA and DRM)
> -Original Message- > From: Vetter, Daniel > Sent: Tuesday, May 20, 2014 11:08 PM > > On 20/05/2014 16:57, Thierry Reding wrote: > > On Tue, May 20, 2014 at 04:45:56PM +0200, Daniel Vetter wrote: > >> >On Tue, May 20, 2014 at 4:29 PM, Imre Deak > wrote: > >>> > >On Tue, 2014-05-20 at 05:52 +0300, Lin, Mengdong wrote: > >>>> > >>This RFC is based on previous discussion to set up a generic > >>>> > >>communication channel between display and audio driver and an > >>>> > >>internal design of Intel MCG/VPG HDMI audio driver. It's still > >>>> > >>an initial draft and your advice would be appreciated to > >>>> > >>improve the design. > >>>> > >> > >>>> > >>The basic idea is to create a new avsink module and let both > >>>> > >>drm and alsa depend on it. > >>>> > >>This new module provides a framework and APIs for > >>>> > >>synchronization between the display and audio driver. > >>>> > >> > >>>> > >>1. Display/Audio Client > >>>> > >> > >>>> > >>The avsink core provides APIs to create, register and lookup a > >>>> > >>display/audio client. > >>>> > >>A specific display driver (eg. i915) or audio driver (eg. > >>>> > >>HD-Audio > >>>> > >>driver) can create a client, add some resources objects (shared > >>>> > >>power wells, display outputs, and audio inputs, register ops) > >>>> > >>to the client, and then register this client to avisink core. > >>>> > >>The peer driver can look up a registered client by a name or > >>>> > >>type, or both. If a client gives a valid peer client name on > >>>> > >>registration, avsink core will bind the two clients as peer for > >>>> > >>each other. And we expect a display client and an audio client > >>>> > >>to be peers for each other in a system. > >>> > > > >>> > >One problem we have at the moment is the order of calling the > >>> > >system suspend/resume handlers of the display driver wrt. that of > >>> > >the audio driver. Since the power well control is part of the > >>> > >display HW block, we need to run the display driver's resume > >>> > >handler first, initialize the HW, and only then let the audio > >>> > >driver's resume handler run. For similar reasons we have to call > >>> > >the audio suspend handler first and only then the display driver > >>> > >resume handler. Currently we solve this using the display > >>> > >driver's late/early suspend/resume hooks, but we'd need a more robust > solution. > >>> > > > >>> > >This seems to be a similar issue to the load time ordering > >>> > >problem that you describe later. Having a real device for avsync > >>> > >that would be a child of the display device would solve the > >>> > >ordering issue in both cases. I admit I haven't looked into it if > >>> > >this is feasible, but I would like to see some solution to this as > >>> > >part of > the plan. > >> > > >> >Yeah, this is a big reason why I want real devices - we have piles > >> >of infrastructure to solve these ordering issues as soon as there's > >> >a struct device around. If we don't use that, we need to reinvent > >> >all those wheels ourselves. > > To make the driver core's magic work I think you'd need to find a way > > to reparent the audio device under the display device. Presumably they > > come from two different parts of the device tree (two different PCI > > devices I would guess for Intel, two different platform devices on > > SoCs). Changing the parent after a device has been registered doesn't > > work as far as I know. But even assuming that would work, I have > > trouble imagining what the implications would be on the rest of the driver > model. > > > > I faced similar problems with the Tegra DRM driver, and the only way I > > can see to make this kind of interaction between devices work is by > > tacking on an extra layer outside the core driver model. > That's why we need a new avsink device which is a proper child of the gfx > device, and the audio driver needs to use the componentized device > framework so that the suspend/resume ordering works correctly. Or at least > that's been my idea, might be we have some small gaps here and there. > -Daniel Hi Daniel, Would you please share more info about your idea? - What would be an avsink device represent here? E.g. on Intel platforms, will the whole display device have a child avsink device or multiple avsink devices for each DDI port? - And for the relationship between audio driver and the avsink device, which would be the master and which would be the component? In addition, the component framework does not touch PM now. And introducing PM to the component framework seems not easy since there can be potential conflict caused by parent-child relationship of the involved devices. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] set up an sync channel between audio and display driver (i.e. ALSA and DRM)
> -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of > Imre Deak > Sent: Thursday, May 22, 2014 1:08 AM > > On Wed, 2014-05-21 at 18:05 +0200, Daniel Vetter wrote: > > On Wed, May 21, 2014 at 5:56 PM, Babu, Ramesh > wrote: > > >> On Tue, May 20, 2014 at 05:29:07PM +0300, Imre Deak wrote: > > >> > On Tue, 2014-05-20 at 05:52 +0300, Lin, Mengdong wrote: > > >> > > This RFC is based on previous discussion to set up a generic > > >> > > communication channel between display and audio driver and an > > >> > > internal design of Intel MCG/VPG HDMI audio driver. It's still > > >> > > an initial draft and your advice would be appreciated to > > >> > > improve the design. > > >> > > > > >> > > The basic idea is to create a new avsink module and let both > > >> > > drm and alsa depend on it. > > >> > > This new module provides a framework and APIs for > > >> > > synchronization between the display and audio driver. > > >> > > > > >> > > 1. Display/Audio Client > > >> > > > > >> > > The avsink core provides APIs to create, register and lookup a > > >> > > display/audio client. > > >> > > A specific display driver (eg. i915) or audio driver (eg. > > >> > > HD-Audio > > >> > > driver) can create a client, add some resources objects (shared > > >> > > power wells, display outputs, and audio inputs, register ops) > > >> > > to the client, and then register this client to avisink core. > > >> > > The peer driver can look up a registered client by a name or type, or > both. > > >> > > If a client gives a valid peer client name on registration, > > >> > > avsink core will bind the two clients as peer for each other. > > >> > > And we expect a display client and an audio client to be peers > > >> > > for each other in a system. > > >> > > > >> > One problem we have at the moment is the order of calling the > > >> > system suspend/resume handlers of the display driver wrt. that of > > >> > the audio driver. Since the power well control is part of the > > >> > display HW block, we need to run the display driver's resume > > >> > handler first, initialize the HW, and only then let the audio > > >> > driver's resume handler run. For similar reasons we have to call > > >> > the audio suspend handler first and only then the display driver > > >> > resume handler. Currently we solve this using the display > > >> > driver's late/early suspend/resume hooks, but we'd need a more robust > solution. > > >> > > > >> > This seems to be a similar issue to the load time ordering > > >> > problem that you describe later. Having a real device for avsync > > >> > that would be a child of the display device would solve the > > >> > ordering issue in both cases. I admit I haven't looked into it if > > >> > this is feasible, but I would like to see some solution to this as > > >> > part of the > plan. > > >> > > >> If we are able create and mandate that HDMI display controller is > > >> parent and audio is child device, then this wouldn't be an issue > > >> and PM frameowrk will ensure parent is suspended last. > > >> > > > If there is a scenario where HDMI audio has to active but display > > > has to go to low power, then > parent-child device is not optimal. > > > There needs to be a mechanism to turn on/off individual hw blocks > > > within the controller. > > > > Our gfx runtime pm code is a _lot_ better than that. We track each > > power domain individually and enable/disable them only when need. > > armsoc drivers could do the same or make sure that the avsink device > > is a child of the right block. Of course if your driver only has > > binary runtime pm and fires up everything then we have a problem. But > > imo that's a problem with that driver, not with making avsink real > > devices as children of something. > > I would also add that at least in case of Haswell, there is really a hard > dependency between the display device and the HDMI audio > functionality: The power well required by HDMI is controlled via the > PWR_WELL
[Intel-gfx] Are there any recent i915 patchs that could possibly affect HDMI audio on BDW?
Hi , Are there any recent i915 patches that could possibly affect HDMI audio, during last two weeks? We got an audio regression on BDW that HDMI audio output becomes intermittent. Maybe HSW also has the same issue. It seems the HW DMA position is not correct and we suspect there maybe something wrong with audio data consuming on display side. Any tips would be appreciated to further check this issue. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] How to check the HDMI TMDS clock frequency and DP link symbol clock frequency, on HSW/BDW/VLV?
Hi, Could someone clarify how to check HDMI TMDS clock frequency and DisplayPort link symbol clock frequency? - Is there some registers which can dump the clock frequency? - Is 'clock' field in 'struct drm_display_mode' reflect the this clock? My platform is Haswell, Broadwell and Baytrail (Valleyview). I want to check - TMDS clock frequency for HDMI - Link Symbol clock for DP One test monitor has problem to play 48khz audio. So I want to make sure N/M/CTS values are right for the display clock and audio sample rate. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Why Baytrail Gfx driver not always uses pipe A when it's free?
Hi Jesse, Could you tell us why Baytrail Gfx driver does not always use pipe A when it's free? We have a Baytrail-M which only have HDMI (port B) and DisplayPort (port C) output. When I connect HDMI or DP only, the Gfx driver always uses pipe B for port B/C, although pipe A is free. This behavior does not change no matter I enable X or not. Is there any special consideration? This is different from what we observe on HSW/BDW, where pipe A is always used if it's free. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
> -Original Message- > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] > Sent: Wednesday, February 19, 2014 7:30 PM > > > > Is there a 1:1 mapping between these connector nodes and ports of > > > > Gfx > > > display engine? > > > > Eg. For Haswell Ultrabook, under > > > > /sys/devices/pci:00/:00:02.0/drm/card0/ > > > > There are four connector nodes, > > > > card0-DP-1 -> DDI port B > > > > card0-eDP-1/-> DDI port A > > > > card0-HDMI-A-1/ -> DDI Port C > > > > card0-HDMI-A-2/ -> Which DDI port ? Haswell-ULT does not > > > support port D, and I think port E is for VGA. > > > > > > There's no fixed mapping with the port and the connector name. The > > > number in the connector name is basically just a running number per > > > connector type. However I do believe we do register the connectors > > > in the order of the ports more or less always, so you can > > > *sometimes* deduce the port name from the connector. > > > > > > I suppose in this example HDMI-A-1 is port B, HDMI-A-2 is port C, > > > and > > > DP-1 can be either port B or port C. DP++ is the reason why we have > > > overlapping DP and HDMI connectors for the same port. > > > > Thanks for clarification, Ville! > > > > Does Haswell support DP++ on port B/C/D? > > Yes. > > > And will the name of these connector node change after system boot, > e.g. after S3/S4 cycles or hot-plug? > > The connector names can't change, except if you unload+reload the driver. It seems okay. We could let the gfx driver notify the audio driver when the connector nodes are destroyed or recreated. And the audio driver can always know the connector names no matter they change or not. > But I suppose DP MST might change this. Or maybe we'll just expose three > new fixed DP connectors per DDI port. One for each potential stream. It would be fine. > > As long as the connector name is constant, it can help to find out the > screen status no matter the port works in HDMI or DP mode. > > > > There is 1:1 mapping between audio output pins and DDI ports. > > So I guess we need to expose the port->connector mapping somehow to > the audio driver. When the gfx driver enables audio, the mapping between transcoder/port/connector is established. And the gfx driver can notify audio the connector name, to help the user space to know the audio could be output to which screen. > How does this work with DP MST? On the display side the audio stuff > happens in the transcoder, not the DDI port, AFAICS. For DP MST each > transcoder can provide a single stream. DP MST is not supported in the audio driver atm. But it could support in the future. E.g. for Haswell, audio driver can know the device index of a monitor connected to a port through HW unsolicited event, when the gfx driver establishes mapping between a transcoder and a monitor and enables audio on the transcoder. Different monitors connected to a same port simultaneously have different device index. Then the audio driver will specify the device index for an audio stream that flows to the right transcoder. And we're planning to replace the HW unsolicited event notification by SW notification between the gfx and audio driver. Thanks Mengdong > > And the new ALSA PCM 'screen' control for a pin can reflect the > connector name in right mode. E.g. for pin/port B, it can return HDMI-A-1 > or DP-1 depending on what kind of monitor is connected. > > > > Thanks > > Mengdong > > > > > > > > > > Hi Takashi, > > > > > > > > To make user space figure out which audio output is connected to > > > > which > > > screen (connector), maybe we can define a new ALSA control for each > > > HDMI/DP PCM device: > > > > e.g. numid=x,iface=PCM,name='Screen',device=3 > > > > Reading the control will return the name of the DRM connector > > > > nodes > > > like ' card0-DP-1'. The audio driver can get the connector name from > > > the gfx driver. > > > > > > > > For DP1.2 Multi-stream transport, it's not supported by i915 and > > > > HD-A > > > driver now. But probably there will be sub-nodes for the DP > > > connector node in the future and an index in their name can be used > > > distinguish monitors connected to the same DP port, like > > > card0-DP-1.1, card0-DP-1.2, > > > card0-DP-1.3 ... These names can be used by the above ALSA PCM > 'Screen' > > > control, so we can still know which audio output is to which monitor. > > > > > > > > Thanks > > > > Mengdong > > > > > > > > ___ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > > > Ville Syrjälä > > > Intel OTC > > -- > Ville Syrjälä > Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
> -Original Message- > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] > Sent: Tuesday, February 18, 2014 10:23 PM > To: Lin, Mengdong > Cc: Daniel Vetter; Takashi Iwai; alsa-de...@alsa-project.org; Barnes, Jesse; > Zanoni, Paulo R; dri-devel; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] Need your advice: Add a new communication > inteface between HD-Audio and Gfx drivers for hotplug notification/ELD > update > > On Tue, Feb 18, 2014 at 01:58:22PM +, Lin, Mengdong wrote: > > Sorry to pick up this thread after a long time. > > > > > -Original Message- > > > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On > > > Behalf Of Daniel Vetter > > > Sent: Thursday, January 23, 2014 4:27 PM > > > To: Takashi Iwai > > > > > On Thu, Jan 23, 2014 at 8:57 AM, Takashi Iwai wrote: > > > >> Thanks for clarification! > > > >> Maybe we can add output info (eg. display port number) to the eld > > > >> entries > > > under /proc/asound/cardx. Is it okay? > > > > > > > > It's possible, but the proc file is just a help. It can't be the API. > > > > For accessing the information, we'll need some new API, or let > > > > inform via sysfs of the new device. > > > > > > Links in sysfs sound like the best approach. drm already has nodes > > > for each connector, so on the gfx side there's a natural endpoint > already. > > > sysfs links also avoids any naming issues from the start, e.g. the > > > above DP connector id might lead to clashes with multiple cards. > > > > Hi Daniel, > > > > Is there a 1:1 mapping between these connector nodes and ports of Gfx > display engine? > > Eg. For Haswell Ultrabook, under > > /sys/devices/pci:00/:00:02.0/drm/card0/ > > There are four connector nodes, > > card0-DP-1 -> DDI port B > > card0-eDP-1/-> DDI port A > > card0-HDMI-A-1/ -> DDI Port C > > card0-HDMI-A-2/ -> Which DDI port ? Haswell-ULT does not > support port D, and I think port E is for VGA. > > There's no fixed mapping with the port and the connector name. The > number in the connector name is basically just a running number per > connector type. However I do believe we do register the connectors in the > order of the ports more or less always, so you can > *sometimes* deduce the port name from the connector. > > I suppose in this example HDMI-A-1 is port B, HDMI-A-2 is port C, and > DP-1 can be either port B or port C. DP++ is the reason why we have > overlapping DP and HDMI connectors for the same port. Thanks for clarification, Ville! Does Haswell support DP++ on port B/C/D? And will the name of these connector node change after system boot, e.g. after S3/S4 cycles or hot-plug? As long as the connector name is constant, it can help to find out the screen status no matter the port works in HDMI or DP mode. There is 1:1 mapping between audio output pins and DDI ports. And the new ALSA PCM 'screen' control for a pin can reflect the connector name in right mode. E.g. for pin/port B, it can return HDMI-A-1 or DP-1 depending on what kind of monitor is connected. Thanks Mengdong > > > > Hi Takashi, > > > > To make user space figure out which audio output is connected to which > screen (connector), maybe we can define a new ALSA control for each > HDMI/DP PCM device: > > e.g. numid=x,iface=PCM,name='Screen',device=3 > > Reading the control will return the name of the DRM connector nodes > like ' card0-DP-1'. The audio driver can get the connector name from the > gfx driver. > > > > For DP1.2 Multi-stream transport, it's not supported by i915 and HD-A > driver now. But probably there will be sub-nodes for the DP connector > node in the future and an index in their name can be used distinguish > monitors connected to the same DP port, like card0-DP-1.1, card0-DP-1.2, > card0-DP-1.3 ... These names can be used by the above ALSA PCM 'Screen' > control, so we can still know which audio output is to which monitor. > > > > Thanks > > Mengdong > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
Sorry to pick up this thread after a long time. > -Original Message- > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Thursday, January 23, 2014 4:27 PM > To: Takashi Iwai > On Thu, Jan 23, 2014 at 8:57 AM, Takashi Iwai wrote: > >> Thanks for clarification! > >> Maybe we can add output info (eg. display port number) to the eld entries > under /proc/asound/cardx. Is it okay? > > > > It's possible, but the proc file is just a help. It can't be the API. > > For accessing the information, we'll need some new API, or let inform > > via sysfs of the new device. > > Links in sysfs sound like the best approach. drm already has nodes for each > connector, so on the gfx side there's a natural endpoint already. > sysfs links also avoids any naming issues from the start, e.g. the above DP > connector id might lead to clashes with multiple cards. Hi Daniel, Is there a 1:1 mapping between these connector nodes and ports of Gfx display engine? Eg. For Haswell Ultrabook, under /sys/devices/pci:00/:00:02.0/drm/card0/ There are four connector nodes, card0-DP-1 -> DDI port B card0-eDP-1/-> DDI port A card0-HDMI-A-1/ -> DDI Port C card0-HDMI-A-2/ -> Which DDI port ? Haswell-ULT does not support port D, and I think port E is for VGA. Hi Takashi, To make user space figure out which audio output is connected to which screen (connector), maybe we can define a new ALSA control for each HDMI/DP PCM device: e.g. numid=x,iface=PCM,name='Screen',device=3 Reading the control will return the name of the DRM connector nodes like ' card0-DP-1'. The audio driver can get the connector name from the gfx driver. For DP1.2 Multi-stream transport, it's not supported by i915 and HD-A driver now. But probably there will be sub-nodes for the DP connector node in the future and an index in their name can be used distinguish monitors connected to the same DP port, like card0-DP-1.1, card0-DP-1.2, card0-DP-1.3 ... These names can be used by the above ALSA PCM 'Screen' control, so we can still know which audio output is to which monitor. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
> -Original Message- > From: Takashi Iwai [mailto:ti...@suse.de] > Sent: Thursday, January 23, 2014 1:19 AM > To: Daniel Vetter > Cc: Lin, Mengdong; Barnes, Jesse; Zanoni, Paulo R; > alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org; dri-devel > Subject: Re: Need your advice: Add a new communication inteface > between HD-Audio and Gfx drivers for hotplug notification/ELD update > > At Wed, 22 Jan 2014 15:18:21 +0100, > Daniel Vetter wrote: > > > > On Wed, Jan 22, 2014 at 12:48:04PM +, Lin, Mengdong wrote: > > > > -Original Message- > > > > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On > > > > Behalf Of Daniel Vetter > > > > Sent: Tuesday, January 21, 2014 9:11 PM > > > > To: Lin, Mengdong > > > > Cc: Takashi Iwai (ti...@suse.de); Barnes, Jesse; Zanoni, Paulo R; > > > > alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org > > > > Subject: Re: Need your advice: Add a new communication inteface > > > > between HD-Audio and Gfx drivers for hotplug notification/ELD > > > > update > > > > > > > > On Tue, Jan 21, 2014 at 1:35 PM, Lin, Mengdong > > > > > > > > wrote: > > > > > Dear audio and gfx stakeholders, > > > > > > > > > > > > > > > > > > > > We hope to add a new interface between audio and gfx driver, for > > > > > gfx driver to notify audio about HDMI/DP hot-plug and ELD update. > > > > > > > > > > Would you please share some comments on the proposal below? > > > > > > > > > > > > > > > > > > > > Background of this issue: On Intel Haswell/Broadwell platforms, > > > > > there is a HW restriction that after the display HD-Audio > > > > > controller is in D3, > > > > > > > > > > it cannot be waken up by HDMI/DP hot-plug. Consequently, > > > > > although the gfx driver can still detect the HDMI/DP hot-plug, > > > > > > > > > > audio driver has no idea about this and cannot notify user space > > > > > whether the external HDMI/DP monitor is available for audio > > > > > playback, > > > > > > > > > > because the audio controller cannot wake up to D0 and receive HW > > > > > unsolicited event about hot-plug from the audio codec. > > > > > > > > > > This limitation will affect user space to decide whether we can > > > > > output audio over HDMI/DP. > > > > > > > > > > > > > > > > > > > > To solve the above limitation, Takashi suggested to add a new > > > > > communication interface between audio and gfx driver: create a > > > > common > > > > > object > > > > > > > > > > containing the ops registered by both graphics and audio > > > > > drivers, then communicate through it, something like > vga_switcheroo. > > > > > > > > > > > > > > > > > > > > Is it okay to create this kernel object in i915 driver? > > > > > > > > > > > > > > > > > > > > I915 can export an API like "display_register_audio_client" for > > > > > audio driver to register a client and hot-plug notification ops. > > > > > > > > > > > > > > > > > > > > I915 can also call some API like "display_register_gfx_client" > > > > > itself and register ops for audio driver to query monitor > > > > > presence and ELD info on a specific port. > > > > > > > > > > It would be faster for audio driver than quering ELD by > > > > > command/response over the HD-A bus, thus avoid delay in i915 > > > > > mode > > > > set. > > > > > > > > > > This will also avoid waking up the audio devices unnecessarily > > > > > if the user space does not really want to use HDMI/DP for audio > playback. > > > > > > > > > > > > > > > > > > > > Whenever i195 enables/disables audio on a port in modeset, it > > > > > can call some API like "display_set_audio_state()" on this > > > > > kernel object and trigger notifications to the audio driver. > > > > > > > > > > > > > > > &g
Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
> -Original Message- > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Tuesday, January 21, 2014 9:11 PM > To: Lin, Mengdong > Cc: Takashi Iwai (ti...@suse.de); Barnes, Jesse; Zanoni, Paulo R; > alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org > Subject: Re: Need your advice: Add a new communication inteface > between HD-Audio and Gfx drivers for hotplug notification/ELD update > > On Tue, Jan 21, 2014 at 1:35 PM, Lin, Mengdong > wrote: > > Dear audio and gfx stakeholders, > > > > > > > > We hope to add a new interface between audio and gfx driver, for gfx > > driver to notify audio about HDMI/DP hot-plug and ELD update. > > > > Would you please share some comments on the proposal below? > > > > > > > > Background of this issue: On Intel Haswell/Broadwell platforms, there > > is a HW restriction that after the display HD-Audio controller is in > > D3, > > > > it cannot be waken up by HDMI/DP hot-plug. Consequently, although the > > gfx driver can still detect the HDMI/DP hot-plug, > > > > audio driver has no idea about this and cannot notify user space > > whether the external HDMI/DP monitor is available for audio playback, > > > > because the audio controller cannot wake up to D0 and receive HW > > unsolicited event about hot-plug from the audio codec. > > > > This limitation will affect user space to decide whether we can output > > audio over HDMI/DP. > > > > > > > > To solve the above limitation, Takashi suggested to add a new > > communication interface between audio and gfx driver: create a > common > > object > > > > containing the ops registered by both graphics and audio drivers, then > > communicate through it, something like vga_switcheroo. > > > > > > > > Is it okay to create this kernel object in i915 driver? > > > > > > > > I915 can export an API like "display_register_audio_client" for audio > > driver to register a client and hot-plug notification ops. > > > > > > > > I915 can also call some API like "display_register_gfx_client" itself > > and register ops for audio driver to query monitor presence and ELD > > info on a specific port. > > > > It would be faster for audio driver than quering ELD by > > command/response over the HD-A bus, thus avoid delay in i915 mode > set. > > > > This will also avoid waking up the audio devices unnecessarily if the > > user space does not really want to use HDMI/DP for audio playback. > > > > > > > > Whenever i195 enables/disables audio on a port in modeset, it can call > > some API like "display_set_audio_state()" on this kernel object and > > trigger notifications to the audio driver. > > > > > > > > When the audio driver is probed (in the delayed probe stage), it can > > request > > i915 API symbol to register the audio client for this communication > > kernel object. > > > > Since the 1st i915 mode set may happen before audio driver registers > > the ops, we'll let audio driver check ELD once after registering the > > audio client ops. > > > > And for the platforms which uses this communication interface, we can > > disable unsolicited event for HDMI/DP hot-plug in the audio driver. > > > > > > > > We hope to hear your feedback and start to work out more details. Thanks for your advice, Daniel! > Yeah, I've discussed this at KS with Takashi and we've agreed that some > common object to facilitate driver interactions. A few things > though: > - This should be common infrastructure useable by all alsa and drm > drivers, not just i915 and snd-hda. Especially on embedded platforms this > issue is fairly rampant ... Agree. Where to put this common object? Is it okay to put it under /driver/gpu/drm, similar to vga_switchroo? Shall we divide clients into audio and gfx categories, and define different ops for them? Since different info/request flow in different direction between audio and gfx. > - While at it it should also encompass power management handling of the > shared hw imo so that we can get rid of the hsw specific hacks for the > power well code. Or at least we need to rework the power well code to > reuse this new infrastructure, I don't really want to maintain a few copies > of the lazy symbol_get logic this kind of stuff requires. Sounds good. > - I think the biggest problem is figuring out who should register these > device nodes. I think it makes the most sense i
[Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
Dear audio and gfx stakeholders, We hope to add a new interface between audio and gfx driver, for gfx driver to notify audio about HDMI/DP hot-plug and ELD update. Would you please share some comments on the proposal below? Background of this issue: On Intel Haswell/Broadwell platforms, there is a HW restriction that after the display HD-Audio controller is in D3, it cannot be waken up by HDMI/DP hot-plug. Consequently, although the gfx driver can still detect the HDMI/DP hot-plug, audio driver has no idea about this and cannot notify user space whether the external HDMI/DP monitor is available for audio playback, because the audio controller cannot wake up to D0 and receive HW unsolicited event about hot-plug from the audio codec. This limitation will affect user space to decide whether we can output audio over HDMI/DP. To solve the above limitation, Takashi suggested to add a new communication interface between audio and gfx driver: create a common object containing the ops registered by both graphics and audio drivers, then communicate through it, something like vga_switcheroo. Is it okay to create this kernel object in i915 driver? I915 can export an API like "display_register_audio_client" for audio driver to register a client and hot-plug notification ops. I915 can also call some API like "display_register_gfx_client" itself and register ops for audio driver to query monitor presence and ELD info on a specific port. It would be faster for audio driver than quering ELD by command/response over the HD-A bus, thus avoid delay in i915 mode set. This will also avoid waking up the audio devices unnecessarily if the user space does not really want to use HDMI/DP for audio playback. Whenever i195 enables/disables audio on a port in modeset, it can call some API like "display_set_audio_state()" on this kernel object and trigger notifications to the audio driver. When the audio driver is probed (in the delayed probe stage), it can request i915 API symbol to register the audio client for this communication kernel object. Since the 1st i915 mode set may happen before audio driver registers the ops, we'll let audio driver check ELD once after registering the audio client ops. And for the platforms which uses this communication interface, we can disable unsolicited event for HDMI/DP hot-plug in the audio driver. We hope to hear your feedback and start to work out more details. Thanks & Best Regards Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915/vlv: enable HDA display audio for Valleyview2
Hi Daniel,
Could you help review this v4 patch?
I'm sorry again this patch does not in-reply-to the previous review thread.
I didn't generate a message id for my previous v2 patch by 'git format-patch'.
So I took the message id generated by 'git send-email' from the my Outlook
inbox, and generated this v4 patch.
But it seems that message id does not make this mail in the proper review
thread.
I'll always use 'git format-patch' to generate the message id in the future to
avoid such annoying issues.
Thanks
Mengdong
> -Original Message-
> From: Lin, Mengdong
> Sent: Friday, November 01, 2013 12:17 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Lin, Mengdong
> Subject: [PATCH v4] drm/i915/vlv: enable HDA display audio for Valleyview2
>
> From: Mengdong Lin
>
> This patch defines HD-Audio configuration registers and enables display audio
> from HDA controller for Valleyview2.
>
> v2: fix missing offset VLV_DISPLAY_BASE
> v3: rename patch from 'enable HDMI audio' to 'enable HDA display audio', since
> it's for both HDMI and DP audio
> v4: use enc_to_dig_port() to get port number, instead of using Haswell
> specific
> function intel_ddi_get_encoder_port()
>
> Signed-off-by: Mengdong Lin
> Reviewed-by: Jesse Barnes
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 47de41f..7f68215 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4924,6 +4924,18 @@
> CPT_AUD_CNTL_ST_B)
> #define CPT_AUD_CNTRL_ST20xE50C0
>
> +#define VLV_HDMIW_HDMIEDID_A (VLV_DISPLAY_BASE + 0x62050)
> +#define VLV_HDMIW_HDMIEDID_B (VLV_DISPLAY_BASE + 0x62150)
> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
> + VLV_HDMIW_HDMIEDID_A, \
> + VLV_HDMIW_HDMIEDID_B)
> +#define VLV_AUD_CNTL_ST_A(VLV_DISPLAY_BASE + 0x620B4)
> +#define VLV_AUD_CNTL_ST_B(VLV_DISPLAY_BASE + 0x621B4)
> +#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
> + VLV_AUD_CNTL_ST_A, \
> + VLV_AUD_CNTL_ST_B)
> +#define VLV_AUD_CNTL_ST2 (VLV_DISPLAY_BASE + 0x620C0)
> +
> /* These are the 4 32-bit write offset registers for each stream
> * output buffer. It determines the offset from the
> * 3DSTATE_SO_BUFFERs that the next streamed vertex output goes to.
> @@ -4940,6 +4952,12 @@
> #define CPT_AUD_CFG(pipe) _PIPE(pipe, \
> CPT_AUD_CONFIG_A, \
> CPT_AUD_CONFIG_B)
> +#define VLV_AUD_CONFIG_A (VLV_DISPLAY_BASE + 0x62000)
> +#define VLV_AUD_CONFIG_B (VLV_DISPLAY_BASE + 0x62100)
> +#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
> + VLV_AUD_CONFIG_A, \
> + VLV_AUD_CONFIG_B)
> +
> #define AUD_CONFIG_N_VALUE_INDEX (1 << 29)
> #define AUD_CONFIG_N_PROG_ENABLE (1 << 28)
> #define AUD_CONFIG_UPPER_N_SHIFT 20
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index c69a5b8..6328caf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6992,6 +6992,11 @@ static void ironlake_write_eld(struct drm_connector
> *connector,
> aud_config = IBX_AUD_CFG(pipe);
> aud_cntl_st = IBX_AUD_CNTL_ST(pipe);
> aud_cntrl_st2 = IBX_AUD_CNTL_ST2;
> + } else if (IS_VALLEYVIEW(connector->dev)) {
> + hdmiw_hdmiedid = VLV_HDMIW_HDMIEDID(pipe);
> + aud_config = VLV_AUD_CFG(pipe);
> + aud_cntl_st = VLV_AUD_CNTL_ST(pipe);
> + aud_cntrl_st2 = VLV_AUD_CNTL_ST2;
> } else {
> hdmiw_hdmiedid = CPT_HDMIW_HDMIEDID(pipe);
> aud_config = CPT_AUD_CFG(pipe);
> @@ -7001,8 +7006,19 @@ static void ironlake_write_eld(struct drm_connector
> *connector,
>
> DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(pipe));
>
> - i = I915_READ(aud_cntl_st);
> - i = (i >> 29) & DIP_PORT_SEL_MASK; /* DIP_Port_Select, 0x1
> =
> PortB */
> + if (IS_VALLEYVIEW(connector->dev)) {
> + struct intel_encoder *intel_encoder;
> + struct intel_digital_port *intel_dig_port;
> +
> + intel_encoder = intel_attached_encoder(connector);
> + intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> + i = intel_dig_
Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: enable HDMI audio for Valleyview2
Hi Daniel, Thanks for your clarification! Could you share more info ... > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Saturday, November 02, 2013 12:03 AM > To: Lin, Mengdong > Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: enable HDMI audio for > Valleyview2 > > On Fri, Nov 01, 2013 at 03:13:17PM +, Lin, Mengdong wrote: > > > Maybe we could use the port CRC stuff to make sure that audio is > > > actually working ... > > > > Would you please clarify what's port CRC stuff? > > The hw has a bunch of CRC (checksum) functions. We've just enabled it at the > pipe level, and it's extremely useful to test whether e.g. the cursor is > displaying > properly or whether it's not shown at all. We've had a few bugs where the > cursor was disabled but shouldnt have been. > > For an example of such a testcase see i-g-t/tests/kms_cursor_crc.c. This is > all > really new, so still in flux. > > Now the hardware also has checksum support for each port, and afaik that > includes the audio stream. Iirc never platforms even have special CRCs for the > individual audio streams. My idea for a testcase would be to expose these port > CRCs. Then check that the CRC is stable for each display frame while no audio > is > playing, and that it changes (and that the right port starts to change) once > we > play an audio stream. What's 'IIRC never platforms'? Where can I find more information about port or audio CRC? In Baytrail b-spec, I saw CRCCtrlRedA-Pipe A CRC Color Channel Control Register can select source: CRC Source Select: These bits select the source of the data to put into the CRC logic. : Pipe A 0110: DisplayPort B 0111: DisplayPort C 1000: Audio DP (Audio for DisplayPort (pcdclk). Only select when Audio is on DisplayPort on Pipe A) 1001: Audio HDMI (Audio for HDMI (dotclock) Only select when Audio is on HDMI on Pipe A) But I still cannot understand how to get CRC for both video and audio. And does the CRC does not change for each display frame, even when a video is playing and pictures content change from frame to frame? I hope to know more about how HW generates the CRC, so your info or any documentation will be appreciated. > We can't test sync issues with that, but routing issues (which seems to have > been the issue here, and I've also seen a few patches for routing issues in > the > past) should be testable. And with an automated testcase we can ensure that no > regression creps in. If the audio CRC help to check audio data arrives to the proper pipe and port, it will help to check routing issues. > > The other upside of an automated test like that is that it should be easy to > test > all port combinations. That's more important for desktop platforms where we > have 3 HDMI/DP ports. > > Anyway I've just thought I'll bring this up as an idea to look into for the > next hw > enabling project. It was a bit an effort to enable pipe CRCs for display > testing, > but I think long-term it will definitely be worth it. > So maybe poke the audio hw engineers/validation ppl a bit and inquire > whether/how exactly they use this? We've had a display micro architect help us > out a bit on the display side. > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: enable HDMI audio for Valleyview2
Hi Daniel,
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Sunday, October 27, 2013 9:59 PM
> To: Lin, Mengdong
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: enable HDMI audio for
> Valleyview2
>
> On Wed, Oct 23, 2013 at 07:08:11PM -0400, mengdong@intel.com wrote:
> > From: Mengdong Lin
> >
> > This patch defines audio configuration registers and adds audio
> > enabling code for Valleyview2.
> >
> > Signed-off-by: Mengdong Lin
> > Reviewed-by: Jesse Barnes
>
> [snip]
>
> > @@ -6905,8 +6910,19 @@ static void ironlake_write_eld(struct
> > drm_connector *connector,
> >
> > DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(pipe));
> >
> > - i = I915_READ(aud_cntl_st);
> > - i = (i >> 29) & DIP_PORT_SEL_MASK; /* DIP_Port_Select, 0x1
> > =
> PortB */
> > + if (IS_VALLEYVIEW(connector->dev)) {
> > + struct intel_encoder *intel_encoder;
> > + int port = 0;
> > + intel_encoder = intel_attached_encoder(connector);
> > + if (intel_encoder)
> > + port = intel_ddi_get_encoder_port(intel_encoder);
>
> This is a haswell specific function. Please use
> enc_to_dig_port(intel_encoder)->port instead, which does the right thing on
> all
> platforms for hdmi/dp ports.
Fixed in v4 patch.
> Also, when poking for review feedback (like you've done in private to Jesse
> and
> me) please consider next time around that:
> - Never drop the public mailings lists. That way other people can also
> notice that a patch needs attention. Also Jesse's r-b tag is now lost
> since he replied to your private mail.
> - Leave more than 8 hours of time for review to happen. In that time frame
> most of our team was off-duty. A few days is a good waiting time.
>
> For the patch itself please add a patch changelog so that everyone knows what
> changed from v1 to v2. This is doubly important since the review happened
> off-list.
>
> Finally please submit updated patches in reply to the original submission or
> the
> patch review. Tightly grouping discussions like that helps with managing the
> mail
> flood on a busy place like intel-gfx.
Many thanks for your suggestions! I'll follow the rules.
> Furthermore v1 was rather decently broken with the wrong register offset due
> to lack of the VLV_DISPLAY_BASE offset. So I'm wondering how solid your
> testing is and whether we can automate this somehow to ensure we cover all
> ugly combinations of ports and pipes. As-is I suspect you often won't notice
> that
> things work simply due to settings left behind by the bios or register default
> values matching what we want.
As you say, v1 patch wrote to a bad address and so HDMI audio happens to work
just because the default value matches the value I want to set.
If I test DP audio with v1 which request changing the default value, sound
cannot be heard at all. I'll be more careful.
> Maybe we could use the port CRC stuff to make sure that audio is actually
> working ...
Would you please clarify what's port CRC stuff?
> I won't block byt enabling on this, but expect me to be a royal pita for the
> next
> platform enabling ;-)
Regards
Mengdong
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell
Hi,
Is it okay to integrate this patch at first?
We can check whether vblank wait can be removed safely later
Thanks
Mengdong
> -Original Message-
> From: Lin, Mengdong
> Sent: Tuesday, September 24, 2013 1:01 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Arora, MukeshX; Lin, Mengdong
> Subject: [PATCH v3 1/2] drm/i915/hsw: Add display Audio codec disable
> sequence for Haswell
>
> From: Mukesh
>
> The code defines a new function intel_disable_audio() to implement the entire
> audio codec disable sequence, called by intel_disable_ddi() in DDI port
> disablement. This audio codec disbale sequence is implemented as per the
> recommendation of the Bspec.
>
> [Patch modified by Mendong to apply to both HDMI and DP port.]
>
> Signed-off-by: Mukesh Arora
> Signed-off-by: Mengdong Lin
> Reviewed-by: Ben Widawsky
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index b042ee5..bda9181 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1088,6 +1088,50 @@ static void intel_ddi_post_disable(struct
> intel_encoder *intel_encoder)
> I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); }
>
> +/* audio codec disable sequence, as per Bspec */ void
> +intel_disable_audio(struct intel_encoder *intel_encoder) {
> + int type = intel_encoder->type;
> + struct drm_encoder *encoder = &intel_encoder->base;
> + struct drm_device *dev = encoder->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> + int pipe = intel_crtc->pipe;
> + int aud_config = HSW_AUD_CFG(pipe);
> + u32 temp;
> +
> + /* need not disable sample fabrication because never enabled */
> +
> + /* disable timestamps */
> + temp = I915_READ(aud_config);
> + if (type == INTEL_OUTPUT_HDMI)
> + temp &= ~AUD_CONFIG_N_VALUE_INDEX;
> + else if (type == INTEL_OUTPUT_DISPLAYPORT)
> + temp |= AUD_CONFIG_N_VALUE_INDEX;
> + else
> + return;
> + temp |= AUD_CONFIG_N_PROG_ENABLE;
> + temp &=
> ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE);
> + I915_WRITE(aud_config, temp);
> + POSTING_READ(aud_config);
> +
> + /* Disable ELDV and ELD buffer */
> + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> + temp &= ~(AUDIO_ELD_VALID_A << (pipe * 4));
> + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
> + POSTING_READ(HSW_AUD_PIN_ELD_CP_VLD);
> +
> + /* Wait for 2 vertical blanks */
> + intel_wait_for_vblank(dev, pipe);
> + intel_wait_for_vblank(dev, pipe);
> +
> + /* Disable audio PD. This is optional as per Bspec. */
> + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> + temp &= ~(AUDIO_OUTPUT_ENABLE_A << (pipe * 4));
> + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
> + POSTING_READ(HSW_AUD_PIN_ELD_CP_VLD);
> +}
> +
> static void intel_enable_ddi(struct intel_encoder *intel_encoder) {
> struct drm_encoder *encoder = &intel_encoder->base; @@ -1132,18
> +1176,10 @@ static void intel_disable_ddi(struct intel_encoder
> *intel_encoder)
> struct drm_encoder *encoder = &intel_encoder->base;
> struct drm_crtc *crtc = encoder->crtc;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> int type = intel_encoder->type;
> - struct drm_device *dev = encoder->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - uint32_t tmp;
>
> - if (intel_crtc->eld_vld && type != INTEL_OUTPUT_EDP) {
> - tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> - tmp &= ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) <<
> - (pipe * 4));
> - I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> - }
> + if (intel_crtc->eld_vld && type != INTEL_OUTPUT_EDP)
> + intel_disable_audio(intel_encoder);
>
> if (type == INTEL_OUTPUT_EDP) {
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> --
> 1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/hsw: add flag has_audio in crtc config
Hi Daniel,
Thanks for your advice!
Would you help working on this patch? Or can I continue after a few days, I
have some Android support task these days.
Regards
Mengdong
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Tuesday, September 24, 2013 8:48 PM
> To: Lin, Mengdong
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/hsw: add flag has_audio in
> crtc
> config
>
> On Tue, Sep 24, 2013 at 01:01:37AM -0400, mengdong@intel.com wrote:
> > From: Mengdong Lin
> >
> > This patch adds a flag "has_audio" for audio presence in intel_crtc->config.
> > HMDI and DP encoders set this flag in their computer_config() if the
> > external monitor supports audio. Later audio sequence will check this flag.
> >
> > Signed-off-by: Mengdong Lin
>
> [snip]
>
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 26e162b..4bc125e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -764,6 +764,8 @@ found:
> >
> > intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
> >
> > + pipe_config->has_audio = intel_dp->has_audio;
> > +
>
> This should only be set when we actually have an audio-capable monitor and
> want to enable the audio output on the port. Furthermore you need to add hw
> state readout support for this boolean to haswell_get_pipe_config and also the
> relevant state check code to intel_pipe_config_compare (by adding
> PIPE_CONF_CHECK_I(has_audio)).
>
> Same applies to the intel_hdmi.c part ofc.
>
> Cheers, Daniel
>
> > return true;
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index b7d6e09..2bdc23c 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -290,6 +290,7 @@ struct intel_crtc_config {
> > struct intel_link_m_n fdi_m_n;
> >
> > bool ips_enabled;
> > + bool has_audio;
> > };
> >
> > struct intel_crtc {
> > @@ -303,7 +304,6 @@ struct intel_crtc {
> > * some outputs connected to this crtc.
> > */
> > bool active;
> > - bool eld_vld;
> > bool primary_disabled; /* is the crtc obscured by a plane? */
> > bool lowfreq_avail;
> > struct intel_overlay *overlay;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 2fd3fd5..09c9d69 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -864,6 +864,8 @@ bool intel_hdmi_compute_config(struct
> intel_encoder *encoder,
> > return false;
> > }
> >
> > + pipe_config->has_audio = intel_hdmi->has_audio;
> > +
> > return true;
> > }
> >
> > --
> > 1.8.1.2
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell
> -Original Message- > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Monday, September 23, 2013 4:57 PM > To: Lin, Mengdong > Cc: ville.syrj...@linux.intel.com; intel-gfx; Arora, MukeshX > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec > disable sequence for Haswell > > On Mon, Sep 23, 2013 at 10:52 AM, Lin, Mengdong > wrote: > >> Also I'd really like to see the audio stuff being tracked in the pipe > >> config instead of splattering these different ad-hoc state bits like > >> intel_crtc->eld_vld all over the place. > >> -Daniel > > > > How about adding a flag "has_audio" to intel_crtc->config? > > If okay, I'll write a patch to clean up checking on intel_crtc->eld_vld > > here and > there. > > That's actually my plan: HDMI/DP encoders should set config->has_audio in > their ->compute_config if we have audio enabled, and everything else should > then just check intel_crtc->config.has_audio. If you do the patch for hsw I'll > volunteer to convert older platforms. > -Daniel > -- Okay. I'll do the patch for HSW. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell
Hi Daniel and Ville, Thanks for your feedback and please see comments below ... I'm very sorry I missed your mails for a long time. > -Original Message- > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Thursday, September 05, 2013 2:50 AM > To: Lin, Mengdong > Cc: intel-gfx; Arora, MukeshX > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec > disable sequence for Haswell > > On Fri, Aug 30, 2013 at 1:50 AM, wrote: > > + /* Wait for 2 vertical blanks */ > > + intel_wait_for_vblank(dev, pipe); > > + intel_wait_for_vblank(dev, pipe); > > + > > + /* Disable audio PD. This is optional as per Bspec. */ > > + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); > > + temp &= ~(AUDIO_OUTPUT_ENABLE_A << (pipe * 4)); > > + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp); > > If this is optional do we really need the two vblank waits above? > Adding them just for fun when we generally try to rip out as many vblank waits > as possible from the modeset code isn't all that great ... The two vblank wait is requested by b-spec, not optional. Can we reserve them now until we got confirmation from HW owner that's safe to remove them? Or until we have better implementation of vblank wait? > Also I'd really like to see the audio stuff being tracked in the pipe config > instead > of splattering these different ad-hoc state bits like intel_crtc->eld_vld all > over > the place. > -Daniel How about adding a flag "has_audio" to intel_crtc->config? If okay, I'll write a patch to clean up checking on intel_crtc->eld_vld here and there. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell
Hi Ben,
Thank you very much for the review. I'm sorry I missed the feedback for quite a
long time.
Please see comments below ...
> -Original Message-
> From: Ben Widawsky [mailto:b...@bwidawsk.net]
> Sent: Thursday, September 05, 2013 1:03 AM
> To: Lin, Mengdong
> Cc: intel-gfx@lists.freedesktop.org; Arora, MukeshX
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec
> disable sequence for Haswell
>
> On Thu, Aug 29, 2013 at 07:50:20PM -0400, mengdong@intel.com wrote:
> > From: Mukesh
> >
> > The code defines a new function intel_disable_audio() to implement the
> > entire audio codec disable sequence, called by intel_disable_ddi() in
> > DDI port disablement. This audio codec disbale sequence is implemented
> > as per the recommendation of the Bspec.
> >
> > [Patch modified by Mendong to apply to both HDMI and DP port.]
> >
> > Signed-off-by: Mukesh Arora
> > Signed-off-by: Mengdong Lin
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index b042ee5..2946fe7 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1088,6 +1088,45 @@ static void intel_ddi_post_disable(struct
> intel_encoder *intel_encoder)
> > I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); }
> >
> > +/* audio codec disable sequence, as per Bspec */ void
> > +intel_disable_audio(struct intel_encoder *intel_encoder) {
> > + int type = intel_encoder->type;
> > + struct drm_encoder *encoder = &intel_encoder->base;
> > + struct drm_device *dev = encoder->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> > + int pipe = intel_crtc->pipe;
> > + int aud_config = HSW_AUD_CFG(pipe);
> > + u32 temp;
> > +
>
> I don't know what it means, but where is the : "Disable sample fabrication"
> step?
No, there is no "Disable sample fabrication" step, because sample fabrication
is never enabled.
I'll add a comment in code about this.
> > + /* disable timestamps */
> > + temp = I915_READ(aud_config);
> > + if (type == INTEL_OUTPUT_HDMI)
> > + temp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > + else if (type == INTEL_OUTPUT_DISPLAYPORT)
> > + temp |= AUD_CONFIG_N_VALUE_INDEX;
> > + else
> > + return;
> > + temp |= AUD_CONFIG_N_PROG_ENABLE;
> > + temp &=
> ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE);
> > + I915_WRITE(aud_config, temp);
> > +
> > + /* Disable ELDV and ELD buffer */
> > + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > + temp &= ~(AUDIO_ELD_VALID_A << (pipe * 4));
> > + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
> > +
>
> POSTING_READ or a comment explaining why you don't need one.
I'll add POST_READ.
> > + /* Wait for 2 vertical blanks */
> > + intel_wait_for_vblank(dev, pipe);
> > + intel_wait_for_vblank(dev, pipe);
> > +
> > + /* Disable audio PD. This is optional as per Bspec. */
> Please add the comment when software might want to skip this step (it seems
> relevant to me for future programming)
> > + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > + temp &= ~(AUDIO_OUTPUT_ENABLE_A << (pipe * 4));
> > + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp); }
> > +
>
> The bspec isn't clear about any implied waits in the steps, but it's probably
> safer to add a POSTING_READ after each I915_WRITE, unless you specifically
> know the HW doesn't need the last write to have landed.
>
> With all comments addressed, it's:
> Reviewed-by: Ben Widawsky
Okay. Thanks!
Mengdong
> > static void intel_enable_ddi(struct intel_encoder *intel_encoder) {
> > struct drm_encoder *encoder = &intel_encoder->base; @@ -1132,18
> > +1171,10 @@ static void intel_disable_ddi(struct intel_encoder
> *intel_encoder)
> > struct drm_encoder *encoder = &intel_encoder->base;
> > struct drm_crtc *crtc = encoder->crtc;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - int pipe = intel_crtc->pipe;
> > int type = intel_encoder->type;
> > - struct drm_device *dev = encoder->dev;
> > - struct drm_i915_private *dev_priv = dev->dev_private;
> > - uint32_t tmp;
> >
> > - if (intel_crtc->eld_vld && type != INTEL_OUTPUT_EDP) {
> > - tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > - tmp &= ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) <<
> > -(pipe * 4));
> > - I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> > - }
> > + if (intel_crtc->eld_vld && type != INTEL_OUTPUT_EDP)
> > + intel_disable_audio(intel_encoder);
> >
> > if (type == INTEL_OUTPUT_EDP) {
> > struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>
> This hunk looks fine.
>
> --
> Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: skip useless vblank wait on Haswell audio sequence
Hi Daniel and Paulo,
I think we need a confirmation from HW owner whether vblank wait can be skipped
in audio enabling and disabling. I'll ping HW owner and involve you.
The original code just follows b-spec but there is no explanation why.
Thanks
Mengdong
> -Original Message-
> From: intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org
> [mailto:intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org] On
> Behalf Of Daniel Vetter
> Sent: Friday, September 20, 2013 4:18 PM
> To: Paulo Zanoni
> Cc: intel-gfx@lists.freedesktop.org; Zanoni, Paulo R
> Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915: skip useless vblank wait on
> Haswell audio sequence
>
> On Thu, Sep 19, 2013 at 05:07:29PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni
> >
> > We call haswell_write_eld at mode_set time, not at crtc_enable time,
> > so the pipes are stopped, and it doesn't really make sense to wait for
> > a vblank: it just delays the modeset in 50ms.
> >
> > Leave the code there (commented with FIXME) for now since maybe we
> > need a bigger rework.
> >
> > Signed-off-by: Paulo Zanoni
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 14 +++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 69e8bb6..b891a0c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6662,7 +6662,6 @@ static void haswell_write_eld(struct
> > drm_connector *connector, {
> > struct drm_i915_private *dev_priv = connector->dev->dev_private;
> > uint8_t *eld = connector->eld;
> > - struct drm_device *dev = crtc->dev;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > uint32_t eldv;
> > uint32_t i;
> > @@ -6684,8 +6683,17 @@ static void haswell_write_eld(struct
> drm_connector *connector,
> > tmp |= (AUDIO_OUTPUT_ENABLE_A << (pipe * 4));
> > I915_WRITE(aud_cntrl_st2, tmp);
> >
> > - /* Wait for 1 vertical blank */
> > - intel_wait_for_vblank(dev, pipe);
> > + /*
> > +* Wait for 1 vertical blank
> > +*
> > +* FIXME: We call this function at mode_set time, when the pipes are all
> > +* stopped, so it doesn't really make any sense to wait for a vblank
> > +* here: it just delays the modeset in 50ms. I'll leave the code here
> > +* because since the wait doesn't make sense at this point, maybe we
> > +* need a bigger rework. We need an Audio authority to audit this code.
> > +*
> > +* intel_wait_for_vblank(dev_priv->dev, pipe);
> > +*/
>
> This might be due to the generic recommendation that infoframes and related
> stuff (audio also gets transmitted when infoframes are) should only be changed
> after the vblank completed when the pipe is on.
>
> Imo we should just ditch this (and cc: the audio guys on the patch so they're
> aware).
> -Daniel
>
> >
> > /* Set ELD valid state */
> > tmp = I915_READ(aud_cntrl_st2);
> > --
> > 1.8.3.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [VPG HSW-A] drm/i915:Added HDMI Audio codec disable sequence for HSW.
Hi Daniel and Mukesh,
This patch is submitted on behalf of Arora Mukesh. Please have a review.
And the 2nd version of patch is also submitted with subject: [PATCH v2]
drm/i915/hsw: Add display Audio codec disable sequence for Haswell. Please also
have a review.
http://lists.freedesktop.org/archives/intel-gfx/2013-August/032488.html
In the new patch, I did some modification to this original patch, to apply to
both HDMI and DP port:
- rename the function from hsw_hdmi_audio_disable() to intel_disable_audio()
- let intel_disable_ddi() call this new function for both HDMI and DP port.
- remove redundant device check, since only Haswell and its successors use DDI
port operations in intel_ddi.c
Thanks
Mengdong
> -Original Message-
> From: intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org
> [mailto:intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org] On
> Behalf Of mengdong@intel.com
> Sent: Friday, August 30, 2013 7:48 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Arora, MukeshX
> Subject: [Intel-gfx] [PATCH] [VPG HSW-A] drm/i915:Added HDMI Audio codec
> disable sequence for HSW.
>
> From: Mukesh
>
> The code implements hsw_hdmi_audio_disable func which sets the relevant
> registers for disabling the audio codec in a call to intel_disable_ddi
> func.This
> audio codec disbale sequence is implemented as per the recommendation of
> the Bspec.
>
> Change-Id: If6eefbfe5ef821db547c759caa9ff5dc18980738
> Signed-off-by: Mukesh Arora
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 41
>
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0de236e..2718d9a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1119,6 +1119,43 @@ static void intel_ddi_post_disable(struct
> intel_encoder *intel_encoder)
> I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); }
>
> +/* Sets the registers for audio codec disable sequence as
> +* mentioned in the Haswell Bspec.
> +*/
> +void hsw_hdmi_audio_disable(struct drm_encoder *encoder) {
> + u32 temp;
> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> + struct drm_device *dev = encoder->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int aud_config = HSW_AUD_CFG(intel_crtc->pipe);
> +
> + /* HDMI audio disable sequence for Haswell*/
> + if (intel_crtc->eld_vld) {
> + /* disable timestamps */
> + temp = I915_READ(aud_config);
> + /* write 0 for HDMI */
> + temp &= ~AUD_CONFIG_N_VALUE_INDEX;
> + /* Set N_programming_enable */
> + temp |= AUD_CONFIG_N_PROG_ENABLE;
> + /* Set Upper_N_value and Lower_N_value
> + (bits 27:20, 15:4) to all "0"s */
> + temp &=
> ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE);
> + I915_WRITE(aud_config, temp);
> + /* Disable ELDV and ELD buffer */
> + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> + temp &= ~(AUDIO_ELD_VALID_A << (intel_crtc->pipe * 4));
> + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
> + /* Wait for 2 vertical blanks */
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + /* Disable audio PD. This is optional as per Bspec. */
> + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> + temp &= ~(AUDIO_OUTPUT_ENABLE_A << (intel_crtc->pipe * 4));
> + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
> + }
> +
> +}
> static void intel_enable_ddi(struct intel_encoder *intel_encoder) {
> struct drm_encoder *encoder = &intel_encoder->base; @@ -1173,6
> +1210,10 @@ static void intel_disable_ddi(struct intel_encoder
> *intel_encoder)
> tmp &= ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) <<
>(pipe * 4));
> I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> + if (IS_HASWELL(dev) && (type == INTEL_OUTPUT_HDMI)) {
> + /*HDMI audio codec disable sequence.*/
> + hsw_hdmi_audio_disable(encoder);
> + }
> }
>
> if (type == INTEL_OUTPUT_EDP) {
> --
> 1.8.1.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] How will Gfx driver support runtime PM on Haswell?
Hi Ben, How will Gfx driver support runtime PM on Haswell? Will the Gfx driver trigger "Adapter Power State" notification to BIOS when GPU switch state between D0 and D3? We need to support RTD3 for HD-A legacy audio on Haswell Ultrabook. And this depends on Gfx driver to implement two BIOS notifications. Here is the background of this dependency: The target of HD-Audio RTD3 is that BIOS can power off the on-board 3rd party audio codec to save power in S0. This would help to save ~60mW. Please note that audio driver cannot power off the codec, because the codec power is controlled by some GPIO which is board specific. The BIOS will power off the audio codec when three requirements are met: (1) System is running on battery (2) HD-Audio controller is in D3. Means no active audio application is using the audio devices. (3) All displays are off. This means system is in "User-Absent Mode", and a long latency for devices to resume back to D0 is allowed, such as ~300ms to power on the audio codec. For the 3rd requirement, BIOS needs Gfx driver to tell whether the system is in user-absent mode. It needs Gfx driver to triger two SCI SW notifications: Display Power State Notification and Adapter Power State Notification. 'Display Power State Notification' was implemented by Jani. But we were told that 'Adapter Power State' notification needs Gfx driver to support runtime PM at first. So would you please share some information about runtime PM support in Gfx driver? Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH] drm/i915: Add private api for power well usage -- alignment between graphic team and audio team
> > On 04/29/2013 05:02 PM, Jesse Barnes wrote: > > > On Sat, 27 Apr 2013 13:35:29 +0200 > > > Daniel Vetter wrote: > > > The high level goal here should be for the audio driver to call into > > > i915 with get/put power well around the sequences where it needs the > > > power to be up (reading/writing registers, playing audio), but not > > > across the whole time the driver is loaded, just like you already do > > > with the powersave work functions, e.g. hda_call_codec_suspend. Hi Daniel, We can modify the patch so that the audio driver will not request the power well across the whole time the HD-A driver is loaded, but request/release the power well at runtime. The HD-Audio driver can support runtime PM. When the codec is idle, it can suspend the codec and further suspend the controller, thus the power well is no longer needed. So we can add haswell-specific code to release the power well in HD-A runtime suspend callback azx_suspend(), and request the power well in audio runtime resume callback azx_resume(). Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
