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 <ramalinga...@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.syrj...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to