[PATCH v2 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements

[PATCH v2 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements

[Hans de Goede]

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>
---
 drivers/acpi/pmic/intel_pmic.c     | 25 +++++++++++++++++++++++++
 drivers/acpi/pmic/intel_pmic.h     |  1 +
 include/linux/mfd/intel_soc_pmic.h |  2 ++
 3 files changed, 28 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index ca18e0d23df9..0d96ca08bb79 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,24 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
+
+void intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data)
+{
+	struct intel_pmic_opregion_data *d;
+
+	if (!intel_pmic_opregion) {
+		pr_warn("%s: No PMIC registered\n", __func__);
+		return;
+	}
+
+	d = intel_pmic_opregion->data;
+	if (!d->exec_mipi_pmic_seq_element) {
+		pr_warn("%s: Not implemented\n", __func__);
+		return;
+	}
+
+	mutex_lock(&intel_pmic_opregion->lock);
+	d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
+	mutex_unlock(&intel_pmic_opregion->lock);
+}
+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..5a7bb33d046a 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);
+	void (*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..ce04ad7d4b6c 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;
 };
 
+void intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data);
+
 #endif	/* __INTEL_SOC_PMIC_H__ */
-- 
2.19.2

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

[PATCH v2 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC

[Hans de Goede]

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
---
 drivers/acpi/pmic/intel_pmic_chtwc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
index 078b0448f30a..049c0cf3b9ed 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,28 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg,
 	return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0);
 }
 
+static void 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 > 255 || reg_address > 255) {
+		pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n",
+			__func__, i2c_client_address, reg_address);
+		return;
+	}
+
+	address = (i2c_client_address << 8) | reg_address;
+	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 +261,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

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

[PATCH v2 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC mipi sequences

[Hans de Goede]

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

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

Re: [PATCH v2 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements

[Mika Westerberg]

On Thu, Dec 06, 2018 at 02:47:03PM +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.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/pmic/intel_pmic.c     | 25 +++++++++++++++++++++++++
>  drivers/acpi/pmic/intel_pmic.h     |  1 +
>  include/linux/mfd/intel_soc_pmic.h |  2 ++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index ca18e0d23df9..0d96ca08bb79 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,24 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
> +

Since this is exported, please add kernel-doc here.

> +void intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data)
> +{
> +	struct intel_pmic_opregion_data *d;
> +
> +	if (!intel_pmic_opregion) {
> +		pr_warn("%s: No PMIC registered\n", __func__);
> +		return;

Why not return error codes instead?

Other ops in struct intel_pmic_opregion_data seem to do so.

> +	}
> +
> +	d = intel_pmic_opregion->data;
> +	if (!d->exec_mipi_pmic_seq_element) {
> +		pr_warn("%s: Not implemented\n", __func__);
> +		return;

Ditto.

> +	}
> +
> +	mutex_lock(&intel_pmic_opregion->lock);
> +	d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
> +	mutex_unlock(&intel_pmic_opregion->lock);
> +}
> +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..5a7bb33d046a 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);
> +	void (*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..ce04ad7d4b6c 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;
>  };
>  
> +void 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

Re: [PATCH v2 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC

[Mika Westerberg]

On Thu, Dec 06, 2018 at 02:47:04PM +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
> ---
>  drivers/acpi/pmic/intel_pmic_chtwc.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
> index 078b0448f30a..049c0cf3b9ed 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,28 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg,
>  	return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0);
>  }
>  
> +static void 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 > 255 || reg_address > 255) {
> +		pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n",
> +			__func__, i2c_client_address, reg_address);
> +		return;

Here also return an error code instead.

> +	}
> +
> +	address = (i2c_client_address << 8) | reg_address;
> +	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 +261,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC mipi sequences

[Ville Syrjälä]

On Thu, Dec 06, 2018 at 02:47:05PM +0100, Hans de Goede wrote:
> Add support for PMIC mipi sequences using the new
> intel_soc_pmic_exec_mipi_pmic_seq_element function.

Please document somewhere which machines you've found to need
this (commit msg should be sufficient I suppose). Can make it
much easier to respond to bug reports like "my machine X with
DSI doesn't work".

> 
> 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

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

Re: [PATCH v2 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements

[Hans de Goede]

Hi,

On 07-12-18 12:39, Mika Westerberg wrote:
> On Thu, Dec 06, 2018 at 02:47:03PM +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.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/acpi/pmic/intel_pmic.c     | 25 +++++++++++++++++++++++++
>>   drivers/acpi/pmic/intel_pmic.h     |  1 +
>>   include/linux/mfd/intel_soc_pmic.h |  2 ++
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
>> index ca18e0d23df9..0d96ca08bb79 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,24 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
>>   	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
>> +
> 
> Since this is exported, please add kernel-doc here.

Done for v3.
> 
>> +void intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data)
>> +{
>> +	struct intel_pmic_opregion_data *d;
>> +
>> +	if (!intel_pmic_opregion) {
>> +		pr_warn("%s: No PMIC registered\n", __func__);
>> +		return;
> 
> Why not return error codes instead?
> 
> Other ops in struct intel_pmic_opregion_data seem to do so.

Ok, I've changed the return-type to int and I'm returning error
codes for v3 of this patch-set.

Regards,

Hans



> 
>> +	}
>> +
>> +	d = intel_pmic_opregion->data;
>> +	if (!d->exec_mipi_pmic_seq_element) {
>> +		pr_warn("%s: Not implemented\n", __func__);
>> +		return;
> 
> Ditto.
> 
>> +	}
>> +
>> +	mutex_lock(&intel_pmic_opregion->lock);
>> +	d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
>> +	mutex_unlock(&intel_pmic_opregion->lock);
>> +}
>> +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..5a7bb33d046a 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);
>> +	void (*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..ce04ad7d4b6c 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;
>>   };
>>   
>> +void intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data);
>> +
>>   #endif	/* __INTEL_SOC_PMIC_H__ */
>> -- 
>> 2.19.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC

[Hans de Goede]

Hi,

On 07-12-18 12:40, Mika Westerberg wrote:
> On Thu, Dec 06, 2018 at 02:47:04PM +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
>> ---
>>   drivers/acpi/pmic/intel_pmic_chtwc.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
>> index 078b0448f30a..049c0cf3b9ed 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,28 @@ static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg,
>>   	return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0);
>>   }
>>   
>> +static void 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 > 255 || reg_address > 255) {
>> +		pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n",
>> +			__func__, i2c_client_address, reg_address);
>> +		return;
> 
> Here also return an error code instead.

Done for v3.

Regards,

Hans




> 
>> +	}
>> +
>> +	address = (i2c_client_address << 8) | reg_address;
>> +	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 +261,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

Re: [PATCH v2 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC mipi sequences

[Hans de Goede]

Hi,

On 07-12-18 18:17, Ville Syrjälä wrote:
> On Thu, Dec 06, 2018 at 02:47:05PM +0100, Hans de Goede wrote:
>> Add support for PMIC mipi sequences using the new
>> intel_soc_pmic_exec_mipi_pmic_seq_element function.
> 
> Please document somewhere which machines you've found to need
> this (commit msg should be sufficient I suppose). Can make it
> much easier to respond to bug reports like "my machine X with
> DSI doesn't work".

Ok, I've added this info to the commit message for v3 of the
patch-set.

Regards,

Hans


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