[Intel-gfx] [PATCH v5 2/2] drm/i915/gmbus: Enable burst read

Ville Syrjälä ville.syrjala at linux.intel.com
Tue May 29 18:05:05 UTC 2018


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 at intel.com>
> ---
>  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.

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

Otherwise this looks good to me.
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>


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

-- 
Ville Syrjälä
Intel


More information about the Intel-gfx mailing list