[RFC PATCH v1 0/22] backlight: add init macros and accessors

[RFC PATCH v1 0/22] backlight: add init macros and accessors

[Sam Ravnborg]

The backlight drivers uses several different patterns when registering
a backlight:

- Register backlight and assign properties later
- Define a local backlight_properties variable and use memset
- Define a const backlight_properties and assign relevant properties

On top of this there was differences in what members was assigned in
backlight_properties.

To align how backlight drivers are initialized introduce following helper macros:
- DECLARE_BACKLIGHT_INIT_FIRMWARE()
- DECLARE_BACKLIGHT_INIT_PLATFORM()
- DECLARE_BACKLIGHT_INIT_RAW()

The macros are introduced in patch 2.

The backlight drivers used direct access to backlight_properties.
Encapsulate these in get/set access operations resulting in following benefits:
- The drivers no longer need to be concerned about the confusing power states,
  as there is now only a set_power_on() and set_power_off() operation.
- The access methods can be called with a NULL pointer so logic around the
  access can be made simpler.
- The code is in most cases more readable with the access operations.
- When everyone uses the access methods refactroring in the backlight core is simpler.

The get/set operations are introduced in patch 3.

The first patch trims backlight_update_status() so it can be called with a NULL
backlight_device. Then the called do not need to add this check just to avoid
a NULL reference.

The fourth patch introduce the new macros and get/set operations for the
gpio backlight driver, as an example.

The remaining patches updates most backlight users in drivers/gpu/drm/*
With this patch set applied then:
- Almost all references to FB_BLANK* are gone from drm/*
- All panel drivers uses devm_ variant for registering backlight
- Almost all direct references to backlight properties are gone

The drm/* patches are  used as examples how drivers can benefit from the
new macros and get/set operations.

Individual patches are only sent to the people listed in the patch + a few more.
Please check https://lore.kernel.org/dri-devel/ for the full series.

Feedback welcome!

	Sam

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: Konrad Dybcio <konradybcio@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-renesas-soc@vger.kernel.org
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
Cc: Philippe CORNU <philippe.cornu@st.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Robert Chiras <robert.chiras@nxp.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Vinay Simha BN <simhavcs@gmail.com>
Cc: Wambui Karuga <wambui.karugax@gmail.com>
Cc: Zheng Bin <zhengbin13@huawei.com>

Sam Ravnborg (22):
      backlight: Silently fail backlight_update_status() if no device
      backlight: Add DECLARE_* macro for device registration
      backlight: Add get/set operations for brightness/power properties
      backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters
      drm/gma500: Backlight support
      drm/panel: asus-z00t-tm5p5-n35596: Backlight update
      drm/panel: jdi-lt070me05000: Backlight update
      drm/panel: novatek-nt35510: Backlight update
      drm/panel: orisetech-otm8009a: Backlight update
      drm/panel: raydium-rm67191: Backlight update
      drm/panel: samsung-s6e63m0: Backlight update
      drm/panel: samsung-s6e63j0x03: Backlight update
      drm/panel: samsung-s6e3ha2: Backlight update
      drm/panel: sony-acx424akp: Backlight update
      drm/panel: sony-acx565akm: Backlight update
      drm/bridge: parade-ps8622: Backlight update
      drm/tilcdc: Backlight update
      drm/radeon: Backlight update
      drm/amdgpu/atom: Backlight update
      drm/i915: Backlight update
      drm/omap: display: Backlight update
      drm/shmobile: Backlight update

 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c     |  15 ++-
 drivers/gpu/drm/bridge/parade-ps8622.c             |  43 ++++----
 drivers/gpu/drm/gma500/backlight.c                 |  35 ++----
 drivers/gpu/drm/gma500/cdv_device.c                |  29 +++--
 drivers/gpu/drm/gma500/mdfld_device.c              |   9 +-
 drivers/gpu/drm/gma500/oaktrail_device.c           |  10 +-
 drivers/gpu/drm/gma500/opregion.c                  |   2 +-
 drivers/gpu/drm/gma500/psb_device.c                |  10 +-
 drivers/gpu/drm/gma500/psb_drv.c                   |   8 +-
 drivers/gpu/drm/i915/display/intel_panel.c         |  88 +++++++--------
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  35 ++----
 .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c     |  17 ++-
 drivers/gpu/drm/panel/panel-novatek-nt35510.c      |   9 +-
 drivers/gpu/drm/panel/panel-orisetech-otm8009a.c   |  14 +--
 drivers/gpu/drm/panel/panel-raydium-rm67191.c      |  11 +-
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      |  68 ++++++------
 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   |  56 +++++-----
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c      |  25 ++---
 drivers/gpu/drm/panel/panel-sony-acx424akp.c       |  49 ++-------
 drivers/gpu/drm/panel/panel-sony-acx565akm.c       |  44 +++-----
 drivers/gpu/drm/radeon/atombios_encoders.c         |  23 ++--
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c    |  15 ++-
 drivers/gpu/drm/shmobile/shmob_drm_backlight.c     |  20 ++--
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  11 +-
 drivers/video/backlight/gpio_backlight.c           |  17 ++-
 include/linux/backlight.h                          | 120 +++++++++++++++++++++
 27 files changed, 377 insertions(+), 421 deletions(-)

[RFC PATCH v1 0/22] backlight: add init macros and accessors

[Sam Ravnborg]

The backlight drivers uses several different patterns when registering
a backlight:

- Register backlight and assign properties later
- Define a local backlight_properties variable and use memset
- Define a const backlight_properties and assign relevant properties

On top of this there was differences in what members was assigned in
backlight_properties.

To align how backlight drivers are initialized introduce following helper macros:
- DECLARE_BACKLIGHT_INIT_FIRMWARE()
- DECLARE_BACKLIGHT_INIT_PLATFORM()
- DECLARE_BACKLIGHT_INIT_RAW()

The macros are introduced in patch 2.

The backlight drivers used direct access to backlight_properties.
Encapsulate these in get/set access operations resulting in following benefits:
- The drivers no longer need to be concerned about the confusing power states,
  as there is now only a set_power_on() and set_power_off() operation.
- The access methods can be called with a NULL pointer so logic around the
  access can be made simpler.
- The code is in most cases more readable with the access operations.
- When everyone uses the access methods refactroring in the backlight core is simpler.

The get/set operations are introduced in patch 3.

The first patch trims backlight_update_status() so it can be called with a NULL
backlight_device. Then the called do not need to add this check just to avoid
a NULL reference.

The fourth patch introduce the new macros and get/set operations for the
gpio backlight driver, as an example.

The remaining patches updates most backlight users in drivers/gpu/drm/*
With this patch set applied then:
- Almost all references to FB_BLANK* are gone from drm/*
- All panel drivers uses devm_ variant for registering backlight
- Almost all direct references to backlight properties are gone

The drm/* patches are  used as examples how drivers can benefit from the
new macros and get/set operations.

Individual patches are only sent to the people listed in the patch + a few more.
Please check https://lore.kernel.org/dri-devel/ for the full series.

Feedback welcome!

	Sam

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: Konrad Dybcio <konradybcio@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-renesas-soc@vger.kernel.org
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
Cc: Philippe CORNU <philippe.cornu@st.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Robert Chiras <robert.chiras@nxp.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Vinay Simha BN <simhavcs@gmail.com>
Cc: Wambui Karuga <wambui.karugax@gmail.com>
Cc: Zheng Bin <zhengbin13@huawei.com>

Sam Ravnborg (22):
      backlight: Silently fail backlight_update_status() if no device
      backlight: Add DECLARE_* macro for device registration
      backlight: Add get/set operations for brightness/power properties
      backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters
      drm/gma500: Backlight support
      drm/panel: asus-z00t-tm5p5-n35596: Backlight update
      drm/panel: jdi-lt070me05000: Backlight update
      drm/panel: novatek-nt35510: Backlight update
      drm/panel: orisetech-otm8009a: Backlight update
      drm/panel: raydium-rm67191: Backlight update
      drm/panel: samsung-s6e63m0: Backlight update
      drm/panel: samsung-s6e63j0x03: Backlight update
      drm/panel: samsung-s6e3ha2: Backlight update
      drm/panel: sony-acx424akp: Backlight update
      drm/panel: sony-acx565akm: Backlight update
      drm/bridge: parade-ps8622: Backlight update
      drm/tilcdc: Backlight update
      drm/radeon: Backlight update
      drm/amdgpu/atom: Backlight update
      drm/i915: Backlight update
      drm/omap: display: Backlight update
      drm/shmobile: Backlight update

 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c     |  15 ++-
 drivers/gpu/drm/bridge/parade-ps8622.c             |  43 ++++----
 drivers/gpu/drm/gma500/backlight.c                 |  35 ++----
 drivers/gpu/drm/gma500/cdv_device.c                |  29 +++--
 drivers/gpu/drm/gma500/mdfld_device.c              |   9 +-
 drivers/gpu/drm/gma500/oaktrail_device.c           |  10 +-
 drivers/gpu/drm/gma500/opregion.c                  |   2 +-
 drivers/gpu/drm/gma500/psb_device.c                |  10 +-
 drivers/gpu/drm/gma500/psb_drv.c                   |   8 +-
 drivers/gpu/drm/i915/display/intel_panel.c         |  88 +++++++--------
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  35 ++----
 .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c     |  17 ++-
 drivers/gpu/drm/panel/panel-novatek-nt35510.c      |   9 +-
 drivers/gpu/drm/panel/panel-orisetech-otm8009a.c   |  14 +--
 drivers/gpu/drm/panel/panel-raydium-rm67191.c      |  11 +-
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      |  68 ++++++------
 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   |  56 +++++-----
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c      |  25 ++---
 drivers/gpu/drm/panel/panel-sony-acx424akp.c       |  49 ++-------
 drivers/gpu/drm/panel/panel-sony-acx565akm.c       |  44 +++-----
 drivers/gpu/drm/radeon/atombios_encoders.c         |  23 ++--
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c    |  15 ++-
 drivers/gpu/drm/shmobile/shmob_drm_backlight.c     |  20 ++--
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  11 +-
 drivers/video/backlight/gpio_backlight.c           |  17 ++-
 include/linux/backlight.h                          | 120 +++++++++++++++++++++
 27 files changed, 377 insertions(+), 421 deletions(-)


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[RFC PATCH v1 0/22] backlight: add init macros and accessors

[Sam Ravnborg]

The backlight drivers uses several different patterns when registering
a backlight:

- Register backlight and assign properties later
- Define a local backlight_properties variable and use memset
- Define a const backlight_properties and assign relevant properties

On top of this there was differences in what members was assigned in
backlight_properties.

To align how backlight drivers are initialized introduce following helper macros:
- DECLARE_BACKLIGHT_INIT_FIRMWARE()
- DECLARE_BACKLIGHT_INIT_PLATFORM()
- DECLARE_BACKLIGHT_INIT_RAW()

The macros are introduced in patch 2.

The backlight drivers used direct access to backlight_properties.
Encapsulate these in get/set access operations resulting in following benefits:
- The drivers no longer need to be concerned about the confusing power states,
  as there is now only a set_power_on() and set_power_off() operation.
- The access methods can be called with a NULL pointer so logic around the
  access can be made simpler.
- The code is in most cases more readable with the access operations.
- When everyone uses the access methods refactroring in the backlight core is simpler.

The get/set operations are introduced in patch 3.

The first patch trims backlight_update_status() so it can be called with a NULL
backlight_device. Then the called do not need to add this check just to avoid
a NULL reference.

The fourth patch introduce the new macros and get/set operations for the
gpio backlight driver, as an example.

The remaining patches updates most backlight users in drivers/gpu/drm/*
With this patch set applied then:
- Almost all references to FB_BLANK* are gone from drm/*
- All panel drivers uses devm_ variant for registering backlight
- Almost all direct references to backlight properties are gone

The drm/* patches are  used as examples how drivers can benefit from the
new macros and get/set operations.

Individual patches are only sent to the people listed in the patch + a few more.
Please check https://lore.kernel.org/dri-devel/ for the full series.

Feedback welcome!

	Sam

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: Konrad Dybcio <konradybcio@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-renesas-soc@vger.kernel.org
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
Cc: Philippe CORNU <philippe.cornu@st.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Robert Chiras <robert.chiras@nxp.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Vinay Simha BN <simhavcs@gmail.com>
Cc: Wambui Karuga <wambui.karugax@gmail.com>
Cc: Zheng Bin <zhengbin13@huawei.com>

Sam Ravnborg (22):
      backlight: Silently fail backlight_update_status() if no device
      backlight: Add DECLARE_* macro for device registration
      backlight: Add get/set operations for brightness/power properties
      backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters
      drm/gma500: Backlight support
      drm/panel: asus-z00t-tm5p5-n35596: Backlight update
      drm/panel: jdi-lt070me05000: Backlight update
      drm/panel: novatek-nt35510: Backlight update
      drm/panel: orisetech-otm8009a: Backlight update
      drm/panel: raydium-rm67191: Backlight update
      drm/panel: samsung-s6e63m0: Backlight update
      drm/panel: samsung-s6e63j0x03: Backlight update
      drm/panel: samsung-s6e3ha2: Backlight update
      drm/panel: sony-acx424akp: Backlight update
      drm/panel: sony-acx565akm: Backlight update
      drm/bridge: parade-ps8622: Backlight update
      drm/tilcdc: Backlight update
      drm/radeon: Backlight update
      drm/amdgpu/atom: Backlight update
      drm/i915: Backlight update
      drm/omap: display: Backlight update
      drm/shmobile: Backlight update

 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c     |  15 ++-
 drivers/gpu/drm/bridge/parade-ps8622.c             |  43 ++++----
 drivers/gpu/drm/gma500/backlight.c                 |  35 ++----
 drivers/gpu/drm/gma500/cdv_device.c                |  29 +++--
 drivers/gpu/drm/gma500/mdfld_device.c              |   9 +-
 drivers/gpu/drm/gma500/oaktrail_device.c           |  10 +-
 drivers/gpu/drm/gma500/opregion.c                  |   2 +-
 drivers/gpu/drm/gma500/psb_device.c                |  10 +-
 drivers/gpu/drm/gma500/psb_drv.c                   |   8 +-
 drivers/gpu/drm/i915/display/intel_panel.c         |  88 +++++++--------
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  35 ++----
 .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c     |  17 ++-
 drivers/gpu/drm/panel/panel-novatek-nt35510.c      |   9 +-
 drivers/gpu/drm/panel/panel-orisetech-otm8009a.c   |  14 +--
 drivers/gpu/drm/panel/panel-raydium-rm67191.c      |  11 +-
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      |  68 ++++++------
 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   |  56 +++++-----
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c      |  25 ++---
 drivers/gpu/drm/panel/panel-sony-acx424akp.c       |  49 ++-------
 drivers/gpu/drm/panel/panel-sony-acx565akm.c       |  44 +++-----
 drivers/gpu/drm/radeon/atombios_encoders.c         |  23 ++--
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c    |  15 ++-
 drivers/gpu/drm/shmobile/shmob_drm_backlight.c     |  20 ++--
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  11 +-
 drivers/video/backlight/gpio_backlight.c           |  17 ++-
 include/linux/backlight.h                          | 120 +++++++++++++++++++++
 27 files changed, 377 insertions(+), 421 deletions(-)


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

[PATCH v1 01/22] backlight: Silently fail backlight_update_status() if no device

[Sam Ravnborg]

backlight_update_status() may be called from code that does not have
any valid backlight device. To avoid ifdeffery and too much conditionals
silently fail if the backlight_device is NULL.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 include/linux/backlight.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 614653e07e3a..190963ffb7fc 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -348,6 +348,9 @@ static inline int backlight_update_status(struct backlight_device *bd)
 {
 	int ret = -ENOENT;
 
+	if (!bd)
+		return 0;
+
 	mutex_lock(&bd->update_lock);
 	if (bd->ops && bd->ops->update_status)
 		ret = bd->ops->update_status(bd);
-- 
2.25.1

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

[PATCH v1 02/22] backlight: Add DECLARE_* macro for device registration

[Sam Ravnborg]

Device registration almost always uses a struct backlight_properties
variable to pass config info. Make it simpler and less error prone
by the introduction of a number of macros.

There is one macro for each type of backlight {firmware, platform, raw}.
All members in struct backlight_properties are initialized.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 include/linux/backlight.h | 45 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 190963ffb7fc..d683c053ec09 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -272,6 +272,51 @@ struct backlight_properties {
 	enum backlight_scale scale;
 };
 
+#define BACKLIGHT_PROPS(_brightness, _max_brightness, _type)	\
+	.brightness = _brightness,				\
+	.max_brightness = _max_brightness,			\
+	.power = FB_BLANK_POWERDOWN,				\
+	.type = _type,						\
+	.fb_blank = 0,						\
+	.state = 0,						\
+	.scale = BACKLIGHT_SCALE_UNKNOWN,
+
+/**
+ * DECLARE_BACKLIGHT_INIT_RAW - backlight_properties to init a raw
+ *                              backlight device
+ *
+ * This macro is used to initialize backlight_properties that is used when
+ * registering a raw backlight device.
+ */
+#define DECLARE_BACKLIGHT_INIT_RAW(name, _brightness, _max_brightness)		\
+	const struct backlight_properties name = {				\
+		BACKLIGHT_PROPS(_brightness, _max_brightness, BACKLIGHT_RAW)	\
+	}
+
+/**
+ * DECLARE_BACKLIGHT_INIT_PLATFORM - backlight_properties to init a platform
+ *                                   backlight device
+ *
+ * This macro is used to initialize backlight_properties that is used when
+ * registering a platform backlight device.
+ */
+#define DECLARE_BACKLIGHT_INIT_PLATFORM(name, _brightness, _max_brightness)		\
+	const struct backlight_properties name = {					\
+		BACKLIGHT_PROPS(_brightness, _max_brightness, BACKLIGHT_PLATFORM)	\
+	}
+
+/**
+ * DECLARE_BACKLIGHT_INIT_FIRMWARE - backlight_properties to init a firmware
+ *                                   backlight device
+ *
+ * This macro is used to initialize backlight_properties that is used when
+ * registering a firmware backlight device.
+ */
+#define DECLARE_BACKLIGHT_INIT_FIRMWARE(name, _brightness, _max_brightness)		\
+	const struct backlight_properties name = {					\
+		BACKLIGHT_PROPS(_brightness, _max_brightness, BACKLIGHT_FIRMWARE)	\
+	}
+
 /**
  * struct backlight_device - backlight device data
  *
-- 
2.25.1

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

[PATCH v1 03/22] backlight: Add get/set operations for brightness/power properties

[Sam Ravnborg]

Add get and set operations to incapsualte access to backlight properties.

One easy win is that the get/set operatiosn can be used when backlight
is not included in the configuration, resulting in simpler code with
less ifdef's and thus more readable code.

The set/get methods hides some of the confusing power states, limiting
the power state to either ON or OFF for the drivers.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 include/linux/backlight.h | 72 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index d683c053ec09..882ceea45ace 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -474,6 +474,78 @@ static inline int backlight_get_brightness(const struct backlight_device *bd)
 		return bd->props.brightness;
 }
 
+/**
+ * backlight_get_actual_brightness - Returns the actual brightness
+ *
+ * On failure a negative error code is returned.
+ */
+static inline int backlight_get_actual_brightness(struct backlight_device *bd)
+{
+	if (bd && bd->ops && bd->ops->get_brightness)
+		return bd->ops->get_brightness(bd);
+	else
+		return -EINVAL;
+}
+
+/**
+ * backlight_get_max_brightness - Returns maximum brightness
+ *
+ * This helper operation is preferred over direct access to
+ * &backlight_properties.max_brightness
+ *
+ * Returns 0 if backlight_device is NULL
+ */
+
+static inline int backlight_get_max_brightness(const struct backlight_device *bd)
+{
+	if (bd)
+		return bd->props.max_brightness;
+	else
+		return 0;
+}
+
+/**
+ * backlight_set_brightness - Set the brightness to the specified value
+ *
+ * This helper operation is preferred over direct assignment to
+ * &backlight_properties.brightness.
+ *
+ * If backlight_device is NULL then silently exit.
+ */
+static inline void backlight_set_brightness(struct backlight_device *bd, int brightness)
+{
+	if (bd)
+		bd->props.brightness = brightness;
+}
+
+/**
+ * backlight_set_power_on - Set power state to ON.
+ *
+ * This helper operation is preferred over direct assignment to
+ * backlight_properties.power.
+ *
+ * If backlight_device is NULL then silently exit.
+ */
+static inline void backlight_set_power_on(struct backlight_device *bd)
+{
+	if (bd)
+		bd->props.power = FB_BLANK_UNBLANK;
+}
+
+/**
+ * backlight_set_power_off - Set power state to OFF.
+ *
+ * This helper operation is preferred over direct assignment to
+ * backlight_properties.power.
+ *
+ * If backlight_device is NULL then silently exit.
+ */
+static inline void backlight_set_power_off(struct backlight_device *bd)
+{
+	if (bd)
+		bd->props.power = FB_BLANK_POWERDOWN;
+}
+
 struct backlight_device *
 backlight_device_register(const char *name, struct device *dev, void *devdata,
 			  const struct backlight_ops *ops,
-- 
2.25.1

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

Re: [PATCH v1 03/22] backlight: Add get/set operations for brightness/power properties

[daniel]

On Sun, Aug 02, 2020 at 01:06:17PM +0200, Sam Ravnborg wrote:
> Add get and set operations to incapsualte access to backlight properties.
> 
> One easy win is that the get/set operatiosn can be used when backlight
> is not included in the configuration, resulting in simpler code with
> less ifdef's and thus more readable code.
> 
> The set/get methods hides some of the confusing power states, limiting
> the power state to either ON or OFF for the drivers.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> ---
>  include/linux/backlight.h | 72 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index d683c053ec09..882ceea45ace 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -474,6 +474,78 @@ static inline int backlight_get_brightness(const struct backlight_device *bd)
>  		return bd->props.brightness;
>  }
>  
> +/**
> + * backlight_get_actual_brightness - Returns the actual brightness
> + *
> + * On failure a negative error code is returned.
> + */
> +static inline int backlight_get_actual_brightness(struct backlight_device *bd)
> +{
> +	if (bd && bd->ops && bd->ops->get_brightness)
> +		return bd->ops->get_brightness(bd);
> +	else
> +		return -EINVAL;
> +}
> +
> +/**
> + * backlight_get_max_brightness - Returns maximum brightness
> + *
> + * This helper operation is preferred over direct access to
> + * &backlight_properties.max_brightness
> + *
> + * Returns 0 if backlight_device is NULL
> + */
> +
> +static inline int backlight_get_max_brightness(const struct backlight_device *bd)
> +{
> +	if (bd)
> +		return bd->props.max_brightness;
> +	else
> +		return 0;
> +}
> +
> +/**
> + * backlight_set_brightness - Set the brightness to the specified value
> + *
> + * This helper operation is preferred over direct assignment to
> + * &backlight_properties.brightness.
> + *
> + * If backlight_device is NULL then silently exit.
> + */
> +static inline void backlight_set_brightness(struct backlight_device *bd, int brightness)
> +{
> +	if (bd)
> +		bd->props.brightness = brightness;

Looking at the drivers I think including a call to backlight_update_status
would simplify a lot of code.

> +}
> +
> +/**
> + * backlight_set_power_on - Set power state to ON.
> + *
> + * This helper operation is preferred over direct assignment to
> + * backlight_properties.power.
> + *
> + * If backlight_device is NULL then silently exit.
> + */
> +static inline void backlight_set_power_on(struct backlight_device *bd)
> +{
> +	if (bd)
> +		bd->props.power = FB_BLANK_UNBLANK;
> +}
> +
> +/**
> + * backlight_set_power_off - Set power state to OFF.
> + *
> + * This helper operation is preferred over direct assignment to
> + * backlight_properties.power.
> + *
> + * If backlight_device is NULL then silently exit.
> + */
> +static inline void backlight_set_power_off(struct backlight_device *bd)

