* [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.