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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx>
> + /*
> + * 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
