Re: [PATCH v5 2/2] drm/i915/gmbus: Enable burst read

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tuesday 29 May 2018 11:35 PM, Ville Syrjälä wrote:
On Fri, May 18, 2018 at 02:54:53PM +0530, Ramalingam C wrote:
Support for Burst read in HW is added for HDCP2.2 compliance
requirement.

This patch enables the burst read for all the gmbus read of more than
511Bytes, on capable platforms.

v2:
   Extra line is removed.
v3:
   Macro is added for detecting the BURST_READ Support [Jani]
   Runtime detection of the need for burst_read [Jani]
   Calculation enhancement.
v4:
   GMBUS0 reg val is passed from caller [ville]
   Removed a extra var [ville]
   Extra brackets are removed [ville]
   Implemented the handling of 512Bytes Burst Read.
v5:
   Burst read max length is fixed at 767Bytes [Ville]

Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h  |  3 ++
  drivers/gpu/drm/i915/i915_reg.h  |  1 +
  drivers/gpu/drm/i915/intel_i2c.c | 62 +++++++++++++++++++++++++++++++++-------
  3 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 028691108125..14293fc1a142 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2552,6 +2552,9 @@ intel_info(const struct drm_i915_private *dev_priv)
   */
  #define HAS_AUX_IRQ(dev_priv)   true
  #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
+#define HAS_GMBUS_BURST_READ(dev_priv) (INTEL_GEN(dev_priv) >= 10 || \
+					IS_GEMINILAKE(dev_priv) || \
+					IS_KABYLAKE(dev_priv))
Note 100% sure about these. The spec say some late stepping SPT has this
already. But I suppose this KBL+ match means just KBP+?

Hmm. Did I ask this already before? Getting some dejavu here.
Ville,

All product sku of KBL will have the WA. And we are enabling this from gen10+ including KBL and GLK for HDCP2.2 requirement.
Thats the reasoning behind this.



  /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
   * rows, which changed the alignment requirements and fence programming.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ebdf7c9d816e..575d9495f3e2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2996,6 +2996,7 @@ enum i915_power_well_id {
  #define   GMBUS_RATE_400KHZ	(2<<8) /* reserved on Pineview */
  #define   GMBUS_RATE_1MHZ	(3<<8) /* reserved on Pineview */
  #define   GMBUS_HOLD_EXT	(1<<7) /* 300ns hold time, rsvd on Pineview */
+#define   GMBUS_BYTE_CNT_OVERRIDE (1<<6)
  #define   GMBUS_PIN_DISABLED	0
  #define   GMBUS_PIN_SSC		1
  #define   GMBUS_PIN_VGADDC	2
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 1c0f6b56b209..9e1142a2f81b 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -371,12 +371,30 @@ unsigned int gmbus_max_xfer_size(struct drm_i915_private *dev_priv)
  static int
  gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
  		      unsigned short addr, u8 *buf, unsigned int len,
-		      u32 gmbus1_index)
+		      u32 gmbus0_reg, u32 gmbus1_index)
  {
+	unsigned int size = len;
+	bool burst_read = len > gmbus_max_xfer_size(dev_priv);
+	bool extra_byte_added = false;
+
+	if (burst_read) {
+
Stray newline.
With this newline removed, I will submit the next version with your reviewed-by.

Thanks
Ram

Otherwise this looks good to me.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>


+		/*
+		 * As per HW Spec, for 512Bytes need to read extra Byte and
+		 * Ignore the extra byte read.
+		 */
+		if (len == 512) {
+			extra_byte_added = true;
+			len++;
+		}
+		size = len % 256 + 256;
+		I915_WRITE_FW(GMBUS0, gmbus0_reg | GMBUS_BYTE_CNT_OVERRIDE);
+	}
+
  	I915_WRITE_FW(GMBUS1,
  		      gmbus1_index |
  		      GMBUS_CYCLE_WAIT |
-		      (len << GMBUS_BYTE_COUNT_SHIFT) |
+		      (size << GMBUS_BYTE_COUNT_SHIFT) |
  		      (addr << GMBUS_SLAVE_ADDR_SHIFT) |
  		      GMBUS_SLAVE_READ | GMBUS_SW_RDY);
  	while (len) {
@@ -389,17 +407,34 @@ gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
val = I915_READ_FW(GMBUS3);
  		do {
+			if (extra_byte_added && len == 1)
+				break;
+
  			*buf++ = val & 0xff;
  			val >>= 8;
  		} while (--len && ++loop < 4);
+
+		if (burst_read && len == size - 4)
+			/* Reset the override bit */
+			I915_WRITE_FW(GMBUS0, gmbus0_reg);
  	}
return 0;
  }
