On Sat, Apr 07, 2018 at 12:06:53AM -0700, Abhinav Kumar wrote:
> Register truly panel as a backlight led device and
> provide methods to control its backlight operation.
>
> Signed-off-by: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/panel/panel-truly-dual-dsi.c | 96 +++++++++++++++++++++++++++-
> 1 file changed, 94 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-truly-dual-dsi.c b/drivers/gpu/drm/panel/panel-truly-dual-dsi.c
> index 47891ee..5d0ef90 100644
> --- a/drivers/gpu/drm/panel/panel-truly-dual-dsi.c
> +++ b/drivers/gpu/drm/panel/panel-truly-dual-dsi.c
> @@ -14,6 +14,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/regulator/consumer.h>
> #include <linux/pinctrl/consumer.h>
> +#include <linux/leds.h>
Includes should be alphabetical.
>
> #include <video/mipi_display.h>
> #include <video/of_videomode.h>
> @@ -23,6 +24,9 @@
> #include <drm/drm_panel.h>
> #include <drm/drm_mipi_dsi.h>
>
> +#define BL_NODE_NAME_SIZE 32
> +#define PRIM_DISPLAY_NODE 0
> +
> struct truly_wqxga {
> struct device *dev;
> struct drm_panel panel;
> @@ -33,6 +37,8 @@ struct truly_wqxga {
> struct gpio_desc *mode_gpio;
>
> struct backlight_device *backlight;
> + /* WLED params */
> + struct led_trigger *wled;
> struct videomode vm;
>
> struct mipi_dsi_device *dsi[2];
> @@ -447,6 +453,83 @@ static void truly_wqxga_panel_del(struct truly_wqxga *ctx)
> put_device(&ctx->dsi[1]->dev);
> }
>
> +static int truly_backlight_device_update_status(struct backlight_device *bd)
> +{
> + int brightness;
> + int max_brightness;
> + int rc = 0;
> +
extra line
> + struct truly_wqxga *ctx = dev_get_drvdata(&bd->dev);
> +
> + brightness = bd->props.brightness;
> + max_brightness = bd->props.max_brightness;
> +
> + if ((bd->props.power != FB_BLANK_UNBLANK) ||
> + (bd->props.state & BL_CORE_FBBLANK) ||
> + (bd->props.state & BL_CORE_SUSPENDED))
> + brightness = 0;
> +
> + if (brightness > max_brightness)
> + brightness = max_brightness;
> +
> + /* Need to check WLED driver capability upstream */
> + if (ctx && ctx->wled)
ctx can't be NULL, so no need to check for that. And if ctx->wled is null, it
doesn't seem like this function will do anything. So how about just not
registering the backlight if wled == NULL (if that's possible).
> + led_trigger_event(ctx->wled, brightness);
> +
> + return rc;
> +}
> +
> +static int truly_backlight_device_get_brightness(struct backlight_device *bd)
> +{
> + return bd->props.brightness;
> +}
> +
> +static const struct backlight_ops truly_backlight_device_ops = {
> + .update_status = truly_backlight_device_update_status,
> + .get_brightness = truly_backlight_device_get_brightness,
> +};
> +
> +static int truly_backlight_setup(struct truly_wqxga *ctx)
> +{
> + struct backlight_properties props;
> + char bl_node_name[BL_NODE_NAME_SIZE];
> +
> + if (!ctx) {
> + dev_err(ctx->dev, "invalid context\n");
> + return -EINVAL;
> + }
This can't happen.
> +
> + if (!ctx->backlight) {
> + memset(&props, 0, sizeof(props));
> + props.type = BACKLIGHT_RAW;
> + props.power = FB_BLANK_UNBLANK;
> + props.max_brightness = 4096;
> +
> + snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
> + PRIM_DISPLAY_NODE);
Given that PRIM_DISPLAY_NODE is always 0, this seems like overkill for a pretty
generic name "panel0-backlight". So let's just call it "truly_backlight" in the
register call.
> +
> + ctx->backlight = backlight_device_register(bl_node_name,
> + ctx->dev, ctx,
> + &truly_backlight_device_ops, &props);
> +
> + if (IS_ERR_OR_NULL(ctx->backlight)) {
> + pr_err("Failed to register backlight\n");
> + ctx->backlight = NULL;
> + return -ENODEV;
> + }
> +
> + /* Register with the LED driver interface */
> + led_trigger_register_simple("bkl-trigger", &ctx->wled);
> +
> + if (!ctx->wled) {
> + pr_err("backlight led registration failed\n");
> + return -ENODEV;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
> {
> struct device *dev = &dsi->dev;
> @@ -466,10 +549,11 @@ static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
> secondary = of_find_mipi_dsi_device_by_node(np);
> of_node_put(np);
>
> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +
Why move this?
> if (!secondary)
> return -EPROBE_DEFER;
>
> - ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> if (!ctx) {
> put_device(&secondary->dev);
> return -ENOMEM;
> @@ -485,6 +569,12 @@ static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
> put_device(&secondary->dev);
> return ret;
> }
> +
> + ret = truly_backlight_setup(ctx);
> + if (ret) {
> + put_device(&secondary->dev);
> + return ret;
> + }
> }
>
> ret = mipi_dsi_attach(dsi);
> @@ -504,8 +594,10 @@ static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
> mipi_dsi_detach(dsi);
>
> /* delete panel only for the DSI1 interface */
> - if (ctx)
> + if (ctx) {
> truly_wqxga_panel_del(ctx);
> + kfree(ctx);
> + }
>
> return 0;
> }
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
