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@xxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel