On Sun, Apr 07, 2013 at 12:52:57PM +0200, Rafa? Mi?ecki wrote:
> Some devices (ATI/AMD cards) don't want passing ELD struct to the
> hardware but just require filling specific registers and then
> hardware/firmware does the rest. In such a cases we need to read info
> from SAD blocks and put them in the correct registers.
> ---
>  drivers/gpu/drm/drm_edid.c |   55 
> ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |   24 +++++++++++++++++++
>  2 files changed, 79 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e2acfdb..7c9e799 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2511,6 +2511,61 @@ 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
> + *
> + * Looks for CEA EDID block and extracts SADs (Short Audio Descriptors) from 
> it.

The function returns an array of SADs with the zeroed SAD as a
terminator. That kind of magic needs clear documentation IMHO.

In fact I think it would be better if you returned the number of 
SADs too, instead of relying on the zero termination. With a
clear "number of SADs" return parameter it's less likely people
would misuse this by accident.

> + */
> +struct cea_sad *drm_edid_to_sad(struct edid *edid)
> +{
> +     struct cea_sad *sads = NULL;
> +     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 NULL;
> +     }
> +
> +     if (cea_revision(cea) < 3) {
> +             DRM_DEBUG_KMS("SAD: wrong CEA revision\n");
> +             return NULL;
> +     }
> +
> +     if (cea_db_offsets(cea, &start, &end)) {
> +             DRM_DEBUG_KMS("SAD: invalid data block offsets\n");
> +             return NULL;
> +     }
> +
> +     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 count = dbl / 3; /* SAD is 3B */
> +                     int j;
> +
> +                     sads = kzalloc(count + 1 * sizeof(*sads), GFP_KERNEL);

Missing parens -> miscalculated size. Should really be kcalloc().

> +                     if (!sads)
> +                             return NULL;
> +
> +                     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];
> +                     }

If the EDID has multiple AUDIO_BLOCKs this will leak memory. 'break'
here would avoid that.

> +             }
> +     }
> +
> +     return sads;
> +}
> +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..b36d052 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);
> +struct cea_sad *drm_edid_to_sad(struct edid *edid);
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrj?l?
syrjala at sci.fi
http://www.sci.fi/~syrjala/

Reply via email to