Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-07-12 Thread Thierry Reding 
On Tue, Jul 12, 2016 at 12:34:02PM +0200, Daniel Vetter wrote:
> On Mon, Jul 11, 2016 at 02:29:37PM +0200, Thierry Reding wrote:
> > On Thu, Jun 16, 2016 at 06:02:53PM +0100, Emil Velikov wrote:
> > > On 16 June 2016 at 04:00, Vinay Simha BN  wrote:
> > [...]
> > > > +static int jdi_panel_disable(struct drm_panel *panel)
> > > > +{
> > > > +   struct jdi_panel *jdi = to_jdi_panel(panel);
> > > > +
> > > > +   if (!jdi->enabled)
> > > > +   return 0;
> > > > +
> > > Thinking out loud:
> > > 
> > > Thierry,
> > > Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
> > > tweak the helpers respectively ? Is there any specific reason for
> > > keeping these in the drivers ?
> > 
> > Yes, I think that would make sense eventually. It's clearly a recurring
> > pattern. Ideally nothing would be calling these functions more than once
> > and thereby making the checks unnecessary. In practice that may mean
> > that we need to put the variables and checks into the drm/panel core
> > because display drivers (as opposed to a sane core implementation) call
> > these. I suppose we could encourage proper usage by adding a couple of
> > WARNs here and there if expectations aren't met.
> > 
> > I don't think doing this is terribly urgent because it's easy to rip out
> > of drivers once the drm/panel core supports it. And it's something that
> > we could even leave within drivers when the core supports it, so trivial
> > to remove one by one after the core patches have landed.
> 
> As long as we have non-atomic drm drivers using this multiple
> enable/disable calls can happen. Atomic drivers should screw this up
> (ignoring a few misguided ones that mix atomic and legacy helpers in bad
> ways, but those are getting fixed).
> 
> I think a good plan would be:
> 1. Move this tracking into drm panel helpers, ditch it from all drivers.
> 2. Add WARN_ON for multiple enables/disables, but only for DRIVER_ATOMIC.
> 
> Makes sure we can remove this boilerplate, makes sure that atomic drivers
> are consistent, leaves existing drivers unharmed.

Yeah, that sounds like a reasonable plan.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-07-12 Thread Thierry Reding 
On Tue, Jul 12, 2016 at 12:34:02PM +0200, Daniel Vetter wrote:
> On Mon, Jul 11, 2016 at 02:29:37PM +0200, Thierry Reding wrote:
> > On Thu, Jun 16, 2016 at 06:02:53PM +0100, Emil Velikov wrote:
> > > On 16 June 2016 at 04:00, Vinay Simha BN  wrote:
> > [...]
> > > > +static int jdi_panel_disable(struct drm_panel *panel)
> > > > +{
> > > > +   struct jdi_panel *jdi = to_jdi_panel(panel);
> > > > +
> > > > +   if (!jdi->enabled)
> > > > +   return 0;
> > > > +
> > > Thinking out loud:
> > > 
> > > Thierry,
> > > Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
> > > tweak the helpers respectively ? Is there any specific reason for
> > > keeping these in the drivers ?
> > 
> > Yes, I think that would make sense eventually. It's clearly a recurring
> > pattern. Ideally nothing would be calling these functions more than once
> > and thereby making the checks unnecessary. In practice that may mean
> > that we need to put the variables and checks into the drm/panel core
> > because display drivers (as opposed to a sane core implementation) call
> > these. I suppose we could encourage proper usage by adding a couple of
> > WARNs here and there if expectations aren't met.
> > 
> > I don't think doing this is terribly urgent because it's easy to rip out
> > of drivers once the drm/panel core supports it. And it's something that
> > we could even leave within drivers when the core supports it, so trivial
> > to remove one by one after the core patches have landed.
> 
> As long as we have non-atomic drm drivers using this multiple
> enable/disable calls can happen. Atomic drivers should screw this up
> (ignoring a few misguided ones that mix atomic and legacy helpers in bad
> ways, but those are getting fixed).
> 
> I think a good plan would be:
> 1. Move this tracking into drm panel helpers, ditch it from all drivers.
> 2. Add WARN_ON for multiple enables/disables, but only for DRIVER_ATOMIC.
> 
> Makes sure we can remove this boilerplate, makes sure that atomic drivers
> are consistent, leaves existing drivers unharmed.

Yeah, that sounds like a reasonable plan.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-07-12 Thread Daniel Vetter 
On Mon, Jul 11, 2016 at 02:29:37PM +0200, Thierry Reding wrote:
> On Thu, Jun 16, 2016 at 06:02:53PM +0100, Emil Velikov wrote:
> > On 16 June 2016 at 04:00, Vinay Simha BN  wrote:
> [...]
> > > +static int jdi_panel_disable(struct drm_panel *panel)
> > > +{
> > > +   struct jdi_panel *jdi = to_jdi_panel(panel);
> > > +
> > > +   if (!jdi->enabled)
> > > +   return 0;
> > > +
> > Thinking out loud:
> > 
> > Thierry,
> > Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
> > tweak the helpers respectively ? Is there any specific reason for
> > keeping these in the drivers ?
> 
> Yes, I think that would make sense eventually. It's clearly a recurring
> pattern. Ideally nothing would be calling these functions more than once
> and thereby making the checks unnecessary. In practice that may mean
> that we need to put the variables and checks into the drm/panel core
> because display drivers (as opposed to a sane core implementation) call
> these. I suppose we could encourage proper usage by adding a couple of
> WARNs here and there if expectations aren't met.
> 
> I don't think doing this is terribly urgent because it's easy to rip out
> of drivers once the drm/panel core supports it. And it's something that
> we could even leave within drivers when the core supports it, so trivial
> to remove one by one after the core patches have landed.

As long as we have non-atomic drm drivers using this multiple
enable/disable calls can happen. Atomic drivers should screw this up
(ignoring a few misguided ones that mix atomic and legacy helpers in bad
ways, but those are getting fixed).

I think a good plan would be:
1. Move this tracking into drm panel helpers, ditch it from all drivers.
2. Add WARN_ON for multiple enables/disables, but only for DRIVER_ATOMIC.

Makes sure we can remove this boilerplate, makes sure that atomic drivers
are consistent, leaves existing drivers unharmed.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-07-12 Thread Daniel Vetter 
On Mon, Jul 11, 2016 at 02:29:37PM +0200, Thierry Reding wrote:
> On Thu, Jun 16, 2016 at 06:02:53PM +0100, Emil Velikov wrote:
> > On 16 June 2016 at 04:00, Vinay Simha BN  wrote:
> [...]
> > > +static int jdi_panel_disable(struct drm_panel *panel)
> > > +{
> > > +   struct jdi_panel *jdi = to_jdi_panel(panel);
> > > +
> > > +   if (!jdi->enabled)
> > > +   return 0;
> > > +
> > Thinking out loud:
> > 
> > Thierry,
> > Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
> > tweak the helpers respectively ? Is there any specific reason for
> > keeping these in the drivers ?
> 
> Yes, I think that would make sense eventually. It's clearly a recurring
> pattern. Ideally nothing would be calling these functions more than once
> and thereby making the checks unnecessary. In practice that may mean
> that we need to put the variables and checks into the drm/panel core
> because display drivers (as opposed to a sane core implementation) call
> these. I suppose we could encourage proper usage by adding a couple of
> WARNs here and there if expectations aren't met.
> 
> I don't think doing this is terribly urgent because it's easy to rip out
> of drivers once the drm/panel core supports it. And it's something that
> we could even leave within drivers when the core supports it, so trivial
> to remove one by one after the core patches have landed.

As long as we have non-atomic drm drivers using this multiple
enable/disable calls can happen. Atomic drivers should screw this up
(ignoring a few misguided ones that mix atomic and legacy helpers in bad
ways, but those are getting fixed).

I think a good plan would be:
1. Move this tracking into drm panel helpers, ditch it from all drivers.
2. Add WARN_ON for multiple enables/disables, but only for DRIVER_ATOMIC.

Makes sure we can remove this boilerplate, makes sure that atomic drivers
are consistent, leaves existing drivers unharmed.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-07-11 Thread Thierry Reding 
On Thu, Jun 16, 2016 at 06:02:53PM +0100, Emil Velikov wrote:
> On 16 June 2016 at 04:00, Vinay Simha BN  wrote:
[...]
> > +static int jdi_panel_disable(struct drm_panel *panel)
> > +{
> > +   struct jdi_panel *jdi = to_jdi_panel(panel);
> > +
> > +   if (!jdi->enabled)
> > +   return 0;
> > +
> Thinking out loud:
> 
> Thierry,
> Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
> tweak the helpers respectively ? Is there any specific reason for
> keeping these in the drivers ?

Yes, I think that would make sense eventually. It's clearly a recurring
pattern. Ideally nothing would be calling these functions more than once
and thereby making the checks unnecessary. In practice that may mean
that we need to put the variables and checks into the drm/panel core
because display drivers (as opposed to a sane core implementation) call
these. I suppose we could encourage proper usage by adding a couple of
WARNs here and there if expectations aren't met.

I don't think doing this is terribly urgent because it's easy to rip out
of drivers once the drm/panel core supports it. And it's something that
we could even leave within drivers when the core supports it, so trivial
to remove one by one after the core patches have landed.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-07-11 Thread Thierry Reding 
On Thu, Jun 16, 2016 at 06:02:53PM +0100, Emil Velikov wrote:
> On 16 June 2016 at 04:00, Vinay Simha BN  wrote:
[...]
> > +static int jdi_panel_disable(struct drm_panel *panel)
> > +{
> > +   struct jdi_panel *jdi = to_jdi_panel(panel);
> > +
> > +   if (!jdi->enabled)
> > +   return 0;
> > +
> Thinking out loud:
> 
> Thierry,
> Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
> tweak the helpers respectively ? Is there any specific reason for
> keeping these in the drivers ?

Yes, I think that would make sense eventually. It's clearly a recurring
pattern. Ideally nothing would be calling these functions more than once
and thereby making the checks unnecessary. In practice that may mean
that we need to put the variables and checks into the drm/panel core
because display drivers (as opposed to a sane core implementation) call
these. I suppose we could encourage proper usage by adding a couple of
WARNs here and there if expectations aren't met.

I don't think doing this is terribly urgent because it's easy to rip out
of drivers once the drm/panel core supports it. And it's something that
we could even leave within drivers when the core supports it, so trivial
to remove one by one after the core patches have landed.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-06-16 Thread Emil Velikov 
Hi Vinay,

I belive I've spotted a few issues. If my understanding is correct,
then I'll defer to Thierry if he'd like them fixed here, or as
follow-ups.

On 16 June 2016 at 04:00, Vinay Simha BN  wrote:


> +#define PANEL_NUM_REGULATORS   3
> +
Nit: #define PANEL_NUM_REGULATORS ARRAY_SIZE(regulator_names) or just
drop the extra define and use the latter directly ?


> +static int jdi_panel_init(struct jdi_panel *jdi)
> +{


> +   if (ret < 0)
> +   return ret;
> +
> +   return 0;
Nit: The above three lines can become one - "return ret;"

> +}
> +
> +static int jdi_panel_on(struct jdi_panel *jdi)
> +{
> +   struct mipi_dsi_device *dsi = jdi->dsi;
> +   int ret;
> +
> +   dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +   ret = mipi_dsi_dcs_set_display_on(dsi);
> +   if (ret < 0)
> +   return ret;
> +
> +   return 0;
Ditto.


> +static int jdi_panel_disable(struct drm_panel *panel)
> +{
> +   struct jdi_panel *jdi = to_jdi_panel(panel);
> +
> +   if (!jdi->enabled)
> +   return 0;
> +
Thinking out loud:

Thierry,
Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
tweak the helpers respectively ? Is there any specific reason for
keeping these in the drivers ?


> +   if (jdi->backlight) {
We seems to be bailing out of jdi_panel_add() when this is NULL. Thus
we can omit the check.



> +static int jdi_panel_unprepare(struct drm_panel *panel)
> +{
> +   struct jdi_panel *jdi = to_jdi_panel(panel);
> +   struct device *dev = >dsi->dev;
> +   int ret;
> +
> +   if (!jdi->prepared)
> +   return 0;
> +
> +   ret = jdi_panel_off(jdi);
> +   if (ret) {
> +   dev_err(panel->dev, "failed to set panel off: %d\n", ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_bulk_disable(ARRAY_SIZE(jdi->supplies), 
> jdi->supplies);
> +   if (ret < 0) {
> +   dev_err(dev, "regulator disable failed, %d\n", ret);
> +   return ret;
> +   }
> +
Since we cannot recover from most/all of the above I'm thinking if one
shouldn't drop the "return ret" lines. Same goes for jdi_panel_off().


> +   if (jdi->reset_gpio)
> +   gpiod_set_value(jdi->reset_gpio, 0);
> +
> +   if (jdi->enable_gpio)
Drop these two checks. The gpios are required, thus one should bail
out in jdi_panel_add()



> +static int jdi_panel_prepare(struct drm_panel *panel)
> +{

> +   if (jdi->reset_gpio) {
> +   gpiod_set_value(jdi->reset_gpio, 1);
> +   usleep_range(10, 20);
> +   }
> +
> +   if (jdi->enable_gpio) {
> +   gpiod_set_value(jdi->enable_gpio, 1);
> +   usleep_range(10, 20);
> +   }
> +


> +poweroff:
> +   if (jdi->reset_gpio)
> +   gpiod_set_value(jdi->reset_gpio, 0);
> +   if (jdi->enable_gpio)
> +   gpiod_set_value(jdi->enable_gpio, 0);
> +
Generic suggestion/nitpick: Please keep the teardown order the inverse
of the setup one. In here one could/should handle enable_gpio first
and then reset_gpio.

> +   return ret;
> +}
> +
> +static int jdi_panel_enable(struct drm_panel *panel)
> +{
> +   struct jdi_panel *jdi = to_jdi_panel(panel);
> +
> +   if (jdi->enabled)
> +   return 0;
> +
> +   if (jdi->backlight) {
Analogous to jdi_panel_disable - drop the check ?



> +static int jdi_panel_add(struct jdi_panel *jdi)
> +{


> +   jdi->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +   if (IS_ERR(jdi->reset_gpio)) {
> +   dev_err(dev, "cannot get reset-gpios %ld\n",
> +   PTR_ERR(jdi->reset_gpio));
> +   jdi->reset_gpio = NULL;
> +   } else {
> +   gpiod_direction_output(jdi->reset_gpio, 0);
> +   }
> +
> +   jdi->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> +   if (IS_ERR(jdi->enable_gpio)) {
> +   dev_err(dev, "cannot get enable-gpio %ld\n",
> +   PTR_ERR(jdi->enable_gpio));
> +   jdi->enable_gpio = NULL;
> +   } else {
> +   gpiod_direction_output(jdi->enable_gpio, 0);
> +   }
> +
As mentioned above - since these two are required, thus we should
error out of this function. Right ?

> +   jdi->backlight = drm_panel_create_dsi_backlight(jdi->dsi);
> +   if (!jdi->backlight)
> +   return -EPROBE_DEFER;
> +
> +   drm_panel_init(>base);
> +   jdi->base.funcs = _panel_funcs;
> +   jdi->base.dev = >dsi->dev;
> +
> +   ret = drm_panel_add(>base);
> +   if (ret < 0)
> +   return ret;
> +
> +   return 0;
return ret;

Regards,
Emil

Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-06-16 Thread Emil Velikov 
Hi Vinay,

I belive I've spotted a few issues. If my understanding is correct,
then I'll defer to Thierry if he'd like them fixed here, or as
follow-ups.

On 16 June 2016 at 04:00, Vinay Simha BN  wrote:


> +#define PANEL_NUM_REGULATORS   3
> +
Nit: #define PANEL_NUM_REGULATORS ARRAY_SIZE(regulator_names) or just
drop the extra define and use the latter directly ?


> +static int jdi_panel_init(struct jdi_panel *jdi)
> +{


> +   if (ret < 0)
> +   return ret;
> +
> +   return 0;
Nit: The above three lines can become one - "return ret;"

> +}
> +
> +static int jdi_panel_on(struct jdi_panel *jdi)
> +{
> +   struct mipi_dsi_device *dsi = jdi->dsi;
> +   int ret;
> +
> +   dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +   ret = mipi_dsi_dcs_set_display_on(dsi);
> +   if (ret < 0)
> +   return ret;
> +
> +   return 0;
Ditto.


> +static int jdi_panel_disable(struct drm_panel *panel)
> +{
> +   struct jdi_panel *jdi = to_jdi_panel(panel);
> +
> +   if (!jdi->enabled)
> +   return 0;
> +
Thinking out loud:

Thierry,
Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
tweak the helpers respectively ? Is there any specific reason for
keeping these in the drivers ?


> +   if (jdi->backlight) {
We seems to be bailing out of jdi_panel_add() when this is NULL. Thus
we can omit the check.



> +static int jdi_panel_unprepare(struct drm_panel *panel)
> +{
> +   struct jdi_panel *jdi = to_jdi_panel(panel);
> +   struct device *dev = >dsi->dev;
> +   int ret;
> +
> +   if (!jdi->prepared)
> +   return 0;
> +
> +   ret = jdi_panel_off(jdi);
> +   if (ret) {
> +   dev_err(panel->dev, "failed to set panel off: %d\n", ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_bulk_disable(ARRAY_SIZE(jdi->supplies), 
> jdi->supplies);
> +   if (ret < 0) {
> +   dev_err(dev, "regulator disable failed, %d\n", ret);
> +   return ret;
> +   }
> +
Since we cannot recover from most/all of the above I'm thinking if one
shouldn't drop the "return ret" lines. Same goes for jdi_panel_off().


> +   if (jdi->reset_gpio)
> +   gpiod_set_value(jdi->reset_gpio, 0);
> +
> +   if (jdi->enable_gpio)
Drop these two checks. The gpios are required, thus one should bail
out in jdi_panel_add()



> +static int jdi_panel_prepare(struct drm_panel *panel)
> +{

> +   if (jdi->reset_gpio) {
> +   gpiod_set_value(jdi->reset_gpio, 1);
> +   usleep_range(10, 20);
> +   }
> +
> +   if (jdi->enable_gpio) {
> +   gpiod_set_value(jdi->enable_gpio, 1);
> +   usleep_range(10, 20);
> +   }
> +


> +poweroff:
> +   if (jdi->reset_gpio)
> +   gpiod_set_value(jdi->reset_gpio, 0);
> +   if (jdi->enable_gpio)
> +   gpiod_set_value(jdi->enable_gpio, 0);
> +
Generic suggestion/nitpick: Please keep the teardown order the inverse
of the setup one. In here one could/should handle enable_gpio first
and then reset_gpio.

> +   return ret;
> +}
> +
> +static int jdi_panel_enable(struct drm_panel *panel)
> +{
> +   struct jdi_panel *jdi = to_jdi_panel(panel);
> +
> +   if (jdi->enabled)
> +   return 0;
> +
> +   if (jdi->backlight) {
Analogous to jdi_panel_disable - drop the check ?



> +static int jdi_panel_add(struct jdi_panel *jdi)
> +{


> +   jdi->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +   if (IS_ERR(jdi->reset_gpio)) {
> +   dev_err(dev, "cannot get reset-gpios %ld\n",
> +   PTR_ERR(jdi->reset_gpio));
> +   jdi->reset_gpio = NULL;
> +   } else {
> +   gpiod_direction_output(jdi->reset_gpio, 0);
> +   }
> +
> +   jdi->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> +   if (IS_ERR(jdi->enable_gpio)) {
> +   dev_err(dev, "cannot get enable-gpio %ld\n",
> +   PTR_ERR(jdi->enable_gpio));
> +   jdi->enable_gpio = NULL;
> +   } else {
> +   gpiod_direction_output(jdi->enable_gpio, 0);
> +   }
> +
As mentioned above - since these two are required, thus we should
error out of this function. Right ?

> +   jdi->backlight = drm_panel_create_dsi_backlight(jdi->dsi);
> +   if (!jdi->backlight)
> +   return -EPROBE_DEFER;
> +
> +   drm_panel_init(>base);
> +   jdi->base.funcs = _panel_funcs;
> +   jdi->base.dev = >dsi->dev;
> +
> +   ret = drm_panel_add(>base);
> +   if (ret < 0)
> +   return ret;
> +
> +   return 0;
return ret;

Regards,
Emil

[PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-06-15 Thread Vinay Simha BN 
Add support for the JDI LT070ME05000 WUXGA DSI panel used in
Nexus 7 2013 devices.

Programming sequence for the panel is was originally found in the
android-msm-flo-3.4-lollipop-release branch from:
https://android.googlesource.com/kernel/msm.git

And video mode setting is from dsi-panel-jdi-dualmipi1-video.dtsi
file in:
git://codeaurora.org/kernel/msm-3.10.git  LNX.LA.3.6_rb1.27

Cc: Archit Taneja 
[sumit.semwal: Ported to the drm/panel framework]
Signed-off-by: Sumit Semwal 
[jstultz: Cherry-picked to mainline, folded down other fixes
 from Vinay and Archit]
Signed-off-by: John Stultz 
[vinay simha bn: removed interface setting cmd mode, video
mode panel setting selection]
Cc: Rob Clark 
Signed-off-by: Vinay Simha BN 

---
v2:
 * incorporated code reviews from theiry, archit
   code style, alphabetical soring in Makefile, Kconfig, regulator_bulk,
   arrays of u8, generic helper function, documentation bindings,

v3:
 * dcs backlight support added
 * tested this panel driver in nexus7 2013 device

v4:
 * backlight interface added in the panel driver
 * incorporated width_mm and height_mm suggested by rob herring

v5:
 * theirry review comments incorporated
   panel model naming consistent, alphabetical soring in Kconfig
   Makefile, MAX_BRIGHTNESS dropped, regulator_names, parameterize
   panel width and height, descprition for control display, cabc
   and interface setting, temporary variable removed, consistent
   error reporting and commit message
 * removed tear on/off, scanline, since these are required only
   for command mode panels
---
 drivers/gpu/drm/panel/Kconfig  |  11 +
 drivers/gpu/drm/panel/Makefile |   1 +
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 515 +
 3 files changed, 527 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 1500ab9..62aba97 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -18,6 +18,17 @@ config DRM_PANEL_SIMPLE
  that it can be automatically turned off when the panel goes into a
  low power state.
 
+config DRM_PANEL_JDI_LT070ME05000
+   tristate "JDI LT070ME05000 WUXGA DSI panel"
+   depends on OF
+   depends on DRM_MIPI_DSI
+   depends on BACKLIGHT_CLASS_DEVICE
+   help
+ Say Y here if you want to enable support for JDI DSI video mode
+ panel as found in Google Nexus 7 (2013) devices.
+ The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses
+ 24 bit per pixel.
+
 config DRM_PANEL_SAMSUNG_LD9040
tristate "Samsung LD9040 RGB/SPI panel"
depends on OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index f277eed..a5c7ec0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
+obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c 
b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
new file mode 100644
index 000..e935a63
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
@@ -0,0 +1,515 @@
+/*
+ * Copyright (C) 2016 InforceComputing
+ * Author: Vinay Simha BN 
+ *
+ * Copyright (C) 2016 Linaro Ltd
+ * Author: Sumit Semwal 
+ *
+ * From internet archives, the panel for Nexus 7 2nd Gen, 2013 model is a
+ * JDI model LT070ME05000, and its data sheet is at:
+ * http://panelone.net/en/7-0-inch/JDI_LT070ME05000_7.0_inch-datasheet
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define PANEL_NUM_REGULATORS   3
+
+static const char * const regulator_names[] = {
+   "vddp",
+   "dcdc_en",
+   "vcc"
+};
+
+struct jdi_panel {
+   struct drm_panel base;
+   struct mipi_dsi_device *dsi;
+
+   struct regulator_bulk_data

[PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

2016-06-15 Thread Vinay Simha BN 
Add support for the JDI LT070ME05000 WUXGA DSI panel used in
Nexus 7 2013 devices.

Programming sequence for the panel is was originally found in the
android-msm-flo-3.4-lollipop-release branch from:
https://android.googlesource.com/kernel/msm.git

And video mode setting is from dsi-panel-jdi-dualmipi1-video.dtsi
file in:
git://codeaurora.org/kernel/msm-3.10.git  LNX.LA.3.6_rb1.27

Cc: Archit Taneja 
[sumit.semwal: Ported to the drm/panel framework]
Signed-off-by: Sumit Semwal 
[jstultz: Cherry-picked to mainline, folded down other fixes
 from Vinay and Archit]
Signed-off-by: John Stultz 
[vinay simha bn: removed interface setting cmd mode, video
mode panel setting selection]
Cc: Rob Clark 
Signed-off-by: Vinay Simha BN 

---
v2:
 * incorporated code reviews from theiry, archit
   code style, alphabetical soring in Makefile, Kconfig, regulator_bulk,
   arrays of u8, generic helper function, documentation bindings,

v3:
 * dcs backlight support added
 * tested this panel driver in nexus7 2013 device

v4:
 * backlight interface added in the panel driver
 * incorporated width_mm and height_mm suggested by rob herring

v5:
 * theirry review comments incorporated
   panel model naming consistent, alphabetical soring in Kconfig
   Makefile, MAX_BRIGHTNESS dropped, regulator_names, parameterize
   panel width and height, descprition for control display, cabc
   and interface setting, temporary variable removed, consistent
   error reporting and commit message
 * removed tear on/off, scanline, since these are required only
   for command mode panels
---
 drivers/gpu/drm/panel/Kconfig  |  11 +
 drivers/gpu/drm/panel/Makefile |   1 +
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 515 +
 3 files changed, 527 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 1500ab9..62aba97 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -18,6 +18,17 @@ config DRM_PANEL_SIMPLE
  that it can be automatically turned off when the panel goes into a
  low power state.
 
+config DRM_PANEL_JDI_LT070ME05000
+   tristate "JDI LT070ME05000 WUXGA DSI panel"
+   depends on OF
+   depends on DRM_MIPI_DSI
+   depends on BACKLIGHT_CLASS_DEVICE
+   help
+ Say Y here if you want to enable support for JDI DSI video mode
+ panel as found in Google Nexus 7 (2013) devices.
+ The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses
+ 24 bit per pixel.
+
 config DRM_PANEL_SAMSUNG_LD9040
tristate "Samsung LD9040 RGB/SPI panel"
depends on OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index f277eed..a5c7ec0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
+obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c 
b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
new file mode 100644
index 000..e935a63
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
@@ -0,0 +1,515 @@
+/*
+ * Copyright (C) 2016 InforceComputing
+ * Author: Vinay Simha BN 
+ *
+ * Copyright (C) 2016 Linaro Ltd
+ * Author: Sumit Semwal 
+ *
+ * From internet archives, the panel for Nexus 7 2nd Gen, 2013 model is a
+ * JDI model LT070ME05000, and its data sheet is at:
+ * http://panelone.net/en/7-0-inch/JDI_LT070ME05000_7.0_inch-datasheet
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define PANEL_NUM_REGULATORS   3
+
+static const char * const regulator_names[] = {
+   "vddp",
+   "dcdc_en",
+   "vcc"
+};
+
+struct jdi_panel {
+   struct drm_panel base;
+   struct mipi_dsi_device *dsi;
+
+   struct regulator_bulk_data supplies[PANEL_NUM_REGULATORS];
+
+   struct gpio_desc *reset_gpio;
+   struct gpio_desc *enable_gpio;
+   struct backlight_device *backlight;
+
+   bool