AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/24] backlight: add init macros and accessors
@ 2020-08-23 10:45 Sam Ravnborg
  2020-08-23 10:45 ` [PATCH v2 20/24] drm/radeon: Backlight update Sam Ravnborg
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Sam Ravnborg @ 2020-08-23 10:45 UTC (permalink / raw)
  To: dri-devel, Jingoo Han, Lee Jones, Daniel Thompson
  Cc: Neil Armstrong, Daniel Vetter, Linus Walleij, Joonas Lahtinen,
	Chris Wilson, Andrzej Hajda, Thierry Reding, Laurent Pinchart,
	Sam Ravnborg, Sebastian Reichel, Ville Syrjälä,
	Manasi Navare, Konrad Dybcio, amd-gfx, Zheng Bin, Tomi Valkeinen,
	Ezequiel Garcia, Robert Chiras, Vinay Simha BN, Hoegeun Kwon,
	Paweł Chmiel, Jonas Karlman, Maarten Lankhorst, Jani Nikula,
	Inki Dae, Hans de Goede, Jyri Sarha, Rodrigo Vivi, Jernej Skrabec,
	Philippe CORNU, Patrik Jakobsson, linux-renesas-soc, Rob Clark,
	Kieran Bingham, Alex Deucher, Wambui Karuga, Christian König

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

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.

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 access methods can be called with a NULL pointer so logic around the
  access can be made simpler.
- The update_brightness and enable_brightness simplifies the users
- The code is in most cases more readable with the access operations.
- When everyone uses the access methods refactoring in the backlight core is simpler.

The get/set operations are introduced in patch 3.

The gpio backlight driver received a small overhaul in a set of three patches.
The result is a smaller and more readable driver.

The remaining patches updates all backlight users in drivers/gpu/drm/*
With this patch set all of drivers/gpu/drm/:
- All backlight references to FB_BLANK* are gone from drm/*
- All direct references to backlight properties are gone
- All panel drivers uses the devm_ variant for registering backlight
  Daniel Vetter had some concerns with this for future updates,
  but we are aligned now and can update if refoctoring demands it
- All panel drivers uses the backlight support in drm_panel

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.

v2:
  - Documented BACKLIGHT_PROPS as it may be used by drivers
  - Dropped backlight_set_power_{on,off}, they were a mistake (Daniel)
  - Added backlight_update_brightness() and use it (Daniel)
  - Added backlight_enable_brightness() and use it
  - Moved remaining drm_panel driver to use backlight support in drm_panel
  - gpio backlight driver overhaul

The patches are made on top of the for-backlight-next branch at
https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git
The branch needs v5.8-rc1 backported to build as dev_err_probe()
is used.

The first 6 patches are candidates for the backlight tree.
If they are applied then this should preferably be to an immutable
branch we can merge to drm-misc-next where the drm patches shall go.

The drm patches has known conflics and shall *not* be applied to the
backlight tree, they are included in this patchset to show how the
new functions are used.

Diffstat for the drm bits alone looks nice:
 25 files changed, 243 insertions(+), 460 deletions(-)

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 (24):
      backlight: Silently fail backlight_update_status() if no device
      backlight: Add DECLARE_* macro for device registration
      backlight: Add get/set operations for brightness properties
      backlight: gpio: Introduce backlight_{enable,disable}
      backlight: gpio: Use dev_err_probe()
      backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW
      drm/gma500: Backlight update
      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     |  19 +--
 drivers/gpu/drm/bridge/parade-ps8622.c             |  43 ++++---
 drivers/gpu/drm/gma500/backlight.c                 |  34 +----
 drivers/gpu/drm/gma500/cdv_device.c                |  24 ++--
 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                   |   7 +-
 drivers/gpu/drm/i915/display/intel_panel.c         |  88 ++++++-------
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  37 +-----
 .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c     |  54 ++------
 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      |  70 +++++------
 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   |  53 ++++----
 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       |  47 +++----
 drivers/gpu/drm/radeon/atombios_encoders.c         |  24 ++--
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c    |  16 +--
 drivers/gpu/drm/shmobile/shmob_drm_backlight.c     |  20 ++-
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              |   9 +-
 drivers/video/backlight/gpio_backlight.c           |  41 +++---
 include/linux/backlight.h                          | 140 +++++++++++++++++++++
 27 files changed, 400 insertions(+), 480 deletions(-)


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 20/24] drm/radeon: Backlight update
  2020-08-23 10:45 [PATCH v2 0/24] backlight: add init macros and accessors Sam Ravnborg
@ 2020-08-23 10:45 ` Sam Ravnborg
  2020-08-23 10:45 ` [PATCH v2 21/24] drm/amdgpu/atom: " Sam Ravnborg
  2020-08-28  9:40 ` [PATCH v2 0/24] backlight: add init macros and accessors Linus Walleij
  2 siblings, 0 replies; 5+ messages in thread
