Re: [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 28, 2014 at 04:20:48PM +0200, Jani Nikula wrote:
> The Baseline_ELD_Len field does not include ELD Header Block size.
> 
> From High Definition Audio Specification, Revision 1.0a:
> 
> 	The header block is a fixed size of 4 bytes. The baseline block
> 	is variable size in multiple of 4 bytes, and its size is defined
> 	in the header block Baseline_ELD_Len field (in number of
> 	DWords).
> 
> Do not include the header size in Baseline_ELD_Len field. Fix all known
> users of eld[2].
> 
> While at it, switch to DIV_ROUND_UP instead of open coding it.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next with a pile of acks and one fixup to make it compile,
thanks for the patch.
-Daniel
> 
> ---
> 
> This is based on an audio rework series which is mid-way being merged to
> i915. I don't think this should be cc: stable worthy, as, AFAICT, we
> don't use the vendor block, and anyone reading SADs respecting SAD_Count
> should stop at the same offset regardless of this patch. So I propose
> this gets eventually merged via i915 without a rush.
> ---
>  drivers/gpu/drm/drm_edid.c             |  7 +++++--
>  drivers/gpu/drm/i915/intel_audio.c     | 16 ++++++++--------
>  drivers/gpu/drm/nouveau/nv50_display.c |  3 ++-
>  3 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3bf999134bcc..45aaa6f5ef36 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3128,9 +3128,12 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>  		}
>  	}
>  	eld[5] |= sad_count << 4;
> -	eld[2] = (20 + mnl + sad_count * 3 + 3) / 4;
>  
> -	DRM_DEBUG_KMS("ELD size %d, SAD count %d\n", (int)eld[2], sad_count);
> +	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);
>  }
>  EXPORT_SYMBOL(drm_edid_to_eld);
>  
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 20af973d7cba..439fa4afa18b 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -107,7 +107,7 @@ static bool intel_eld_uptodate(struct drm_connector *connector,
>  	tmp &= ~bits_elda;
>  	I915_WRITE(reg_elda, tmp);
>  
> -	for (i = 0; i < eld[2]; i++)
> +	for (i = 0; i < drm_eld_size(eld) / 4; i++)
>  		if (I915_READ(reg_edid) != *((uint32_t *)eld + i))
>  			return false;
>  
> @@ -162,7 +162,7 @@ static void g4x_audio_codec_enable(struct drm_connector *connector,
>  	len = (tmp >> 9) & 0x1f;		/* ELD buffer size */
>  	I915_WRITE(G4X_AUD_CNTL_ST, tmp);
>  
> -	len = min_t(int, eld[2], len);
> +	len = min(drm_eld_size(eld) / 4, len);
>  	DRM_DEBUG_DRIVER("ELD size %d\n", len);
>  	for (i = 0; i < len; i++)
>  		I915_WRITE(G4X_HDMIW_HDMIEDID, *((uint32_t *)eld + i));
> @@ -209,7 +209,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  	int len, i;
>  
>  	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> -		      pipe_name(pipe), eld[2]);
> +		      pipe_name(pipe), drm_eld_size(eld));
>  
>  	/* Enable audio presence detect, invalidate ELD */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> @@ -225,8 +225,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  	I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);
>  
>  	/* Up to 84 bytes of hw ELD buffer */
> -	len = min_t(int, eld[2], 21);
> -	for (i = 0; i < len; i++)
> +	len = min(drm_eld_size(eld), 84);
> +	for (i = 0; i < len / 4; i++)
>  		I915_WRITE(HSW_AUD_EDID_DATA(pipe), *((uint32_t *)eld + i));
>  
>  	/* ELD valid */
> @@ -315,7 +315,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  	int aud_cntrl_st2;
>  
>  	DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes ELD\n",
> -		      port_name(port), pipe_name(pipe), eld[2]);
> +		      port_name(port), pipe_name(pipe), drm_eld_size(eld));
>  
>  	/* XXX: vblank wait here */
>  
> @@ -354,8 +354,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  	I915_WRITE(aud_cntl_st, tmp);
>  
>  	/* Up to 84 bytes of hw ELD buffer */
> -	len = min_t(int, eld[2], 21);
> -	for (i = 0; i < len; i++)
> +	len = min(drm_eld_size(eld), 84);
> +	for (i = 0; i < len / 4; i++)
>  		I915_WRITE(hdmiw_hdmiedid, *((uint32_t *)eld + i));
>  
>  	/* ELD valid */
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index ae873d1a8d46..d92c11484bd9 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1680,7 +1680,8 @@ nv50_audio_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode)
>  	drm_edid_to_eld(&nv_connector->base, nv_connector->edid);
>  	memcpy(args.data, nv_connector->base.eld, sizeof(args.data));
>  
> -	nvif_mthd(disp->disp, 0, &args, sizeof(args.base) + args.data[2] * 4);
> +	nvif_mthd(disp->disp, 0, &args,
> +		  sizeof(args.base) + drm_eld_size(args.data);
>  }
>  
>  static void
> -- 
> 2.1.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel