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(-)
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);
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 *
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,
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
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
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
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);
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,
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);
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
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;
- 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; }
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
- 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; }
- 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);
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
- 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; }
- 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; }
- 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; }
- 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)
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
- 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); }
/* -----------------------------------------------------------------------------
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);
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
- 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;
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-mac... 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
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));@@ -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
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,
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-mac... 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
- 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");
- 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");
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);
- 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);
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
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
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
- 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;
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(-)
dri-devel@lists.freedesktop.org
-
daniel＠ffwll.ch -
kernel test robot -
Linus Walleij -
Sam Ravnborg -
Sebastian Reichel