All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH] control_external: Add ability to specify TLV data.
@ 2012-04-15  2:36 Dylan Reid
  2012-04-15 13:45 ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Dylan Reid @ 2012-04-15  2:36 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, Dylan Reid, clemens

Allow external control plugins to provide TLV data.  This allows
user-space pcms to specify dB ranges for controls.

This follows the same model as the ALSA drivers for accessing the
data.  The code is based on that implementation.  The control can
provide static data or a callback.  The data is accessed or modified
in the new snd_ctl_ext_elem_tlv callback.

Rev bump the protocol version to enable checking if an external
control supports TLV.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
---
 include/control_external.h |   17 +++++++++++++++-
 src/control/control_ext.c  |   46 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/include/control_external.h b/include/control_external.h
index 7c066cf..5392ea6 100644
--- a/include/control_external.h
+++ b/include/control_external.h
@@ -60,13 +60,16 @@ typedef struct snd_ctl_ext snd_ctl_ext_t;
 typedef struct snd_ctl_ext_callback snd_ctl_ext_callback_t;
 /** Key to access a control pointer */
 typedef unsigned long snd_ctl_ext_key_t;
+/** Callback to handle TLV commands. */
+typedef int (snd_ctl_ext_tlv_rw_t)(snd_ctl_ext_t *ext, snd_ctl_ext_key_t key, int op_flag, unsigned int numid,
+				   unsigned int *tlv, unsigned int tlv_size);
 
 /*
  * Protocol version
  */
 #define SND_CTL_EXT_VERSION_MAJOR	1	/**< Protocol major version */
 #define SND_CTL_EXT_VERSION_MINOR	0	/**< Protocol minor version */
-#define SND_CTL_EXT_VERSION_TINY	0	/**< Protocol tiny version */
+#define SND_CTL_EXT_VERSION_TINY	1	/**< Protocol tiny version */
 /**
  * external plugin protocol version
  */
