2023-09-12 07:51:16

by Jani Nikula

[permalink] [raw]
Subject: Re: linux-next: Tree for Sep 11 (drivers/gpu/drm/i915/display/intel_backlight.o)

On Mon, 11 Sep 2023, Randy Dunlap <[email protected]> wrote:
> On 9/10/23 19:11, Stephen Rothwell wrote:
>> Hi all,
>>
>> Please do *not* include material destined for v6.7 in your linux-next
>> included branches until *after* v6.6-rc1 has been released. Also,
>> do *not* rebase your linu-next included branches onto v6.5.
>>
>> Changes since 20230908:
>>
>> Non-merge commits (relative to Linus' tree): 643
>> 614 files changed, 227990 insertions(+), 9502 deletions(-)
>>
>> ----------------------------------------------------------------------------
>
> on x86_64:
>
> # CONFIG_ACPI is not set
> CONFIG_DRM_I915=y
> CONFIG_BACKLIGHT_CLASS_DEVICE=m
>
> I915 selects BACKLIGHT_CLASS_DEVICE if ACPI is set.
>
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
> intel_backlight.c:(.text+0x4988): undefined reference to `backlight_device_get_by_name'
> ld: intel_backlight.c:(.text+0x4a1b): undefined reference to `backlight_device_register'
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
> intel_backlight.c:(.text+0x4b56): undefined reference to `backlight_device_unregister'

This comes up periodically. The fix is for i915 to depend on backlight,
but it's not possible to fix just i915, as it'll lead to circular deps
unless *all* select backlight is switched to depend on backlight.

I've gone through it once [1], and not keen on doing it again unless
there's buy-in.

IS_REACHABLE() is often suggested as a workaround, but I think it's just
plain wrong. i915=y backlight=m is not a configuration that makes
sense. Kernel configuration is hard enough, there's no point in allowing
dumb configs that just silently don't work.


BR,
Jani.


[1] https://lore.kernel.org/r/[email protected]



--
Jani Nikula, Intel


2023-09-12 20:52:55

by Randy Dunlap

[permalink] [raw]
Subject: Re: linux-next: Tree for Sep 11 (drivers/gpu/drm/i915/display/intel_backlight.o)



On 9/12/23 00:47, Jani Nikula wrote:
> On Mon, 11 Sep 2023, Randy Dunlap <[email protected]> wrote:
>> On 9/10/23 19:11, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> Please do *not* include material destined for v6.7 in your linux-next
>>> included branches until *after* v6.6-rc1 has been released. Also,
>>> do *not* rebase your linu-next included branches onto v6.5.
>>>
>>> Changes since 20230908:
>>>
>>> Non-merge commits (relative to Linus' tree): 643
>>> 614 files changed, 227990 insertions(+), 9502 deletions(-)
>>>
>>> ----------------------------------------------------------------------------
>>
>> on x86_64:
>>
>> # CONFIG_ACPI is not set
>> CONFIG_DRM_I915=y
>> CONFIG_BACKLIGHT_CLASS_DEVICE=m
>>
>> I915 selects BACKLIGHT_CLASS_DEVICE if ACPI is set.
>>
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>> intel_backlight.c:(.text+0x4988): undefined reference to `backlight_device_get_by_name'
>> ld: intel_backlight.c:(.text+0x4a1b): undefined reference to `backlight_device_register'
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>> intel_backlight.c:(.text+0x4b56): undefined reference to `backlight_device_unregister'
>
> This comes up periodically. The fix is for i915 to depend on backlight,
> but it's not possible to fix just i915, as it'll lead to circular deps
> unless *all* select backlight is switched to depend on backlight.
>
> I've gone through it once [1], and not keen on doing it again unless
> there's buy-in.
>
> IS_REACHABLE() is often suggested as a workaround, but I think it's just
> plain wrong. i915=y backlight=m is not a configuration that makes
> sense. Kernel configuration is hard enough, there's no point in allowing
> dumb configs that just silently don't work.
>

Yes, IS_REACHABLE() is just fugly nonsense.

Thanks for the reminder of your attempt(s).

>
> BR,
> Jani.
>
>
> [1] https://lore.kernel.org/r/[email protected]
>
>
>

--
~Randy

2023-09-13 07:47:32

by Randy Dunlap

[permalink] [raw]
Subject: Re: linux-next: Tree for Sep 11 (drivers/gpu/drm/i915/display/intel_backlight.o)

Hi Jani,

On 9/12/23 07:52, Randy Dunlap wrote:
>
>
> On 9/12/23 00:47, Jani Nikula wrote:
>> On Mon, 11 Sep 2023, Randy Dunlap <[email protected]> wrote:
>>> On 9/10/23 19:11, Stephen Rothwell wrote:
>>>> Hi all,
>>>>
>>>> Please do *not* include material destined for v6.7 in your linux-next
>>>> included branches until *after* v6.6-rc1 has been released. Also,
>>>> do *not* rebase your linu-next included branches onto v6.5.
>>>>
>>>> Changes since 20230908:
>>>>
>>>> Non-merge commits (relative to Linus' tree): 643
>>>> 614 files changed, 227990 insertions(+), 9502 deletions(-)
>>>>
>>>> ----------------------------------------------------------------------------
>>>
>>> on x86_64:
>>>
>>> # CONFIG_ACPI is not set
>>> CONFIG_DRM_I915=y
>>> CONFIG_BACKLIGHT_CLASS_DEVICE=m
>>>
>>> I915 selects BACKLIGHT_CLASS_DEVICE if ACPI is set.
>>>
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>>> intel_backlight.c:(.text+0x4988): undefined reference to `backlight_device_get_by_name'
>>> ld: intel_backlight.c:(.text+0x4a1b): undefined reference to `backlight_device_register'
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>>> intel_backlight.c:(.text+0x4b56): undefined reference to `backlight_device_unregister'
>>
>> This comes up periodically. The fix is for i915 to depend on backlight,
>> but it's not possible to fix just i915, as it'll lead to circular deps
>> unless *all* select backlight is switched to depend on backlight.
>>
>> I've gone through it once [1], and not keen on doing it again unless
>> there's buy-in.
>>
>> IS_REACHABLE() is often suggested as a workaround, but I think it's just
>> plain wrong. i915=y backlight=m is not a configuration that makes
>> sense. Kernel configuration is hard enough, there's no point in allowing
>> dumb configs that just silently don't work.
>>
>
> Yes, IS_REACHABLE() is just fugly nonsense.
>
> Thanks for the reminder of your attempt(s).
>
>>
>> BR,
>> Jani.
>>
>>
>> [1] https://lore.kernel.org/r/[email protected]

I did a partial patch series (eliminated the I915 problems with 9 patches,
without build testing -- only kconfig testing -- so more changes may be
needed), then I looked at your patch [1] above.

I like it but even if Tomi and Daniel didn't have problems with it,
I am concerned that it would cause problems with existing working .config files.

Still, something should be done about the mixed usage of select and depends on
for BACKLIGHT_CLASS_DEVICE (et al).

thanks.
--
~Randy