Re: [RFC PATCH 2/3] drm/panel: sharp-lq101r1sx01: Use panel-common helpers

[Sean Paul]

On Tue, Mar 21, 2017 at 02:06:26PM -0700, Eric Anholt wrote:
> Sean Paul  writes:
> 
> > Instead of duplicating common code from panel-simple, use the panel-common 
> > helpers.
> >
> > Signed-off-by: Sean Paul 
> > ---
> >  drivers/gpu/drm/panel/Kconfig   |  1 +
> >  drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 79 
> > +++--
> >  2 files changed, 24 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index be8590724042..98db78d22a13 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -73,6 +73,7 @@ config DRM_PANEL_SHARP_LQ101R1SX01
> > depends on OF
> > depends on DRM_MIPI_DSI
> > depends on BACKLIGHT_CLASS_DEVICE
> > +   select DRM_PANEL_COMMON
> > help
> >   Say Y here if you want to enable support for Sharp LQ101R1SX01
> >   TFT-LCD modules. The panel has a 2560x1600 resolution and uses
> > diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c 
> > b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> > index 3cce3ca19601..fb2bf67449ab 100644
> > --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> > +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> > @@ -6,11 +6,8 @@
> >   * published by the Free Software Foundation.
> >   */
> >  
> > -#include 
> > -#include 
> >  #include 
> >  #include 
> > -#include 
> >  
> >  #include 
> >  #include 
> > @@ -19,17 +16,17 @@
> >  
> >  #include 
> >  
> > +#include "panel-common.h"
> > +
> >  struct sharp_panel {
> > struct drm_panel base;
> > +   struct panel_common common;
> > +
> > /* the datasheet refers to them as DSI-LINK1 and DSI-LINK2 */
> > struct mipi_dsi_device *link1;
> > struct mipi_dsi_device *link2;
> >  
> > -   struct backlight_device *backlight;
> > -   struct regulator *supply;
> > -
> > bool prepared;
> > -   bool enabled;
> >  
> > const struct drm_display_mode *mode;
> >  };
> > @@ -93,17 +90,7 @@ static int sharp_panel_disable(struct drm_panel *panel)
> >  {
> > struct sharp_panel *sharp = to_sharp_panel(panel);
> >  
> > -   if (!sharp->enabled)
> > -   return 0;
> > -
> > -   if (sharp->backlight) {
> > -   sharp->backlight->props.power = FB_BLANK_POWERDOWN;
> > -   backlight_update_status(sharp->backlight);
> > -   }
> > -
> > -   sharp->enabled = false;
> > -
> > -   return 0;
> > +   return panel_common_disable(>common, 0);
> >  }
> >  
> >  static int sharp_panel_unprepare(struct drm_panel *panel)
> > @@ -126,11 +113,13 @@ static int sharp_panel_unprepare(struct drm_panel 
> > *panel)
> >  
> > msleep(120);
> >  
> > -   regulator_disable(sharp->supply);
> > +   err = panel_common_unprepare(>common, 0);
> > +   if (err < 0)
> > +   dev_err(panel->dev, "failed common unprepare: %d\n", err);
> >  
> > sharp->prepared = false;
> >  
> > -   return 0;
> > +   return err;
> >  }
> >  
> >  static int sharp_setup_symmetrical_split(struct mipi_dsi_device *left,
> > @@ -176,17 +165,15 @@ static int sharp_panel_prepare(struct drm_panel 
> > *panel)
> > if (sharp->prepared)
> > return 0;
> >  
> > -   err = regulator_enable(sharp->supply);
> > -   if (err < 0)
> > -   return err;
> > -
> > /*
> >  * According to the datasheet, the panel needs around 10 ms to fully
> >  * power up. At least another 120 ms is required before exiting sleep
> >  * mode to make sure the panel is ready. Throw in another 20 ms for
> >  * good measure.
> >  */
> > -   msleep(150);
> > +   err = panel_common_prepare(>common, 150);
> > +   if (err < 0)
> > +   return err;
> 
> Aside: I like how 120 + 20 = 150, because always round up your delays :)

You missed the initial 10ms to "fully power up", so it's 10 + 120 + 20 = 150
That said, you can never have enough fudge in your delays :-)

