Re: [PATCH v3 1/2] drm/i915/gmbus: Increase the Bytes per Rd/Wr Op

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tuesday 17 April 2018 11:39 PM, Ville Syrjälä wrote:
On Tue, Apr 17, 2018 at 02:25:32PM +0530, Ramalingam C wrote:
>From Gen9 onwards Bspec says HW supports Max Bytes per single RD/WR op is
511Bytes instead of previous 256Bytes used in SW.

This change allows the max bytes per op upto 511Bytes from Gen9 onwards.

v2:
   No Change.
v3:
   Inline function for max_xfer_size and renaming of the macro.[Jani]

Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_reg.h  |  1 +
  drivers/gpu/drm/i915/intel_i2c.c | 11 +++++++++--
  2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 475cac07d3e6..be6114a0e8ab 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3013,6 +3013,7 @@ enum i915_power_well_id {
  #define   GMBUS_CYCLE_STOP	(4<<25)
  #define   GMBUS_BYTE_COUNT_SHIFT 16
  #define   GMBUS_BYTE_COUNT_MAX   256U
+#define   GEN9_GMBUS_BYTE_COUNT_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..4367827d7661 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -361,6 +361,13 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv)
  	return ret;
  }
+static inline
+unsigned int gmbus_max_xfer_size(struct drm_i915_private *dev_priv)
+{
+	return (INTEL_GEN(dev_priv) >= 9) ? GEN9_GMBUS_BYTE_COUNT_MAX :
+		GMBUS_BYTE_COUNT_MAX;
Hmm. You sure about this 256 limit on older HW? The spec does sort of
say that 0-256 is the valid range, but the SPT+ docs still have that
same text, and the register has always had 9 bits for byte count. I
don't see any statements saying that they changed this in any way for
SPT. It only talks about >511 bytes needing the special treatment.

If we do this the I think you should just drop the defines and put the
raw numbers into this function. The extra indirection just makes life
harder. Also pointless parens around the GEN>9 check.
Even I couldn't get any place where BSpec says 256Bytes is the limit for any platform. Everywhere 9bits are used. And when I cross verified with other OS usage 511Bytes is used as limit across all platforms.

Just to be cautious for not breaking any older platforms out in linux world, I limited the extension of the limit to the known
and easily testable platforms at my desk (Gen9+)

Do you suggest we should apply 511Bytes as max limit for all platforms?
Do we have any means to test this new limit on all supported legacy platforms?

Except enabling the full potential of the HW in SW, I dont see any ROI here as most of the GMBUS reqs are <256Bytes.
Only in case of HDCP2.2 we need single read cycle for 538Bytes.

we have couple of options here: Please share your opinion to choose one of them. 1. Just dont change the upper limit for RD/WR. Keep it as it is at 256Bytes. Anyway no user demands it. 2. As per HW capability, Change the upper limit for RD/WR to 511Bytes for all platforms. This is needs the functional verification on all legacy plat supported. 3. Change the upper limit for RD/WR to 511Bytes for newer platforms, say Gen9+.

--Ram

+}
+
  static int
  gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
  		      unsigned short addr, u8 *buf, unsigned int len,
@@ -400,7 +407,7 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
  	int ret;
do {
-		len = min(rx_size, GMBUS_BYTE_COUNT_MAX);
+		len = min(rx_size, gmbus_max_xfer_size(dev_priv));
ret = gmbus_xfer_read_chunk(dev_priv, msg->addr,
  					    buf, len, gmbus1_index);
@@ -462,7 +469,7 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
  	int ret;
do {
-		len = min(tx_size, GMBUS_BYTE_COUNT_MAX);
+		len = min(tx_size, gmbus_max_xfer_size(dev_priv));
ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len,
  					     gmbus1_index);
--
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux