On Wed, 28 Apr 2021, Ville Syrjälä <ville.syrj...@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.syrj...@linux.intel.com>

Thanks,
Jani.

>
>> 
>> v2: Keep using intel_backlight name for first backlight device
>> 
>> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2794
>> Signed-off-by: Jani Nikula <jani.nik...@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

Reply via email to