Message ID | 20211015080826.34847-1-o-takashi@sakamocchi.jp (mailing list archive) |
---|---|
Headers | show |
Series | ALSA: firewire-motu: add ioctl commands to retrieve information in messages delivered by isoc packet | expand |
On Fri, 15 Oct 2021 10:08:15 +0200, Takashi Sakamoto wrote: > > Hi, > > The purpose of this patchset is to add message parser to ALSA > firewire-motu driver so that userspace applications can read information > in message delivered by isochronous packet as well as PCM frames. > The message includes information about hardware meter and user action > over hardware component such as knob, and MIDI message bytes. > > Models in MOTU FireWire series can be categorized to 4 groups in regard > of message mechanism: > > Group 1. 828 and 896 > * quadlet message to registered destination address > Group 2. 828mk2, 896hd, Traveler, 8 pre, Ultralite, 4 pre, and Audio Express > * quadlet message to registered destination address > * message delivered by isochronous packet > Group 3. 828mk3, 896mk3, Ultralite mk3, Traveler mk3, and Track 16 > * quadlet message to registered destination address > * message delivered by isochronous packet > * block message to registered destination address, including command > Group 4. V3HD/V4HD > * quadlet message to registered destination address > * block message to registered destination address > > The patchset is for message delivered by isochronous packet in group 2 > and 3. In Group 2, the message includes information of hardware meter, > information of user action over hardware component. The model in Group > 2 is called as 'register DSP' in the patchset since parameters of DSP > can be configured by asynchronous transaction for register access. In > Group 3, the message includes information of hardware meter only. The > model in Group 3 is called as 'command DSP' since software and device > communicate with commands transferred by asynchronous transaction in > regard of DSP parameters. Two types of message parser is going to be > added so that the driver caches images for the information. The cache > is available for userspace application by ioctl commands newly introduced. > > I note that no control element is added for the hardware meters and user > actions. It's expected for userspace application to retrieve and parse the > information of image then operate for user-defined control element set. > > Takashi Sakamoto (11): > ALSA: firewire-motu: add message parser to gather meter information in > register DSP model > ALSA: firewire-motu: add message parser for meter information in > command DSP model > ALSA: firewire-motu: add ioctl command to read cached hardware meter > ALSA: firewire-motu: parse messages for mixer source parameters in > register-DSP model > ALSA: firewire-motu: parse messages for mixer output parameters in > register DSP model > ALSA: firewire-motu: parse messages for output parameters in register > DSP model > ALSA: firewire-motu: parse messages for line input parameters in > register DSP model > ALSA: firewire-motu: parse messages for input parameters in register > DSP model > ALSA: firewire-motu: add ioctl command to read cached DSP parameters > ALSA: firewire-motu: queue event for parameter change in register DSP > model > ALSA: firewire-motu: notify event for parameter change in register DSP > model Applied all patches now. Thanks. Takashi
Hi, On Fri, Oct 15, 2021 at 05:54:13PM +0200, Takashi Iwai wrote: > On Fri, 15 Oct 2021 10:08:15 +0200, > Takashi Sakamoto wrote: > > > > Hi, > > > > The purpose of this patchset is to add message parser to ALSA > > firewire-motu driver so that userspace applications can read information > > in message delivered by isochronous packet as well as PCM frames. > > The message includes information about hardware meter and user action > > over hardware component such as knob, and MIDI message bytes. > > > > Models in MOTU FireWire series can be categorized to 4 groups in regard > > of message mechanism: > > > > Group 1. 828 and 896 > > * quadlet message to registered destination address > > Group 2. 828mk2, 896hd, Traveler, 8 pre, Ultralite, 4 pre, and Audio Express > > * quadlet message to registered destination address > > * message delivered by isochronous packet > > Group 3. 828mk3, 896mk3, Ultralite mk3, Traveler mk3, and Track 16 > > * quadlet message to registered destination address > > * message delivered by isochronous packet > > * block message to registered destination address, including command > > Group 4. V3HD/V4HD > > * quadlet message to registered destination address > > * block message to registered destination address > > > > The patchset is for message delivered by isochronous packet in group 2 > > and 3. In Group 2, the message includes information of hardware meter, > > information of user action over hardware component. The model in Group > > 2 is called as 'register DSP' in the patchset since parameters of DSP > > can be configured by asynchronous transaction for register access. In > > Group 3, the message includes information of hardware meter only. The > > model in Group 3 is called as 'command DSP' since software and device > > communicate with commands transferred by asynchronous transaction in > > regard of DSP parameters. Two types of message parser is going to be > > added so that the driver caches images for the information. The cache > > is available for userspace application by ioctl commands newly introduced. > > > > I note that no control element is added for the hardware meters and user > > actions. It's expected for userspace application to retrieve and parse the > > information of image then operate for user-defined control element set. > > > > Takashi Sakamoto (11): > > ALSA: firewire-motu: add message parser to gather meter information in > > register DSP model > > ALSA: firewire-motu: add message parser for meter information in > > command DSP model > > ALSA: firewire-motu: add ioctl command to read cached hardware meter > > ALSA: firewire-motu: parse messages for mixer source parameters in > > register-DSP model > > ALSA: firewire-motu: parse messages for mixer output parameters in > > register DSP model > > ALSA: firewire-motu: parse messages for output parameters in register > > DSP model > > ALSA: firewire-motu: parse messages for line input parameters in > > register DSP model > > ALSA: firewire-motu: parse messages for input parameters in register > > DSP model > > ALSA: firewire-motu: add ioctl command to read cached DSP parameters > > ALSA: firewire-motu: queue event for parameter change in register DSP > > model > > ALSA: firewire-motu: notify event for parameter change in register DSP > > model > > Applied all patches now. Thanks. Thanks for your applying the above patches into your tree. I have some slight concerns about them. I'd like to ask your opinion. The snd_firewire_motu_command_dsp_meter structure includes array of 32 bit storage elements. As I added its documentation, the storage includes IEEE 764 binary32 values between 0.0 and +1.0. In the patchset I use __u32 type since I can find just a few usage of float type in UAPI header. In driver side, no floating point arithmetic is used since the float value is just constructed by gathering messages from target device. In the case, is it adequate to expose the value as float type in UAPI? Additionally, current ALSA control interface have no support for control element with float value, like SNDRV_CTL_ELEM_TYPE_IEEE764_BINARY32. As long as I know, no discussion about it in the list for recent decades. Have you ever seen such discussion in the list? (Here I intensionally ignore that we have several binary expressions for floating point since I'm just interested in the existence of discussion.) Regards Takashi Sakamoto
On Sun, 17 Oct 2021 03:22:31 +0200, Takashi Sakamoto wrote: > > Hi, > > On Fri, Oct 15, 2021 at 05:54:13PM +0200, Takashi Iwai wrote: > > On Fri, 15 Oct 2021 10:08:15 +0200, > > Takashi Sakamoto wrote: > > > > > > Hi, > > > > > > The purpose of this patchset is to add message parser to ALSA > > > firewire-motu driver so that userspace applications can read information > > > in message delivered by isochronous packet as well as PCM frames. > > > The message includes information about hardware meter and user action > > > over hardware component such as knob, and MIDI message bytes. > > > > > > Models in MOTU FireWire series can be categorized to 4 groups in regard > > > of message mechanism: > > > > > > Group 1. 828 and 896 > > > * quadlet message to registered destination address > > > Group 2. 828mk2, 896hd, Traveler, 8 pre, Ultralite, 4 pre, and Audio Express > > > * quadlet message to registered destination address > > > * message delivered by isochronous packet > > > Group 3. 828mk3, 896mk3, Ultralite mk3, Traveler mk3, and Track 16 > > > * quadlet message to registered destination address > > > * message delivered by isochronous packet > > > * block message to registered destination address, including command > > > Group 4. V3HD/V4HD > > > * quadlet message to registered destination address > > > * block message to registered destination address > > > > > > The patchset is for message delivered by isochronous packet in group 2 > > > and 3. In Group 2, the message includes information of hardware meter, > > > information of user action over hardware component. The model in Group > > > 2 is called as 'register DSP' in the patchset since parameters of DSP > > > can be configured by asynchronous transaction for register access. In > > > Group 3, the message includes information of hardware meter only. The > > > model in Group 3 is called as 'command DSP' since software and device > > > communicate with commands transferred by asynchronous transaction in > > > regard of DSP parameters. Two types of message parser is going to be > > > added so that the driver caches images for the information. The cache > > > is available for userspace application by ioctl commands newly introduced. > > > > > > I note that no control element is added for the hardware meters and user > > > actions. It's expected for userspace application to retrieve and parse the > > > information of image then operate for user-defined control element set. > > > > > > Takashi Sakamoto (11): > > > ALSA: firewire-motu: add message parser to gather meter information in > > > register DSP model > > > ALSA: firewire-motu: add message parser for meter information in > > > command DSP model > > > ALSA: firewire-motu: add ioctl command to read cached hardware meter > > > ALSA: firewire-motu: parse messages for mixer source parameters in > > > register-DSP model > > > ALSA: firewire-motu: parse messages for mixer output parameters in > > > register DSP model > > > ALSA: firewire-motu: parse messages for output parameters in register > > > DSP model > > > ALSA: firewire-motu: parse messages for line input parameters in > > > register DSP model > > > ALSA: firewire-motu: parse messages for input parameters in register > > > DSP model > > > ALSA: firewire-motu: add ioctl command to read cached DSP parameters > > > ALSA: firewire-motu: queue event for parameter change in register DSP > > > model > > > ALSA: firewire-motu: notify event for parameter change in register DSP > > > model > > > > Applied all patches now. Thanks. > > Thanks for your applying the above patches into your tree. I have some > slight concerns about them. I'd like to ask your opinion. > > The snd_firewire_motu_command_dsp_meter structure includes array of 32 bit > storage elements. As I added its documentation, the storage includes > IEEE 764 binary32 values between 0.0 and +1.0. In the patchset I use __u32 > type since I can find just a few usage of float type in UAPI header. In > driver side, no floating point arithmetic is used since the float value > is just constructed by gathering messages from target device. In the case, > is it adequate to expose the value as float type in UAPI? > > Additionally, current ALSA control interface have no support for control > element with float value, like SNDRV_CTL_ELEM_TYPE_IEEE764_BINARY32. As > long as I know, no discussion about it in the list for recent decades. > Have you ever seen such discussion in the list? (Here I intensionally > ignore that we have several binary expressions for floating point since > I'm just interested in the existence of discussion.) It's not been proposed, AFAIK. The biggest concern is that, *if* any reference or calculation of the float type is required, what to do. e.g. the kernel has a validation code for each values (min/max check), and how could it be implemented for the float type? thanks, Takashi
On Sun, Oct 17, 2021 at 09:02:30AM +0200, Takashi Iwai wrote: > On Sun, 17 Oct 2021 03:22:31 +0200, > Takashi Sakamoto wrote: > > > > Hi, > > > > On Fri, Oct 15, 2021 at 05:54:13PM +0200, Takashi Iwai wrote: > > > On Fri, 15 Oct 2021 10:08:15 +0200, > > > Takashi Sakamoto wrote: > > > > > > > > Hi, > > > > > > > > The purpose of this patchset is to add message parser to ALSA > > > > firewire-motu driver so that userspace applications can read information > > > > in message delivered by isochronous packet as well as PCM frames. > > > > The message includes information about hardware meter and user action > > > > over hardware component such as knob, and MIDI message bytes. > > > > > > > > Models in MOTU FireWire series can be categorized to 4 groups in regard > > > > of message mechanism: > > > > > > > > Group 1. 828 and 896 > > > > * quadlet message to registered destination address > > > > Group 2. 828mk2, 896hd, Traveler, 8 pre, Ultralite, 4 pre, and Audio Express > > > > * quadlet message to registered destination address > > > > * message delivered by isochronous packet > > > > Group 3. 828mk3, 896mk3, Ultralite mk3, Traveler mk3, and Track 16 > > > > * quadlet message to registered destination address > > > > * message delivered by isochronous packet > > > > * block message to registered destination address, including command > > > > Group 4. V3HD/V4HD > > > > * quadlet message to registered destination address > > > > * block message to registered destination address > > > > > > > > The patchset is for message delivered by isochronous packet in group 2 > > > > and 3. In Group 2, the message includes information of hardware meter, > > > > information of user action over hardware component. The model in Group > > > > 2 is called as 'register DSP' in the patchset since parameters of DSP > > > > can be configured by asynchronous transaction for register access. In > > > > Group 3, the message includes information of hardware meter only. The > > > > model in Group 3 is called as 'command DSP' since software and device > > > > communicate with commands transferred by asynchronous transaction in > > > > regard of DSP parameters. Two types of message parser is going to be > > > > added so that the driver caches images for the information. The cache > > > > is available for userspace application by ioctl commands newly introduced. > > > > > > > > I note that no control element is added for the hardware meters and user > > > > actions. It's expected for userspace application to retrieve and parse the > > > > information of image then operate for user-defined control element set. > > > > > > > > Takashi Sakamoto (11): > > > > ALSA: firewire-motu: add message parser to gather meter information in > > > > register DSP model > > > > ALSA: firewire-motu: add message parser for meter information in > > > > command DSP model > > > > ALSA: firewire-motu: add ioctl command to read cached hardware meter > > > > ALSA: firewire-motu: parse messages for mixer source parameters in > > > > register-DSP model > > > > ALSA: firewire-motu: parse messages for mixer output parameters in > > > > register DSP model > > > > ALSA: firewire-motu: parse messages for output parameters in register > > > > DSP model > > > > ALSA: firewire-motu: parse messages for line input parameters in > > > > register DSP model > > > > ALSA: firewire-motu: parse messages for input parameters in register > > > > DSP model > > > > ALSA: firewire-motu: add ioctl command to read cached DSP parameters > > > > ALSA: firewire-motu: queue event for parameter change in register DSP > > > > model > > > > ALSA: firewire-motu: notify event for parameter change in register DSP > > > > model > > > > > > Applied all patches now. Thanks. > > > > Thanks for your applying the above patches into your tree. I have some > > slight concerns about them. I'd like to ask your opinion. > > > > The snd_firewire_motu_command_dsp_meter structure includes array of 32 bit > > storage elements. As I added its documentation, the storage includes > > IEEE 764 binary32 values between 0.0 and +1.0. In the patchset I use __u32 > > type since I can find just a few usage of float type in UAPI header. In > > driver side, no floating point arithmetic is used since the float value > > is just constructed by gathering messages from target device. In the case, > > is it adequate to expose the value as float type in UAPI? > > > > Additionally, current ALSA control interface have no support for control > > element with float value, like SNDRV_CTL_ELEM_TYPE_IEEE764_BINARY32. As > > long as I know, no discussion about it in the list for recent decades. > > Have you ever seen such discussion in the list? (Here I intensionally > > ignore that we have several binary expressions for floating point since > > I'm just interested in the existence of discussion.) > > It's not been proposed, AFAIK. > > The biggest concern is that, *if* any reference or calculation of the > float type is required, what to do. e.g. the kernel has a validation > code for each values (min/max check), and how could it be implemented > for the float type? Indeed. It's probably unavoidable to min/max check and it brings issue to ALSA control core. It's absolutely out of my scope and thanks for your indication. Would I ask you opinion about my concern about firewire UAPI header? Is it allowed to use float type instead of __u32 type? > thanks, > > Takashi Thanks Takashi Sakamoto
On Sun, 17 Oct 2021 11:25:37 +0200, Takashi Sakamoto wrote: > > Would I ask you opinion about my concern about firewire UAPI header? Is > it allowed to use float type instead of __u32 type? I guess the safest way would be like include/uapi/linux/acct.h. The ifdef KERNEL will be omitted at installation, so user-space would see only float type while the kernel sees only __u32. thanks, Takashi
Hi, On Mon, Oct 18, 2021 at 10:05:32AM +0200, Takashi Iwai wrote: > On Sun, 17 Oct 2021 11:25:37 +0200, > Takashi Sakamoto wrote: > > > > Would I ask you opinion about my concern about firewire UAPI header? Is > > it allowed to use float type instead of __u32 type? > > I guess the safest way would be like include/uapi/linux/acct.h. > The ifdef KERNEL will be omitted at installation, so user-space would > see only float type while the kernel sees only __u32. Yes. I've already found the header and it would be the safest, However, conditional macro sometimes confuses userspace developers... I decide to keep current usage of __u32 as is. Thanks for your opinion! Regards Takashi Sakamoto
On Mon, 18 Oct 2021 14:46:54 +0200, Takashi Sakamoto wrote: > > Hi, > > On Mon, Oct 18, 2021 at 10:05:32AM +0200, Takashi Iwai wrote: > > On Sun, 17 Oct 2021 11:25:37 +0200, > > Takashi Sakamoto wrote: > > > > > > Would I ask you opinion about my concern about firewire UAPI header? Is > > > it allowed to use float type instead of __u32 type? > > > > I guess the safest way would be like include/uapi/linux/acct.h. > > The ifdef KERNEL will be omitted at installation, so user-space would > > see only float type while the kernel sees only __u32. > > Yes. I've already found the header and it would be the safest, However, > conditional macro sometimes confuses userspace developers... I believe that the ifdef will be dropped automagically during installing the files, so that user-space will see only float. You can try to patch and run make headers_install. Takashi
Hi, On Mon, Oct 18, 2021 at 02:57:29PM +0200, Takashi Iwai wrote: > On Mon, 18 Oct 2021 14:46:54 +0200, > Takashi Sakamoto wrote: > > > > Hi, > > > > On Mon, Oct 18, 2021 at 10:05:32AM +0200, Takashi Iwai wrote: > > > On Sun, 17 Oct 2021 11:25:37 +0200, > > > Takashi Sakamoto wrote: > > > > > > > > Would I ask you opinion about my concern about firewire UAPI header? Is > > > > it allowed to use float type instead of __u32 type? > > > > > > I guess the safest way would be like include/uapi/linux/acct.h. > > > The ifdef KERNEL will be omitted at installation, so user-space would > > > see only float type while the kernel sees only __u32. > > > > Yes. I've already found the header and it would be the safest, However, > > conditional macro sometimes confuses userspace developers... > > I believe that the ifdef will be dropped automagically during > installing the files, so that user-space will see only float. > > You can try to patch and run make headers_install. Oh, I thought that the headers are simply copied when installing... Indeed, I can find 'scripts/headers_install.sh' does the work in 'headers' make target invoked by the 'headers_install' target. ``` $ tail -n10 include/uapi/sound/firewire.h */ struct snd_firewire_motu_command_dsp_meter { #ifdef __KERNEL__ __u32 data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; #else float data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; #endif }; #endif /* _UAPI_SOUND_FIREWIRE_H_INCLUDED */ $ make headers $ tail -n5 usr/include/sound/firewire.h */ struct snd_firewire_motu_command_dsp_meter { float data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; }; #endif /* _SOUND_FIREWIRE_H_INCLUDED */ ``` It would be achieved to use different type with the same storage size between kernel and userspace. After writing some userspace test applications, I'll post the patch. Thanks for your advice. > Takashi Regards Takashi Sakamoto