[RFC v2 0/4] Crystal Cove PMIC based Panel and Backlight Control

[RFC v2 0/4] Crystal Cove PMIC based Panel and Backlight Control

[Shobhit Kumar]

Hi All
Please find modifed set of patches. Sending as a separate thread as initial
patches were a crude implementation to trigger discussion, but these have been 
tested and also do not need intel_soc_pmic_writeb/readb functionaly, but uses
the regmap interface as suggested by Jacob.

These patches implement a drm_panel as a platform driver for the mfd_cell device
declared in intel_soc_pmic_core.c. 

Still there are opens, where I need closure - 
1. Added a new drm_panel, but how to find the panel in lack of OF info. For now
   added a drm_panel function to find panel by name.
2. Backlight also needs similar pmic based control. Is it okay to add a backlight
   class driver also as part of this panel driver ?

For now I am doing Backlight Enable/Disable also during panel/enable as this will
at least save power. Backlight control will need a backlight class driver.

Regards
Shobhit

Shobhit Kumar (4):
  drm: Add support to find drm_panel by name
  mfd: Add a new cell device for panel controlled by crystal cove pmic
  drm/panel: Add new panel driver based on crystal cove pmic
  drm/i915: Enable DSI panel enable/disable based on PMIC

 drivers/gpu/drm/drm_panel.c                |  18 +++
 drivers/gpu/drm/i915/Kconfig               |   1 +
 drivers/gpu/drm/i915/intel_dsi.c           |  16 +++
 drivers/gpu/drm/i915/intel_dsi.h           |   6 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   1 +
 drivers/gpu/drm/panel/Kconfig              |   7 ++
 drivers/gpu/drm/panel/Makefile             |   1 +
 drivers/gpu/drm/panel/panel-crystalcove.c  | 191 +++++++++++++++++++++++++++++
 drivers/mfd/intel_soc_pmic_crc.c           |   3 +
 include/drm/drm_panel.h                    |   3 +
 10 files changed, 247 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-crystalcove.c

-- 
1.9.1

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

[RFC v2 1/4] drm: Add support to find drm_panel by name

[Shobhit Kumar]

For scenarios where OF is not available, we can use panel identification by
name.

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/drm_panel.c | 18 ++++++++++++++++++
 include/drm/drm_panel.h     |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 2ef988e..773ebd6 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
 EXPORT_SYMBOL(of_drm_find_panel);
 #endif
 
+struct drm_panel *name_drm_find_panel(const char *name)
+{
+	struct drm_panel *panel;
+
+	mutex_lock(&panel_lock);
+
+	list_for_each_entry(panel, &panel_list, list) {
+		if (strcmp(panel->name, name) == 0) {
+			mutex_unlock(&panel_lock);
+			return panel;
+		}
+	}
+
+	mutex_unlock(&panel_lock);
+	return NULL;
+}
+EXPORT_SYMBOL(name_drm_find_panel);
+
 MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
 MODULE_DESCRIPTION("DRM panel infrastructure");
 MODULE_LICENSE("GPL and additional rights");
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 1fbcc96..b120b5d 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -74,6 +74,7 @@ struct drm_panel {
 	struct drm_device *drm;
 	struct drm_connector *connector;
 	struct device *dev;
+	char name[NAME_MAX];
 
 	const struct drm_panel_funcs *funcs;
 
@@ -137,4 +138,6 @@ static inline struct drm_panel *of_drm_find_panel(struct device_node *np)
 }
 #endif
 
+struct drm_panel *name_drm_find_panel(const char *name);
+
 #endif
-- 
1.9.1

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

Re: [RFC v2 1/4] drm: Add support to find drm_panel by name

[Jani Nikula]

On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> For scenarios where OF is not available, we can use panel identification by
> name.
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/drm_panel.c | 18 ++++++++++++++++++
>  include/drm/drm_panel.h     |  3 +++
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 2ef988e..773ebd6 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
>  EXPORT_SYMBOL(of_drm_find_panel);
>  #endif
>  
> +struct drm_panel *name_drm_find_panel(const char *name)
> +{
> +	struct drm_panel *panel;
> +
> +	mutex_lock(&panel_lock);
> +
> +	list_for_each_entry(panel, &panel_list, list) {
> +		if (strcmp(panel->name, name) == 0) {
> +			mutex_unlock(&panel_lock);
> +			return panel;
> +		}
> +	}
> +
> +	mutex_unlock(&panel_lock);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(name_drm_find_panel);

This patch needs to be sent to drm-devel.

The name should probably be something like drm_find_panel_by_name.

I have a slightly uneasy feeling about handing out drm_panel pointers
(both from here and of_drm_find_panel) without refcounting. If the panel
driver gets removed, whoever called the find functions will have a
dangling pointer. I supposed this will be discussed on drm-devel.

BR,
Jani.

> +
>  MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
>  MODULE_DESCRIPTION("DRM panel infrastructure");
>  MODULE_LICENSE("GPL and additional rights");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..b120b5d 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -74,6 +74,7 @@ struct drm_panel {
>  	struct drm_device *drm;
>  	struct drm_connector *connector;
>  	struct device *dev;
> +	char name[NAME_MAX];
>  
>  	const struct drm_panel_funcs *funcs;
>  
> @@ -137,4 +138,6 @@ static inline struct drm_panel *of_drm_find_panel(struct device_node *np)
>  }
>  #endif
>  
> +struct drm_panel *name_drm_find_panel(const char *name);
> +
>  #endif
> -- 
> 1.9.1
>

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

Re: [RFC v2 1/4] drm: Add support to find drm_panel by name

[Kumar, Shobhit]

On 1/9/2015 6:20 PM, Jani Nikula wrote:
> On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>> For scenarios where OF is not available, we can use panel identification by
>> name.
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_panel.c | 18 ++++++++++++++++++
>>   include/drm/drm_panel.h     |  3 +++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index 2ef988e..773ebd6 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
>>   EXPORT_SYMBOL(of_drm_find_panel);
>>   #endif
>>
>> +struct drm_panel *name_drm_find_panel(const char *name)
>> +{
>> +	struct drm_panel *panel;
>> +
>> +	mutex_lock(&panel_lock);
>> +
>> +	list_for_each_entry(panel, &panel_list, list) {
>> +		if (strcmp(panel->name, name) == 0) {
>> +			mutex_unlock(&panel_lock);
>> +			return panel;
>> +		}
>> +	}
>> +
>> +	mutex_unlock(&panel_lock);
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL(name_drm_find_panel);
>
> This patch needs to be sent to drm-devel.
>
> The name should probably be something like drm_find_panel_by_name.
>
> I have a slightly uneasy feeling about handing out drm_panel pointers
> (both from here and of_drm_find_panel) without refcounting. If the panel
> driver gets removed, whoever called the find functions will have a
> dangling pointer. I supposed this will be discussed on drm-devel.

Right its a valid point. I Will post the updated patch there and see 
what all comes up.

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

Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name

[Daniel Vetter]

On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula <jani.nikula@intel.com> wrote:
> I have a slightly uneasy feeling about handing out drm_panel pointers
> (both from here and of_drm_find_panel) without refcounting. If the panel
> driver gets removed, whoever called the find functions will have a
> dangling pointer. I supposed this will be discussed on drm-devel.

There's been some discussion already about exactly this problem (but
with drm bridges) with Thierry and some other people. Cc'ed them all
hopefully. Especially when the panel/bridge is a separate driver
there's imo indeed an issue.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name

[Andrzej Hajda]

On 01/13/2015 12:08 AM, Daniel Vetter wrote:
> On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula <jani.nikula@intel.com> wrote:
>> I have a slightly uneasy feeling about handing out drm_panel pointers
>> (both from here and of_drm_find_panel) without refcounting. If the panel
>> driver gets removed, whoever called the find functions will have a
>> dangling pointer. I supposed this will be discussed on drm-devel.

refcounting does not seems to me a good solution, drm_panel is
exposed by device driver and device driver can be unbound unconditionally
at any time. This problem affects many frameworks not only drm_panel.

I work on resource tracking framework which tries to solve the problem
in a generic way[1].

[1]: https://lkml.org/lkml/2014/12/10/342

Regards
Andrzej


> There's been some discussion already about exactly this problem (but
> with drm bridges) with Thierry and some other people. Cc'ed them all
> hopefully. Especially when the panel/bridge is a separate driver
> there's imo indeed an issue.
> -Daniel

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

Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1417 bytes --]

On Tue, Jan 13, 2015 at 12:08:11AM +0100, Daniel Vetter wrote:
> On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula <jani.nikula@intel.com> wrote:
> > I have a slightly uneasy feeling about handing out drm_panel pointers
> > (both from here and of_drm_find_panel) without refcounting. If the panel
> > driver gets removed, whoever called the find functions will have a
> > dangling pointer. I supposed this will be discussed on drm-devel.
> 
> There's been some discussion already about exactly this problem (but
> with drm bridges) with Thierry and some other people. Cc'ed them all
> hopefully. Especially when the panel/bridge is a separate driver
> there's imo indeed an issue.

I posted patches some time ago to create a generic registry to do the
actual ref-counting[0]. It didn't seem to be very well received by Greg
for the core, so perhaps we could test-drive it in DRM first for panels
and bridges and once it's matured a bit it could still be promoted to
the driver core, or maybe lib/.

The difficult part about it is that while reference counting gives you
the benefit of keeping a valid pointer around, you may still want to
have a method of getting notified of the panel going away. I've thought
a bit about that and I think we could probably integrate that into the
registry, since that will notice anyway.

Thierry

[0]: http://www.spinics.net/linux/lists/kernel/msg1859333.html

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[RFC v2 2/4] mfd: Add a new cell device for panel controlled by crystal cove pmic

[Shobhit Kumar]

