dri-devel Archive on lore.kernel.org
 help / color / Atom feed
From: daniel@ffwll.ch
Cc: Daniel Thompson <daniel.thompson@linaro.org>,
	Jingoo Han <jingoohan1@gmail.com>,
	dri-devel@lists.freedesktop.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH v1 15/22] drm/panel: sony-acx565akm: Backlight update
Date: Tue, 4 Aug 2020 19:09:59 +0200
Message-ID: <20200804170959.GP6419@phenom.ffwll.local> (raw)
In-Reply-To: <20200802110636.1018743-16-sam@ravnborg.org>

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

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

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

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

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

  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-02 11:06 [RFC PATCH v1 0/22] backlight: add init macros and accessors Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 01/22] backlight: Silently fail backlight_update_status() if no device Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 02/22] backlight: Add DECLARE_* macro for device registration Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 03/22] backlight: Add get/set operations for brightness/power properties Sam Ravnborg
2020-08-04 16:43   ` daniel
2020-08-04 19:56     ` Sam Ravnborg
2020-08-05  7:16       ` daniel
2020-08-02 11:06 ` [PATCH v1 04/22] backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 05/22] drm/gma500: Backlight support Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 06/22] drm/panel: asus-z00t-tm5p5-n35596: Backlight update Sam Ravnborg
2020-08-04 16:59   ` daniel
2020-08-02 11:06 ` [PATCH v1 07/22] drm/panel: jdi-lt070me05000: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 08/22] drm/panel: novatek-nt35510: " Sam Ravnborg
2020-08-04 21:29   ` Linus Walleij
2020-08-02 11:06 ` [PATCH v1 09/22] drm/panel: orisetech-otm8009a: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 10/22] drm/panel: raydium-rm67191: " Sam Ravnborg
2020-08-04 17:04   ` daniel
2020-08-02 11:06 ` [PATCH v1 11/22] drm/panel: samsung-s6e63m0: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 12/22] drm/panel: samsung-s6e63j0x03: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 13/22] drm/panel: samsung-s6e3ha2: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 14/22] drm/panel: sony-acx424akp: " Sam Ravnborg
2020-08-04 21:31   ` Linus Walleij
2020-08-02 11:06 ` [PATCH v1 15/22] drm/panel: sony-acx565akm: " Sam Ravnborg
2020-08-04 17:09   ` daniel [this message]
2020-08-02 11:06 ` [PATCH v1 16/22] drm/bridge: parade-ps8622: " Sam Ravnborg
2020-08-02 14:05   ` kernel test robot
2020-08-02 14:32   ` Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 17/22] drm/tilcdc: " Sam Ravnborg
2020-08-02 13:21   ` kernel test robot
2020-08-02 11:06 ` [PATCH v1 18/22] drm/radeon: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 19/22] drm/amdgpu/atom: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 20/22] drm/i915: " Sam Ravnborg
2020-08-02 11:06 ` [PATCH v1 21/22] drm/omap: display: " Sam Ravnborg
2020-08-02 14:26   ` Sebastian Reichel
2020-08-02 14:32     ` Sam Ravnborg
2020-08-02 22:48       ` Sebastian Reichel
2020-08-02 11:06 ` [PATCH v1 22/22] drm/shmobile: " Sam Ravnborg
2020-08-04 16:51 ` [RFC PATCH v1 0/22] backlight: add init macros and accessors daniel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200804170959.GP6419@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=daniel.thompson@linaro.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jingoohan1@gmail.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=lee.jones@linaro.org \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

dri-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dri-devel/0 dri-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dri-devel dri-devel/ https://lore.kernel.org/dri-devel \
		dri-devel@lists.freedesktop.org
	public-inbox-index dri-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.dri-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git