[PATCH v3 36/40] drm/i915: Implement gmbus burst read

Daniel Vetter daniel at ffwll.ch
Tue Apr 3 16:40:14 UTC 2018


On Tue, Apr 03, 2018 at 07:27:49PM +0530, Ramalingam C wrote:
> Implements a interface for single burst read of data that is larger
> than 512 Bytes through gmbus.
> 
> HDCP2.2 spec expects HDCP2.2 transmitter to read 522Bytes of HDCP
> receiver certificates in single burst read. On gmbus, to read more
> than 511Bytes, HW provides a workaround for burst read.
> 
> This patch passes the burst read request through gmbus read functions.
> And implements the sequence of enabling and disabling the burst read.
> 
> v2:
>   No Changes.
> v3:
>   No Changes.
> 
> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>

Why only enable this burst_read mode for hdcp, and not for all i2c
transactions? Seems to unecessarily complicate the code, since it requires
that you pass burst_read through the entire call chain. For other changes
we've done for hdcp (like enabling the read/write mode and other stuff)
we've enabled it for all i2c transactions. That also means more testing,
since it will be used even when HDCP is not in use.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   2 +
>  drivers/gpu/drm/i915/i915_reg.h  |   3 +
>  drivers/gpu/drm/i915/intel_i2c.c | 124 +++++++++++++++++++++++++++++++++------
>  3 files changed, 112 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6e740f6fe33f..72534a1e544b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3688,6 +3688,8 @@ extern void intel_teardown_gmbus(struct drm_i915_private *dev_priv);
>  extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>  				     unsigned int pin);
>  extern int intel_gmbus_output_aksv(struct i2c_adapter *adapter);
> +extern int intel_gmbus_burst_read(struct i2c_adapter *adapter,
> +				  unsigned int offset, void *buf, size_t size);
>  
>  extern struct i2c_adapter *
>  intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f04ad3c15abd..56979bc4e9d8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3123,6 +3123,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
> @@ -3150,8 +3151,10 @@ enum i915_power_well_id {
>  #define   GMBUS_CYCLE_WAIT	(1<<25)
>  #define   GMBUS_CYCLE_INDEX	(2<<25)
>  #define   GMBUS_CYCLE_STOP	(4<<25)
> +#define   GMBUS_CYCLE_MASK	(7<<25)
>  #define   GMBUS_BYTE_COUNT_SHIFT 16
>  #define   GMBUS_BYTE_COUNT_MAX   256U
> +#define   GMBUS_BYTE_COUNT_HW_MAX 511U
>  #define   GMBUS_SLAVE_INDEX_SHIFT 8
>  #define   GMBUS_SLAVE_ADDR_SHIFT 1
>  #define   GMBUS_SLAVE_READ	(1<<0)
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index e6875509bcd9..dcb2be0d54ee 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -364,21 +364,30 @@ 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 size = len;
> +	int ret;
> +
> +	if (burst_read) {
> +		/* Seq to enable Burst Read */
> +		I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) |
> +			      GMBUS_BYTE_CNT_OVERRIDE));
> +		size = GMBUS_BYTE_COUNT_HW_MAX;
> +	}
> +
>  	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) {
> -		int ret;
>  		u32 val, loop = 0;
>  
>  		ret = gmbus_wait(dev_priv, GMBUS_HW_RDY, GMBUS_HW_RDY_EN);
>  		if (ret)
> -			return ret;
> +			goto exit;
>  
>  		val = I915_READ_FW(GMBUS3);
>  		do {
> @@ -387,12 +396,29 @@ gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
>  		} while (--len && ++loop < 4);
>  	}
>  
> -	return 0;
> +exit:
> +	if (burst_read) {
> +
> +		/* Seq to disable the Burst Read */
> +		I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) &
> +			      ~GMBUS_BYTE_CNT_OVERRIDE));
> +		I915_WRITE_FW(GMBUS1, (I915_READ_FW(GMBUS1) &
> +			      ~GMBUS_CYCLE_MASK) | GMBUS_CYCLE_STOP);
> +
> +		/*
> +		 * On Burst read disable, GMBUS need more time to settle
> +		 * down to Idle State.
> +		 */
> +		ret = intel_wait_for_register_fw(dev_priv, GMBUS2,
> +						 GMBUS_ACTIVE, 0, 50);
> +	}
> +
> +	return ret;
>  }
>  
>  static int
>  gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
> -		u32 gmbus1_index)
> +		u32 gmbus1_index, bool burst_read)
>  {
>  	u8 *buf = msg->buf;
>  	unsigned int rx_size = msg->len;
> @@ -400,10 +426,13 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>  	int ret;
>  
>  	do {
> -		len = min(rx_size, GMBUS_BYTE_COUNT_MAX);
> +		if (burst_read)
> +			len = rx_size;
> +		else
> +			len = min(rx_size, GMBUS_BYTE_COUNT_MAX);
>  
> -		ret = gmbus_xfer_read_chunk(dev_priv, msg->addr,
> -					    buf, len, gmbus1_index);
> +		ret = gmbus_xfer_read_chunk(dev_priv, msg->addr, buf, len,
> +					    gmbus1_index, burst_read);
>  		if (ret)
>  			return ret;
>  
> @@ -491,7 +520,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,
> +		 bool burst_read)
>  {
>  	u32 gmbus1_index = 0;
>  	u32 gmbus5 = 0;
> @@ -509,7 +539,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],
> +				      gmbus1_index, burst_read);
>  	else
>  		ret = gmbus_xfer_write(dev_priv, &msgs[1], gmbus1_index);
>  
> @@ -522,7 +553,7 @@ gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs)
>  
>  static int
>  do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num,
> -	      u32 gmbus0_source)
> +	      u32 gmbus0_source, bool burst_read)
>  {
>  	struct intel_gmbus *bus = container_of(adapter,
>  					       struct intel_gmbus,
> @@ -544,15 +575,20 @@ 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], burst_read);
>  			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],
> +					      0, burst_read);
>  		} else {
>  			ret = gmbus_xfer_write(dev_priv, &msgs[i], 0);
>  		}
>  
> -		if (!ret)
> +		/*
> +		 * Burst read Sequence ends with STOP. So Dont expect
> +		 * HW wait phase.
> +		 */
> +		if (!ret && !burst_read)
>  			ret = gmbus_wait(dev_priv,
>  					 GMBUS_HW_WAIT_PHASE, GMBUS_HW_WAIT_EN);
>  		if (ret == -ETIMEDOUT)
> @@ -664,7 +700,7 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>  		if (ret < 0)
>  			bus->force_bit &= ~GMBUS_FORCE_BIT_RETRY;
>  	} else {
> -		ret = do_gmbus_xfer(adapter, msgs, num, 0);
> +		ret = do_gmbus_xfer(adapter, msgs, num, 0, false);
>  		if (ret == -EAGAIN)
>  			bus->force_bit |= GMBUS_FORCE_BIT_RETRY;
>  	}
> @@ -705,7 +741,8 @@ int intel_gmbus_output_aksv(struct i2c_adapter *adapter)
>  	 * pass the i2c command, and tell GMBUS to use the HW-provided value
>  	 * instead of sourcing GMBUS3 for the data.
>  	 */
> -	ret = do_gmbus_xfer(adapter, msgs, ARRAY_SIZE(msgs), GMBUS_AKSV_SELECT);
> +	ret = do_gmbus_xfer(adapter, msgs, ARRAY_SIZE(msgs),
> +			    GMBUS_AKSV_SELECT, false);
>  
>  	mutex_unlock(&dev_priv->gmbus_mutex);
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> @@ -713,6 +750,59 @@ int intel_gmbus_output_aksv(struct i2c_adapter *adapter)
>  	return ret;
>  }
>  
> +static inline
> +bool intel_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;
> +}
> +
> +int intel_gmbus_burst_read(struct i2c_adapter *adapter, unsigned int offset,
> +			   void *buf, size_t size)
> +{
> +	struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus,
> +					       adapter);
> +	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	int ret;
> +	u8 start = offset & 0xff;
> +	struct i2c_msg msgs[] = {
> +		{
> +			.addr = DRM_HDCP_DDC_ADDR,
> +			.flags = 0,
> +			.len = 1,
> +			.buf = &start,
> +		},
> +		{
> +			.addr = DRM_HDCP_DDC_ADDR,
> +			.flags = I2C_M_RD,
> +			.len = size,
> +			.buf = buf,
> +		}
> +	};
> +
> +	if (!intel_gmbus_burst_read_supported(dev_priv))
> +		return -EINVAL;
> +
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> +	mutex_lock(&dev_priv->gmbus_mutex);
> +
> +	/*
> +	 * In order to read the complete length(More than GMBus Limit) of data,
> +	 * in burst mode, implement the Workaround supported in HW.
> +	 */
> +	ret = do_gmbus_xfer(adapter, msgs, ARRAY_SIZE(msgs), 0, true);
> +
> +	mutex_unlock(&dev_priv->gmbus_mutex);
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> +
> +	if (ret == ARRAY_SIZE(msgs))
> +		return 0;
> +
> +	return ret >= 0 ? -EIO : ret;
> +}
> +
>  static u32 gmbus_func(struct i2c_adapter *adapter)
>  {
>  	return i2c_bit_algo.functionality(adapter) &
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list