Hi Linus,
On Thu, Jul 15, 2021 at 11:28:08AM +0200, Linus Walleij wrote:
> This converts the internal backlight in the Sony ACX424AKP
> driver to do it the canonical way:
>
> - Assign the panel->backlight during probe.
> - Let the panel framework handle the backlight.
> - Make the backlight .set_brightness() turn the backlight
> off completely if blank.
> - Fix some dev_err_probe() use cases along the way.
Very nice cleanup - thanks.
One issue below, with that fixed:
Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
I assume you will apply the patch yourself.
Sam
>
> Tested on the U8500 HREF520 reference design.
>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> drivers/gpu/drm/panel/panel-sony-acx424akp.c | 84 +++++++-------------
> 1 file changed, 28 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
> index 95659a4d15e9..163f0e0cee1c 100644
> --- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c
> +++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
> @@ -40,7 +40,6 @@
> struct acx424akp {
> struct drm_panel panel;
> struct device *dev;
> - struct backlight_device *bl;
> struct regulator *supply;
> struct gpio_desc *reset_gpio;
> bool video_mode;
> @@ -102,6 +101,20 @@ static int acx424akp_set_brightness(struct backlight_device *bl)
> u8 par;
> int ret;
>
> +
> + if (backlight_is_blank(bl)) {
> + /* Disable backlight */
> + par = 0x00;
> + ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
> + &par, 1);
> + if (ret) {
> + dev_err(acx->dev, "failed to disable display backlight (%d)\n", ret);
> + return ret;
> + }
> + return 0;
> + }
> +
> +
> /* Calculate the PWM duty cycle in n/256's */
> pwm_ratio = max(((duty_ns * 256) / period_ns) - 1, 1);
> pwm_div = max(1,
> @@ -172,6 +185,12 @@ static const struct backlight_ops acx424akp_bl_ops = {
> .update_status = acx424akp_set_brightness,
> };
>
> +static const struct backlight_properties acx424akp_bl_props = {
> + .type = BACKLIGHT_PLATFORM,
Other dsi panels uses BACKLIGHT_RAW here, which I think is more
correct. So unless there is good arguments for use of PLATFORM, please
change this to RAW.
It only influences how backlight is reported via sysfs, and there is no
functional change AFAICT.
Sam
