[Intel-gfx] [PATCH v5 2/2] drm/i915/gmbus: Enable burst read
Ramalingam C
ramalingam.c at intel.com
Fri Jun 1 11:09:51 UTC 2018
On Tuesday 29 May 2018 11:35 PM, Ville Syrjälä wrote:
> 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 at 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.
Ville,
All product sku of KBL will have the WA. And we are enabling this from
gen10+ including KBL and GLK for HDCP2.2 requirement.
Thats the reasoning behind this.
>
>> /* 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.
With this newline removed, I will submit the next version with your
reviewed-by.
Thanks
Ram
>
> Otherwise this looks good to me.
> Reviewed-by: Ville Syrjälä <ville.syrjala at 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
More information about the Intel-gfx
mailing list