linux-kernel - RE: [PATCH] ALSA: hda/hdmi: run eld notify in delay work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CY5PR11MB6257CB33E1EDA90CE2B2F99D97549@CY5PR11MB6257.namprd11.prod.outlook.com>
Date:   Wed, 28 Sep 2022 02:06:45 +0000
From:   "Lu, Brent" <brent.lu@...el.com>
To:     Takashi Iwai <tiwai@...e.de>
CC:     "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "Jaroslav Kysela" <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Mohan Kumar <mkumard@...dia.com>,
        Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        "Zhi, Yong" <yong.zhi@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] ALSA: hda/hdmi: run eld notify in delay work

> >
> > During resolution change, display driver would disable HDMI audio then
> > enable it in a short time. There is possibility that eld notify for
> > HDMI audio enable is called when previous runtime suspend is still
> > running. In this case, the elf nofity just returns and not updating
> > the status of corresponding HDMI pin/port. Here we move the eld nofity
> > to a delay work so we don't lose it.
> >
> > Signed-off-by: Brent Lu <brent.lu@...el.com>
> 
> We have already a dedicated per-pin work for the delayed ELD check.
> Can we reuse it instead of inventing yet another work?
> More work needs more cares, and better to avoid unless really needed (e.g.
> you forgot cleanup at suspend/removal in this patch).
> 
> 
> thanks,
> 
> Takashi

Hi Takashi,

I've checked the hdmi_repoll_eld() and check_presence_and_report() function to see
if we can reuse the per-pin work. I've some questions about reusing the per-pin work:

1. hdmi_repoll_eld() calls snd_hda_jack_tbl_get_mst() function while
   check_presence_and_report() doesn't. Is it ok? 
2. snd_hdac_i915_set_bclk() is called in intel_pin_eld_notify() function. Since it's
   skipped, we need to call it in the per-pin work. Need to add a flag in hdmi_spec_per_pin
   to indicate this situation.
3. We can schedule the per-pin work in intel_pin_eld_notify() when snd_hdac_is_in_pm()
   returns true but there is no guarantee the runtime suspend will finished when the per-pin
  work is schedule to run.

static void hdmi_repoll_eld(struct work_struct *work)
{
	struct hdmi_spec_per_pin *per_pin =
	container_of(to_delayed_work(work), struct hdmi_spec_per_pin, work);
	struct hda_codec *codec = per_pin->codec;
	struct hdmi_spec *spec = codec->spec;
	struct hda_jack_tbl *jack;

	jack = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid,
					per_pin->dev_id);
	if (jack)
		jack->jack_dirty = 1;

	if (per_pin->repoll_count++ > 6)
		per_pin->repoll_count = 0;

	mutex_lock(&spec->pcm_lock);
	hdmi_present_sense(per_pin, per_pin->repoll_count);
	mutex_unlock(&spec->pcm_lock);
}

static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid,
				      int dev_id)
{
	struct hdmi_spec *spec = codec->spec;
	int pin_idx = pin_id_to_pin_index(codec, nid, dev_id);

	if (pin_idx < 0)
		return;
	mutex_lock(&spec->pcm_lock);
	hdmi_present_sense(get_pin(spec, pin_idx), 1);
	mutex_unlock(&spec->pcm_lock);
}

Regards,
Brent