Skip to content

Hdmi hotplug detection #935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
plbossart merged 1 commit into from
May 17, 2019
Merged

Hdmi hotplug detection #935

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions sound/soc/intel/boards/skl_hda_dsp_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ int skl_hda_hdmi_jack_init(struct snd_soc_card *card)
if (err)
return err;

err = snd_jack_add_new_kctl(pcm->hdmi_jack.jack,
jack_name, SND_JACK_AVOUT);
if (err)
dev_warn(component->dev, "failed creating Jack kctl\n");

@plbossart plbossart May 15, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you clarify for me how this is released? there is no matching 'release' for the 'new'.

Also should we add those controls once the hdac_hdmi_jack_init() succceds below, otherwise you may expose controls that are not functional.

@libinyang libinyang May 16, 2019 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you clarify for me how this is released? there is no matching 'release' for the 'new'.

Also should we add those controls once the hdac_hdmi_jack_init() succceds below, otherwise you may expose controls that are not functional.

@plbossart ALSA jack has a mechanism that it will release them automatically. Jack is seemed as a snd_device. So when driver is removed, the snd_deivce's dev_free() will be called by alsa core. In Jack's dev_free(), it will free all the resource. So you will see there is no free kctl code in our audio driver. You can refer jack.c: snd_jack_dev_free()

@libinyang libinyang May 16, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also should we add those controls once the hdac_hdmi_jack_init() succceds below, otherwise you may expose controls that are not functional.

the hdac_hdmi_jack_init() function has no relationship with the HDMI Jack kctl. They are totally doing the different things. snd_jack_add_new_kctl() only depends on the "snd_soc_card_jack_new()".

@plbossart plbossart May 17, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@libinyang what do you mean that they are doing different things, they use the same argument and both have a jack in the function name. Can you please elaborate. This may be very clear to you but if you are not sharing the information we don't progress as a team. Thanks

@libinyang libinyang May 17, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart hdac_hdmi_jack_init() main task is creating hdac_hdmi_pcm instance and calling snd_hdac_add_chmap_ctls(). It is creating a normal channel map ctls.

@libinyang libinyang May 17, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A jack kctl is a instance of struct snd_jack_kctl.

@plbossart plbossart May 17, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you are saying the function name hdac_hdmi_jack_init is misleading, should we rename it then?

@libinyang libinyang May 17, 2019 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think the name is not good. However, it does do a little thing related jack init: pcm->jack = jack to assign the jack to hdac_hdmi_pcm.

@plbossart plbossart May 17, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@libinyang your circular explanations leave me scratching my head. The function has nothing to do with jack, except that it "assigns the jack to hdac_hdmi_pcm".
I am going to approve and move on, can't keep doing this.

err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
&pcm->hdmi_jack);
if (err < 0)
Expand Down