Re: [PATCH v2 6/8] drm/i915: Add function to output Aksv over GMBUS

[Ville Syrjälä]

On Fri, Dec 01, 2017 at 12:20:28PM -0500, Sean Paul wrote:
> Once the Aksv is available in the PCH, we need to get it on the wire to
> the receiver via DDC. The hardware doesn't allow us to read the value
> directly, so we need to tell GMBUS to source the Aksv internally and
> send it to the right offset on the receiver.
> 
> The way we do this is to initiate an indexed write where the index is
> the Aksv register offset. We write dummy values to GMBUS3 as if we were
> sending the key, and the hardware slips in the "real" values when it
> goes out.
> 
> Changes in v2:
> - None
> 
> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_i2c.c | 54 ++++++++++++++++++++++++++++++++++------
>  3 files changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 36bb4927484a..10f740c9e571 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -4043,6 +4043,7 @@ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv);
>  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 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 6dca305ccbf7..8b71a20882ca 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3040,6 +3040,7 @@ enum i915_power_well_id {
>  # define GPIO_DATA_PULLUP_DISABLE	(1 << 13)
>  
>  #define GMBUS0			_MMIO(dev_priv->gpio_mmio_base + 0x5100) /* clock/port select */
> +#define   GMBUS_AKSV_SELECT	(1<<11)
>  #define   GMBUS_RATE_100KHZ	(0<<8)
>  #define   GMBUS_RATE_50KHZ	(1<<8)
>  #define   GMBUS_RATE_400KHZ	(2<<8) /* reserved on Pineview */
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index eb5827110d8f..c01156bf0f27 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -30,6 +30,7 @@
>  #include <linux/i2c-algo-bit.h>
>  #include <linux/export.h>
>  #include <drm/drmP.h>
> +#include <drm/drm_hdcp.h>
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> @@ -373,7 +374,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>  
>  static int
>  gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
> -		       unsigned short addr, u8 *buf, unsigned int len)
> +		       unsigned short addr, u8 *buf, unsigned int len,
> +		       u32 gmbus1_index)
>  {
>  	unsigned int chunk_size = len;
>  	u32 val, loop;
> @@ -386,7 +388,7 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
>  
>  	I915_WRITE_FW(GMBUS3, val);
>  	I915_WRITE_FW(GMBUS1,
> -		      GMBUS_CYCLE_WAIT |
> +		      gmbus1_index | GMBUS_CYCLE_WAIT |
>  		      (chunk_size << GMBUS_BYTE_COUNT_SHIFT) |
>  		      (addr << GMBUS_SLAVE_ADDR_SHIFT) |
>  		      GMBUS_SLAVE_WRITE | GMBUS_SW_RDY);
> @@ -409,7 +411,8 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
>  }
>  
>  static int
> -gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
> +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
> +		 u32 gmbus1_index)
>  {
>  	u8 *buf = msg->buf;
>  	unsigned int tx_size = msg->len;
> @@ -419,7 +422,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
>  	do {
>  		len = min(tx_size, GMBUS_BYTE_COUNT_MAX);
>  
> -		ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len);
> +		ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len,
> +					     gmbus1_index);
>  		if (ret)
>  			return ret;
>  
> @@ -470,7 +474,8 @@ gmbus_xfer_index_read(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)
> +do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num,
> +	      u32 gmbus0_source, u32 gmbus1_index)
>  {
>  	struct intel_gmbus *bus = container_of(adapter,
>  					       struct intel_gmbus,
> @@ -480,7 +485,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>  	int ret = 0;
>  
>  retry:
> -	I915_WRITE_FW(GMBUS0, bus->reg0);
> +	I915_WRITE_FW(GMBUS0, gmbus0_source | bus->reg0);
>  
>  	for (; i < num; i += inc) {
>  		inc = 1;
> @@ -490,7 +495,8 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>  		} else if (msgs[i].flags & I2C_M_RD) {
>  			ret = gmbus_xfer_read(dev_priv, &msgs[i], 0);
>  		} else {
> -			ret = gmbus_xfer_write(dev_priv, &msgs[i]);
> +			ret = gmbus_xfer_write(dev_priv, &msgs[i],
> +					       gmbus1_index);
>  		}
>  
>  		if (!ret)
> @@ -598,7 +604,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);
> +		ret = do_gmbus_xfer(adapter, msgs, num, 0, 0);
>  		if (ret == -EAGAIN)
>  			bus->force_bit |= GMBUS_FORCE_BIT_RETRY;
>  	}
> @@ -608,6 +614,38 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>  	return ret;
>  }
>  
> +int intel_gmbus_output_aksv(struct i2c_adapter *adapter)
> +{
> +	struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus,
> +					       adapter);
> +	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	int ret;
> +	u8 buf[DRM_HDCP_KSV_LEN] = { 0 };
> +	struct i2c_msg msg = {
> +		.addr = DRM_HDCP_DDC_ADDR,
> +		.flags = 0,
> +		.len = sizeof(buf),
> +		.buf = buf,
> +	};
> +
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> +	mutex_lock(&dev_priv->gmbus_mutex);
> +
> +	/*
> +	 * In order to output Aksv to the receiver, use an indexed write to
> +	 * 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, &msg, 1, GMBUS_AKSV_SELECT,
> +			    GMBUS_CYCLE_INDEX |
> +			    (DRM_HDCP_DDC_AKSV << GMBUS_SLAVE_INDEX_SHIFT));

It might be nicer to just use two msgs here and modify
gmbus_xfer_index_read() to support indexed writes as well.
Would avoid having to pass in the GMBUS1 value all the way.

Not sure what to do with GMBUS_AKSV_SELECT. I guess either pass
it in explicitly like you're doing here, or maybe add a small
helper ala gmbus_is_index_read() so that do_gmbus_xfer() could
sniff out which source we should use based on the passed in msgs?

> +
> +	mutex_unlock(&dev_priv->gmbus_mutex);
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> +
> +	return ret;
> +}
> +
>  static u32 gmbus_func(struct i2c_adapter *adapter)
>  {
>  	return i2c_bit_algo.functionality(adapter) &
> -- 
> 2.15.0.531.g2ccb3012c9-goog
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC