| Message ID | 6b5f1e2cec0137d5aab089a7e7497972ff5addb1.1651326000.git.mchehab@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Let userspace know when snd-hda-intel needs i915 | expand |
Hi Mauro,
I love your patch! Yet something to improve:
[auto build test ERROR on mcgrof/modules-next]
[also build test ERROR on linus/master v5.18-rc4 next-20220429]
[cannot apply to tiwai-sound/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mauro-Carvalho-Chehab/Let-userspace-know-when-snd-hda-intel-needs-i915/20220430-214332
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next
config: ia64-randconfig-r023-20220428 (https://download.01.org/0day-ci/archive/20220501/202205010035.FAx0YtE1-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/32f6557b5cc77c3cc2fcf6e68f11d989e31c954d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mauro-Carvalho-Chehab/Let-userspace-know-when-snd-hda-intel-needs-i915/20220430-214332
git checkout 32f6557b5cc77c3cc2fcf6e68f11d989e31c954d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash sound/hda/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from arch/ia64/include/asm/pgtable.h:153,
from include/linux/pgtable.h:6,
from arch/ia64/include/asm/uaccess.h:40,
from include/linux/uaccess.h:11,
from arch/ia64/include/asm/sections.h:11,
from include/linux/interrupt.h:21,
from include/linux/pci.h:38,
from sound/hda/hdac_component.c:6:
arch/ia64/include/asm/mmu_context.h: In function 'reload_context':
arch/ia64/include/asm/mmu_context.h:127:48: warning: variable 'old_rr4' set but not used [-Wunused-but-set-variable]
127 | unsigned long rr0, rr1, rr2, rr3, rr4, old_rr4;
| ^~~~~~~
sound/hda/hdac_component.c: In function 'hdac_component_master_bind':
>> sound/hda/hdac_component.c:202:14: error: implicit declaration of function '__try_module_get'; did you mean 'try_module_get'? [-Werror=implicit-function-declaration]
202 | if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) {
| ^~~~~~~~~~~~~~~~
| try_module_get
cc1: some warnings being treated as errors
vim +202 sound/hda/hdac_component.c
3
4 #include <linux/init.h>
5 #include <linux/module.h>
> 6 #include <linux/pci.h>
7 #include <linux/component.h>
8 #include <sound/core.h>
9 #include <sound/hdaudio.h>
10 #include <sound/hda_component.h>
11 #include <sound/hda_register.h>
12
13 static void hdac_acomp_release(struct device *dev, void *res)
14 {
15 }
16
17 static struct drm_audio_component *hdac_get_acomp(struct device *dev)
18 {
19 return devres_find(dev, hdac_acomp_release, NULL, NULL);
20 }
21
22 /**
23 * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
24 * @bus: HDA core bus
25 * @enable: enable or disable the wakeup
26 *
27 * This function is supposed to be used only by a HD-audio controller
28 * driver that needs the interaction with graphics driver.
29 *
30 * This function should be called during the chip reset, also called at
31 * resume for updating STATESTS register read.
32 *
33 * Returns zero for success or a negative error code.
34 */
35 int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
36 {
37 struct drm_audio_component *acomp = bus->audio_component;
38
39 if (!acomp || !acomp->ops)
40 return -ENODEV;
41
42 if (!acomp->ops->codec_wake_override)
43 return 0;
44
45 dev_dbg(bus->dev, "%s codec wakeup\n",
46 enable ? "enable" : "disable");
47
48 acomp->ops->codec_wake_override(acomp->dev, enable);
49
50 return 0;
51 }
52 EXPORT_SYMBOL_GPL(snd_hdac_set_codec_wakeup);
53
54 /**
55 * snd_hdac_display_power - Power up / down the power refcount
56 * @bus: HDA core bus
57 * @idx: HDA codec address, pass HDA_CODEC_IDX_CONTROLLER for controller
58 * @enable: power up or down
59 *
60 * This function is used by either HD-audio controller or codec driver that
61 * needs the interaction with graphics driver.
62 *
63 * This function updates the power status, and calls the get_power() and
64 * put_power() ops accordingly, toggling the codec wakeup, too.
65 */
66 void snd_hdac_display_power(struct hdac_bus *bus, unsigned int idx, bool enable)
67 {
68 struct drm_audio_component *acomp = bus->audio_component;
69
70 dev_dbg(bus->dev, "display power %s\n",
71 enable ? "enable" : "disable");
72
73 mutex_lock(&bus->lock);
74 if (enable)
75 set_bit(idx, &bus->display_power_status);
76 else
77 clear_bit(idx, &bus->display_power_status);
78
79 if (!acomp || !acomp->ops)
80 goto unlock;
81
82 if (bus->display_power_status) {
83 if (!bus->display_power_active) {
84 unsigned long cookie = -1;
85
86 if (acomp->ops->get_power)
87 cookie = acomp->ops->get_power(acomp->dev);
88
89 snd_hdac_set_codec_wakeup(bus, true);
90 snd_hdac_set_codec_wakeup(bus, false);
91 bus->display_power_active = cookie;
92 }
93 } else {
94 if (bus->display_power_active) {
95 unsigned long cookie = bus->display_power_active;
96
97 if (acomp->ops->put_power)
98 acomp->ops->put_power(acomp->dev, cookie);
99
100 bus->display_power_active = 0;
101 }
102 }
103 unlock:
104 mutex_unlock(&bus->lock);
105 }
106 EXPORT_SYMBOL_GPL(snd_hdac_display_power);
107
108 /**
109 * snd_hdac_sync_audio_rate - Set N/CTS based on the sample rate
110 * @codec: HDA codec
111 * @nid: the pin widget NID
112 * @dev_id: device identifier
113 * @rate: the sample rate to set
114 *
115 * This function is supposed to be used only by a HD-audio controller
116 * driver that needs the interaction with graphics driver.
117 *
118 * This function sets N/CTS value based on the given sample rate.
119 * Returns zero for success, or a negative error code.
120 */
121 int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
122 int dev_id, int rate)
123 {
124 struct hdac_bus *bus = codec->bus;
125 struct drm_audio_component *acomp = bus->audio_component;
126 int port, pipe;
127
128 if (!acomp || !acomp->ops || !acomp->ops->sync_audio_rate)
129 return -ENODEV;
130 port = nid;
131 if (acomp->audio_ops && acomp->audio_ops->pin2port) {
132 port = acomp->audio_ops->pin2port(codec, nid);
133 if (port < 0)
134 return -EINVAL;
135 }
136 pipe = dev_id;
137 return acomp->ops->sync_audio_rate(acomp->dev, port, pipe, rate);
138 }
139 EXPORT_SYMBOL_GPL(snd_hdac_sync_audio_rate);
140
141 /**
142 * snd_hdac_acomp_get_eld - Get the audio state and ELD via component
143 * @codec: HDA codec
144 * @nid: the pin widget NID
145 * @dev_id: device identifier
146 * @audio_enabled: the pointer to store the current audio state
147 * @buffer: the buffer pointer to store ELD bytes
148 * @max_bytes: the max bytes to be stored on @buffer
149 *
150 * This function is supposed to be used only by a HD-audio controller
151 * driver that needs the interaction with graphics driver.
152 *
153 * This function queries the current state of the audio on the given
154 * digital port and fetches the ELD bytes onto the given buffer.
155 * It returns the number of bytes for the total ELD data, zero for
156 * invalid ELD, or a negative error code.
157 *
158 * The return size is the total bytes required for the whole ELD bytes,
159 * thus it may be over @max_bytes. If it's over @max_bytes, it implies
160 * that only a part of ELD bytes have been fetched.
161 */
162 int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
163 bool *audio_enabled, char *buffer, int max_bytes)
164 {
165 struct hdac_bus *bus = codec->bus;
166 struct drm_audio_component *acomp = bus->audio_component;
167 int port, pipe;
168
169 if (!acomp || !acomp->ops || !acomp->ops->get_eld)
170 return -ENODEV;
171
172 port = nid;
173 if (acomp->audio_ops && acomp->audio_ops->pin2port) {
174 port = acomp->audio_ops->pin2port(codec, nid);
175 if (port < 0)
176 return -EINVAL;
177 }
178 pipe = dev_id;
179 return acomp->ops->get_eld(acomp->dev, port, pipe, audio_enabled,
180 buffer, max_bytes);
181 }
182 EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
183
184 static int hdac_component_master_bind(struct device *dev)
185 {
186 struct drm_audio_component *acomp = hdac_get_acomp(dev);
187 int ret;
188
189 if (WARN_ON(!acomp))
190 return -EINVAL;
191
192 ret = component_bind_all(dev, acomp);
193 if (ret < 0)
194 return ret;
195
196 if (WARN_ON(!(acomp->dev && acomp->ops))) {
197 ret = -EINVAL;
198 goto out_unbind;
199 }
200
201 /* pin the module to avoid dynamic unbinding, but only if given */
> 202 if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) {
203 ret = -ENODEV;
204 goto out_unbind;
205 }
206
207 if (acomp->audio_ops && acomp->audio_ops->master_bind) {
208 ret = acomp->audio_ops->master_bind(dev, acomp);
209 if (ret < 0)
210 goto module_put;
211 }
212
213 complete_all(&acomp->master_bind_complete);
214 return 0;
215
216 module_put:
217 module_put(acomp->ops->owner);
218 out_unbind:
219 component_unbind_all(dev, acomp);
220 complete_all(&acomp->master_bind_complete);
221
222 return ret;
223 }
224
Hi Mauro,
I love your patch! Yet something to improve:
[auto build test ERROR on mcgrof/modules-next]
[also build test ERROR on linus/master v5.18-rc4 next-20220429]
[cannot apply to tiwai-sound/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mauro-Carvalho-Chehab/Let-userspace-know-when-snd-hda-intel-needs-i915/20220430-214332
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next
config: riscv-buildonly-randconfig-r003-20220428 (https://download.01.org/0day-ci/archive/20220501/202205010257.ZFhZYEG9-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 400775649969b9baf3bc2a510266e7912bb16ae9)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/32f6557b5cc77c3cc2fcf6e68f11d989e31c954d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mauro-Carvalho-Chehab/Let-userspace-know-when-snd-hda-intel-needs-i915/20220430-214332
git checkout 32f6557b5cc77c3cc2fcf6e68f11d989e31c954d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash sound/hda/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> sound/hda/hdac_component.c:202:7: error: call to undeclared function '__try_module_get'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) {
^
sound/hda/hdac_component.c:202:7: note: did you mean 'try_module_get'?
include/linux/module.h:759:20: note: 'try_module_get' declared here
static inline bool try_module_get(struct module *module)
^
1 error generated.
vim +/__try_module_get +202 sound/hda/hdac_component.c
183
184 static int hdac_component_master_bind(struct device *dev)
185 {
186 struct drm_audio_component *acomp = hdac_get_acomp(dev);
187 int ret;
188
189 if (WARN_ON(!acomp))
190 return -EINVAL;
191
192 ret = component_bind_all(dev, acomp);
193 if (ret < 0)
194 return ret;
195
196 if (WARN_ON(!(acomp->dev && acomp->ops))) {
197 ret = -EINVAL;
198 goto out_unbind;
199 }
200
201 /* pin the module to avoid dynamic unbinding, but only if given */
> 202 if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) {
203 ret = -ENODEV;
204 goto out_unbind;
205 }
206
207 if (acomp->audio_ops && acomp->audio_ops->master_bind) {
208 ret = acomp->audio_ops->master_bind(dev, acomp);
209 if (ret < 0)
210 goto module_put;
211 }
212
213 complete_all(&acomp->master_bind_complete);
214 return 0;
215
216 module_put:
217 module_put(acomp->ops->owner);
218 out_unbind:
219 component_unbind_all(dev, acomp);
220 complete_all(&acomp->master_bind_complete);
221
222 return ret;
223 }
224
diff --git a/sound/hda/hdac_component.c b/sound/hda/hdac_component.c index bb37e7e0bd79..30e130457272 100644 --- a/sound/hda/hdac_component.c +++ b/sound/hda/hdac_component.c @@ -199,7 +199,7 @@ static int hdac_component_master_bind(struct device *dev) } /* pin the module to avoid dynamic unbinding, but only if given */ - if (!try_module_get(acomp->ops->owner)) { + if (!__try_module_get(acomp->ops->owner, dev->driver->owner)) { ret = -ENODEV; goto out_unbind; }
On some devices, the hda driver needs to hook into a video driver, in order to be able to properly access the audio hardware and/or the power management function. That's the case of several snd_hda_intel devices that depends on i915 driver. Ensure that a proper reference between the snd-hda driver needing such binding is shown at /proc/modules, in order to allow userspace to know about such binding. Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> --- See [PATCH v3 0/2] at: https://lore.kernel.org/all/cover.1651326000.git.mchehab@kernel.org/ sound/hda/hdac_component.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)