On Thu, 12 Apr 2018, Ramalingam C <ramalinga...@intel.com> 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.
>
> Signed-off-by: Ramalingam C <ramalinga...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_i2c.c | 38 ++++++++++++++++++++++++++++++++++----
>  2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4f583da0cee9..0ef162ee9ce0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2984,6 +2984,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 7e92c7934657..2ee9cf2effcd 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -364,12 +364,24 @@ 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)

I think you could throw out the burst_read parameter. See below.

>  {
> +     unsigned int bytes_af_override, size;
> +
> +     if (burst_read) {
> +             bytes_af_override = len - (((len / 256) - 1) * 256);
> +             size = bytes_af_override;
> +
> +             I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) |
> +                           GMBUS_BYTE_CNT_OVERRIDE));
> +     } else {
> +             size = len;
> +     }
> +
>       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) {
> @@ -385,11 +397,23 @@ gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
>                       *buf++ = val & 0xff;
>                       val >>= 8;
>               } while (--len && ++loop < 4);
> +
> +             if (burst_read && len == (bytes_af_override - 4))
> +                     I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) &
> +                                   ~GMBUS_BYTE_CNT_OVERRIDE));
>       }
>  
>       return 0;
>  }
>  
> +static bool 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;
> +}

Name this HAS_GMBUS_BURST_READ() and put below HAS_GMBUS_IRQ() in
i915_drv.h.

> +
>  static int
>  gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>               u32 gmbus1_index)
> @@ -398,15 +422,21 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, 
> struct i2c_msg *msg,
>       unsigned int rx_size = msg->len;
>       unsigned int len;
>       int ret;
> +     bool burst_read = false;
> +
> +     if (rx_size > BXT_GMBUS_BYTE_COUNT_MAX)
> +             burst_read = gmbus_burst_read_supported(dev_priv);
>  
>       do {
> -             if (INTEL_GEN(dev_priv) >= 9)
> +             if (burst_read)
> +                     len = rx_size;
> +             else if (INTEL_GEN(dev_priv) >= 9)
>                       len = min(rx_size, BXT_GMBUS_BYTE_COUNT_MAX);
>               else
>                       len = min(rx_size, GMBUS_BYTE_COUNT_MAX);

If you abstracted the max transfer length in the previous patch, I think
you could condence this into something like:

                if (HAS_GMBUS_BURST_READ(dev_priv))
                        len = rx_size;
                else
                        len = min(rx_size, gmbus_max_xfer_size(dev_priv))

You could throw away the burst_read variable and parameter altogether,
and have gmbus_xfer_read_chunk() decide internally if burst read is to
be used by:

        burst_read = len > gmbus_max_xfer_size(dev_priv);

(Potentially with a WARN_ON(burst_read && !HAS_GMBUS_BURST_READ()) if
you want to be paranoid.)

BR,
Jani.

>  
>               ret = gmbus_xfer_read_chunk(dev_priv, msg->addr,
> -                                         buf, len, gmbus1_index);
> +                                         buf, len, gmbus1_index, burst_read);
>               if (ret)
>                       return ret;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to