drm/dp: drm_edp_backlight_set_level: do not always send 3-byte commands

[PATCH] drm/dp: drm_edp_backlight_set_level: do not always send 3-byte commands

Posted by Val Packett 3 months ago

At least some panels using the LSB register are not happy with the
unconditional increase of the command buffer to 3 bytes.

With the BOE NE14QDM in my Dell Latitude 7455, the recent patches for
luminance based brightness have introduced a regression: the brightness
range stopped being contiguous and became nonsensical (it probably was
interpreting the last 2 bytes of the buffer and not the first 2).

Change from using a fixed sizeof() to a length variable that's only
set to 3 when luminance is used. Let's leave the default as 2 even for
the single-byte version, since that's how it worked before.

Fixes: f2db78e37fe7 ("drm/dp: Modify drm_edp_backlight_set_level")
Signed-off-by: Val Packett <val@packett.cool>
---

Video evidence (haha): https://files.catbox.moe/sp1g9v.mp4

As this fix is tiny, if you prefer to fix it differently somehow, feel free to
just redo it yourselves without waiting for me to respin it :)

 drivers/gpu/drm/display/drm_dp_helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index db7896c7edb8..7eaa118d78c3 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -3962,6 +3962,7 @@ int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_bac
 	int ret;
 	unsigned int offset = DP_EDP_BACKLIGHT_BRIGHTNESS_MSB;
 	u8 buf[3] = { 0 };
+	size_t len = 2;
 
 	/* The panel uses the PWM for controlling brightness levels */
 	if (!(bl->aux_set || bl->luminance_set))
@@ -3974,6 +3975,7 @@ int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_bac
 		buf[1] = (level & 0x00ff00) >> 8;
 		buf[2] = (level & 0xff0000) >> 16;
 		offset = DP_EDP_PANEL_TARGET_LUMINANCE_VALUE;
+		len = 3;
 	} else if (bl->lsb_reg_used) {
 		buf[0] = (level & 0xff00) >> 8;
 		buf[1] = (level & 0x00ff);
@@ -3981,7 +3983,7 @@ int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_bac
 		buf[0] = level;
 	}
 
-	ret = drm_dp_dpcd_write_data(aux, offset, buf, sizeof(buf));
+	ret = drm_dp_dpcd_write_data(aux, offset, buf, len);
 	if (ret < 0) {
 		drm_err(aux->drm_dev,
 			"%s: Failed to write aux backlight level: %d\n",
-- 
2.49.0

Re: [PATCH] drm/dp: drm_edp_backlight_set_level: do not always send 3-byte commands

Posted by Dmitry Baryshkov 1 month, 2 weeks ago

On Sun, 06 Jul 2025 17:42:24 -0300, Val Packett wrote:
> At least some panels using the LSB register are not happy with the
> unconditional increase of the command buffer to 3 bytes.
> 
> With the BOE NE14QDM in my Dell Latitude 7455, the recent patches for
> luminance based brightness have introduced a regression: the brightness
> range stopped being contiguous and became nonsensical (it probably was
> interpreting the last 2 bytes of the buffer and not the first 2).
> 
> [...]

Applied to drm-misc-next, thanks!

[1/1] drm/dp: drm_edp_backlight_set_level: do not always send 3-byte commands
      commit: 4aa8961b1b9c7498550b41168a91cf1558133dd3

Best regards,
-- 
With best wishes
Dmitry

Re: [PATCH] drm/dp: drm_edp_backlight_set_level: do not always send 3-byte commands

Posted by Abel Vesa 1 month, 2 weeks ago

On 25-07-06 17:42:24, Val Packett wrote:
> At least some panels using the LSB register are not happy with the
> unconditional increase of the command buffer to 3 bytes.
> 
> With the BOE NE14QDM in my Dell Latitude 7455, the recent patches for
> luminance based brightness have introduced a regression: the brightness
> range stopped being contiguous and became nonsensical (it probably was
> interpreting the last 2 bytes of the buffer and not the first 2).
> 
> Change from using a fixed sizeof() to a length variable that's only
> set to 3 when luminance is used. Let's leave the default as 2 even for
> the single-byte version, since that's how it worked before.
> 
> Fixes: f2db78e37fe7 ("drm/dp: Modify drm_edp_backlight_set_level")
> Signed-off-by: Val Packett <val@packett.cool>

Tested on Dell XPS 13 9345 which has LGD 134WT1 panel.
Without this, the panel doesn't come up at all on boot.

Tested-by: Abel Vesa <abel.vesa@linaro.org>

RE: [PATCH] drm/dp: drm_edp_backlight_set_level: do not always send 3-byte commands

Posted by Kandpal, Suraj 3 months ago


> -----Original Message-----
> From: Val Packett <val@packett.cool>
> Sent: Monday, July 7, 2025 2:12 AM
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>;
> David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Kandpal,
> Suraj <suraj.kandpal@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
> Cc: Val Packett <val@packett.cool>; Nautiyal, Ankit K
> <ankit.k.nautiyal@intel.com>; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] drm/dp: drm_edp_backlight_set_level: do not always send
> 3-byte commands
> 
> At least some panels using the LSB register are not happy with the
> unconditional increase of the command buffer to 3 bytes.
> 
> With the BOE NE14QDM in my Dell Latitude 7455, the recent patches for
> luminance based brightness have introduced a regression: the brightness
> range stopped being contiguous and became nonsensical (it probably was
> interpreting the last 2 bytes of the buffer and not the first 2).
> 
> Change from using a fixed sizeof() to a length variable that's only set to 3
> when luminance is used. Let's leave the default as 2 even for the single-byte
> version, since that's how it worked before.
> 
> Fixes: f2db78e37fe7 ("drm/dp: Modify drm_edp_backlight_set_level")
> Signed-off-by: Val Packett <val@packett.cool>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
> 
> Video evidence (haha): https://files.catbox.moe/sp1g9v.mp4
> 
> As this fix is tiny, if you prefer to fix it differently somehow, feel free to just
> redo it yourselves without waiting for me to respin it :)
> 
>  drivers/gpu/drm/display/drm_dp_helper.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index db7896c7edb8..7eaa118d78c3 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -3962,6 +3962,7 @@ int drm_edp_backlight_set_level(struct
> drm_dp_aux *aux, const struct drm_edp_bac
>  	int ret;
>  	unsigned int offset = DP_EDP_BACKLIGHT_BRIGHTNESS_MSB;
>  	u8 buf[3] = { 0 };
> +	size_t len = 2;
> 
>  	/* The panel uses the PWM for controlling brightness levels */
>  	if (!(bl->aux_set || bl->luminance_set)) @@ -3974,6 +3975,7 @@ int
> drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct
> drm_edp_bac
>  		buf[1] = (level & 0x00ff00) >> 8;
>  		buf[2] = (level & 0xff0000) >> 16;
>  		offset = DP_EDP_PANEL_TARGET_LUMINANCE_VALUE;
> +		len = 3;
>  	} else if (bl->lsb_reg_used) {
>  		buf[0] = (level & 0xff00) >> 8;
>  		buf[1] = (level & 0x00ff);
> @@ -3981,7 +3983,7 @@ int drm_edp_backlight_set_level(struct
> drm_dp_aux *aux, const struct drm_edp_bac
>  		buf[0] = level;
>  	}
> 
> -	ret = drm_dp_dpcd_write_data(aux, offset, buf, sizeof(buf));
> +	ret = drm_dp_dpcd_write_data(aux, offset, buf, len);
>  	if (ret < 0) {
>  		drm_err(aux->drm_dev,
>  			"%s: Failed to write aux backlight level: %d\n",
> --
> 2.49.0