All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
@ 2018-12-13 11:21 Hans de Goede
  2018-12-13 11:21 ` [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Hans de Goede @ 2018-12-13 11:21 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: Hans de Goede, intel-gfx, dri-devel, 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.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
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     | 42 ++++++++++++++++++++++++++++++
 drivers/acpi/pmic/intel_pmic.h     |  1 +
 include/linux/mfd/intel_soc_pmic.h |  2 ++
 3 files changed, 45 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index ca18e0d23df9..61f99735fd41 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,41 @@ 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
+ * @data: Pointer to *15* byte PMIC MIPI sequence element
+ *
+ * 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(const u8 *data)
+{
+	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;
+	if (!d->exec_mipi_pmic_seq_element) {
+		pr_warn("%s: Not implemented\n", __func__);
+		pr_warn("%s: Data: %15ph\n", __func__, data);
+		return -EOPNOTSUPP;
+	}
+
+	mutex_lock(&intel_pmic_opregion->lock);
+	ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
+	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..1cc5e69af0c3 100644
--- a/drivers/acpi/pmic/intel_pmic.h
+++ b/drivers/acpi/pmic/intel_pmic.h
@@ -15,6 +15,7 @@ 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, const u8 *data);
 	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..71a8607a0a8c 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -26,4 +26,6 @@ struct intel_soc_pmic {
 	struct device *dev;
 };
 
+int intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data);
+
 #endif	/* __INTEL_SOC_PMIC_H__ */
-- 
2.19.2

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

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

* [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2018-12-13 11:21 [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Hans de Goede
@ 2018-12-13 11:21 ` Hans de Goede
  2018-12-13 12:14   ` Ville Syrjälä
  2018-12-13 11:21 ` [PATCH v3 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2018-12-13 11:21 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: Hans de Goede, intel-gfx, dri-devel, 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.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Interpret data passed to the PMIC MIPI elements according to the docs
 instead of my own reverse engineered interpretation
Changes in v3:
-Use hex values for out of range checks
-Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
---
 drivers/acpi/pmic/intel_pmic_chtwc.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
index 078b0448f30a..8ede74e9b89f 100644
--- a/drivers/acpi/pmic/intel_pmic_chtwc.c
+++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
@@ -12,6 +12,7 @@
 #include <linux/mfd/intel_soc_pmic.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <asm/unaligned.h>
 #include "intel_pmic.h"
 
 #define CHT_WC_V1P05A_CTRL		0x6e3b
@@ -231,6 +232,29 @@ 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,
+						   const u8 *data)
+{
+	u32 value, mask, reg_address, address;
+	u16 i2c_client_address;
+
+	/* byte 0 aka PMIC Flag is reserved */
+	i2c_client_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);
+
+	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 +262,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.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v3 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
  2018-12-13 11:21 [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Hans de Goede
  2018-12-13 11:21 ` [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
@ 2018-12-13 11:21 ` Hans de Goede
  2018-12-13 13:49   ` Jani Nikula
  2018-12-13 12:29 ` ✓ Fi.CI.BAT: success for series starting with [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2018-12-13 11:21 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: Hans de Goede, intel-gfx, dri-devel, 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.

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

diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index f27af47c6e49..6a2ed1ca72e0 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -29,6 +29,7 @@
 #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>
@@ -371,7 +372,11 @@ 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)
 {
+#ifdef CONFIG_PMIC_OPREGION
+	intel_soc_pmic_exec_mipi_pmic_seq_element(data);
+#else
 	DRM_DEBUG_KMS("Skipping PMIC element execution\n");
+#endif
 
 	return data + 15;
 }
-- 
2.19.2

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

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

* Re: [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2018-12-13 11:21 ` [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
@ 2018-12-13 12:14   ` Ville Syrjälä
  2018-12-13 12:40     ` Hans de Goede
  0 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2018-12-13 12:14 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, dri-devel,
	Rodrigo Vivi, Andy Shevchenko, Mika Westerberg, Len Brown

On Thu, Dec 13, 2018 at 12:21:35PM +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.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Interpret data passed to the PMIC MIPI elements according to the docs
>  instead of my own reverse engineered interpretation
> Changes in v3:
> -Use hex values for out of range checks
> -Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
> ---
>  drivers/acpi/pmic/intel_pmic_chtwc.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
> index 078b0448f30a..8ede74e9b89f 100644
> --- a/drivers/acpi/pmic/intel_pmic_chtwc.c
> +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
> @@ -12,6 +12,7 @@
>  #include <linux/mfd/intel_soc_pmic.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> +#include <asm/unaligned.h>
>  #include "intel_pmic.h"
>  
>  #define CHT_WC_V1P05A_CTRL		0x6e3b
> @@ -231,6 +232,29 @@ 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,
> +						   const u8 *data)
> +{
> +	u32 value, mask, reg_address, address;
> +	u16 i2c_client_address;
> +
> +	/* byte 0 aka PMIC Flag is reserved */
> +	i2c_client_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);

Upon further reflection maybe it would better to do this decoding in
the i915 code and just pass each parameter to this hook separately?
That way we wouldn't be spreading the vbt details all over the place.

> +
> +	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 +262,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.19.2

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

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

* ✓ Fi.CI.BAT: success for series starting with [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
  2018-12-13 11:21 [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Hans de Goede
  2018-12-13 11:21 ` [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
  2018-12-13 11:21 ` [PATCH v3 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
@ 2018-12-13 12:29 ` Patchwork
  2018-12-13 13:45 ` [PATCH v3 1/3] " Jani Nikula
  2018-12-13 14:18 ` ✓ Fi.CI.IGT: success for series starting with [v3,1/3] " Patchwork
  4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-12-13 12:29 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
URL   : https://patchwork.freedesktop.org/series/53986/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5311 -> Patchwork_11085
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11085 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11085, 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/53986/revisions/1/mbox/

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

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

### IGT changes ###

#### Warnings ####

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       PASS -> DMESG-WARN [fdo#108965]

  * igt@debugfs_test@read_all_entries:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#103558] / [fdo#105602]

  * igt@gem_exec_suspend@basic-s3:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602]

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#105602] / [fdo#108529] +1

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-kbl-7567u:       PASS -> FAIL [fdo#108767] +2

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-icl-u3:          NOTRUN -> DMESG-WARN [fdo#108924] / [fdo#109044]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-kbl-7567u:       PASS -> DMESG-FAIL [fdo#105079]

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-ilk-650:         PASS -> DMESG-WARN [fdo#106387]

  * igt@pm_rpm@module-reload:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#108529]

  * {igt@runner@aborted}:
    - fi-icl-u3:          NOTRUN -> FAIL [fdo#108924]
    - fi-icl-y:           NOTRUN -> FAIL [fdo#108070]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-kbl-7560u:       INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       DMESG-WARN [fdo#108473] -> PASS

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#108767] -> PASS

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

  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
  [fdo#108473]: https://bugs.freedesktop.org/show_bug.cgi?id=108473
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108924]: https://bugs.freedesktop.org/show_bug.cgi?id=108924
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109044]: https://bugs.freedesktop.org/show_bug.cgi?id=109044


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

  Additional (2): fi-icl-y fi-icl-u3 
  Missing    (4): fi-kbl-soraka fi-ctg-p8600 fi-byt-squawks fi-ilk-m540 


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

    * Linux: CI_DRM_5311 -> Patchwork_11085

  CI_DRM_5311: a42fd8bf199784ee4ff1cdb5ee03eedd9a535d4a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4746: 2c793666d8c8328733f5769b16ae5858fee97f3f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11085: a6a19013e822938da76771d319deefebfc6a5360 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a6a19013e822 drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
b397452a9437 ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
9ca8ee14648f 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_11085/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2018-12-13 12:14   ` Ville Syrjälä
@ 2018-12-13 12:40     ` Hans de Goede
  2018-12-13 13:08       ` Ville Syrjälä
  0 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2018-12-13 12:40 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, dri-devel,
	Rodrigo Vivi, Andy Shevchenko, Mika Westerberg, Len Brown

Hi,

On 13-12-18 13:14, Ville Syrjälä wrote:
> On Thu, Dec 13, 2018 at 12:21:35PM +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.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Interpret data passed to the PMIC MIPI elements according to the docs
>>   instead of my own reverse engineered interpretation
>> Changes in v3:
>> -Use hex values for out of range checks
>> -Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
>> ---
>>   drivers/acpi/pmic/intel_pmic_chtwc.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
>> index 078b0448f30a..8ede74e9b89f 100644
>> --- a/drivers/acpi/pmic/intel_pmic_chtwc.c
>> +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/mfd/intel_soc_pmic.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/regmap.h>
>> +#include <asm/unaligned.h>
>>   #include "intel_pmic.h"
>>   
>>   #define CHT_WC_V1P05A_CTRL		0x6e3b
>> @@ -231,6 +232,29 @@ 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,
>> +						   const u8 *data)
>> +{
>> +	u32 value, mask, reg_address, address;
>> +	u16 i2c_client_address;
>> +
>> +	/* byte 0 aka PMIC Flag is reserved */
>> +	i2c_client_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);
> 
> Upon further reflection maybe it would better to do this decoding in
> the i915 code and just pass each parameter to this hook separately?
> That way we wouldn't be spreading the vbt details all over the place.

Interesting point, if the VBT spec says that this encoding is PMIC
independent, then yes we should probably fo the decoding in the VBT
code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
prototype to:

int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
					      u32 value, u32 mask);

If you agree please let me know and I will do a v4 of the patchset.

I've also been thinking about trying to make the implementation
under drivers/acpi/pmic pmic independent, but not all pmic
drivers use the regmap the same way. The CHT Whiskey Cove PMIC
mfd driver uses a regmap with 16 bit addresses where the upper
byte is the i2c client address and the lower byte is the register
address (this PMIC listens on multiple addresses, with different
registers behind each i2c address).

Where as most PMIC mfd drivers simply use the standard
devm_regmap_init_i2c() method of creating a regmap.  For these
others we could do a standard implementation where we check the
passed in i2c_address is what we expect (for that type PMIC) and
then pass the other 3 parameters to regmap_update_bits.

But I think it would be best to wait with such a generic implementation
until we encounter a device using the PMIC MIPI sequence element
with another type of PMIC.  Since we still need the special
implementation for the CHT WC case, we still need an operation
pointer for this in intel_pmic_opregion_data anyways, so we can
easily plug in the generic implementation for others later.

Regards,

Hans





>  
>> +
>> +	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 +262,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.19.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2018-12-13 12:40     ` Hans de Goede
@ 2018-12-13 13:08       ` Ville Syrjälä
  2018-12-13 13:46         ` Jani Nikula
  2018-12-13 14:21         ` Hans de Goede
  0 siblings, 2 replies; 13+ messages in thread
From: Ville Syrjälä @ 2018-12-13 13:08 UTC (permalink / raw)
  To: Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, dri-devel,
	Rodrigo Vivi, Andy Shevchenko, Mika Westerberg, Len Brown

On Thu, Dec 13, 2018 at 01:40:27PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 13-12-18 13:14, Ville Syrjälä wrote:
> > On Thu, Dec 13, 2018 at 12:21:35PM +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.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >> Changes in v2:
> >> -Interpret data passed to the PMIC MIPI elements according to the docs
> >>   instead of my own reverse engineered interpretation
> >> Changes in v3:
> >> -Use hex values for out of range checks
> >> -Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
> >> ---
> >>   drivers/acpi/pmic/intel_pmic_chtwc.c | 25 +++++++++++++++++++++++++
> >>   1 file changed, 25 insertions(+)
> >>
> >> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
> >> index 078b0448f30a..8ede74e9b89f 100644
> >> --- a/drivers/acpi/pmic/intel_pmic_chtwc.c
> >> +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
> >> @@ -12,6 +12,7 @@
> >>   #include <linux/mfd/intel_soc_pmic.h>
> >>   #include <linux/platform_device.h>
> >>   #include <linux/regmap.h>
> >> +#include <asm/unaligned.h>
> >>   #include "intel_pmic.h"
> >>   
> >>   #define CHT_WC_V1P05A_CTRL		0x6e3b
> >> @@ -231,6 +232,29 @@ 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,
> >> +						   const u8 *data)
> >> +{
> >> +	u32 value, mask, reg_address, address;
> >> +	u16 i2c_client_address;
> >> +
> >> +	/* byte 0 aka PMIC Flag is reserved */
> >> +	i2c_client_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);
> > 
> > Upon further reflection maybe it would better to do this decoding in
> > the i915 code and just pass each parameter to this hook separately?
> > That way we wouldn't be spreading the vbt details all over the place.
> 
> Interesting point, if the VBT spec says that this encoding is PMIC
> independent, then yes we should probably fo the decoding in the VBT
> code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
> prototype to:
> 
> int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
> 					      u32 value, u32 mask);
> 
> If you agree please let me know and I will do a v4 of the patchset.

Yeah, I think that's probably better. The spec has just the one
interpretation for the sequence.

> 
> I've also been thinking about trying to make the implementation
> under drivers/acpi/pmic pmic independent, but not all pmic
> drivers use the regmap the same way. The CHT Whiskey Cove PMIC
> mfd driver uses a regmap with 16 bit addresses where the upper
> byte is the i2c client address and the lower byte is the register
> address (this PMIC listens on multiple addresses, with different
> registers behind each i2c address).
> 
> Where as most PMIC mfd drivers simply use the standard
> devm_regmap_init_i2c() method of creating a regmap.  For these
> others we could do a standard implementation where we check the
> passed in i2c_address is what we expect (for that type PMIC) and
> then pass the other 3 parameters to regmap_update_bits.
> 
> But I think it would be best to wait with such a generic implementation
> until we encounter a device using the PMIC MIPI sequence element
> with another type of PMIC.  Since we still need the special
> implementation for the CHT WC case, we still need an operation
> pointer for this in intel_pmic_opregion_data anyways, so we can
> easily plug in the generic implementation for others later.

Yeah, probably not worth worrying about this until we
encounter a machine that needs it.

Oh, and we should probably change the DRM_DEBUG_KMS() for the
PMIC_OPREGION=n case to a DRM_ERROR() which tells people to
enable PMIC_OPREGION=y. Not sure why all these random knobs are
even user configurable. No one can really be expected to know
how to configure them properly. There was a recent problem with
someone having set I2C_DESIGNWARE_BAYTRAIL=n as well because
they had a CHT/BSW instead of a BYT :(

-- 
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] 13+ messages in thread

* Re: [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
  2018-12-13 11:21 [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Hans de Goede
                   ` (2 preceding siblings ...)
  2018-12-13 12:29 ` ✓ Fi.CI.BAT: success for series starting with [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Patchwork
@ 2018-12-13 13:45 ` Jani Nikula
  2018-12-13 14:14   ` Hans de Goede
  2018-12-13 14:18 ` ✓ Fi.CI.IGT: success for series starting with [v3,1/3] " Patchwork
  4 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2018-12-13 13:45 UTC (permalink / raw)
  To: Joonas Lahtinen, Rodrigo Vivi, Ville Syrjälä,
	Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: Hans de Goede, intel-gfx, dri-devel, linux-acpi

On Thu, 13 Dec 2018, Hans de Goede <hdegoede@redhat.com> 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.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> 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     | 42 ++++++++++++++++++++++++++++++
>  drivers/acpi/pmic/intel_pmic.h     |  1 +
>  include/linux/mfd/intel_soc_pmic.h |  2 ++
>  3 files changed, 45 insertions(+)
>
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index ca18e0d23df9..61f99735fd41 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,41 @@ 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
> + * @data: Pointer to *15* byte PMIC MIPI sequence element
> + *
> + * 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(const u8 *data)
> +{
> +	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;
> +	if (!d->exec_mipi_pmic_seq_element) {
> +		pr_warn("%s: Not implemented\n", __func__);
> +		pr_warn("%s: Data: %15ph\n", __func__, data);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	mutex_lock(&intel_pmic_opregion->lock);
> +	ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
> +	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..1cc5e69af0c3 100644
> --- a/drivers/acpi/pmic/intel_pmic.h
> +++ b/drivers/acpi/pmic/intel_pmic.h
> @@ -15,6 +15,7 @@ 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, const u8 *data);
>  	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..71a8607a0a8c 100644
> --- a/include/linux/mfd/intel_soc_pmic.h
> +++ b/include/linux/mfd/intel_soc_pmic.h
> @@ -26,4 +26,6 @@ struct intel_soc_pmic {
>  	struct device *dev;
>  };
>  
> +int intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data);

Add a static inline dummy implementation with error return for
CONFIG_PMIC_OPREGION=n? Maybe even with pr_err or pr_warn or something?

BR,
Jani.

> +
>  #endif	/* __INTEL_SOC_PMIC_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2018-12-13 13:08       ` Ville Syrjälä
@ 2018-12-13 13:46         ` Jani Nikula
  2018-12-13 14:21         ` Hans de Goede
  1 sibling, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2018-12-13 13:46 UTC (permalink / raw)
  To: Ville Syrjälä, Hans de Goede
  Cc: intel-gfx, Rafael J . Wysocki, dri-devel, linux-acpi,
	Rodrigo Vivi, Andy Shevchenko, Mika Westerberg, Len Brown

On Thu, 13 Dec 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Dec 13, 2018 at 01:40:27PM +0100, Hans de Goede wrote:
>> Hi,
>> 
>> On 13-12-18 13:14, Ville Syrjälä wrote:
>> > On Thu, Dec 13, 2018 at 12:21:35PM +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.
>> >>
>> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> >> ---
>> >> Changes in v2:
>> >> -Interpret data passed to the PMIC MIPI elements according to the docs
>> >>   instead of my own reverse engineered interpretation
>> >> Changes in v3:
>> >> -Use hex values for out of range checks
>> >> -Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
>> >> ---
>> >>   drivers/acpi/pmic/intel_pmic_chtwc.c | 25 +++++++++++++++++++++++++
>> >>   1 file changed, 25 insertions(+)
>> >>
>> >> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
>> >> index 078b0448f30a..8ede74e9b89f 100644
>> >> --- a/drivers/acpi/pmic/intel_pmic_chtwc.c
>> >> +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
>> >> @@ -12,6 +12,7 @@
>> >>   #include <linux/mfd/intel_soc_pmic.h>
>> >>   #include <linux/platform_device.h>
>> >>   #include <linux/regmap.h>
>> >> +#include <asm/unaligned.h>
>> >>   #include "intel_pmic.h"
>> >>   
>> >>   #define CHT_WC_V1P05A_CTRL		0x6e3b
>> >> @@ -231,6 +232,29 @@ 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,
>> >> +						   const u8 *data)
>> >> +{
>> >> +	u32 value, mask, reg_address, address;
>> >> +	u16 i2c_client_address;
>> >> +
>> >> +	/* byte 0 aka PMIC Flag is reserved */
>> >> +	i2c_client_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);
>> > 
>> > Upon further reflection maybe it would better to do this decoding in
>> > the i915 code and just pass each parameter to this hook separately?
>> > That way we wouldn't be spreading the vbt details all over the place.
>> 
>> Interesting point, if the VBT spec says that this encoding is PMIC
>> independent, then yes we should probably fo the decoding in the VBT
>> code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
>> prototype to:
>> 
>> int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
>> 					      u32 value, u32 mask);
>> 
>> If you agree please let me know and I will do a v4 of the patchset.
>
> Yeah, I think that's probably better. The spec has just the one
> interpretation for the sequence.

Agreed.

BR,
Jani.

>
>> 
>> I've also been thinking about trying to make the implementation
>> under drivers/acpi/pmic pmic independent, but not all pmic
>> drivers use the regmap the same way. The CHT Whiskey Cove PMIC
>> mfd driver uses a regmap with 16 bit addresses where the upper
>> byte is the i2c client address and the lower byte is the register
>> address (this PMIC listens on multiple addresses, with different
>> registers behind each i2c address).
>> 
>> Where as most PMIC mfd drivers simply use the standard
>> devm_regmap_init_i2c() method of creating a regmap.  For these
>> others we could do a standard implementation where we check the
>> passed in i2c_address is what we expect (for that type PMIC) and
>> then pass the other 3 parameters to regmap_update_bits.
>> 
>> But I think it would be best to wait with such a generic implementation
>> until we encounter a device using the PMIC MIPI sequence element
>> with another type of PMIC.  Since we still need the special
>> implementation for the CHT WC case, we still need an operation
>> pointer for this in intel_pmic_opregion_data anyways, so we can
>> easily plug in the generic implementation for others later.
>
> Yeah, probably not worth worrying about this until we
> encounter a machine that needs it.
>
> Oh, and we should probably change the DRM_DEBUG_KMS() for the
> PMIC_OPREGION=n case to a DRM_ERROR() which tells people to
> enable PMIC_OPREGION=y. Not sure why all these random knobs are
> even user configurable. No one can really be expected to know
> how to configure them properly. There was a recent problem with
> someone having set I2C_DESIGNWARE_BAYTRAIL=n as well because
> they had a CHT/BSW instead of a BYT :(

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
  2018-12-13 11:21 ` [PATCH v3 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
@ 2018-12-13 13:49   ` Jani Nikula
  0 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2018-12-13 13:49 UTC (permalink / raw)
  To: Joonas Lahtinen, Rodrigo Vivi, Ville Syrjälä,
	Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: Hans de Goede, intel-gfx, dri-devel, linux-acpi

On Thu, 13 Dec 2018, Hans de Goede <hdegoede@redhat.com> 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.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index f27af47c6e49..6a2ed1ca72e0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -29,6 +29,7 @@
>  #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>
> @@ -371,7 +372,11 @@ 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)
>  {
> +#ifdef CONFIG_PMIC_OPREGION
> +	intel_soc_pmic_exec_mipi_pmic_seq_element(data);
> +#else
>  	DRM_DEBUG_KMS("Skipping PMIC element execution\n");
> +#endif

Please check the return value, and log with DRM_DEBUG_KMS or
DRM_ERROR. With the dummy implementation of
intel_soc_pmic_exec_mipi_pmic_seq_element() returning e.g. -ENODEV this
would also cover the CONFIG_PMIC_OPREGION=n case.

BR,
Jani.

>  
>  	return data + 15;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
  2018-12-13 13:45 ` [PATCH v3 1/3] " Jani Nikula
@ 2018-12-13 14:14   ` Hans de Goede
  0 siblings, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2018-12-13 14:14 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg
  Cc: linux-acpi, intel-gfx, dri-devel

HI,

On 13-12-18 14:45, Jani Nikula wrote:
> On Thu, 13 Dec 2018, Hans de Goede <hdegoede@redhat.com> 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.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> 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     | 42 ++++++++++++++++++++++++++++++
>>   drivers/acpi/pmic/intel_pmic.h     |  1 +
>>   include/linux/mfd/intel_soc_pmic.h |  2 ++
>>   3 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
>> index ca18e0d23df9..61f99735fd41 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,41 @@ 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
>> + * @data: Pointer to *15* byte PMIC MIPI sequence element
>> + *
>> + * 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(const u8 *data)
>> +{
>> +	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;
>> +	if (!d->exec_mipi_pmic_seq_element) {
>> +		pr_warn("%s: Not implemented\n", __func__);
>> +		pr_warn("%s: Data: %15ph\n", __func__, data);
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	mutex_lock(&intel_pmic_opregion->lock);
>> +	ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
>> +	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..1cc5e69af0c3 100644
>> --- a/drivers/acpi/pmic/intel_pmic.h
>> +++ b/drivers/acpi/pmic/intel_pmic.h
>> @@ -15,6 +15,7 @@ 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, const u8 *data);
>>   	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..71a8607a0a8c 100644
>> --- a/include/linux/mfd/intel_soc_pmic.h
>> +++ b/include/linux/mfd/intel_soc_pmic.h
>> @@ -26,4 +26,6 @@ struct intel_soc_pmic {
>>   	struct device *dev;
>>   };
>>   
>> +int intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data);
> 
> Add a static inline dummy implementation with error return for
> CONFIG_PMIC_OPREGION=n? Maybe even with pr_err or pr_warn or something?

There is only one caller and Ville has requested for the caller to
do a DRM_ERROR("Your hardware requires CONFIG_PMIC_OPREGION and it is not set")
if we hit that code-path and CONFIG_PMIC_OPREGION is not set.

We could do the DRM_ERROR from the wrapper, but that would make the
wrapper DRM specific which feels wrong to me.

Regards,

Hans

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

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

* ✓ Fi.CI.IGT: success for series starting with [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
  2018-12-13 11:21 [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Hans de Goede
                   ` (3 preceding siblings ...)
  2018-12-13 13:45 ` [PATCH v3 1/3] " Jani Nikula
@ 2018-12-13 14:18 ` Patchwork
  4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-12-13 14:18 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
URL   : https://patchwork.freedesktop.org/series/53986/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5311_full -> Patchwork_11085_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11085_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11085_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_11085_full:

### IGT changes ###

#### Warnings ####

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

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

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

### IGT changes ###

#### Issues hit ####

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

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

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

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#107956]

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

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107725] +2

  * igt@kms_color@pipe-a-ctm-blue-to-red:
    - shard-skl:          PASS -> FAIL [fdo#107201]

  * igt@kms_color@pipe-b-ctm-0-75:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +4

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103232]

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

  * igt@kms_draw_crc@draw-method-xrgb8888-blt-ytiled:
    - shard-skl:          PASS -> FAIL [fdo#107589]

  * igt@kms_draw_crc@fill-fb:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_flip@2x-busy-flip:
    - shard-glk:          PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-skl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-apl:          PASS -> DMESG-FAIL [fdo#103167] / [fdo#103558] / [fdo#105602]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-skl:          PASS -> FAIL [fdo#105682]

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-iclb:         NOTRUN -> FAIL [fdo#108948] +1

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

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +3

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

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

  * igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#108950]

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

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#104108] / [fdo#107773]

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

  * igt@pm_rpm@debugfs-read:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108840]

  
#### Possible fixes ####

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-iclb:         TIMEOUT -> PASS

  * igt@gem_workarounds@suspend-resume-context:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS

  * igt@i915_suspend@sysfs-reader:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

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

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

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS

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

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

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

  * igt@kms_cursor_crc@cursor-512x512-onscreen:
    - shard-apl:          INCOMPLETE [fdo#103927] -> SKIP

  * igt@kms_cursor_crc@cursor-64x21-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS

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

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-apl:          FAIL [fdo#103060] -> PASS

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

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

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

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

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

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

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

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-glk:          DMESG-FAIL [fdo#105763] / [fdo#106538] -> PASS

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

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

  * igt@pm_rpm@drm-resources-equal:
    - shard-iclb:         INCOMPLETE [fdo#108840] -> PASS

  * igt@pm_rpm@legacy-planes:
    - shard-iclb:         DMESG-WARN [fdo#108654] -> PASS

  
#### Warnings ####

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-apl:          FAIL [fdo#103232] -> DMESG-FAIL [fdo#103232] / [fdo#103558] / [fdo#105602]

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

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [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#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [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#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107589]: https://bugs.freedesktop.org/show_bug.cgi?id=107589
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [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#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108887]: https://bugs.freedesktop.org/show_bug.cgi?id=108887
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


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

    * Linux: CI_DRM_5311 -> Patchwork_11085

  CI_DRM_5311: a42fd8bf199784ee4ff1cdb5ee03eedd9a535d4a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4746: 2c793666d8c8328733f5769b16ae5858fee97f3f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11085: a6a19013e822938da76771d319deefebfc6a5360 @ 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_11085/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
  2018-12-13 13:08       ` Ville Syrjälä
  2018-12-13 13:46         ` Jani Nikula
@ 2018-12-13 14:21         ` Hans de Goede
  1 sibling, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2018-12-13 14:21 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Rafael J . Wysocki, linux-acpi, dri-devel,
	Rodrigo Vivi, Andy Shevchenko, Mika Westerberg, Len Brown

Hi,

On 13-12-18 14:08, Ville Syrjälä wrote:
> On Thu, Dec 13, 2018 at 01:40:27PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 13-12-18 13:14, Ville Syrjälä wrote:

<snip>

>>>> +static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
>>>> +						   const u8 *data)
>>>> +{
>>>> +	u32 value, mask, reg_address, address;
>>>> +	u16 i2c_client_address;
>>>> +
>>>> +	/* byte 0 aka PMIC Flag is reserved */
>>>> +	i2c_client_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);
>>>
>>> Upon further reflection maybe it would better to do this decoding in
>>> the i915 code and just pass each parameter to this hook separately?
>>> That way we wouldn't be spreading the vbt details all over the place.
>>
>> Interesting point, if the VBT spec says that this encoding is PMIC
>> independent, then yes we should probably fo the decoding in the VBT
>> code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
>> prototype to:
>>
>> int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
>> 					      u32 value, u32 mask);
>>
>> If you agree please let me know and I will do a v4 of the patchset.
> 
> Yeah, I think that's probably better. The spec has just the one
> interpretation for the sequence.

Ok, v4. coming up.

> Oh, and we should probably change the DRM_DEBUG_KMS() for the
> PMIC_OPREGION=n case to a DRM_ERROR() which tells people to
> enable PMIC_OPREGION=y.

Will do.

Regards,

Hans

p.s.

Have you also seen these 2 DSI related series (I've not see any feedback
on these 2 series yet) ?  :

https://patchwork.kernel.org/patch/10707629/  (patch 1/4)
https://patchwork.kernel.org/patch/10707647/  (patch 1/4)


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2018-12-13 14:21 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-13 11:21 [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Hans de Goede
2018-12-13 11:21 ` [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC Hans de Goede
2018-12-13 12:14   ` Ville Syrjälä
2018-12-13 12:40     ` Hans de Goede
2018-12-13 13:08       ` Ville Syrjälä
2018-12-13 13:46         ` Jani Nikula
2018-12-13 14:21         ` Hans de Goede
2018-12-13 11:21 ` [PATCH v3 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences Hans de Goede
2018-12-13 13:49   ` Jani Nikula
2018-12-13 12:29 ` ✓ Fi.CI.BAT: success for series starting with [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements Patchwork
2018-12-13 13:45 ` [PATCH v3 1/3] " Jani Nikula
2018-12-13 14:14   ` Hans de Goede
2018-12-13 14:18 ` ✓ Fi.CI.IGT: success for series starting with [v3,1/3] " 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.