> 
> >  
> > err = mipi_dsi_dcs_exit_sleep_mode(sharp->link1);
> > if (err < 0) {
> > @@ -252,7 +239,7 @@ static int sharp_panel_prepare(struct drm_panel *panel)
> > return 0;
> >  
> >  poweroff:
> > -   regulator_disable(sharp->supply);
> > +   panel_common_unprepare(>common, 0);
> > return err;
> >  }
> >  
> > @@ -260,17 +247,7 @@ static int sharp_panel_enable(struct drm_panel *panel)
> >  {
> > struct sharp_panel *sharp = to_sharp_panel(panel);
> >  
> > -   if (sharp->enabled)
> > -   return 0;
> > -
> > -   if (sharp->backlight) {
> > -   sharp->backlight->props.power = FB_BLANK_UNBLANK;
> > -   backlight_update_status(sharp->backlight);
> > -   }
> > -
> > -   sharp->enabled = true;
> > -
> > -   return 0;
> > +   return panel_common_enable(>common, 0);
> >  }
> >  
> >  static const struct drm_display_mode default_mode = {
> > @@ -324,23 +301,15 @@ MODULE_DEVICE_TABLE(of, sharp_of_match);
> >  
> >  static int sharp_panel_add(struct sharp_panel *sharp)
> >  {
> > -   struct 

Re: [RFC PATCH 2/3] drm/panel: sharp-lq101r1sx01: Use panel-common helpers

[Eric Anholt]

Sean Paul  writes:

> Instead of duplicating common code from panel-simple, use the panel-common 
> helpers.
>
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/panel/Kconfig   |  1 +
>  drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 79 
> +++--
>  2 files changed, 24 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index be8590724042..98db78d22a13 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -73,6 +73,7 @@ config DRM_PANEL_SHARP_LQ101R1SX01
>   depends on OF
>   depends on DRM_MIPI_DSI
>   depends on BACKLIGHT_CLASS_DEVICE
> + select DRM_PANEL_COMMON
>   help
> Say Y here if you want to enable support for Sharp LQ101R1SX01
> TFT-LCD modules. The panel has a 2560x1600 resolution and uses
> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c 
> b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> index 3cce3ca19601..fb2bf67449ab 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> @@ -6,11 +6,8 @@
>   * published by the Free Software Foundation.
>   */
>  
> -#include 
> -#include 
>  #include 
>  #include 
> -#include 
>  
>  #include 
>  #include 
> @@ -19,17 +16,17 @@
>  
>  #include 
>  
> +#include "panel-common.h"
> +
>  struct sharp_panel {
>   struct drm_panel base;
> + struct panel_common common;
> +
>   /* the datasheet refers to them as DSI-LINK1 and DSI-LINK2 */
>   struct mipi_dsi_device *link1;
>   struct mipi_dsi_device *link2;
>  
> - struct backlight_device *backlight;
> - struct regulator *supply;
> -
>   bool prepared;
> - bool enabled;
>  
>   const struct drm_display_mode *mode;
>  };
> @@ -93,17 +90,7 @@ static int sharp_panel_disable(struct drm_panel *panel)
>  {
>   struct sharp_panel *sharp = to_sharp_panel(panel);
>  
> - if (!sharp->enabled)
> - return 0;
> -
> - if (sharp->backlight) {
> - sharp->backlight->props.power = FB_BLANK_POWERDOWN;
> - backlight_update_status(sharp->backlight);
> - }
> -
> - sharp->enabled = false;
> -
> - return 0;
> + return panel_common_disable(>common, 0);
>  }
>  
>  static int sharp_panel_unprepare(struct drm_panel *panel)
> @@ -126,11 +113,13 @@ static int sharp_panel_unprepare(struct drm_panel 
> *panel)
>  
>   msleep(120);
>  
> - regulator_disable(sharp->supply);
> + err = panel_common_unprepare(>common, 0);
> + if (err < 0)
> + dev_err(panel->dev, "failed common unprepare: %d\n", err);
>  
>   sharp->prepared = false;
>  
> - return 0;
> + return err;
>  }
>  
>  static int sharp_setup_symmetrical_split(struct mipi_dsi_device *left,
> @@ -176,17 +165,15 @@ static int sharp_panel_prepare(struct drm_panel *panel)
>   if (sharp->prepared)
>   return 0;
>  
> - err = regulator_enable(sharp->supply);
> - if (err < 0)
> - return err;
> -
>   /*
>* According to the datasheet, the panel needs around 10 ms to fully
>* power up. At least another 120 ms is required before exiting sleep
>* mode to make sure the panel is ready. Throw in another 20 ms for
>* good measure.
>*/
> - msleep(150);
> + err = panel_common_prepare(>common, 150);
> + if (err < 0)
> + return err;

Aside: I like how 120 + 20 = 150, because always round up your delays :)

>  
>   err = mipi_dsi_dcs_exit_sleep_mode(sharp->link1);
>   if (err < 0) {
> @@ -252,7 +239,7 @@ static int sharp_panel_prepare(struct drm_panel *panel)
>   return 0;
>  
>  poweroff:
> - regulator_disable(sharp->supply);
> + panel_common_unprepare(>common, 0);
>   return err;
>  }
>  
> @@ -260,17 +247,7 @@ static int sharp_panel_enable(struct drm_panel *panel)
>  {
>   struct sharp_panel *sharp = to_sharp_panel(panel);
>  
> - if (sharp->enabled)
> - return 0;
> -
> - if (sharp->backlight) {
> - sharp->backlight->props.power = FB_BLANK_UNBLANK;
> - backlight_update_status(sharp->backlight);
> - }
> -
> - sharp->enabled = true;
> -
> - return 0;
> + return panel_common_enable(>common, 0);
>  }
>  
>  static const struct drm_display_mode default_mode = {
> @@ -324,23 +301,15 @@ MODULE_DEVICE_TABLE(of, sharp_of_match);
>  
>  static int sharp_panel_add(struct sharp_panel *sharp)
>  {
> - struct device_node *np;
>   int err;
>  
>   sharp->mode = _mode;
> + sharp->prepared = false;
>  
> - sharp->supply = devm_regulator_get(>link1->dev, "power");
> - if (IS_ERR(sharp->supply))
> - return PTR_ERR(sharp->supply);
> -
> - np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
> - if (np) {
> - sharp->backlight = 

[RFC PATCH 2/3] drm/panel: sharp-lq101r1sx01: Use panel-common helpers

[Sean Paul]

Instead of duplicating common code from panel-simple, use the panel-common 
helpers.

Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/panel/Kconfig   |  1 +
 drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 79 +++--
 2 files changed, 24 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index be8590724042..98db78d22a13 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -73,6 +73,7 @@ config DRM_PANEL_SHARP_LQ101R1SX01
depends on OF
depends on DRM_MIPI_DSI
depends on BACKLIGHT_CLASS_DEVICE
+   select DRM_PANEL_COMMON
help
  Say Y here if you want to enable support for Sharp LQ101R1SX01
  TFT-LCD modules. The panel has a 2560x1600 resolution and uses
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c 
b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 3cce3ca19601..fb2bf67449ab 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -6,11 +6,8 @@
  * published by the Free Software Foundation.
  */
 
-#include 
-#include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -19,17 +16,17 @@
 
 #include 
 
+#include "panel-common.h"
+
 struct sharp_panel {
struct drm_panel base;
+   struct panel_common common;
+
/* the datasheet refers to them as DSI-LINK1 and DSI-LINK2 */
struct mipi_dsi_device *link1;
struct mipi_dsi_device *link2;
 
-   struct backlight_device *backlight;
-   struct regulator *supply;
-
bool prepared;
-   bool enabled;
 
const struct drm_display_mode *mode;
 };
@@ -93,17 +90,7 @@ static int sharp_panel_disable(struct drm_panel *panel)
 {
struct sharp_panel *sharp = to_sharp_panel(panel);
 
-   if (!sharp->enabled)
-   return 0;
-
-   if (sharp->backlight) {
-   sharp->backlight->props.power = FB_BLANK_POWERDOWN;
-   backlight_update_status(sharp->backlight);
-   }
-
-   sharp->enabled = false;
-
-   return 0;
+   return panel_common_disable(>common, 0);
 }
 
 static int sharp_panel_unprepare(struct drm_panel *panel)
@@ -126,11 +113,13 @@ static int sharp_panel_unprepare(struct drm_panel *panel)
 
msleep(120);
 
-   regulator_disable(sharp->supply);
+   err = panel_common_unprepare(>common, 0);
+   if (err < 0)
+   dev_err(panel->dev, "failed common unprepare: %d\n", err);
 
sharp->prepared = false;
 
-   return 0;
+   return err;
 }
 
 static int sharp_setup_symmetrical_split(struct mipi_dsi_device *left,
@@ -176,17 +165,15 @@ static int sharp_panel_prepare(struct drm_panel *panel)
if (sharp->prepared)
return 0;
 
-   err = regulator_enable(sharp->supply);
-   if (err < 0)
-   return err;
-
/*
 * According to the datasheet, the panel needs around 10 ms to fully
 * power up. At least another 120 ms is required before exiting sleep
 * mode to make sure the panel is ready. Throw in another 20 ms for
 * good measure.
 */
-   msleep(150);
+   err = panel_common_prepare(>common, 150);
+   if (err < 0)
+   return err;
 
err = mipi_dsi_dcs_exit_sleep_mode(sharp->link1);
if (err < 0) {
@@ -252,7 +239,7 @@ static int sharp_panel_prepare(struct drm_panel *panel)
return 0;
 
 poweroff:
-   regulator_disable(sharp->supply);
+   panel_common_unprepare(>common, 0);
return err;
 }
 
@@ -260,17 +247,7 @@ static int sharp_panel_enable(struct drm_panel *panel)
 {
struct sharp_panel *sharp = to_sharp_panel(panel);
 
-   if (sharp->enabled)
-   return 0;
-
-   if (sharp->backlight) {
-   sharp->backlight->props.power = FB_BLANK_UNBLANK;
-   backlight_update_status(sharp->backlight);
-   }
-
-   sharp->enabled = true;
-
-   return 0;
+   return panel_common_enable(>common, 0);
 }
 
 static const struct drm_display_mode default_mode = {
@@ -324,23 +301,15 @@ MODULE_DEVICE_TABLE(of, sharp_of_match);
 
 static int sharp_panel_add(struct sharp_panel *sharp)
 {
-   struct device_node *np;
int err;
 
sharp->mode = _mode;
+   sharp->prepared = false;
 
-   sharp->supply = devm_regulator_get(>link1->dev, "power");
-   if (IS_ERR(sharp->supply))
-   return PTR_ERR(sharp->supply);
-
-   np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
-   if (np) {
-   sharp->backlight = of_find_backlight_by_node(np);
-   of_node_put(np);
-
-   if (!sharp->backlight)
-   return -EPROBE_DEFER;
-   }
+   err = panel_common_init(>link1->dev, >common, "supply",
+   "gpio", "backlight");
+   if (err < 0)
+