mbox series

[v2,0/24] backlight: add init macros and accessors

Message ID 20200823104532.1024798-1-sam@ravnborg.org (mailing list archive)
Headers show
Series backlight: add init macros and accessors | expand

Message

Sam Ravnborg Aug. 23, 2020, 10:45 a.m. UTC
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(-)

Comments

Linus Walleij Aug. 28, 2020, 9:40 a.m. UTC | #1
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
Daniel Thompson Sept. 2, 2020, 11:29 a.m. UTC | #2
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.