diff mbox

[RFC,V2,1/2] drm: add drm_edid_to_eld helper extracting SADs from EDID

Message ID 1365358712-16607-1-git-send-email-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rafał Miłecki April 7, 2013, 6:18 p.m. UTC
Some devices (ATI/AMD cards) don't support passing ELD struct to the
hardware but just require filling specific registers and then the
hardware/firmware does the rest. In such cases we need to read the info
from SAD blocks and put them in the correct registers.
---
V2: fix commit message
    return number of SADs
    take pointer as an argument
    break after finding AUDIO_BLOCK    
---
 drivers/gpu/drm/drm_edid.c |   58 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |   24 ++++++++++++++++++
 2 files changed, 82 insertions(+)

Comments

Ville Syrjälä April 8, 2013, 11:46 a.m. UTC | #1
On Sun, Apr 07, 2013 at 08:18:31PM +0200, Rafa? Mi?ecki wrote:
> Some devices (ATI/AMD cards) don't support passing ELD struct to the
> hardware but just require filling specific registers and then the
> hardware/firmware does the rest. In such cases we need to read the info
> from SAD blocks and put them in the correct registers.
> ---
> V2: fix commit message
>     return number of SADs
>     take pointer as an argument
>     break after finding AUDIO_BLOCK    
> ---
>  drivers/gpu/drm/drm_edid.c |   58 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |   24 ++++++++++++++++++
>  2 files changed, 82 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e2acfdb..079d97f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2511,6 +2511,64 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>  EXPORT_SYMBOL(drm_edid_to_eld);
>  
>  /**
> + * drm_edid_to_sad - extracts SADs from EDID
> + * @edid: EDID to parse
> + * @sads: pointer that will be set to the extracted SADs
> + *
> + * Looks for CEA EDID block and extracts SADs (Short Audio Descriptors) from it.
> + *
> + * Return number of found SADs or negative number on error.
> + */
> +int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads)
> +{
> +	int count = 0;
> +	int i, start, end, dbl;
> +	u8 *db, *cea;
> +
> +	cea = drm_find_cea_extension(edid);
> +	if (!cea) {
> +		DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
> +		return -ENOENT;
> +	}
> +
> +	if (cea_revision(cea) < 3) {
> +		DRM_DEBUG_KMS("SAD: wrong CEA revision\n");
> +		return -ENOTSUPP;
> +	}
> +
> +	if (cea_db_offsets(cea, &start, &end)) {
> +		DRM_DEBUG_KMS("SAD: invalid data block offsets\n");
> +		return -EPROTO;
> +	}
> +
> +	for_each_cea_db(cea, i, start, end) {
> +		db = &cea[i];
> +		dbl = cea_db_payload_len(db);
> +
> +		if (cea_db_tag(db) == AUDIO_BLOCK) {
> +			int j;
> +
> +			count = dbl / 3; /* SAD is 3B */
> +			*sads = kzalloc(count * sizeof(*sads), GFP_KERNEL);

Still looks a bit wrong.

kcalloc(count, sizeof(**sads), GFP_KERNEL);

Also a minor nit, but the scope of some variables is needlessly large.
db and dbl are only needed inside the outer loop, and count is only
needed inside the 'if (... == AUDIO_BLOCK)' block. But this is not a big
deal, so feel free to ignore me on this point if you're feeling lazy :)

> +			if (!*sads)
> +				return -ENOMEM;
> +			for (j = 0; j < count; j++) {
> +				u8 *sad = &db[1 + j * 3];
> +
> +				(*sads)[j].format = (sad[0] & 0x78) >> 3;
> +				(*sads)[j].channels = sad[0] & 0x7;
> +				(*sads)[j].freq = sad[1] & 0x7F;
> +				(*sads)[j].byte2 = sad[2];
> +			}
> +			break;
> +		}
> +	}
> +
> +	return count;
> +}
> +EXPORT_SYMBOL(drm_edid_to_sad);
> +
> +/**
>   * drm_av_sync_delay - HDMI/DP sink audio-video sync delay in millisecond
>   * @connector: connector associated with the HDMI/DP sink
>   * @mode: the display mode
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 5da1b4a..7082c82 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -244,12 +244,36 @@ struct edid {
>  
>  #define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
>  
> +#define SAD_FORMAT_LPCM			0x01
> +#define SAD_FORMAT_AC3			0x02
> +#define SAD_FORMAT_MPEG1		0x03
> +#define SAD_FORMAT_MP3			0x04
> +#define SAD_FORMAT_MPEG2		0x05
> +#define SAD_FORMAT_AAC			0x06
> +#define SAD_FORMAT_DTS			0x07
> +#define SAD_FORMAT_ATRAC		0x08
> +#define SAD_FORMAT_ONE_BIT_AUDIO	0x09
> +#define SAD_FORMAT_DOLBY_DIGITAL	0x0a
> +#define SAD_FORMAT_DTS_HD		0x0b
> +#define SAD_FORMAT_MAT_MLP		0x0c
> +#define SAD_FORMAT_DST			0x0d
> +#define SAD_FORMAT_WMA_PRO		0x0e

We already have names for these in include/linux/hdmi.h. No need to
duplicate them here.

> +
> +/* Short Audio Descriptor */
> +struct cea_sad {
> +	u8 format;
> +	u8 channels; /* max number of channels - 1 */
> +	u8 freq;
> +	u8 byte2; /* meaning depends on format */
> +};
> +
>  struct drm_encoder;
>  struct drm_connector;
>  struct drm_display_mode;
>  struct hdmi_avi_infoframe;
>  
>  void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
> +int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
>  int drm_av_sync_delay(struct drm_connector *connector,
>  		      struct drm_display_mode *mode);
>  struct drm_connector *drm_select_eld(struct drm_encoder *encoder,
> -- 
> 1.7.10.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Rafał Miłecki April 8, 2013, 11:49 a.m. UTC | #2
2013/4/8 Ville Syrjälä <ville.syrjala@linux.intel.com>:
>> +                     *sads = kzalloc(count * sizeof(*sads), GFP_KERNEL);
>
> Still looks a bit wrong.
>
> kcalloc(count, sizeof(**sads), GFP_KERNEL);
>
> Also a minor nit, but the scope of some variables is needlessly large.
> db and dbl are only needed inside the outer loop, and count is only
> needed inside the 'if (... == AUDIO_BLOCK)' block. But this is not a big
> deal, so feel free to ignore me on this point if you're feeling lazy :)

>> +#define SAD_FORMAT_WMA_PRO           0x0e
>
> We already have names for these in include/linux/hdmi.h. No need to
> duplicate them here.

Thanks for your comments Ville! I appreciate that :)
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e2acfdb..079d97f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2511,6 +2511,64 @@  void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 EXPORT_SYMBOL(drm_edid_to_eld);
 
 /**
+ * drm_edid_to_sad - extracts SADs from EDID
+ * @edid: EDID to parse
+ * @sads: pointer that will be set to the extracted SADs
+ *
+ * Looks for CEA EDID block and extracts SADs (Short Audio Descriptors) from it.
+ *
+ * Return number of found SADs or negative number on error.
+ */
+int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads)
+{
+	int count = 0;
+	int i, start, end, dbl;
+	u8 *db, *cea;
+
+	cea = drm_find_cea_extension(edid);
+	if (!cea) {
+		DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
+		return -ENOENT;
+	}
+
+	if (cea_revision(cea) < 3) {
+		DRM_DEBUG_KMS("SAD: wrong CEA revision\n");
+		return -ENOTSUPP;
+	}
+
+	if (cea_db_offsets(cea, &start, &end)) {
+		DRM_DEBUG_KMS("SAD: invalid data block offsets\n");
+		return -EPROTO;
+	}
+
+	for_each_cea_db(cea, i, start, end) {
+		db = &cea[i];
+		dbl = cea_db_payload_len(db);
+
+		if (cea_db_tag(db) == AUDIO_BLOCK) {
+			int j;
+
+			count = dbl / 3; /* SAD is 3B */
+			*sads = kzalloc(count * sizeof(*sads), GFP_KERNEL);
+			if (!*sads)
+				return -ENOMEM;
+			for (j = 0; j < count; j++) {
+				u8 *sad = &db[1 + j * 3];
+
+				(*sads)[j].format = (sad[0] & 0x78) >> 3;
+				(*sads)[j].channels = sad[0] & 0x7;
+				(*sads)[j].freq = sad[1] & 0x7F;
+				(*sads)[j].byte2 = sad[2];
+			}
+			break;
+		}
+	}
+
+	return count;
+}
+EXPORT_SYMBOL(drm_edid_to_sad);
+
+/**
  * drm_av_sync_delay - HDMI/DP sink audio-video sync delay in millisecond
  * @connector: connector associated with the HDMI/DP sink
  * @mode: the display mode
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 5da1b4a..7082c82 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -244,12 +244,36 @@  struct edid {
 
 #define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
 
+#define SAD_FORMAT_LPCM			0x01
+#define SAD_FORMAT_AC3			0x02
+#define SAD_FORMAT_MPEG1		0x03
+#define SAD_FORMAT_MP3			0x04
+#define SAD_FORMAT_MPEG2		0x05
+#define SAD_FORMAT_AAC			0x06
+#define SAD_FORMAT_DTS			0x07
+#define SAD_FORMAT_ATRAC		0x08
+#define SAD_FORMAT_ONE_BIT_AUDIO	0x09
+#define SAD_FORMAT_DOLBY_DIGITAL	0x0a
+#define SAD_FORMAT_DTS_HD		0x0b
+#define SAD_FORMAT_MAT_MLP		0x0c
+#define SAD_FORMAT_DST			0x0d
+#define SAD_FORMAT_WMA_PRO		0x0e
+
+/* Short Audio Descriptor */
+struct cea_sad {
+	u8 format;
+	u8 channels; /* max number of channels - 1 */
+	u8 freq;
+	u8 byte2; /* meaning depends on format */
+};
+
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
 struct hdmi_avi_infoframe;
 
 void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
+int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
 int drm_av_sync_delay(struct drm_connector *connector,
 		      struct drm_display_mode *mode);
 struct drm_connector *drm_select_eld(struct drm_encoder *encoder,