I'm not clear why we need these two above? I thought the long-term plan is
only use backlight_enable/disable and then remove the tri-state power
handling once everyone is converted over?

Or maybe I'm just confused about the goal here ...
-Daniel

> +{
> +	if (bd)
> +		bd->props.power = FB_BLANK_POWERDOWN;
> +}
> +
>  struct backlight_device *
>  backlight_device_register(const char *name, struct device *dev, void *devdata,
>  			  const struct backlight_ops *ops,
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 03/22] backlight: Add get/set operations for brightness/power properties

[Sam Ravnborg]

Hi Daniel et al.
On Tue, Aug 04, 2020 at 06:43:30PM +0200, daniel@ffwll.ch wrote:
> On Sun, Aug 02, 2020 at 01:06:17PM +0200, Sam Ravnborg wrote:
> > Add get and set operations to incapsualte access to backlight properties.
> > 
> > One easy win is that the get/set operatiosn can be used when backlight
> > is not included in the configuration, resulting in simpler code with
> > less ifdef's and thus more readable code.
> > 
> > The set/get methods hides some of the confusing power states, limiting
> > the power state to either ON or OFF for the drivers.
> > 
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Daniel Thompson <daniel.thompson@linaro.org>
> > Cc: Jingoo Han <jingoohan1@gmail.com>
> > ---
> >  include/linux/backlight.h | 72 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > index d683c053ec09..882ceea45ace 100644
> > --- a/include/linux/backlight.h
> > +++ b/include/linux/backlight.h
> > @@ -474,6 +474,78 @@ static inline int backlight_get_brightness(const struct backlight_device *bd)
> >  		return bd->props.brightness;
> >  }
> >  
> > +/**
> > + * backlight_get_actual_brightness - Returns the actual brightness
> > + *
> > + * On failure a negative error code is returned.
> > + */
> > +static inline int backlight_get_actual_brightness(struct backlight_device *bd)
> > +{
> > +	if (bd && bd->ops && bd->ops->get_brightness)
> > +		return bd->ops->get_brightness(bd);
> > +	else
> > +		return -EINVAL;
> > +}
> > +
> > +/**
> > + * backlight_get_max_brightness - Returns maximum brightness
> > + *
> > + * This helper operation is preferred over direct access to
> > + * &backlight_properties.max_brightness
> > + *
> > + * Returns 0 if backlight_device is NULL
> > + */
> > +
> > +static inline int backlight_get_max_brightness(const struct backlight_device *bd)
> > +{
> > +	if (bd)
> > +		return bd->props.max_brightness;
> > +	else
> > +		return 0;
> > +}
> > +
> > +/**
> > + * backlight_set_brightness - Set the brightness to the specified value
> > + *
> > + * This helper operation is preferred over direct assignment to
> > + * &backlight_properties.brightness.
> > + *
> > + * If backlight_device is NULL then silently exit.
> > + */
> > +static inline void backlight_set_brightness(struct backlight_device *bd, int brightness)
> > +{
> > +	if (bd)
> > +		bd->props.brightness = brightness;
> 
> Looking at the drivers I think including a call to backlight_update_status
> would simplify a lot of code.
> 
> > +}
> > +
> > +/**
> > + * backlight_set_power_on - Set power state to ON.
> > + *
> > + * This helper operation is preferred over direct assignment to
> > + * backlight_properties.power.
> > + *
> > + * If backlight_device is NULL then silently exit.
> > + */
> > +static inline void backlight_set_power_on(struct backlight_device *bd)
> > +{
> > +	if (bd)
> > +		bd->props.power = FB_BLANK_UNBLANK;
> > +}
> > +
> > +/**
> > + * backlight_set_power_off - Set power state to OFF.
> > + *
> > + * This helper operation is preferred over direct assignment to
> > + * backlight_properties.power.
> > + *
> > + * If backlight_device is NULL then silently exit.
> > + */
> > +static inline void backlight_set_power_off(struct backlight_device *bd)
> 
> I'm not clear why we need these two above? I thought the long-term plan is
> only use backlight_enable/disable and then remove the tri-state power
> handling once everyone is converted over?
> 
> Or maybe I'm just confused about the goal here ...

My TODO for v2:
- Check all get_brightness implmentations.
  Using backlight_get_brightness is wrong - find a better way
  Check that they do return the actual brightness and not just the copy
  from the backlight core
- Get rid of all uses of power_on/off - this is just another way to
  disable backlight - so be explicit about it
- Consolidate the backlight_set_brightness(); backlight_update() pattern
  to a helper
- Look into a mipi helper for backlight
- Consider if we can change the backlight core to use an u32 for
  brightness
- Take a look at Daniels rambling about drm_connector and backlight
- Convert some platform backlight drivers to updated interface - to verify
  that they do not add new demends

The above should address feedback from Daniel etc. Thanks!
No promises when I get time to do it - this list was mostly
to help myself so I did not forget.

Note: My ISP blocked my attempt to reply to PATCH 0 - so I replied to
this with the TODO list.

        Sam


> -Daniel
> 
> > +{
> > +	if (bd)
> > +		bd->props.power = FB_BLANK_POWERDOWN;
> > +}
> > +
> >  struct backlight_device *
> >  backlight_device_register(const char *name, struct device *dev, void *devdata,
> >  			  const struct backlight_ops *ops,
> > -- 
> > 2.25.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 03/22] backlight: Add get/set operations for brightness/power properties

[daniel]

On Tue, Aug 04, 2020 at 09:56:00PM +0200, Sam Ravnborg wrote:
> Hi Daniel et al.
> On Tue, Aug 04, 2020 at 06:43:30PM +0200, daniel@ffwll.ch wrote:
> > On Sun, Aug 02, 2020 at 01:06:17PM +0200, Sam Ravnborg wrote:
> > > Add get and set operations to incapsualte access to backlight properties.
> > > 
> > > One easy win is that the get/set operatiosn can be used when backlight
> > > is not included in the configuration, resulting in simpler code with
> > > less ifdef's and thus more readable code.
> > > 
> > > The set/get methods hides some of the confusing power states, limiting
> > > the power state to either ON or OFF for the drivers.
> > > 
> > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > > Cc: Lee Jones <lee.jones@linaro.org>
> > > Cc: Daniel Thompson <daniel.thompson@linaro.org>
> > > Cc: Jingoo Han <jingoohan1@gmail.com>
> > > ---
> > >  include/linux/backlight.h | 72 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 72 insertions(+)
> > > 
> > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > > index d683c053ec09..882ceea45ace 100644
> > > --- a/include/linux/backlight.h
> > > +++ b/include/linux/backlight.h
> > > @@ -474,6 +474,78 @@ static inline int backlight_get_brightness(const struct backlight_device *bd)
> > >  		return bd->props.brightness;
> > >  }
> > >  
> > > +/**
> > > + * backlight_get_actual_brightness - Returns the actual brightness
> > > + *
> > > + * On failure a negative error code is returned.
> > > + */
> > > +static inline int backlight_get_actual_brightness(struct backlight_device *bd)
> > > +{
> > > +	if (bd && bd->ops && bd->ops->get_brightness)
> > > +		return bd->ops->get_brightness(bd);
> > > +	else
> > > +		return -EINVAL;
> > > +}
> > > +
> > > +/**
> > > + * backlight_get_max_brightness - Returns maximum brightness
> > > + *
> > > + * This helper operation is preferred over direct access to
> > > + * &backlight_properties.max_brightness
> > > + *
> > > + * Returns 0 if backlight_device is NULL
> > > + */
> > > +
> > > +static inline int backlight_get_max_brightness(const struct backlight_device *bd)
> > > +{
> > > +	if (bd)
> > > +		return bd->props.max_brightness;
> > > +	else
> > > +		return 0;
> > > +}
> > > +
> > > +/**
> > > + * backlight_set_brightness - Set the brightness to the specified value
> > > + *
> > > + * This helper operation is preferred over direct assignment to
> > > + * &backlight_properties.brightness.
> > > + *
> > > + * If backlight_device is NULL then silently exit.
> > > + */
> > > +static inline void backlight_set_brightness(struct backlight_device *bd, int brightness)
> > > +{
> > > +	if (bd)
> > > +		bd->props.brightness = brightness;
> > 
> > Looking at the drivers I think including a call to backlight_update_status
> > would simplify a lot of code.
> > 
> > > +}
> > > +
> > > +/**
> > > + * backlight_set_power_on - Set power state to ON.
> > > + *
> > > + * This helper operation is preferred over direct assignment to
> > > + * backlight_properties.power.
> > > + *
> > > + * If backlight_device is NULL then silently exit.
> > > + */
> > > +static inline void backlight_set_power_on(struct backlight_device *bd)
> > > +{
> > > +	if (bd)
> > > +		bd->props.power = FB_BLANK_UNBLANK;
> > > +}
> > > +
> > > +/**
> > > + * backlight_set_power_off - Set power state to OFF.
> > > + *
> > > + * This helper operation is preferred over direct assignment to
> > > + * backlight_properties.power.
> > > + *
> > > + * If backlight_device is NULL then silently exit.
> > > + */
> > > +static inline void backlight_set_power_off(struct backlight_device *bd)
> > 
> > I'm not clear why we need these two above? I thought the long-term plan is
> > only use backlight_enable/disable and then remove the tri-state power
> > handling once everyone is converted over?
> > 
> > Or maybe I'm just confused about the goal here ...
> 
> My TODO for v2:
> - Check all get_brightness implmentations.
>   Using backlight_get_brightness is wrong - find a better way
>   Check that they do return the actual brightness and not just the copy
>   from the backlight core

Well it's only for the get_brigthness callback where I think this is
problemantic. In update_state callback I think it's a good helper.

> - Get rid of all uses of power_on/off - this is just another way to
>   disable backlight - so be explicit about it
> - Consolidate the backlight_set_brightness(); backlight_update() pattern
>   to a helper
> - Look into a mipi helper for backlight

Imo perfectly fine to leave that out for some todo, otherwise this will
get huge.

> - Consider if we can change the backlight core to use an u32 for
>   brightness
> - Take a look at Daniels rambling about drm_connector and backlight

Also something we can postpone I think. We can still used devm_ together
with a refcounted backlight (like we do with devm_drm_dev_alloc), and my
gut feel says refcounted backlight is probably the way to go eventually.

But also, we've been talking about drm_connector->panel for years, there's
no rush at all.

> - Convert some platform backlight drivers to updated interface - to verify
>   that they do not add new demends
> 
> The above should address feedback from Daniel etc. Thanks!
> No promises when I get time to do it - this list was mostly
> to help myself so I did not forget.

Sounds all good to me, and thanks for doing all this work!

Cheers, Daniel

> 
> Note: My ISP blocked my attempt to reply to PATCH 0 - so I replied to
> this with the TODO list.
> 
>         Sam
> 
> 
> > -Daniel
> > 
> > > +{
> > > +	if (bd)
> > > +		bd->props.power = FB_BLANK_POWERDOWN;
> > > +}
> > > +
> > >  struct backlight_device *
> > >  backlight_device_register(const char *name, struct device *dev, void *devdata,
> > >  			  const struct backlight_ops *ops,
> > > -- 
> > > 2.25.1
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 04/22] backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters

[Sam Ravnborg]

Introduce use of DECLARE_BACKLIGHT_INIT_RAW when registering the
backlight. This makes the device registration a little simpler.

Use get/set operations for power thus avoid the use of the
rather confusion power states.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/video/backlight/gpio_backlight.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 6f78d928f054..c94fbfa755c4 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -49,7 +49,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
 	struct device_node *of_node = dev->of_node;
-	struct backlight_properties props;
+	DECLARE_BACKLIGHT_INIT_RAW(props, 1, 1);
 	struct backlight_device *bl;
 	struct gpio_backlight *gbl;
 	int ret, init_brightness, def_value;
