All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-04 10:28 ` Jiri Slaby (SUSE)
  0 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby (SUSE) @ 2022-10-04 10:28 UTC (permalink / raw)
  To: jani.nikula
  Cc: linux-kernel, Jiri Slaby (SUSE),
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
	Daniel Vetter, intel-gfx, dri-devel, Martin Liška

When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'

ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'

To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
with the above config, backlight support is disabled.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Reported-by: Martin Liška <mliska@suse.cz>
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
 drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index beba39a38c87..c1ba68796b6d 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
 	mutex_unlock(&dev_priv->display.backlight.lock);
 }
 
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
 static u32 intel_panel_get_backlight(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
index 339643f63897..207fe1c613d8 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.h
+++ b/drivers/gpu/drm/i915/display/intel_backlight.h
@@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
 u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
 u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
 
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
 int intel_backlight_device_register(struct intel_connector *connector);
 void intel_backlight_device_unregister(struct intel_connector *connector);
 #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-04 10:28 ` Jiri Slaby (SUSE)
  0 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby (SUSE) @ 2022-10-04 10:28 UTC (permalink / raw)
  To: jani.nikula
  Cc: Tvrtko Ursulin, Jiri Slaby (SUSE),
	intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi,
	Martin Liška

When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'

ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'

To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
with the above config, backlight support is disabled.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Reported-by: Martin Liška <mliska@suse.cz>
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
 drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index beba39a38c87..c1ba68796b6d 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
 	mutex_unlock(&dev_priv->display.backlight.lock);
 }
 
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
 static u32 intel_panel_get_backlight(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
index 339643f63897..207fe1c613d8 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.h
+++ b/drivers/gpu/drm/i915/display/intel_backlight.h
@@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
 u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
 u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
 
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
 int intel_backlight_device_register(struct intel_connector *connector);
 void intel_backlight_device_unregister(struct intel_connector *connector);
 #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-gfx] [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-04 10:28 ` Jiri Slaby (SUSE)
  0 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby (SUSE) @ 2022-10-04 10:28 UTC (permalink / raw)
  To: jani.nikula
  Cc: Jiri Slaby (SUSE),
	intel-gfx, linux-kernel, dri-devel, Daniel Vetter, Rodrigo Vivi,
	David Airlie, Martin Liška

When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'

ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'

To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
with the above config, backlight support is disabled.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Reported-by: Martin Liška <mliska@suse.cz>
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
 drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index beba39a38c87..c1ba68796b6d 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
 	mutex_unlock(&dev_priv->display.backlight.lock);
 }
 
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
 static u32 intel_panel_get_backlight(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
index 339643f63897..207fe1c613d8 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.h
+++ b/drivers/gpu/drm/i915/display/intel_backlight.h
@@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
 u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
 u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
 
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
 int intel_backlight_device_register(struct intel_connector *connector);
 void intel_backlight_device_unregister(struct intel_connector *connector);
 #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] drm/i915: remove circ_buf.h includes
  2022-10-04 10:28 ` Jiri Slaby (SUSE)
  (?)
@ 2022-10-04 10:28   ` Jiri Slaby (SUSE)
  -1 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby (SUSE) @ 2022-10-04 10:28 UTC (permalink / raw)
  To: jani.nikula
  Cc: linux-kernel, Jiri Slaby (SUSE),
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
	Daniel Vetter, intel-gfx, dri-devel

The last user of macros from that include was removed in 2018 by the
commit below.

Fixes: 6cc42152b02b ("drm/i915: Remove support for legacy debugfs crc interface")
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/gpu/drm/i915/display/intel_pipe_crc.c | 1 -
 drivers/gpu/drm/i915/i915_irq.c               | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
index 8ac263f471be..9070935b0443 100644
--- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
@@ -24,7 +24,6 @@
  *
  */
 
-#include <linux/circ_buf.h>
 #include <linux/ctype.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 86a42d9e8041..09d728b34a47 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -28,7 +28,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/circ_buf.h>
 #include <linux/slab.h>
 #include <linux/sysrq.h>
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] drm/i915: remove circ_buf.h includes
@ 2022-10-04 10:28   ` Jiri Slaby (SUSE)
  0 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby (SUSE) @ 2022-10-04 10:28 UTC (permalink / raw)
  To: jani.nikula
  Cc: Tvrtko Ursulin, David Airlie, intel-gfx, linux-kernel, dri-devel,
	Rodrigo Vivi, Jiri Slaby (SUSE)

The last user of macros from that include was removed in 2018 by the
commit below.

Fixes: 6cc42152b02b ("drm/i915: Remove support for legacy debugfs crc interface")
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/gpu/drm/i915/display/intel_pipe_crc.c | 1 -
 drivers/gpu/drm/i915/i915_irq.c               | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
index 8ac263f471be..9070935b0443 100644
--- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
@@ -24,7 +24,6 @@
  *
  */
 
-#include <linux/circ_buf.h>
 #include <linux/ctype.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 86a42d9e8041..09d728b34a47 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -28,7 +28,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/circ_buf.h>
 #include <linux/slab.h>
 #include <linux/sysrq.h>
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-gfx] [PATCH 2/2] drm/i915: remove circ_buf.h includes
@ 2022-10-04 10:28   ` Jiri Slaby (SUSE)
  0 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby (SUSE) @ 2022-10-04 10:28 UTC (permalink / raw)
  To: jani.nikula
  Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Daniel Vetter,
	Rodrigo Vivi, Jiri Slaby (SUSE)

The last user of macros from that include was removed in 2018 by the
commit below.

Fixes: 6cc42152b02b ("drm/i915: Remove support for legacy debugfs crc interface")
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/gpu/drm/i915/display/intel_pipe_crc.c | 1 -
 drivers/gpu/drm/i915/i915_irq.c               | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
index 8ac263f471be..9070935b0443 100644
--- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
@@ -24,7 +24,6 @@
  *
  */
 
-#include <linux/circ_buf.h>
 #include <linux/ctype.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 86a42d9e8041..09d728b34a47 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -28,7 +28,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/circ_buf.h>
 #include <linux/slab.h>
 #include <linux/sysrq.h>
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/display: fix randconfig build
  2022-10-04 10:28 ` Jiri Slaby (SUSE)
  (?)
@ 2022-10-04 10:52   ` Jani Nikula
  -1 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-04 10:52 UTC (permalink / raw)
  To: Jiri Slaby (SUSE)
  Cc: linux-kernel, Jiri Slaby (SUSE),
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
	Daniel Vetter, intel-gfx, dri-devel, Martin Liška

On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>
> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
> with the above config, backlight support is disabled.

So I don't want this. I'll take a patch that fixes the dependencies to
block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
and IMO using IS_REACHABLE() is a workaround to hide a broken config
under the carpet.

The right thing to do is

config DRM_I915
	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.

We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
too, and a combo of selecting and depending leads to circular
dependencies. But depending is the right fix.

Documentation/kbuild/kconfig-language.rst:

  Note:
	select should be used with care. select will force
	a symbol to a value without visiting the dependencies.
	By abusing select you are able to select a symbol FOO even
	if FOO depends on BAR that is not set.
	In general use select only for non-visible symbols
	(no prompts anywhere) and for symbols with no dependencies.
	That will limit the usefulness but on the other hand avoid
	the illegal configurations all over.


BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Reported-by: Martin Liška <mliska@suse.cz>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index beba39a38c87..c1ba68796b6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>  	mutex_unlock(&dev_priv->display.backlight.lock);
>  }
>  
> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  static u32 intel_panel_get_backlight(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
> index 339643f63897..207fe1c613d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>  u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>  u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>  
> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  int intel_backlight_device_register(struct intel_connector *connector);
>  void intel_backlight_device_unregister(struct intel_connector *connector);
>  #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-04 10:52   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-04 10:52 UTC (permalink / raw)
  To: Jiri Slaby (SUSE)
  Cc: Jiri Slaby (SUSE),
	intel-gfx, linux-kernel, dri-devel, Daniel Vetter, Rodrigo Vivi,
	David Airlie, Martin Liška

On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>
> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
> with the above config, backlight support is disabled.

So I don't want this. I'll take a patch that fixes the dependencies to
block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
and IMO using IS_REACHABLE() is a workaround to hide a broken config
under the carpet.

The right thing to do is

config DRM_I915
	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.

We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
too, and a combo of selecting and depending leads to circular
dependencies. But depending is the right fix.

Documentation/kbuild/kconfig-language.rst:

  Note:
	select should be used with care. select will force
	a symbol to a value without visiting the dependencies.
	By abusing select you are able to select a symbol FOO even
	if FOO depends on BAR that is not set.
	In general use select only for non-visible symbols
	(no prompts anywhere) and for symbols with no dependencies.
	That will limit the usefulness but on the other hand avoid
	the illegal configurations all over.


BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Reported-by: Martin Liška <mliska@suse.cz>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index beba39a38c87..c1ba68796b6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>  	mutex_unlock(&dev_priv->display.backlight.lock);
>  }
>  
> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  static u32 intel_panel_get_backlight(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
> index 339643f63897..207fe1c613d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>  u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>  u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>  
> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  int intel_backlight_device_register(struct intel_connector *connector);
>  void intel_backlight_device_unregister(struct intel_connector *connector);
>  #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-04 10:52   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-04 10:52 UTC (permalink / raw)
  To: Jiri Slaby (SUSE)
  Cc: Tvrtko Ursulin, Jiri Slaby (SUSE),
	intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi,
	Martin Liška

On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>
> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>
> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
> with the above config, backlight support is disabled.

So I don't want this. I'll take a patch that fixes the dependencies to
block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
and IMO using IS_REACHABLE() is a workaround to hide a broken config
under the carpet.

The right thing to do is

config DRM_I915
	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.

We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
too, and a combo of selecting and depending leads to circular
dependencies. But depending is the right fix.

Documentation/kbuild/kconfig-language.rst:

  Note:
	select should be used with care. select will force
	a symbol to a value without visiting the dependencies.
	By abusing select you are able to select a symbol FOO even
	if FOO depends on BAR that is not set.
	In general use select only for non-visible symbols
	(no prompts anywhere) and for symbols with no dependencies.
	That will limit the usefulness but on the other hand avoid
	the illegal configurations all over.


BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Reported-by: Martin Liška <mliska@suse.cz>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index beba39a38c87..c1ba68796b6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>  	mutex_unlock(&dev_priv->display.backlight.lock);
>  }
>  
> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  static u32 intel_panel_get_backlight(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
> index 339643f63897..207fe1c613d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>  u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>  u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>  
> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  int intel_backlight_device_register(struct intel_connector *connector);
>  void intel_backlight_device_unregister(struct intel_connector *connector);
>  #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] drm/i915: remove circ_buf.h includes
  2022-10-04 10:28   ` Jiri Slaby (SUSE)
  (?)
@ 2022-10-04 10:53     ` Jani Nikula
  -1 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-04 10:53 UTC (permalink / raw)
  To: Jiri Slaby (SUSE)
  Cc: Tvrtko Ursulin, David Airlie, intel-gfx, linux-kernel, dri-devel,
	Rodrigo Vivi, Jiri Slaby (SUSE)

On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
> The last user of macros from that include was removed in 2018 by the
> commit below.
>
> Fixes: 6cc42152b02b ("drm/i915: Remove support for legacy debugfs crc interface")
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_pipe_crc.c | 1 -
>  drivers/gpu/drm/i915/i915_irq.c               | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> index 8ac263f471be..9070935b0443 100644
> --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> @@ -24,7 +24,6 @@
>   *
>   */
>  
> -#include <linux/circ_buf.h>
>  #include <linux/ctype.h>
>  #include <linux/debugfs.h>
>  #include <linux/seq_file.h>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 86a42d9e8041..09d728b34a47 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -28,7 +28,6 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/circ_buf.h>
>  #include <linux/slab.h>
>  #include <linux/sysrq.h>

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] drm/i915: remove circ_buf.h includes
@ 2022-10-04 10:53     ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-04 10:53 UTC (permalink / raw)
  To: Jiri Slaby (SUSE)
  Cc: linux-kernel, Jiri Slaby (SUSE),
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
	Daniel Vetter, intel-gfx, dri-devel

On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
> The last user of macros from that include was removed in 2018 by the
> commit below.
>
> Fixes: 6cc42152b02b ("drm/i915: Remove support for legacy debugfs crc interface")
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_pipe_crc.c | 1 -
>  drivers/gpu/drm/i915/i915_irq.c               | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> index 8ac263f471be..9070935b0443 100644
> --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> @@ -24,7 +24,6 @@
>   *
>   */
>  
> -#include <linux/circ_buf.h>
>  #include <linux/ctype.h>
>  #include <linux/debugfs.h>
>  #include <linux/seq_file.h>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 86a42d9e8041..09d728b34a47 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -28,7 +28,6 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/circ_buf.h>
>  #include <linux/slab.h>
>  #include <linux/sysrq.h>

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915: remove circ_buf.h includes
@ 2022-10-04 10:53     ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-04 10:53 UTC (permalink / raw)
  To: Jiri Slaby (SUSE)
  Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Daniel Vetter,
	Rodrigo Vivi, Jiri Slaby (SUSE)

On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
> The last user of macros from that include was removed in 2018 by the
> commit below.
>
> Fixes: 6cc42152b02b ("drm/i915: Remove support for legacy debugfs crc interface")
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_pipe_crc.c | 1 -
>  drivers/gpu/drm/i915/i915_irq.c               | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> index 8ac263f471be..9070935b0443 100644
> --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
> @@ -24,7 +24,6 @@
>   *
>   */
>  
> -#include <linux/circ_buf.h>
>  #include <linux/ctype.h>
>  #include <linux/debugfs.h>
>  #include <linux/seq_file.h>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 86a42d9e8041..09d728b34a47 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -28,7 +28,6 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/circ_buf.h>
>  #include <linux/slab.h>
>  #include <linux/sysrq.h>

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/display: fix randconfig build
  2022-10-04 10:52   ` [Intel-gfx] " Jani Nikula
  (?)
@ 2022-10-05 11:12     ` Jiri Slaby
  -1 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby @ 2022-10-05 11:12 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	David Airlie, Daniel Vetter, intel-gfx, dri-devel,
	Martin Liška

On 04. 10. 22, 12:52, Jani Nikula wrote:
> On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
>> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>>
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>>
>> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
>> with the above config, backlight support is disabled.
> 
> So I don't want this. I'll take a patch that fixes the dependencies to
> block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
> and IMO using IS_REACHABLE() is a workaround to hide a broken config
> under the carpet.
> 
> The right thing to do is
> 
> config DRM_I915
> 	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.
> 
> We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
> too, and a combo of selecting and depending leads to circular
> dependencies. But depending is the right fix.

I'm not sure what should I do now. If I do:
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -4,6 +4,7 @@ config DRM_I915
         depends on DRM
         depends on X86 && PCI
         depends on !PREEMPT_RT
+       depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
(BACKLIGHT_CLASS_DEVICE=n && ACPI=n)
         select INTEL_GTT if X86
         select INTERVAL_TREE
         # we need shmfs for the swappable backing store, and in particular
@@ -21,7 +22,6 @@ config DRM_I915
         select IRQ_WORK
         # i915 depends on ACPI_VIDEO when ACPI is enabled
         # but for select to work, need to select ACPI_VIDEO's 
dependencies, ick
-       select BACKLIGHT_CLASS_DEVICE if ACPI
         select INPUT if ACPI
         select X86_PLATFORM_DEVICES if ACPI
         select ACPI_WMI if ACPI

I get:
drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:143:    symbol BACKLIGHT_CLASS_DEVICE is 
selected by DRM_FSL_DCU
drivers/gpu/drm/fsl-dcu/Kconfig:2:      symbol DRM_FSL_DCU depends on 
COMMON_CLK
drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
X86_PLATFORM_DEVICES
drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
by DRM_I915


Those dependencies are really cumbersome :/.

> Documentation/kbuild/kconfig-language.rst:
> 
>    Note:
> 	select should be used with care. select will force
> 	a symbol to a value without visiting the dependencies.
> 	By abusing select you are able to select a symbol FOO even
> 	if FOO depends on BAR that is not set.
> 	In general use select only for non-visible symbols
> 	(no prompts anywhere) and for symbols with no dependencies.
> 	That will limit the usefulness but on the other hand avoid
> 	the illegal configurations all over.
> 
> 
> BR,
> Jani.
> 
>>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Reported-by: Martin Liška <mliska@suse.cz>
>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>> ---
>>   drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>>   drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
>> index beba39a38c87..c1ba68796b6d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>>   	mutex_unlock(&dev_priv->display.backlight.lock);
>>   }
>>   
>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   static u32 intel_panel_get_backlight(struct intel_connector *connector)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
>> index 339643f63897..207fe1c613d8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
>> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>>   u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>>   u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>>   
>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   int intel_backlight_device_register(struct intel_connector *connector);
>>   void intel_backlight_device_unregister(struct intel_connector *connector);
>>   #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
> 

-- 
js
suse labs


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-05 11:12     ` Jiri Slaby
  0 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby @ 2022-10-05 11:12 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Tvrtko Ursulin, Martin Liška, intel-gfx, linux-kernel,
	dri-devel, Rodrigo Vivi

On 04. 10. 22, 12:52, Jani Nikula wrote:
> On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
>> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>>
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>>
>> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
>> with the above config, backlight support is disabled.
> 
> So I don't want this. I'll take a patch that fixes the dependencies to
> block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
> and IMO using IS_REACHABLE() is a workaround to hide a broken config
> under the carpet.
> 
> The right thing to do is
> 
> config DRM_I915
> 	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.
> 
> We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
> too, and a combo of selecting and depending leads to circular
> dependencies. But depending is the right fix.

I'm not sure what should I do now. If I do:
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -4,6 +4,7 @@ config DRM_I915
         depends on DRM
         depends on X86 && PCI
         depends on !PREEMPT_RT
+       depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
(BACKLIGHT_CLASS_DEVICE=n && ACPI=n)
         select INTEL_GTT if X86
         select INTERVAL_TREE
         # we need shmfs for the swappable backing store, and in particular
@@ -21,7 +22,6 @@ config DRM_I915
         select IRQ_WORK
         # i915 depends on ACPI_VIDEO when ACPI is enabled
         # but for select to work, need to select ACPI_VIDEO's 
dependencies, ick
-       select BACKLIGHT_CLASS_DEVICE if ACPI
         select INPUT if ACPI
         select X86_PLATFORM_DEVICES if ACPI
         select ACPI_WMI if ACPI

I get:
drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:143:    symbol BACKLIGHT_CLASS_DEVICE is 
selected by DRM_FSL_DCU
drivers/gpu/drm/fsl-dcu/Kconfig:2:      symbol DRM_FSL_DCU depends on 
COMMON_CLK
drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
X86_PLATFORM_DEVICES
drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
by DRM_I915


Those dependencies are really cumbersome :/.

> Documentation/kbuild/kconfig-language.rst:
> 
>    Note:
> 	select should be used with care. select will force
> 	a symbol to a value without visiting the dependencies.
> 	By abusing select you are able to select a symbol FOO even
> 	if FOO depends on BAR that is not set.
> 	In general use select only for non-visible symbols
> 	(no prompts anywhere) and for symbols with no dependencies.
> 	That will limit the usefulness but on the other hand avoid
> 	the illegal configurations all over.
> 
> 
> BR,
> Jani.
> 
>>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Reported-by: Martin Liška <mliska@suse.cz>
>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>> ---
>>   drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>>   drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
>> index beba39a38c87..c1ba68796b6d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>>   	mutex_unlock(&dev_priv->display.backlight.lock);
>>   }
>>   
>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   static u32 intel_panel_get_backlight(struct intel_connector *connector)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
>> index 339643f63897..207fe1c613d8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
>> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>>   u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>>   u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>>   
>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   int intel_backlight_device_register(struct intel_connector *connector);
>>   void intel_backlight_device_unregister(struct intel_connector *connector);
>>   #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
> 

-- 
js
suse labs


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-05 11:12     ` Jiri Slaby
  0 siblings, 0 replies; 19+ messages in thread
From: Jiri Slaby @ 2022-10-05 11:12 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Martin Liška, intel-gfx, linux-kernel, dri-devel,
	Daniel Vetter, Rodrigo Vivi, David Airlie

On 04. 10. 22, 12:52, Jani Nikula wrote:
> On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
>> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>>
>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>>
>> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
>> with the above config, backlight support is disabled.
> 
> So I don't want this. I'll take a patch that fixes the dependencies to
> block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
> and IMO using IS_REACHABLE() is a workaround to hide a broken config
> under the carpet.
> 
> The right thing to do is
> 
> config DRM_I915
> 	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.
> 
> We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
> too, and a combo of selecting and depending leads to circular
> dependencies. But depending is the right fix.

I'm not sure what should I do now. If I do:
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -4,6 +4,7 @@ config DRM_I915
         depends on DRM
         depends on X86 && PCI
         depends on !PREEMPT_RT
+       depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
(BACKLIGHT_CLASS_DEVICE=n && ACPI=n)
         select INTEL_GTT if X86
         select INTERVAL_TREE
         # we need shmfs for the swappable backing store, and in particular
@@ -21,7 +22,6 @@ config DRM_I915
         select IRQ_WORK
         # i915 depends on ACPI_VIDEO when ACPI is enabled
         # but for select to work, need to select ACPI_VIDEO's 
dependencies, ick
-       select BACKLIGHT_CLASS_DEVICE if ACPI
         select INPUT if ACPI
         select X86_PLATFORM_DEVICES if ACPI
         select ACPI_WMI if ACPI

I get:
drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:143:    symbol BACKLIGHT_CLASS_DEVICE is 
selected by DRM_FSL_DCU
drivers/gpu/drm/fsl-dcu/Kconfig:2:      symbol DRM_FSL_DCU depends on 
COMMON_CLK
drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
X86_PLATFORM_DEVICES
drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
by DRM_I915


Those dependencies are really cumbersome :/.

> Documentation/kbuild/kconfig-language.rst:
> 
>    Note:
> 	select should be used with care. select will force
> 	a symbol to a value without visiting the dependencies.
> 	By abusing select you are able to select a symbol FOO even
> 	if FOO depends on BAR that is not set.
> 	In general use select only for non-visible symbols
> 	(no prompts anywhere) and for symbols with no dependencies.
> 	That will limit the usefulness but on the other hand avoid
> 	the illegal configurations all over.
> 
> 
> BR,
> Jani.
> 
>>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Reported-by: Martin Liška <mliska@suse.cz>
>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>> ---
>>   drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>>   drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
>> index beba39a38c87..c1ba68796b6d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>>   	mutex_unlock(&dev_priv->display.backlight.lock);
>>   }
>>   
>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   static u32 intel_panel_get_backlight(struct intel_connector *connector)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
>> index 339643f63897..207fe1c613d8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
>> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>>   u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>>   u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>>   
>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   int intel_backlight_device_register(struct intel_connector *connector);
>>   void intel_backlight_device_unregister(struct intel_connector *connector);
>>   #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
> 

-- 
js
suse labs


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/display: fix randconfig build
  2022-10-05 11:12     ` Jiri Slaby
  (?)
@ 2022-10-05 11:21       ` Jani Nikula
  -1 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-05 11:21 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: linux-kernel, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	David Airlie, Daniel Vetter, intel-gfx, dri-devel,
	Martin Liška

On Wed, 05 Oct 2022, Jiri Slaby <jirislaby@kernel.org> wrote:
> On 04. 10. 22, 12:52, Jani Nikula wrote:
>> On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
>>> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>>> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>>>
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>>> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>>>
>>> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
>>> with the above config, backlight support is disabled.
>> 
>> So I don't want this. I'll take a patch that fixes the dependencies to
>> block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
>> and IMO using IS_REACHABLE() is a workaround to hide a broken config
>> under the carpet.
>> 
>> The right thing to do is
>> 
>> config DRM_I915
>> 	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.
>> 
>> We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
>> too, and a combo of selecting and depending leads to circular
>> dependencies. But depending is the right fix.
>
> I'm not sure what should I do now. If I do:
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -4,6 +4,7 @@ config DRM_I915
>          depends on DRM
>          depends on X86 && PCI
>          depends on !PREEMPT_RT
> +       depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
> (BACKLIGHT_CLASS_DEVICE=n && ACPI=n)
>          select INTEL_GTT if X86
>          select INTERVAL_TREE
>          # we need shmfs for the swappable backing store, and in particular
> @@ -21,7 +22,6 @@ config DRM_I915
>          select IRQ_WORK
>          # i915 depends on ACPI_VIDEO when ACPI is enabled
>          # but for select to work, need to select ACPI_VIDEO's 
> dependencies, ick
> -       select BACKLIGHT_CLASS_DEVICE if ACPI
>          select INPUT if ACPI
>          select X86_PLATFORM_DEVICES if ACPI
>          select ACPI_WMI if ACPI
>
> I get:
> drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
> drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
> BACKLIGHT_CLASS_DEVICE
> drivers/video/backlight/Kconfig:143:    symbol BACKLIGHT_CLASS_DEVICE is 
> selected by DRM_FSL_DCU
> drivers/gpu/drm/fsl-dcu/Kconfig:2:      symbol DRM_FSL_DCU depends on 
> COMMON_CLK
> drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
> arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
> X86_PLATFORM_DEVICES
> drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
> by DRM_I915
>
>
> Those dependencies are really cumbersome :/.

Yes. They could be fixed, though [1]. IS_REACHABLE() is just band-aid.

BR,
Jani.


[1] https://lore.kernel.org/r/1413580403-16225-1-git-send-email-jani.nikula@intel.com

>
>> Documentation/kbuild/kconfig-language.rst:
>> 
>>    Note:
>> 	select should be used with care. select will force
>> 	a symbol to a value without visiting the dependencies.
>> 	By abusing select you are able to select a symbol FOO even
>> 	if FOO depends on BAR that is not set.
>> 	In general use select only for non-visible symbols
>> 	(no prompts anywhere) and for symbols with no dependencies.
>> 	That will limit the usefulness but on the other hand avoid
>> 	the illegal configurations all over.
>> 
>> 
>> BR,
>> Jani.
>> 
>>>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>>> Cc: David Airlie <airlied@gmail.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: intel-gfx@lists.freedesktop.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Reported-by: Martin Liška <mliska@suse.cz>
>>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>>>   drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> index beba39a38c87..c1ba68796b6d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>>>   	mutex_unlock(&dev_priv->display.backlight.lock);
>>>   }
>>>   
>>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>>   static u32 intel_panel_get_backlight(struct intel_connector *connector)
>>>   {
>>>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
>>> index 339643f63897..207fe1c613d8 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
>>> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>>>   u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>>>   u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>>>   
>>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>>   int intel_backlight_device_register(struct intel_connector *connector);
>>>   void intel_backlight_device_unregister(struct intel_connector *connector);
>>>   #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-05 11:21       ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-05 11:21 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Tvrtko Ursulin, Martin Liška, intel-gfx, linux-kernel,
	dri-devel, Rodrigo Vivi

On Wed, 05 Oct 2022, Jiri Slaby <jirislaby@kernel.org> wrote:
> On 04. 10. 22, 12:52, Jani Nikula wrote:
>> On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
>>> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>>> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>>>
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>>> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>>>
>>> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
>>> with the above config, backlight support is disabled.
>> 
>> So I don't want this. I'll take a patch that fixes the dependencies to
>> block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
>> and IMO using IS_REACHABLE() is a workaround to hide a broken config
>> under the carpet.
>> 
>> The right thing to do is
>> 
>> config DRM_I915
>> 	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.
>> 
>> We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
>> too, and a combo of selecting and depending leads to circular
>> dependencies. But depending is the right fix.
>
> I'm not sure what should I do now. If I do:
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -4,6 +4,7 @@ config DRM_I915
>          depends on DRM
>          depends on X86 && PCI
>          depends on !PREEMPT_RT
> +       depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
> (BACKLIGHT_CLASS_DEVICE=n && ACPI=n)
>          select INTEL_GTT if X86
>          select INTERVAL_TREE
>          # we need shmfs for the swappable backing store, and in particular
> @@ -21,7 +22,6 @@ config DRM_I915
>          select IRQ_WORK
>          # i915 depends on ACPI_VIDEO when ACPI is enabled
>          # but for select to work, need to select ACPI_VIDEO's 
> dependencies, ick
> -       select BACKLIGHT_CLASS_DEVICE if ACPI
>          select INPUT if ACPI
>          select X86_PLATFORM_DEVICES if ACPI
>          select ACPI_WMI if ACPI
>
> I get:
> drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
> drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
> BACKLIGHT_CLASS_DEVICE
> drivers/video/backlight/Kconfig:143:    symbol BACKLIGHT_CLASS_DEVICE is 
> selected by DRM_FSL_DCU
> drivers/gpu/drm/fsl-dcu/Kconfig:2:      symbol DRM_FSL_DCU depends on 
> COMMON_CLK
> drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
> arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
> X86_PLATFORM_DEVICES
> drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
> by DRM_I915
>
>
> Those dependencies are really cumbersome :/.

Yes. They could be fixed, though [1]. IS_REACHABLE() is just band-aid.

BR,
Jani.


[1] https://lore.kernel.org/r/1413580403-16225-1-git-send-email-jani.nikula@intel.com

>
>> Documentation/kbuild/kconfig-language.rst:
>> 
>>    Note:
>> 	select should be used with care. select will force
>> 	a symbol to a value without visiting the dependencies.
>> 	By abusing select you are able to select a symbol FOO even
>> 	if FOO depends on BAR that is not set.
>> 	In general use select only for non-visible symbols
>> 	(no prompts anywhere) and for symbols with no dependencies.
>> 	That will limit the usefulness but on the other hand avoid
>> 	the illegal configurations all over.
>> 
>> 
>> BR,
>> Jani.
>> 
>>>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>>> Cc: David Airlie <airlied@gmail.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: intel-gfx@lists.freedesktop.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Reported-by: Martin Liška <mliska@suse.cz>
>>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>>>   drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> index beba39a38c87..c1ba68796b6d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>>>   	mutex_unlock(&dev_priv->display.backlight.lock);
>>>   }
>>>   
>>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>>   static u32 intel_panel_get_backlight(struct intel_connector *connector)
>>>   {
>>>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
>>> index 339643f63897..207fe1c613d8 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
>>> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>>>   u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>>>   u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>>>   
>>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>>   int intel_backlight_device_register(struct intel_connector *connector);
>>>   void intel_backlight_device_unregister(struct intel_connector *connector);
>>>   #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: fix randconfig build
@ 2022-10-05 11:21       ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2022-10-05 11:21 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Martin Liška, intel-gfx, linux-kernel, dri-devel,
	Daniel Vetter, Rodrigo Vivi, David Airlie

On Wed, 05 Oct 2022, Jiri Slaby <jirislaby@kernel.org> wrote:
> On 04. 10. 22, 12:52, Jani Nikula wrote:
>> On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
>>> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_register':
>>> intel_backlight.c:(.text+0x5587): undefined reference to `backlight_device_get_by_name'
>>>
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function `intel_backlight_device_unregister':
>>> intel_backlight.c:(.text+0x576e): undefined reference to `backlight_device_unregister'
>>>
>>> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
>>> with the above config, backlight support is disabled.
>> 
>> So I don't want this. I'll take a patch that fixes the dependencies to
>> block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
>> and IMO using IS_REACHABLE() is a workaround to hide a broken config
>> under the carpet.
>> 
>> The right thing to do is
>> 
>> config DRM_I915
>> 	depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.
>> 
>> We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
>> too, and a combo of selecting and depending leads to circular
>> dependencies. But depending is the right fix.
>
> I'm not sure what should I do now. If I do:
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -4,6 +4,7 @@ config DRM_I915
>          depends on DRM
>          depends on X86 && PCI
>          depends on !PREEMPT_RT
> +       depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
> (BACKLIGHT_CLASS_DEVICE=n && ACPI=n)
>          select INTEL_GTT if X86
>          select INTERVAL_TREE
>          # we need shmfs for the swappable backing store, and in particular
> @@ -21,7 +22,6 @@ config DRM_I915
>          select IRQ_WORK
>          # i915 depends on ACPI_VIDEO when ACPI is enabled
>          # but for select to work, need to select ACPI_VIDEO's 
> dependencies, ick
> -       select BACKLIGHT_CLASS_DEVICE if ACPI
>          select INPUT if ACPI
>          select X86_PLATFORM_DEVICES if ACPI
>          select ACPI_WMI if ACPI
>
> I get:
> drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
> drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
> BACKLIGHT_CLASS_DEVICE
> drivers/video/backlight/Kconfig:143:    symbol BACKLIGHT_CLASS_DEVICE is 
> selected by DRM_FSL_DCU
> drivers/gpu/drm/fsl-dcu/Kconfig:2:      symbol DRM_FSL_DCU depends on 
> COMMON_CLK
> drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
> arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
> X86_PLATFORM_DEVICES
> drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
> by DRM_I915
>
>
> Those dependencies are really cumbersome :/.

Yes. They could be fixed, though [1]. IS_REACHABLE() is just band-aid.

BR,
Jani.


[1] https://lore.kernel.org/r/1413580403-16225-1-git-send-email-jani.nikula@intel.com

>
>> Documentation/kbuild/kconfig-language.rst:
>> 
>>    Note:
>> 	select should be used with care. select will force
>> 	a symbol to a value without visiting the dependencies.
>> 	By abusing select you are able to select a symbol FOO even
>> 	if FOO depends on BAR that is not set.
>> 	In general use select only for non-visible symbols
>> 	(no prompts anywhere) and for symbols with no dependencies.
>> 	That will limit the usefulness but on the other hand avoid
>> 	the illegal configurations all over.
>> 
>> 
>> BR,
>> Jani.
>> 
>>>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>>> Cc: David Airlie <airlied@gmail.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: intel-gfx@lists.freedesktop.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Reported-by: Martin Liška <mliska@suse.cz>
>>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>>>   drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> index beba39a38c87..c1ba68796b6d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>>>   	mutex_unlock(&dev_priv->display.backlight.lock);
>>>   }
>>>   
>>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>>   static u32 intel_panel_get_backlight(struct intel_connector *connector)
>>>   {
>>>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
>>> index 339643f63897..207fe1c613d8 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
>>> @@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector *connector, u32 leve
>>>   u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 level);
>>>   u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
>>>   
>>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>>   int intel_backlight_device_register(struct intel_connector *connector);
>>>   void intel_backlight_device_unregister(struct intel_connector *connector);
>>>   #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/2] drm/i915/display: fix randconfig build (rev2)
  2022-10-04 10:28 ` Jiri Slaby (SUSE)
                   ` (3 preceding siblings ...)
  (?)
@ 2022-10-10 14:43 ` Patchwork
  -1 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2022-10-10 14:43 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/display: fix randconfig build (rev2)
URL   : https://patchwork.freedesktop.org/series/109542/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/109542/revisions/2/mbox/ not applied
Applying: drm/i915/display: fix randconfig build
error: corrupt patch at line 8
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/display: fix randconfig build
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-10-10 14:43 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-04 10:28 [PATCH 1/2] drm/i915/display: fix randconfig build Jiri Slaby (SUSE)
2022-10-04 10:28 ` [Intel-gfx] " Jiri Slaby (SUSE)
2022-10-04 10:28 ` Jiri Slaby (SUSE)
2022-10-04 10:28 ` [PATCH 2/2] drm/i915: remove circ_buf.h includes Jiri Slaby (SUSE)
2022-10-04 10:28   ` [Intel-gfx] " Jiri Slaby (SUSE)
2022-10-04 10:28   ` Jiri Slaby (SUSE)
2022-10-04 10:53   ` Jani Nikula
2022-10-04 10:53     ` [Intel-gfx] " Jani Nikula
2022-10-04 10:53     ` Jani Nikula
2022-10-04 10:52 ` [PATCH 1/2] drm/i915/display: fix randconfig build Jani Nikula
2022-10-04 10:52   ` Jani Nikula
2022-10-04 10:52   ` [Intel-gfx] " Jani Nikula
2022-10-05 11:12   ` Jiri Slaby
2022-10-05 11:12     ` [Intel-gfx] " Jiri Slaby
2022-10-05 11:12     ` Jiri Slaby
2022-10-05 11:21     ` Jani Nikula
2022-10-05 11:21       ` [Intel-gfx] " Jani Nikula
2022-10-05 11:21       ` Jani Nikula
2022-10-10 14:43 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/2] drm/i915/display: fix randconfig build (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.