diff mbox

[RFC,v2,2/2] drm/i915/gmbus: Enable burst read

Message ID 1523548876-22449-1-git-send-email-ramalingam.c@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ramalingam C April 12, 2018, 4:01 p.m. UTC
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.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  1 +
 drivers/gpu/drm/i915/intel_i2c.c | 38 ++++++++++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 4 deletions(-)

Comments

Ramalingam C April 13, 2018, 10:17 a.m. UTC | #1
On Thursday 12 April 2018 09:31 PM, 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.
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h  |  1 +
>   drivers/gpu/drm/i915/intel_i2c.c | 38 ++++++++++++++++++++++++++++++++++----
>   2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4f583da0cee9..0ef162ee9ce0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2984,6 +2984,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 7e92c7934657..2ee9cf2effcd 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -364,12 +364,24 @@ gmbus_wait_idle(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 gmbus1_index, bool burst_read)
>   {
> +	unsigned int bytes_af_override, size;
> +
> +	if (burst_read) {
> +		bytes_af_override = len - (((len / 256) - 1) * 256);
Just realized that this could be simplified as (len % 256) + 256; Will 
update the next version.

--Ram
> +		size = bytes_af_override;
> +
> +		I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) |
> +			      GMBUS_BYTE_CNT_OVERRIDE));
> +	} else {
> +		size = len;
> +	}
> +
>   	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) {
> @@ -385,11 +397,23 @@ gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
>   			*buf++ = val & 0xff;
>   			val >>= 8;
>   		} while (--len && ++loop < 4);
> +
> +		if (burst_read && len == (bytes_af_override - 4))
> +			I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) &
> +				      ~GMBUS_BYTE_CNT_OVERRIDE));
>   	}
>   
>   	return 0;
>   }
>   
> +static bool gmbus_burst_read_supported(struct drm_i915_private *dev_priv)
> +{
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> +	    IS_KABYLAKE(dev_priv))
> +		return true;
> +	return false;
> +}
> +
>   static int
>   gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>   		u32 gmbus1_index)
> @@ -398,15 +422,21 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>   	unsigned int rx_size = msg->len;
>   	unsigned int len;
>   	int ret;
> +	bool burst_read = false;
> +
> +	if (rx_size > BXT_GMBUS_BYTE_COUNT_MAX)
> +		burst_read = gmbus_burst_read_supported(dev_priv);
>   
>   	do {
> -		if (INTEL_GEN(dev_priv) >= 9)
> +		if (burst_read)
> +			len = rx_size;
> +		else if (INTEL_GEN(dev_priv) >= 9)
>   			len = min(rx_size, BXT_GMBUS_BYTE_COUNT_MAX);
>   		else
>   			len = min(rx_size, GMBUS_BYTE_COUNT_MAX);
>   
>   		ret = gmbus_xfer_read_chunk(dev_priv, msg->addr,
> -					    buf, len, gmbus1_index);
> +					    buf, len, gmbus1_index, burst_read);
>   		if (ret)
>   			return ret;
>
Jani Nikula April 13, 2018, 12:55 p.m. UTC | #2
On Thu, 12 Apr 2018, Ramalingam C <ramalingam.c@intel.com> 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.
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_i2c.c | 38 ++++++++++++++++++++++++++++++++++----
>  2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4f583da0cee9..0ef162ee9ce0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2984,6 +2984,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 7e92c7934657..2ee9cf2effcd 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -364,12 +364,24 @@ gmbus_wait_idle(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 gmbus1_index, bool burst_read)

I think you could throw out the burst_read parameter. See below.

>  {
> +	unsigned int bytes_af_override, size;
> +
> +	if (burst_read) {
> +		bytes_af_override = len - (((len / 256) - 1) * 256);
> +		size = bytes_af_override;
> +
> +		I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) |
> +			      GMBUS_BYTE_CNT_OVERRIDE));
> +	} else {
> +		size = len;
> +	}
> +
>  	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) {
> @@ -385,11 +397,23 @@ gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
>  			*buf++ = val & 0xff;
>  			val >>= 8;
>  		} while (--len && ++loop < 4);
> +
> +		if (burst_read && len == (bytes_af_override - 4))
> +			I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) &
> +				      ~GMBUS_BYTE_CNT_OVERRIDE));
>  	}
>  
>  	return 0;
>  }
>  
> +static bool gmbus_burst_read_supported(struct drm_i915_private *dev_priv)
> +{
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> +	    IS_KABYLAKE(dev_priv))
> +		return true;
> +	return false;
> +}

Name this HAS_GMBUS_BURST_READ() and put below HAS_GMBUS_IRQ() in
i915_drv.h.

> +
>  static int
>  gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>  		u32 gmbus1_index)
> @@ -398,15 +422,21 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>  	unsigned int rx_size = msg->len;
>  	unsigned int len;
>  	int ret;
> +	bool burst_read = false;
> +
> +	if (rx_size > BXT_GMBUS_BYTE_COUNT_MAX)
> +		burst_read = gmbus_burst_read_supported(dev_priv);
>  
>  	do {
> -		if (INTEL_GEN(dev_priv) >= 9)
> +		if (burst_read)
> +			len = rx_size;
> +		else if (INTEL_GEN(dev_priv) >= 9)
>  			len = min(rx_size, BXT_GMBUS_BYTE_COUNT_MAX);
>  		else
>  			len = min(rx_size, GMBUS_BYTE_COUNT_MAX);

If you abstracted the max transfer length in the previous patch, I think
you could condence this into something like:

		if (HAS_GMBUS_BURST_READ(dev_priv))
                	len = rx_size;
                else
                	len = min(rx_size, gmbus_max_xfer_size(dev_priv))

You could throw away the burst_read variable and parameter altogether,
and have gmbus_xfer_read_chunk() decide internally if burst read is to
be used by:

	burst_read = len > gmbus_max_xfer_size(dev_priv);

(Potentially with a WARN_ON(burst_read && !HAS_GMBUS_BURST_READ()) if
you want to be paranoid.)

BR,
Jani.

>  
>  		ret = gmbus_xfer_read_chunk(dev_priv, msg->addr,
> -					    buf, len, gmbus1_index);
> +					    buf, len, gmbus1_index, burst_read);
>  		if (ret)
>  			return ret;
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4f583da0cee9..0ef162ee9ce0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2984,6 +2984,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 7e92c7934657..2ee9cf2effcd 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -364,12 +364,24 @@  gmbus_wait_idle(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 gmbus1_index, bool burst_read)
 {
+	unsigned int bytes_af_override, size;
+
+	if (burst_read) {
+		bytes_af_override = len - (((len / 256) - 1) * 256);
+		size = bytes_af_override;
+
+		I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) |
+			      GMBUS_BYTE_CNT_OVERRIDE));
+	} else {
+		size = len;
+	}
+
 	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) {
@@ -385,11 +397,23 @@  gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
 			*buf++ = val & 0xff;
 			val >>= 8;
 		} while (--len && ++loop < 4);
+
+		if (burst_read && len == (bytes_af_override - 4))
+			I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) &
+				      ~GMBUS_BYTE_CNT_OVERRIDE));
 	}
 
 	return 0;
 }
 
+static bool gmbus_burst_read_supported(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
+	    IS_KABYLAKE(dev_priv))
+		return true;
+	return false;
+}
+
 static int
 gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
 		u32 gmbus1_index)
@@ -398,15 +422,21 @@  gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
 	unsigned int rx_size = msg->len;
 	unsigned int len;
 	int ret;
+	bool burst_read = false;
+
+	if (rx_size > BXT_GMBUS_BYTE_COUNT_MAX)
+		burst_read = gmbus_burst_read_supported(dev_priv);
 
 	do {
-		if (INTEL_GEN(dev_priv) >= 9)
+		if (burst_read)
+			len = rx_size;
+		else if (INTEL_GEN(dev_priv) >= 9)
 			len = min(rx_size, BXT_GMBUS_BYTE_COUNT_MAX);
 		else
 			len = min(rx_size, GMBUS_BYTE_COUNT_MAX);
 
 		ret = gmbus_xfer_read_chunk(dev_priv, msg->addr,
-					    buf, len, gmbus1_index);
+					    buf, len, gmbus1_index, burst_read);
 		if (ret)
 			return ret;