[PATCH] drm/edid: Extract SADs properly from multiple audio data blocks

[Daniel Vetter]

On Fri, Mar 11, 2016 at 04:24:35PM +0200, Jani Nikula wrote:
> On Fri, 11 Mar 2016, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > [ text/plain ]
> > On Fri, Mar 11, 2016 at 03:36:48PM +0200, Jani Nikula wrote:
> >> On Wed, 09 Mar 2016, ville.syrjala at linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> >
> >> > SADs may span multiple CEA audio data blocks in the EDID.
> >> >
> >> > CEA-861-E says:
> >> > "The order of the Data Blocks is not constrained. It is also possible
> >> > to have more than one of a specific type of data block if necessary to
> >> > include all of the descriptors needed to describe the sink’s capabilities."
> >> >
> >> > Each audio data block can carry up to 10 SADs, whereas the ELD SAD limit
> >> > is 15 according to HDA 1.0a spec. So we should support at least two data
> >> > blocks. And apparently some devices take a more liberal interpretation
> >> > and stuff only one SAD per data block even when they would fit into one.
> >> >
> >> > So let's try to extract all the SADs we can fit into the ELD even when
> >> > they span multiple data blocks.
> >> >
> >> > While at it, toss in a comment to explain the 13 byte monitor name
> >> > string limit which confused me at first.
> >> >
> >> > Cc: Arturo Pérez <artur999555 at gmail.com>
> >> > Tested-by: Arturo Pérez <artur999555 at gmail.com>
> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94197
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> > ---
> >> >  drivers/gpu/drm/drm_edid.c | 17 +++++++++++------
> >> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> > index fdb1eb014586..414d7f61aa05 100644
> >> > --- a/drivers/gpu/drm/drm_edid.c
> >> > +++ b/drivers/gpu/drm/drm_edid.c
> >> > @@ -3308,7 +3308,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> >> >  	u8 *cea;
> >> >  	u8 *name;
> >> >  	u8 *db;
> >> > -	int sad_count = 0;
> >> > +	int total_sad_count = 0;
> >> >  	int mnl;
> >> >  	int dbl;
> >> >  
> >> > @@ -3322,6 +3322,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> >> >  
> >> >  	name = NULL;
> >> >  	drm_for_each_detailed_block((u8 *)edid, monitor_name, &name);
> >> > +	/* max: 13 bytes EDID, 16 bytes ELD */
> >> >  	for (mnl = 0; name && mnl < 13; mnl++) {
> >> >  		if (name[mnl] == 0x0a)
> >> >  			break;
> >> > @@ -3350,11 +3351,15 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> >> >  			dbl = cea_db_payload_len(db);
> >> >  
> >> >  			switch (cea_db_tag(db)) {
> >> > +				int sad_count;
> >> > +
> >> >  			case AUDIO_BLOCK:
> >> >  				/* Audio Data Block, contains SADs */
> >> > -				sad_count = dbl / 3;
> >> > -				if (dbl >= 1)
> >> > -					memcpy(eld + 20 + mnl, &db[1], dbl);
> >> > +				sad_count = min(dbl / 3, 15 - total_sad_count);
> >> 
> >> What if total_sad_count > 15? At a glance, this seems to self correct by
> >> subtracting the excess every second time... ;)
> >
> > Since sad_count will not exceed 15-total_sad_count, total_sad_count will never
> > exceed 15.
> 
> Right. I guess it could be spelled out for folks like me. :/
> 
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
> 
> If you run out of things to do (that'll be the day), you could follow up
> with replacing some of the magic here for offsets etc. with the
> DRM_ELD_* defines I added in drm_edid.h.

Applied to drm-misc but not yet pushed since I want to roll forward to
latest drm-next as soon as Dave has merged everything.
-Daniel

> 
> BR,
> Jani.
> 
> 
> >
> >> 
> >> BR,
> >> Jani.
> >> 
> >> > +				if (sad_count >= 1)
> >> > +					memcpy(eld + 20 + mnl + total_sad_count * 3,
> >> > +					       &db[1], sad_count * 3);
> >> > +				total_sad_count += sad_count;
> >> >  				break;
> >> >  			case SPEAKER_BLOCK:
> >> >  				/* Speaker Allocation Data Block */
> >> > @@ -3371,13 +3376,13 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> >> >  			}
> >> >  		}
> >> >  	}
> >> > -	eld[5] |= sad_count << 4;
> >> > +	eld[5] |= total_sad_count << 4;
> >> >  
> >> >  	eld[DRM_ELD_BASELINE_ELD_LEN] =
> >> >  		DIV_ROUND_UP(drm_eld_calc_baseline_block_size(eld), 4);
> >> >  
> >> >  	DRM_DEBUG_KMS("ELD size %d, SAD count %d\n",
> >> > -		      drm_eld_size(eld), sad_count);
> >> > +		      drm_eld_size(eld), total_sad_count);
> >> >  }
> >> >  EXPORT_SYMBOL(drm_edid_to_eld);
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch