All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements
@ 2019-01-07 11:15 Hans de Goede
  2019-01-07 11:15 ` [PATCH v6 1/4] ACPI / PMIC: Add support for executing " Hans de Goede
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Hans de Goede @ 2019-01-07 11:15 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Rafael J . Wysocki,
	Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: intel-gfx, linux-acpi

Hi All,

This patch-set has been on the list for a while now, it would be nice
if we can get this merged. I already have an ack for merging the ACPI
bits through drm-intel-next-queued, so we really need an ack for
the last 2 patches from one of the intel-gfx folks so that I can push
this to dinq.

Compared to version 5 this version has one new patch:
"ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling"
which adds a generic regmap based handler and enables that for AXP288
PMIC-s. This new patch is one of the patches needing a review / ack.

Thanks & Regards,

Hans

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v6 1/4] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
  2019-01-07 11:15 [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements Hans de Goede
@ 2019-01-07 11:15 ` Hans de Goede
  2019-01-07 15:35   ` Andy Shevchenko
  2019-01-07 11:15 ` [PATCH v6 2/4] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2019-01-07 11:15 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Rafael J . Wysocki,
	Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: intel-gfx, linux-acpi

DSI LCD panels describe an initialization sequence in the Video BIOS
Tables using so called MIPI sequences. One possible element in these
sequences is a PMIC specific element of 15 bytes.

Although this is not really an ACPI opregion, the ACPI opregion code is the
closest thing we have. We need to have support for these PMIC specific MIPI
sequence elements somwhere. Since we already instantiate a special platform
device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
with PMIC specific implementations of the OpRegion, the handling of MIPI
sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.

This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
function, which is to be backed by a PMIC specific
exec_mipi_pmic_seq_element callback. This function will be called by the
i915 code to execture MIPI sequence PMIC elements.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v6:
-Small refactoring of intel_soc_pmic_exec_mipi_pmic_seq_element to make
 adding a generic implementation later easier

Changes in v5:
-Pass i2c-address + register-address + value + mask as separate arguments to
 the intel_soc_pmic_exec_mipi_pmic_seq_element function. Instead of passing
 the 15 bytes of raw MIPI sequence data. The decoding will be done in the
 i915 VBT code instead.

Changes in v3:
-Add kerneldoc for intel_soc_pmic_exec_mipi_pmic_seq_element
-Make intel_soc_pmic_exec_mipi_pmic_seq_element return errors
---
 drivers/acpi/pmic/intel_pmic.c     | 52 ++++++++++++++++++++++++++++++
 drivers/acpi/pmic/intel_pmic.h     |  2 ++
 include/linux/mfd/intel_soc_pmic.h |  3 ++
 3 files changed, 57 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index ca18e0d23df9..471afeea87c2 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -15,6 +15,7 @@
 
 #include <linux/export.h>
 #include <linux/acpi.h>
+#include <linux/mfd/intel_soc_pmic.h>
 #include <linux/regmap.h>
 #include <acpi/acpi_lpat.h>
 #include "intel_pmic.h"
@@ -36,6 +37,8 @@ struct intel_pmic_opregion {
 	struct intel_pmic_regs_handler_ctx ctx;
 };
 
+static struct intel_pmic_opregion *intel_pmic_opregion;
+
 static int pmic_get_reg_bit(int address, struct pmic_table *table,
 			    int count, int *reg, int *bit)
 {
@@ -304,6 +307,7 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 	}
 
 	opregion->data = d;
+	intel_pmic_opregion = opregion;
 	return 0;
 
 out_remove_thermal_handler:
@@ -319,3 +323,51 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
+
+/**
+ * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence
+ * @i2c_address:  I2C client address for the PMIC
+ * @reg_address:  PMIC register address
+ * @value:        New value for the register bits to change
+ * @mask:         Mask indicating which register bits to change
+ *
+ * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
+ * BIOS Tables) using so called MIPI sequences. One possible element in these
+ * sequences is a PMIC specific element of 15 bytes.
+ *
+ * This function executes these PMIC specific elements sending the embedded
+ * commands to the PMIC.
+ *
+ * Return 0 on success, < 0 on failure.
+ */
+int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
+					      u32 value, u32 mask)
+{
+	struct intel_pmic_opregion_data *d;
+	int ret;
+
+	if (!intel_pmic_opregion) {
+		pr_warn("%s: No PMIC registered\n", __func__);
+		return -ENXIO;
+	}
+
+	d = intel_pmic_opregion->data;
+
+	mutex_lock(&intel_pmic_opregion->lock);
+
+	if (d->exec_mipi_pmic_seq_element) {
+		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
+						    i2c_address, reg_address,
+						    value, mask);
+	} else {
+		pr_warn("%s: Not implemented\n", __func__);
+		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
+			__func__, i2c_address, reg_address, value, mask);
+		ret = -EOPNOTSUPP;
+	}
+
+	mutex_unlock(&intel_pmic_opregion->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(intel_soc_pmic_exec_mipi_pmic_seq_element);
diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
index 095afc96952e..5cd195fabca8 100644
--- a/drivers/acpi/pmic/intel_pmic.h
+++ b/drivers/acpi/pmic/intel_pmic.h
@@ -15,6 +15,8 @@ struct intel_pmic_opregion_data {
 	int (*update_aux)(struct regmap *r, int reg, int raw_temp);
 	int (*get_policy)(struct regmap *r, int reg, int bit, u64 *value);
 	int (*update_policy)(struct regmap *r, int reg, int bit, int enable);
+	int (*exec_mipi_pmic_seq_element)(struct regmap *r, u16 i2c_address,
+					  u32 reg_address, u32 value, u32 mask);
 	struct pmic_table *power_table;
 	int power_table_count;
 	struct pmic_table *thermal_table;
diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
index ed1dfba5e5f9..bfecd6bd4990 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -26,4 +26,7 @@ struct intel_soc_pmic {
 	struct device *dev;
 };
 
+int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
+					      u32 value, u32 mask);
+
 #endif	/* __INTEL_SOC_PMIC_H__ */
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 2/4] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2019-01-07 11:15 [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements Hans de Goede
  2019-01-07 11:15 ` [PATCH v6 1/4] ACPI / PMIC: Add support for executing " Hans de Goede
@ 2019-01-07 11:15 ` Hans de Goede
  2019-01-07 15:38   ` Andy Shevchenko
  2019-01-07 11:15 ` [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling Hans de Goede
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2019-01-07 11:15 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Rafael J . Wysocki,
	Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: intel-gfx, linux-acpi

Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove
PMIC.

On some CHT devices this fixes the LCD panel not lighting up when it was
not initialized by the GOP, because an external monitor was plugged in and
the GOP initialized only the external monitor.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v4:
-The decoding of the raw data of the PMIC MIPI sequence element is now done
 in our caller, so drop this and adjust the callback prototype to accept
 the decoded addresses, value and mask

Changes in v3:
-Use hex values for out of range checks
-Make intel_cht_wc_exec_mipi_pmic_seq_element return errors

Changes in v2:
-Interpret data passed to the PMIC MIPI elements according to the docs
 instead of my own reverse engineered interpretation
---
 drivers/acpi/pmic/intel_pmic_chtwc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
index 078b0448f30a..7ffd5624b8e1 100644
--- a/drivers/acpi/pmic/intel_pmic_chtwc.c
+++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
@@ -231,6 +231,24 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg,
 	return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0);
 }
 
