dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/edid: add #defines and helpers for ELD
       [not found] <cover.1414419818.git.jani.nikula@intel.com>
@ 2014-10-28 14:20 ` Jani Nikula
  2014-10-28 14:20   ` [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld() Jani Nikula
  2014-11-04 23:53   ` [PATCH 1/2] drm/edid: add #defines and helpers for ELD Rodrigo Vivi
  0 siblings, 2 replies; 7+ messages in thread
From: Jani Nikula @ 2014-10-28 14:20 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: jani.nikula, Ben Skeggs

In the interest of reducing magic numbers and having to cross check with
the specs all the time.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 include/drm/drm_edid.h | 102 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b96031d947a0..c2f1bfa22010 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -207,6 +207,61 @@ struct detailed_timing {
 #define DRM_EDID_HDMI_DC_30               (1 << 4)
 #define DRM_EDID_HDMI_DC_Y444             (1 << 3)
 
+/* ELD Header Block */
+#define DRM_ELD_HEADER_BLOCK_SIZE	4
+
+#define DRM_ELD_VER			0
+# define DRM_ELD_VER_SHIFT		3
+# define DRM_ELD_VER_MASK		(0x1f << 3)
+
+#define DRM_ELD_BASELINE_ELD_LEN	2	/* in dwords! */
+
+/* ELD Baseline Block for ELD_Ver == 2 */
+#define DRM_ELD_CEA_EDID_VER_MNL	4
+# define DRM_ELD_CEA_EDID_VER_SHIFT	5
+# define DRM_ELD_CEA_EDID_VER_MASK	(7 << 5)
+# define DRM_ELD_CEA_EDID_VER_NONE	(0 << 5)
+# define DRM_ELD_CEA_EDID_VER_CEA861	(1 << 5)
+# define DRM_ELD_CEA_EDID_VER_CEA861A	(2 << 5)
+# define DRM_ELD_CEA_EDID_VER_CEA861BCD	(3 << 5)
+# define DRM_ELD_MNL_SHIFT		0
+# define DRM_ELD_MNL_MASK		(0x1f << 0)
+
+#define DRM_ELD_SAD_COUNT_CONN_TYPE	5
+# define DRM_ELD_SAD_COUNT_SHIFT	4
+# define DRM_ELD_SAD_COUNT_MASK		(0xf << 4)
+# define DRM_ELD_CONN_TYPE_SHIFT	2
+# define DRM_ELD_CONN_TYPE_MASK		(3 << 2)
+# define DRM_ELD_CONN_TYPE_HDMI		(0 << 2)
+# define DRM_ELD_CONN_TYPE_DP		(1 << 2)
+# define DRM_ELD_SUPPORTS_AI		(1 << 1)
+# define DRM_ELD_SUPPORTS_HDCP		(1 << 0)
+
+#define DRM_ELD_AUD_SYNCH_DELAY		6	/* in units of 2 ms */
+# define DRM_ELD_AUD_SYNCH_DELAY_MAX	0xfa	/* 500 ms */
+
+#define DRM_ELD_SPEAKER			7
+# define DRM_ELD_SPEAKER_RLRC		(1 << 6)
+# define DRM_ELD_SPEAKER_FLRC		(1 << 5)
+# define DRM_ELD_SPEAKER_RC		(1 << 4)
+# define DRM_ELD_SPEAKER_RLR		(1 << 3)
+# define DRM_ELD_SPEAKER_FC		(1 << 2)
+# define DRM_ELD_SPEAKER_LFE		(1 << 1)
+# define DRM_ELD_SPEAKER_FLR		(1 << 0)
+
+#define DRM_ELD_PORT_ID			8	/* offsets 8..15 inclusive */
+# define DRM_ELD_PORT_ID_LEN		8
+
+#define DRM_ELD_MANUFACTURER_NAME0	16
+#define DRM_ELD_MANUFACTURER_NAME1	17
+
+#define DRM_ELD_PRODUCT_CODE0		18
+#define DRM_ELD_PRODUCT_CODE1		19
+
+#define DRM_ELD_MONITOR_NAME_STRING	20	/* offsets 20..(20+mnl-1) inclusive */
+
+#define DRM_ELD_CEA_SAD(mnl, sad)	(20 + (mnl) + 3 * (sad))
+
 struct edid {
 	u8 header[8];
 	/* Vendor & product info */
@@ -279,4 +334,51 @@ int
 drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 					    const struct drm_display_mode *mode);
 
+/**
+ * drm_eld_mnl - Get ELD monitor name length in bytes.
+ * @eld: pointer to an eld memory structure with mnl set
+ */
+static inline int drm_eld_mnl(const uint8_t *eld)
+{
+	return (eld[DRM_ELD_CEA_EDID_VER_MNL] & DRM_ELD_MNL_MASK) >> DRM_ELD_MNL_SHIFT;
+}
+
+/**
+ * drm_eld_sad_count - Get ELD SAD count.
+ * @eld: pointer to an eld memory structure with sad_count set
+ */
+static inline int drm_eld_sad_count(const uint8_t *eld)
+{
+	return (eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_SAD_COUNT_MASK) >>
+		DRM_ELD_SAD_COUNT_SHIFT;
+}
+
+/**
+ * drm_eld_calc_baseline_block_size - Calculate baseline block size in bytes
+ * @eld: pointer to an eld memory structure with mnl and sad_count set
+ *
+ * This is a helper for determining the payload size of the baseline block, in
+ * bytes, for e.g. setting the Baseline_ELD_Len field in the ELD header block.
+ */
+static inline int drm_eld_calc_baseline_block_size(const uint8_t *eld)
+{
+	return DRM_ELD_MONITOR_NAME_STRING - DRM_ELD_HEADER_BLOCK_SIZE +
+		drm_eld_mnl(eld) + drm_eld_sad_count(eld) * 3;
+}
+
+/**
+ * drm_eld_size - Get ELD size in bytes
+ * @eld: pointer to a complete eld memory structure
+ *
+ * The returned value does not include the vendor block. It's vendor specific,
+ * and comprises of the remaining bytes in the ELD memory buffer after
+ * drm_eld_size() bytes of header and baseline block.
+ *
+ * The returned value is guaranteed to be a multiple of 4.
+ */
+static inline int drm_eld_size(const uint8_t *eld)
+{
+	return DRM_ELD_HEADER_BLOCK_SIZE + eld[DRM_ELD_BASELINE_ELD_LEN] * 4;
+}
+
 #endif /* __DRM_EDID_H__ */
-- 
2.1.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()
  2014-10-28 14:20 ` [PATCH 1/2] drm/edid: add #defines and helpers for ELD Jani Nikula
@ 2014-10-28 14:20   ` Jani Nikula
  2014-11-05 15:37     ` Rodrigo Vivi
                       ` (2 more replies)
  2014-11-04 23:53   ` [PATCH 1/2] drm/edid: add #defines and helpers for ELD Rodrigo Vivi
  1 sibling, 3 replies; 7+ messages in thread
From: Jani Nikula @ 2014-10-28 14:20 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: David Airlie, jani.nikula, Ben Skeggs

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>

---

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drm/edid: add #defines and helpers for ELD
  2014-10-28 14:20 ` [PATCH 1/2] drm/edid: add #defines and helpers for ELD Jani Nikula
  2014-10-28 14:20   ` [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld() Jani Nikula
@ 2014-11-04 23:53   ` Rodrigo Vivi
  1 sibling, 0 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2014-11-04 23:53 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Ben Skeggs, DRI mailing list

On Tue, Oct 28, 2014 at 7:20 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> In the interest of reducing magic numbers and having to cross check with
> the specs all the time.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  include/drm/drm_edid.h | 102 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index b96031d947a0..c2f1bfa22010 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -207,6 +207,61 @@ struct detailed_timing {
>  #define DRM_EDID_HDMI_DC_30               (1 << 4)
>  #define DRM_EDID_HDMI_DC_Y444             (1 << 3)
>
> +/* ELD Header Block */
> +#define DRM_ELD_HEADER_BLOCK_SIZE      4
> +
> +#define DRM_ELD_VER                    0
> +# define DRM_ELD_VER_SHIFT             3
> +# define DRM_ELD_VER_MASK              (0x1f << 3)
> +
> +#define DRM_ELD_BASELINE_ELD_LEN       2       /* in dwords! */
> +
> +/* ELD Baseline Block for ELD_Ver == 2 */
> +#define DRM_ELD_CEA_EDID_VER_MNL       4
Oh, this VER_MNL name confused me a lot during review.... confusing
with MNN_SHIFT.
But since I don't have a better suggestions nevermind ;)

> +# define DRM_ELD_CEA_EDID_VER_SHIFT    5
> +# define DRM_ELD_CEA_EDID_VER_MASK     (7 << 5)
> +# define DRM_ELD_CEA_EDID_VER_NONE     (0 << 5)
> +# define DRM_ELD_CEA_EDID_VER_CEA861   (1 << 5)
> +# define DRM_ELD_CEA_EDID_VER_CEA861A  (2 << 5)
> +# define DRM_ELD_CEA_EDID_VER_CEA861BCD        (3 << 5)
> +# define DRM_ELD_MNL_SHIFT             0
> +# define DRM_ELD_MNL_MASK              (0x1f << 0)
> +
> +#define DRM_ELD_SAD_COUNT_CONN_TYPE    5
> +# define DRM_ELD_SAD_COUNT_SHIFT       4
> +# define DRM_ELD_SAD_COUNT_MASK                (0xf << 4)
> +# define DRM_ELD_CONN_TYPE_SHIFT       2
> +# define DRM_ELD_CONN_TYPE_MASK                (3 << 2)
> +# define DRM_ELD_CONN_TYPE_HDMI                (0 << 2)
> +# define DRM_ELD_CONN_TYPE_DP          (1 << 2)
> +# define DRM_ELD_SUPPORTS_AI           (1 << 1)
> +# define DRM_ELD_SUPPORTS_HDCP         (1 << 0)
> +
> +#define DRM_ELD_AUD_SYNCH_DELAY                6       /* in units of 2 ms */
> +# define DRM_ELD_AUD_SYNCH_DELAY_MAX   0xfa    /* 500 ms */
> +
> +#define DRM_ELD_SPEAKER                        7
> +# define DRM_ELD_SPEAKER_RLRC          (1 << 6)
> +# define DRM_ELD_SPEAKER_FLRC          (1 << 5)
> +# define DRM_ELD_SPEAKER_RC            (1 << 4)
> +# define DRM_ELD_SPEAKER_RLR           (1 << 3)
> +# define DRM_ELD_SPEAKER_FC            (1 << 2)
> +# define DRM_ELD_SPEAKER_LFE           (1 << 1)
> +# define DRM_ELD_SPEAKER_FLR           (1 << 0)
> +
> +#define DRM_ELD_PORT_ID                        8       /* offsets 8..15 inclusive */
> +# define DRM_ELD_PORT_ID_LEN           8
> +
> +#define DRM_ELD_MANUFACTURER_NAME0     16
> +#define DRM_ELD_MANUFACTURER_NAME1     17
> +
> +#define DRM_ELD_PRODUCT_CODE0          18
> +#define DRM_ELD_PRODUCT_CODE1          19
> +
> +#define DRM_ELD_MONITOR_NAME_STRING    20      /* offsets 20..(20+mnl-1) inclusive */
> +
> +#define DRM_ELD_CEA_SAD(mnl, sad)      (20 + (mnl) + 3 * (sad))
> +
>  struct edid {
>         u8 header[8];
>         /* Vendor & product info */
> @@ -279,4 +334,51 @@ int
>  drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>                                             const struct drm_display_mode *mode);
>
> +/**
> + * drm_eld_mnl - Get ELD monitor name length in bytes.
> + * @eld: pointer to an eld memory structure with mnl set
> + */
> +static inline int drm_eld_mnl(const uint8_t *eld)
> +{
> +       return (eld[DRM_ELD_CEA_EDID_VER_MNL] & DRM_ELD_MNL_MASK) >> DRM_ELD_MNL_SHIFT;
> +}
> +
> +/**
> + * drm_eld_sad_count - Get ELD SAD count.
> + * @eld: pointer to an eld memory structure with sad_count set
> + */
> +static inline int drm_eld_sad_count(const uint8_t *eld)
> +{
> +       return (eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_SAD_COUNT_MASK) >>
> +               DRM_ELD_SAD_COUNT_SHIFT;
> +}
> +
> +/**
> + * drm_eld_calc_baseline_block_size - Calculate baseline block size in bytes
> + * @eld: pointer to an eld memory structure with mnl and sad_count set
> + *
> + * This is a helper for determining the payload size of the baseline block, in
> + * bytes, for e.g. setting the Baseline_ELD_Len field in the ELD header block.
> + */
> +static inline int drm_eld_calc_baseline_block_size(const uint8_t *eld)
> +{
> +       return DRM_ELD_MONITOR_NAME_STRING - DRM_ELD_HEADER_BLOCK_SIZE +
> +               drm_eld_mnl(eld) + drm_eld_sad_count(eld) * 3;
> +}
> +
> +/**
> + * drm_eld_size - Get ELD size in bytes
> + * @eld: pointer to a complete eld memory structure
> + *
> + * The returned value does not include the vendor block. It's vendor specific,
> + * and comprises of the remaining bytes in the ELD memory buffer after
> + * drm_eld_size() bytes of header and baseline block.
> + *
> + * The returned value is guaranteed to be a multiple of 4.
> + */
> +static inline int drm_eld_size(const uint8_t *eld)
> +{
> +       return DRM_ELD_HEADER_BLOCK_SIZE + eld[DRM_ELD_BASELINE_ELD_LEN] * 4;
> +}
> +
>  #endif /* __DRM_EDID_H__ */
> --
> 2.1.1
>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()
  2014-10-28 14:20   ` [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld() Jani Nikula
@ 2014-11-05 15:37     ` Rodrigo Vivi
  2014-11-10 13:39     ` Daniel Vetter
  2014-11-10 23:13     ` Daniel Vetter
  2 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2014-11-05 15:37 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Ben Skeggs, DRI mailing list

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

On Tue, Oct 28, 2014 at 7:20 AM, Jani Nikula <jani.nikula@intel.com> 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>
>
> ---
>
> 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
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()
  2014-10-28 14:20   ` [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld() Jani Nikula
  2014-11-05 15:37     ` Rodrigo Vivi
@ 2014-11-10 13:39     ` Daniel Vetter
       [not found]       ` <CAKMK7uF0EFz5DDTJJS9KJ405vgH=FQS5d3FT3spTYp9XxCr-UQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2014-11-10 23:13     ` Daniel Vetter
  2 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2014-11-10 13:39 UTC (permalink / raw)
  To: Jani Nikula, Ben Skeggs, Nouveau Dev, Dave Airlie; +Cc: intel-gfx, dri-devel

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 <jani.nikula@intel.com> 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>
>
> ---
>
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()
       [not found]       ` <CAKMK7uF0EFz5DDTJJS9KJ405vgH=FQS5d3FT3spTYp9XxCr-UQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-11-10 22:38         ` Ben Skeggs
  0 siblings, 0 replies; 7+ messages in thread
From: Ben Skeggs @ 2014-11-10 22:38 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Jani Nikula, Nouveau Dev, intel-gfx, dri-devel, Dave Airlie,
	Ben Skeggs

On Mon, Nov 10, 2014 at 11:39 PM, Daniel Vetter <daniel@ffwll.ch> 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 <jani.nikula@intel.com> 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>
Acked-by: Ben Skeggs <bskeggs@redhat.com>

>>
>> ---
>>
>> 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
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()
  2014-10-28 14:20   ` [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld() Jani Nikula
  2014-11-05 15:37     ` Rodrigo Vivi
  2014-11-10 13:39     ` Daniel Vetter
@ 2014-11-10 23:13     ` Daniel Vetter
  2 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2014-11-10 23:13 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Ben Skeggs, dri-devel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-11-10 23:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1414419818.git.jani.nikula@intel.com>
2014-10-28 14:20 ` [PATCH 1/2] drm/edid: add #defines and helpers for ELD Jani Nikula
2014-10-28 14:20   ` [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld() Jani Nikula
2014-11-05 15:37     ` Rodrigo Vivi
2014-11-10 13:39     ` Daniel Vetter
     [not found]       ` <CAKMK7uF0EFz5DDTJJS9KJ405vgH=FQS5d3FT3spTYp9XxCr-UQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-10 22:38         ` Ben Skeggs
2014-11-10 23:13     ` Daniel Vetter
2014-11-04 23:53   ` [PATCH 1/2] drm/edid: add #defines and helpers for ELD Rodrigo Vivi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox