| Message ID | 1554975299-25343-1-git-send-email-libin.yang@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ASoC: intel: add device_link to HDMI audio | expand |
On Thu, 11 Apr 2019 11:34:47 +0200, libin.yang@intel.com wrote: > > From: Libin Yang <libin.yang@intel.com> > > This patchset add the device_link between the machine devices > of intel boards and HDMI audio codec. This can make sure that > display audio power domain is always turned on before operating > on the HDMI audio codecs. > > patch 2 adds the helper functions in a new created header file. > However skl_hda_dsp_generic doesn't use these helper functions > because skl_hda_dsp_generic is a special driver, the add link > and delete link operations are in different source code files. > If it includes the header file, there is compiling warning. Now I took a look at the core implementation, and wonder whether we may drop the device_link_del() call if we create the link with DL_FLAG_AUTOREMOVE_CONSUMER? If that works, you don't have to track the link pointer, so it can be dropped as well; i.e. the only addition would be just the extra call of device_link_add() for each machine driver. thanks, Takashi > > Libin Yang (12): > ASoC: intel: skl_hda_dsp_generic: add device_link to HDMI audio > ASoC: intel: boards: define some general functions for hdac_hdmi > ASoC: intel: bxt_da7219_max98357a: add device_link to HDMI audio > ASoC: intel: bxt_rt298: add device_link to HDMI audio > ASoC: intel: glk_rt5682_max98357a: add device_link to HDMI audio > ASoC: intel: kbl_da7219_max98357a: add device_link to HDMI audio > ASoC: intel: kbl_da7219_max98927: add device_link to HDMI audio > ASoC: intel: kbl_rt5660: add device_link to HDMI audio > ASoC: intel: kbl_rt5663_max98927 add device_link to HDMI audio > ASoC: intel: kbl_rt5663_rt5514_max98927 add device_link to HDMI audio > ASoC: intel: skl_nau88l25_max98357a add device_link to HDMI audio > ASoC: intel: skl_nau88l25_ssm4567 add device_link to HDMI audio > > sound/soc/intel/boards/bxt_da7219_max98357a.c | 14 +++++++ > sound/soc/intel/boards/bxt_rt298.c | 14 ++++++- > sound/soc/intel/boards/glk_rt5682_max98357a.c | 14 +++++++ > sound/soc/intel/boards/hdac_hdmi_common.h | 46 ++++++++++++++++++++++ > sound/soc/intel/boards/kbl_da7219_max98357a.c | 14 +++++++ > sound/soc/intel/boards/kbl_da7219_max98927.c | 14 +++++++ > sound/soc/intel/boards/kbl_rt5660.c | 14 ++++++- > sound/soc/intel/boards/kbl_rt5663_max98927.c | 14 ++++++- > .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 14 ++++++- > sound/soc/intel/boards/skl_hda_dsp_common.c | 22 +++++++++++ > sound/soc/intel/boards/skl_hda_dsp_common.h | 1 + > sound/soc/intel/boards/skl_hda_dsp_generic.c | 12 ++++++ > sound/soc/intel/boards/skl_nau88l25_max98357a.c | 36 ++++++++++++++++- > sound/soc/intel/boards/skl_nau88l25_ssm4567.c | 36 ++++++++++++++++- > 14 files changed, 259 insertions(+), 6 deletions(-) > create mode 100644 sound/soc/intel/boards/hdac_hdmi_common.h > > -- > 2.7.4 >
Hi Takashi, >-----Original Message----- >From: Takashi Iwai [mailto:tiwai@suse.de] >Sent: Thursday, April 11, 2019 6:10 PM >To: Yang, Libin <libin.yang@intel.com> >Cc: alsa-devel@alsa-project.org; broonie@kernel.org; pierre- >louis.bossart@linux.intel.com >Subject: Re: [PATCH 00/12] ASoC: intel: add device_link to HDMI audio > >On Thu, 11 Apr 2019 11:34:47 +0200, >libin.yang@intel.com wrote: >> >> From: Libin Yang <libin.yang@intel.com> >> >> This patchset add the device_link between the machine devices of intel >> boards and HDMI audio codec. This can make sure that display audio >> power domain is always turned on before operating on the HDMI audio >> codecs. >> >> patch 2 adds the helper functions in a new created header file. >> However skl_hda_dsp_generic doesn't use these helper functions because >> skl_hda_dsp_generic is a special driver, the add link and delete link >> operations are in different source code files. >> If it includes the header file, there is compiling warning. > >Now I took a look at the core implementation, and wonder whether we may >drop the device_link_del() call if we create the link with >DL_FLAG_AUTOREMOVE_CONSUMER? If that works, you don't have to track >the link pointer, so it can be dropped as well; i.e. the only addition would be >just the extra call of device_link_add() for each machine driver. In the machine drivers, each dai_link will call device_link_add(). So I use the link pointer to check whether it is already created or not to avoid creating the link several times. Like the below code: + if (!(*link)) + *link = device_link_add(consumer, supplier, DL_FLAG_RPM_ACTIVE); Regards, Libin > > >thanks, > >Takashi > >> >> Libin Yang (12): >> ASoC: intel: skl_hda_dsp_generic: add device_link to HDMI audio >> ASoC: intel: boards: define some general functions for hdac_hdmi >> ASoC: intel: bxt_da7219_max98357a: add device_link to HDMI audio >> ASoC: intel: bxt_rt298: add device_link to HDMI audio >> ASoC: intel: glk_rt5682_max98357a: add device_link to HDMI audio >> ASoC: intel: kbl_da7219_max98357a: add device_link to HDMI audio >> ASoC: intel: kbl_da7219_max98927: add device_link to HDMI audio >> ASoC: intel: kbl_rt5660: add device_link to HDMI audio >> ASoC: intel: kbl_rt5663_max98927 add device_link to HDMI audio >> ASoC: intel: kbl_rt5663_rt5514_max98927 add device_link to HDMI audio >> ASoC: intel: skl_nau88l25_max98357a add device_link to HDMI audio >> ASoC: intel: skl_nau88l25_ssm4567 add device_link to HDMI audio >> >> sound/soc/intel/boards/bxt_da7219_max98357a.c | 14 +++++++ >> sound/soc/intel/boards/bxt_rt298.c | 14 ++++++- >> sound/soc/intel/boards/glk_rt5682_max98357a.c | 14 +++++++ >> sound/soc/intel/boards/hdac_hdmi_common.h | 46 >++++++++++++++++++++++ >> sound/soc/intel/boards/kbl_da7219_max98357a.c | 14 +++++++ >> sound/soc/intel/boards/kbl_da7219_max98927.c | 14 +++++++ >> sound/soc/intel/boards/kbl_rt5660.c | 14 ++++++- >> sound/soc/intel/boards/kbl_rt5663_max98927.c | 14 ++++++- >> .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 14 ++++++- >> sound/soc/intel/boards/skl_hda_dsp_common.c | 22 +++++++++++ >> sound/soc/intel/boards/skl_hda_dsp_common.h | 1 + >> sound/soc/intel/boards/skl_hda_dsp_generic.c | 12 ++++++ >> sound/soc/intel/boards/skl_nau88l25_max98357a.c | 36 >++++++++++++++++- >> sound/soc/intel/boards/skl_nau88l25_ssm4567.c | 36 >++++++++++++++++- >> 14 files changed, 259 insertions(+), 6 deletions(-) create mode >> 100644 sound/soc/intel/boards/hdac_hdmi_common.h >> >> -- >> 2.7.4 >>
On Thu, 11 Apr 2019 14:24:13 +0200, Yang, Libin wrote: > > Hi Takashi, > > >-----Original Message----- > >From: Takashi Iwai [mailto:tiwai@suse.de] > >Sent: Thursday, April 11, 2019 6:10 PM > >To: Yang, Libin <libin.yang@intel.com> > >Cc: alsa-devel@alsa-project.org; broonie@kernel.org; pierre- > >louis.bossart@linux.intel.com > >Subject: Re: [PATCH 00/12] ASoC: intel: add device_link to HDMI audio > > > >On Thu, 11 Apr 2019 11:34:47 +0200, > >libin.yang@intel.com wrote: > >> > >> From: Libin Yang <libin.yang@intel.com> > >> > >> This patchset add the device_link between the machine devices of intel > >> boards and HDMI audio codec. This can make sure that display audio > >> power domain is always turned on before operating on the HDMI audio > >> codecs. > >> > >> patch 2 adds the helper functions in a new created header file. > >> However skl_hda_dsp_generic doesn't use these helper functions because > >> skl_hda_dsp_generic is a special driver, the add link and delete link > >> operations are in different source code files. > >> If it includes the header file, there is compiling warning. > > > >Now I took a look at the core implementation, and wonder whether we may > >drop the device_link_del() call if we create the link with > >DL_FLAG_AUTOREMOVE_CONSUMER? If that works, you don't have to track > >the link pointer, so it can be dropped as well; i.e. the only addition would be > >just the extra call of device_link_add() for each machine driver. > > In the machine drivers, each dai_link will call device_link_add(). So I use > the link pointer to check whether it is already created or not to avoid > creating the link several times. Like the below code: > + if (!(*link)) > + *link = device_link_add(consumer, supplier, DL_FLAG_RPM_ACTIVE); Yes, that's fine. What I meant is the rest part, device_link_del() call and keeping the link pointer. Both look superfluous once when you create a device link with DL_FLAG_AUTOREMOVE_CONSUMER flag, then the device link will be automatically cleaned up at the device removal. thanks, Takashi
From: Libin Yang <libin.yang@intel.com> This patchset add the device_link between the machine devices of intel boards and HDMI audio codec. This can make sure that display audio power domain is always turned on before operating on the HDMI audio codecs. patch 2 adds the helper functions in a new created header file. However skl_hda_dsp_generic doesn't use these helper functions because skl_hda_dsp_generic is a special driver, the add link and delete link operations are in different source code files. If it includes the header file, there is compiling warning. Libin Yang (12): ASoC: intel: skl_hda_dsp_generic: add device_link to HDMI audio ASoC: intel: boards: define some general functions for hdac_hdmi ASoC: intel: bxt_da7219_max98357a: add device_link to HDMI audio ASoC: intel: bxt_rt298: add device_link to HDMI audio ASoC: intel: glk_rt5682_max98357a: add device_link to HDMI audio ASoC: intel: kbl_da7219_max98357a: add device_link to HDMI audio ASoC: intel: kbl_da7219_max98927: add device_link to HDMI audio ASoC: intel: kbl_rt5660: add device_link to HDMI audio ASoC: intel: kbl_rt5663_max98927 add device_link to HDMI audio ASoC: intel: kbl_rt5663_rt5514_max98927 add device_link to HDMI audio ASoC: intel: skl_nau88l25_max98357a add device_link to HDMI audio ASoC: intel: skl_nau88l25_ssm4567 add device_link to HDMI audio sound/soc/intel/boards/bxt_da7219_max98357a.c | 14 +++++++ sound/soc/intel/boards/bxt_rt298.c | 14 ++++++- sound/soc/intel/boards/glk_rt5682_max98357a.c | 14 +++++++ sound/soc/intel/boards/hdac_hdmi_common.h | 46 ++++++++++++++++++++++ sound/soc/intel/boards/kbl_da7219_max98357a.c | 14 +++++++ sound/soc/intel/boards/kbl_da7219_max98927.c | 14 +++++++ sound/soc/intel/boards/kbl_rt5660.c | 14 ++++++- sound/soc/intel/boards/kbl_rt5663_max98927.c | 14 ++++++- .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 14 ++++++- sound/soc/intel/boards/skl_hda_dsp_common.c | 22 +++++++++++ sound/soc/intel/boards/skl_hda_dsp_common.h | 1 + sound/soc/intel/boards/skl_hda_dsp_generic.c | 12 ++++++ sound/soc/intel/boards/skl_nau88l25_max98357a.c | 36 ++++++++++++++++- sound/soc/intel/boards/skl_nau88l25_ssm4567.c | 36 ++++++++++++++++- 14 files changed, 259 insertions(+), 6 deletions(-) create mode 100644 sound/soc/intel/boards/hdac_hdmi_common.h