On Tue, Dec 01, 2015 at 01:37:37PM +0100, Takashi Iwai wrote:
> On Tue, 01 Dec 2015 18:47:01 +0100,
> Subhransu S. Prusty wrote:
> >
> > This patch uses i915 component framework to register for hotplug
> > notification. And once it identifies valid pin sense and valid eld,
> > reads the eld into the corresponding pin map buffer. For now it
> > directly sends the verbs and reads the eld. Later this will use
> > the i915 framework to populate ELD buffer once available.
>
> This will have the same problem as we had in patch_hdmi.c.
> When a sound driver goes to sleep before i915 and i915 triggers the
> eld_notify for audio disablement, it can be screwed up.
>
> See the commit 8ae743e82f0b86f3b860c27fc2c8f574cf959fd0
> ALSA: hda - Skip ELD notification during system suspend
> in for-linus branch of sound git tree.
Yes. Will add this fix in the driver.
>
>
> Takashi
>
> >
> > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@xxxxxxxxx>
> > Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx>
> > ---
> > sound/soc/codecs/hdac_hdmi.c | 125 ++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 118 insertions(+), 7 deletions(-)
> >
> > diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> > index 0869855..203c99f 100644
> > --- a/sound/soc/codecs/hdac_hdmi.c
> > +++ b/sound/soc/codecs/hdac_hdmi.c
> > @@ -34,6 +34,9 @@
> >
> > #define HDA_MAX_CONNECTIONS 32
> >
> > +#define ELD_MAX_SIZE 256
> > +#define ELD_FIXED_BYTES 20
> > +
> > struct hdac_hdmi_cvt_params {
> > unsigned int channels_min;
> > unsigned int channels_max;
> > @@ -48,11 +51,22 @@ struct hdac_hdmi_cvt {
> > struct hdac_hdmi_cvt_params params;
> > };
> >
> > +struct hdmi_eld {
> > + bool monitor_present;
> > + bool eld_valid;
> > + int eld_size;
> > + char eld_buffer[ELD_MAX_SIZE];
> > +};
> > +
> > struct hdac_hdmi_pin {
> > struct list_head head;
> > hda_nid_t nid;
> > int num_mux_nids;
> > hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
> > + struct hdmi_eld eld;
> > + struct hdac_ext_device *edev;
> > + int repoll_count;
> > + struct delayed_work work;
> > };
> >
> > struct hdac_hdmi_dai_pin_map {
> > @@ -246,7 +260,6 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
> > struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
> > struct hdac_hdmi_priv *hdmi = hdac->private_data;
> > struct hdac_hdmi_dai_pin_map *dai_map;
> > - int val;
> >
> > if (dai->id > 0) {
> > dev_err(&hdac->hdac.dev, "Only one dai supported as of now\n");
> > @@ -255,12 +268,11 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
> >
> > dai_map = &hdmi->dai_map[dai->id];
> >
> > - val = snd_hdac_codec_read(&hdac->hdac, dai_map->pin->nid, 0,
> > - AC_VERB_GET_PIN_SENSE, 0);
> > - dev_info(&hdac->hdac.dev, "Val for AC_VERB_GET_PIN_SENSE: %x\n", val);
> > -
> > - if ((!(val & AC_PINSENSE_PRESENCE)) || (!(val & AC_PINSENSE_ELDV))) {
> > - dev_err(&hdac->hdac.dev, "Monitor presence invalid with val: %x\n", val);
> > + if ((!dai_map->pin->eld.monitor_present) ||
> > + (!dai_map->pin->eld.eld_valid)) {
> > + dev_err(&hdac->hdac.dev, "Failed: montior present? %d eld valid?: %d\n",
> > + dai_map->pin->eld.monitor_present,
> > + dai_map->pin->eld.eld_valid);
> > return -ENODEV;
> > }
> >
> > @@ -432,6 +444,68 @@ static int hdac_hdmi_add_cvt(struct hdac_ext_device *edev, hda_nid_t nid)
> > return hdac_hdmi_query_cvt_params(&edev->hdac, cvt);
> > }
> >
> > +static void hdac_hdmi_present_sense(struct hdac_hdmi_pin *pin, int repoll)
> > +{
> > + struct hdac_ext_device *edev = pin->edev;
> > + int val;
> > +
> > + if (!edev)
> > + return;
> > +
> > + pin->repoll_count = repoll;
> > +
> > + pm_runtime_get_sync(&edev->hdac.dev);
> > + val = snd_hdac_codec_read(&edev->hdac, pin->nid, 0,
> > + AC_VERB_GET_PIN_SENSE, 0);
> > +
> > + dev_dbg(&edev->hdac.dev, "Pin sense val %x for pin: %d\n",
> > + val, pin->nid);
> > + pin->eld.monitor_present = !!(val & AC_PINSENSE_PRESENCE);
> > + pin->eld.eld_valid = !!(val & AC_PINSENSE_ELDV);
> > +
> > + if (!pin->eld.monitor_present || !pin->eld.eld_valid) {
> > + dev_info(&edev->hdac.dev, "%s: disconnect or eld_invalid\n",
> > + __func__);
> > + goto put_hdac_device;
> > + }
> > +
> > + if (pin->eld.monitor_present && pin->eld.eld_valid) {
> > + /* TODO: Use i915 cmpnt framework when available */
> > + if (snd_hdac_get_eld(&edev->hdac, pin->nid,
> > + pin->eld.eld_buffer,
> > + &pin->eld.eld_size) == 0) {
> > + print_hex_dump_bytes("Eld: ", DUMP_PREFIX_OFFSET,
> > + pin->eld.eld_buffer, pin->eld.eld_size);
> > + } else {
> > + dev_err(&edev->hdac.dev, "ELD invalid\n");
> > + pin->eld.monitor_present = false;
> > + pin->eld.eld_valid = false;
> > + }
> > +
> > + }
> > +
> > + /*
> > + * Sometimes the pin_sense may present invalid monitor
> > + * present and eld_valid. If eld data is not valid loop, few
> > + * more times to get correct pin sense and valid eld.
> > + */
> > + if ((!pin->eld.monitor_present || !pin->eld.eld_valid) && repoll)
> > + schedule_delayed_work(&pin->work, msecs_to_jiffies(300));
> > +
> > +put_hdac_device:
> > + pm_runtime_put_sync(&edev->hdac.dev);
> > +}
> > +static void hdac_hdmi_repoll_eld(struct work_struct *work)
> > +{
> > + struct hdac_hdmi_pin *pin =
> > + container_of(to_delayed_work(work), struct hdac_hdmi_pin, work);
> > +
> > + /* picked from legacy */
> > + if (pin->repoll_count++ > 6)
> > + pin->repoll_count = 0;
> > +
> > + hdac_hdmi_present_sense(pin, pin->repoll_count);
> > +}
> > static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
> > {
> > struct hdac_hdmi_priv *hdmi = edev->private_data;
> > @@ -446,6 +520,9 @@ static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
> > list_add_tail(&pin->head, &hdmi->pin_list);
> > hdmi->num_pin++;
> >
> > + pin->edev = edev;
> > + INIT_DELAYED_WORK(&pin->work, hdac_hdmi_repoll_eld);
> > +
> > return 0;
> > }
> >
> > @@ -503,17 +580,50 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
> > return hdac_hdmi_init_dai_map(edev);
> > }
> >
> > +static void hdac_hdmi_eld_notify_cb(void *aptr, int port)
> > +{
> > + struct hdac_ext_device *edev = aptr;
> > + struct hdac_hdmi_priv *hdmi = edev->private_data;
> > + struct hdac_hdmi_pin *pin;
> > + /* Don't know how this mapping is derived */
> > + hda_nid_t pin_nid = port + 0x04;
> > +
> > + dev_dbg(&edev->hdac.dev, "%s: for pin: %d\n", __func__, pin_nid);
> > +
> > + list_for_each_entry(pin, &hdmi->pin_list, head) {
> > + if (pin->nid == pin_nid)
> > + hdac_hdmi_present_sense(pin, 1);
> > + }
> > +}
> > +
> > +static struct i915_audio_component_audio_ops aops = {
> > + .pin_eld_notify = hdac_hdmi_eld_notify_cb,
> > +};
> > +
> > static int hdmi_codec_probe(struct snd_soc_codec *codec)
> > {
> > struct hdac_ext_device *edev = snd_soc_codec_get_drvdata(codec);
> > struct hdac_hdmi_priv *hdmi = edev->private_data;
> > struct snd_soc_dapm_context *dapm =
> > snd_soc_component_get_dapm(&codec->component);
> > + struct hdac_hdmi_pin *pin;
> > + int ret;
> >
> > edev->scodec = codec;
> >
> > create_fill_widget_route_map(dapm, &hdmi->dai_map[0]);
> >
> > + aops.audio_ptr = edev;
> > + ret = snd_hdac_i915_register_notifier(&aops);
> > + if (ret < 0) {
> > + dev_err(&edev->hdac.dev, "notifier register failed: err: %d\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + list_for_each_entry(pin, &hdmi->pin_list, head)
> > + hdac_hdmi_eld_notify_cb(edev, (pin->nid - 0x04));
> > +
> > /* Imp: Store the card pointer in hda_codec */
> > edev->card = dapm->card->snd_card;
> >
> > @@ -644,6 +754,7 @@ static int hdac_hdmi_runtime_resume(struct device *dev)
> > if (!bus)
> > return 0;
> >
> > +
> > err = snd_hdac_display_power(bus, true);
> > if (err < 0) {
> > dev_err(bus->dev, "Cannot turn on display power on i915\n");
> > --
> > 1.9.1
> >
--
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
