On Wed, 28 Apr 2021, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> Thanks, Jani. > >> >> v2: Keep using intel_backlight name for first backlight device >> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2794 >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
