[PATCH 0/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[PATCH 0/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[Andreas Kemnade]

This panel has a backlight, so add a property describing that and
add the code to use that.
This makes things like xset dpms force off
also turn off the backlight, so we do not need to rely on additional
userspace programs to do that.

Andreas Kemnade (2):
  drm/omap: panel-tpo-td028ttec1: add backlight support
  dt-bindings: panel: td028ttec1: add backlight property

 .../bindings/display/panel/tpo,td028ttec1.txt          |  2 ++
 .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c    | 18 +++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

-- 
2.11.0

[PATCH 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[Andreas Kemnade]

This panel has a backlight, so fetch it from devicetree using the
as documented in panel-common.txt. It is implemented the same way as in
panel-dpi.c
This ensures the backlight is also disabled when the display is
turned off like when doing xset dpms force off.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c    | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
index 7ddc8c574a61..f326ba9dcf62 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
@@ -35,6 +35,8 @@ struct panel_drv_data {
 
 	struct videomode vm;
 
+	struct backlight_device *backlight;
+
 	struct spi_device *spi_dev;
 };
 
@@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
 
 	r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON);
 
+	backlight_enable(ddata->backlight);
+
 	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
 
 transfer_err:
@@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
 	if (!omapdss_device_is_enabled(dssdev))
 		return;
 
+	backlight_disable(ddata->backlight);
+
 	dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n");
 
 	jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF);
@@ -321,6 +327,15 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
 
 	dev_dbg(&spi->dev, "%s\n", __func__);
 
+	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
+	if (ddata == NULL)
+		return -ENOMEM;
+
+	ddata->backlight = devm_of_find_backlight(&spi->dev);
+
+	if (IS_ERR(ddata->backlight))
+		return PTR_ERR(ddata->backlight);
+
 	spi->bits_per_word = 9;
 	spi->mode = SPI_MODE_3;
 
@@ -330,9 +345,6 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
 		return r;
 	}
 
-	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
-	if (ddata == NULL)
-		return -ENOMEM;
 
 	dev_set_drvdata(&spi->dev, ddata);
 
-- 
2.11.0

Re: [PATCH 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[Tomi Valkeinen]

Hi,

On 19/01/2019 20:21, Andreas Kemnade wrote:
> This panel has a backlight, so fetch it from devicetree using the
> as documented in panel-common.txt. It is implemented the same way as in

Extra words above, or maybe some are missing...

> panel-dpi.c
> This ensures the backlight is also disabled when the display is
> turned off like when doing xset dpms force off.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c    | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> index 7ddc8c574a61..f326ba9dcf62 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> @@ -35,6 +35,8 @@ struct panel_drv_data {
>  
>  	struct videomode vm;
>  
> +	struct backlight_device *backlight;
> +
>  	struct spi_device *spi_dev;
>  };
>  
> @@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
>  
>  	r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON);
>  
> +	backlight_enable(ddata->backlight);
> +
>  	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
>  
>  transfer_err:
> @@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
>  	if (!omapdss_device_is_enabled(dssdev))
>  		return;
>  
> +	backlight_disable(ddata->backlight);
> +
>  	dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n");
>  
>  	jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF);
> @@ -321,6 +327,15 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
>  
>  	dev_dbg(&spi->dev, "%s\n", __func__);
>  
> +	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
> +	if (ddata == NULL)
> +		return -ENOMEM;
> +
> +	ddata->backlight = devm_of_find_backlight(&spi->dev);
> +
> +	if (IS_ERR(ddata->backlight))
> +		return PTR_ERR(ddata->backlight);
> +

Is there a reason for moving the ddata alloc here, instead of keeping it
where it was?

>  	spi->bits_per_word = 9;
>  	spi->mode = SPI_MODE_3;
>  
> @@ -330,9 +345,6 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
>  		return r;
>  	}
>  
> -	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
> -	if (ddata == NULL)
> -		return -ENOMEM;
>  