On BYT-T configuration, panel enable/disable signals are routed through
PMIC. Add a cell device for the same.

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index c85e2ec..c8ccc24 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -109,6 +109,9 @@ static struct mfd_cell crystal_cove_dev[] = {
 	{
 		.name = "crystal_cove_pmic",
 	},
+	{
+		.name = "crystal_cove_panel",
+	},
 };
 
 static struct regmap_config crystal_cove_regmap_config = {
-- 
1.9.1

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

[RFC v2 3/4] drm/panel: Add new panel driver based on crystal cove pmic

[Shobhit Kumar]

This driver provides support for the "crystal_cove_panel" cell device.
On BYT-T pmic has to be used to enable/disable panel.

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/panel/Kconfig             |   7 ++
 drivers/gpu/drm/panel/Makefile            |   1 +
 drivers/gpu/drm/panel/panel-crystalcove.c | 191 ++++++++++++++++++++++++++++++
 3 files changed, 199 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-crystalcove.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 024e98e..d813bd1 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -40,4 +40,11 @@ config DRM_PANEL_SHARP_LQ101R1SX01
 	  To compile this driver as a module, choose M here: the module
 	  will be called panel-sharp-lq101r1sx01.
 
+config DRM_PANEL_CRYSTALCOVE
+	tristate "Crystalcove PMIC controlled panel"
+	depends on INTEL_SOC_PMIC
+	help
+	  Say Y here if you want to enable support for DSI panel and backlight
+	  control using crystalcove pmic. This is used for BYT CR configurations
+
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 4b2a043..c05824e 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
 obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
+obj-$(CONFIG_DRM_PANEL_CRYSTALCOVE) += panel-crystalcove.o
diff --git a/drivers/gpu/drm/panel/panel-crystalcove.c b/drivers/gpu/drm/panel/panel-crystalcove.c
new file mode 100644
index 0000000..22f7ff5
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-crystalcove.c
@@ -0,0 +1,191 @@
+/*
+ * Copyright © 2006-2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *	Shobhit Kumar <shobhit.kumar@intel.com>
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_panel.h>
+#include <linux/mfd/intel_soc_pmic.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define PMIC_PANEL_EN		0x52
+#define PMIC_PWM_EN		0x51
+#define PMIC_BKL_EN		0x4B
+#define PMIC_PWM_LEVEL		0x4E
+
+struct crystalcove_panel {
+	struct drm_panel base;
+	char name[NAME_MAX];
+	bool prepared;
+	bool enabled;
+
+	struct mutex lock;
+
+	/* crystal cove pmic regmap */
+	struct regmap *regmap;
+};
+
+static inline struct crystalcove_panel *to_crystalcove_panel(struct drm_panel *panel)
+{
+	return container_of(panel, struct crystalcove_panel, base);
+}
+
+static int crystalcove_panel_disable(struct drm_panel *panel)
+{
+	struct crystalcove_panel *p = to_crystalcove_panel(panel);
+
+	if (!p->enabled)
+		return 0;
+
+	DRM_DEBUG_KMS("\n");
+
+	/* invoke the pmic driver */
+	regmap_write(p->regmap, PMIC_PANEL_EN, 0x00);
+
+	/* Disable backlight as well */
+	regmap_write(p->regmap, PMIC_PWM_LEVEL, 0);
+	msleep(20);
+	regmap_write(p->regmap, PMIC_PWM_EN, 0x00);
+	regmap_write(p->regmap, PMIC_BKL_EN, 0x7F);
+
+	p->enabled = false;
+
+	return 0;
+}
+
+static int crystalcove_panel_unprepare(struct drm_panel *panel)
+{
+	struct crystalcove_panel *p = to_crystalcove_panel(panel);
+
+	if (!p->prepared)
+		return 0;
+
+	/* Nothing needed */
+	p->prepared = false;
+
+	return 0;
+}
+
+static int crystalcove_panel_prepare(struct drm_panel *panel)
+{
+	struct crystalcove_panel *p = to_crystalcove_panel(panel);
+
+	if (p->prepared)
+		return 0;
+
+	/* Nothing needed */
+	p->prepared = true;
+
+	return 0;
+}
+
+static int crystalcove_panel_enable(struct drm_panel *panel)
+{
+	struct crystalcove_panel *p = to_crystalcove_panel(panel);
+
+	if (p->enabled)
+		return 0;
+
+	DRM_DEBUG_KMS("\n");
+
+	/* invoke the pmic driver */
+	regmap_write(p->regmap, PMIC_PANEL_EN, 0x01);
+
+	/* Enable BKL as well */
+	regmap_write(p->regmap, PMIC_BKL_EN, 0xFF);
+	regmap_write(p->regmap, PMIC_PWM_EN, 0x01);
+	msleep(20);
+	regmap_write(p->regmap, PMIC_PWM_LEVEL, 255);
+
+	p->enabled = true;
+
+	return 0;
+}
+
+static const struct drm_panel_funcs crystalcove_panel_funcs = {
+	.disable = crystalcove_panel_disable,
+	.unprepare = crystalcove_panel_unprepare,
+	.prepare = crystalcove_panel_prepare,
+	.enable = crystalcove_panel_enable,
+};
+
+static int crystalcove_panel_probe(struct platform_device *pdev)
+{
+	struct crystalcove_panel *panel;
+	int retval;
+	struct device *dev = pdev->dev.parent;
+	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
+
+	panel = devm_kzalloc(&pdev->dev, sizeof(*panel), GFP_KERNEL);
+	if (!panel)
+		return -ENOMEM;
+
+	DRM_DEBUG_KMS("\n");
+
+	platform_set_drvdata(pdev, panel);
+
+	strcpy(panel->base.name, "crystal_cove_panel");
+	panel->regmap = pmic->regmap;
+
+	regmap_read(panel->regmap, PMIC_PANEL_EN, &retval);
+	panel->enabled = panel->prepared = retval ? true : false;
+
+	drm_panel_init(&panel->base);
+	panel->base.dev = dev;
+	panel->base.funcs = &crystalcove_panel_funcs;
+
+	drm_panel_add(&panel->base);
+
+	return 0;
+}
+
+static int crystalcove_panel_remove(struct platform_device *pdev)
+{
+	struct crystalcove_panel *panel = platform_get_drvdata(pdev);
+
+	DRM_DEBUG_KMS("\n");
+
+	drm_panel_detach(&panel->base);
+	drm_panel_remove(&panel->base);
+
+	crystalcove_panel_disable(&panel->base);
+
+	return 0;
+}
+
+static struct platform_driver crystalcove_panel_driver = {
+	.probe = crystalcove_panel_probe,
+	.remove = crystalcove_panel_remove,
+	.driver = {
+		.name = "crystal_cove_panel",
+	},
+};
+
+module_platform_driver(crystalcove_panel_driver);
+
+MODULE_AUTHOR("Shobhit Kumar <shobhit.kumar@linux.intel.com");
+MODULE_DESCRIPTION("Intel Crystal Cove Panel Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

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

Re: [RFC v2 3/4] drm/panel: Add new panel driver based on crystal cove pmic

[Jani Nikula]

On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> This driver provides support for the "crystal_cove_panel" cell device.
> On BYT-T pmic has to be used to enable/disable panel.

This needs to be sent to dri-devel.

With the comments below addressed, and with the disclaimer that I have
no idea about the pmic registers or required sleeps, this is

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/panel/Kconfig             |   7 ++
>  drivers/gpu/drm/panel/Makefile            |   1 +
>  drivers/gpu/drm/panel/panel-crystalcove.c | 191 ++++++++++++++++++++++++++++++
>  3 files changed, 199 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-crystalcove.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 024e98e..d813bd1 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -40,4 +40,11 @@ config DRM_PANEL_SHARP_LQ101R1SX01
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called panel-sharp-lq101r1sx01.
>  
> +config DRM_PANEL_CRYSTALCOVE
> +	tristate "Crystalcove PMIC controlled panel"
> +	depends on INTEL_SOC_PMIC

INTEL_SOC_PMIC is still a bool (and requires I2C=y), and IMO should be
fixed to support tristate. Or are there fundamental reasons this can't
be done?

> +	help
> +	  Say Y here if you want to enable support for DSI panel and backlight
> +	  control using crystalcove pmic. This is used for BYT CR configurations
> +
>  endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 4b2a043..c05824e 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -2,3 +2,4 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
>  obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> +obj-$(CONFIG_DRM_PANEL_CRYSTALCOVE) += panel-crystalcove.o
> diff --git a/drivers/gpu/drm/panel/panel-crystalcove.c b/drivers/gpu/drm/panel/panel-crystalcove.c
> new file mode 100644
> index 0000000..22f7ff5
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-crystalcove.c
> @@ -0,0 +1,191 @@
> +/*
> + * Copyright © 2006-2014 Intel Corporation

2006?!

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + *	Shobhit Kumar <shobhit.kumar@intel.com>
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_panel.h>
> +#include <linux/mfd/intel_soc_pmic.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define PMIC_PANEL_EN		0x52
> +#define PMIC_PWM_EN		0x51
> +#define PMIC_BKL_EN		0x4B
> +#define PMIC_PWM_LEVEL		0x4E
> +
> +struct crystalcove_panel {
> +	struct drm_panel base;
> +	char name[NAME_MAX];

Unused.

> +	bool prepared;
> +	bool enabled;
> +
> +	struct mutex lock;

Unused.

> +
> +	/* crystal cove pmic regmap */
> +	struct regmap *regmap;
> +};
> +
> +static inline struct crystalcove_panel *to_crystalcove_panel(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct crystalcove_panel, base);
> +}
> +
> +static int crystalcove_panel_disable(struct drm_panel *panel)
> +{
> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> +	if (!p->enabled)
> +		return 0;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	/* invoke the pmic driver */
> +	regmap_write(p->regmap, PMIC_PANEL_EN, 0x00);
> +
> +	/* Disable backlight as well */
> +	regmap_write(p->regmap, PMIC_PWM_LEVEL, 0);
> +	msleep(20);

Seems like a long delay.

> +	regmap_write(p->regmap, PMIC_PWM_EN, 0x00);
> +	regmap_write(p->regmap, PMIC_BKL_EN, 0x7F);
> +
> +	p->enabled = false;
> +
> +	return 0;
> +}
> +
> +static int crystalcove_panel_unprepare(struct drm_panel *panel)
> +{
> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> +	if (!p->prepared)
> +		return 0;
> +
> +	/* Nothing needed */
> +	p->prepared = false;
> +
> +	return 0;
> +}
> +
> +static int crystalcove_panel_prepare(struct drm_panel *panel)
> +{
> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> +	if (p->prepared)
> +		return 0;
> +
> +	/* Nothing needed */
> +	p->prepared = true;
> +
> +	return 0;
> +}

If you don't need prepare/unprepare, you can just leave them out
altogether. It's easy to add them if you need them later.

> +
> +static int crystalcove_panel_enable(struct drm_panel *panel)
> +{
> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
> +
> +	if (p->enabled)
> +		return 0;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	/* invoke the pmic driver */
> +	regmap_write(p->regmap, PMIC_PANEL_EN, 0x01);
> +
> +	/* Enable BKL as well */
> +	regmap_write(p->regmap, PMIC_BKL_EN, 0xFF);
> +	regmap_write(p->regmap, PMIC_PWM_EN, 0x01);
> +	msleep(20);
> +	regmap_write(p->regmap, PMIC_PWM_LEVEL, 255);
> +
> +	p->enabled = true;
> +
> +	return 0;
> +}
> +
> +static const struct drm_panel_funcs crystalcove_panel_funcs = {
> +	.disable = crystalcove_panel_disable,
> +	.unprepare = crystalcove_panel_unprepare,
> +	.prepare = crystalcove_panel_prepare,
> +	.enable = crystalcove_panel_enable,
> +};
> +
> +static int crystalcove_panel_probe(struct platform_device *pdev)
> +{
> +	struct crystalcove_panel *panel;
> +	int retval;
> +	struct device *dev = pdev->dev.parent;
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> +
> +	panel = devm_kzalloc(&pdev->dev, sizeof(*panel), GFP_KERNEL);
> +	if (!panel)
> +		return -ENOMEM;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	platform_set_drvdata(pdev, panel);
> +
> +	strcpy(panel->base.name, "crystal_cove_panel");
> +	panel->regmap = pmic->regmap;
> +
> +	regmap_read(panel->regmap, PMIC_PANEL_EN, &retval);
> +	panel->enabled = panel->prepared = retval ? true : false;

For bools you can just assign retval, or if you want to emphasis it's a
bool you can use !!retval.

> +
> +	drm_panel_init(&panel->base);
> +	panel->base.dev = dev;
> +	panel->base.funcs = &crystalcove_panel_funcs;
> +
> +	drm_panel_add(&panel->base);
> +
> +	return 0;
> +}
> +
> +static int crystalcove_panel_remove(struct platform_device *pdev)
> +{
> +	struct crystalcove_panel *panel = platform_get_drvdata(pdev);
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	drm_panel_detach(&panel->base);
> +	drm_panel_remove(&panel->base);
> +
> +	crystalcove_panel_disable(&panel->base);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver crystalcove_panel_driver = {
> +	.probe = crystalcove_panel_probe,
> +	.remove = crystalcove_panel_remove,
> +	.driver = {
> +		.name = "crystal_cove_panel",
> +	},
> +};
> +
> +module_platform_driver(crystalcove_panel_driver);
> +
> +MODULE_AUTHOR("Shobhit Kumar <shobhit.kumar@linux.intel.com");
> +MODULE_DESCRIPTION("Intel Crystal Cove Panel Driver");
> +MODULE_LICENSE("GPL v2");

This conflicts with the copyright header of the file.


> -- 
> 1.9.1
>

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

Re: [RFC v2 3/4] drm/panel: Add new panel driver based on crystal cove pmic

[Kumar, Shobhit]

On 1/9/2015 6:38 PM, Jani Nikula wrote:
> On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>> This driver provides support for the "crystal_cove_panel" cell device.
>> On BYT-T pmic has to be used to enable/disable panel.
>
> This needs to be sent to dri-devel.

Will do for the updated patch after addressing all your comments.

>
> With the comments below addressed, and with the disclaimer that I have
> no idea about the pmic registers or required sleeps, this is
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/panel/Kconfig             |   7 ++
>>   drivers/gpu/drm/panel/Makefile            |   1 +
>>   drivers/gpu/drm/panel/panel-crystalcove.c | 191 ++++++++++++++++++++++++++++++
>>   3 files changed, 199 insertions(+)
>>   create mode 100644 drivers/gpu/drm/panel/panel-crystalcove.c
>>
>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>> index 024e98e..d813bd1 100644
>> --- a/drivers/gpu/drm/panel/Kconfig
>> +++ b/drivers/gpu/drm/panel/Kconfig
>> @@ -40,4 +40,11 @@ config DRM_PANEL_SHARP_LQ101R1SX01
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called panel-sharp-lq101r1sx01.
>>
>> +config DRM_PANEL_CRYSTALCOVE
>> +	tristate "Crystalcove PMIC controlled panel"
>> +	depends on INTEL_SOC_PMIC
>
> INTEL_SOC_PMIC is still a bool (and requires I2C=y), and IMO should be
> fixed to support tristate. Or are there fundamental reasons this can't
> be done?

Jacob, can you answer this one for the PMIC driver. This has come up 
multiple times earlier also in some discussions.

>
>> +	help
>> +	  Say Y here if you want to enable support for DSI panel and backlight
>> +	  control using crystalcove pmic. This is used for BYT CR configurations
>> +
>>   endmenu
>> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
>> index 4b2a043..c05824e 100644
>> --- a/drivers/gpu/drm/panel/Makefile
>> +++ b/drivers/gpu/drm/panel/Makefile
>> @@ -2,3 +2,4 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>>   obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
>>   obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
>>   obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>> +obj-$(CONFIG_DRM_PANEL_CRYSTALCOVE) += panel-crystalcove.o
>> diff --git a/drivers/gpu/drm/panel/panel-crystalcove.c b/drivers/gpu/drm/panel/panel-crystalcove.c
>> new file mode 100644
>> index 0000000..22f7ff5
>> --- /dev/null
>> +++ b/drivers/gpu/drm/panel/panel-crystalcove.c
>> @@ -0,0 +1,191 @@
>> +/*
>> + * Copyright © 2006-2014 Intel Corporation
>
> 2006?!
>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + *	Shobhit Kumar <shobhit.kumar@intel.com>
>> + */
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_panel.h>
>> +#include <linux/mfd/intel_soc_pmic.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#define PMIC_PANEL_EN		0x52
>> +#define PMIC_PWM_EN		0x51
>> +#define PMIC_BKL_EN		0x4B
>> +#define PMIC_PWM_LEVEL		0x4E
>> +
>> +struct crystalcove_panel {
>> +	struct drm_panel base;
>> +	char name[NAME_MAX];
>
> Unused.
>
>> +	bool prepared;
>> +	bool enabled;
>> +
>> +	struct mutex lock;
>
> Unused.
>
>> +
>> +	/* crystal cove pmic regmap */
>> +	struct regmap *regmap;
>> +};
>> +
>> +static inline struct crystalcove_panel *to_crystalcove_panel(struct drm_panel *panel)
>> +{
>> +	return container_of(panel, struct crystalcove_panel, base);
>> +}
>> +
>> +static int crystalcove_panel_disable(struct drm_panel *panel)
>> +{
>> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
>> +
>> +	if (!p->enabled)
>> +		return 0;
>> +
>> +	DRM_DEBUG_KMS("\n");
>> +
>> +	/* invoke the pmic driver */
>> +	regmap_write(p->regmap, PMIC_PANEL_EN, 0x00);
>> +
>> +	/* Disable backlight as well */
>> +	regmap_write(p->regmap, PMIC_PWM_LEVEL, 0);
>> +	msleep(20);
>
> Seems like a long delay.
>
>> +	regmap_write(p->regmap, PMIC_PWM_EN, 0x00);
>> +	regmap_write(p->regmap, PMIC_BKL_EN, 0x7F);
>> +
>> +	p->enabled = false;
>> +
>> +	return 0;
>> +}
>> +
>> +static int crystalcove_panel_unprepare(struct drm_panel *panel)
>> +{
>> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
>> +
>> +	if (!p->prepared)
>> +		return 0;
>> +
>> +	/* Nothing needed */
>> +	p->prepared = false;
>> +
>> +	return 0;
>> +}
>> +
>> +static int crystalcove_panel_prepare(struct drm_panel *panel)
>> +{
>> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
>> +
>> +	if (p->prepared)
>> +		return 0;
>> +
>> +	/* Nothing needed */
>> +	p->prepared = true;
>> +
>> +	return 0;
>> +}
>
> If you don't need prepare/unprepare, you can just leave them out
> altogether. It's easy to add them if you need them later.
>
>> +
>> +static int crystalcove_panel_enable(struct drm_panel *panel)
>> +{
>> +	struct crystalcove_panel *p = to_crystalcove_panel(panel);
>> +
>> +	if (p->enabled)
>> +		return 0;
>> +
>> +	DRM_DEBUG_KMS("\n");
>> +
>> +	/* invoke the pmic driver */
>> +	regmap_write(p->regmap, PMIC_PANEL_EN, 0x01);
>> +
>> +	/* Enable BKL as well */
>> +	regmap_write(p->regmap, PMIC_BKL_EN, 0xFF);
>> +	regmap_write(p->regmap, PMIC_PWM_EN, 0x01);
>> +	msleep(20);
>> +	regmap_write(p->regmap, PMIC_PWM_LEVEL, 255);
>> +
>> +	p->enabled = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct drm_panel_funcs crystalcove_panel_funcs = {
>> +	.disable = crystalcove_panel_disable,
>> +	.unprepare = crystalcove_panel_unprepare,
>> +	.prepare = crystalcove_panel_prepare,
>> +	.enable = crystalcove_panel_enable,
>> +};
>> +
>> +static int crystalcove_panel_probe(struct platform_device *pdev)
>> +{
>> +	struct crystalcove_panel *panel;
>> +	int retval;
>> +	struct device *dev = pdev->dev.parent;
>> +	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
>> +
>> +	panel = devm_kzalloc(&pdev->dev, sizeof(*panel), GFP_KERNEL);
>> +	if (!panel)
>> +		return -ENOMEM;
>> +
>> +	DRM_DEBUG_KMS("\n");
>> +
>> +	platform_set_drvdata(pdev, panel);
>> +
>> +	strcpy(panel->base.name, "crystal_cove_panel");
>> +	panel->regmap = pmic->regmap;
>> +
>> +	regmap_read(panel->regmap, PMIC_PANEL_EN, &retval);
>> +	panel->enabled = panel->prepared = retval ? true : false;
>
> For bools you can just assign retval, or if you want to emphasis it's a
> bool you can use !!retval.
>
>> +
>> +	drm_panel_init(&panel->base);
>> +	panel->base.dev = dev;
>> +	panel->base.funcs = &crystalcove_panel_funcs;
>> +
>> +	drm_panel_add(&panel->base);
>> +
>> +	return 0;
>> +}
>> +
>> +static int crystalcove_panel_remove(struct platform_device *pdev)
>> +{
>> +	struct crystalcove_panel *panel = platform_get_drvdata(pdev);
>> +
>> +	DRM_DEBUG_KMS("\n");
>> +
>> +	drm_panel_detach(&panel->base);
>> +	drm_panel_remove(&panel->base);
>> +
>> +	crystalcove_panel_disable(&panel->base);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver crystalcove_panel_driver = {
>> +	.probe = crystalcove_panel_probe,
>> +	.remove = crystalcove_panel_remove,
>> +	.driver = {
>> +		.name = "crystal_cove_panel",
>> +	},
>> +};
>> +
>> +module_platform_driver(crystalcove_panel_driver);
>> +
>> +MODULE_AUTHOR("Shobhit Kumar <shobhit.kumar@linux.intel.com");
>> +MODULE_DESCRIPTION("Intel Crystal Cove Panel Driver");
>> +MODULE_LICENSE("GPL v2");
>
> This conflicts with the copyright header of the file.
>
>
>> --
>> 1.9.1
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC v2 3/4] drm/panel: Add new panel driver based on crystal cove pmic

[Kumar, Shobhit]

On 1/12/2015 1:56 PM, Kumar, Shobhit wrote:
> On 1/9/2015 6:38 PM, Jani Nikula wrote:
>> On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>>> This driver provides support for the "crystal_cove_panel" cell device.
>>> On BYT-T pmic has to be used to enable/disable panel.
>>
>> This needs to be sent to dri-devel.
>
> Will do for the updated patch after addressing all your comments.

Also do we really need to make it as part of drm. How about keeping this 
internal to i915 as most likely no one else other than i915 will use it ?

>
>>
>> With the comments below addressed, and with the disclaimer that I have
>> no idea about the pmic registers or required sleeps, this is
>>
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>>
>>>
>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>> ---

>>> +module_platform_driver(crystalcove_panel_driver);
>>> +
>>> +MODULE_AUTHOR("Shobhit Kumar <shobhit.kumar@linux.intel.com");
>>> +MODULE_DESCRIPTION("Intel Crystal Cove Panel Driver");
>>> +MODULE_LICENSE("GPL v2");
>>
>> This conflicts with the copyright header of the file.

I have seen drivers in kernel header both with Copyright and GPL V2. 
Example - "drivers/gpu/drm/panel/panel-s6e8aa0.c"

I can change to  MODULE_LICENSE("GPL and additional rights");

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

[RFC v2 4/4] drm/i915: Enable DSI panel enable/disable based on PMIC

[Shobhit Kumar]

This allows for proper PPS during enable/disable of BYT-T platforms
where these signals are routed through PMIC. Needs DRM_PANEL to be
selected by default as well

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/Kconfig               |  1 +
 drivers/gpu/drm/i915/intel_dsi.c           | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.h           |  6 ++++++
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  1 +
 4 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 4e39ab3..3210dbb 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -18,6 +18,7 @@ config DRM_I915
 	select INPUT if ACPI
 	select ACPI_VIDEO if ACPI
 	select ACPI_BUTTON if ACPI
+	select DRM_PANEL
 	help
 	  Choose this option if you have a system that has "Intel Graphics
 	  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 42b6d6f..431e7cb 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -26,6 +26,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_panel.h>
 #include <drm/i915_drm.h>
 #include <linux/slab.h>
 #include "i915_drv.h"
@@ -230,6 +231,8 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 
 	DRM_DEBUG_KMS("\n");
 
+	drm_panel_enable(intel_dsi->panel);
+
 	/* Disable DPOunit clock gating, can stall pipe
 	 * and we need DPLL REFA always enabled */
 	tmp = I915_READ(DPLL(pipe));
@@ -392,6 +395,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
 
 	msleep(intel_dsi->panel_off_delay);
 	msleep(intel_dsi->panel_pwr_cycle_delay);
+
+	drm_panel_disable(intel_dsi->panel);
 }
 
 static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
@@ -896,6 +901,17 @@ void intel_dsi_init(struct drm_device *dev)
 	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
 
+	/* Initialize the PMIC based drm_panel if available on the platform */
+	if (intel_dsi->pps_blc == PPS_BLC_PMIC) {
+		intel_dsi->panel = name_drm_find_panel("crystal_cove_panel");
+		if (!intel_dsi->panel) {
+			DRM_ERROR("PMIC Panel control will not work !!\n");
+			return;
+		}
+
+		drm_panel_attach(intel_dsi->panel, connector);
+	}
+
 	return;
 
 err:
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 8fe2064..4a9242d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -33,6 +33,9 @@
 #define DSI_DUAL_LINK_FRONT_BACK	1
 #define DSI_DUAL_LINK_PIXEL_ALT		2
 
+#define PPS_BLC_PMIC	0
+#define PPS_BLC_SOC	1
+
 struct intel_dsi_device {
 	unsigned int panel_id;
 	const char *name;
@@ -83,6 +86,8 @@ struct intel_dsi {
 
 	struct intel_connector *attached_connector;
 
+	struct drm_panel *panel;
+
 	/* bit mask of ports being driven */
 	u16 ports;
 
@@ -116,6 +121,7 @@ struct intel_dsi {
 	u32 dphy_reg;
 	u32 video_frmt_cfg_bits;
 	u16 lp_byte_clk;
+	u8 pps_blc;
 
 	/* timeouts in byte clocks */
 	u16 lp_rx_timeout;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 5493aef..0612d33 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -297,6 +297,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
 	intel_dsi->dual_link = mipi_config->dual_link;
 	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
+	intel_dsi->pps_blc = mipi_config->pwm_blc;
 
 	if (intel_dsi->dual_link)
 		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
-- 
1.9.1

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

Re: [RFC v2 4/4] drm/i915: Enable DSI panel enable/disable based on PMIC

[Jani Nikula]

On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> This allows for proper PPS during enable/disable of BYT-T platforms
> where these signals are routed through PMIC. Needs DRM_PANEL to be
> selected by default as well
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/Kconfig               |  1 +
>  drivers/gpu/drm/i915/intel_dsi.c           | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.h           |  6 ++++++
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  1 +
>  4 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 4e39ab3..3210dbb 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -18,6 +18,7 @@ config DRM_I915
>  	select INPUT if ACPI
>  	select ACPI_VIDEO if ACPI
>  	select ACPI_BUTTON if ACPI
> +	select DRM_PANEL
>  	help
>  	  Choose this option if you have a system that has "Intel Graphics
>  	  Media Accelerator" or "HD Graphics" integrated graphics,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 42b6d6f..431e7cb 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -26,6 +26,7 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_panel.h>
>  #include <drm/i915_drm.h>
>  #include <linux/slab.h>
>  #include "i915_drv.h"
> @@ -230,6 +231,8 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	drm_panel_enable(intel_dsi->panel);
> +
>  	/* Disable DPOunit clock gating, can stall pipe
>  	 * and we need DPLL REFA always enabled */
>  	tmp = I915_READ(DPLL(pipe));
> @@ -392,6 +395,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>  
>  	msleep(intel_dsi->panel_off_delay);
>  	msleep(intel_dsi->panel_pwr_cycle_delay);
> +
> +	drm_panel_disable(intel_dsi->panel);
>  }

As I explained in a private mail, I intend to convert all of our panel
driver callbacks to the drm_panel model. So we'd have two sets of
drm_panel_* hooks sprinkled here, one for handling the panel power and
the other for the generic vbt panel driver. An alternative would be to
have the vbt panel driver handle this internally, but I don't really
know which one is better. In any case this has a fairly small footprint
so it's easy to change one way or another.

>  
>  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> @@ -896,6 +901,17 @@ void intel_dsi_init(struct drm_device *dev)
>  	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
>  	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>  
> +	/* Initialize the PMIC based drm_panel if available on the platform */
> +	if (intel_dsi->pps_blc == PPS_BLC_PMIC) {
> +		intel_dsi->panel = name_drm_find_panel("crystal_cove_panel");
> +		if (!intel_dsi->panel) {
> +			DRM_ERROR("PMIC Panel control will not work !!\n");
> +			return;
> +		}
> +
> +		drm_panel_attach(intel_dsi->panel, connector);
> +	}
> +

I think there's an init order problem here. If the panel driver hasn't
been registered yet this will fail. I don't know what the answer to that
should be.

>  	return;
>  
>  err:
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 8fe2064..4a9242d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -33,6 +33,9 @@
>  #define DSI_DUAL_LINK_FRONT_BACK	1
>  #define DSI_DUAL_LINK_PIXEL_ALT		2
>  
> +#define PPS_BLC_PMIC	0
> +#define PPS_BLC_SOC	1
> +
>  struct intel_dsi_device {
>  	unsigned int panel_id;
>  	const char *name;
> @@ -83,6 +86,8 @@ struct intel_dsi {
>  
>  	struct intel_connector *attached_connector;
>  
> +	struct drm_panel *panel;
> +
>  	/* bit mask of ports being driven */
>  	u16 ports;
>  
> @@ -116,6 +121,7 @@ struct intel_dsi {
>  	u32 dphy_reg;
>  	u32 video_frmt_cfg_bits;
>  	u16 lp_byte_clk;
> +	u8 pps_blc;
>  
>  	/* timeouts in byte clocks */
>  	u16 lp_rx_timeout;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 5493aef..0612d33 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -297,6 +297,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
>  	intel_dsi->dual_link = mipi_config->dual_link;
>  	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
> +	intel_dsi->pps_blc = mipi_config->pwm_blc;

If the drm_panel for crystal cove is going to be like in this patch, I
think this part belongs in intel_dsi.c.

>  
>  	if (intel_dsi->dual_link)
>  		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
> -- 
> 1.9.1
>

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

Re: [RFC v2 4/4] drm/i915: Enable DSI panel enable/disable based on PMIC

[Kumar, Shobhit]

On 1/9/2015 6:47 PM, Jani Nikula wrote:
> On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>> This allows for proper PPS during enable/disable of BYT-T platforms
>> where these signals are routed through PMIC. Needs DRM_PANEL to be
>> selected by default as well
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/Kconfig               |  1 +
>>   drivers/gpu/drm/i915/intel_dsi.c           | 16 ++++++++++++++++
>>   drivers/gpu/drm/i915/intel_dsi.h           |  6 ++++++
>>   drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  1 +
>>   4 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
>> index 4e39ab3..3210dbb 100644
>> --- a/drivers/gpu/drm/i915/Kconfig
>> +++ b/drivers/gpu/drm/i915/Kconfig
>> @@ -18,6 +18,7 @@ config DRM_I915
>>   	select INPUT if ACPI
>>   	select ACPI_VIDEO if ACPI
>>   	select ACPI_BUTTON if ACPI
>> +	select DRM_PANEL
>>   	help
>>   	  Choose this option if you have a system that has "Intel Graphics
>>   	  Media Accelerator" or "HD Graphics" integrated graphics,
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 42b6d6f..431e7cb 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -26,6 +26,7 @@
>>   #include <drm/drmP.h>
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_edid.h>
>> +#include <drm/drm_panel.h>
>>   #include <drm/i915_drm.h>
>>   #include <linux/slab.h>
>>   #include "i915_drv.h"
>> @@ -230,6 +231,8 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>>
>>   	DRM_DEBUG_KMS("\n");
>>
>> +	drm_panel_enable(intel_dsi->panel);
>> +
>>   	/* Disable DPOunit clock gating, can stall pipe
>>   	 * and we need DPLL REFA always enabled */
>>   	tmp = I915_READ(DPLL(pipe));
>> @@ -392,6 +395,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>>
>>   	msleep(intel_dsi->panel_off_delay);
>>   	msleep(intel_dsi->panel_pwr_cycle_delay);
>> +
>> +	drm_panel_disable(intel_dsi->panel);
>>   }
>
> As I explained in a private mail, I intend to convert all of our panel
> driver callbacks to the drm_panel model. So we'd have two sets of
> drm_panel_* hooks sprinkled here, one for handling the panel power and
> the other for the generic vbt panel driver. An alternative would be to
> have the vbt panel driver handle this internally, but I don't really
> know which one is better. In any case this has a fairly small footprint
> so it's easy to change one way or another.

PMIC based control is something unique for current platforms (BYT/CHT) 
and all future platforms will go with SoC based control. Given that, I 
think we should not put this is VBT panel driver and keep separate.

>
>>
>>   static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>> @@ -896,6 +901,17 @@ void intel_dsi_init(struct drm_device *dev)
>>   	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
>>   	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>>
>> +	/* Initialize the PMIC based drm_panel if available on the platform */
>> +	if (intel_dsi->pps_blc == PPS_BLC_PMIC) {
>> +		intel_dsi->panel = name_drm_find_panel("crystal_cove_panel");
>> +		if (!intel_dsi->panel) {
>> +			DRM_ERROR("PMIC Panel control will not work !!\n");
>> +			return;
>> +		}
>> +
>> +		drm_panel_attach(intel_dsi->panel, connector);
>> +	}
>> +
>
> I think there's an init order problem here. If the panel driver hasn't
> been registered yet this will fail. I don't know what the answer to that
> should be.

Agree and for my testing I made both PMIC and This panel driver as 
in-built and i915 as module. I was hoping for some answer !!

>
>>   	return;
>>
>>   err:
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> index 8fe2064..4a9242d 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -33,6 +33,9 @@
>>   #define DSI_DUAL_LINK_FRONT_BACK	1
>>   #define DSI_DUAL_LINK_PIXEL_ALT		2
>>
>> +#define PPS_BLC_PMIC	0
>> +#define PPS_BLC_SOC	1
>> +
>>   struct intel_dsi_device {
>>   	unsigned int panel_id;
>>   	const char *name;
>> @@ -83,6 +86,8 @@ struct intel_dsi {
>>
>>   	struct intel_connector *attached_connector;
>>
>> +	struct drm_panel *panel;
>> +
>>   	/* bit mask of ports being driven */
>>   	u16 ports;
>>
>> @@ -116,6 +121,7 @@ struct intel_dsi {
>>   	u32 dphy_reg;
>>   	u32 video_frmt_cfg_bits;
>>   	u16 lp_byte_clk;
>> +	u8 pps_blc;
>>
>>   	/* timeouts in byte clocks */
>>   	u16 lp_rx_timeout;
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> index 5493aef..0612d33 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> @@ -297,6 +297,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
>>   	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
>>   	intel_dsi->dual_link = mipi_config->dual_link;
>>   	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
>> +	intel_dsi->pps_blc = mipi_config->pwm_blc;
>
> If the drm_panel for crystal cove is going to be like in this patch, I
> think this part belongs in intel_dsi.c.

If you are suggesting check mipi_config->pwm_blc directly in intel_dsi, 
that can be done. Was just abstracting all VBT fields in generic VBT 
based panel driver

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

Re: [RFC v2 4/4] drm/i915: Enable DSI panel enable/disable based on PMIC

[Daniel Vetter]

On Mon, Jan 12, 2015 at 01:53:08PM +0530, Kumar, Shobhit wrote:
> >
> >>
> >>  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> >>@@ -896,6 +901,17 @@ void intel_dsi_init(struct drm_device *dev)
> >>   fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> >>   intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> >>
> >>+ /* Initialize the PMIC based drm_panel if available on the platform */
> >>+ if (intel_dsi->pps_blc == PPS_BLC_PMIC) {
> >>+ intel_dsi->panel = name_drm_find_panel("crystal_cove_panel");
> >>+ if (!intel_dsi->panel) {
> >>+ DRM_ERROR("PMIC Panel control will not work !!\n");
> >>+ return;
> >>+ }
> >>+
> >>+ drm_panel_attach(intel_dsi->panel, connector);
> >>+ }
> >>+
> >
> >I think there's an init order problem here. If the panel driver hasn't
> >been registered yet this will fail. I don't know what the answer to that
> >should be.
>
> Agree and for my testing I made both PMIC and This panel driver as in-built
> and i915 as module. I was hoping for some answer !!

The answer is to use the component framework and delayed probing. Afaik
there's unfortunately not yet any ready-made integration with drm
panel/bridge yet since this is all very much in flux.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC v2 0/4] Crystal Cove PMIC based Panel and Backlight Control

[Kumar, Shobhit]

On 1/2/2015 7:11 PM, Shobhit Kumar wrote:
> Hi All
> Please find modifed set of patches. Sending as a separate thread as initial
> patches were a crude implementation to trigger discussion, but these have been
> tested and also do not need intel_soc_pmic_writeb/readb functionaly, but uses
> the regmap interface as suggested by Jacob.
>
> These patches implement a drm_panel as a platform driver for the mfd_cell device
> declared in intel_soc_pmic_core.c.
>
> Still there are opens, where I need closure -
> 1. Added a new drm_panel, but how to find the panel in lack of OF info. For now
>     added a drm_panel function to find panel by name.
> 2. Backlight also needs similar pmic based control. Is it okay to add a backlight
>     class driver also as part of this panel driver ?
>

Are there any suggestions/comments for the approach/opens in this patch 
set ?

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

Re: [RFC v2 0/4] Crystal Cove PMIC based Panel and Backlight Control

[Jani Nikula]

On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> Hi All
> Please find modifed set of patches. Sending as a separate thread as initial
> patches were a crude implementation to trigger discussion, but these have been 
> tested and also do not need intel_soc_pmic_writeb/readb functionaly, but uses
> the regmap interface as suggested by Jacob.
>
> These patches implement a drm_panel as a platform driver for the mfd_cell device
> declared in intel_soc_pmic_core.c. 
>
> Still there are opens, where I need closure - 
> 1. Added a new drm_panel, but how to find the panel in lack of OF info. For now
>    added a drm_panel function to find panel by name.

I don't have an answer.

> 2. Backlight also needs similar pmic based control. Is it okay to add a backlight
>    class driver also as part of this panel driver ?

Is it fathomable you have designs with pmic panel enable/disable but soc
pwm backlight control? If yes, I think you need to separate the two. If
they always go together, I think you can keep them together.

BR,
Jani.

>
> For now I am doing Backlight Enable/Disable also during panel/enable as this will
> at least save power. Backlight control will need a backlight class driver.
>
> Regards
> Shobhit
>
> Shobhit Kumar (4):
>   drm: Add support to find drm_panel by name
>   mfd: Add a new cell device for panel controlled by crystal cove pmic
>   drm/panel: Add new panel driver based on crystal cove pmic
>   drm/i915: Enable DSI panel enable/disable based on PMIC
>
>  drivers/gpu/drm/drm_panel.c                |  18 +++
>  drivers/gpu/drm/i915/Kconfig               |   1 +
>  drivers/gpu/drm/i915/intel_dsi.c           |  16 +++
>  drivers/gpu/drm/i915/intel_dsi.h           |   6 +
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   1 +
>  drivers/gpu/drm/panel/Kconfig              |   7 ++
>  drivers/gpu/drm/panel/Makefile             |   1 +
>  drivers/gpu/drm/panel/panel-crystalcove.c  | 191 +++++++++++++++++++++++++++++
>  drivers/mfd/intel_soc_pmic_crc.c           |   3 +
>  include/drm/drm_panel.h                    |   3 +
>  10 files changed, 247 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-crystalcove.c
>
> -- 
> 1.9.1
>

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