@@ -72,9 +72,6 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	memset(&props, 0, sizeof(props));
-	props.type = BACKLIGHT_RAW;
-	props.max_brightness = 1;
 	bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
 					    &gpio_backlight_ops, &props);
 	if (IS_ERR(bl)) {
@@ -85,15 +82,15 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 	/* Set the initial power state */
 	if (!of_node || !of_node->phandle)
 		/* Not booted with device tree or no phandle link to the node */
-		bl->props.power = def_value ? FB_BLANK_UNBLANK
-					    : FB_BLANK_POWERDOWN;
+		if (def_value)
+			backlight_set_power_on(bl);
+		else
+			backlight_set_power_off(bl);
 	else if (gpiod_get_direction(gbl->gpiod) == 0 &&
 		 gpiod_get_value_cansleep(gbl->gpiod) == 0)
-		bl->props.power = FB_BLANK_POWERDOWN;
+		backlight_set_power_off(bl);
 	else
-		bl->props.power = FB_BLANK_UNBLANK;
-
-	bl->props.brightness = 1;
+		backlight_set_power_on(bl);
 
 	init_brightness = backlight_get_brightness(bl);
 	ret = gpiod_direction_output(gbl->gpiod, init_brightness);
-- 
2.25.1

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

[PATCH v1 05/22] drm/gma500: Backlight support

[Sam Ravnborg]

The backlight support is updated to utilise newly added macros and
functions thus simplifying the code.

- Introduced backlight_set_brightness() that can be called with a
  NULL backlight_device
- backlight_update_status() can be called with a NULL backlight_device.
  Benefit from this by removing helper that otherwise was iffed'ed out
- Use DECLARE_BACKLIGHT_INIT_PLATFORM() when creating backlight devices
- Replace direct access to backlight_properties with get/set methods

No functional changes, but a nice reduction in complexity and code.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/backlight.c       | 35 ++++++------------------
 drivers/gpu/drm/gma500/cdv_device.c      | 29 +++++++++-----------
 drivers/gpu/drm/gma500/mdfld_device.c    |  9 ++----
 drivers/gpu/drm/gma500/oaktrail_device.c | 10 ++-----
 drivers/gpu/drm/gma500/opregion.c        |  2 +-
 drivers/gpu/drm/gma500/psb_device.c      | 10 ++-----
 drivers/gpu/drm/gma500/psb_drv.c         |  8 ++----
 7 files changed, 31 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/gma500/backlight.c b/drivers/gpu/drm/gma500/backlight.c
index 35600d070cb5..48b166a702fa 100644
--- a/drivers/gpu/drm/gma500/backlight.c
+++ b/drivers/gpu/drm/gma500/backlight.c
@@ -13,48 +13,31 @@
 #include "intel_bios.h"
 #include "power.h"
 
-#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
-static void do_gma_backlight_set(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = dev->dev_private;
-	backlight_update_status(dev_priv->backlight_device);
-}
-#endif
-
 void gma_backlight_enable(struct drm_device *dev)
 {
-#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	dev_priv->backlight_enabled = true;
-	if (dev_priv->backlight_device) {
-		dev_priv->backlight_device->props.brightness = dev_priv->backlight_level;
-		do_gma_backlight_set(dev);
-	}
-#endif	
+	backlight_set_brightness(dev_priv->backlight_device,
+				 dev_priv->backlight_level);
+	backlight_update_status(dev_priv->backlight_device);
 }
 
 void gma_backlight_disable(struct drm_device *dev)
 {
-#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	dev_priv->backlight_enabled = false;
-	if (dev_priv->backlight_device) {
-		dev_priv->backlight_device->props.brightness = 0;
-		do_gma_backlight_set(dev);
-	}
-#endif	
+	backlight_set_brightness(dev_priv->backlight_device, 0);
+	backlight_update_status(dev_priv->backlight_device);
 }
 
 void gma_backlight_set(struct drm_device *dev, int v)
 {
-#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	dev_priv->backlight_level = v;
-	if (dev_priv->backlight_device && dev_priv->backlight_enabled) {
-		dev_priv->backlight_device->props.brightness = v;
-		do_gma_backlight_set(dev);
+	if (dev_priv->backlight_enabled) {
+		backlight_set_brightness(dev_priv->backlight_device, v);
+		backlight_update_status(dev_priv->backlight_device);
 	}
-#endif	
 }
 
 int gma_backlight_init(struct drm_device *dev)
@@ -73,7 +56,7 @@ void gma_backlight_exit(struct drm_device *dev)
 #ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	if (dev_priv->backlight_device) {
-		dev_priv->backlight_device->props.brightness = 0;
+		backlight_set_brightness(dev_priv->backlight_device, 0);
 		backlight_update_status(dev_priv->backlight_device);
 		backlight_device_unregister(dev_priv->backlight_device);
 	}
diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c
index 4d216a0205f2..31a1eef35a3c 100644
--- a/drivers/gpu/drm/gma500/cdv_device.c
+++ b/drivers/gpu/drm/gma500/cdv_device.c
@@ -111,7 +111,7 @@ static int cdv_get_brightness(struct backlight_device *bd)
 static int cdv_set_brightness(struct backlight_device *bd)
 {
 	struct drm_device *dev = bl_get_data(bd);
-	int level = bd->props.brightness;
+	int level = backlight_get_brightness(bd);
 	u32 blc_pwm_ctl;
 
 	/* Percentage 1-100% being valid */
@@ -145,21 +145,18 @@ static const struct backlight_ops cdv_ops = {
 static int cdv_backlight_init(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = dev->dev_private;
-	struct backlight_properties props;
-
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.max_brightness = 100;
-	props.type = BACKLIGHT_PLATFORM;
-
-	cdv_backlight_device = backlight_device_register("psb-bl",
-					NULL, (void *)dev, &cdv_ops, &props);
-	if (IS_ERR(cdv_backlight_device))
-		return PTR_ERR(cdv_backlight_device);
-
-	cdv_backlight_device->props.brightness =
-			cdv_get_brightness(cdv_backlight_device);
-	backlight_update_status(cdv_backlight_device);
-	dev_priv->backlight_device = cdv_backlight_device;
+	DECLARE_BACKLIGHT_INIT_PLATFORM(props, 0, 100);
+	struct backlight_device *bl;
+
+	bl = backlight_device_register("psb-bl",
+				       NULL, (void *)dev, &cdv_ops, &props);
+	if (IS_ERR(bl))
+		return PTR_ERR(bl);
+
+	cdv_backlight_device = bl;
+	backlight_set_brightness(bl, cdv_get_brightness(bl));
+	backlight_update_status(bl);
+	dev_priv->backlight_device = bl;
 	dev_priv->backlight_enabled = true;
 	return 0;
 }
diff --git a/drivers/gpu/drm/gma500/mdfld_device.c b/drivers/gpu/drm/gma500/mdfld_device.c
index b718efccdcf2..674e6e619d9a 100644
--- a/drivers/gpu/drm/gma500/mdfld_device.c
+++ b/drivers/gpu/drm/gma500/mdfld_device.c
@@ -42,7 +42,7 @@ int mdfld_set_brightness(struct backlight_device *bd)
 	struct drm_device *dev =
 		(struct drm_device *)bl_get_data(mdfld_backlight_device);
 	struct drm_psb_private *dev_priv = dev->dev_private;
-	int level = bd->props.brightness;
+	int level = backlight_get_brightness(bd);
 
 	DRM_DEBUG_DRIVER("backlight level set to %d\n", level);
 
@@ -113,12 +113,9 @@ static int device_backlight_init(struct drm_device *dev)
 
 static int mdfld_backlight_init(struct drm_device *dev)
 {
-	struct backlight_properties props;
+	DECLARE_BACKLIGHT_INIT_PLATFORM(props, BRIGHTNESS_MAX_LEVEL, BRIGHTNESS_MAX_LEVEL);
 	int ret = 0;
 
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.max_brightness = BRIGHTNESS_MAX_LEVEL;
-	props.type = BACKLIGHT_PLATFORM;
 	mdfld_backlight_device = backlight_device_register("mdfld-bl",
 				NULL, (void *)dev, &mdfld_ops, &props);
 
@@ -129,8 +126,6 @@ static int mdfld_backlight_init(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	mdfld_backlight_device->props.brightness = BRIGHTNESS_MAX_LEVEL;
-	mdfld_backlight_device->props.max_brightness = BRIGHTNESS_MAX_LEVEL;
 	backlight_update_status(mdfld_backlight_device);
 	return 0;
 }
diff --git a/drivers/gpu/drm/gma500/oaktrail_device.c b/drivers/gpu/drm/gma500/oaktrail_device.c
index ade7e2416a66..52c0f1a35d3f 100644
--- a/drivers/gpu/drm/gma500/oaktrail_device.c
+++ b/drivers/gpu/drm/gma500/oaktrail_device.c
@@ -55,7 +55,7 @@ static int oaktrail_set_brightness(struct backlight_device *bd)
 {
 	struct drm_device *dev = bl_get_data(oaktrail_backlight_device);
 	struct drm_psb_private *dev_priv = dev->dev_private;
-	int level = bd->props.brightness;
+	int level = backlight_get_brightness(bd);
 	u32 blc_pwm_ctl;
 	u32 max_pwm_blc;
 
@@ -136,13 +136,9 @@ static const struct backlight_ops oaktrail_ops = {
 
 static int oaktrail_backlight_init(struct drm_device *dev)
 {
+	DECLARE_BACKLIGHT_INIT_PLATFORM(props, 100, 100);
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	int ret;
-	struct backlight_properties props;
-
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.max_brightness = 100;
-	props.type = BACKLIGHT_PLATFORM;
 
 	oaktrail_backlight_device = backlight_device_register("oaktrail-bl",
 				NULL, (void *)dev, &oaktrail_ops, &props);
@@ -155,8 +151,6 @@ static int oaktrail_backlight_init(struct drm_device *dev)
 		backlight_device_unregister(oaktrail_backlight_device);
 		return ret;
 	}
-	oaktrail_backlight_device->props.brightness = 100;
-	oaktrail_backlight_device->props.max_brightness = 100;
 	backlight_update_status(oaktrail_backlight_device);
 	dev_priv->backlight_device = oaktrail_backlight_device;
 	return 0;
diff --git a/drivers/gpu/drm/gma500/opregion.c b/drivers/gpu/drm/gma500/opregion.c
index eab6d889bde9..68587cdf6206 100644
--- a/drivers/gpu/drm/gma500/opregion.c
+++ b/drivers/gpu/drm/gma500/opregion.c
@@ -163,7 +163,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 	if (bclp > 255)
 		return ASLE_BACKLIGHT_FAILED;
 
-	gma_backlight_set(dev, bclp * bd->props.max_brightness / 255);
+	gma_backlight_set(dev, bclp * backlight_get_max_brightness(bd) / 255);
 
 	asle->cblv = (bclp * 0x64) / 0xff | ASLE_CBLV_VALID;
 
diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
index ece994c4c21a..857681b860c4 100644
--- a/drivers/gpu/drm/gma500/psb_device.c
+++ b/drivers/gpu/drm/gma500/psb_device.c
@@ -92,7 +92,7 @@ static int psb_backlight_setup(struct drm_device *dev)
 static int psb_set_brightness(struct backlight_device *bd)
 {
 	struct drm_device *dev = bl_get_data(psb_backlight_device);
-	int level = bd->props.brightness;
+	int level = backlight_get_brightness(bd);
 
 	/* Percentage 1-100% being valid */
 	if (level < 1)
@@ -110,13 +110,9 @@ static const struct backlight_ops psb_ops = {
 
 static int psb_backlight_init(struct drm_device *dev)
 {
+	DECLARE_BACKLIGHT_INIT_PLATFORM(props, 100, 100);
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	int ret;
-	struct backlight_properties props;
-
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.max_brightness = 100;
-	props.type = BACKLIGHT_PLATFORM;
 
 	psb_backlight_device = backlight_device_register("psb-bl",
 					NULL, (void *)dev, &psb_ops, &props);
@@ -129,8 +125,6 @@ static int psb_backlight_init(struct drm_device *dev)
 		psb_backlight_device = NULL;
 		return ret;
 	}
-	psb_backlight_device->props.brightness = 100;
-	psb_backlight_device->props.max_brightness = 100;
 	backlight_update_status(psb_backlight_device);
 	dev_priv->backlight_device = psb_backlight_device;
 
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 2411eb9827b8..0da56d4b6f7b 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -398,12 +398,8 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 
 static inline void get_brightness(struct backlight_device *bd)
 {
-#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
-	if (bd) {
-		bd->props.brightness = bd->ops->get_brightness(bd);
-		backlight_update_status(bd);
-	}
-#endif
+	backlight_set_brightness(bd, backlight_get_actual_brightness(bd));
+	backlight_update_status(bd);
 }
 
 static long psb_unlocked_ioctl(struct file *filp, unsigned int cmd,
-- 
2.25.1

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

[PATCH v1 06/22] drm/panel: asus-z00t-tm5p5-n35596: Backlight update

[Sam Ravnborg]

Update backlight to use macro for initialization and the
backlight_get_brightness() operation to simply the update operation.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Konrad Dybcio <konradybcio@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c  | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c
index 39e0f0373f3c..c090fc6f1ed5 100644
--- a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c
+++ b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c
@@ -216,14 +216,9 @@ static const struct drm_panel_funcs tm5p5_nt35596_panel_funcs = {
 static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl)
 {
 	struct mipi_dsi_device *dsi = bl_get_data(bl);
-	u16 brightness = bl->props.brightness;
+	int brightness = backlight_get_brightness(bl);
 	int ret;
 
-	if (bl->props.power != FB_BLANK_UNBLANK ||
-	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
-	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
-		brightness = 0;
-
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
 	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
@@ -238,7 +233,7 @@ static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl)
 static int tm5p5_nt35596_bl_get_brightness(struct backlight_device *bl)
 {
 	struct mipi_dsi_device *dsi = bl_get_data(bl);
-	u16 brightness = bl->props.brightness;
+	u16 brightness = backlight_get_brightness(bl);
 	int ret;
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
@@ -261,11 +256,7 @@ static struct backlight_device *
 tm5p5_nt35596_create_backlight(struct mipi_dsi_device *dsi)
 {
 	struct device *dev = &dsi->dev;
-	const struct backlight_properties props = {
-		.type = BACKLIGHT_RAW,
-		.brightness = 255,
-		.max_brightness = 255,
-	};
+	DECLARE_BACKLIGHT_INIT_RAW(props, 255, 255);
 
 	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
 					      &tm5p5_nt35596_bl_ops, &props);
-- 
2.25.1

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

Re: [PATCH v1 06/22] drm/panel: asus-z00t-tm5p5-n35596: Backlight update

[daniel]

On Sun, Aug 02, 2020 at 01:06:20PM +0200, Sam Ravnborg wrote:
> Update backlight to use macro for initialization and the
> backlight_get_brightness() operation to simply the update operation.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Konrad Dybcio <konradybcio@gmail.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> ---
>  .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c  | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c
> index 39e0f0373f3c..c090fc6f1ed5 100644
> --- a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c
> +++ b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c
> @@ -216,14 +216,9 @@ static const struct drm_panel_funcs tm5p5_nt35596_panel_funcs = {
>  static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl)
>  {
>  	struct mipi_dsi_device *dsi = bl_get_data(bl);
> -	u16 brightness = bl->props.brightness;
> +	int brightness = backlight_get_brightness(bl);
>  	int ret;
>  
> -	if (bl->props.power != FB_BLANK_UNBLANK ||
> -	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
> -	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> -		brightness = 0;
> -
>  	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>  
>  	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
> @@ -238,7 +233,7 @@ static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl)
>  static int tm5p5_nt35596_bl_get_brightness(struct backlight_device *bl)
>  {
>  	struct mipi_dsi_device *dsi = bl_get_data(bl);
> -	u16 brightness = bl->props.brightness;
> +	u16 brightness = backlight_get_brightness(bl);

I'm not sure why we do this, but your patch here changes behaviour in a
way that has bitten us in the past: This now reports a brightness of 0
when the backlight is off. On some backlights (especially firmware ones) 0
means "lowest value", not actually off, so that's one confusion. The other
problem is then that userspace tends to use this as the backlight value to
restore on next boot (or after resume, or after vt switch, resulting in a
very dark or black screen).

Therefore I think in these cases we actually need the direct
bl->props.brightness value.

I think an even cleaner way to solve this would be to change the
get_brightness code in actual_brightness_show to handle negative error
codes from ->get_brightness and use that to fall back to
bd->props.brightness, then we could remove this code here.

That reminds me, probably not a good idea to store a negative value in
backlight_force_update() if this goes wrong into bl->props.brightness.
-Daniel

>  	int ret;
>  
>  	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> @@ -261,11 +256,7 @@ static struct backlight_device *
>  tm5p5_nt35596_create_backlight(struct mipi_dsi_device *dsi)
>  {
>  	struct device *dev = &dsi->dev;
> -	const struct backlight_properties props = {
> -		.type = BACKLIGHT_RAW,
> -		.brightness = 255,
> -		.max_brightness = 255,
> -	};
> +	DECLARE_BACKLIGHT_INIT_RAW(props, 255, 255);
>  
>  	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
>  					      &tm5p5_nt35596_bl_ops, &props);
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 07/22] drm/panel: jdi-lt070me05000: Backlight update

[Sam Ravnborg]

Update backlight to use macro for initialization and the
backlight_get_brightness() operation to simply the update operation.

Moved init of backlight device so it comes after drm_panel_init().
This is the order that is required by drm_panel.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Vinay Simha BN <simhavcs@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
index 4bfd8c877c8e..bacb1b5bc524 100644
--- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
+++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
@@ -329,7 +329,7 @@ static int dsi_dcs_bl_get_brightness(struct backlight_device *bl)
 {
 	struct mipi_dsi_device *dsi = bl_get_data(bl);
 	int ret;
-	u16 brightness = bl->props.brightness;
+	u16 brightness = backlight_get_brightness(bl);
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
@@ -349,7 +349,7 @@ static int dsi_dcs_bl_update_status(struct backlight_device *bl)
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
-	ret = mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness);
+	ret = mipi_dsi_dcs_set_display_brightness(dsi, backlight_get_brightness(bl));
 	if (ret < 0)
 		return ret;
 
@@ -367,12 +367,7 @@ static struct backlight_device *
 drm_panel_create_dsi_backlight(struct mipi_dsi_device *dsi)
 {
 	struct device *dev = &dsi->dev;
-	struct backlight_properties props;
-
-	memset(&props, 0, sizeof(props));
-	props.type = BACKLIGHT_RAW;
-	props.brightness = 255;
-	props.max_brightness = 255;
+	DECLARE_BACKLIGHT_INIT_RAW(props, 255, 255);
 
 	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
 					      &dsi_bl_ops, &props);
@@ -431,6 +426,9 @@ static int jdi_panel_add(struct jdi_panel *jdi)
 		return ret;
 	}
 
+	drm_panel_init(&jdi->base, &jdi->dsi->dev, &jdi_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
 	jdi->backlight = drm_panel_create_dsi_backlight(jdi->dsi);
 	if (IS_ERR(jdi->backlight)) {
 		ret = PTR_ERR(jdi->backlight);
@@ -438,9 +436,6 @@ static int jdi_panel_add(struct jdi_panel *jdi)
 		return ret;
 	}
 
-	drm_panel_init(&jdi->base, &jdi->dsi->dev, &jdi_panel_funcs,
-		       DRM_MODE_CONNECTOR_DSI);
-
 	ret = drm_panel_add(&jdi->base);
 
 	return ret;
-- 
2.25.1

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

[PATCH v1 08/22] drm/panel: novatek-nt35510: Backlight update

[Sam Ravnborg]

- Replace direct access to backlight_properties with
  backlight_get_brightness().
- Drop debug printout
- Use macro for initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-novatek-nt35510.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
index 4a8fa908a2cf..ee4919a27480 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
@@ -654,10 +654,9 @@ static int nt35510_set_brightness(struct backlight_device *bl)
 {
 	struct nt35510 *nt = bl_get_data(bl);
 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
-	u8 brightness = bl->props.brightness;
+	u8 brightness = backlight_get_brightness(bl);
 	int ret;
 
-	DRM_DEV_DEBUG(nt->dev, "set brightness %d\n", brightness);
 	ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
 				 &brightness,
 				 sizeof(brightness));
@@ -943,16 +942,14 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
 	}
 	if (!nt->panel.backlight) {
 		struct backlight_device *bl;
+		DECLARE_BACKLIGHT_INIT_RAW(props, 255, 255);
 
 		bl = devm_backlight_device_register(dev, "nt35510", dev, nt,
-						    &nt35510_bl_ops, NULL);
+						    &nt35510_bl_ops, &props);
 		if (IS_ERR(bl)) {
 			DRM_DEV_ERROR(dev, "failed to register backlight device\n");
 			return PTR_ERR(bl);
 		}
-		bl->props.max_brightness = 255;
-		bl->props.brightness = 255;
-		bl->props.power = FB_BLANK_POWERDOWN;
 		nt->panel.backlight = bl;
 	}
 
-- 
2.25.1

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

Re: [PATCH v1 08/22] drm/panel: novatek-nt35510: Backlight update

[Linus Walleij]

On Sun, Aug 2, 2020 at 1:07 PM Sam Ravnborg <sam@ravnborg.org> wrote:

> - Replace direct access to backlight_properties with
>   backlight_get_brightness().
> - Drop debug printout
> - Use macro for initialization
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 09/22] drm/panel: orisetech-otm8009a: Backlight update

[Sam Ravnborg]

- Replace direct access to backlight_properties with
  backlight_get_brightness().
- Use brightness and not power to determine if backlight is off
- Use the devm_ variant for registering backlight device, and drop
  all explicit unregistering of the backlight device.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Philippe CORNU <philippe.cornu@st.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-orisetech-otm8009a.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
index bb0c992171e8..e6534cca2a84 100644
--- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
+++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
@@ -388,6 +388,7 @@ static const struct drm_panel_funcs otm8009a_drm_funcs = {
 static int otm8009a_backlight_update_status(struct backlight_device *bd)
 {
 	struct otm8009a *ctx = bl_get_data(bd);
+	u8 brightness = backlight_get_brightness(bd);
 	u8 data[2];
 
 	if (!ctx->prepared) {
@@ -395,13 +396,13 @@ static int otm8009a_backlight_update_status(struct backlight_device *bd)
 		return -ENXIO;
 	}
 
-	if (bd->props.power <= FB_BLANK_NORMAL) {
+	if (brightness > 0) {
 		/* Power on the backlight with the requested brightness
 		 * Note We can not use mipi_dsi_dcs_set_display_brightness()
 		 * as otm8009a driver support only 8-bit brightness (1 param).
 		 */
 		data[0] = MIPI_DCS_SET_DISPLAY_BRIGHTNESS;
-		data[1] = bd->props.brightness;
+		data[1] = brightness;
 		otm8009a_dcs_write_buf_hs(ctx, data, ARRAY_SIZE(data));
 
 		/* set Brightness Control & Backlight on */
@@ -428,6 +429,7 @@ static int otm8009a_probe(struct mipi_dsi_device *dsi)
 	struct device *dev = &dsi->dev;
 	struct otm8009a *ctx;
 	int ret;
+	DECLARE_BACKLIGHT_INIT_RAW(props, OTM8009A_BACKLIGHT_DEFAULT, OTM8009A_BACKLIGHT_MAX);
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
@@ -462,25 +464,19 @@ static int otm8009a_probe(struct mipi_dsi_device *dsi)
 	ctx->bl_dev = devm_backlight_device_register(dev, dev_name(dev),
 						     dsi->host->dev, ctx,
 						     &otm8009a_backlight_ops,
-						     NULL);
+						     &props);
 	if (IS_ERR(ctx->bl_dev)) {
 		ret = PTR_ERR(ctx->bl_dev);
 		dev_err(dev, "failed to register backlight: %d\n", ret);
 		return ret;
 	}
 
-	ctx->bl_dev->props.max_brightness = OTM8009A_BACKLIGHT_MAX;
-	ctx->bl_dev->props.brightness = OTM8009A_BACKLIGHT_DEFAULT;
-	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
-	ctx->bl_dev->props.type = BACKLIGHT_RAW;
-
 	drm_panel_add(&ctx->panel);
 
 	ret = mipi_dsi_attach(dsi);
 	if (ret < 0) {
 		dev_err(dev, "mipi_dsi_attach failed. Is host ready?\n");
 		drm_panel_remove(&ctx->panel);
-		backlight_device_unregister(ctx->bl_dev);
 		return ret;
 	}
 
-- 
2.25.1

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

[PATCH v1 10/22] drm/panel: raydium-rm67191: Backlight update

[Sam Ravnborg]

- Replace direct access to backlight_properties with
  backlight_get_brightness().
- Use macro for initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Robert Chiras <robert.chiras@nxp.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-raydium-rm67191.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
index 313637d53d28..5553db385dd5 100644
--- a/drivers/gpu/drm/panel/panel-raydium-rm67191.c
+++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
@@ -479,7 +479,7 @@ static int rad_bl_get_brightness(struct backlight_device *bl)
 	if (ret < 0)
 		return ret;
 
-	bl->props.brightness = brightness;
+	backlight_set_brightness(bl, brightness);
 
 	return brightness & 0xff;
 }
@@ -495,7 +495,7 @@ static int rad_bl_update_status(struct backlight_device *bl)
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
-	ret = mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness);
+	ret = mipi_dsi_dcs_set_display_brightness(dsi, backlight_get_brightness(bl));
 	if (ret < 0)
 		return ret;
 
@@ -539,10 +539,10 @@ static int rad_init_regulators(struct rad_panel *rad)
 
 static int rad_panel_probe(struct mipi_dsi_device *dsi)
 {
+	DECLARE_BACKLIGHT_INIT_RAW(bl_props, 255, 255);
 	struct device *dev = &dsi->dev;
 	struct device_node *np = dev->of_node;
 	struct rad_panel *panel;
-	struct backlight_properties bl_props;
 	int ret;
 	u32 video_mode;
 
@@ -588,11 +588,6 @@ static int rad_panel_probe(struct mipi_dsi_device *dsi)
 	if (IS_ERR(panel->reset))
 		return PTR_ERR(panel->reset);
 
-	memset(&bl_props, 0, sizeof(bl_props));
-	bl_props.type = BACKLIGHT_RAW;
-	bl_props.brightness = 255;
-	bl_props.max_brightness = 255;
-
 	panel->backlight = devm_backlight_device_register(dev, dev_name(dev),
 							  dev, dsi, &rad_bl_ops,
 							  &bl_props);
-- 
2.25.1

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

Re: [PATCH v1 10/22] drm/panel: raydium-rm67191: Backlight update

[daniel]

On Sun, Aug 02, 2020 at 01:06:24PM +0200, Sam Ravnborg wrote:
> - Replace direct access to backlight_properties with
>   backlight_get_brightness().
> - Use macro for initialization
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/drm/panel/panel-raydium-rm67191.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
> index 313637d53d28..5553db385dd5 100644
> --- a/drivers/gpu/drm/panel/panel-raydium-rm67191.c
> +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
> @@ -479,7 +479,7 @@ static int rad_bl_get_brightness(struct backlight_device *bl)
>  	if (ret < 0)
>  		return ret;
>  
> -	bl->props.brightness = brightness;
> +	backlight_set_brightness(bl, brightness);

This sounds like a bad idea, again because this overrides the software
brightness value potentially when the backlight is off. Althought that's
checked a bit higher up, so not too terrible.

I'm also feeling like a helper for standard mipi dsi panel backlight would
be great, it's pretty much all the same code.
-Daniel

>  
>  	return brightness & 0xff;
>  }
> @@ -495,7 +495,7 @@ static int rad_bl_update_status(struct backlight_device *bl)
>  
>  	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>  
> -	ret = mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness);
> +	ret = mipi_dsi_dcs_set_display_brightness(dsi, backlight_get_brightness(bl));
>  	if (ret < 0)
>  		return ret;
>  
> @@ -539,10 +539,10 @@ static int rad_init_regulators(struct rad_panel *rad)
>  
>  static int rad_panel_probe(struct mipi_dsi_device *dsi)
>  {
> +	DECLARE_BACKLIGHT_INIT_RAW(bl_props, 255, 255);
>  	struct device *dev = &dsi->dev;
>  	struct device_node *np = dev->of_node;
>  	struct rad_panel *panel;
> -	struct backlight_properties bl_props;
>  	int ret;
>  	u32 video_mode;
>  
> @@ -588,11 +588,6 @@ static int rad_panel_probe(struct mipi_dsi_device *dsi)
>  	if (IS_ERR(panel->reset))
>  		return PTR_ERR(panel->reset);
>  
> -	memset(&bl_props, 0, sizeof(bl_props));
> -	bl_props.type = BACKLIGHT_RAW;
> -	bl_props.brightness = 255;
> -	bl_props.max_brightness = 255;
> -
>  	panel->backlight = devm_backlight_device_register(dev, dev_name(dev),
>  							  dev, dsi, &rad_bl_ops,
>  							  &bl_props);
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 11/22] drm/panel: samsung-s6e63m0: Backlight update

[Sam Ravnborg]

- Use drm_panel backlight support
- Use macro for backlight initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 25 +++++++------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
index a5f76eb4fa25..e30ef655a3c3 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
@@ -89,7 +89,6 @@ static u8 const s6e63m0_gamma_22[NUM_GAMMA_LEVELS][GAMMA_TABLE_COUNT] = {
 struct s6e63m0 {
 	struct device *dev;
 	struct drm_panel panel;
-	struct backlight_device *bl_dev;
 
 	struct regulator_bulk_data supplies[2];
 	struct gpio_desc *reset_gpio;
@@ -293,8 +292,6 @@ static int s6e63m0_disable(struct drm_panel *panel)
 	if (!ctx->enabled)
 		return 0;
 
-	backlight_disable(ctx->bl_dev);
-
 	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
 	msleep(200);
 
@@ -355,8 +352,6 @@ static int s6e63m0_enable(struct drm_panel *panel)
 
 	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
 
-	backlight_enable(ctx->bl_dev);
-
 	ctx->enabled = true;
 
 	return 0;
@@ -395,7 +390,7 @@ static int s6e63m0_set_brightness(struct backlight_device *bd)
 {
 	struct s6e63m0 *ctx = bl_get_data(bd);
 
-	int brightness = bd->props.brightness;
+	int brightness = backlight_get_brightness(bd);
 
 	/* disable and set new gamma */
 	s6e63m0_dcs_write(ctx, s6e63m0_gamma_22[brightness],
@@ -413,23 +408,21 @@ static const struct backlight_ops s6e63m0_backlight_ops = {
 
 static int s6e63m0_backlight_register(struct s6e63m0 *ctx)
 {
-	struct backlight_properties props = {
-		.type		= BACKLIGHT_RAW,
-		.brightness	= MAX_BRIGHTNESS,
-		.max_brightness = MAX_BRIGHTNESS
-	};
+	DECLARE_BACKLIGHT_INIT_RAW(props, MAX_BRIGHTNESS, MAX_BRIGHTNESS);
 	struct device *dev = ctx->dev;
+	struct backlight_device *bd;
 	int ret = 0;
 
-	ctx->bl_dev = devm_backlight_device_register(dev, "panel", dev, ctx,
-						     &s6e63m0_backlight_ops,
-						     &props);
-	if (IS_ERR(ctx->bl_dev)) {
-		ret = PTR_ERR(ctx->bl_dev);
+	bd = devm_backlight_device_register(dev, "panel", dev, ctx,
+					    &s6e63m0_backlight_ops, &props);
+	if (IS_ERR(bd)) {
+		ret = PTR_ERR(bd);
 		DRM_DEV_ERROR(dev, "error registering backlight device (%d)\n",
 			      ret);
+		return ret;
 	}
 
+	ctx->panel.backlight = bd;
 	return ret;
 }
 
-- 
2.25.1

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

[PATCH v1 12/22] drm/panel: samsung-s6e63j0x03: Backlight update

[Sam Ravnborg]

- Use backlight support from drm_panel.
  This shifts this driver away from manually handling of power state.
- Add helper function for registering backlight, like other samsung
  panel drivers do.
- Use devm_ for backlight register thus benefit from automatic
  unregistering. Drop all explicit unregistering.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 .../gpu/drm/panel/panel-samsung-s6e63j0x03.c  | 56 +++++++++----------
 1 file changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
index a3570e0a90a8..f8dbb1cf1429 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
@@ -36,7 +36,6 @@
 struct s6e63j0x03 {
 	struct device *dev;
 	struct drm_panel panel;
-	struct backlight_device *bl_dev;
 
 	struct regulator_bulk_data supplies[2];
 	struct gpio_desc *reset_gpio;
@@ -181,10 +180,8 @@ static unsigned int s6e63j0x03_get_brightness_index(unsigned int brightness)
 	return index;
 }
 
-static int s6e63j0x03_update_gamma(struct s6e63j0x03 *ctx,
-					unsigned int brightness)
+static int s6e63j0x03_update_gamma(struct s6e63j0x03 *ctx, int brightness)
 {
-	struct backlight_device *bl_dev = ctx->bl_dev;
 	unsigned int index = s6e63j0x03_get_brightness_index(brightness);
 	int ret;
 
@@ -200,15 +197,13 @@ static int s6e63j0x03_update_gamma(struct s6e63j0x03 *ctx,
 	if (ret < 0)
 		return ret;
 
-	bl_dev->props.brightness = brightness;
-
 	return 0;
 }
 
 static int s6e63j0x03_set_brightness(struct backlight_device *bl_dev)
 {
 	struct s6e63j0x03 *ctx = bl_get_data(bl_dev);
-	unsigned int brightness = bl_dev->props.brightness;
+	unsigned int brightness = backlight_get_brightness(bl_dev);
 
 	return s6e63j0x03_update_gamma(ctx, brightness);
 }
@@ -227,8 +222,6 @@ static int s6e63j0x03_disable(struct drm_panel *panel)
 	if (ret < 0)
 		return ret;
 
-	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
-
 	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
 	if (ret < 0)
 		return ret;
@@ -247,8 +240,6 @@ static int s6e63j0x03_unprepare(struct drm_panel *panel)
 	if (ret < 0)
 		return ret;
 
-	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
-
 	return 0;
 }
 
@@ -334,8 +325,6 @@ static int s6e63j0x03_prepare(struct drm_panel *panel)
 	if (ret < 0)
 		goto err;
 
-	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
-
 	return 0;
 
 err:
@@ -395,8 +384,6 @@ static int s6e63j0x03_enable(struct drm_panel *panel)
 	if (ret < 0)
 		return ret;
 
-	ctx->bl_dev->props.power = FB_BLANK_UNBLANK;
-
 	return 0;
 }
 
@@ -432,6 +419,25 @@ static const struct drm_panel_funcs s6e63j0x03_funcs = {
 	.get_modes = s6e63j0x03_get_modes,
 };
 
+static int s6e63j0x03_backlight_register(struct s6e63j0x03 *ctx)
+{
+	DECLARE_BACKLIGHT_INIT_RAW(props, DEFAULT_BRIGHTNESS, MAX_BRIGHTNESS);
+	struct device *dev = ctx->dev;
+	struct backlight_device *bd;
+	int ret = 0;
+
+	bd = devm_backlight_device_register(dev, "panel", dev, ctx,
+					    &s6e63j0x03_bl_ops, &props);
+	if (IS_ERR(bd)) {
+		ret = PTR_ERR(bd);
+		DRM_DEV_ERROR(dev, "error registering backlight device (%d)\n", ret);
+		return ret;
+	}
+
+	ctx->panel.backlight = bd;
+	return ret;
+}
+
 static int s6e63j0x03_probe(struct mipi_dsi_device *dsi)
 {
 	struct device *dev = &dsi->dev;
@@ -469,20 +475,13 @@ static int s6e63j0x03_probe(struct mipi_dsi_device *dsi)
 	drm_panel_init(&ctx->panel, dev, &s6e63j0x03_funcs,
 		       DRM_MODE_CONNECTOR_DSI);
 
-	ctx->bl_dev = backlight_device_register("s6e63j0x03", dev, ctx,
-						&s6e63j0x03_bl_ops, NULL);
-	if (IS_ERR(ctx->bl_dev)) {
-		dev_err(dev, "failed to register backlight device\n");
-		return PTR_ERR(ctx->bl_dev);
-	}
-
-	ctx->bl_dev->props.max_brightness = MAX_BRIGHTNESS;
-	ctx->bl_dev->props.brightness = DEFAULT_BRIGHTNESS;
-	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
+	ret = s6e63j0x03_backlight_register(ctx);
+	if (ret)
+		return ret;
 
 	ret = drm_panel_add(&ctx->panel);
 	if (ret < 0)
-		goto unregister_backlight;
+		return ret;
 
 	ret = mipi_dsi_attach(dsi);
 	if (ret < 0)
@@ -493,9 +492,6 @@ static int s6e63j0x03_probe(struct mipi_dsi_device *dsi)
 remove_panel:
 	drm_panel_remove(&ctx->panel);
 
-unregister_backlight:
-	backlight_device_unregister(ctx->bl_dev);
-
 	return ret;
 }
 
@@ -506,8 +502,6 @@ static int s6e63j0x03_remove(struct mipi_dsi_device *dsi)
 	mipi_dsi_detach(dsi);
 	drm_panel_remove(&ctx->panel);
 
-	backlight_device_unregister(ctx->bl_dev);
-
 	return 0;
 }
 
-- 
2.25.1

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

[PATCH v1 13/22] drm/panel: samsung-s6e3ha2: Backlight update

[Sam Ravnborg]

- Use backlight support from drm_panel.
  This shifts this driver away from manually handling of power state.
- Add helper function for registering backlight, like other samsung
  panel drivers do.
- Use devm_ for backlight register thus benefit from automatic
  unregistering. Drop all explicit unregistering.

In s6e3ha2_disable() a 40 ms delay was dropped. Using drm_panel support
backlight is not disable before display is turned off, so delay after
turning off the display is irrelevant.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
---
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 68 ++++++++-----------
 1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
index 36ebd5a4ac7b..cef781985684 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
@@ -233,7 +233,6 @@ struct s6e3ha2_panel_desc {
 struct s6e3ha2 {
 	struct device *dev;
 	struct drm_panel panel;
-	struct backlight_device *bl_dev;
 
 	struct regulator_bulk_data supplies[2];
 	struct gpio_desc *reset_gpio;
@@ -408,15 +407,10 @@ static int s6e3ha2_gamma_update(struct s6e3ha2 *ctx)
 	return 0;
 }
 
-static int s6e3ha2_get_brightness(struct backlight_device *bl_dev)
-{
-	return bl_dev->props.brightness;
-}
-
 static int s6e3ha2_set_vint(struct s6e3ha2 *ctx)
 {
-	struct backlight_device *bl_dev = ctx->bl_dev;
-	unsigned int brightness = bl_dev->props.brightness;
+	struct backlight_device *bl_dev = ctx->panel.backlight;
+	unsigned int brightness = backlight_get_brightness(bl_dev);
 	unsigned char data[] = { 0xf4, 0x8b,
 			vint_table[brightness * (S6E3HA2_VINT_STATUS_MAX - 1) /
 			S6E3HA2_MAX_BRIGHTNESS] };
@@ -432,7 +426,7 @@ static unsigned int s6e3ha2_get_brightness_index(unsigned int brightness)
 
 static int s6e3ha2_update_gamma(struct s6e3ha2 *ctx, unsigned int brightness)
 {
-	struct backlight_device *bl_dev = ctx->bl_dev;
+	struct backlight_device *bl_dev = ctx->panel.backlight;
 	unsigned int index = s6e3ha2_get_brightness_index(brightness);
 	u8 data[S6E3HA2_GAMMA_CMD_CNT + 1] = { 0xca, };
 	int ret;
@@ -442,7 +436,7 @@ static int s6e3ha2_update_gamma(struct s6e3ha2 *ctx, unsigned int brightness)
 				s6e3ha2_dcs_write(ctx, data, ARRAY_SIZE(data)));
 
 	s6e3ha2_call_write_func(ret, s6e3ha2_gamma_update(ctx));
-	bl_dev->props.brightness = brightness;
+	backlight_set_brightness(bl_dev, brightness);
 
 	return 0;
 }
@@ -450,18 +444,15 @@ static int s6e3ha2_update_gamma(struct s6e3ha2 *ctx, unsigned int brightness)
 static int s6e3ha2_set_brightness(struct backlight_device *bl_dev)
 {
 	struct s6e3ha2 *ctx = bl_get_data(bl_dev);
-	unsigned int brightness = bl_dev->props.brightness;
+	unsigned int brightness = backlight_get_brightness(bl_dev);
 	int ret;
 
 	if (brightness < S6E3HA2_MIN_BRIGHTNESS ||
-		brightness > bl_dev->props.max_brightness) {
+		brightness > backlight_get_max_brightness(bl_dev)) {
 		dev_err(ctx->dev, "Invalid brightness: %u\n", brightness);
 		return -EINVAL;
 	}
 
-	if (bl_dev->props.power > FB_BLANK_NORMAL)
-		return -EPERM;
-
 	s6e3ha2_call_write_func(ret, s6e3ha2_test_key_on_f0(ctx));
 	s6e3ha2_call_write_func(ret, s6e3ha2_update_gamma(ctx, brightness));
 	s6e3ha2_call_write_func(ret, s6e3ha2_aor_control(ctx));
@@ -472,7 +463,6 @@ static int s6e3ha2_set_brightness(struct backlight_device *bl_dev)
 }
 
 static const struct backlight_ops s6e3ha2_bl_ops = {
-	.get_brightness = s6e3ha2_get_brightness,
 	.update_status = s6e3ha2_set_brightness,
 };
 
@@ -508,9 +498,6 @@ static int s6e3ha2_disable(struct drm_panel *panel)
 	s6e3ha2_call_write_func(ret, mipi_dsi_dcs_enter_sleep_mode(dsi));
 	s6e3ha2_call_write_func(ret, mipi_dsi_dcs_set_display_off(dsi));
 
-	msleep(40);
-	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
-
 	return 0;
 }
 
@@ -555,8 +542,6 @@ static int s6e3ha2_prepare(struct drm_panel *panel)
 	if (ret < 0)
 		goto err;
 
-	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
-
 	return 0;
 
 err:
@@ -588,7 +573,7 @@ static int s6e3ha2_enable(struct drm_panel *panel)
 	s6e3ha2_call_write_func(ret, s6e3ha2_te_start_setting(ctx));
 
 	/* brightness setting */
-	s6e3ha2_call_write_func(ret, s6e3ha2_set_brightness(ctx->bl_dev));
+	s6e3ha2_call_write_func(ret, s6e3ha2_set_brightness(panel->backlight));
 	s6e3ha2_call_write_func(ret, s6e3ha2_aor_control(ctx));
 	s6e3ha2_call_write_func(ret, s6e3ha2_caps_elvss_set(ctx));
 	s6e3ha2_call_write_func(ret, s6e3ha2_gamma_update(ctx));
@@ -602,7 +587,6 @@ static int s6e3ha2_enable(struct drm_panel *panel)
 	s6e3ha2_call_write_func(ret, s6e3ha2_test_key_off_f0(ctx));
 
 	s6e3ha2_call_write_func(ret, mipi_dsi_dcs_set_display_on(dsi));
-	ctx->bl_dev->props.power = FB_BLANK_UNBLANK;
 
 	return 0;
 }
@@ -678,6 +662,25 @@ static const struct drm_panel_funcs s6e3ha2_drm_funcs = {
 	.get_modes = s6e3ha2_get_modes,
 };
 
+static int s6e3ha2_backlight_register(struct s6e3ha2 *ctx)
+{
+	DECLARE_BACKLIGHT_INIT_RAW(props, S6E3HA2_DEFAULT_BRIGHTNESS, S6E3HA2_MAX_BRIGHTNESS);
+	struct device *dev = ctx->dev;
+	struct backlight_device *bd;
+	int ret = 0;
+
+	bd = devm_backlight_device_register(dev, "panel", dev, ctx,
+					    &s6e3ha2_bl_ops, &props);
+	if (IS_ERR(bd)) {
+		ret = PTR_ERR(bd);
+		DRM_DEV_ERROR(dev, "error registering backlight device (%d)\n", ret);
+		return ret;
+	}
+
+	ctx->panel.backlight = bd;
+	return ret;
+}
+
 static int s6e3ha2_probe(struct mipi_dsi_device *dsi)
 {
 	struct device *dev = &dsi->dev;
@@ -721,23 +724,16 @@ static int s6e3ha2_probe(struct mipi_dsi_device *dsi)
 		return PTR_ERR(ctx->enable_gpio);
 	}
 
-	ctx->bl_dev = backlight_device_register("s6e3ha2", dev, ctx,
-						&s6e3ha2_bl_ops, NULL);
-	if (IS_ERR(ctx->bl_dev)) {
-		dev_err(dev, "failed to register backlight device\n");
-		return PTR_ERR(ctx->bl_dev);
-	}
-
-	ctx->bl_dev->props.max_brightness = S6E3HA2_MAX_BRIGHTNESS;
-	ctx->bl_dev->props.brightness = S6E3HA2_DEFAULT_BRIGHTNESS;
-	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
+	ret = s6e3ha2_backlight_register(ctx);
+	if (ret)
+		return ret;
 
 	drm_panel_init(&ctx->panel, dev, &s6e3ha2_drm_funcs,
 		       DRM_MODE_CONNECTOR_DSI);
 
 	ret = drm_panel_add(&ctx->panel);
 	if (ret < 0)
-		goto unregister_backlight;
+		return ret;
 
 	ret = mipi_dsi_attach(dsi);
 	if (ret < 0)
@@ -748,9 +744,6 @@ static int s6e3ha2_probe(struct mipi_dsi_device *dsi)
 remove_panel:
 	drm_panel_remove(&ctx->panel);
 
-unregister_backlight:
-	backlight_device_unregister(ctx->bl_dev);
-
 	return ret;
 }
 
@@ -760,7 +753,6 @@ static int s6e3ha2_remove(struct mipi_dsi_device *dsi)
 
 	mipi_dsi_detach(dsi);
 	drm_panel_remove(&ctx->panel);
-	backlight_device_unregister(ctx->bl_dev);
 
 	return 0;
 }
-- 
2.25.1

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

[PATCH v1 14/22] drm/panel: sony-acx424akp: Backlight update

[Sam Ravnborg]

- Use get method to read brightness
- Use drm_panel support for backlight
  - This drops enable/disable operations as they are no longer needed.
    The enable/disable operations had some backlight related comments
    that are no longer valid. The only correct way to enable/disable
    backlight is using the backlight enable/disable helpers.
- Use macro for backlight initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Linus Walleij <linus.walleij@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-sony-acx424akp.c | 49 ++++----------------
 1 file changed, 9 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
index c91e55b2d7a3..ce9ae8f1f5d7 100644
--- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c
+++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
@@ -99,7 +99,7 @@ static int acx424akp_set_brightness(struct backlight_device *bl)
 	struct acx424akp *acx = bl_get_data(bl);
 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
 	int period_ns = 1023;
-	int duty_ns = bl->props.brightness;
+	int duty_ns = backlight_get_brightness(bl);
 	u8 pwm_ratio;
 	u8 pwm_div;
 	u8 par;
@@ -332,8 +332,6 @@ static int acx424akp_prepare(struct drm_panel *panel)
 		}
 	}
 
-	acx->bl->props.power = FB_BLANK_NORMAL;
-
 	return 0;
 
 err_power_off:
@@ -376,34 +374,6 @@ static int acx424akp_unprepare(struct drm_panel *panel)
 	msleep(85);
 
 	acx424akp_power_off(acx);
-	acx->bl->props.power = FB_BLANK_POWERDOWN;
-
-	return 0;
-}
-
-static int acx424akp_enable(struct drm_panel *panel)
-{
-	struct acx424akp *acx = panel_to_acx424akp(panel);
-
-	/*
-	 * The backlight is on as long as the display is on
-	 * so no use to call backlight_enable() here.
-	 */
-	acx->bl->props.power = FB_BLANK_UNBLANK;
-
-	return 0;
-}
-
-static int acx424akp_disable(struct drm_panel *panel)
-{
-	struct acx424akp *acx = panel_to_acx424akp(panel);
-
-	/*
-	 * The backlight is on as long as the display is on
-	 * so no use to call backlight_disable() here.
-	 */
-	acx->bl->props.power = FB_BLANK_NORMAL;
-
 	return 0;
 }
 
@@ -435,18 +405,18 @@ static int acx424akp_get_modes(struct drm_panel *panel,
 }
 
 static const struct drm_panel_funcs acx424akp_drm_funcs = {
-	.disable = acx424akp_disable,
 	.unprepare = acx424akp_unprepare,
 	.prepare = acx424akp_prepare,
-	.enable = acx424akp_enable,
 	.get_modes = acx424akp_get_modes,
 };
 
 static int acx424akp_probe(struct mipi_dsi_device *dsi)
 {
+	struct backlight_device *bd;
 	struct device *dev = &dsi->dev;
 	struct acx424akp *acx;
 	int ret;
+	DECLARE_BACKLIGHT_INIT_RAW(props, 512, 1023);
 
 	acx = devm_kzalloc(dev, sizeof(struct acx424akp), GFP_KERNEL);
 	if (!acx)
@@ -496,15 +466,14 @@ static int acx424akp_probe(struct mipi_dsi_device *dsi)
 	drm_panel_init(&acx->panel, dev, &acx424akp_drm_funcs,
 		       DRM_MODE_CONNECTOR_DSI);
 
-	acx->bl = devm_backlight_device_register(dev, "acx424akp", dev, acx,
-						 &acx424akp_bl_ops, NULL);
-	if (IS_ERR(acx->bl)) {
+	bd = devm_backlight_device_register(dev, "acx424akp", dev, acx,
+					    &acx424akp_bl_ops, &props);
+	if (IS_ERR(bd)) {
 		DRM_DEV_ERROR(dev, "failed to register backlight device\n");
-		return PTR_ERR(acx->bl);
+		return PTR_ERR(bd);
 	}
-	acx->bl->props.max_brightness = 1023;
-	acx->bl->props.brightness = 512;
-	acx->bl->props.power = FB_BLANK_POWERDOWN;
+
+	acx->panel.backlight = bd;
 
 	ret = drm_panel_add(&acx->panel);
 	if (ret < 0)
-- 
2.25.1

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

Re: [PATCH v1 14/22] drm/panel: sony-acx424akp: Backlight update

[Linus Walleij]

On Sun, Aug 2, 2020 at 1:07 PM Sam Ravnborg <sam@ravnborg.org> wrote:

> - Use get method to read brightness
> - Use drm_panel support for backlight
>   - This drops enable/disable operations as they are no longer needed.
>     The enable/disable operations had some backlight related comments
>     that are no longer valid. The only correct way to enable/disable
>     backlight is using the backlight enable/disable helpers.
> - Use macro for backlight initialization
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Linus Walleij <linus.walleij@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 15/22] drm/panel: sony-acx565akm: Backlight update

[Sam Ravnborg]

- Use backlight_get_brightness() helper
- Use backlight_is_blank() helper
- Use macro for initialization
- Drop direct access to backlight properties
- Use the devm_ variant for registering backlight device, and drop
  all explicit unregistering of the backlight device.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-sony-acx565akm.c | 44 +++++++-------------
 1 file changed, 15 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
index 5c4b6f6e5c2d..3fc572d1de13 100644
--- a/drivers/gpu/drm/panel/panel-sony-acx565akm.c
+++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
@@ -298,13 +298,7 @@ static void acx565akm_set_brightness(struct acx565akm_panel *lcd, int level)
 static int acx565akm_bl_update_status_locked(struct backlight_device *dev)
 {
 	struct acx565akm_panel *lcd = dev_get_drvdata(&dev->dev);
-	int level;
-
-	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-	    dev->props.power == FB_BLANK_UNBLANK)
-		level = dev->props.brightness;
-	else
-		level = 0;
+	int level = backlight_get_brightness(dev);
 
 	acx565akm_set_brightness(lcd, level);
 
@@ -330,8 +324,7 @@ static int acx565akm_bl_get_intensity(struct backlight_device *dev)
 
 	mutex_lock(&lcd->mutex);
 
-	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-	    dev->props.power == FB_BLANK_UNBLANK)
+	if (backlight_is_blank(dev))
 		intensity = acx565akm_get_actual_brightness(lcd);
 	else
 		intensity = 0;
@@ -348,39 +341,34 @@ static const struct backlight_ops acx565akm_bl_ops = {
 
 static int acx565akm_backlight_init(struct acx565akm_panel *lcd)
 {
-	struct backlight_properties props = {
-		.fb_blank = FB_BLANK_UNBLANK,
-		.power = FB_BLANK_UNBLANK,
-		.type = BACKLIGHT_RAW,
-	};
 	int ret;
-
-	lcd->backlight = backlight_device_register(lcd->name, &lcd->spi->dev,
-						   lcd, &acx565akm_bl_ops,
-						   &props);
-	if (IS_ERR(lcd->backlight)) {
-		ret = PTR_ERR(lcd->backlight);
-		lcd->backlight = NULL;
+	struct backlight_device *bd;
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, 255);
+
+	bd = devm_backlight_device_register(&lcd->spi->dev, lcd->name,
+					    &lcd->spi->dev, lcd,
+					    &acx565akm_bl_ops, &props);
+	if (IS_ERR(bd)) {
+		ret = PTR_ERR(bd);
 		return ret;
 	}
 
+	lcd->backlight = bd;
 	if (lcd->has_cabc) {
-		ret = sysfs_create_group(&lcd->backlight->dev.kobj,
+		ret = sysfs_create_group(&bd->dev.kobj,
 					 &acx565akm_cabc_attr_group);
 		if (ret < 0) {
 			dev_err(&lcd->spi->dev,
 				"%s failed to create sysfs files\n", __func__);
-			backlight_device_unregister(lcd->backlight);
 			return ret;
 		}
 
 		lcd->cabc_mode = acx565akm_get_hw_cabc_mode(lcd);
 	}
 
-	lcd->backlight->props.max_brightness = 255;
-	lcd->backlight->props.brightness = acx565akm_get_actual_brightness(lcd);
-
-	acx565akm_bl_update_status_locked(lcd->backlight);
+	backlight_set_brightness(bd, acx565akm_get_actual_brightness(lcd));
+	backlight_set_power_on(bd);
+	backlight_update_status(bd);
 
 	return 0;
 }
@@ -390,8 +378,6 @@ static void acx565akm_backlight_cleanup(struct acx565akm_panel *lcd)
 	if (lcd->has_cabc)
 		sysfs_remove_group(&lcd->backlight->dev.kobj,
 				   &acx565akm_cabc_attr_group);
-
-	backlight_device_unregister(lcd->backlight);
 }
 
 /* -----------------------------------------------------------------------------
-- 
2.25.1

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

Re: [PATCH v1 15/22] drm/panel: sony-acx565akm: Backlight update

[daniel]

On Sun, Aug 02, 2020 at 01:06:29PM +0200, Sam Ravnborg wrote:
> - Use backlight_get_brightness() helper
> - Use backlight_is_blank() helper
> - Use macro for initialization
> - Drop direct access to backlight properties
> - Use the devm_ variant for registering backlight device, and drop
>   all explicit unregistering of the backlight device.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c | 44 +++++++-------------
>  1 file changed, 15 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
> index 5c4b6f6e5c2d..3fc572d1de13 100644
> --- a/drivers/gpu/drm/panel/panel-sony-acx565akm.c
> +++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
> @@ -298,13 +298,7 @@ static void acx565akm_set_brightness(struct acx565akm_panel *lcd, int level)
>  static int acx565akm_bl_update_status_locked(struct backlight_device *dev)
>  {
>  	struct acx565akm_panel *lcd = dev_get_drvdata(&dev->dev);
> -	int level;
> -
> -	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> -	    dev->props.power == FB_BLANK_UNBLANK)
> -		level = dev->props.brightness;
> -	else
> -		level = 0;
> +	int level = backlight_get_brightness(dev);
>  
>  	acx565akm_set_brightness(lcd, level);
>  
> @@ -330,8 +324,7 @@ static int acx565akm_bl_get_intensity(struct backlight_device *dev)
>  
>  	mutex_lock(&lcd->mutex);
>  
> -	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> -	    dev->props.power == FB_BLANK_UNBLANK)
> +	if (backlight_is_blank(dev))
>  		intensity = acx565akm_get_actual_brightness(lcd);
>  	else
>  		intensity = 0;
> @@ -348,39 +341,34 @@ static const struct backlight_ops acx565akm_bl_ops = {
>  
>  static int acx565akm_backlight_init(struct acx565akm_panel *lcd)
>  {
> -	struct backlight_properties props = {
> -		.fb_blank = FB_BLANK_UNBLANK,
> -		.power = FB_BLANK_UNBLANK,
> -		.type = BACKLIGHT_RAW,
> -	};
>  	int ret;
> -
> -	lcd->backlight = backlight_device_register(lcd->name, &lcd->spi->dev,
> -						   lcd, &acx565akm_bl_ops,
> -						   &props);
> -	if (IS_ERR(lcd->backlight)) {
> -		ret = PTR_ERR(lcd->backlight);
> -		lcd->backlight = NULL;
> +	struct backlight_device *bd;
> +	DECLARE_BACKLIGHT_INIT_RAW(props, 0, 255);
> +
> +	bd = devm_backlight_device_register(&lcd->spi->dev, lcd->name,
> +					    &lcd->spi->dev, lcd,
> +					    &acx565akm_bl_ops, &props);

It's been in a bunch of earlier patches already, but devm_bl freaks me out
a bit with our long-term goal of storing a backlight pointer into
drm_connector->backlight.

Since drm_connector and the underlying backlight device have different
lifetimes that would mean we need to refcount somewhere, or protect
drm_connector->backlight with some lock. The lock might not work because
the drm connector property paths come from the other direction than the
backlight driver unload ... so probably needs to be refcounting.
-Daniel

> +	if (IS_ERR(bd)) {
> +		ret = PTR_ERR(bd);
>  		return ret;
>  	}
>  
> +	lcd->backlight = bd;
>  	if (lcd->has_cabc) {
> -		ret = sysfs_create_group(&lcd->backlight->dev.kobj,
> +		ret = sysfs_create_group(&bd->dev.kobj,
>  					 &acx565akm_cabc_attr_group);
>  		if (ret < 0) {
>  			dev_err(&lcd->spi->dev,
>  				"%s failed to create sysfs files\n", __func__);
> -			backlight_device_unregister(lcd->backlight);
>  			return ret;
>  		}
>  
>  		lcd->cabc_mode = acx565akm_get_hw_cabc_mode(lcd);
>  	}
>  
> -	lcd->backlight->props.max_brightness = 255;
> -	lcd->backlight->props.brightness = acx565akm_get_actual_brightness(lcd);
> -
> -	acx565akm_bl_update_status_locked(lcd->backlight);
> +	backlight_set_brightness(bd, acx565akm_get_actual_brightness(lcd));
> +	backlight_set_power_on(bd);
> +	backlight_update_status(bd);
>  
>  	return 0;
>  }
> @@ -390,8 +378,6 @@ static void acx565akm_backlight_cleanup(struct acx565akm_panel *lcd)
>  	if (lcd->has_cabc)
>  		sysfs_remove_group(&lcd->backlight->dev.kobj,
>  				   &acx565akm_cabc_attr_group);
> -
> -	backlight_device_unregister(lcd->backlight);
>  }
>  
>  /* -----------------------------------------------------------------------------
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 16/22] drm/bridge: parade-ps8622: Backlight update

[Sam Ravnborg]

- Use blacklight_get_brightness() helper
- Use devm_ variant to register backlight device and drop explicit
  unregister
- Use macro for initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/parade-ps8622.c | 43 +++++++++++++-------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c b/drivers/gpu/drm/bridge/parade-ps8622.c
index d789ea2a7fb9..9304484e7f71 100644
--- a/drivers/gpu/drm/bridge/parade-ps8622.c
+++ b/drivers/gpu/drm/bridge/parade-ps8622.c
@@ -284,8 +284,7 @@ static int ps8622_send_config(struct ps8622_bridge *ps8622)
 			goto error;
 
 		/* FFh for 100% brightness, 0h for 0% brightness */
-		err = ps8622_set(cl, 0x01, 0xa7,
-				ps8622->bl->props.brightness);
+		err = ps8622_set(cl, 0x01, 0xa7, backlight_get_brightness(ps8622->bl));
 		if (err)
 			goto error;
 	} else {
@@ -331,18 +330,11 @@ static int ps8622_send_config(struct ps8622_bridge *ps8622)
 static int ps8622_backlight_update(struct backlight_device *bl)
 {
 	struct ps8622_bridge *ps8622 = dev_get_drvdata(&bl->dev);
-	int ret, brightness = bl->props.brightness;
-
-	if (bl->props.power != FB_BLANK_UNBLANK ||
-	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
-		brightness = 0;
 
 	if (!ps8622->enabled)
 		return -EINVAL;
 
-	ret = ps8622_set(ps8622->client, 0x01, 0xa7, brightness);
-
-	return ret;
+	return ps8622_set(ps8622->client, 0x01, 0xa7, blacklight_get_brightness(bl));
 }
 
 static const struct backlight_ops ps8622_backlight_ops = {
@@ -521,7 +513,23 @@ static const struct drm_bridge_funcs ps8622_bridge_funcs = {
 	.attach = ps8622_attach,
 };
 
-static const struct of_device_id ps8622_devices[] = {
+static int ps8622_register_blacklight(struct ps8622_bridge *ps8622)
+{
+	DECLARE_BACKLIGHT_INIT_RAW(props, PS8622_MAX_BRIGHTNESS, PS8622_MAX_BRIGHTNESS);
+	backlight_device *bl;
+
+	bl = devm_backlight_device_register(dev, dev_name(dev), dev,
+					    ps8622, &ps8622_backlight_ops, &props);
+	if (IS_ERR(bl)) {
+		DRM_ERROR("failed to register backlight\n");
+		return PTR_ERR(bl);
+	}
+
+	ps8622->bl = bl;
+	return 0;
+}
+
+const struct of_device_id ps8622_devices[] = {
 	{.compatible = "parade,ps8622",},
 	{.compatible = "parade,ps8625",},
 	{}
@@ -581,17 +589,9 @@ static int ps8622_probe(struct i2c_client *client,
 	}
 
 	if (!of_find_property(dev->of_node, "use-external-pwm", NULL)) {
-		ps8622->bl = backlight_device_register("ps8622-backlight",
-				dev, ps8622, &ps8622_backlight_ops,
-				NULL);
-		if (IS_ERR(ps8622->bl)) {
-			DRM_ERROR("failed to register backlight\n");
-			ret = PTR_ERR(ps8622->bl);
-			ps8622->bl = NULL;
+		ret = ps8622_register_blacklight(ps8622);
+		if (ret)
 			return ret;
-		}
-		ps8622->bl->props.max_brightness = PS8622_MAX_BRIGHTNESS;
-		ps8622->bl->props.brightness = PS8622_MAX_BRIGHTNESS;
 	}
 
 	ps8622->bridge.funcs = &ps8622_bridge_funcs;
@@ -607,7 +607,6 @@ static int ps8622_remove(struct i2c_client *client)
 {
 	struct ps8622_bridge *ps8622 = i2c_get_clientdata(client);
 
-	backlight_device_unregister(ps8622->bl);
 	drm_bridge_remove(&ps8622->bridge);
 
 	return 0;
-- 
2.25.1

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

Re: [PATCH v1 16/22] drm/bridge: parade-ps8622: Backlight update

[kernel test robot]

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

Hi Sam,

I love your patch! Yet something to improve:

[auto build test ERROR on backlight/for-backlight-next]
[also build test ERROR on next-20200731]
[cannot apply to drm-intel/for-linux-next drm-tip/drm-tip linus/master drm/drm-next v5.8-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sam-Ravnborg/backlight-add-init-macros-and-accessors/20200802-190940
base:    for-backlight-next
config: x86_64-randconfig-a011-20200802 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 25af353b0e74907d5d50c8616b885bd1f73a68b3)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/parade-ps8622.c:337:48: error: implicit declaration of function 'blacklight_get_brightness' [-Werror,-Wimplicit-function-declaration]
           return ps8622_set(ps8622->client, 0x01, 0xa7, blacklight_get_brightness(bl));
                                                         ^
   drivers/gpu/drm/bridge/parade-ps8622.c:337:48: note: did you mean 'backlight_get_brightness'?
   include/linux/backlight.h:469:19: note: 'backlight_get_brightness' declared here
   static inline int backlight_get_brightness(const struct backlight_device *bd)
                     ^
>> drivers/gpu/drm/bridge/parade-ps8622.c:519:2: error: must use 'struct' tag to refer to type 'backlight_device'
           backlight_device *bl;
           ^
           struct 
>> drivers/gpu/drm/bridge/parade-ps8622.c:521:52: error: use of undeclared identifier 'dev'
           bl = devm_backlight_device_register(dev, dev_name(dev), dev,
                                                             ^
   drivers/gpu/drm/bridge/parade-ps8622.c:521:38: error: use of undeclared identifier 'dev'
           bl = devm_backlight_device_register(dev, dev_name(dev), dev,
                                               ^
   drivers/gpu/drm/bridge/parade-ps8622.c:521:58: error: use of undeclared identifier 'dev'
           bl = devm_backlight_device_register(dev, dev_name(dev), dev,
                                                                   ^
   5 errors generated.

vim +/blacklight_get_brightness +337 drivers/gpu/drm/bridge/parade-ps8622.c

   329	
   330	static int ps8622_backlight_update(struct backlight_device *bl)
   331	{
   332		struct ps8622_bridge *ps8622 = dev_get_drvdata(&bl->dev);
   333	
   334		if (!ps8622->enabled)
   335			return -EINVAL;
   336	
 > 337		return ps8622_set(ps8622->client, 0x01, 0xa7, blacklight_get_brightness(bl));
   338	}
   339	
   340	static const struct backlight_ops ps8622_backlight_ops = {
   341		.update_status	= ps8622_backlight_update,
   342	};
   343	
   344	static void ps8622_pre_enable(struct drm_bridge *bridge)
   345	{
   346		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   347		int ret;
   348	
   349		if (ps8622->enabled)
   350			return;
   351	
   352		gpiod_set_value(ps8622->gpio_rst, 0);
   353	
   354		if (ps8622->v12) {
   355			ret = regulator_enable(ps8622->v12);
   356			if (ret)
   357				DRM_ERROR("fails to enable ps8622->v12");
   358		}
   359	
   360		if (drm_panel_prepare(ps8622->panel)) {
   361			DRM_ERROR("failed to prepare panel\n");
   362			return;
   363		}
   364	
   365		gpiod_set_value(ps8622->gpio_slp, 1);
   366	
   367		/*
   368		 * T1 is the range of time that it takes for the power to rise after we
   369		 * enable the lcd/ps8622 fet. T2 is the range of time in which the
   370		 * data sheet specifies we should deassert the reset pin.
   371		 *
   372		 * If it takes T1.max for the power to rise, we need to wait atleast
   373		 * T2.min before deasserting the reset pin. If it takes T1.min for the
   374		 * power to rise, we need to wait at most T2.max before deasserting the
   375		 * reset pin.
   376		 */
   377		usleep_range(PS8622_RST_HIGH_T2_MIN_US + PS8622_POWER_RISE_T1_MAX_US,
   378			     PS8622_RST_HIGH_T2_MAX_US + PS8622_POWER_RISE_T1_MIN_US);
   379	
   380		gpiod_set_value(ps8622->gpio_rst, 1);
   381	
   382		/* wait 20ms after RST high */
   383		usleep_range(20000, 30000);
   384	
   385		ret = ps8622_send_config(ps8622);
   386		if (ret) {
   387			DRM_ERROR("Failed to send config to bridge (%d)\n", ret);
   388			return;
   389		}
   390	
   391		ps8622->enabled = true;
   392	}
   393	
   394	static void ps8622_enable(struct drm_bridge *bridge)
   395	{
   396		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   397	
   398		if (drm_panel_enable(ps8622->panel)) {
   399			DRM_ERROR("failed to enable panel\n");
   400			return;
   401		}
   402	}
   403	
   404	static void ps8622_disable(struct drm_bridge *bridge)
   405	{
   406		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   407	
   408		if (drm_panel_disable(ps8622->panel)) {
   409			DRM_ERROR("failed to disable panel\n");
   410			return;
   411		}
   412		msleep(PS8622_PWMO_END_T12_MS);
   413	}
   414	
   415	static void ps8622_post_disable(struct drm_bridge *bridge)
   416	{
   417		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   418	
   419		if (!ps8622->enabled)
   420			return;
   421	
   422		ps8622->enabled = false;
   423	
   424		/*
   425		 * This doesn't matter if the regulators are turned off, but something
   426		 * else might keep them on. In that case, we want to assert the slp gpio
   427		 * to lower power.
   428		 */
   429		gpiod_set_value(ps8622->gpio_slp, 0);
   430	
   431		if (drm_panel_unprepare(ps8622->panel)) {
   432			DRM_ERROR("failed to unprepare panel\n");
   433			return;
   434		}
   435	
   436		if (ps8622->v12)
   437			regulator_disable(ps8622->v12);
   438	
   439		/*
   440		 * Sleep for at least the amount of time that it takes the power rail to
   441		 * fall to prevent asserting the rst gpio from doing anything.
   442		 */
   443		usleep_range(PS8622_POWER_FALL_T16_MAX_US,
   444			     2 * PS8622_POWER_FALL_T16_MAX_US);
   445		gpiod_set_value(ps8622->gpio_rst, 0);
   446	
   447		msleep(PS8622_POWER_OFF_T17_MS);
   448	}
   449	
   450	static int ps8622_get_modes(struct drm_connector *connector)
   451	{
   452		struct ps8622_bridge *ps8622;
   453	
   454		ps8622 = connector_to_ps8622(connector);
   455	
   456		return drm_panel_get_modes(ps8622->panel, connector);
   457	}
   458	
   459	static const struct drm_connector_helper_funcs ps8622_connector_helper_funcs = {
   460		.get_modes = ps8622_get_modes,
   461	};
   462	
   463	static const struct drm_connector_funcs ps8622_connector_funcs = {
   464		.fill_modes = drm_helper_probe_single_connector_modes,
   465		.destroy = drm_connector_cleanup,
   466		.reset = drm_atomic_helper_connector_reset,
   467		.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
   468		.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
   469	};
   470	
   471	static int ps8622_attach(struct drm_bridge *bridge,
   472				 enum drm_bridge_attach_flags flags)
   473	{
   474		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   475		int ret;
   476	
   477		if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
   478			DRM_ERROR("Fix bridge driver to make connector optional!");
   479			return -EINVAL;
   480		}
   481	
   482		if (!bridge->encoder) {
   483			DRM_ERROR("Parent encoder object not found");
   484			return -ENODEV;
   485		}
   486	
   487		ps8622->connector.polled = DRM_CONNECTOR_POLL_HPD;
   488		ret = drm_connector_init(bridge->dev, &ps8622->connector,
   489				&ps8622_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
   490		if (ret) {
   491			DRM_ERROR("Failed to initialize connector with drm\n");
   492			return ret;
   493		}
   494		drm_connector_helper_add(&ps8622->connector,
   495						&ps8622_connector_helper_funcs);
   496		drm_connector_register(&ps8622->connector);
   497		drm_connector_attach_encoder(&ps8622->connector,
   498								bridge->encoder);
   499	
   500		if (ps8622->panel)
   501			drm_panel_attach(ps8622->panel, &ps8622->connector);
   502	
   503		drm_helper_hpd_irq_event(ps8622->connector.dev);
   504	
   505		return ret;
   506	}
   507	
   508	static const struct drm_bridge_funcs ps8622_bridge_funcs = {
   509		.pre_enable = ps8622_pre_enable,
   510		.enable = ps8622_enable,
   511		.disable = ps8622_disable,
   512		.post_disable = ps8622_post_disable,
   513		.attach = ps8622_attach,
   514	};
   515	
   516	static int ps8622_register_blacklight(struct ps8622_bridge *ps8622)
   517	{
   518		DECLARE_BACKLIGHT_INIT_RAW(props, PS8622_MAX_BRIGHTNESS, PS8622_MAX_BRIGHTNESS);
 > 519		backlight_device *bl;
   520	
 > 521		bl = devm_backlight_device_register(dev, dev_name(dev), dev,
   522						    ps8622, &ps8622_backlight_ops, &props);
   523		if (IS_ERR(bl)) {
   524			DRM_ERROR("failed to register backlight\n");
   525			return PTR_ERR(bl);
   526		}
   527	
   528		ps8622->bl = bl;
   529		return 0;
   530	}
   531	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38729 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v1 16/22] drm/bridge: parade-ps8622: Backlight update

[kernel test robot]

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

Hi Sam,

I love your patch! Yet something to improve:

[auto build test ERROR on backlight/for-backlight-next]
[also build test ERROR on next-20200731]
[cannot apply to drm-intel/for-linux-next drm-tip/drm-tip linus/master drm/drm-next v5.8-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sam-Ravnborg/backlight-add-init-macros-and-accessors/20200802-190940
base:    for-backlight-next
config: x86_64-randconfig-a011-20200802 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 25af353b0e74907d5d50c8616b885bd1f73a68b3)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/parade-ps8622.c:337:48: error: implicit declaration of function 'blacklight_get_brightness' [-Werror,-Wimplicit-function-declaration]
           return ps8622_set(ps8622->client, 0x01, 0xa7, blacklight_get_brightness(bl));
                                                         ^
   drivers/gpu/drm/bridge/parade-ps8622.c:337:48: note: did you mean 'backlight_get_brightness'?
   include/linux/backlight.h:469:19: note: 'backlight_get_brightness' declared here
   static inline int backlight_get_brightness(const struct backlight_device *bd)
                     ^
>> drivers/gpu/drm/bridge/parade-ps8622.c:519:2: error: must use 'struct' tag to refer to type 'backlight_device'
           backlight_device *bl;
           ^
           struct 
>> drivers/gpu/drm/bridge/parade-ps8622.c:521:52: error: use of undeclared identifier 'dev'
           bl = devm_backlight_device_register(dev, dev_name(dev), dev,
                                                             ^
   drivers/gpu/drm/bridge/parade-ps8622.c:521:38: error: use of undeclared identifier 'dev'
           bl = devm_backlight_device_register(dev, dev_name(dev), dev,
                                               ^
   drivers/gpu/drm/bridge/parade-ps8622.c:521:58: error: use of undeclared identifier 'dev'
           bl = devm_backlight_device_register(dev, dev_name(dev), dev,
                                                                   ^
   5 errors generated.

vim +/blacklight_get_brightness +337 drivers/gpu/drm/bridge/parade-ps8622.c

   329	
   330	static int ps8622_backlight_update(struct backlight_device *bl)
   331	{
   332		struct ps8622_bridge *ps8622 = dev_get_drvdata(&bl->dev);
   333	
   334		if (!ps8622->enabled)
   335			return -EINVAL;
   336	
 > 337		return ps8622_set(ps8622->client, 0x01, 0xa7, blacklight_get_brightness(bl));
   338	}
   339	
   340	static const struct backlight_ops ps8622_backlight_ops = {
   341		.update_status	= ps8622_backlight_update,
   342	};
   343	
   344	static void ps8622_pre_enable(struct drm_bridge *bridge)
   345	{
   346		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   347		int ret;
   348	
   349		if (ps8622->enabled)
   350			return;
   351	
   352		gpiod_set_value(ps8622->gpio_rst, 0);
   353	
   354		if (ps8622->v12) {
   355			ret = regulator_enable(ps8622->v12);
   356			if (ret)
   357				DRM_ERROR("fails to enable ps8622->v12");
   358		}
   359	
   360		if (drm_panel_prepare(ps8622->panel)) {
   361			DRM_ERROR("failed to prepare panel\n");
   362			return;
   363		}
   364	
   365		gpiod_set_value(ps8622->gpio_slp, 1);
   366	
   367		/*
   368		 * T1 is the range of time that it takes for the power to rise after we
   369		 * enable the lcd/ps8622 fet. T2 is the range of time in which the
   370		 * data sheet specifies we should deassert the reset pin.
   371		 *
   372		 * If it takes T1.max for the power to rise, we need to wait atleast
   373		 * T2.min before deasserting the reset pin. If it takes T1.min for the
   374		 * power to rise, we need to wait at most T2.max before deasserting the
   375		 * reset pin.
   376		 */
   377		usleep_range(PS8622_RST_HIGH_T2_MIN_US + PS8622_POWER_RISE_T1_MAX_US,
   378			     PS8622_RST_HIGH_T2_MAX_US + PS8622_POWER_RISE_T1_MIN_US);
   379	
   380		gpiod_set_value(ps8622->gpio_rst, 1);
   381	
   382		/* wait 20ms after RST high */
   383		usleep_range(20000, 30000);
   384	
   385		ret = ps8622_send_config(ps8622);
   386		if (ret) {
   387			DRM_ERROR("Failed to send config to bridge (%d)\n", ret);
   388			return;
   389		}
   390	
   391		ps8622->enabled = true;
   392	}
   393	
   394	static void ps8622_enable(struct drm_bridge *bridge)
   395	{
   396		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   397	
   398		if (drm_panel_enable(ps8622->panel)) {
   399			DRM_ERROR("failed to enable panel\n");
   400			return;
   401		}
   402	}
   403	
   404	static void ps8622_disable(struct drm_bridge *bridge)
   405	{
   406		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   407	
   408		if (drm_panel_disable(ps8622->panel)) {
   409			DRM_ERROR("failed to disable panel\n");
   410			return;
   411		}
   412		msleep(PS8622_PWMO_END_T12_MS);
   413	}
   414	
   415	static void ps8622_post_disable(struct drm_bridge *bridge)
   416	{
   417		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   418	
   419		if (!ps8622->enabled)
   420			return;
   421	
   422		ps8622->enabled = false;
   423	
   424		/*
   425		 * This doesn't matter if the regulators are turned off, but something
   426		 * else might keep them on. In that case, we want to assert the slp gpio
   427		 * to lower power.
   428		 */
   429		gpiod_set_value(ps8622->gpio_slp, 0);
   430	
   431		if (drm_panel_unprepare(ps8622->panel)) {
   432			DRM_ERROR("failed to unprepare panel\n");
   433			return;
   434		}
   435	
   436		if (ps8622->v12)
   437			regulator_disable(ps8622->v12);
   438	
   439		/*
   440		 * Sleep for at least the amount of time that it takes the power rail to
   441		 * fall to prevent asserting the rst gpio from doing anything.
   442		 */
   443		usleep_range(PS8622_POWER_FALL_T16_MAX_US,
   444			     2 * PS8622_POWER_FALL_T16_MAX_US);
   445		gpiod_set_value(ps8622->gpio_rst, 0);
   446	
   447		msleep(PS8622_POWER_OFF_T17_MS);
   448	}
   449	
   450	static int ps8622_get_modes(struct drm_connector *connector)
   451	{
   452		struct ps8622_bridge *ps8622;
   453	
   454		ps8622 = connector_to_ps8622(connector);
   455	
   456		return drm_panel_get_modes(ps8622->panel, connector);
   457	}
   458	
   459	static const struct drm_connector_helper_funcs ps8622_connector_helper_funcs = {
   460		.get_modes = ps8622_get_modes,
   461	};
   462	
   463	static const struct drm_connector_funcs ps8622_connector_funcs = {
   464		.fill_modes = drm_helper_probe_single_connector_modes,
   465		.destroy = drm_connector_cleanup,
   466		.reset = drm_atomic_helper_connector_reset,
   467		.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
   468		.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
   469	};
   470	
   471	static int ps8622_attach(struct drm_bridge *bridge,
   472				 enum drm_bridge_attach_flags flags)
   473	{
   474		struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
   475		int ret;
   476	
   477		if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
   478			DRM_ERROR("Fix bridge driver to make connector optional!");
   479			return -EINVAL;
   480		}
   481	
   482		if (!bridge->encoder) {
   483			DRM_ERROR("Parent encoder object not found");
   484			return -ENODEV;
   485		}
   486	
   487		ps8622->connector.polled = DRM_CONNECTOR_POLL_HPD;
   488		ret = drm_connector_init(bridge->dev, &ps8622->connector,
   489				&ps8622_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
   490		if (ret) {
   491			DRM_ERROR("Failed to initialize connector with drm\n");
   492			return ret;
   493		}
   494		drm_connector_helper_add(&ps8622->connector,
   495						&ps8622_connector_helper_funcs);
   496		drm_connector_register(&ps8622->connector);
   497		drm_connector_attach_encoder(&ps8622->connector,
   498								bridge->encoder);
   499	
   500		if (ps8622->panel)
   501			drm_panel_attach(ps8622->panel, &ps8622->connector);
   502	
   503		drm_helper_hpd_irq_event(ps8622->connector.dev);
   504	
   505		return ret;
   506	}
   507	
   508	static const struct drm_bridge_funcs ps8622_bridge_funcs = {
   509		.pre_enable = ps8622_pre_enable,
   510		.enable = ps8622_enable,
   511		.disable = ps8622_disable,
   512		.post_disable = ps8622_post_disable,
   513		.attach = ps8622_attach,
   514	};
   515	
   516	static int ps8622_register_blacklight(struct ps8622_bridge *ps8622)
   517	{
   518		DECLARE_BACKLIGHT_INIT_RAW(props, PS8622_MAX_BRIGHTNESS, PS8622_MAX_BRIGHTNESS);
 > 519		backlight_device *bl;
   520	
 > 521		bl = devm_backlight_device_register(dev, dev_name(dev), dev,
   522						    ps8622, &ps8622_backlight_ops, &props);
   523		if (IS_ERR(bl)) {
   524			DRM_ERROR("failed to register backlight\n");
   525			return PTR_ERR(bl);
   526		}
   527	
   528		ps8622->bl = bl;
   529		return 0;
   530	}
   531	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38729 bytes --]

Re: [PATCH v1 16/22] drm/bridge: parade-ps8622: Backlight update

[Sam Ravnborg]

On Sun, Aug 02, 2020 at 01:06:30PM +0200, Sam Ravnborg wrote:
> - Use blacklight_get_brightness() helper
> - Use devm_ variant to register backlight device and drop explicit
>   unregister
> - Use macro for initialization
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>

Build errors fixed, will be included in v2.

	Sam

> ---
>  drivers/gpu/drm/bridge/parade-ps8622.c | 43 +++++++++++++-------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c b/drivers/gpu/drm/bridge/parade-ps8622.c
> index d789ea2a7fb9..9304484e7f71 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8622.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8622.c
> @@ -284,8 +284,7 @@ static int ps8622_send_config(struct ps8622_bridge *ps8622)
>  			goto error;
>  
>  		/* FFh for 100% brightness, 0h for 0% brightness */
> -		err = ps8622_set(cl, 0x01, 0xa7,
> -				ps8622->bl->props.brightness);
> +		err = ps8622_set(cl, 0x01, 0xa7, backlight_get_brightness(ps8622->bl));
>  		if (err)
>  			goto error;
>  	} else {
> @@ -331,18 +330,11 @@ static int ps8622_send_config(struct ps8622_bridge *ps8622)
>  static int ps8622_backlight_update(struct backlight_device *bl)
>  {
>  	struct ps8622_bridge *ps8622 = dev_get_drvdata(&bl->dev);
> -	int ret, brightness = bl->props.brightness;
> -
> -	if (bl->props.power != FB_BLANK_UNBLANK ||
> -	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> -		brightness = 0;
>  
>  	if (!ps8622->enabled)
>  		return -EINVAL;
>  
> -	ret = ps8622_set(ps8622->client, 0x01, 0xa7, brightness);
> -
> -	return ret;
> +	return ps8622_set(ps8622->client, 0x01, 0xa7, blacklight_get_brightness(bl));
>  }
>  
>  static const struct backlight_ops ps8622_backlight_ops = {
> @@ -521,7 +513,23 @@ static const struct drm_bridge_funcs ps8622_bridge_funcs = {
>  	.attach = ps8622_attach,
>  };
>  
> -static const struct of_device_id ps8622_devices[] = {
> +static int ps8622_register_blacklight(struct ps8622_bridge *ps8622)
> +{
> +	DECLARE_BACKLIGHT_INIT_RAW(props, PS8622_MAX_BRIGHTNESS, PS8622_MAX_BRIGHTNESS);
> +	backlight_device *bl;
> +
> +	bl = devm_backlight_device_register(dev, dev_name(dev), dev,
> +					    ps8622, &ps8622_backlight_ops, &props);
> +	if (IS_ERR(bl)) {
> +		DRM_ERROR("failed to register backlight\n");
> +		return PTR_ERR(bl);
> +	}
> +
> +	ps8622->bl = bl;
> +	return 0;
> +}
> +
> +const struct of_device_id ps8622_devices[] = {
>  	{.compatible = "parade,ps8622",},
>  	{.compatible = "parade,ps8625",},
>  	{}
> @@ -581,17 +589,9 @@ static int ps8622_probe(struct i2c_client *client,
>  	}
>  
>  	if (!of_find_property(dev->of_node, "use-external-pwm", NULL)) {
> -		ps8622->bl = backlight_device_register("ps8622-backlight",
> -				dev, ps8622, &ps8622_backlight_ops,
> -				NULL);
> -		if (IS_ERR(ps8622->bl)) {
> -			DRM_ERROR("failed to register backlight\n");
> -			ret = PTR_ERR(ps8622->bl);
> -			ps8622->bl = NULL;
> +		ret = ps8622_register_blacklight(ps8622);
> +		if (ret)
>  			return ret;
> -		}
> -		ps8622->bl->props.max_brightness = PS8622_MAX_BRIGHTNESS;
> -		ps8622->bl->props.brightness = PS8622_MAX_BRIGHTNESS;
>  	}
>  
>  	ps8622->bridge.funcs = &ps8622_bridge_funcs;
> @@ -607,7 +607,6 @@ static int ps8622_remove(struct i2c_client *client)
>  {
>  	struct ps8622_bridge *ps8622 = i2c_get_clientdata(client);
>  
> -	backlight_device_unregister(ps8622->bl);
>  	drm_bridge_remove(&ps8622->bridge);
>  
>  	return 0;
> -- 
> 2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 17/22] drm/tilcdc: Backlight update

[Sam Ravnborg]

Avoid using direct access to backlight_properties by introducing
set methods for power.

Dropped extra check as both set methods and backlight_update_status()
both accepts a NULL backlight device.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 12823d60c4e8..54824999720b 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -47,11 +47,12 @@ static void panel_encoder_dpms(struct drm_encoder *encoder, int mode)
 	struct backlight_device *backlight = panel_encoder->mod->backlight;
 	struct gpio_desc *gpio = panel_encoder->mod->enable_gpio;
 
-	if (backlight) {
-		backlight->props.power = mode == DRM_MODE_DPMS_ON ?
-					 FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
-		backlight_update_status(backlight);
-	}
+	if (pmode == DRM_MODE_DPMS_O)
+		backlight_set_power_on(backlight);
+	else
+		backlight_set_power_off(backlight);
+
+	backlight_update_status(backlight);
 
 	if (gpio)
 		gpiod_set_value_cansleep(gpio,
-- 
2.25.1

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

Re: [PATCH v1 17/22] drm/tilcdc: Backlight update

[kernel test robot]

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

Hi Sam,

I love your patch! Yet something to improve:

[auto build test ERROR on backlight/for-backlight-next]
[also build test ERROR on next-20200731]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master drm/drm-next v5.8-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sam-Ravnborg/backlight-add-init-macros-and-accessors/20200802-190940
base:    for-backlight-next
config: arm-randconfig-r016-20200802 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/tilcdc/tilcdc_panel.c: In function 'panel_encoder_dpms':
>> drivers/gpu/drm/tilcdc/tilcdc_panel.c:50:6: error: 'pmode' undeclared (first use in this function); did you mean 'mode'?
      50 |  if (pmode == DRM_MODE_DPMS_O)
         |      ^~~~~
         |      mode
   drivers/gpu/drm/tilcdc/tilcdc_panel.c:50:6: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/tilcdc/tilcdc_panel.c:50:15: error: 'DRM_MODE_DPMS_O' undeclared (first use in this function); did you mean 'DRM_MODE_DPMS_ON'?
      50 |  if (pmode == DRM_MODE_DPMS_O)
         |               ^~~~~~~~~~~~~~~
         |               DRM_MODE_DPMS_ON

vim +50 drivers/gpu/drm/tilcdc/tilcdc_panel.c

    43	
    44	static void panel_encoder_dpms(struct drm_encoder *encoder, int mode)
    45	{
    46		struct panel_encoder *panel_encoder = to_panel_encoder(encoder);
    47		struct backlight_device *backlight = panel_encoder->mod->backlight;
    48		struct gpio_desc *gpio = panel_encoder->mod->enable_gpio;
    49	
  > 50		if (pmode == DRM_MODE_DPMS_O)
    51			backlight_set_power_on(backlight);
    52		else
    53			backlight_set_power_off(backlight);
    54	
    55		backlight_update_status(backlight);
    56	
    57		if (gpio)
    58			gpiod_set_value_cansleep(gpio,
    59						 mode == DRM_MODE_DPMS_ON ? 1 : 0);
    60	}
    61	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30521 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v1 17/22] drm/tilcdc: Backlight update

[kernel test robot]

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

Hi Sam,

I love your patch! Yet something to improve:

[auto build test ERROR on backlight/for-backlight-next]
[also build test ERROR on next-20200731]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master drm/drm-next v5.8-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sam-Ravnborg/backlight-add-init-macros-and-accessors/20200802-190940
base:    for-backlight-next
config: arm-randconfig-r016-20200802 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/tilcdc/tilcdc_panel.c: In function 'panel_encoder_dpms':
>> drivers/gpu/drm/tilcdc/tilcdc_panel.c:50:6: error: 'pmode' undeclared (first use in this function); did you mean 'mode'?
      50 |  if (pmode == DRM_MODE_DPMS_O)
         |      ^~~~~
         |      mode
   drivers/gpu/drm/tilcdc/tilcdc_panel.c:50:6: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/tilcdc/tilcdc_panel.c:50:15: error: 'DRM_MODE_DPMS_O' undeclared (first use in this function); did you mean 'DRM_MODE_DPMS_ON'?
      50 |  if (pmode == DRM_MODE_DPMS_O)
         |               ^~~~~~~~~~~~~~~
         |               DRM_MODE_DPMS_ON

vim +50 drivers/gpu/drm/tilcdc/tilcdc_panel.c

    43	
    44	static void panel_encoder_dpms(struct drm_encoder *encoder, int mode)
    45	{
    46		struct panel_encoder *panel_encoder = to_panel_encoder(encoder);
    47		struct backlight_device *backlight = panel_encoder->mod->backlight;
    48		struct gpio_desc *gpio = panel_encoder->mod->enable_gpio;
    49	
  > 50		if (pmode == DRM_MODE_DPMS_O)
    51			backlight_set_power_on(backlight);
    52		else
    53			backlight_set_power_off(backlight);
    54	
    55		backlight_update_status(backlight);
    56	
    57		if (gpio)
    58			gpiod_set_value_cansleep(gpio,
    59						 mode == DRM_MODE_DPMS_ON ? 1 : 0);
    60	}
    61	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30521 bytes --]

[PATCH v1 18/22] drm/radeon: Backlight update

[Sam Ravnborg]

- Use macros for initialization
- Replace direct access to backlight_properties with get and set
  operations

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/atombios_encoders.c    | 23 ++++++++++---------
 .../gpu/drm/radeon/radeon_legacy_encoders.c   | 15 ++++++------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index cc5ee1b3af84..c9431af12fed 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -145,14 +145,15 @@ atombios_set_backlight_level(struct radeon_encoder *radeon_encoder, u8 level)
 static u8 radeon_atom_bl_level(struct backlight_device *bd)
 {
 	u8 level;
+	int brightness = backlight_get_brightness(bd);
 
 	/* Convert brightness to hardware level */
-	if (bd->props.brightness < 0)
+	if (brightness < 0)
 		level = 0;
-	else if (bd->props.brightness > RADEON_MAX_BL_LEVEL)
+	else if (brightness > RADEON_MAX_BL_LEVEL)
 		level = RADEON_MAX_BL_LEVEL;
 	else
-		level = bd->props.brightness;
+		level = brightness;
 
 	return level;
 }
@@ -185,12 +186,13 @@ static const struct backlight_ops radeon_atom_backlight_ops = {
 void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 				struct drm_connector *drm_connector)
 {
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, RADEON_MAX_BL_LEVEL);
 	struct drm_device *dev = radeon_encoder->base.dev;
 	struct radeon_device *rdev = dev->dev_private;
 	struct backlight_device *bd;
-	struct backlight_properties props;
 	struct radeon_backlight_privdata *pdata;
 	struct radeon_encoder_atom_dig *dig;
+	int brightness;
 	char bl_name[16];
 
 	/* Mac laptops with multiple GPUs use the gmux driver for backlight
@@ -215,9 +217,6 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 		goto error;
 	}
 
-	memset(&props, 0, sizeof(props));
-	props.max_brightness = RADEON_MAX_BL_LEVEL;
-	props.type = BACKLIGHT_RAW;
 	snprintf(bl_name, sizeof(bl_name),
 		 "radeon_bl%d", dev->primary->index);
 	bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -232,15 +231,17 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 	dig = radeon_encoder->enc_priv;
 	dig->bl_dev = bd;
 
-	bd->props.brightness = radeon_atom_backlight_get_brightness(bd);
+	brightness = radeon_atom_backlight_get_brightness(bd);
 	/* Set a reasonable default here if the level is 0 otherwise
 	 * fbdev will attempt to turn the backlight on after console
 	 * unblanking and it will try and restore 0 which turns the backlight
 	 * off again.
 	 */
-	if (bd->props.brightness == 0)
-		bd->props.brightness = RADEON_MAX_BL_LEVEL;
-	bd->props.power = FB_BLANK_UNBLANK;
+
+	if (brightness == 0)
+		brightness = RADEON_MAX_BL_LEVEL;
+	backlight_set_brightness(bd, brightness);
+	backlight_set_power_on(bd);
 	backlight_update_status(bd);
 
 	DRM_INFO("radeon atom DIG backlight initialized\n");
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 44d060f75318..cf2d1776b975 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -323,14 +323,15 @@ static uint8_t radeon_legacy_lvds_level(struct backlight_device *bd)
 {
 	struct radeon_backlight_privdata *pdata = bl_get_data(bd);
 	uint8_t level;
+	int brightness = backlight_get_brightness(bd);
 
 	/* Convert brightness to hardware level */
-	if (bd->props.brightness < 0)
+	if (brightness < 0)
 		level = 0;
-	else if (bd->props.brightness > RADEON_MAX_BL_LEVEL)
+	else if (brightness > RADEON_MAX_BL_LEVEL)
 		level = RADEON_MAX_BL_LEVEL;
 	else
-		level = bd->props.brightness;
+		level = brightness;
 
 	if (pdata->negative)
 		level = RADEON_MAX_BL_LEVEL - level;
@@ -371,6 +372,7 @@ static const struct backlight_ops radeon_backlight_ops = {
 void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
 				  struct drm_connector *drm_connector)
 {
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, RADEON_MAX_BL_LEVEL);
 	struct drm_device *dev = radeon_encoder->base.dev;
 	struct radeon_device *rdev = dev->dev_private;
 	struct backlight_device *bd;
@@ -394,9 +396,6 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
 		goto error;
 	}
 
-	memset(&props, 0, sizeof(props));
-	props.max_brightness = RADEON_MAX_BL_LEVEL;
-	props.type = BACKLIGHT_RAW;
 	snprintf(bl_name, sizeof(bl_name),
 		 "radeon_bl%d", dev->primary->index);
 	bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -443,8 +442,8 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
 		lvds->bl_dev = bd;
 	}
 
-	bd->props.brightness = radeon_legacy_backlight_get_brightness(bd);
-	bd->props.power = FB_BLANK_UNBLANK;
+	backlight_set_brightness(bd, radeon_legacy_backlight_get_brightness(bd));
+	backlight_set_power_on(bd);
 	backlight_update_status(bd);
 
 	DRM_INFO("radeon legacy LVDS backlight initialized\n");
-- 
2.25.1

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

[PATCH v1 18/22] drm/radeon: Backlight update

[Sam Ravnborg]

- Use macros for initialization
- Replace direct access to backlight_properties with get and set
  operations

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/atombios_encoders.c    | 23 ++++++++++---------
 .../gpu/drm/radeon/radeon_legacy_encoders.c   | 15 ++++++------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index cc5ee1b3af84..c9431af12fed 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -145,14 +145,15 @@ atombios_set_backlight_level(struct radeon_encoder *radeon_encoder, u8 level)
 static u8 radeon_atom_bl_level(struct backlight_device *bd)
 {
 	u8 level;
+	int brightness = backlight_get_brightness(bd);
 
 	/* Convert brightness to hardware level */
-	if (bd->props.brightness < 0)
+	if (brightness < 0)
 		level = 0;
-	else if (bd->props.brightness > RADEON_MAX_BL_LEVEL)
+	else if (brightness > RADEON_MAX_BL_LEVEL)
 		level = RADEON_MAX_BL_LEVEL;
 	else
-		level = bd->props.brightness;
+		level = brightness;
 
 	return level;
 }
@@ -185,12 +186,13 @@ static const struct backlight_ops radeon_atom_backlight_ops = {
 void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 				struct drm_connector *drm_connector)
 {
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, RADEON_MAX_BL_LEVEL);
 	struct drm_device *dev = radeon_encoder->base.dev;
 	struct radeon_device *rdev = dev->dev_private;
 	struct backlight_device *bd;
-	struct backlight_properties props;
 	struct radeon_backlight_privdata *pdata;
 	struct radeon_encoder_atom_dig *dig;
+	int brightness;
 	char bl_name[16];
 
 	/* Mac laptops with multiple GPUs use the gmux driver for backlight
@@ -215,9 +217,6 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 		goto error;
 	}
 
-	memset(&props, 0, sizeof(props));
-	props.max_brightness = RADEON_MAX_BL_LEVEL;
-	props.type = BACKLIGHT_RAW;
 	snprintf(bl_name, sizeof(bl_name),
 		 "radeon_bl%d", dev->primary->index);
 	bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -232,15 +231,17 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 	dig = radeon_encoder->enc_priv;
 	dig->bl_dev = bd;
 
-	bd->props.brightness = radeon_atom_backlight_get_brightness(bd);
+	brightness = radeon_atom_backlight_get_brightness(bd);
 	/* Set a reasonable default here if the level is 0 otherwise
 	 * fbdev will attempt to turn the backlight on after console
 	 * unblanking and it will try and restore 0 which turns the backlight
 	 * off again.
 	 */
-	if (bd->props.brightness == 0)
-		bd->props.brightness = RADEON_MAX_BL_LEVEL;
-	bd->props.power = FB_BLANK_UNBLANK;
+
+	if (brightness == 0)
+		brightness = RADEON_MAX_BL_LEVEL;
+	backlight_set_brightness(bd, brightness);
+	backlight_set_power_on(bd);
 	backlight_update_status(bd);
 
 	DRM_INFO("radeon atom DIG backlight initialized\n");
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 44d060f75318..cf2d1776b975 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -323,14 +323,15 @@ static uint8_t radeon_legacy_lvds_level(struct backlight_device *bd)
 {
 	struct radeon_backlight_privdata *pdata = bl_get_data(bd);
 	uint8_t level;
+	int brightness = backlight_get_brightness(bd);
 
 	/* Convert brightness to hardware level */
-	if (bd->props.brightness < 0)
+	if (brightness < 0)
 		level = 0;
-	else if (bd->props.brightness > RADEON_MAX_BL_LEVEL)
+	else if (brightness > RADEON_MAX_BL_LEVEL)
 		level = RADEON_MAX_BL_LEVEL;
 	else
-		level = bd->props.brightness;
+		level = brightness;
 
 	if (pdata->negative)
 		level = RADEON_MAX_BL_LEVEL - level;
@@ -371,6 +372,7 @@ static const struct backlight_ops radeon_backlight_ops = {
 void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
 				  struct drm_connector *drm_connector)
 {
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, RADEON_MAX_BL_LEVEL);
 	struct drm_device *dev = radeon_encoder->base.dev;
 	struct radeon_device *rdev = dev->dev_private;
 	struct backlight_device *bd;
@@ -394,9 +396,6 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
 		goto error;
 	}
 
-	memset(&props, 0, sizeof(props));
-	props.max_brightness = RADEON_MAX_BL_LEVEL;
-	props.type = BACKLIGHT_RAW;
 	snprintf(bl_name, sizeof(bl_name),
 		 "radeon_bl%d", dev->primary->index);
 	bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -443,8 +442,8 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
 		lvds->bl_dev = bd;
 	}
 
-	bd->props.brightness = radeon_legacy_backlight_get_brightness(bd);
-	bd->props.power = FB_BLANK_UNBLANK;
+	backlight_set_brightness(bd, radeon_legacy_backlight_get_brightness(bd));
+	backlight_set_power_on(bd);
 	backlight_update_status(bd);
 
 	DRM_INFO("radeon legacy LVDS backlight initialized\n");
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v1 19/22] drm/amdgpu/atom: Backlight update

[Sam Ravnborg]

- Use macros for initialization
- Replace direct access to backlight_properties with get and set
  operations

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index 1e94a9b652f7..4338577eb7ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
@@ -122,15 +122,16 @@ amdgpu_atombios_encoder_set_backlight_level(struct amdgpu_encoder *amdgpu_encode
 
 static u8 amdgpu_atombios_encoder_backlight_level(struct backlight_device *bd)
 {
+	int brightness = backlight_get_brightness(bd);
 	u8 level;
 
 	/* Convert brightness to hardware level */
-	if (bd->props.brightness < 0)
+	if (brightness < 0)
 		level = 0;
-	else if (bd->props.brightness > AMDGPU_MAX_BL_LEVEL)
+	else if (brightness > AMDGPU_MAX_BL_LEVEL)
 		level = AMDGPU_MAX_BL_LEVEL;
 	else
-		level = bd->props.brightness;
+		level = brightness;
 
 	return level;
 }
@@ -165,6 +166,7 @@ static const struct backlight_ops amdgpu_atombios_encoder_backlight_ops = {
 void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encoder,
 				     struct drm_connector *drm_connector)
 {
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, AMDGPU_MAX_BL_LEVEL);
 	struct drm_device *dev = amdgpu_encoder->base.dev;
 	struct amdgpu_device *adev = dev->dev_private;
 	struct backlight_device *bd;
@@ -193,9 +195,6 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode
 		goto error;
 	}
 
-	memset(&props, 0, sizeof(props));
-	props.max_brightness = AMDGPU_MAX_BL_LEVEL;
-	props.type = BACKLIGHT_RAW;
 	snprintf(bl_name, sizeof(bl_name),
 		 "amdgpu_bl%d", dev->primary->index);
 	bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -212,8 +211,8 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode
 	dig = amdgpu_encoder->enc_priv;
 	dig->bl_dev = bd;
 
-	bd->props.brightness = amdgpu_atombios_encoder_get_backlight_brightness(bd);
-	bd->props.power = FB_BLANK_UNBLANK;
+	backlight_set_brightness(bd, amdgpu_atombios_encoder_get_backlight_brightness(bd));
+	backlight_set_power_on(bd);
 	backlight_update_status(bd);
 
 	DRM_INFO("amdgpu atom DIG backlight initialized\n");
-- 
2.25.1

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

[PATCH v1 19/22] drm/amdgpu/atom: Backlight update

[Sam Ravnborg]

- Use macros for initialization
- Replace direct access to backlight_properties with get and set
  operations

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index 1e94a9b652f7..4338577eb7ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
@@ -122,15 +122,16 @@ amdgpu_atombios_encoder_set_backlight_level(struct amdgpu_encoder *amdgpu_encode
 
 static u8 amdgpu_atombios_encoder_backlight_level(struct backlight_device *bd)
 {
+	int brightness = backlight_get_brightness(bd);
 	u8 level;
 
 	/* Convert brightness to hardware level */
-	if (bd->props.brightness < 0)
+	if (brightness < 0)
 		level = 0;
-	else if (bd->props.brightness > AMDGPU_MAX_BL_LEVEL)
+	else if (brightness > AMDGPU_MAX_BL_LEVEL)
 		level = AMDGPU_MAX_BL_LEVEL;
 	else
-		level = bd->props.brightness;
+		level = brightness;
 
 	return level;
 }
@@ -165,6 +166,7 @@ static const struct backlight_ops amdgpu_atombios_encoder_backlight_ops = {
 void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encoder,
 				     struct drm_connector *drm_connector)
 {
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, AMDGPU_MAX_BL_LEVEL);
 	struct drm_device *dev = amdgpu_encoder->base.dev;
 	struct amdgpu_device *adev = dev->dev_private;
 	struct backlight_device *bd;
@@ -193,9 +195,6 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode
 		goto error;
 	}
 
-	memset(&props, 0, sizeof(props));
-	props.max_brightness = AMDGPU_MAX_BL_LEVEL;
-	props.type = BACKLIGHT_RAW;
 	snprintf(bl_name, sizeof(bl_name),
 		 "amdgpu_bl%d", dev->primary->index);
 	bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -212,8 +211,8 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode
 	dig = amdgpu_encoder->enc_priv;
 	dig->bl_dev = bd;
 
-	bd->props.brightness = amdgpu_atombios_encoder_get_backlight_brightness(bd);
-	bd->props.power = FB_BLANK_UNBLANK;
+	backlight_set_brightness(bd, amdgpu_atombios_encoder_get_backlight_brightness(bd));
+	backlight_set_power_on(bd);
 	backlight_update_status(bd);
 
 	DRM_INFO("amdgpu atom DIG backlight initialized\n");
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v1 20/22] drm/i915: Backlight update

[Sam Ravnborg]

Update backlight implementation to utilize newly added backlight
functionality.

- Use macros for initialization
- Replace direct access to backlight_properties with get and set
  operations
- Dropped extra checks as some methods accepts a NULL backlight device.

One side-effect of these changes is that the confusing power states
are now replaced by two simple set functions.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Wambui Karuga <wambui.karugax@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/i915/display/intel_panel.c | 88 +++++++++++-----------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 3c5056dbf607..ff37dac9d3e8 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -716,11 +716,15 @@ void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state
 	hw_level = clamp_user_to_hw(connector, user_level, user_max);
 	panel->backlight.level = hw_level;
 
-	if (panel->backlight.device)
-		panel->backlight.device->props.brightness =
-			scale_hw_to_user(connector,
-					 panel->backlight.level,
-					 panel->backlight.device->props.max_brightness);
+	if (panel->backlight.device) {
+		int brightness;
+		int max = backlight_get_max_brightness(panel->backlight.device);
+
+		brightness = scale_hw_to_user(connector,
+					      panel->backlight.level,
+					      max);
+		backlight_set_brightness(panel->backlight.device, brightness);
+	}
 
 	if (panel->backlight.enabled)
 		intel_panel_actually_set_backlight(conn_state, hw_level);
@@ -871,8 +875,7 @@ void intel_panel_disable_backlight(const struct drm_connector_state *old_conn_st
 
 	mutex_lock(&dev_priv->backlight_lock);
 
-	if (panel->backlight.device)
-		panel->backlight.device->props.power = FB_BLANK_POWERDOWN;
+	backlight_set_power_off(panel->backlight.device);
 	panel->backlight.enabled = false;
 	panel->backlight.disable(old_conn_state);
 
@@ -1192,17 +1195,20 @@ static void __intel_panel_enable_backlight(const struct intel_crtc_state *crtc_s
 
 	if (panel->backlight.level <= panel->backlight.min) {
 		panel->backlight.level = panel->backlight.max;
-		if (panel->backlight.device)
-			panel->backlight.device->props.brightness =
-				scale_hw_to_user(connector,
-						 panel->backlight.level,
-						 panel->backlight.device->props.max_brightness);
+		if (panel->backlight.device) {
+			int brightness;
+			int max = backlight_get_max_brightness(panel->backlight.device);
+
+			brightness = scale_hw_to_user(connector,
+						      panel->backlight.level,
+						      max);
+			backlight_set_brightness(panel->backlight.device, brightness);
+		}
 	}
 
 	panel->backlight.enable(crtc_state, conn_state);
 	panel->backlight.enabled = true;
-	if (panel->backlight.device)
-		panel->backlight.device->props.power = FB_BLANK_UNBLANK;
+	backlight_set_power_on(panel->backlight.device);
 }
 
 void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
@@ -1288,10 +1294,11 @@ static int intel_backlight_device_update_status(struct backlight_device *bd)
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
-		      bd->props.brightness, bd->props.max_brightness);
-	intel_panel_set_backlight(connector->base.state, bd->props.brightness,
-				  bd->props.max_brightness);
-
+		      backlight_get_brightness(bd),
+		      backlight_get_max_brightness(bd));
+	intel_panel_set_backlight(connector->base.state,
+				  backlight_get_brightness(bd),
+				  backlight_get_max_brightness(bd));
 	/*
 	 * Allow flipping bl_power as a sub-state of enabled. Sadly the
 	 * backlight class device does not make it easy to to differentiate
@@ -1299,13 +1306,10 @@ static int intel_backlight_device_update_status(struct backlight_device *bd)
 	 * callback needs to take this into account.
 	 */
 	if (panel->backlight.enabled) {
-		if (panel->backlight.power) {
-			bool enable = bd->props.power == FB_BLANK_UNBLANK &&
-				bd->props.brightness != 0;
-			panel->backlight.power(connector, enable);
-		}
+		if (panel->backlight.power)
+			panel->backlight.power(connector, !backlight_is_blank(bd));
 	} else {
-		bd->props.power = FB_BLANK_POWERDOWN;
+		backlight_set_power_off(bd);
 	}
 
 	drm_modeset_unlock(&dev->mode_config.connection_mutex);
@@ -1322,12 +1326,12 @@ static int intel_backlight_device_get_brightness(struct backlight_device *bd)
 
 	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
 		u32 hw_level;
+		int max = backlight_get_max_brightness(bd);
 
 		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 
 		hw_level = intel_panel_get_backlight(connector);
-		ret = scale_hw_to_user(connector,
-				       hw_level, bd->props.max_brightness);
+		ret = scale_hw_to_user(connector, hw_level, max);
 
 		drm_modeset_unlock(&dev->mode_config.connection_mutex);
 	}
@@ -1344,7 +1348,12 @@ int intel_backlight_device_register(struct intel_connector *connector)
 {
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	struct backlight_properties props;
+	/*
+	 * Note: Everything should work even if the backlight device max
+	 * presented to the userspace is arbitrarily chosen.
+	 */
+	DECLARE_BACKLIGHT_INIT_RAW(props, 0, panel->backlight.max);
+	int brightness;
 
 	if (WARN_ON(panel->backlight.device))
 		return -ENODEV;
@@ -1354,23 +1363,6 @@ int intel_backlight_device_register(struct intel_connector *connector)
 
 	WARN_ON(panel->backlight.max == 0);
 
-	memset(&props, 0, sizeof(props));
-	props.type = BACKLIGHT_RAW;
-
-	/*
-	 * Note: Everything should work even if the backlight device max
-	 * presented to the userspace is arbitrarily chosen.
-	 */
-	props.max_brightness = panel->backlight.max;
-	props.brightness = scale_hw_to_user(connector,
-					    panel->backlight.level,
-					    props.max_brightness);
-
-	if (panel->backlight.enabled)
-		props.power = FB_BLANK_UNBLANK;
-	else
-		props.power = FB_BLANK_POWERDOWN;
-
 	/*
 	 * Note: using the same name independent of the connector prevents
 	 * registration of multiple backlight devices in the driver.
@@ -1388,6 +1380,14 @@ int intel_backlight_device_register(struct intel_connector *connector)
 		return -ENODEV;
 	}
 
+	brightness = scale_hw_to_user(connector, panel->backlight.level, panel->backlight.max);
+	backlight_set_brightness(panel->backlight.device, brightness);
+
+	if (panel->backlight.enabled)
+		backlight_set_power_on(panel->backlight.device);
+	else
+		backlight_set_power_off(panel->backlight.device);
+
 	drm_dbg_kms(&i915->drm,
 		    "Connector %s backlight sysfs interface registered\n",
 		    connector->base.name);
-- 
2.25.1

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

[PATCH v1 21/22] drm/omap: display: Backlight update

[Sam Ravnborg]

- Introduce backlight_{enable/disable)
- Use get/set methods for backlight_properties
- Drop redundant get_brightness() implementation
  The default implementation return the current brightness value
- Use macro for backlight initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Zheng Bin <zhengbin13@huawei.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 35 ++++---------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 3484b5d4a91c..433e240896b3 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -110,15 +110,10 @@ static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable)
 	else
 		return;
 
-	if (enable) {
-		backlight->props.fb_blank = FB_BLANK_UNBLANK;
-		backlight->props.state = ~(BL_CORE_FBBLANK | BL_CORE_SUSPENDED);
-		backlight->props.power = FB_BLANK_UNBLANK;
-	} else {
-		backlight->props.fb_blank = FB_BLANK_NORMAL;
-		backlight->props.power = FB_BLANK_POWERDOWN;
-		backlight->props.state |= BL_CORE_FBBLANK | BL_CORE_SUSPENDED;
-	}
+	if (enable)
+		backlight_enable(backlight);
+	else
+		backlight_disable(backlight);
 
 	backlight_update_status(backlight);
 }
@@ -363,13 +358,7 @@ static int dsicm_bl_update_status(struct backlight_device *dev)
 	struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
 	struct omap_dss_device *src = ddata->src;
 	int r = 0;
-	int level;
-
-	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-			dev->props.power == FB_BLANK_UNBLANK)
-		level = dev->props.brightness;
-	else
-		level = 0;
+	int level = backlight_get_brightness(dev);
 
 	dev_dbg(&ddata->pdev->dev, "update brightness to %d\n", level);
 
@@ -390,17 +379,7 @@ static int dsicm_bl_update_status(struct backlight_device *dev)
 	return r;
 }
 
-static int dsicm_bl_get_intensity(struct backlight_device *dev)
-{
-	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-			dev->props.power == FB_BLANK_UNBLANK)
-		return dev->props.brightness;
-
-	return 0;
-}
-
 static const struct backlight_ops dsicm_bl_ops = {
-	.get_brightness = dsicm_bl_get_intensity,
 	.update_status  = dsicm_bl_update_status,
 };
 
@@ -1305,9 +1284,7 @@ static int dsicm_probe(struct platform_device *pdev)
 	dsicm_hw_reset(ddata);
 
 	if (ddata->use_dsi_backlight) {
-		struct backlight_properties props = { 0 };
-		props.max_brightness = 255;
-		props.type = BACKLIGHT_RAW;
+		DECLARE_BACKLIGHT_INIT_RAW(props, 0, 255);
 
 		bldev = devm_backlight_device_register(dev, dev_name(dev),
 			dev, ddata, &dsicm_bl_ops, &props);
-- 
2.25.1

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

Re: [PATCH v1 21/22] drm/omap: display: Backlight update

[Sebastian Reichel]

Hi,

On Sun, Aug 02, 2020 at 01:06:35PM +0200, Sam Ravnborg wrote:
> - Introduce backlight_{enable/disable)
> - Use get/set methods for backlight_properties
> - Drop redundant get_brightness() implementation
>   The default implementation return the current brightness value
> - Use macro for backlight initialization
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Zheng Bin <zhengbin13@huawei.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> ---
>  .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 35 ++++---------------
>  1 file changed, 6 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index 3484b5d4a91c..433e240896b3 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -110,15 +110,10 @@ static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable)
>  	else
>  		return;
>  
> -	if (enable) {
> -		backlight->props.fb_blank = FB_BLANK_UNBLANK;
> -		backlight->props.state = ~(BL_CORE_FBBLANK | BL_CORE_SUSPENDED);
> -		backlight->props.power = FB_BLANK_UNBLANK;
> -	} else {
> -		backlight->props.fb_blank = FB_BLANK_NORMAL;
> -		backlight->props.power = FB_BLANK_POWERDOWN;
> -		backlight->props.state |= BL_CORE_FBBLANK | BL_CORE_SUSPENDED;
> -	}
> +	if (enable)
> +		backlight_enable(backlight);
> +	else
> +		backlight_disable(backlight);
>  
>  	backlight_update_status(backlight);

backlight_update_status() is already called by backlight_enable/disable.

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

Re: [PATCH v1 21/22] drm/omap: display: Backlight update

[Sam Ravnborg]

Hi Sebastian.

On Sun, Aug 02, 2020 at 04:26:05PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Sun, Aug 02, 2020 at 01:06:35PM +0200, Sam Ravnborg wrote:
> > - Introduce backlight_{enable/disable)
> > - Use get/set methods for backlight_properties
> > - Drop redundant get_brightness() implementation
> >   The default implementation return the current brightness value
> > - Use macro for backlight initialization
> > 
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Zheng Bin <zhengbin13@huawei.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > ---
> >  .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 35 ++++---------------
> >  1 file changed, 6 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> > index 3484b5d4a91c..433e240896b3 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> > @@ -110,15 +110,10 @@ static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable)
> >  	else
> >  		return;
> >  
> > -	if (enable) {
> > -		backlight->props.fb_blank = FB_BLANK_UNBLANK;
> > -		backlight->props.state = ~(BL_CORE_FBBLANK | BL_CORE_SUSPENDED);
> > -		backlight->props.power = FB_BLANK_UNBLANK;
> > -	} else {
> > -		backlight->props.fb_blank = FB_BLANK_NORMAL;
> > -		backlight->props.power = FB_BLANK_POWERDOWN;
> > -		backlight->props.state |= BL_CORE_FBBLANK | BL_CORE_SUSPENDED;
> > -	}
> > +	if (enable)
> > +		backlight_enable(backlight);
> > +	else
> > +		backlight_disable(backlight);
> >  
> >  	backlight_update_status(backlight);
> 
> backlight_update_status() is already called by backlight_enable/disable.

Right, thanks.
Dropped in v2.

Let me know if you already have a similar patch and if I shall drop
this.

It would be nice to have the panel parts of omapdrm migrated in this cycle.
I recall you have 50+ patches pending.

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

Re: [PATCH v1 21/22] drm/omap: display: Backlight update

[Sebastian Reichel]

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

Hi,

On Sun, Aug 02, 2020 at 04:32:07PM +0200, Sam Ravnborg wrote:
> On Sun, Aug 02, 2020 at 04:26:05PM +0200, Sebastian Reichel wrote:
> > On Sun, Aug 02, 2020 at 01:06:35PM +0200, Sam Ravnborg wrote:
> > > - Introduce backlight_{enable/disable)
> > > - Use get/set methods for backlight_properties
> > > - Drop redundant get_brightness() implementation
> > >   The default implementation return the current brightness value
> > > - Use macro for backlight initialization
> > > 
> > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Cc: Zheng Bin <zhengbin13@huawei.com>
> > > Cc: Sam Ravnborg <sam@ravnborg.org>
> > > ---
> > >  .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 35 ++++---------------
> > >  1 file changed, 6 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> > > index 3484b5d4a91c..433e240896b3 100644
> > > --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> > > +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> > > @@ -110,15 +110,10 @@ static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable)
> > >  	else
> > >  		return;
> > >  
> > > -	if (enable) {
> > > -		backlight->props.fb_blank = FB_BLANK_UNBLANK;
> > > -		backlight->props.state = ~(BL_CORE_FBBLANK | BL_CORE_SUSPENDED);
> > > -		backlight->props.power = FB_BLANK_UNBLANK;
> > > -	} else {
> > > -		backlight->props.fb_blank = FB_BLANK_NORMAL;
> > > -		backlight->props.power = FB_BLANK_POWERDOWN;
> > > -		backlight->props.state |= BL_CORE_FBBLANK | BL_CORE_SUSPENDED;
> > > -	}
> > > +	if (enable)
> > > +		backlight_enable(backlight);
> > > +	else
> > > +		backlight_disable(backlight);
> > >  
> > >  	backlight_update_status(backlight);
> > 
> > backlight_update_status() is already called by backlight_enable/disable.
> 
> Right, thanks.
> Dropped in v2.
> 
> Let me know if you already have a similar patch and if I shall
> drop this.

I did not touch the backlight bits and I can easily rebase my patches. I think
this should be kept.

> It would be nice to have the panel parts of omapdrm migrated in
> this cycle. I recall you have 50+ patches pending.

I plan to send only the first part and go step by step.

-- Sebastian

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

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

[PATCH v1 22/22] drm/shmobile: Backlight update

[Sam Ravnborg]

- Use get/set methods for backlight_properties
- Use macro for backlight initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: linux-renesas-soc@vger.kernel.org
---
 .../gpu/drm/shmobile/shmob_drm_backlight.c    | 20 ++++++++-----------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_backlight.c b/drivers/gpu/drm/shmobile/shmob_drm_backlight.c
index f6628a5ee95f..407028df0212 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_backlight.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_backlight.c
@@ -18,13 +18,8 @@ static int shmob_drm_backlight_update(struct backlight_device *bdev)
 	struct shmob_drm_connector *scon = bl_get_data(bdev);
 	struct shmob_drm_device *sdev = scon->connector.dev->dev_private;
 	const struct shmob_drm_backlight_data *bdata = &sdev->pdata->backlight;
-	int brightness = bdev->props.brightness;
 
-	if (bdev->props.power != FB_BLANK_UNBLANK ||
-	    bdev->props.state & BL_CORE_SUSPENDED)
-		brightness = 0;
-
-	return bdata->set_brightness(brightness);
+	return bdata->set_brightness(backlight_get_brightness(bdev));
 }
 
 static int shmob_drm_backlight_get_brightness(struct backlight_device *bdev)
@@ -47,8 +42,11 @@ void shmob_drm_backlight_dpms(struct shmob_drm_connector *scon, int mode)
 	if (scon->backlight == NULL)
 		return;
 
-	scon->backlight->props.power = mode == DRM_MODE_DPMS_ON
-				     ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
+	if (mode == DRM_MODE_DPMS_ON)
+		backlight_set_power_on(scon->backlight);
+	else
+		backlight_set_power_off(scon->backlight);
+
 	backlight_update_status(scon->backlight);
 }
 
@@ -59,21 +57,19 @@ int shmob_drm_backlight_init(struct shmob_drm_connector *scon)
 	struct drm_connector *connector = &scon->connector;
 	struct drm_device *dev = connector->dev;
 	struct backlight_device *backlight;
+	DECLARE_BACKLIGHT_INIT_RAW(props, bdata->max_brightness, bdata->max_brightness);
 
 	if (!bdata->max_brightness)
 		return 0;
 
 	backlight = backlight_device_register(bdata->name, dev->dev, scon,
-					      &shmob_drm_backlight_ops, NULL);
+					      &shmob_drm_backlight_ops, &props);
 	if (IS_ERR(backlight)) {
 		dev_err(dev->dev, "unable to register backlight device: %ld\n",
 			PTR_ERR(backlight));
 		return PTR_ERR(backlight);
 	}
 
-	backlight->props.max_brightness = bdata->max_brightness;
-	backlight->props.brightness = bdata->max_brightness;
-	backlight->props.power = FB_BLANK_POWERDOWN;
 	backlight_update_status(backlight);
 
 	scon->backlight = backlight;
-- 
2.25.1

[PATCH v1 22/22] drm/shmobile: Backlight update

[Sam Ravnborg]

- Use get/set methods for backlight_properties
- Use macro for backlight initialization

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: linux-renesas-soc@vger.kernel.org
---
 .../gpu/drm/shmobile/shmob_drm_backlight.c    | 20 ++++++++-----------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_backlight.c b/drivers/gpu/drm/shmobile/shmob_drm_backlight.c
index f6628a5ee95f..407028df0212 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_backlight.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_backlight.c
@@ -18,13 +18,8 @@ static int shmob_drm_backlight_update(struct backlight_device *bdev)
 	struct shmob_drm_connector *scon = bl_get_data(bdev);
 	struct shmob_drm_device *sdev = scon->connector.dev->dev_private;
 	const struct shmob_drm_backlight_data *bdata = &sdev->pdata->backlight;
-	int brightness = bdev->props.brightness;
 
-	if (bdev->props.power != FB_BLANK_UNBLANK ||
-	    bdev->props.state & BL_CORE_SUSPENDED)
-		brightness = 0;
-
-	return bdata->set_brightness(brightness);
+	return bdata->set_brightness(backlight_get_brightness(bdev));
 }
 
 static int shmob_drm_backlight_get_brightness(struct backlight_device *bdev)
@@ -47,8 +42,11 @@ void shmob_drm_backlight_dpms(struct shmob_drm_connector *scon, int mode)
 	if (scon->backlight == NULL)
 		return;
 
-	scon->backlight->props.power = mode == DRM_MODE_DPMS_ON
-				     ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
+	if (mode == DRM_MODE_DPMS_ON)
+		backlight_set_power_on(scon->backlight);
+	else
+		backlight_set_power_off(scon->backlight);
+
 	backlight_update_status(scon->backlight);
 }
 
@@ -59,21 +57,19 @@ int shmob_drm_backlight_init(struct shmob_drm_connector *scon)
 	struct drm_connector *connector = &scon->connector;
 	struct drm_device *dev = connector->dev;
 	struct backlight_device *backlight;
+	DECLARE_BACKLIGHT_INIT_RAW(props, bdata->max_brightness, bdata->max_brightness);
 
 	if (!bdata->max_brightness)
 		return 0;
 
 	backlight = backlight_device_register(bdata->name, dev->dev, scon,
-					      &shmob_drm_backlight_ops, NULL);
+					      &shmob_drm_backlight_ops, &props);
 	if (IS_ERR(backlight)) {
 		dev_err(dev->dev, "unable to register backlight device: %ld\n",
 			PTR_ERR(backlight));
 		return PTR_ERR(backlight);
 	}
 
-	backlight->props.max_brightness = bdata->max_brightness;
-	backlight->props.brightness = bdata->max_brightness;
-	backlight->props.power = FB_BLANK_POWERDOWN;
 	backlight_update_status(backlight);
 
 	scon->backlight = backlight;
-- 
2.25.1

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

Re: [RFC PATCH v1 0/22] backlight: add init macros and accessors

[daniel]

On Sun, Aug 02, 2020 at 01:06:14PM +0200, Sam Ravnborg wrote:
> The backlight drivers uses several different patterns when registering
> a backlight:
> 
> - Register backlight and assign properties later
> - Define a local backlight_properties variable and use memset
> - Define a const backlight_properties and assign relevant properties
> 
> On top of this there was differences in what members was assigned in
> backlight_properties.
> 
> To align how backlight drivers are initialized introduce following helper macros:
> - DECLARE_BACKLIGHT_INIT_FIRMWARE()
> - DECLARE_BACKLIGHT_INIT_PLATFORM()
> - DECLARE_BACKLIGHT_INIT_RAW()
> 
> The macros are introduced in patch 2.
> 
> The backlight drivers used direct access to backlight_properties.
> Encapsulate these in get/set access operations resulting in following benefits:
> - The drivers no longer need to be concerned about the confusing power states,
>   as there is now only a set_power_on() and set_power_off() operation.
> - The access methods can be called with a NULL pointer so logic around the
>   access can be made simpler.
> - The code is in most cases more readable with the access operations.
> - When everyone uses the access methods refactroring in the backlight core is simpler.
> 
> The get/set operations are introduced in patch 3.
> 
> The first patch trims backlight_update_status() so it can be called with a NULL
> backlight_device. Then the called do not need to add this check just to avoid
> a NULL reference.
> 
> The fourth patch introduce the new macros and get/set operations for the
> gpio backlight driver, as an example.
> 
> The remaining patches updates most backlight users in drivers/gpu/drm/*
> With this patch set applied then:
> - Almost all references to FB_BLANK* are gone from drm/*
> - All panel drivers uses devm_ variant for registering backlight
> - Almost all direct references to backlight properties are gone
> 
> The drm/* patches are  used as examples how drivers can benefit from the
> new macros and get/set operations.
> 
> Individual patches are only sent to the people listed in the patch + a few more.
> Please check https://lore.kernel.org/dri-devel/ for the full series.
> 
> Feedback welcome!

Since this needs backlight patches queued up outside of drm there's two
options:

- merge the backlight stuff through drm-misc (imo simplest, we have all
  the fbdev stuff in there too by now)
- shared topic branch merged in drm-misc and optionally backlight tree

Otherwise this is going to be a pain to merge.
-Daniel

> 
> 	Sam
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Konrad Dybcio <konradybcio@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> Cc: Philippe CORNU <philippe.cornu@st.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Vinay Simha BN <simhavcs@gmail.com>
> Cc: Wambui Karuga <wambui.karugax@gmail.com>
> Cc: Zheng Bin <zhengbin13@huawei.com>
> 
> Sam Ravnborg (22):
>       backlight: Silently fail backlight_update_status() if no device
>       backlight: Add DECLARE_* macro for device registration
>       backlight: Add get/set operations for brightness/power properties
>       backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters
>       drm/gma500: Backlight support
>       drm/panel: asus-z00t-tm5p5-n35596: Backlight update
>       drm/panel: jdi-lt070me05000: Backlight update
>       drm/panel: novatek-nt35510: Backlight update
>       drm/panel: orisetech-otm8009a: Backlight update
>       drm/panel: raydium-rm67191: Backlight update
>       drm/panel: samsung-s6e63m0: Backlight update
>       drm/panel: samsung-s6e63j0x03: Backlight update
>       drm/panel: samsung-s6e3ha2: Backlight update
>       drm/panel: sony-acx424akp: Backlight update
>       drm/panel: sony-acx565akm: Backlight update
>       drm/bridge: parade-ps8622: Backlight update
>       drm/tilcdc: Backlight update
>       drm/radeon: Backlight update
>       drm/amdgpu/atom: Backlight update
>       drm/i915: Backlight update
>       drm/omap: display: Backlight update
>       drm/shmobile: Backlight update
> 
>  drivers/gpu/drm/amd/amdgpu/atombios_encoders.c     |  15 ++-
>  drivers/gpu/drm/bridge/parade-ps8622.c             |  43 ++++----
>  drivers/gpu/drm/gma500/backlight.c                 |  35 ++----
>  drivers/gpu/drm/gma500/cdv_device.c                |  29 +++--
>  drivers/gpu/drm/gma500/mdfld_device.c              |   9 +-
>  drivers/gpu/drm/gma500/oaktrail_device.c           |  10 +-
>  drivers/gpu/drm/gma500/opregion.c                  |   2 +-
>  drivers/gpu/drm/gma500/psb_device.c                |  10 +-
>  drivers/gpu/drm/gma500/psb_drv.c                   |   8 +-
>  drivers/gpu/drm/i915/display/intel_panel.c         |  88 +++++++--------
>  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  35 ++----
>  .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
>  drivers/gpu/drm/panel/panel-jdi-lt070me05000.c     |  17 ++-
>  drivers/gpu/drm/panel/panel-novatek-nt35510.c      |   9 +-
>  drivers/gpu/drm/panel/panel-orisetech-otm8009a.c   |  14 +--
>  drivers/gpu/drm/panel/panel-raydium-rm67191.c      |  11 +-
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      |  68 ++++++------
>  drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   |  56 +++++-----
>  drivers/gpu/drm/panel/panel-samsung-s6e63m0.c      |  25 ++---
>  drivers/gpu/drm/panel/panel-sony-acx424akp.c       |  49 ++-------
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c       |  44 +++-----
>  drivers/gpu/drm/radeon/atombios_encoders.c         |  23 ++--
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    |  15 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_backlight.c     |  20 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  11 +-
>  drivers/video/backlight/gpio_backlight.c           |  17 ++-
>  include/linux/backlight.h                          | 120 +++++++++++++++++++++
>  27 files changed, 377 insertions(+), 421 deletions(-)
> 
> 

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

Re: [RFC PATCH v1 0/22] backlight: add init macros and accessors

[daniel]

On Sun, Aug 02, 2020 at 01:06:14PM +0200, Sam Ravnborg wrote:
> The backlight drivers uses several different patterns when registering
> a backlight:
> 
> - Register backlight and assign properties later
> - Define a local backlight_properties variable and use memset
> - Define a const backlight_properties and assign relevant properties
> 
> On top of this there was differences in what members was assigned in
> backlight_properties.
> 
> To align how backlight drivers are initialized introduce following helper macros:
> - DECLARE_BACKLIGHT_INIT_FIRMWARE()
> - DECLARE_BACKLIGHT_INIT_PLATFORM()
> - DECLARE_BACKLIGHT_INIT_RAW()
> 
> The macros are introduced in patch 2.
> 
> The backlight drivers used direct access to backlight_properties.
> Encapsulate these in get/set access operations resulting in following benefits:
> - The drivers no longer need to be concerned about the confusing power states,
>   as there is now only a set_power_on() and set_power_off() operation.
> - The access methods can be called with a NULL pointer so logic around the
>   access can be made simpler.
> - The code is in most cases more readable with the access operations.
> - When everyone uses the access methods refactroring in the backlight core is simpler.
> 
> The get/set operations are introduced in patch 3.
> 
> The first patch trims backlight_update_status() so it can be called with a NULL
> backlight_device. Then the called do not need to add this check just to avoid
> a NULL reference.
> 
> The fourth patch introduce the new macros and get/set operations for the
> gpio backlight driver, as an example.
> 
> The remaining patches updates most backlight users in drivers/gpu/drm/*
> With this patch set applied then:
> - Almost all references to FB_BLANK* are gone from drm/*
> - All panel drivers uses devm_ variant for registering backlight
> - Almost all direct references to backlight properties are gone
> 
> The drm/* patches are  used as examples how drivers can benefit from the
> new macros and get/set operations.
> 
> Individual patches are only sent to the people listed in the patch + a few more.
> Please check https://lore.kernel.org/dri-devel/ for the full series.
> 
> Feedback welcome!

Since this needs backlight patches queued up outside of drm there's two
options:

- merge the backlight stuff through drm-misc (imo simplest, we have all
  the fbdev stuff in there too by now)
- shared topic branch merged in drm-misc and optionally backlight tree

Otherwise this is going to be a pain to merge.
-Daniel

> 
> 	Sam
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Konrad Dybcio <konradybcio@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> Cc: Philippe CORNU <philippe.cornu@st.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Vinay Simha BN <simhavcs@gmail.com>
> Cc: Wambui Karuga <wambui.karugax@gmail.com>
> Cc: Zheng Bin <zhengbin13@huawei.com>
> 
> Sam Ravnborg (22):
>       backlight: Silently fail backlight_update_status() if no device
>       backlight: Add DECLARE_* macro for device registration
>       backlight: Add get/set operations for brightness/power properties
>       backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters
>       drm/gma500: Backlight support
>       drm/panel: asus-z00t-tm5p5-n35596: Backlight update
>       drm/panel: jdi-lt070me05000: Backlight update
>       drm/panel: novatek-nt35510: Backlight update
>       drm/panel: orisetech-otm8009a: Backlight update
>       drm/panel: raydium-rm67191: Backlight update
>       drm/panel: samsung-s6e63m0: Backlight update
>       drm/panel: samsung-s6e63j0x03: Backlight update
>       drm/panel: samsung-s6e3ha2: Backlight update
>       drm/panel: sony-acx424akp: Backlight update
>       drm/panel: sony-acx565akm: Backlight update
>       drm/bridge: parade-ps8622: Backlight update
>       drm/tilcdc: Backlight update
>       drm/radeon: Backlight update
>       drm/amdgpu/atom: Backlight update
>       drm/i915: Backlight update
>       drm/omap: display: Backlight update
>       drm/shmobile: Backlight update
> 
>  drivers/gpu/drm/amd/amdgpu/atombios_encoders.c     |  15 ++-
>  drivers/gpu/drm/bridge/parade-ps8622.c             |  43 ++++----
>  drivers/gpu/drm/gma500/backlight.c                 |  35 ++----
>  drivers/gpu/drm/gma500/cdv_device.c                |  29 +++--
>  drivers/gpu/drm/gma500/mdfld_device.c              |   9 +-
>  drivers/gpu/drm/gma500/oaktrail_device.c           |  10 +-
>  drivers/gpu/drm/gma500/opregion.c                  |   2 +-
>  drivers/gpu/drm/gma500/psb_device.c                |  10 +-
>  drivers/gpu/drm/gma500/psb_drv.c                   |   8 +-
>  drivers/gpu/drm/i915/display/intel_panel.c         |  88 +++++++--------
>  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  35 ++----
>  .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
>  drivers/gpu/drm/panel/panel-jdi-lt070me05000.c     |  17 ++-
>  drivers/gpu/drm/panel/panel-novatek-nt35510.c      |   9 +-
>  drivers/gpu/drm/panel/panel-orisetech-otm8009a.c   |  14 +--
>  drivers/gpu/drm/panel/panel-raydium-rm67191.c      |  11 +-
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      |  68 ++++++------
>  drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   |  56 +++++-----
>  drivers/gpu/drm/panel/panel-samsung-s6e63m0.c      |  25 ++---
>  drivers/gpu/drm/panel/panel-sony-acx424akp.c       |  49 ++-------
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c       |  44 +++-----
>  drivers/gpu/drm/radeon/atombios_encoders.c         |  23 ++--
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    |  15 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_backlight.c     |  20 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  11 +-
>  drivers/video/backlight/gpio_backlight.c           |  17 ++-
>  include/linux/backlight.h                          | 120 +++++++++++++++++++++
>  27 files changed, 377 insertions(+), 421 deletions(-)
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC PATCH v1 0/22] backlight: add init macros and accessors

[daniel]

On Sun, Aug 02, 2020 at 01:06:14PM +0200, Sam Ravnborg wrote:
> The backlight drivers uses several different patterns when registering
> a backlight:
> 
> - Register backlight and assign properties later
> - Define a local backlight_properties variable and use memset
> - Define a const backlight_properties and assign relevant properties
> 
> On top of this there was differences in what members was assigned in
> backlight_properties.
> 
> To align how backlight drivers are initialized introduce following helper macros:
> - DECLARE_BACKLIGHT_INIT_FIRMWARE()
> - DECLARE_BACKLIGHT_INIT_PLATFORM()
> - DECLARE_BACKLIGHT_INIT_RAW()
> 
> The macros are introduced in patch 2.
> 
> The backlight drivers used direct access to backlight_properties.
> Encapsulate these in get/set access operations resulting in following benefits:
> - The drivers no longer need to be concerned about the confusing power states,
>   as there is now only a set_power_on() and set_power_off() operation.
> - The access methods can be called with a NULL pointer so logic around the
>   access can be made simpler.
> - The code is in most cases more readable with the access operations.
> - When everyone uses the access methods refactroring in the backlight core is simpler.
> 
> The get/set operations are introduced in patch 3.
> 
> The first patch trims backlight_update_status() so it can be called with a NULL
> backlight_device. Then the called do not need to add this check just to avoid
> a NULL reference.
> 
> The fourth patch introduce the new macros and get/set operations for the
> gpio backlight driver, as an example.
> 
> The remaining patches updates most backlight users in drivers/gpu/drm/*
> With this patch set applied then:
> - Almost all references to FB_BLANK* are gone from drm/*
> - All panel drivers uses devm_ variant for registering backlight
> - Almost all direct references to backlight properties are gone
> 
> The drm/* patches are  used as examples how drivers can benefit from the
> new macros and get/set operations.
> 
> Individual patches are only sent to the people listed in the patch + a few more.
> Please check https://lore.kernel.org/dri-devel/ for the full series.
> 
> Feedback welcome!

Since this needs backlight patches queued up outside of drm there's two
options:

- merge the backlight stuff through drm-misc (imo simplest, we have all
  the fbdev stuff in there too by now)
- shared topic branch merged in drm-misc and optionally backlight tree

Otherwise this is going to be a pain to merge.
-Daniel

> 
> 	Sam
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Konrad Dybcio <konradybcio@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> Cc: Philippe CORNU <philippe.cornu@st.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Vinay Simha BN <simhavcs@gmail.com>
> Cc: Wambui Karuga <wambui.karugax@gmail.com>
> Cc: Zheng Bin <zhengbin13@huawei.com>
> 
> Sam Ravnborg (22):
>       backlight: Silently fail backlight_update_status() if no device
>       backlight: Add DECLARE_* macro for device registration
>       backlight: Add get/set operations for brightness/power properties
>       backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters
>       drm/gma500: Backlight support
>       drm/panel: asus-z00t-tm5p5-n35596: Backlight update
>       drm/panel: jdi-lt070me05000: Backlight update
>       drm/panel: novatek-nt35510: Backlight update
>       drm/panel: orisetech-otm8009a: Backlight update
>       drm/panel: raydium-rm67191: Backlight update
>       drm/panel: samsung-s6e63m0: Backlight update
>       drm/panel: samsung-s6e63j0x03: Backlight update
>       drm/panel: samsung-s6e3ha2: Backlight update
>       drm/panel: sony-acx424akp: Backlight update
>       drm/panel: sony-acx565akm: Backlight update
>       drm/bridge: parade-ps8622: Backlight update
>       drm/tilcdc: Backlight update
>       drm/radeon: Backlight update
>       drm/amdgpu/atom: Backlight update
>       drm/i915: Backlight update
>       drm/omap: display: Backlight update
>       drm/shmobile: Backlight update
> 
>  drivers/gpu/drm/amd/amdgpu/atombios_encoders.c     |  15 ++-
>  drivers/gpu/drm/bridge/parade-ps8622.c             |  43 ++++----
>  drivers/gpu/drm/gma500/backlight.c                 |  35 ++----
>  drivers/gpu/drm/gma500/cdv_device.c                |  29 +++--
>  drivers/gpu/drm/gma500/mdfld_device.c              |   9 +-
>  drivers/gpu/drm/gma500/oaktrail_device.c           |  10 +-
>  drivers/gpu/drm/gma500/opregion.c                  |   2 +-
>  drivers/gpu/drm/gma500/psb_device.c                |  10 +-
>  drivers/gpu/drm/gma500/psb_drv.c                   |   8 +-
>  drivers/gpu/drm/i915/display/intel_panel.c         |  88 +++++++--------
>  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  35 ++----
>  .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
>  drivers/gpu/drm/panel/panel-jdi-lt070me05000.c     |  17 ++-
>  drivers/gpu/drm/panel/panel-novatek-nt35510.c      |   9 +-
>  drivers/gpu/drm/panel/panel-orisetech-otm8009a.c   |  14 +--
>  drivers/gpu/drm/panel/panel-raydium-rm67191.c      |  11 +-
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      |  68 ++++++------
>  drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   |  56 +++++-----
>  drivers/gpu/drm/panel/panel-samsung-s6e63m0.c      |  25 ++---
>  drivers/gpu/drm/panel/panel-sony-acx424akp.c       |  49 ++-------
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c       |  44 +++-----
>  drivers/gpu/drm/radeon/atombios_encoders.c         |  23 ++--
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c    |  15 ++-
>  drivers/gpu/drm/shmobile/shmob_drm_backlight.c     |  20 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c              |  11 +-
>  drivers/video/backlight/gpio_backlight.c           |  17 ++-
>  include/linux/backlight.h                          | 120 +++++++++++++++++++++
>  27 files changed, 377 insertions(+), 421 deletions(-)
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel