Re: [Intel-gfx] [RFC] drm/i915/backlight: switch to unique backlight device names

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 07, 2021 at 12:08:12PM +0300, Jani Nikula wrote:
> Change the backlight device names from intel_backlight to
> card%d-%s-backlight format, for example card0-eDP-1-backlight, to make
> them unique. Otherwise, registering multiple backlight devices with
> intel_backlight name will fail, regardless of whether they're two
> connectors in the same device or two different devices.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2794
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> Is the name "intel_backlight" part of the UABI? In theory the userspace
> should only look at the names and types available under
> /sys/class/backlight, not the exact names.

Quick glance at some things suggest this might affect some
custom setups people have.

There is an xorg.conf option in the intel ddx to force the
backlight device selection. But I didn't see any hardcoding
of the backlight device name, so unless the user has
configured that knob things should keep working I think.

Also googling says systemd might have some backlight
name to service name magic going on. But I was too lazy
to actually read the code so don't know the details.

> ---
>  drivers/gpu/drm/i915/display/intel_panel.c | 29 ++++++++++++++--------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index 10022d1575e1..5ebf8cb0e8e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -1372,6 +1372,8 @@ int intel_backlight_device_register(struct intel_connector *connector)
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	struct backlight_properties props;
> +	const char *name;
> +	int ret = 0;
>  
>  	if (WARN_ON(panel->backlight.device))
>  		return -ENODEV;
> @@ -1398,28 +1400,33 @@ int intel_backlight_device_register(struct intel_connector *connector)
>  	else
>  		props.power = FB_BLANK_POWERDOWN;
>  
> -	/*
> -	 * Note: using the same name independent of the connector prevents
> -	 * registration of multiple backlight devices in the driver.
> -	 */
> +	name = kasprintf(GFP_KERNEL, "card%d-%s-backlight", i915->drm.primary->index,
> +			 connector->base.name);
> +	if (!name)
> +		return -ENOMEM;
> +
>  	panel->backlight.device =
> -		backlight_device_register("intel_backlight",
> +		backlight_device_register(name,
>  					  connector->base.kdev,
>  					  connector,
>  					  &intel_backlight_device_ops, &props);
>  
>  	if (IS_ERR(panel->backlight.device)) {
> -		drm_err(&i915->drm, "Failed to register backlight: %ld\n",
> -			PTR_ERR(panel->backlight.device));
> +		drm_err(&i915->drm, "Failed to register backlight %s: %ld\n",
> +			name, PTR_ERR(panel->backlight.device));
>  		panel->backlight.device = NULL;
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out;
>  	}
>  
>  	drm_dbg_kms(&i915->drm,
> -		    "Connector %s backlight sysfs interface registered\n",
> -		    connector->base.name);
> +		    "Connector %s backlight sysfs interface %s registered\n",
> +		    name, connector->base.name);
>  
> -	return 0;
> +out:
> +	kfree(name);
> +
> +	return ret;
>  }
>  
>  void intel_backlight_device_unregister(struct intel_connector *connector)
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx