Looks good to me.
Reviewed-by: Nikhil Mahale <nmahale@xxxxxxxxxx>
On 2/6/20 9:58 PM, Takashi Iwai wrote:
> External email: Use caution opening links or attachments
>
>
> This is a final step of the cleanup series: move the HDMI ELD parser
> call into update_eld() function so that we can unify the calls.
> The ELD validity check is unified in update_eld(), too.
>
> Along with it, the repoll scheduling is moved to update_eld() as well,
> where sync_eld_via_acomp() just passes 0 for skipping it.
>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> ---
> sound/pci/hda/patch_hdmi.c | 110 ++++++++++++++++++++-------------------------
> 1 file changed, 48 insertions(+), 62 deletions(-)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 437177294d78..bb287a916dae 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1466,21 +1466,60 @@ static void hdmi_pcm_reset_pin(struct hdmi_spec *spec,
> per_pin->channels = 0;
> }
>
> +static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + struct hdmi_spec *spec = codec->spec;
> +
> + if (per_pin->pcm_idx >= 0)
> + return spec->pcm_rec[per_pin->pcm_idx].jack;
> + else
> + return NULL;
> +}
> +
> /* update per_pin ELD from the given new ELD;
> * setup info frame and notification accordingly
> + * also notify ELD kctl and report jack status changes
> */
> -static bool update_eld(struct hda_codec *codec,
> +static void update_eld(struct hda_codec *codec,
> struct hdmi_spec_per_pin *per_pin,
> - struct hdmi_eld *eld)
> + struct hdmi_eld *eld,
> + int repoll)
> {
> struct hdmi_eld *pin_eld = &per_pin->sink_eld;
> struct hdmi_spec *spec = codec->spec;
> + struct snd_jack *pcm_jack;
> bool old_eld_valid = pin_eld->eld_valid;
> bool eld_changed;
> int pcm_idx;
>
> + if (eld->eld_valid) {
> + if (eld->eld_size <= 0 ||
> + snd_hdmi_parse_eld(codec, &eld->info, eld->eld_buffer,
> + eld->eld_size) < 0) {
> + eld->eld_valid = false;
> + if (repoll) {
> + schedule_delayed_work(&per_pin->work,
> + msecs_to_jiffies(300));
> + return;
> + }
> + }
> + }
> +
> + if (!eld->eld_valid || eld->eld_size <= 0) {
> + eld->eld_valid = false;
> + eld->eld_size = 0;
> + }
> +
> /* for monitor disconnection, save pcm_idx firstly */
> pcm_idx = per_pin->pcm_idx;
> +
> + /*
> + * pcm_idx >=0 before update_eld() means it is in monitor
> + * disconnected event. Jack must be fetched before update_eld().
> + */
> + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
> +
> if (spec->dyn_pcm_assign) {
> if (eld->eld_valid) {
> hdmi_attach_hda_pcm(spec, per_pin);
> @@ -1495,6 +1534,8 @@ static bool update_eld(struct hda_codec *codec,
> */
> if (pcm_idx == -1)
> pcm_idx = per_pin->pcm_idx;
> + if (!pcm_jack)
> + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
>
> if (eld->eld_valid)
> snd_hdmi_show_eld(codec, &eld->info);
> @@ -1533,36 +1574,8 @@ static bool update_eld(struct hda_codec *codec,
> SNDRV_CTL_EVENT_MASK_VALUE |
> SNDRV_CTL_EVENT_MASK_INFO,
> &get_hdmi_pcm(spec, pcm_idx)->eld_ctl->id);
> - return eld_changed;
> -}
> -
> -static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec,
> - struct hdmi_spec_per_pin *per_pin)
> -{
> - struct hdmi_spec *spec = codec->spec;
>
> - if (per_pin->pcm_idx >= 0)
> - return spec->pcm_rec[per_pin->pcm_idx].jack;
> - else
> - return NULL;
> -}
> -
> -static void do_update_eld(struct hda_codec *codec,
> - struct hdmi_spec_per_pin *per_pin,
> - struct hdmi_eld *eld)
> -{
> - struct snd_jack *pcm_jack;
> - bool changed;
> -
> - /*
> - * pcm_idx >=0 before update_eld() means it is in monitor
> - * disconnected event. Jack must be fetched before update_eld().
> - */
> - pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
> - changed = update_eld(codec, per_pin, eld);
> - if (!pcm_jack)
> - pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
> - if (changed && pcm_jack)
> + if (eld_changed && pcm_jack)
> snd_jack_report(pcm_jack,
> (eld->monitor_present && eld->eld_valid) ?
> SND_JACK_AVOUT : 0);
> @@ -1586,7 +1599,6 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
> * the unsolicited response to avoid custom WARs.
> */
> int present;
> - bool do_repoll = false;
> int ret;
>
> ret = snd_hda_power_up_pm(codec);
> @@ -1610,20 +1622,9 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
> if (spec->ops.pin_get_eld(codec, pin_nid, dev_id,
> eld->eld_buffer, &eld->eld_size) < 0)
> eld->eld_valid = false;
> - else {
> - if (snd_hdmi_parse_eld(codec, &eld->info, eld->eld_buffer,
> - eld->eld_size) < 0)
> - eld->eld_valid = false;
> - }
> - if (!eld->eld_valid && repoll)
> - do_repoll = true;
> }
>
> - if (do_repoll)
> - schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300));
> - else
> - do_update_eld(codec, per_pin, eld);
> -
> + update_eld(codec, per_pin, eld, repoll);
> mutex_unlock(&per_pin->lock);
> out:
> snd_hda_power_down_pm(codec);
> @@ -1635,29 +1636,14 @@ static void sync_eld_via_acomp(struct hda_codec *codec,
> {
> struct hdmi_spec *spec = codec->spec;
> struct hdmi_eld *eld = &spec->temp_eld;
> - int size;
>
> mutex_lock(&per_pin->lock);
> eld->monitor_present = false;
> - size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid,
> + eld->eld_size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid,
> per_pin->dev_id, &eld->monitor_present,
> eld->eld_buffer, ELD_MAX_SIZE);
> - if (size > 0) {
> - size = min(size, ELD_MAX_SIZE);
> - if (snd_hdmi_parse_eld(codec, &eld->info,
> - eld->eld_buffer, size) < 0)
> - size = -EINVAL;
> - }
> -
> - if (size > 0) {
> - eld->eld_valid = true;
> - eld->eld_size = size;
> - } else {
> - eld->eld_valid = false;
> - eld->eld_size = 0;
> - }
> -
> - do_update_eld(codec, per_pin, eld);
> + eld->eld_valid = (eld->eld_size > 0);
> + update_eld(codec, per_pin, eld, 0);
> mutex_unlock(&per_pin->lock);
> }
>
> --
> 2.16.4
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
