Re: [Intel-gfx] [PATCH 2/2] drm/i915/backlight: use unique backlight device names

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 28 Apr 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Apr 28, 2021 at 01:14:29PM +0300, Jani Nikula wrote:
>> Registering multiple backlight devices with intel_backlight name will
>> obviously fail, regardless of whether they're two connectors in the same
>> drm device or two different drm devices.
>> 
>> It would be preferrable to switch to completely unique names, and sunset
>> the generic intel_backlight name. However, there are apparently users
>> out there that hardcode the name, so the change would break backward
>> compatibility.
>> 
>> As a compromise, register the first device with intel_backlight name. In
>> the common case, this is the only backlight device anyway. From the
>> second device on, use card%d-%s-backlight format, for example
>> card0-eDP-2-backlight, to make the name unique.
>> 
>> This approach does not preclude us from registering the first device
>> using the same naming scheme in the future.
>
> "intel_backlight" symlink for the first backlight might be an option I
> guess.

I looked into it, but I think it would require changes in the backlight
driver. It's not very nice to poke there directly. And having this now
does not prevent us from adding that later.

> Series is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks,
Jani.

>
>> 
>> v2: Keep using intel_backlight name for first backlight device
>> 
>> 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>
>> ---
>>  drivers/gpu/drm/i915/display/intel_panel.c | 23 ++++++++++++++++++----
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
>> index 3088677ab8a7..a20761079ae0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
>> @@ -1401,16 +1401,31 @@ 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 = kstrdup("intel_backlight", GFP_KERNEL);
>>  	if (!name)
>>  		return -ENOMEM;
>>  
>>  	bd = backlight_device_register(name, connector->base.kdev, connector,
>>  				       &intel_backlight_device_ops, &props);
>> +
>> +	/*
>> +	 * Using the same name independent of the drm device or connector
>> +	 * prevents registration of multiple backlight devices in the
>> +	 * driver. However, we need to use the default name for backward
>> +	 * compatibility. Use unique names for subsequent backlight devices as a
>> +	 * fallback when the default name already exists.
>> +	 */
>> +	if (IS_ERR(bd) && PTR_ERR(bd) == -EEXIST) {
>> +		kfree(name);
>> +		name = kasprintf(GFP_KERNEL, "card%d-%s-backlight",
>> +				 i915->drm.primary->index, connector->base.name);
>> +		if (!name)
>> +			return -ENOMEM;
>> +
>> +		bd = backlight_device_register(name, connector->base.kdev, connector,
>> +					       &intel_backlight_device_ops, &props);
>> +	}
>> +
>>  	if (IS_ERR(bd)) {
>>  		drm_err(&i915->drm,
>>  			"[CONNECTOR:%d:%s] backlight device %s register failed: %ld\n",
>> -- 
>> 2.20.1

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx