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>,
	Konrad Dybcio <konradybcio@gmail.com>,
	dri-devel@lists.freedesktop.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH v1 06/22] drm/panel: asus-z00t-tm5p5-n35596: Backlight update
Date: Tue, 4 Aug 2020 18:59:18 +0200
Message-ID: <20200804165918.GN6419@phenom.ffwll.local> (raw)
In-Reply-To: <20200802110636.1018743-7-sam@ravnborg.org>

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

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

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

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

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

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

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

  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 [this message]
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
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=20200804165918.GN6419@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=konradybcio@gmail.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