[PATCH 1/4] drm/gma500: Refactor backlight support

[Sam Ravnborg]

Hi Hans,

just a few minor things. See comments.
I like the diff - removes much more than it adds.

	Sam

On Sat, Sep 10, 2022 at 10:50:58PM +0200, Hans de Goede wrote:
> Refactor backlight support so that the gma_backlight_enable() /
> gma_backlight_disable() / gma_backlight_set() functions used by
> the Opregion handle will also work if no backlight_device gets
> registered.
> 
> This is a preparation patch for not registering the gma500's own backlight
> device when acpi_video should be used, since registering 2 backlight
> devices for a single display really is undesirable.
> 
> Since the acpi-video interface often uses the OpRegion we need to keep
> the OpRegion functional even when dev_priv->backlight_device is NULL.
> 
> As a result of this refactor the actual backlight_device_register()
> call is moved to the shared backlight.c code and all #ifdefs related to
> CONFIG_BACKLIGHT_CLASS_DEVICE are now also limited to backlight.c .
> 
> No functional changes intended.
> 
> This has been tested on a Packard Bell Dot SC (Intel Atom N2600, cedarview)
> and a Sony Vaio vpc-x11s1e (Intel N540, poulsbo) laptop.
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---

> +static int gma_backlight_update_status(struct backlight_device *bd)
> +{
> +	struct drm_device *dev = bl_get_data(bd);
> +	int level = bd->props.brightness;

Here backlight_get_brightness(bd) should be used.


>  int gma_backlight_init(struct drm_device *dev)
>  {
> -#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +	struct backlight_properties props = {};
> +	int ret;
> +
>  	dev_priv->backlight_enabled = true;
> -	return dev_priv->ops->backlight_init(dev);
> -#else
> -	return 0;
> +	dev_priv->backlight_level = 100;
> +
> +	ret = dev_priv->ops->backlight_init(dev);
> +	if (ret)
> +		return ret;
> +
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> +	props.brightness = dev_priv->backlight_level;
> +	props.max_brightness = PSB_MAX_BRIGHTNESS;
> +	props.type = BACKLIGHT_PLATFORM;
> +
> +	dev_priv->backlight_device =
> +		backlight_device_register(dev_priv->ops->backlight_name,
> +					  dev->dev, dev,
> +					  &gma_backlight_ops, &props);

Consider using the devm_backlight_device_register() variant.
Then you can drop gma_backlight_exit() - I think..

> +	if (IS_ERR(dev_priv->backlight_device))
> +		return PTR_ERR(dev_priv->backlight_device);
>  #endif
> +
> +	return 0;
>  }
>  
>  void gma_backlight_exit(struct drm_device *dev)
>  {
>  #ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -	if (dev_priv->backlight_device) {
> -		dev_priv->backlight_device->props.brightness = 0;
> -		backlight_update_status(dev_priv->backlight_device);
> +
> +	if (dev_priv->backlight_device)
>  		backlight_device_unregister(dev_priv->backlight_device);
> -	}
>  #endif
>  }