Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Lu, Brent" <brent.lu@intel.com>
To:     Takashi Iwai <tiwai@suse.de>
CC:     "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
        "Jaroslav Kysela" <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
        Kai Vehmanen <kai.vehmanen@linux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        Mohan Kumar <mkumard@nvidia.com>,
        =?iso-8859-1?Q?Ville_Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>,
        "Zhi, Yong" <yong.zhi@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] ALSA: hda/hdmi: run eld notify in delay work
Thread-Topic: [PATCH] ALSA: hda/hdmi: run eld notify in delay work
Thread-Index: AQHY0nlqZat1Y5WzgkyD+5XdJ6OJtq3zUR2AgADBjQA=
Date:   Wed, 28 Sep 2022 02:06:45 +0000
Message-ID: <CY5PR11MB6257CB33E1EDA90CE2B2F99D97549@CY5PR11MB6257.namprd11.prod.outlook.com>
References: <20220927135807.4097052-1-brent.lu@intel.com>
 <87ill8gb5c.wl-tiwai@suse.de>
In-Reply-To: <87ill8gb5c.wl-tiwai@suse.de>
Accept-Language: en-US
Content-Language: en-US
dlp-version: 11.6.500.17
dlp-product: dlpe-windows
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?X+siVkxOtSQUuWF01XRxSxYjKdOC9oFSvzkOLpounx3PH6cTPpkAbLqv9c?=
 =?iso-8859-1?Q?tQF9z5AeGtVKcxxB+fhX1n6M27+4AJw2I6OpuBw7ZfVrXQiJkS/ReoKSXq?=
 =?iso-8859-1?Q?nD8LZ++s7sneXlENjKKnTfAx+wAy7SSSEE/zMrc5Qnz9ydgyuO6tzSt2hs?=
 =?iso-8859-1?Q?QVfBOf5+bdQEQqha1UjxcY82jtS1zUhl9LN48OkcxZnXyQHYg7qMH51Z6h?=
 =?iso-8859-1?Q?ghIsHE+Rr49lubgnEdiHviFIV8ClVm0XqSDI/FVKLrV2PyiWVAwteCiBM1?=
 =?iso-8859-1?Q?wehZk14XL6yfu4oxUPXyuczCyU8JUMnElZ38hlsolov0K/y3lTVeP/Djj+?=
 =?iso-8859-1?Q?ALpuIkLDsTLk0OxdKE4BioqpsK692ilv3uZ4l+DSu9WHuqJHiD5jFXbs24?=
 =?iso-8859-1?Q?zC0l8S7MTb14667FCMLBD3EeJSTRFn4wIiVcFjQuS6+HOktYdwNbKqNhKd?=
 =?iso-8859-1?Q?jBvzxEaayB1Xs3HTDgR89E+yYpDkk/IIIO/yUdk2KuGLH2uVd1eXAody3v?=
 =?iso-8859-1?Q?wNe6Mp10VL2IVbdWRkPXgSF4v9wnK7oMMBB1hempBXR1TUdvBdiyZeA92P?=
 =?iso-8859-1?Q?pWKZSQbVbwr6K1IAaXrfDw2JMx93q3lZ1k0x+KWRtsTpadNcvOkMucC61F?=
 =?iso-8859-1?Q?zFIVOk9/a+JODujOE/2/1iPEo/DYWZLH6roCucwEPlXAMPCrj51nI4OnJP?=
 =?iso-8859-1?Q?AtO5zicse5MG4XEXSMPH/nenrI/gYTKdnLALwr6yoG9m20xXE0dx9AXTuA?=
 =?iso-8859-1?Q?+sHr2B8FjJWY4hgjofXcHEB0+TARjbHvGTXiNysTCjniB/YcxdyE2tfrcS?=
 =?iso-8859-1?Q?7fuUwA+NnZeI5q9BjFv2IVc61g7AZmXnHng/BtO13GDu5w+XyAkB/MdYf0?=
 =?iso-8859-1?Q?VYD5d89TalhPROgfE7EEDYcqfKZ5g+p9JymL4OZ22r5kGq41sCEuQrm7rj?=
 =?iso-8859-1?Q?KeHNl4qwD1tmWV7W+Cyu6POPcT+gslqQwNZ/uBFsYO7qUq6atn9Pvgqs/C?=
 =?iso-8859-1?Q?gkgCWiJweoSV9fj/2ki51vX2smenLYY/ZnpIjOUDVhzoiVlnJD7U+PnYf/?=
 =?iso-8859-1?Q?M02JzayWu9om2XDc+zR4JwaZ8O6NqfGDpu6VQYJoDNmVieoglmLxJhZpkU?=
 =?iso-8859-1?Q?q11ttr6v7gYo6iSgDn6N/nPIySOGyz05IUOBJRpIGdUZIxdGJKW2Z/dm0u?=
 =?iso-8859-1?Q?uVEkVViq2i5++BWy0PbDT8zq+1zqOZyYhv9bzgwpkPQ83ITi2eN0way210?=
 =?iso-8859-1?Q?RWIChiL/zBVgxMJzPFKBA0nan4PdyRTQnmRldLIEFK7om9qBzu0CcVbhL5?=
 =?iso-8859-1?Q?SLLIO+JoofJMLH4tgZaT8v3or2ReaODV7BxEcA7fLQBk0MfKroC4GneHCZ?=
 =?iso-8859-1?Q?y29wcAoh7MGTWXj/R+C3BnVzakcYJzbtQ7AssFEahH7jGQDoMFwScAavE9?=
 =?iso-8859-1?Q?nBsuvvxDlapma8x/OclLnlAB3Ln0mIUCAxpLmPsUAJckPyvI7RjjCt8iq1?=
 =?iso-8859-1?Q?OQ0tch/uNBe1zvibyw0nqi/d6MId2y4ng+QPl0lqT5/3TeqCyXvXIrLgBH?=
 =?iso-8859-1?Q?AIVdZniOXu5cvt1RiUrUJjK3Q+r/Gh45atuLyIeh00Lu0s7rPG6Iljqtwp?=
 =?iso-8859-1?Q?soDB1Wy8qpcIARAvvVAYhHATaJHfH+L4ou?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CY5PR11MB6257.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 99bef1ca-084c-407a-71e5-08daa0f61866
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Sep 2022 02:06:45.7487
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: AinFqgkLHxqyPNkx5R9m7YxDMmlctNFatJH0jbcK4o4UxSNZ33d6KXnO3zaUhmqR+venwtPNYHn/cA8wlv1PBg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB5828
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> >
> > 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@intel.com>
>=20
> 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).
>=20
>=20
> thanks,
>=20
> 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?=20
2. snd_hdac_i915_set_bclk() is called in intel_pin_eld_notify() function. S=
ince it's
   skipped, we need to call it in the per-pin work. Need to add a flag in h=
dmi_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 =3D
	container_of(to_delayed_work(work), struct hdmi_spec_per_pin, work);
	struct hda_codec *codec =3D per_pin->codec;
	struct hdmi_spec *spec =3D codec->spec;
	struct hda_jack_tbl *jack;

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

	if (per_pin->repoll_count++ > 6)
		per_pin->repoll_count =3D 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 ni=
d,
				      int dev_id)
{
	struct hdmi_spec *spec =3D codec->spec;
	int pin_idx =3D 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