+static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
+						   u16 i2c_client_address,
+						   u32 reg_address,
+						   u32 value, u32 mask)
+{
+	u32 address;
+
+	if (i2c_client_address > 0xff || reg_address > 0xff) {
+		pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n",
+			__func__, i2c_client_address, reg_address);
+		return -ERANGE;
+	}
+
+	address = (i2c_client_address << 8) | reg_address;
+
+	return regmap_update_bits(regmap, address, mask, value);
+}
+
 /*
  * The thermal table and ops are empty, we do not support the Thermal opregion
  * (DPTF) due to lacking documentation.
@@ -238,6 +256,7 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg,
 static struct intel_pmic_opregion_data intel_cht_wc_pmic_opregion_data = {
 	.get_power		= intel_cht_wc_pmic_get_power,
 	.update_power		= intel_cht_wc_pmic_update_power,
+	.exec_mipi_pmic_seq_element = intel_cht_wc_exec_mipi_pmic_seq_element,
 	.power_table		= power_table,
 	.power_table_count	= ARRAY_SIZE(power_table),
 };
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-07 11:15 [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements Hans de Goede
  2019-01-07 11:15 ` [PATCH v6 1/4] ACPI / PMIC: Add support for executing " Hans de Goede
  2019-01-07 11:15 ` [PATCH v6 2/4] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
@ 2019-01-07 11:15 ` Hans de Goede
  2019-01-07 15:31   ` Ville Syrjälä
  2019-01-07 15:46   ` Andy Shevchenko
  2019-01-07 11:15 ` [PATCH v6 4/4] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Hans de Goede @ 2019-01-07 11:15 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Rafael J . Wysocki,
	Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: intel-gfx, linux-acpi

Most PMIC-s use only a single i2c-address, so after verifying the
i2c-address matches, we can simply pass the call to regmap_update_bits.

This commit adds support for this and hooks this up for the xpower AXP288
PMIC by setting the new pmic_i2c_address field.

This fixes the following errors on display on / off on a Jumper Ezpad
mini 3 and an Onda V80 plus tablet, both of which use the AXP288:

intel_soc_pmic_exec_mipi_pmic_seq_element: Not implemented
intel_soc_pmic_exec_mipi_pmic_seq_element: i2c-addr: 0x34 reg-addr ...
[drm:mipi_exec_pmic [i915]] *ERROR* mipi_exec_pmic failed, error: -95

Instead of these errors on both devices we now correctly turn on / off
DLDO3 (through direct register manipulation). On the Onda V80 plus this
fixes an issue with the backlight being brighter around the borders after
an off / on cycle. This should also help to save some power when the
display is off.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v6:
-This is a new patch in v6 of this patch-set
---
 drivers/acpi/pmic/intel_pmic.c        | 9 +++++++++
 drivers/acpi/pmic/intel_pmic.h        | 2 ++
 drivers/acpi/pmic/intel_pmic_xpower.c | 1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index 471afeea87c2..c14cfaea92e2 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -359,6 +359,15 @@ int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
 		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
 						    i2c_address, reg_address,
 						    value, mask);
+	} else if (d->pmic_i2c_address) {
+		if (i2c_address == d->pmic_i2c_address) {
+			ret = regmap_update_bits(intel_pmic_opregion->regmap,
+						 reg_address, mask, value);
+		} else {
+			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
+			       __func__, i2c_address, reg_address, value, mask);
+			ret = -ENXIO;
+		}
 	} else {
 		pr_warn("%s: Not implemented\n", __func__);
 		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
index 5cd195fabca8..89379476a1df 100644
--- a/drivers/acpi/pmic/intel_pmic.h
+++ b/drivers/acpi/pmic/intel_pmic.h
@@ -21,6 +21,8 @@ struct intel_pmic_opregion_data {
 	int power_table_count;
 	struct pmic_table *thermal_table;
 	int thermal_table_count;
+	/* For generic exec_mipi_pmic_seq_element handling */
+	int pmic_i2c_address;
 };
 
 int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle, struct regmap *regmap, struct intel_pmic_opregion_data *d);
diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index e7c0006e6602..a091d5a8392c 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -265,6 +265,7 @@ static struct intel_pmic_opregion_data intel_xpower_pmic_opregion_data = {
 	.power_table_count = ARRAY_SIZE(power_table),
 	.thermal_table = thermal_table,
 	.thermal_table_count = ARRAY_SIZE(thermal_table),
+	.pmic_i2c_address = 0x34,
 };
 
 static acpi_status intel_xpower_pmic_gpio_handler(u32 function,
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 4/4] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
  2019-01-07 11:15 [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements Hans de Goede
                   ` (2 preceding siblings ...)
  2019-01-07 11:15 ` [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling Hans de Goede
@ 2019-01-07 11:15 ` Hans de Goede
  2019-01-07 15:31   ` Ville Syrjälä
  2019-01-07 12:47 ` ✓ Fi.CI.BAT: success for ACPI/i915: Add support for PMIC MIPI sequence elements (rev2) Patchwork
  2019-01-07 16:19 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2019-01-07 11:15 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Rafael J . Wysocki,
	Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: intel-gfx, linux-acpi

Add support for PMIC MIPI sequences using the new
intel_soc_pmic_exec_mipi_pmic_seq_element function.

This fixes the DSI LCD panel not lighting up when not initialized by the
GOP (because an external monitor was connected) on GPD win and GPD pocket
devices.

Specifically the LCD panel seems to need GPIO pin 9 on the PMIC to be
driven high, which is done through a PMIC MIPI sequence. Before this commit
if the sequence was not executed by the GOP the pin would stay low causing
the LCD panel to not work. Having the MIPI sequences properly control this
GPIO should also help save some power when the panel is off.

Changes in v2, v3:
-Only changes to other patches in this patch-set

Changes in v4:
-Move decoding of the raw 15 bytes PMIC MIPI sequence element into
 i2c-address, register-address, value and mask into the mipi_exec_pmic()
 function instead of passing the raw data to
 intel_soc_pmic_exec_mipi_pmic_seq_element()

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index 3d1fa1a03a66..240664ef294c 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -29,9 +29,11 @@
 #include <drm/drm_edid.h>
 #include <drm/i915_drm.h>
 #include <linux/gpio/consumer.h>
+#include <linux/mfd/intel_soc_pmic.h>
 #include <linux/slab.h>
 #include <video/mipi_display.h>
 #include <asm/intel-mid.h>
+#include <asm/unaligned.h>
 #include "i915_drv.h"
 #include "intel_drv.h"
 #include "intel_dsi.h"
@@ -392,7 +394,25 @@ static const u8 *mipi_exec_spi(struct intel_dsi *intel_dsi, const u8 *data)
 
 static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
 {
-	DRM_DEBUG_KMS("Skipping PMIC element execution\n");
+#ifdef CONFIG_PMIC_OPREGION
+	u32 value, mask, reg_address;
+	u16 i2c_address;
+	int ret;
+
+	/* byte 0 aka PMIC Flag is reserved */
+	i2c_address	= get_unaligned_le16(data + 1);
+	reg_address	= get_unaligned_le32(data + 3);
+	value		= get_unaligned_le32(data + 7);
+	mask		= get_unaligned_le32(data + 11);
+
+	ret = intel_soc_pmic_exec_mipi_pmic_seq_element(i2c_address,
+							reg_address,
+							value, mask);
+	if (ret)
+		DRM_ERROR("%s failed, error: %d\n", __func__, ret);
+#else
+	DRM_ERROR("Your hardware requires CONFIG_PMIC_OPREGION and it is not set\n");
+#endif
 
 	return data + 15;
 }
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* ✓ Fi.CI.BAT: success for ACPI/i915: Add support for PMIC MIPI sequence elements (rev2)
  2019-01-07 11:15 [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements Hans de Goede
                   ` (3 preceding siblings ...)
  2019-01-07 11:15 ` [PATCH v6 4/4] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
@ 2019-01-07 12:47 ` Patchwork
  2019-01-07 16:19 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2019-01-07 12:47 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: ACPI/i915: Add support for PMIC MIPI sequence elements (rev2)
URL   : https://patchwork.freedesktop.org/series/54050/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5368 -> Patchwork_11199
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11199 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11199, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54050/revisions/2/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11199:

### IGT changes ###

#### Warnings ####

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       PASS -> SKIP +2

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - fi-kbl-7567u:       SKIP -> PASS +33

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - fi-hsw-4770:        PASS -> SKIP +4

  
Known issues
------------

  Here are the changes found in Patchwork_11199 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362] +1

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@pm_rpm@module-reload:
    - fi-icl-u2:          DMESG-WARN [fdo#108654] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915


Participating hosts (47 -> 45)
------------------------------

  Additional (3): fi-icl-y fi-byt-j1900 fi-icl-u3 
  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * Linux: CI_DRM_5368 -> Patchwork_11199

  CI_DRM_5368: 64bd30ea3ce0edd057a5b393569947a955472757 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11199: 901d195444787d9174b6f5c8c18e44ed0adbd60b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

901d19544478 drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
477f2670daa3 ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
3283d4be483f ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
5e6bd13cdb2b ACPI / PMIC: Add support for executing PMIC MIPI sequence elements

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11199/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 4/4] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
  2019-01-07 11:15 ` [PATCH v6 4/4] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
@ 2019-01-07 15:31   ` Ville Syrjälä
  2019-01-09  9:40     ` Hans de Goede
  0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2019-01-07 15:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Andy Shevchenko, Mika Westerberg, Len Brown

On Mon, Jan 07, 2019 at 12:15:56PM +0100, Hans de Goede wrote:
> Add support for PMIC MIPI sequences using the new
> intel_soc_pmic_exec_mipi_pmic_seq_element function.
> 
> This fixes the DSI LCD panel not lighting up when not initialized by the
> GOP (because an external monitor was connected) on GPD win and GPD pocket
> devices.
> 
> Specifically the LCD panel seems to need GPIO pin 9 on the PMIC to be
> driven high, which is done through a PMIC MIPI sequence. Before this commit
> if the sequence was not executed by the GOP the pin would stay low causing
> the LCD panel to not work. Having the MIPI sequences properly control this
> GPIO should also help save some power when the panel is off.
> 
> Changes in v2, v3:
> -Only changes to other patches in this patch-set
> 
> Changes in v4:
> -Move decoding of the raw 15 bytes PMIC MIPI sequence element into
>  i2c-address, register-address, value and mask into the mipi_exec_pmic()
>  function instead of passing the raw data to
>  intel_soc_pmic_exec_mipi_pmic_seq_element()
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index 3d1fa1a03a66..240664ef294c 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -29,9 +29,11 @@
>  #include <drm/drm_edid.h>
>  #include <drm/i915_drm.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/mfd/intel_soc_pmic.h>
>  #include <linux/slab.h>
>  #include <video/mipi_display.h>
>  #include <asm/intel-mid.h>
> +#include <asm/unaligned.h>
>  #include "i915_drv.h"
>  #include "intel_drv.h"
>  #include "intel_dsi.h"
> @@ -392,7 +394,25 @@ static const u8 *mipi_exec_spi(struct intel_dsi *intel_dsi, const u8 *data)
>  
>  static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
>  {
> -	DRM_DEBUG_KMS("Skipping PMIC element execution\n");
> +#ifdef CONFIG_PMIC_OPREGION
> +	u32 value, mask, reg_address;
> +	u16 i2c_address;
> +	int ret;
> +
> +	/* byte 0 aka PMIC Flag is reserved */
> +	i2c_address	= get_unaligned_le16(data + 1);
> +	reg_address	= get_unaligned_le32(data + 3);
> +	value		= get_unaligned_le32(data + 7);
> +	mask		= get_unaligned_le32(data + 11);
> +
> +	ret = intel_soc_pmic_exec_mipi_pmic_seq_element(i2c_address,
> +							reg_address,
> +							value, mask);
> +	if (ret)
> +		DRM_ERROR("%s failed, error: %d\n", __func__, ret);
> +#else
> +	DRM_ERROR("Your hardware requires CONFIG_PMIC_OPREGION and it is not set\n");
> +#endif
>  
>  	return data + 15;
>  }
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-07 11:15 ` [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling Hans de Goede
@ 2019-01-07 15:31   ` Ville Syrjälä
  2019-01-07 15:46   ` Andy Shevchenko
  1 sibling, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2019-01-07 15:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Andy Shevchenko, Mika Westerberg, Len Brown

On Mon, Jan 07, 2019 at 12:15:55PM +0100, Hans de Goede wrote:
> Most PMIC-s use only a single i2c-address, so after verifying the
> i2c-address matches, we can simply pass the call to regmap_update_bits.
> 
> This commit adds support for this and hooks this up for the xpower AXP288
> PMIC by setting the new pmic_i2c_address field.
> 
> This fixes the following errors on display on / off on a Jumper Ezpad
> mini 3 and an Onda V80 plus tablet, both of which use the AXP288:
> 
> intel_soc_pmic_exec_mipi_pmic_seq_element: Not implemented
> intel_soc_pmic_exec_mipi_pmic_seq_element: i2c-addr: 0x34 reg-addr ...
> [drm:mipi_exec_pmic [i915]] *ERROR* mipi_exec_pmic failed, error: -95
> 
> Instead of these errors on both devices we now correctly turn on / off
> DLDO3 (through direct register manipulation). On the Onda V80 plus this
> fixes an issue with the backlight being brighter around the borders after
> an off / on cycle. This should also help to save some power when the
> display is off.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v6:
> -This is a new patch in v6 of this patch-set
> ---
>  drivers/acpi/pmic/intel_pmic.c        | 9 +++++++++
>  drivers/acpi/pmic/intel_pmic.h        | 2 ++
>  drivers/acpi/pmic/intel_pmic_xpower.c | 1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index 471afeea87c2..c14cfaea92e2 100644
> --- a/drivers/acpi/pmic/intel_pmic.c
> +++ b/drivers/acpi/pmic/intel_pmic.c
> @@ -359,6 +359,15 @@ int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
>  		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
>  						    i2c_address, reg_address,
>  						    value, mask);
> +	} else if (d->pmic_i2c_address) {
> +		if (i2c_address == d->pmic_i2c_address) {
> +			ret = regmap_update_bits(intel_pmic_opregion->regmap,
> +						 reg_address, mask, value);
> +		} else {
> +			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
> +			       __func__, i2c_address, reg_address, value, mask);
> +			ret = -ENXIO;
> +		}
>  	} else {
>  		pr_warn("%s: Not implemented\n", __func__);
>  		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
> diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
> index 5cd195fabca8..89379476a1df 100644
> --- a/drivers/acpi/pmic/intel_pmic.h
> +++ b/drivers/acpi/pmic/intel_pmic.h
> @@ -21,6 +21,8 @@ struct intel_pmic_opregion_data {
>  	int power_table_count;
>  	struct pmic_table *thermal_table;
>  	int thermal_table_count;
> +	/* For generic exec_mipi_pmic_seq_element handling */
> +	int pmic_i2c_address;
>  };
>  
>  int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle, struct regmap *regmap, struct intel_pmic_opregion_data *d);
> diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
> index e7c0006e6602..a091d5a8392c 100644
> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> @@ -265,6 +265,7 @@ static struct intel_pmic_opregion_data intel_xpower_pmic_opregion_data = {
>  	.power_table_count = ARRAY_SIZE(power_table),
>  	.thermal_table = thermal_table,
>  	.thermal_table_count = ARRAY_SIZE(thermal_table),
> +	.pmic_i2c_address = 0x34,

Seems to match the axp288 datasheet.

FWIW
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  };
>  
>  static acpi_status intel_xpower_pmic_gpio_handler(u32 function,
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 1/4] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
  2019-01-07 11:15 ` [PATCH v6 1/4] ACPI / PMIC: Add support for executing " Hans de Goede
@ 2019-01-07 15:35   ` Andy Shevchenko
  2019-01-08 13:40     ` Hans de Goede
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2019-01-07 15:35 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

On Mon, Jan 07, 2019 at 12:15:53PM +0100, Hans de Goede wrote:
> DSI LCD panels describe an initialization sequence in the Video BIOS
> Tables using so called MIPI sequences. One possible element in these
> sequences is a PMIC specific element of 15 bytes.
> 
> Although this is not really an ACPI opregion, the ACPI opregion code is the
> closest thing we have. We need to have support for these PMIC specific MIPI
> sequence elements somwhere. Since we already instantiate a special platform
> device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
> with PMIC specific implementations of the OpRegion, the handling of MIPI
> sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.
> 
> This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
> function, which is to be backed by a PMIC specific
> exec_mipi_pmic_seq_element callback. This function will be called by the
> i915 code to execture MIPI sequence PMIC elements.

> +/**
> + * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence

I wonder if we need pmic duplication in the name.

> + * @i2c_address:  I2C client address for the PMIC
> + * @reg_address:  PMIC register address
> + * @value:        New value for the register bits to change
> + * @mask:         Mask indicating which register bits to change
> + *
> + * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
> + * BIOS Tables) using so called MIPI sequences. One possible element in these
> + * sequences is a PMIC specific element of 15 bytes.
> + *
> + * This function executes these PMIC specific elements sending the embedded
> + * commands to the PMIC.
> + *
> + * Return 0 on success, < 0 on failure.
> + */
> +int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
> +					      u32 value, u32 mask)
> +{
> +	struct intel_pmic_opregion_data *d;
> +	int ret;
> +
> +	if (!intel_pmic_opregion) {
> +		pr_warn("%s: No PMIC registered\n", __func__);
> +		return -ENXIO;
> +	}
> +
> +	d = intel_pmic_opregion->data;
> +
> +	mutex_lock(&intel_pmic_opregion->lock);
> +
> +	if (d->exec_mipi_pmic_seq_element) {

> +		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
> +						    i2c_address, reg_address,
> +						    value, mask);

Here it's not quite a dup, but it's implied by the name of structure...

> +	} else {
> +		pr_warn("%s: Not implemented\n", __func__);
> +		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
> +			__func__, i2c_address, reg_address, value, mask);
> +		ret = -EOPNOTSUPP;
> +	}
> +
> +	mutex_unlock(&intel_pmic_opregion->lock);
> +
> +	return ret;
> +}

-- 
With Best Regards,
Andy Shevchenko


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 2/4] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2019-01-07 11:15 ` [PATCH v6 2/4] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
@ 2019-01-07 15:38   ` Andy Shevchenko
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2019-01-07 15:38 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

On Mon, Jan 07, 2019 at 12:15:54PM +0100, Hans de Goede wrote:
> Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove
> PMIC.
> 
> On some CHT devices this fixes the LCD panel not lighting up when it was
> not initialized by the GOP, because an external monitor was plugged in and
> the GOP initialized only the external monitor.

> +	if (i2c_client_address > 0xff || reg_address > 0xff) {
> +		pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n",
> +			__func__, i2c_client_address, reg_address);

I tried to parse the message and failed.
Perhaps something like
"%s: addresses too big for client 0x%x, reg 0x%x\n"
would be easier to understand?

> +		return -ERANGE;
> +	}

-- 
With Best Regards,
Andy Shevchenko


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-07 11:15 ` [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling Hans de Goede
  2019-01-07 15:31   ` Ville Syrjälä
@ 2019-01-07 15:46   ` Andy Shevchenko
  2019-01-08 13:45     ` Hans de Goede
  1 sibling, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2019-01-07 15:46 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

On Mon, Jan 07, 2019 at 12:15:55PM +0100, Hans de Goede wrote:
> Most PMIC-s use only a single i2c-address, so after verifying the
> i2c-address matches, we can simply pass the call to regmap_update_bits.
> 
> This commit adds support for this and hooks this up for the xpower AXP288
> PMIC by setting the new pmic_i2c_address field.
> 
> This fixes the following errors on display on / off on a Jumper Ezpad
> mini 3 and an Onda V80 plus tablet, both of which use the AXP288:
> 
> intel_soc_pmic_exec_mipi_pmic_seq_element: Not implemented
> intel_soc_pmic_exec_mipi_pmic_seq_element: i2c-addr: 0x34 reg-addr ...
> [drm:mipi_exec_pmic [i915]] *ERROR* mipi_exec_pmic failed, error: -95
> 
> Instead of these errors on both devices we now correctly turn on / off
> DLDO3 (through direct register manipulation). On the Onda V80 plus this
> fixes an issue with the backlight being brighter around the borders after
> an off / on cycle. This should also help to save some power when the
> display is off.

> +	} else if (d->pmic_i2c_address) {
> +		if (i2c_address == d->pmic_i2c_address) {
> +			ret = regmap_update_bits(intel_pmic_opregion->regmap,
> +						 reg_address, mask, value);
> +		} else {
> +			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
> +			       __func__, i2c_address, reg_address, value, mask);
> +			ret = -ENXIO;
> +		}

> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> +	.pmic_i2c_address = 0x34,

Can we just have a hook here instead of exposing PMIC I2C address?
Am I missing something in case it's not possible?

-- 
With Best Regards,
Andy Shevchenko


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* ✓ Fi.CI.IGT: success for ACPI/i915: Add support for PMIC MIPI sequence elements (rev2)
  2019-01-07 11:15 [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements Hans de Goede
                   ` (4 preceding siblings ...)
  2019-01-07 12:47 ` ✓ Fi.CI.BAT: success for ACPI/i915: Add support for PMIC MIPI sequence elements (rev2) Patchwork
@ 2019-01-07 16:19 ` Patchwork
  5 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2019-01-07 16:19 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: ACPI/i915: Add support for PMIC MIPI sequence elements (rev2)
URL   : https://patchwork.freedesktop.org/series/54050/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5368_full -> Patchwork_11199_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11199_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11199_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11199_full:

### IGT changes ###

#### Warnings ####

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          SKIP -> PASS

  
Known issues
------------

  Here are the changes found in Patchwork_11199_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-internal-immediate:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@gem_exec_schedule@pi-ringfull-vebox:
    - shard-skl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_exec_whisper@normal:
    - shard-skl:          PASS -> TIMEOUT [fdo#108592]

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-skl:          NOTRUN -> TIMEOUT [fdo#108039]

  * igt@kms_atomic_transition@1x-modeset-transitions:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108470]

  * igt@kms_atomic_transition@plane-all-transition:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#109225]

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956] +1

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956] +4

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-c-bad-aux-stride:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +1

  * igt@kms_chv_cursor_fail@pipe-b-128x128-top-edge:
    - shard-skl:          NOTRUN -> FAIL [fdo#104671]

  * igt@kms_chv_cursor_fail@pipe-b-64x64-left-edge:
    - shard-skl:          PASS -> FAIL [fdo#104671]

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-snb:          PASS -> DMESG-WARN [fdo#102365] / [fdo#109241]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103232]

  * igt@kms_draw_crc@draw-method-rgb565-render-xtiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          NOTRUN -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-apl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - shard-skl:          NOTRUN -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane@plane-panning-bottom-right-pipe-b-planes:
    - shard-skl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#107724]

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-skl:          NOTRUN -> FAIL [fdo#103166] / [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_rotation_crc@primary-y-tiled-reflect-x-0:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +2

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@pm_rpm@debugfs-read:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@legacy-planes-dpms:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@universal-planes-dpms:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#108654] +1

  * igt@pm_rps@min-max-config-loaded:
    - shard-glk:          PASS -> FAIL [fdo#102250]

  
#### Possible fixes ####

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-skl:          TIMEOUT [fdo#108887] -> PASS

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          FAIL [fdo#106641] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-snb:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-sliding:
    - shard-skl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1
    - shard-glk:          FAIL [fdo#103167] -> PASS +5

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +1

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - shard-skl:          FAIL [fdo#107362] -> PASS

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS

  * igt@kms_plane@plane-panning-top-left-pipe-a-planes:
    - shard-skl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-iclb:         FAIL [fdo#105010] -> PASS

  * igt@pm_rpm@dpms-non-lpsp:
    - shard-kbl:          DMESG-WARN [fdo#103313] / [fdo#105345] -> PASS +1

  * igt@pm_rpm@modeset-lpsp-stress:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@pm_rpm@system-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] / [fdo#107807] -> PASS

  * igt@pm_rps@min-max-config-loaded:
    - shard-skl:          FAIL [fdo#102250] -> PASS

  
#### Warnings ####

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-iclb:         FAIL [fdo#103232] -> INCOMPLETE [fdo#107713]

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-iclb:         FAIL [fdo#107882] -> DMESG-FAIL [fdo#107724]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
  [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105345]: https://bugs.freedesktop.org/show_bug.cgi?id=105345
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107882]: https://bugs.freedesktop.org/show_bug.cgi?id=107882
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
  [fdo#108592]: https://bugs.freedesktop.org/show_bug.cgi?id=108592
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108756]: https://bugs.freedesktop.org/show_bug.cgi?id=108756
  [fdo#108887]: https://bugs.freedesktop.org/show_bug.cgi?id=108887
  [fdo#109225]: https://bugs.freedesktop.org/show_bug.cgi?id=109225
  [fdo#109241]: https://bugs.freedesktop.org/show_bug.cgi?id=109241
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5368 -> Patchwork_11199

  CI_DRM_5368: 64bd30ea3ce0edd057a5b393569947a955472757 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11199: 901d195444787d9174b6f5c8c18e44ed0adbd60b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11199/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 1/4] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
  2019-01-07 15:35   ` Andy Shevchenko
@ 2019-01-08 13:40     ` Hans de Goede
  0 siblings, 0 replies; 19+ messages in thread
From: Hans de Goede @ 2019-01-08 13:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

Hi,

On 07-01-19 16:35, Andy Shevchenko wrote:
> On Mon, Jan 07, 2019 at 12:15:53PM +0100, Hans de Goede wrote:
>> DSI LCD panels describe an initialization sequence in the Video BIOS
>> Tables using so called MIPI sequences. One possible element in these
>> sequences is a PMIC specific element of 15 bytes.
>>
>> Although this is not really an ACPI opregion, the ACPI opregion code is the
>> closest thing we have. We need to have support for these PMIC specific MIPI
>> sequence elements somwhere. Since we already instantiate a special platform
>> device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
>> with PMIC specific implementations of the OpRegion, the handling of MIPI
>> sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.
>>
>> This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
>> function, which is to be backed by a PMIC specific
>> exec_mipi_pmic_seq_element callback. This function will be called by the
>> i915 code to execture MIPI sequence PMIC elements.
> 
>> +/**
>> + * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence
> 
> I wonder if we need pmic duplication in the name.

Mipi sequences can do a bunch of things, talk to the panel over
the MIPI bus, set GPIOs, read-modify-write PMIC registers, etc.

This function can only execute the PMIC bits, not the rest,
so the second pmic is there to indiciate we are executing
a PMIC MIPI sequence and not e.g. a GPIO one. So I believe that
keeping this as is is best.

Regards,

Hans


>> + * @i2c_address:  I2C client address for the PMIC
>> + * @reg_address:  PMIC register address
>> + * @value:        New value for the register bits to change
>> + * @mask:         Mask indicating which register bits to change
>> + *
>> + * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
>> + * BIOS Tables) using so called MIPI sequences. One possible element in these
>> + * sequences is a PMIC specific element of 15 bytes.
>> + *
>> + * This function executes these PMIC specific elements sending the embedded
>> + * commands to the PMIC.
>> + *
>> + * Return 0 on success, < 0 on failure.
>> + */
>> +int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
>> +					      u32 value, u32 mask)
>> +{
>> +	struct intel_pmic_opregion_data *d;
>> +	int ret;
>> +
>> +	if (!intel_pmic_opregion) {
>> +		pr_warn("%s: No PMIC registered\n", __func__);
>> +		return -ENXIO;
>> +	}
>> +
>> +	d = intel_pmic_opregion->data;
>> +
>> +	mutex_lock(&intel_pmic_opregion->lock);
>> +
>> +	if (d->exec_mipi_pmic_seq_element) {
> 
>> +		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
>> +						    i2c_address, reg_address,
>> +						    value, mask);
> 
> Here it's not quite a dup, but it's implied by the name of structure...
> 
>> +	} else {
>> +		pr_warn("%s: Not implemented\n", __func__);
>> +		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
>> +			__func__, i2c_address, reg_address, value, mask);
>> +		ret = -EOPNOTSUPP;
>> +	}
>> +
>> +	mutex_unlock(&intel_pmic_opregion->lock);
>> +
>> +	return ret;
>> +}
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-07 15:46   ` Andy Shevchenko
@ 2019-01-08 13:45     ` Hans de Goede
  2019-01-08 14:51       ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2019-01-08 13:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

Hi,

On 07-01-19 16:46, Andy Shevchenko wrote:
> On Mon, Jan 07, 2019 at 12:15:55PM +0100, Hans de Goede wrote:
>> Most PMIC-s use only a single i2c-address, so after verifying the
>> i2c-address matches, we can simply pass the call to regmap_update_bits.
>>
>> This commit adds support for this and hooks this up for the xpower AXP288
>> PMIC by setting the new pmic_i2c_address field.
>>
>> This fixes the following errors on display on / off on a Jumper Ezpad
>> mini 3 and an Onda V80 plus tablet, both of which use the AXP288:
>>
>> intel_soc_pmic_exec_mipi_pmic_seq_element: Not implemented
>> intel_soc_pmic_exec_mipi_pmic_seq_element: i2c-addr: 0x34 reg-addr ...
>> [drm:mipi_exec_pmic [i915]] *ERROR* mipi_exec_pmic failed, error: -95
>>
>> Instead of these errors on both devices we now correctly turn on / off
>> DLDO3 (through direct register manipulation). On the Onda V80 plus this
>> fixes an issue with the backlight being brighter around the borders after
>> an off / on cycle. This should also help to save some power when the
>> display is off.
> 
>> +	} else if (d->pmic_i2c_address) {
>> +		if (i2c_address == d->pmic_i2c_address) {
>> +			ret = regmap_update_bits(intel_pmic_opregion->regmap,
>> +						 reg_address, mask, value);
>> +		} else {
>> +			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
>> +			       __func__, i2c_address, reg_address, value, mask);
>> +			ret = -ENXIO;
>> +		}
> 
>> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
>> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
>> +	.pmic_i2c_address = 0x34,
> 
> Can we just have a hook here instead of exposing PMIC I2C address?
> Am I missing something in case it's not possible?

We already have a hook, but it isn't really necessary to implement
that for each model PMIC. The MFD device which is the PMIC's parent
in most cases will give us a regmap to access the PMIC registers and
that allows us to do a generic implementation.

But the MIPI PMIC sequence includes an i2c-address as some PMICs
span multiple i2c-addresses. For the simple single i2c-address case
the regmap gives us access to the registers behind that single address
and we can use a generic solution. In this case we should verify the
i2c-addr is what we expect, which is where the pmic_i2c_address comes in.

Regards,

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-08 13:45     ` Hans de Goede
@ 2019-01-08 14:51       ` Andy Shevchenko
  2019-01-08 15:35         ` Hans de Goede
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2019-01-08 14:51 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

On Tue, Jan 08, 2019 at 02:45:39PM +0100, Hans de Goede wrote:
> On 07-01-19 16:46, Andy Shevchenko wrote:
> > On Mon, Jan 07, 2019 at 12:15:55PM +0100, Hans de Goede wrote:

> > > +	} else if (d->pmic_i2c_address) {
> > > +		if (i2c_address == d->pmic_i2c_address) {
> > > +			ret = regmap_update_bits(intel_pmic_opregion->regmap,
> > > +						 reg_address, mask, value);
> > > +		} else {
> > > +			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
> > > +			       __func__, i2c_address, reg_address, value, mask);
> > > +			ret = -ENXIO;
> > > +		}
> > 
> > > --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> > > +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> > > +	.pmic_i2c_address = 0x34,
> > 
> > Can we just have a hook here instead of exposing PMIC I2C address?
> > Am I missing something in case it's not possible?
> 
> We already have a hook, but it isn't really necessary to implement
> that for each model PMIC. The MFD device which is the PMIC's parent
> in most cases will give us a regmap to access the PMIC registers and
> that allows us to do a generic implementation.
> 
> But the MIPI PMIC sequence includes an i2c-address as some PMICs
> span multiple i2c-addresses. For the simple single i2c-address case
> the regmap gives us access to the registers behind that single address
> and we can use a generic solution. In this case we should verify the
> i2c-addr is what we expect, which is where the pmic_i2c_address comes in.

But we also can have a generic hook in intel_pmic.c and assign it whenever
it's the case?

-- 
With Best Regards,
Andy Shevchenko


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-08 14:51       ` Andy Shevchenko
@ 2019-01-08 15:35         ` Hans de Goede
  2019-01-08 17:33           ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2019-01-08 15:35 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

Hi,

On 08-01-19 15:51, Andy Shevchenko wrote:
> On Tue, Jan 08, 2019 at 02:45:39PM +0100, Hans de Goede wrote:
>> On 07-01-19 16:46, Andy Shevchenko wrote:
>>> On Mon, Jan 07, 2019 at 12:15:55PM +0100, Hans de Goede wrote:
> 
>>>> +	} else if (d->pmic_i2c_address) {
>>>> +		if (i2c_address == d->pmic_i2c_address) {
>>>> +			ret = regmap_update_bits(intel_pmic_opregion->regmap,
>>>> +						 reg_address, mask, value);
>>>> +		} else {
>>>> +			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
>>>> +			       __func__, i2c_address, reg_address, value, mask);
>>>> +			ret = -ENXIO;
>>>> +		}
>>>
>>>> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
>>>> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
>>>> +	.pmic_i2c_address = 0x34,
>>>
>>> Can we just have a hook here instead of exposing PMIC I2C address?
>>> Am I missing something in case it's not possible?
>>
>> We already have a hook, but it isn't really necessary to implement
>> that for each model PMIC. The MFD device which is the PMIC's parent
>> in most cases will give us a regmap to access the PMIC registers and
>> that allows us to do a generic implementation.
>>
>> But the MIPI PMIC sequence includes an i2c-address as some PMICs
>> span multiple i2c-addresses. For the simple single i2c-address case
>> the regmap gives us access to the registers behind that single address
>> and we can use a generic solution. In this case we should verify the
>> i2c-addr is what we expect, which is where the pmic_i2c_address comes in.
> 
> But we also can have a generic hook in intel_pmic.c and assign it whenever
> it's the case?

We could, but then we still need the pmic_i2c_address member and now the
hook would need to passed both the regmap and the intel_pmic_opregion_data
pointer so that it can verify the i2c address so handling the generic case
directly inside intel_soc_pmic_exec_mipi_pmic_seq_element is easier.

Regards,

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-08 15:35         ` Hans de Goede
@ 2019-01-08 17:33           ` Andy Shevchenko
  2019-01-09  9:26             ` Hans de Goede
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2019-01-08 17:33 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

On Tue, Jan 08, 2019 at 04:35:45PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 08-01-19 15:51, Andy Shevchenko wrote:
> > On Tue, Jan 08, 2019 at 02:45:39PM +0100, Hans de Goede wrote:
> > > On 07-01-19 16:46, Andy Shevchenko wrote:
> > > > On Mon, Jan 07, 2019 at 12:15:55PM +0100, Hans de Goede wrote:
> > 
> > > > > +	} else if (d->pmic_i2c_address) {
> > > > > +		if (i2c_address == d->pmic_i2c_address) {
> > > > > +			ret = regmap_update_bits(intel_pmic_opregion->regmap,
> > > > > +						 reg_address, mask, value);
> > > > > +		} else {
> > > > > +			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
> > > > > +			       __func__, i2c_address, reg_address, value, mask);
> > > > > +			ret = -ENXIO;
> > > > > +		}
> > > > 
> > > > > --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> > > > > +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> > > > > +	.pmic_i2c_address = 0x34,
> > > > 
> > > > Can we just have a hook here instead of exposing PMIC I2C address?
> > > > Am I missing something in case it's not possible?
> > > 
> > > We already have a hook, but it isn't really necessary to implement
> > > that for each model PMIC. The MFD device which is the PMIC's parent
> > > in most cases will give us a regmap to access the PMIC registers and
> > > that allows us to do a generic implementation.
> > > 
> > > But the MIPI PMIC sequence includes an i2c-address as some PMICs
> > > span multiple i2c-addresses. For the simple single i2c-address case
> > > the regmap gives us access to the registers behind that single address
> > > and we can use a generic solution. In this case we should verify the
> > > i2c-addr is what we expect, which is where the pmic_i2c_address comes in.
> > 
> > But we also can have a generic hook in intel_pmic.c and assign it whenever
> > it's the case?
> 
> We could, but then we still need the pmic_i2c_address member and now the
> hook would need to passed both the regmap and the intel_pmic_opregion_data
> pointer so that it can verify the i2c address so handling the generic case
> directly inside intel_soc_pmic_exec_mipi_pmic_seq_element is easier.

I see.

One more question, can we unify somehow error messages and do something like

if (...) {
	...
} else if (pmic_i2c_address && i2c_address == pmic_i2c_address) {
	ret = regmap_update_bits(...);
} else {
	...
}

?

-- 
With Best Regards,
Andy Shevchenko


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling
  2019-01-08 17:33           ` Andy Shevchenko
@ 2019-01-09  9:26             ` Hans de Goede
  0 siblings, 0 replies; 19+ messages in thread
From: Hans de Goede @ 2019-01-09  9:26 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Mika Westerberg, Len Brown

Hi,

On 08-01-19 18:33, Andy Shevchenko wrote:
> On Tue, Jan 08, 2019 at 04:35:45PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 08-01-19 15:51, Andy Shevchenko wrote:
>>> On Tue, Jan 08, 2019 at 02:45:39PM +0100, Hans de Goede wrote:
>>>> On 07-01-19 16:46, Andy Shevchenko wrote:
>>>>> On Mon, Jan 07, 2019 at 12:15:55PM +0100, Hans de Goede wrote:
>>>
>>>>>> +	} else if (d->pmic_i2c_address) {
>>>>>> +		if (i2c_address == d->pmic_i2c_address) {
>>>>>> +			ret = regmap_update_bits(intel_pmic_opregion->regmap,
>>>>>> +						 reg_address, mask, value);
>>>>>> +		} else {
>>>>>> +			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
>>>>>> +			       __func__, i2c_address, reg_address, value, mask);
>>>>>> +			ret = -ENXIO;
>>>>>> +		}
>>>>>
>>>>>> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
>>>>>> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
>>>>>> +	.pmic_i2c_address = 0x34,
>>>>>
>>>>> Can we just have a hook here instead of exposing PMIC I2C address?
>>>>> Am I missing something in case it's not possible?
>>>>
>>>> We already have a hook, but it isn't really necessary to implement
>>>> that for each model PMIC. The MFD device which is the PMIC's parent
>>>> in most cases will give us a regmap to access the PMIC registers and
>>>> that allows us to do a generic implementation.
>>>>
>>>> But the MIPI PMIC sequence includes an i2c-address as some PMICs
>>>> span multiple i2c-addresses. For the simple single i2c-address case
>>>> the regmap gives us access to the registers behind that single address
>>>> and we can use a generic solution. In this case we should verify the
>>>> i2c-addr is what we expect, which is where the pmic_i2c_address comes in.
>>>
>>> But we also can have a generic hook in intel_pmic.c and assign it whenever
>>> it's the case?
>>
>> We could, but then we still need the pmic_i2c_address member and now the
>> hook would need to passed both the regmap and the intel_pmic_opregion_data
>> pointer so that it can verify the i2c address so handling the generic case
>> directly inside intel_soc_pmic_exec_mipi_pmic_seq_element is easier.
> 
> I see.
> 
> One more question, can we unify somehow error messages and do something like
> 
> if (...) {
> 	...
> } else if (pmic_i2c_address && i2c_address == pmic_i2c_address) {
> 	ret = regmap_update_bits(...);
> } else {
> 	...
> }

I can understand where you are coming from with this request but I would
prefer to keep the messages separate, merging them doing something like this:

if (...) {
  	...
} else if (pmic_i2c_address && i2c_address == pmic_i2c_address) {
  	ret = regmap_update_bits(...);
} else {
  	...
}

if (ret)
	pr_err()

Would mean that the messages become less clear and the user would need to
go by the error code to figure out what is going on. Also currently one
of the 2 messages this would merge is a pr_err, while the other is a pr_warn.

I hope that the clear error messages lead to clear bug reports (or help
the user over the threshold to report a bug at all when they are hit).

Regards,

Hans



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 4/4] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
  2019-01-07 15:31   ` Ville Syrjälä
@ 2019-01-09  9:40     ` Hans de Goede
  0 siblings, 0 replies; 19+ messages in thread
From: Hans de Goede @ 2019-01-09  9:40 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, Rodrigo Vivi,
	Andy Shevchenko, Mika Westerberg, Len Brown

Hi,

On 07-01-19 16:31, Ville Syrjälä wrote:
> On Mon, Jan 07, 2019 at 12:15:56PM +0100, Hans de Goede wrote:
>> Add support for PMIC MIPI sequences using the new
>> intel_soc_pmic_exec_mipi_pmic_seq_element function.
>>
>> This fixes the DSI LCD panel not lighting up when not initialized by the
>> GOP (because an external monitor was connected) on GPD win and GPD pocket
>> devices.
>>
>> Specifically the LCD panel seems to need GPIO pin 9 on the PMIC to be
>> driven high, which is done through a PMIC MIPI sequence. Before this commit
>> if the sequence was not executed by the GOP the pin would stay low causing
>> the LCD panel to not work. Having the MIPI sequences properly control this
>> GPIO should also help save some power when the panel is off.
>>
>> Changes in v2, v3:
>> -Only changes to other patches in this patch-set
>>
>> Changes in v4:
>> -Move decoding of the raw 15 bytes PMIC MIPI sequence element into
>>   i2c-address, register-address, value and mask into the mipi_exec_pmic()
>>   function instead of passing the raw data to
>>   intel_soc_pmic_exec_mipi_pmic_seq_element()
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thank you.

I've pushed this series to drm-intel-next-queued now.

Regards,

Hans



> 
>> ---
>>   drivers/gpu/drm/i915/intel_dsi_vbt.c | 22 +++++++++++++++++++++-
>>   1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> index 3d1fa1a03a66..240664ef294c 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> @@ -29,9 +29,11 @@
>>   #include <drm/drm_edid.h>
>>   #include <drm/i915_drm.h>
>>   #include <linux/gpio/consumer.h>
>> +#include <linux/mfd/intel_soc_pmic.h>
>>   #include <linux/slab.h>
>>   #include <video/mipi_display.h>
>>   #include <asm/intel-mid.h>
>> +#include <asm/unaligned.h>
>>   #include "i915_drv.h"
>>   #include "intel_drv.h"
>>   #include "intel_dsi.h"
>> @@ -392,7 +394,25 @@ static const u8 *mipi_exec_spi(struct intel_dsi *intel_dsi, const u8 *data)
>>   
>>   static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
>>   {
>> -	DRM_DEBUG_KMS("Skipping PMIC element execution\n");
>> +#ifdef CONFIG_PMIC_OPREGION
>> +	u32 value, mask, reg_address;
>> +	u16 i2c_address;
>> +	int ret;
>> +
>> +	/* byte 0 aka PMIC Flag is reserved */
>> +	i2c_address	= get_unaligned_le16(data + 1);
>> +	reg_address	= get_unaligned_le32(data + 3);
>> +	value		= get_unaligned_le32(data + 7);
>> +	mask		= get_unaligned_le32(data + 11);
>> +
>> +	ret = intel_soc_pmic_exec_mipi_pmic_seq_element(i2c_address,
>> +							reg_address,
>> +							value, mask);
>> +	if (ret)
>> +		DRM_ERROR("%s failed, error: %d\n", __func__, ret);
>> +#else
>> +	DRM_ERROR("Your hardware requires CONFIG_PMIC_OPREGION and it is not set\n");
>> +#endif
>>   
>>   	return data + 15;
>>   }
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-01-09  9:40 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 11:15 [PATCH v6 0/4] ACPI/i915: Add support for PMIC MIPI sequence elements Hans de Goede
2019-01-07 11:15 ` [PATCH v6 1/4] ACPI / PMIC: Add support for executing " Hans de Goede
2019-01-07 15:35   ` Andy Shevchenko
2019-01-08 13:40     ` Hans de Goede
2019-01-07 11:15 ` [PATCH v6 2/4] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
2019-01-07 15:38   ` Andy Shevchenko
2019-01-07 11:15 ` [PATCH v6 3/4] ACPI / PMIC: Add generic intel_soc_pmic_exec_mipi_pmic_seq_element handling Hans de Goede
2019-01-07 15:31   ` Ville Syrjälä
2019-01-07 15:46   ` Andy Shevchenko
2019-01-08 13:45     ` Hans de Goede
2019-01-08 14:51       ` Andy Shevchenko
2019-01-08 15:35         ` Hans de Goede
2019-01-08 17:33           ` Andy Shevchenko
2019-01-09  9:26             ` Hans de Goede
2019-01-07 11:15 ` [PATCH v6 4/4] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
2019-01-07 15:31   ` Ville Syrjälä
2019-01-09  9:40     ` Hans de Goede
2019-01-07 12:47 ` ✓ Fi.CI.BAT: success for ACPI/i915: Add support for PMIC MIPI sequence elements (rev2) Patchwork
2019-01-07 16:19 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.