[RFC][PATCH] drm: add helper extracting SADs from EDID

[Rafał Miłecki]

Thanks for comments!

2013/4/7 Paul Menzel <paulepanter at users.sourceforge.net>:
> Am Sonntag, den 07.04.2013, 12:52 +0200 schrieb Rafał Miłecki:
>> +struct cea_sad *drm_edid_to_sad(struct edid *edid)
>> +{
>> +     struct cea_sad *sads = NULL;
>
> No need to set it NULL, as it gets assigned later on? Looks like in the
> end there is a corner case, where nothing gets assigned in the
> `for_each_cea_db` loop. So the compiler is going to complain.

Right, I can drop that. Curious, my gcc didn't complain.


>> +     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;
>> +     }
>
> Could the above be put in some helper too:
> `is_cea_valid_for_version(cea, version)`?

Not sure if it's worth a helper. But maybe I'll go and just use a single check?
if (cea_revision(cea) < 3 || cea_db_offsets(...))


>> +     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);
>> +                     if (!sads)
>
> Add an error too? »kzallac failed« or something like this?

Allocation failures are aloud enough on the lower level. I was
corrected few times already to don't put warnings when alloc fails. If
you want me to point to the discussions, I'll have to search though.


>> +#define SAD_FORMAT_LPCM                      0x01
>> +#define SAD_FORMAT_AC3                       0x02
>
> At least in my MUA indentation looks different.

It's just a matter of .patch format. Patch file has "+" sign at a
beginning of line and \t can look different when viewing patch. Please
apply a patch and check drm_edid.h then - it'll be OK.

-- 
Rafał