diff mbox series

[v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements

Message ID 20181213112136.7790-1-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements | expand

Commit Message

Hans de Goede Dec. 13, 2018, 11:21 a.m. UTC
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(+)

Comments

Jani Nikula Dec. 13, 2018, 1:45 p.m. UTC | #1
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__ */
Hans de Goede Dec. 13, 2018, 2:14 p.m. UTC | #2
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
diff mbox series

Patch

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__ */