This leaves an extra empty line here.

>  	dev_set_drvdata(&spi->dev, ddata);
>  

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[Tomi Valkeinen]

Hi,

On 19/01/2019 20:21, Andreas Kemnade wrote:
> This panel has a backlight, so fetch it from devicetree using the
> as documented in panel-common.txt. It is implemented the same way as in

Extra words above, or maybe some are missing...

> panel-dpi.c
> This ensures the backlight is also disabled when the display is
> turned off like when doing xset dpms force off.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c    | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> index 7ddc8c574a61..f326ba9dcf62 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> @@ -35,6 +35,8 @@ struct panel_drv_data {
>  
>  	struct videomode vm;
>  
> +	struct backlight_device *backlight;
> +
>  	struct spi_device *spi_dev;
>  };
>  
> @@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
>  
>  	r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON);
>  
> +	backlight_enable(ddata->backlight);
> +
>  	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
>  
>  transfer_err:
> @@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
>  	if (!omapdss_device_is_enabled(dssdev))
>  		return;
>  
> +	backlight_disable(ddata->backlight);
> +
>  	dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n");
>  
>  	jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF);
> @@ -321,6 +327,15 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
>  
>  	dev_dbg(&spi->dev, "%s\n", __func__);
>  
> +	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
> +	if (ddata == NULL)
> +		return -ENOMEM;
> +
> +	ddata->backlight = devm_of_find_backlight(&spi->dev);
> +
> +	if (IS_ERR(ddata->backlight))
> +		return PTR_ERR(ddata->backlight);
> +

Is there a reason for moving the ddata alloc here, instead of keeping it
where it was?

>  	spi->bits_per_word = 9;
>  	spi->mode = SPI_MODE_3;
>  
> @@ -330,9 +345,6 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
>  		return r;
>  	}
>  
> -	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
> -	if (ddata == NULL)
> -		return -ENOMEM;
>  

This leaves an extra empty line here.

>  	dev_set_drvdata(&spi->dev, ddata);
>  

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

Hi,

On Mon, 4 Feb 2019 10:13:46 +0200
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> Hi,
> 
> On 19/01/2019 20:21, Andreas Kemnade wrote:
> > This panel has a backlight, so fetch it from devicetree using the
> > as documented in panel-common.txt. It is implemented the same way as in  
> 
> Extra words above, or maybe some are missing...
> 
oops, 
This panel has a backlight, so fetch it from devicetree using the properties
as documented in panel-common.txt. It is implemented the same way as in 
panel-dpi.c


> > panel-dpi.c
> > This ensures the backlight is also disabled when the display is
> > turned off like when doing xset dpms force off.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >  .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c    | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > index 7ddc8c574a61..f326ba9dcf62 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > @@ -35,6 +35,8 @@ struct panel_drv_data {
> >  
> >  	struct videomode vm;
> >  
> > +	struct backlight_device *backlight;
> > +
> >  	struct spi_device *spi_dev;
> >  };
> >  
> > @@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
> >  
> >  	r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON);
> >  
> > +	backlight_enable(ddata->backlight);
> > +
> >  	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> >  
> >  transfer_err:
> > @@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
> >  	if (!omapdss_device_is_enabled(dssdev))
> >  		return;
> >  
> > +	backlight_disable(ddata->backlight);
> > +
> >  	dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n");
> >  
> >  	jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF);
> > @@ -321,6 +327,15 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
> >  
> >  	dev_dbg(&spi->dev, "%s\n", __func__);
> >  
> > +	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
> > +	if (ddata == NULL)
> > +		return -ENOMEM;
> > +
> > +	ddata->backlight = devm_of_find_backlight(&spi->dev);
> > +
> > +	if (IS_ERR(ddata->backlight))
> > +		return PTR_ERR(ddata->backlight);
> > +  
> 
> Is there a reason for moving the ddata alloc here, instead of keeping it
> where it was?
> 
Well, I was just unsure if the spi_setup needs to be undone on error, so I
moved things around. But the kzalloc() error check would face the same problem
and other error checks further on, too.

So I can rather keep it as is.

I will send a v2.

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

Hi,

On Mon, 4 Feb 2019 10:13:46 +0200
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> Hi,
> 
> On 19/01/2019 20:21, Andreas Kemnade wrote:
> > This panel has a backlight, so fetch it from devicetree using the
> > as documented in panel-common.txt. It is implemented the same way as in  
> 
> Extra words above, or maybe some are missing...
> 
oops, 
This panel has a backlight, so fetch it from devicetree using the properties
as documented in panel-common.txt. It is implemented the same way as in 
panel-dpi.c


> > panel-dpi.c
> > This ensures the backlight is also disabled when the display is
> > turned off like when doing xset dpms force off.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >  .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c    | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > index 7ddc8c574a61..f326ba9dcf62 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > @@ -35,6 +35,8 @@ struct panel_drv_data {
> >  
> >  	struct videomode vm;
> >  
> > +	struct backlight_device *backlight;
> > +
> >  	struct spi_device *spi_dev;
> >  };
> >  
> > @@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
> >  
> >  	r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON);
> >  
> > +	backlight_enable(ddata->backlight);
> > +
> >  	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> >  
> >  transfer_err:
> > @@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
> >  	if (!omapdss_device_is_enabled(dssdev))
> >  		return;
> >  
> > +	backlight_disable(ddata->backlight);
> > +
> >  	dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n");
> >  
> >  	jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF);
> > @@ -321,6 +327,15 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
> >  
> >  	dev_dbg(&spi->dev, "%s\n", __func__);
> >  
> > +	ddata = devm_kzalloc(&spi->dev, sizeof(*ddata), GFP_KERNEL);
> > +	if (ddata == NULL)
> > +		return -ENOMEM;
> > +
> > +	ddata->backlight = devm_of_find_backlight(&spi->dev);
> > +
> > +	if (IS_ERR(ddata->backlight))
> > +		return PTR_ERR(ddata->backlight);
> > +  
> 
> Is there a reason for moving the ddata alloc here, instead of keeping it
> where it was?
> 
Well, I was just unsure if the spi_setup needs to be undone on error, so I
moved things around. But the kzalloc() error check would face the same problem
and other error checks further on, too.

So I can rather keep it as is.

I will send a v2.

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/2] dt-bindings: panel: td028ttec1: add backlight property

[Andreas Kemnade]

This add an additional backlight property as described
in panel-common.txt

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
index ed34253d9fb1..898e06ecf4ef 100644
--- a/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
+++ b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
@@ -6,6 +6,7 @@ Required properties:
 
 Optional properties:
 - label: a symbolic name for the panel
+- backlight: phandle of the backlight device
 
 Required nodes:
 - Video port for DPI input
@@ -21,6 +22,7 @@ lcd-panel: td028ttec1@0 {
 	spi-cpha;
 
 	label = "lcd";
+	backlight = <&backlight>;
 	port {
 		lcd_in: endpoint {
 			remote-endpoint = <&dpi_out>;
-- 
2.11.0

Re: [PATCH 0/2] drm/omap: panel-tpo-td028ttec1: add backlight support

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

ping

On Sat, 19 Jan 2019 19:21:29 +0100
Andreas Kemnade <andreas@kemnade.info> wrote:

> This panel has a backlight, so add a property describing that and
> add the code to use that.
> This makes things like xset dpms force off
> also turn off the backlight, so we do not need to rely on additional
> userspace programs to do that.
> 
> Andreas Kemnade (2):
>   drm/omap: panel-tpo-td028ttec1: add backlight support
>   dt-bindings: panel: td028ttec1: add backlight property
> 
>  .../bindings/display/panel/tpo,td028ttec1.txt          |  2 ++
>  .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c    | 18 +++++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> -- 
> 2.11.0
> 
> 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]