@@ -122,6 +125,13 @@ struct snd_ctl_ext {
 	 * control handle filled by #snd_ctl_ext_create()
 	 */
 	snd_ctl_t *handle;
+	/**
+	 * optional TLV data for the control.
+	 */
+	union {
+		snd_ctl_ext_tlv_rw_t *c;
+		const unsigned int *p;
+	} tlv;
 
 	int nonblock;			/**< non-block mode; read-only */
 	int subscribed;			/**< events subscribed; read-only */
@@ -245,7 +255,12 @@ typedef enum snd_ctl_ext_access {
 	SND_CTL_EXT_ACCESS_WRITE = (1<<1),
 	SND_CTL_EXT_ACCESS_READWRITE = (3<<0),
 	SND_CTL_EXT_ACCESS_VOLATILE = (1<<2),
+	SND_CTL_EXT_ACCESS_TLV_READ = (1<<4),
+	SND_CTL_EXT_ACCESS_TLV_WRITE = (1<<5),
+	SND_CTL_EXT_ACCESS_TLV_READWRITE = (3<<4),
+	SND_CTL_EXT_ACCESS_TLV_COMMAND = (1<<6),
 	SND_CTL_EXT_ACCESS_INACTIVE = (1<<8),
+	SND_CTL_EXT_ACCESS_TLV_CALLBACK = (1<<28),
 } snd_ctl_ext_access_t;
 
 /**
diff --git a/src/control/control_ext.c b/src/control/control_ext.c
index e20d4f3..0a0b397 100644
--- a/src/control/control_ext.c
+++ b/src/control/control_ext.c
@@ -324,6 +324,51 @@ static int snd_ctl_ext_elem_unlock(snd_ctl_t *handle ATTRIBUTE_UNUSED,
 	return -ENXIO;
 }
 
+static int snd_ctl_ext_elem_tlv(snd_ctl_t *handle, int op_flag,
+				unsigned int numid,
+				unsigned int *tlv, unsigned int tlv_size)
+{
+	snd_ctl_ext_t *ext = handle->private_data;
+	snd_ctl_ext_key_t key;
+	int type, ret;
+	unsigned int access, count, len;
+	snd_ctl_elem_id_t id;
+
+	/* we don't support TLV on protocol ver 1.0.0 or earlier */
+	if (ext->version < SNDRV_PROTOCOL_VERSION(1, 0, 0))
+		return -ENXIO;
+
+	snd_ctl_elem_id_clear(&id);
+	if (numid > 0) {
+		ext->callback->elem_list(ext, numid - 1, &id);
+		id.numid = numid;
+	} else
+		id.numid = 0;
+	key = ext->callback->find_elem(ext, &id);
+
+	if (key == SND_CTL_EXT_KEY_NOT_FOUND)
+		return -ENOENT;
+	ret = ext->callback->get_attribute(ext, key, &type, &access, &count);
+	if (ret < 0)
+		return ret;
+
+	if ((op_flag == 0 && (access & SND_CTL_EXT_ACCESS_TLV_READ) == 0) ||
+	    (op_flag > 0 && (access & SND_CTL_EXT_ACCESS_TLV_WRITE) == 0) ||
+	    (op_flag < 0 && (access & SND_CTL_EXT_ACCESS_TLV_COMMAND) == 0))
+		return -ENXIO;
+	if (access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
+		return ext->tlv.c(ext, key, op_flag, numid, tlv, tlv_size);
+	} else {
+		if (op_flag)
+			return -ENXIO;
+		len = ext->tlv.p[1] + 2 * sizeof(unsigned int);
+		if (tlv_size < len)
+			return -ENOMEM;
+		memcpy(tlv, ext->tlv.p, len);
+		return 0;
+	}
+}
+
 static int snd_ctl_ext_next_device(snd_ctl_t *handle ATTRIBUTE_UNUSED,
 				   int *device ATTRIBUTE_UNUSED)
 {
@@ -429,6 +474,7 @@ static const snd_ctl_ops_t snd_ctl_ext_ops = {
 	.element_write = snd_ctl_ext_elem_write,
 	.element_lock = snd_ctl_ext_elem_lock,
 	.element_unlock = snd_ctl_ext_elem_unlock,
+	.element_tlv = snd_ctl_ext_elem_tlv,
 	.hwdep_next_device = snd_ctl_ext_next_device,
 	.hwdep_info = snd_ctl_ext_hwdep_info,
 	.pcm_next_device = snd_ctl_ext_next_device,
-- 
1.7.9.rc0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC/PATCH] control_external: Add ability to specify TLV data.
  2012-04-15  2:36 [RFC/PATCH] control_external: Add ability to specify TLV data Dylan Reid
@ 2012-04-15 13:45 ` Takashi Iwai
  2012-04-15 23:59   ` Dylan Reid
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2012-04-15 13:45 UTC (permalink / raw)
  To: Dylan Reid; +Cc: alsa-devel, clemens

At Sat, 14 Apr 2012 19:36:26 -0700,
Dylan Reid wrote:
> 
> Allow external control plugins to provide TLV data.  This allows
> user-space pcms to specify dB ranges for controls.
> 
> This follows the same model as the ALSA drivers for accessing the
> data.  The code is based on that implementation.  The control can
> provide static data or a callback.  The data is accessed or modified
> in the new snd_ctl_ext_elem_tlv callback.
> 
> Rev bump the protocol version to enable checking if an external
> control supports TLV.
> 
> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> ---
>  include/control_external.h |   17 +++++++++++++++-
>  src/control/control_ext.c  |   46 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 1 deletions(-)
> 
> diff --git a/include/control_external.h b/include/control_external.h
> index 7c066cf..5392ea6 100644
> --- a/include/control_external.h
> +++ b/include/control_external.h
> @@ -60,13 +60,16 @@ typedef struct snd_ctl_ext snd_ctl_ext_t;
>  typedef struct snd_ctl_ext_callback snd_ctl_ext_callback_t;
>  /** Key to access a control pointer */
>  typedef unsigned long snd_ctl_ext_key_t;
> +/** Callback to handle TLV commands. */
> +typedef int (snd_ctl_ext_tlv_rw_t)(snd_ctl_ext_t *ext, snd_ctl_ext_key_t key, int op_flag, unsigned int numid,
> +				   unsigned int *tlv, unsigned int tlv_size);
>  
>  /*
>   * Protocol version
>   */
>  #define SND_CTL_EXT_VERSION_MAJOR	1	/**< Protocol major version */
>  #define SND_CTL_EXT_VERSION_MINOR	0	/**< Protocol minor version */
> -#define SND_CTL_EXT_VERSION_TINY	0	/**< Protocol tiny version */
> +#define SND_CTL_EXT_VERSION_TINY	1	/**< Protocol tiny version */
>  /**
>   * external plugin protocol version
>   */
> @@ -122,6 +125,13 @@ struct snd_ctl_ext {
>  	 * control handle filled by #snd_ctl_ext_create()
>  	 */
>  	snd_ctl_t *handle;
> +	/**
> +	 * optional TLV data for the control.
> +	 */
> +	union {
> +		snd_ctl_ext_tlv_rw_t *c;
> +		const unsigned int *p;
> +	} tlv;
>  
>  	int nonblock;			/**< non-block mode; read-only */
>  	int subscribed;			/**< events subscribed; read-only */
> @@ -245,7 +255,12 @@ typedef enum snd_ctl_ext_access {
>  	SND_CTL_EXT_ACCESS_WRITE = (1<<1),
>  	SND_CTL_EXT_ACCESS_READWRITE = (3<<0),
>  	SND_CTL_EXT_ACCESS_VOLATILE = (1<<2),
> +	SND_CTL_EXT_ACCESS_TLV_READ = (1<<4),
> +	SND_CTL_EXT_ACCESS_TLV_WRITE = (1<<5),
> +	SND_CTL_EXT_ACCESS_TLV_READWRITE = (3<<4),
> +	SND_CTL_EXT_ACCESS_TLV_COMMAND = (1<<6),
>  	SND_CTL_EXT_ACCESS_INACTIVE = (1<<8),
> +	SND_CTL_EXT_ACCESS_TLV_CALLBACK = (1<<28),
>  } snd_ctl_ext_access_t;
>  
>  /**
> diff --git a/src/control/control_ext.c b/src/control/control_ext.c
> index e20d4f3..0a0b397 100644
> --- a/src/control/control_ext.c
> +++ b/src/control/control_ext.c
> @@ -324,6 +324,51 @@ static int snd_ctl_ext_elem_unlock(snd_ctl_t *handle ATTRIBUTE_UNUSED,
>  	return -ENXIO;
>  }
>  
> +static int snd_ctl_ext_elem_tlv(snd_ctl_t *handle, int op_flag,
> +				unsigned int numid,
> +				unsigned int *tlv, unsigned int tlv_size)
> +{
> +	snd_ctl_ext_t *ext = handle->private_data;
> +	snd_ctl_ext_key_t key;
> +	int type, ret;
> +	unsigned int access, count, len;
> +	snd_ctl_elem_id_t id;
> +
> +	/* we don't support TLV on protocol ver 1.0.0 or earlier */
> +	if (ext->version < SNDRV_PROTOCOL_VERSION(1, 0, 0))
> +		return -ENXIO;

Shouldn't this be "<=" ?

Other than that, I like the implementation, looks pretty
straightforward.


thanks,

Takashi


> +
> +	snd_ctl_elem_id_clear(&id);
> +	if (numid > 0) {
> +		ext->callback->elem_list(ext, numid - 1, &id);
> +		id.numid = numid;
> +	} else
> +		id.numid = 0;
> +	key = ext->callback->find_elem(ext, &id);
> +
> +	if (key == SND_CTL_EXT_KEY_NOT_FOUND)
> +		return -ENOENT;
> +	ret = ext->callback->get_attribute(ext, key, &type, &access, &count);
> +	if (ret < 0)
> +		return ret;
> +
> +	if ((op_flag == 0 && (access & SND_CTL_EXT_ACCESS_TLV_READ) == 0) ||
> +	    (op_flag > 0 && (access & SND_CTL_EXT_ACCESS_TLV_WRITE) == 0) ||
> +	    (op_flag < 0 && (access & SND_CTL_EXT_ACCESS_TLV_COMMAND) == 0))
> +		return -ENXIO;
> +	if (access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
> +		return ext->tlv.c(ext, key, op_flag, numid, tlv, tlv_size);
> +	} else {
> +		if (op_flag)
> +			return -ENXIO;
> +		len = ext->tlv.p[1] + 2 * sizeof(unsigned int);
> +		if (tlv_size < len)
> +			return -ENOMEM;
> +		memcpy(tlv, ext->tlv.p, len);
> +		return 0;
> +	}
> +}
> +
>  static int snd_ctl_ext_next_device(snd_ctl_t *handle ATTRIBUTE_UNUSED,
>  				   int *device ATTRIBUTE_UNUSED)
>  {
> @@ -429,6 +474,7 @@ static const snd_ctl_ops_t snd_ctl_ext_ops = {
>  	.element_write = snd_ctl_ext_elem_write,
>  	.element_lock = snd_ctl_ext_elem_lock,
>  	.element_unlock = snd_ctl_ext_elem_unlock,
> +	.element_tlv = snd_ctl_ext_elem_tlv,
>  	.hwdep_next_device = snd_ctl_ext_next_device,
>  	.hwdep_info = snd_ctl_ext_hwdep_info,
>  	.pcm_next_device = snd_ctl_ext_next_device,
> -- 
> 1.7.9.rc0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC/PATCH] control_external: Add ability to specify TLV data.
  2012-04-15 13:45 ` Takashi Iwai
@ 2012-04-15 23:59   ` Dylan Reid
  0 siblings, 0 replies; 3+ messages in thread
From: Dylan Reid @ 2012-04-15 23:59 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Sun, Apr 15, 2012 at 6:45 AM, Takashi Iwai <tiwai@suse.de> wrote:
> At Sat, 14 Apr 2012 19:36:26 -0700,
> Dylan Reid wrote:
>>
>> Allow external control plugins to provide TLV data.  This allows
>> user-space pcms to specify dB ranges for controls.
>>
>> This follows the same model as the ALSA drivers for accessing the
>> data.  The code is based on that implementation.  The control can
>> provide static data or a callback.  The data is accessed or modified
>> in the new snd_ctl_ext_elem_tlv callback.
>>
>> Rev bump the protocol version to enable checking if an external
>> control supports TLV.
>>
>> Signed-off-by: Dylan Reid <dgreid@chromium.org>
>> ---
>>  include/control_external.h |   17 +++++++++++++++-
>>  src/control/control_ext.c  |   46 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 62 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/control_external.h b/include/control_external.h
>> index 7c066cf..5392ea6 100644
>> --- a/include/control_external.h
>> +++ b/include/control_external.h
>> @@ -60,13 +60,16 @@ typedef struct snd_ctl_ext snd_ctl_ext_t;
>>  typedef struct snd_ctl_ext_callback snd_ctl_ext_callback_t;
>>  /** Key to access a control pointer */
>>  typedef unsigned long snd_ctl_ext_key_t;
>> +/** Callback to handle TLV commands. */
>> +typedef int (snd_ctl_ext_tlv_rw_t)(snd_ctl_ext_t *ext, snd_ctl_ext_key_t key, int op_flag, unsigned int numid,
>> +                                unsigned int *tlv, unsigned int tlv_size);
>>
>>  /*
>>   * Protocol version
>>   */
>>  #define SND_CTL_EXT_VERSION_MAJOR    1       /**< Protocol major version */
>>  #define SND_CTL_EXT_VERSION_MINOR    0       /**< Protocol minor version */
>> -#define SND_CTL_EXT_VERSION_TINY     0       /**< Protocol tiny version */
>> +#define SND_CTL_EXT_VERSION_TINY     1       /**< Protocol tiny version */
>>  /**
>>   * external plugin protocol version
>>   */
>> @@ -122,6 +125,13 @@ struct snd_ctl_ext {
>>        * control handle filled by #snd_ctl_ext_create()
>>        */
>>       snd_ctl_t *handle;
>> +     /**
>> +      * optional TLV data for the control.
>> +      */
>> +     union {
>> +             snd_ctl_ext_tlv_rw_t *c;
>> +             const unsigned int *p;
>> +     } tlv;
>>
>>       int nonblock;                   /**< non-block mode; read-only */
>>       int subscribed;                 /**< events subscribed; read-only */
>> @@ -245,7 +255,12 @@ typedef enum snd_ctl_ext_access {
>>       SND_CTL_EXT_ACCESS_WRITE = (1<<1),
>>       SND_CTL_EXT_ACCESS_READWRITE = (3<<0),
>>       SND_CTL_EXT_ACCESS_VOLATILE = (1<<2),
>> +     SND_CTL_EXT_ACCESS_TLV_READ = (1<<4),
>> +     SND_CTL_EXT_ACCESS_TLV_WRITE = (1<<5),
>> +     SND_CTL_EXT_ACCESS_TLV_READWRITE = (3<<4),
>> +     SND_CTL_EXT_ACCESS_TLV_COMMAND = (1<<6),
>>       SND_CTL_EXT_ACCESS_INACTIVE = (1<<8),
>> +     SND_CTL_EXT_ACCESS_TLV_CALLBACK = (1<<28),
>>  } snd_ctl_ext_access_t;
>>
>>  /**
>> diff --git a/src/control/control_ext.c b/src/control/control_ext.c
>> index e20d4f3..0a0b397 100644
>> --- a/src/control/control_ext.c
>> +++ b/src/control/control_ext.c
>> @@ -324,6 +324,51 @@ static int snd_ctl_ext_elem_unlock(snd_ctl_t *handle ATTRIBUTE_UNUSED,
>>       return -ENXIO;
>>  }
>>
>> +static int snd_ctl_ext_elem_tlv(snd_ctl_t *handle, int op_flag,
>> +                             unsigned int numid,
>> +                             unsigned int *tlv, unsigned int tlv_size)
>> +{
>> +     snd_ctl_ext_t *ext = handle->private_data;
>> +     snd_ctl_ext_key_t key;
>> +     int type, ret;
>> +     unsigned int access, count, len;
>> +     snd_ctl_elem_id_t id;
>> +
>> +     /* we don't support TLV on protocol ver 1.0.0 or earlier */
>> +     if (ext->version < SNDRV_PROTOCOL_VERSION(1, 0, 0))
>> +             return -ENXIO;
>
> Shouldn't this be "<=" ?
It certainly should.
>
> Other than that, I like the implementation, looks pretty
> straightforward.

Thanks for taking the time to look at it, fixed version on the way.

-dg
>
>
> thanks,
>
> Takashi
>
>
>> +
>> +     snd_ctl_elem_id_clear(&id);
>> +     if (numid > 0) {
>> +             ext->callback->elem_list(ext, numid - 1, &id);
>> +             id.numid = numid;
>> +     } else
>> +             id.numid = 0;
>> +     key = ext->callback->find_elem(ext, &id);
>> +
>> +     if (key == SND_CTL_EXT_KEY_NOT_FOUND)
>> +             return -ENOENT;
>> +     ret = ext->callback->get_attribute(ext, key, &type, &access, &count);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     if ((op_flag == 0 && (access & SND_CTL_EXT_ACCESS_TLV_READ) == 0) ||
>> +         (op_flag > 0 && (access & SND_CTL_EXT_ACCESS_TLV_WRITE) == 0) ||
>> +         (op_flag < 0 && (access & SND_CTL_EXT_ACCESS_TLV_COMMAND) == 0))
>> +             return -ENXIO;
>> +     if (access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
>> +             return ext->tlv.c(ext, key, op_flag, numid, tlv, tlv_size);
>> +     } else {
>> +             if (op_flag)
>> +                     return -ENXIO;
>> +             len = ext->tlv.p[1] + 2 * sizeof(unsigned int);
>> +             if (tlv_size < len)
>> +                     return -ENOMEM;
>> +             memcpy(tlv, ext->tlv.p, len);
>> +             return 0;
>> +     }
>> +}
>> +
>>  static int snd_ctl_ext_next_device(snd_ctl_t *handle ATTRIBUTE_UNUSED,
>>                                  int *device ATTRIBUTE_UNUSED)
>>  {
>> @@ -429,6 +474,7 @@ static const snd_ctl_ops_t snd_ctl_ext_ops = {
>>       .element_write = snd_ctl_ext_elem_write,
>>       .element_lock = snd_ctl_ext_elem_lock,
>>       .element_unlock = snd_ctl_ext_elem_unlock,
>> +     .element_tlv = snd_ctl_ext_elem_tlv,
>>       .hwdep_next_device = snd_ctl_ext_next_device,
>>       .hwdep_info = snd_ctl_ext_hwdep_info,
>>       .pcm_next_device = snd_ctl_ext_next_device,
>> --
>> 1.7.9.rc0
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-04-15 23:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-15  2:36 [RFC/PATCH] control_external: Add ability to specify TLV data Dylan Reid
2012-04-15 13:45 ` Takashi Iwai
2012-04-15 23:59   ` Dylan Reid

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.