+/*
+ * HW spec says that 512Bytes in Burst read need special treatment.
+ * But it doesn't talk about other multiple of 256Bytes. And couldn't locate
+ * an I2C slave, which supports such a lengthy burst read too for experiments.
+ *
+ * So until things get clarified on HW support, to avoid the burst read length
+ * in fold of 256Bytes except 512, max burst read length is fixed at 767Bytes.
+ */
+#define INTEL_GMBUS_BURST_READ_MAX_LEN		767U
+
  static int
  gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
-		u32 gmbus1_index)
+		u32 gmbus0_reg, u32 gmbus1_index)
  {
  	u8 *buf = msg->buf;
  	unsigned int rx_size = msg->len;
@@ -407,10 +442,13 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
  	int ret;
do {
-		len = min(rx_size, gmbus_max_xfer_size(dev_priv));
+		if (HAS_GMBUS_BURST_READ(dev_priv))
+			len = min(rx_size, INTEL_GMBUS_BURST_READ_MAX_LEN);
+		else
+			len = min(rx_size, gmbus_max_xfer_size(dev_priv));
- ret = gmbus_xfer_read_chunk(dev_priv, msg->addr,
-					    buf, len, gmbus1_index);
+		ret = gmbus_xfer_read_chunk(dev_priv, msg->addr, buf, len,
+					    gmbus0_reg, gmbus1_index);
  		if (ret)
  			return ret;
@@ -498,7 +536,8 @@ gmbus_is_index_xfer(struct i2c_msg *msgs, int i, int num)
  }
static int
-gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs)
+gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs,
+		 u32 gmbus0_reg)
  {
  	u32 gmbus1_index = 0;
  	u32 gmbus5 = 0;
@@ -516,7 +555,8 @@ gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs)
  		I915_WRITE_FW(GMBUS5, gmbus5);
if (msgs[1].flags & I2C_M_RD)
-		ret = gmbus_xfer_read(dev_priv, &msgs[1], gmbus1_index);
+		ret = gmbus_xfer_read(dev_priv, &msgs[1], gmbus0_reg,
+				      gmbus1_index);
  	else
  		ret = gmbus_xfer_write(dev_priv, &msgs[1], gmbus1_index);
@@ -551,10 +591,12 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num,
  	for (; i < num; i += inc) {
  		inc = 1;
  		if (gmbus_is_index_xfer(msgs, i, num)) {
-			ret = gmbus_index_xfer(dev_priv, &msgs[i]);
+			ret = gmbus_index_xfer(dev_priv, &msgs[i],
+					       gmbus0_source | bus->reg0);
  			inc = 2; /* an index transmission is two msgs */
  		} else if (msgs[i].flags & I2C_M_RD) {
-			ret = gmbus_xfer_read(dev_priv, &msgs[i], 0);
+			ret = gmbus_xfer_read(dev_priv, &msgs[i],
+					      gmbus0_source | bus->reg0, 0);
  		} else {
  			ret = gmbus_xfer_write(dev_priv, &msgs[i], 0);
  		}
--
2.7.4

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux