On Wednesday 18 April 2018 08:47 PM, Ville Syrjälä wrote:
On Wed, Apr 18, 2018 at 09:20:23AM +0300, Jani Nikula wrote:
On Wed, 18 Apr 2018, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
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+.
Please let's not change the limit for old platforms for absolutely no
gain. And if Ville insists anyway, let's leave that as a separate
follow-up change that can easily be reverted later.
I might consider using the 511 limit only for platforms that
HAS_GMBUS_BURST_READ too.
The original limit seems to have been added in 9535c4757b88 ("drm/i915:
cope with large i2c transfers") citing "the specs". Any recollection
anyone? Chris?
"Duble buffered data register and a 9 bit counter support 0 byte to 256
Byte transfers."
has always been in the spec (and still is for SPT+).
Seems like gap in Bspec. Just today HW team has clarified that all
platforms support 511Bytes for single Gmbus read and write cycle.
--Ram
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
