[Intel-gfx] [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()

2014-10-28 Thread Jani Nikula
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 

---

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.dat

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

2014-11-05 Thread Rodrigo Vivi
Reviewed-by: Rodrigo Vivi 

On Tue, Oct 28, 2014 at 7:20 AM, 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 
>
> ---
>
> 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 +1

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

2014-11-10 Thread Daniel Vetter
Hi Ben,

The below patch from Jani also touches nouveau, can you please take a
look at it an ack? The core part + nouveau apply on top of drm-next,
the i915 part needs stuff from my next queue. So I'd prefer if we can
get this in through drm-intel-next.

Hi Dave,

Ack on that from your side?

Cheers, Daniel

On Tue, Oct 28, 2014 at 3:20 PM, 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 
>
> ---
>
> 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 + 

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

2014-11-10 Thread Daniel Vetter
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 

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
> @@ -

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

2014-11-11 Thread Ben Skeggs
On Mon, Nov 10, 2014 at 11:39 PM, Daniel Vetter  wrote:
> Hi Ben,
>
> The below patch from Jani also touches nouveau, can you please take a
> look at it an ack? The core part + nouveau apply on top of drm-next,
> the i915 part needs stuff from my next queue. So I'd prefer if we can
> get this in through drm-intel-next.
>
> Hi Dave,
>
> Ack on that from your side?
>
> Cheers, Daniel
>
> On Tue, Oct 28, 2014 at 3:20 PM, 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 
Acked-by: Ben Skeggs 

>>
>> ---
>>
>> 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 */
>> -