From: Sam Ravnborg @ 2020-08-23 10:45 UTC (permalink / raw)
  To: dri-devel, Jingoo Han, Lee Jones, Daniel Thompson
  Cc: Alex Deucher, Daniel Vetter, Sam Ravnborg, Christian König,
	amd-gfx

- 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    | 24 +++++++++----------
 .../gpu/drm/radeon/radeon_legacy_encoders.c   | 16 +++++--------
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index cc5ee1b3af84..300bec94dbff 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,16 +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;
-	backlight_update_status(bd);
+
+	if (brightness == 0)
+		brightness = RADEON_MAX_BL_LEVEL;
+
+	backlight_enable_brightness(bd, brightness);
 
 	DRM_INFO("radeon atom DIG backlight initialized\n");
 	rdev->mode_info.bl_encoder = radeon_encoder;
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 44d060f75318..55e656acaedb 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,10 +372,10 @@ 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;
-	struct backlight_properties props;
 	struct radeon_backlight_privdata *pdata;
 	uint8_t backlight_level;
 	char bl_name[16];
@@ -394,9 +395,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,9 +441,7 @@ 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_update_status(bd);
+	backlight_enable_brightness(bd, radeon_legacy_backlight_get_brightness(bd));
 
 	DRM_INFO("radeon legacy LVDS backlight initialized\n");
 	rdev->mode_info.bl_encoder = radeon_encoder;
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 21/24] drm/amdgpu/atom: Backlight update
  2020-08-23 10:45 [PATCH v2 0/24] backlight: add init macros and accessors Sam Ravnborg
  2020-08-23 10:45 ` [PATCH v2 20/24] drm/radeon: Backlight update Sam Ravnborg
@ 2020-08-23 10:45 ` Sam Ravnborg
  2020-08-28  9:40 ` [PATCH v2 0/24] backlight: add init macros and accessors Linus Walleij
  2 siblings, 0 replies; 5+ messages in thread
From: Sam Ravnborg @ 2020-08-23 10:45 UTC (permalink / raw)
  To: dri-devel, Jingoo Han, Lee Jones, Daniel Thompson
  Cc: Alex Deucher, Daniel Vetter, Sam Ravnborg, Christian König,
	amd-gfx

- 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>
---
 .../gpu/drm/amd/amdgpu/atombios_encoders.c    | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index 1e94a9b652f7..882e1a3dad8b 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,13 +166,12 @@ 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;
-	struct backlight_properties props;
 	struct amdgpu_backlight_privdata *pdata;
 	struct amdgpu_encoder_atom_dig *dig;
-	u8 backlight_level;
 	char bl_name[16];
 
 	/* Mac laptops with multiple GPUs use the gmux driver for backlight
@@ -193,9 +193,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,
@@ -207,14 +204,10 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode
 
 	pdata->encoder = amdgpu_encoder;
 
-	backlight_level = amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
-
 	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_update_status(bd);
+	backlight_enable_brightness(bd, amdgpu_atombios_encoder_get_backlight_brightness(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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 0/24] backlight: add init macros and accessors
  2020-08-23 10:45 [PATCH v2 0/24] backlight: add init macros and accessors Sam Ravnborg
  2020-08-23 10:45 ` [PATCH v2 20/24] drm/radeon: Backlight update Sam Ravnborg
  2020-08-23 10:45 ` [PATCH v2 21/24] drm/amdgpu/atom: " Sam Ravnborg
@ 2020-08-28  9:40 ` Linus Walleij
  2020-09-02 11:29   ` Daniel Thompson
  2 siblings, 1 reply; 5+ messages in thread
From: Linus Walleij @ 2020-08-28  9:40 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Neil Armstrong, Daniel Vetter, Joonas Lahtinen,
	open list:DRM PANEL DRIVERS, Chris Wilson, Andrzej Hajda,
	Thierry Reding, Laurent Pinchart, Lee Jones, Sebastian Reichel,
	Ville Syrjälä, Manasi Navare, Daniel Thompson,
	Konrad Dybcio, amd-gfx, Zheng Bin, Tomi Valkeinen,
	Ezequiel Garcia, Robert Chiras, Vinay Simha BN, Hoegeun Kwon,
	Paweł Chmiel, Jonas Karlman, Maarten Lankhorst, Jani Nikula,
	Inki Dae, Hans de Goede, Jyri Sarha, Rodrigo Vivi, Jernej Skrabec,
	Jingoo Han, Philippe CORNU, Patrik Jakobsson, Linux-Renesas,
	Rob Clark, Kieran Bingham, Alex Deucher, Wambui Karuga,
	Christian König

On Sun, Aug 23, 2020 at 12:45 PM Sam Ravnborg <sam@ravnborg.org> wrote:

> The first patch trims backlight_update_status() so it can be called with a NULL
> backlight_device. Then the caller do not need to add this check just to avoid
> a NULL reference.
>
> 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.
>
> 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 access methods can be called with a NULL pointer so logic around the
>   access can be made simpler.
> - The update_brightness and enable_brightness simplifies the users
> - The code is in most cases more readable with the access operations.
> - When everyone uses the access methods refactoring in the backlight core is simpler.
>
> The get/set operations are introduced in patch 3.
>
> The gpio backlight driver received a small overhaul in a set of three patches.
> The result is a smaller and more readable driver.
>
> The remaining patches updates all backlight users in drivers/gpu/drm/*
> With this patch set all of drivers/gpu/drm/:
> - All backlight references to FB_BLANK* are gone from drm/*
> - All direct references to backlight properties are gone
> - All panel drivers uses the devm_ variant for registering backlight
>   Daniel Vetter had some concerns with this for future updates,
>   but we are aligned now and can update if refoctoring demands it
> - All panel drivers uses the backlight support in drm_panel
>
> 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.
>
> v2:
>   - Documented BACKLIGHT_PROPS as it may be used by drivers
>   - Dropped backlight_set_power_{on,off}, they were a mistake (Daniel)
>   - Added backlight_update_brightness() and use it (Daniel)
>   - Added backlight_enable_brightness() and use it
>   - Moved remaining drm_panel driver to use backlight support in drm_panel
>   - gpio backlight driver overhaul
>
> The patches are made on top of the for-backlight-next branch at
> https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git
> The branch needs v5.8-rc1 backported to build as dev_err_probe()
> is used.
>
> The first 6 patches are candidates for the backlight tree.
> If they are applied then this should preferably be to an immutable
> branch we can merge to drm-misc-next where the drm patches shall go.
>
> The drm patches has known conflics and shall *not* be applied to the
> backlight tree, they are included in this patchset to show how the
> new functions are used.
>
> Diffstat for the drm bits alone looks nice:
>  25 files changed, 243 insertions(+), 460 deletions(-)
>
> Feedback welcome!

Thank you for trying to make backlight easier for developers.
I am a big supporter of this type of simplifications and
generalizations, it is what makes DRM great.

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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 0/24] backlight: add init macros and accessors
  2020-08-28  9:40 ` [PATCH v2 0/24] backlight: add init macros and accessors Linus Walleij
@ 2020-09-02 11:29   ` Daniel Thompson
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Thompson @ 2020-09-02 11:29 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Neil Armstrong, Daniel Vetter, Joonas Lahtinen,
	open list:DRM PANEL DRIVERS, Chris Wilson, Andrzej Hajda,
	Thierry Reding, Laurent Pinchart, Sam Ravnborg, Sebastian Reichel,
	Ville Syrjälä, Manasi Navare, Lee Jones, Konrad Dybcio,
	amd-gfx, Zheng Bin, Tomi Valkeinen, Ezequiel Garcia,
	Robert Chiras, Vinay Simha BN, Hoegeun Kwon, Paweł Chmiel,
	Jonas Karlman, Maarten Lankhorst, Jani Nikula, Inki Dae,
	Hans de Goede, Jyri Sarha, Rodrigo Vivi, Jernej Skrabec,
	Jingoo Han, Philippe CORNU, Patrik Jakobsson, Linux-Renesas,
	Rob Clark, Kieran Bingham, Alex Deucher, Wambui Karuga,
	Christian König

On Fri, Aug 28, 2020 at 11:40:28AM +0200, Linus Walleij wrote:
> On Sun, Aug 23, 2020 at 12:45 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > The first patch trims backlight_update_status() so it can be called with a NULL
> > backlight_device. Then the caller do not need to add this check just to avoid
> > a NULL reference.
> >
> > 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.
> >
> > 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 access methods can be called with a NULL pointer so logic around the
> >   access can be made simpler.
> > - The update_brightness and enable_brightness simplifies the users
> > - The code is in most cases more readable with the access operations.
> > - When everyone uses the access methods refactoring in the backlight core is simpler.
> >
> > The get/set operations are introduced in patch 3.
> >
> > The gpio backlight driver received a small overhaul in a set of three patches.
> > The result is a smaller and more readable driver.
> >
> > The remaining patches updates all backlight users in drivers/gpu/drm/*
> > With this patch set all of drivers/gpu/drm/:
> > - All backlight references to FB_BLANK* are gone from drm/*
> > - All direct references to backlight properties are gone
> > - All panel drivers uses the devm_ variant for registering backlight
> >   Daniel Vetter had some concerns with this for future updates,
> >   but we are aligned now and can update if refoctoring demands it
> > - All panel drivers uses the backlight support in drm_panel
> >
> > 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.
> >
> > v2:
> >   - Documented BACKLIGHT_PROPS as it may be used by drivers
> >   - Dropped backlight_set_power_{on,off}, they were a mistake (Daniel)
> >   - Added backlight_update_brightness() and use it (Daniel)
> >   - Added backlight_enable_brightness() and use it
> >   - Moved remaining drm_panel driver to use backlight support in drm_panel
> >   - gpio backlight driver overhaul
> >
> > The patches are made on top of the for-backlight-next branch at
> > https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git
> > The branch needs v5.8-rc1 backported to build as dev_err_probe()
> > is used.
> >
> > The first 6 patches are candidates for the backlight tree.
> > If they are applied then this should preferably be to an immutable
> > branch we can merge to drm-misc-next where the drm patches shall go.
> >
> > The drm patches has known conflics and shall *not* be applied to the
> > backlight tree, they are included in this patchset to show how the
> > new functions are used.
> >
> > Diffstat for the drm bits alone looks nice:
> >  25 files changed, 243 insertions(+), 460 deletions(-)
> >
> > Feedback welcome!
> 
> Thank you for trying to make backlight easier for developers.
> I am a big supporter of this type of simplifications and
> generalizations, it is what makes DRM great.

+1!

I've reviewed and sent out patch by patch replies for the backlight
patches.

I've eyeballed the drm patches but not reviewed at same depth
and FWIW for all the patches whose subject *doesn't* start with
backlight then:
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-02 13:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-23 10:45 [PATCH v2 0/24] backlight: add init macros and accessors Sam Ravnborg
2020-08-23 10:45 ` [PATCH v2 20/24] drm/radeon: Backlight update Sam Ravnborg
2020-08-23 10:45 ` [PATCH v2 21/24] drm/amdgpu/atom: " Sam Ravnborg
2020-08-28  9:40 ` [PATCH v2 0/24] backlight: add init macros and accessors Linus Walleij
2020-09-02 11:29   ` Daniel Thompson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox