[PATCH 1/3] drm/i915/gmbus: Switch GMBUS fully to _fw() register accesses

[Ville Syrjala <ville.syrjala@linux.intel.com>]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Switch the gmbus code over from _notrace() to _fw() register
accesses.

My original goal was just to eliminate the _notrace() variants
to shrink the intel_de.h API a bit, but while doing this I
stumbled on a huge performance issue with the current code;
Bit banging turns out to be extremely expensive on PCH
platforms while in DC5/6, and cause turns out not to be the
actual south GMBUS/GPIO register accesses, but rather the
extra north FPGA_DBG register access done from the non _fw()
register access functions. Those accesses cause a DC5/6 exit
which take a huge chunk of time.

$ time echo detect > /sys/class/drm/card0/HDMI-A-1/status
-real	0m16.281s
+real	0m1.459s
...

There does still seem to be a smallish (maybe ~15%) performance
penalty for accessing PCH registers while in DC5/6. Not really
sure what's causing it, but seems to be small enough to not cause
a huge performance problem for bit banging. When not in DC5/6
the same HDMI detect cycle takes ~1.25 seconds. If we want to
further speed that up we could completely disable DC5/6 during
big banged i2c transfers. Currently we only do that on BXT/GLK
because there the GMBUS hardware lives on the SoC and thus is
affected by DC states, even for non-bit banged transfers.

Note that by using the _fw() register accessors we also lose the
uncore.lock protection, but that's fine because everything is
already protected by the gmbus mutex, and nothing else should
live on the same cachelines of registers. So even platforms like
IVB/HSW which may have some issues with concurrent register
accesses to the same cacheline are still covered.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_de.h    |  6 ++++
 drivers/gpu/drm/i915/display/intel_gmbus.c | 34 +++++++++++-----------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
index 102979019429..8144588d3cf7 100644
--- a/drivers/gpu/drm/i915/display/intel_de.h
+++ b/drivers/gpu/drm/i915/display/intel_de.h
@@ -158,6 +158,12 @@ intel_de_rmw_fw(struct intel_display *display, intel_reg_t reg, u32 clear, u32 s
 	return old;
 }
 
+static inline void
+intel_de_posting_read_fw(struct intel_display *display, intel_reg_t reg)
+{
+	intel_de_read_fw(display, reg);
+}
+
 static inline u32
 intel_de_read_notrace(struct intel_display *display, intel_reg_t reg)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
index 60a70dea5d85..09171f3db298 100644
--- a/drivers/gpu/drm/i915/display/intel_gmbus.c
+++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
@@ -218,8 +218,8 @@ to_intel_gmbus(struct i2c_adapter *i2c)
 void
 intel_gmbus_reset(struct intel_display *display)
 {
-	intel_de_write(display, GMBUS0(display), 0);
-	intel_de_write(display, GMBUS4(display), 0);
+	intel_de_write_fw(display, GMBUS0(display), 0);
+	intel_de_write_fw(display, GMBUS4(display), 0);
 }
 
 static void pnv_gmbus_clock_gating(struct intel_display *display,
@@ -262,7 +262,7 @@ static u32 get_reserved(struct intel_gmbus *bus)
 		preserve_bits |= GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_VAL_MASK |
 				 GPIO_DATA_DIR_MASK | GPIO_DATA_VAL_MASK;
 
-	return intel_de_read_notrace(display, bus->gpio_reg) & preserve_bits;
+	return intel_de_read_fw(display, bus->gpio_reg) & preserve_bits;
 }
 
 static int get_clock(void *data)
@@ -271,10 +271,10 @@ static int get_clock(void *data)
 	struct intel_display *display = bus->display;
 	u32 reserved = get_reserved(bus);
 
-	intel_de_write_notrace(display, bus->gpio_reg, reserved | GPIO_CLOCK_DIR_MASK);
-	intel_de_write_notrace(display, bus->gpio_reg, reserved);
+	intel_de_write_fw(display, bus->gpio_reg, reserved | GPIO_CLOCK_DIR_MASK);
+	intel_de_write_fw(display, bus->gpio_reg, reserved);
 
-	return (intel_de_read_notrace(display, bus->gpio_reg) & GPIO_CLOCK_VAL_IN) != 0;
+	return (intel_de_read_fw(display, bus->gpio_reg) & GPIO_CLOCK_VAL_IN) != 0;
 }
 
 static int get_data(void *data)
@@ -283,10 +283,10 @@ static int get_data(void *data)
 	struct intel_display *display = bus->display;
 	u32 reserved = get_reserved(bus);
 
-	intel_de_write_notrace(display, bus->gpio_reg, reserved | GPIO_DATA_DIR_MASK);
-	intel_de_write_notrace(display, bus->gpio_reg, reserved);
+	intel_de_write_fw(display, bus->gpio_reg, reserved | GPIO_DATA_DIR_MASK);
+	intel_de_write_fw(display, bus->gpio_reg, reserved);
 
-	return (intel_de_read_notrace(display, bus->gpio_reg) & GPIO_DATA_VAL_IN) != 0;
+	return (intel_de_read_fw(display, bus->gpio_reg) & GPIO_DATA_VAL_IN) != 0;
 }
 
 static void set_clock(void *data, int state_high)
@@ -302,8 +302,8 @@ static void set_clock(void *data, int state_high)
 		clock_bits = GPIO_CLOCK_DIR_OUT | GPIO_CLOCK_DIR_MASK |
 			     GPIO_CLOCK_VAL_MASK;
 
-	intel_de_write_notrace(display, bus->gpio_reg, reserved | clock_bits);
-	intel_de_posting_read(display, bus->gpio_reg);
+	intel_de_write_fw(display, bus->gpio_reg, reserved | clock_bits);
+	intel_de_posting_read_fw(display, bus->gpio_reg);
 }
 
 static void set_data(void *data, int state_high)
@@ -319,15 +319,15 @@ static void set_data(void *data, int state_high)
 		data_bits = GPIO_DATA_DIR_OUT | GPIO_DATA_DIR_MASK |
 			GPIO_DATA_VAL_MASK;
 
-	intel_de_write_notrace(display, bus->gpio_reg, reserved | data_bits);
-	intel_de_posting_read(display, bus->gpio_reg);
+	intel_de_write_fw(display, bus->gpio_reg, reserved | data_bits);
+	intel_de_posting_read_fw(display, bus->gpio_reg);
 }
 
 static void
 ptl_handle_mask_bits(struct intel_gmbus *bus, bool set)
 {
 	struct intel_display *display = bus->display;
-	u32 reg_val = intel_de_read_notrace(display, bus->gpio_reg);
+	u32 reg_val = intel_de_read_fw(display, bus->gpio_reg);
 	u32 mask_bits = GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_VAL_MASK |
 			GPIO_DATA_DIR_MASK | GPIO_DATA_VAL_MASK;
 	if (set)
@@ -335,8 +335,8 @@ ptl_handle_mask_bits(struct intel_gmbus *bus, bool set)
 	else
 		reg_val &= ~mask_bits;
 
-	intel_de_write_notrace(display, bus->gpio_reg, reg_val);
-	intel_de_posting_read(display, bus->gpio_reg);
+	intel_de_write_fw(display, bus->gpio_reg, reg_val);
+	intel_de_posting_read_fw(display, bus->gpio_reg);
 }
 
 static int
@@ -454,7 +454,7 @@ gmbus_wait_idle(struct intel_display *display)
 	add_wait_queue(&display->gmbus.wait_queue, &wait);
 	intel_de_write_fw(display, GMBUS4(display), irq_enable);
 
-	ret = intel_de_wait_fw_ms(display, GMBUS2(display), GMBUS_ACTIVE, 0, 10, NULL);
+	ret = intel_de_wait_ms(display, GMBUS2(display), GMBUS_ACTIVE, 0, 10, NULL);
 
 	intel_de_write_fw(display, GMBUS4(display), 0);
 	remove_wait_queue(&display->gmbus.wait_queue, &wait);
-- 
2.54.0