* [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower
@ 2025-06-06 9:05 Jani Nikula
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-06 9:05 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Doug Anderson, Maxime Ripard,
Jani Nikula
Hi all -
This series expands panel follower to ACPI, and enables drm_panel on
i915.
Patch 1 makes panel follower matching independent of DT, making it also
work with ACPI. Similar to DT, you can use a _DSD "panel" property to
describe the dependency.
Patches 2-4 add drm_panel support to i915 DSI, making it possible to
have followers be notified of panel states.
It's not a whole lot of code, but I simply could not have done it
alone. Thanks to Heikki and Rafael for ACPI insights, Shawn for testing
and feedback, Simon for providing ASL, Doug and Maxime for drm_panel
insights. Much appreciated!
Alas that's not all. While this has been tested to work on an ACPI
system, it has not been tested to not regress on DT systems. I wouldn't
dream of merging this before that, but we don't have such systems
handy. Could anyone provide their Tested-by on a DT system with panel
followers, please?
BR,
Jani.
Jani Nikula (4):
drm/panel: use fwnode based lookups for panel followers
drm/i915/panel: add panel register/unregister
drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+
DSI
drm/i915/panel: sync panel prepared state at register
drivers/gpu/drm/drm_panel.c | 39 ++++--
drivers/gpu/drm/i915/display/icl_dsi.c | 4 +
.../gpu/drm/i915/display/intel_connector.c | 23 ++-
.../drm/i915/display/intel_display_types.h | 4 +
drivers/gpu/drm/i915/display/intel_panel.c | 131 ++++++++++++++++++
drivers/gpu/drm/i915/display/intel_panel.h | 6 +
6 files changed, 187 insertions(+), 20 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers
2025-06-06 9:05 [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
@ 2025-06-06 9:05 ` Jani Nikula
2025-06-06 15:45 ` Lee, Shawn C
` (4 more replies)
2025-06-06 9:05 ` [PATCH 2/4] drm/i915/panel: add panel register/unregister Jani Nikula
` (3 subsequent siblings)
4 siblings, 5 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-06 9:05 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Doug Anderson, Maxime Ripard,
Jani Nikula, Neil Armstrong, Jessica Zhang
Use firmware node based lookups for panel followers, to make the code
independent of OF and device tree, and make it work also for ACPI with
an appropriate _DSD.
ASL example:
Package (0x02)
{
"panel", \_SB.PCI0.GFX0.LCD0
}
Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Doug Anderson <dianders@chromium.org>
Cc: Lee Shawn C <shawn.c.lee@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_panel.c | 39 +++++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index fee65dc65979..3eb0a615f7a9 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -473,17 +473,40 @@ int of_drm_get_panel_orientation(const struct device_node *np,
EXPORT_SYMBOL(of_drm_get_panel_orientation);
#endif
-static struct drm_panel *of_find_panel(struct device *follower_dev)
+/* Find panel by fwnode */
+static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle *fwnode)
{
- struct device_node *panel_np;
struct drm_panel *panel;
- panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
- if (!panel_np)
+ if (!fwnode_device_is_available(fwnode))
return ERR_PTR(-ENODEV);
- panel = of_drm_find_panel(panel_np);
- of_node_put(panel_np);
+ mutex_lock(&panel_lock);
+
+ list_for_each_entry(panel, &panel_list, list) {
+ if (dev_fwnode(panel->dev) == fwnode) {
+ mutex_unlock(&panel_lock);
+ return panel;
+ }
+ }
+
+ mutex_unlock(&panel_lock);
+
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+/* Find panel by follower device */
+static struct drm_panel *find_panel_by_dev(struct device *follower_dev)
+{
+ struct fwnode_handle *fwnode;
+ struct drm_panel *panel;
+
+ fwnode = fwnode_find_reference(dev_fwnode(follower_dev), "panel", 0);
+ if (IS_ERR_OR_NULL(fwnode))
+ return ERR_PTR(-ENODEV);
+
+ panel = find_panel_by_fwnode(fwnode);
+ fwnode_handle_put(fwnode);
return panel;
}
@@ -506,7 +529,7 @@ bool drm_is_panel_follower(struct device *dev)
* don't bother trying to parse it here. We just need to know if the
* property is there.
*/
- return of_property_present(dev->of_node, "panel");
+ return device_property_present(dev, "panel");
}
EXPORT_SYMBOL(drm_is_panel_follower);
@@ -536,7 +559,7 @@ int drm_panel_add_follower(struct device *follower_dev,
struct drm_panel *panel;
int ret;
- panel = of_find_panel(follower_dev);
+ panel = find_panel_by_dev(follower_dev);
if (IS_ERR(panel))
return PTR_ERR(panel);
--
2.39.5
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/4] drm/i915/panel: add panel register/unregister
2025-06-06 9:05 [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
@ 2025-06-06 9:05 ` Jani Nikula
2025-06-06 15:48 ` Lee, Shawn C
` (2 more replies)
2025-06-06 9:05 ` [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI Jani Nikula
` (2 subsequent siblings)
4 siblings, 3 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-06 9:05 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Doug Anderson, Maxime Ripard,
Jani Nikula
Add panel register/unregister functions, and handle backlight
register/unregister from there. This is in preparation for adding more
panel specific register/unregister functionality.
Cc: Lee Shawn C <shawn.c.lee@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
.../gpu/drm/i915/display/intel_connector.c | 23 +++++++++----------
drivers/gpu/drm/i915/display/intel_panel.c | 10 ++++++++
drivers/gpu/drm/i915/display/intel_panel.h | 2 ++
3 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
index 9a61c972dce9..2867d76d1a5e 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -32,7 +32,6 @@
#include "i915_drv.h"
#include "i915_utils.h"
-#include "intel_backlight.h"
#include "intel_connector.h"
#include "intel_display_core.h"
#include "intel_display_debugfs.h"
@@ -153,36 +152,36 @@ void intel_connector_destroy(struct drm_connector *connector)
kfree(connector);
}
-int intel_connector_register(struct drm_connector *connector)
+int intel_connector_register(struct drm_connector *_connector)
{
- struct intel_connector *intel_connector = to_intel_connector(connector);
- struct drm_i915_private *i915 = to_i915(connector->dev);
+ struct intel_connector *connector = to_intel_connector(_connector);
+ struct drm_i915_private *i915 = to_i915(_connector->dev);
int ret;
- ret = intel_backlight_device_register(intel_connector);
+ ret = intel_panel_register(connector);
if (ret)
goto err;
if (i915_inject_probe_failure(i915)) {
ret = -EFAULT;
- goto err_backlight;
+ goto err_panel;
}
- intel_connector_debugfs_add(intel_connector);
+ intel_connector_debugfs_add(connector);
return 0;
-err_backlight:
- intel_backlight_device_unregister(intel_connector);
+err_panel:
+ intel_panel_unregister(connector);
err:
return ret;
}
-void intel_connector_unregister(struct drm_connector *connector)
+void intel_connector_unregister(struct drm_connector *_connector)
{
- struct intel_connector *intel_connector = to_intel_connector(connector);
+ struct intel_connector *connector = to_intel_connector(_connector);
- intel_backlight_device_unregister(intel_connector);
+ intel_panel_unregister(connector);
}
void intel_connector_attach_encoder(struct intel_connector *connector,
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index f5c972880391..5ae302bee191 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -462,3 +462,13 @@ void intel_panel_fini(struct intel_connector *connector)
drm_mode_destroy(connector->base.dev, fixed_mode);
}
}
+
+int intel_panel_register(struct intel_connector *connector)
+{
+ return intel_backlight_device_register(connector);
+}
+
+void intel_panel_unregister(struct intel_connector *connector)
+{
+ intel_backlight_device_unregister(connector);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index b60d12322e5d..3d592a4404f3 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -23,6 +23,8 @@ void intel_panel_init_alloc(struct intel_connector *connector);
int intel_panel_init(struct intel_connector *connector,
const struct drm_edid *fixed_edid);
void intel_panel_fini(struct intel_connector *connector);
+int intel_panel_register(struct intel_connector *connector);
+void intel_panel_unregister(struct intel_connector *connector);
enum drm_connector_status
intel_panel_detect(struct drm_connector *connector, bool force);
bool intel_panel_use_ssc(struct intel_display *display);
--
2.39.5
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI
2025-06-06 9:05 [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
2025-06-06 9:05 ` [PATCH 2/4] drm/i915/panel: add panel register/unregister Jani Nikula
@ 2025-06-06 9:05 ` Jani Nikula
2025-06-06 15:50 ` Lee, Shawn C
` (2 more replies)
2025-06-06 9:05 ` [PATCH 4/4] drm/i915/panel: sync panel prepared state at register Jani Nikula
2025-06-11 10:07 ` [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
4 siblings, 3 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-06 9:05 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Doug Anderson, Maxime Ripard,
Jani Nikula
Allocate and register a drm_panel so that drm_panel_followers can find
the panel. Pass the drm_connector::kdev device to drm_panel allocation
for matching. That's only available after drm_sysfs_connector_add(), so
we need to postpone the drm_panel allocation until .late_register()
hook.
The drm_panel framework is moving towards devm_drm_panel_alloc(). It
requires a wrapper struct, and struct intel_panel would be the natural
candidate. However, we can't postpone its allocation until
.late_register(), so we have to use __devm_drm_panel_alloc() directly
for now.
Call the drm_panel_prepare() and drm_panel_unprepare() functions for
ICL+ DSI, so that followers get notified of the panel power state
changes. This can later be expanded to VLV+ DSI and eDP.
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Lee Shawn C <shawn.c.lee@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/display/icl_dsi.c | 4 +
.../drm/i915/display/intel_display_types.h | 4 +
drivers/gpu/drm/i915/display/intel_panel.c | 82 ++++++++++++++++++-
drivers/gpu/drm/i915/display/intel_panel.h | 4 +
4 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 026361354e6f..81410b3aed51 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1276,6 +1276,8 @@ static void gen11_dsi_enable(struct intel_atomic_state *state,
intel_backlight_enable(crtc_state, conn_state);
intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
+ intel_panel_prepare(crtc_state, conn_state);
+
intel_crtc_vblank_on(crtc_state);
}
@@ -1409,6 +1411,8 @@ static void gen11_dsi_disable(struct intel_atomic_state *state,
{
struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
+ intel_panel_unprepare(old_conn_state);
+
/* step1: turn off backlight */
intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
intel_backlight_disable(old_conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index ed4d743fc7c5..30c7315fc25e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -37,6 +37,7 @@
#include <drm/drm_crtc.h>
#include <drm/drm_encoder.h>
#include <drm/drm_framebuffer.h>
+#include <drm/drm_panel.h>
#include <drm/drm_rect.h>
#include <drm/drm_vblank_work.h>
#include <drm/intel/i915_hdcp_interface.h>
@@ -384,6 +385,9 @@ struct intel_vbt_panel_data {
};
struct intel_panel {
+ /* Simple drm_panel */
+ struct drm_panel *base;
+
/* Fixed EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
const struct drm_edid *fixed_edid;
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 5ae302bee191..b1d549e6cf53 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -463,12 +463,92 @@ void intel_panel_fini(struct intel_connector *connector)
}
}
+const struct drm_panel_funcs dummy_panel_funcs = {
+};
+
int intel_panel_register(struct intel_connector *connector)
{
- return intel_backlight_device_register(connector);
+ struct intel_display *display = to_intel_display(connector);
+ struct intel_panel *panel = &connector->panel;
+ int ret;
+
+ ret = intel_backlight_device_register(connector);
+ if (ret)
+ return ret;
+
+ if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) {
+ struct device *dev = connector->base.kdev;
+ struct drm_panel *base;
+
+ /* Sanity check. */
+ if (drm_WARN_ON(display->drm, !dev))
+ goto out;
+
+ /*
+ * We need drm_connector::kdev for allocating the panel, to make
+ * drm_panel_add_follower() lookups work. The kdev is
+ * initialized in drm_sysfs_connector_add(), just before the
+ * connector .late_register() hooks. So we can't allocate the
+ * panel at connector init time, and can't allocate struct
+ * intel_panel with a drm_panel sub-struct. For now, use
+ * __devm_drm_panel_alloc() directly.
+ *
+ * The lookups also depend on drm_connector::fwnode being set in
+ * intel_acpi_assign_connector_fwnodes(). However, if that's
+ * missing, it will gracefully lead to -EPROBE_DEFER in
+ * drm_panel_add_follower().
+ */
+ base = __devm_drm_panel_alloc(dev, sizeof(*base), 0,
+ &dummy_panel_funcs,
+ connector->base.connector_type);
+ if (IS_ERR(base)) {
+ ret = PTR_ERR(base);
+ goto err;
+ }
+
+ panel->base = base;
+
+ drm_panel_add(panel->base);
+
+ drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Registered panel device '%s', has fwnode: %s\n",
+ connector->base.base.id, connector->base.name,
+ dev_name(dev), str_yes_no(dev_fwnode(dev)));
+ }
+
+out:
+ return 0;
+
+err:
+ intel_backlight_device_unregister(connector);
+
+ return ret;
}
void intel_panel_unregister(struct intel_connector *connector)
{
+ struct intel_panel *panel = &connector->panel;
+
+ if (panel->base)
+ drm_panel_remove(panel->base);
+
intel_backlight_device_unregister(connector);
}
+
+/* Notify followers, if any, about power being up. */
+void intel_panel_prepare(const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state)
+{
+ struct intel_connector *connector = to_intel_connector(conn_state->connector);
+ struct intel_panel *panel = &connector->panel;
+
+ drm_panel_prepare(panel->base);
+}
+
+/* Notify followers, if any, about power going down. */
+void intel_panel_unprepare(const struct drm_connector_state *old_conn_state)
+{
+ struct intel_connector *connector = to_intel_connector(old_conn_state->connector);
+ struct intel_panel *panel = &connector->panel;
+
+ drm_panel_unprepare(panel->base);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index 3d592a4404f3..56a6412cf0fb 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -53,4 +53,8 @@ void intel_panel_add_vbt_sdvo_fixed_mode(struct intel_connector *connector);
void intel_panel_add_encoder_fixed_mode(struct intel_connector *connector,
struct intel_encoder *encoder);
+void intel_panel_prepare(const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state);
+void intel_panel_unprepare(const struct drm_connector_state *old_conn_state);
+
#endif /* __INTEL_PANEL_H__ */
--
2.39.5
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/4] drm/i915/panel: sync panel prepared state at register
2025-06-06 9:05 [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
` (2 preceding siblings ...)
2025-06-06 9:05 ` [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI Jani Nikula
@ 2025-06-06 9:05 ` Jani Nikula
2025-06-06 15:52 ` Lee, Shawn C
` (2 more replies)
2025-06-11 10:07 ` [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
4 siblings, 3 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-06 9:05 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Doug Anderson, Maxime Ripard,
Jani Nikula
If the panel is enabled at probe, and we take over the hardware state,
the drm_panel prepared state will be out of sync. We'll need to notify
drm_panel framework about the state at probe, so it can in turn notify
the panel followers.
Cc: Lee Shawn C <shawn.c.lee@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/display/intel_panel.c | 41 ++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index b1d549e6cf53..f956919dc648 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -463,6 +463,45 @@ void intel_panel_fini(struct intel_connector *connector)
}
}
+/*
+ * If the panel was already enabled at probe, and we took over the state, the
+ * panel prepared state is out of sync, and the panel followers won't be
+ * notified. We need to call drm_panel_prepare() on enabled panels.
+ *
+ * It would be natural to handle this e.g. in the connector ->sync_state hook at
+ * intel_modeset_readout_hw_state(), but that's unfortunately too early. We
+ * don't have drm_connector::kdev at that time. For now, figure out the state at
+ * ->late_register, and sync there.
+ */
+static void intel_panel_sync_state(struct intel_connector *connector)
+{
+ struct intel_display *display = to_intel_display(connector);
+ struct drm_connector_state *conn_state;
+ struct intel_crtc *crtc;
+ int ret;
+
+ ret = drm_modeset_lock(&display->drm->mode_config.connection_mutex, NULL);
+ if (ret)
+ return;
+
+ conn_state = connector->base.state;
+
+ crtc = to_intel_crtc(conn_state->crtc);
+ if (crtc) {
+ struct intel_crtc_state *crtc_state;
+
+ crtc_state = to_intel_crtc_state(crtc->base.state);
+
+ if (crtc_state->hw.active) {
+ drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Panel prepare\n",
+ connector->base.base.id, connector->base.name);
+ intel_panel_prepare(crtc_state, conn_state);
+ }
+ }
+
+ drm_modeset_unlock(&display->drm->mode_config.connection_mutex);
+}
+
const struct drm_panel_funcs dummy_panel_funcs = {
};
@@ -513,6 +552,8 @@ int intel_panel_register(struct intel_connector *connector)
drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Registered panel device '%s', has fwnode: %s\n",
connector->base.base.id, connector->base.name,
dev_name(dev), str_yes_no(dev_fwnode(dev)));
+
+ intel_panel_sync_state(connector);
}
out:
--
2.39.5
^ permalink raw reply related [flat|nested] 23+ messages in thread
* RE: [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
@ 2025-06-06 15:45 ` Lee, Shawn C
2025-06-10 0:06 ` Doug Anderson
` (3 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: Lee, Shawn C @ 2025-06-06 15:45 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Yang, Simon1, Doug Anderson,
Maxime Ripard, Neil Armstrong, Jessica Zhang
On June 6, 2025, 9:05, Jani Nikula <jani.nikula@intel.com> wrote:
>Use firmware node based lookups for panel followers, to make the code independent of OF and device tree, and make it work also for ACPI with an appropriate _DSD.
>
>ASL example:
>
> Package (0x02)
> {
> "panel", \_SB.PCI0.GFX0.LCD0
> }
>
>Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>Cc: Neil Armstrong <neil.armstrong@linaro.org>
>Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>Cc: Maxime Ripard <mripard@kernel.org>
>Cc: Doug Anderson <dianders@chromium.org>
>Cc: Lee Shawn C <shawn.c.lee@intel.com>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
This patch series was tested on my local device. And panel follower works well.
Tested-by: Lee Shawn C <shawn.c.lee@intel.com>
>---
> drivers/gpu/drm/drm_panel.c | 39 +++++++++++++++++++++++++++++--------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index fee65dc65979..3eb0a615f7a9 100644
>--- a/drivers/gpu/drm/drm_panel.c
>+++ b/drivers/gpu/drm/drm_panel.c
>@@ -473,17 +473,40 @@ int of_drm_get_panel_orientation(const struct device_node *np, EXPORT_SYMBOL(of_drm_get_panel_orientation);
> #endif
>
>-static struct drm_panel *of_find_panel(struct device *follower_dev)
>+/* Find panel by fwnode */
>+static struct drm_panel *find_panel_by_fwnode(const struct
>+fwnode_handle *fwnode)
> {
>- struct device_node *panel_np;
> struct drm_panel *panel;
>
>- panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
>- if (!panel_np)
>+ if (!fwnode_device_is_available(fwnode))
> return ERR_PTR(-ENODEV);
>
>- panel = of_drm_find_panel(panel_np);
>- of_node_put(panel_np);
>+ mutex_lock(&panel_lock);
>+
>+ list_for_each_entry(panel, &panel_list, list) {
>+ if (dev_fwnode(panel->dev) == fwnode) {
>+ mutex_unlock(&panel_lock);
>+ return panel;
>+ }
>+ }
>+
>+ mutex_unlock(&panel_lock);
>+
>+ return ERR_PTR(-EPROBE_DEFER);
>+}
>+
>+/* Find panel by follower device */
>+static struct drm_panel *find_panel_by_dev(struct device *follower_dev)
>+{
>+ struct fwnode_handle *fwnode;
>+ struct drm_panel *panel;
>+
>+ fwnode = fwnode_find_reference(dev_fwnode(follower_dev), "panel", 0);
>+ if (IS_ERR_OR_NULL(fwnode))
>+ return ERR_PTR(-ENODEV);
>+
>+ panel = find_panel_by_fwnode(fwnode);
>+ fwnode_handle_put(fwnode);
>
> return panel;
> }
>@@ -506,7 +529,7 @@ bool drm_is_panel_follower(struct device *dev)
> * don't bother trying to parse it here. We just need to know if the
> * property is there.
> */
>- return of_property_present(dev->of_node, "panel");
>+ return device_property_present(dev, "panel");
> }
> EXPORT_SYMBOL(drm_is_panel_follower);
>
>@@ -536,7 +559,7 @@ int drm_panel_add_follower(struct device *follower_dev,
> struct drm_panel *panel;
> int ret;
>
>- panel = of_find_panel(follower_dev);
>+ panel = find_panel_by_dev(follower_dev);
> if (IS_ERR(panel))
> return PTR_ERR(panel);
>
>--
>2.39.5
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 2/4] drm/i915/panel: add panel register/unregister
2025-06-06 9:05 ` [PATCH 2/4] drm/i915/panel: add panel register/unregister Jani Nikula
@ 2025-06-06 15:48 ` Lee, Shawn C
2025-06-10 8:09 ` Murthy, Arun R
2025-06-10 12:01 ` Maxime Ripard
2 siblings, 0 replies; 23+ messages in thread
From: Lee, Shawn C @ 2025-06-06 15:48 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Yang, Simon1, Doug Anderson,
Maxime Ripard
On June 6, 2025, 9:05, Jani Nikula <jani.nikula@intel.com> wrote:
>Add panel register/unregister functions, and handle backlight register/unregister from there. This is in preparation for adding more panel specific register/unregister functionality.
>
>Cc: Lee Shawn C <shawn.c.lee@intel.com>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
This patch series was tested on my local device. And panel follower works well.
Tested-by: Lee Shawn C <shawn.c.lee@intel.com>
> .../gpu/drm/i915/display/intel_connector.c | 23 +++++++++----------
> drivers/gpu/drm/i915/display/intel_panel.c | 10 ++++++++
> drivers/gpu/drm/i915/display/intel_panel.h | 2 ++
> 3 files changed, 23 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
>index 9a61c972dce9..2867d76d1a5e 100644
>--- a/drivers/gpu/drm/i915/display/intel_connector.c
>+++ b/drivers/gpu/drm/i915/display/intel_connector.c
>@@ -32,7 +32,6 @@
>
> #include "i915_drv.h"
> #include "i915_utils.h"
>-#include "intel_backlight.h"
> #include "intel_connector.h"
> #include "intel_display_core.h"
> #include "intel_display_debugfs.h"
>@@ -153,36 +152,36 @@ void intel_connector_destroy(struct drm_connector *connector)
> kfree(connector);
> }
>
>-int intel_connector_register(struct drm_connector *connector)
>+int intel_connector_register(struct drm_connector *_connector)
> {
>- struct intel_connector *intel_connector = to_intel_connector(connector);
>- struct drm_i915_private *i915 = to_i915(connector->dev);
>+ struct intel_connector *connector = to_intel_connector(_connector);
>+ struct drm_i915_private *i915 = to_i915(_connector->dev);
> int ret;
>
>- ret = intel_backlight_device_register(intel_connector);
>+ ret = intel_panel_register(connector);
> if (ret)
> goto err;
>
> if (i915_inject_probe_failure(i915)) {
> ret = -EFAULT;
>- goto err_backlight;
>+ goto err_panel;
> }
>
>- intel_connector_debugfs_add(intel_connector);
>+ intel_connector_debugfs_add(connector);
>
> return 0;
>
>-err_backlight:
>- intel_backlight_device_unregister(intel_connector);
>+err_panel:
>+ intel_panel_unregister(connector);
> err:
> return ret;
> }
>
>-void intel_connector_unregister(struct drm_connector *connector)
>+void intel_connector_unregister(struct drm_connector *_connector)
> {
>- struct intel_connector *intel_connector = to_intel_connector(connector);
>+ struct intel_connector *connector = to_intel_connector(_connector);
>
>- intel_backlight_device_unregister(intel_connector);
>+ intel_panel_unregister(connector);
> }
>
> void intel_connector_attach_encoder(struct intel_connector *connector, diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
>index f5c972880391..5ae302bee191 100644
>--- a/drivers/gpu/drm/i915/display/intel_panel.c
>+++ b/drivers/gpu/drm/i915/display/intel_panel.c
>@@ -462,3 +462,13 @@ void intel_panel_fini(struct intel_connector *connector)
> drm_mode_destroy(connector->base.dev, fixed_mode);
> }
> }
>+
>+int intel_panel_register(struct intel_connector *connector) {
>+ return intel_backlight_device_register(connector);
>+}
>+
>+void intel_panel_unregister(struct intel_connector *connector) {
>+ intel_backlight_device_unregister(connector);
>+}
>diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
>index b60d12322e5d..3d592a4404f3 100644
>--- a/drivers/gpu/drm/i915/display/intel_panel.h
>+++ b/drivers/gpu/drm/i915/display/intel_panel.h
>@@ -23,6 +23,8 @@ void intel_panel_init_alloc(struct intel_connector *connector); int intel_panel_init(struct intel_connector *connector,
> const struct drm_edid *fixed_edid); void intel_panel_fini(struct intel_connector *connector);
>+int intel_panel_register(struct intel_connector *connector); void
>+intel_panel_unregister(struct intel_connector *connector);
> enum drm_connector_status
> intel_panel_detect(struct drm_connector *connector, bool force); bool intel_panel_use_ssc(struct intel_display *display);
>--
>2.39.5
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI
2025-06-06 9:05 ` [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI Jani Nikula
@ 2025-06-06 15:50 ` Lee, Shawn C
2025-06-10 8:21 ` Murthy, Arun R
2025-06-10 12:01 ` Maxime Ripard
2 siblings, 0 replies; 23+ messages in thread
From: Lee, Shawn C @ 2025-06-06 15:50 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Yang, Simon1, Doug Anderson,
Maxime Ripard
On June 6, 2025, 9:05, Jani Nikula <jani.nikula@intel.com> wrote:
>Allocate and register a drm_panel so that drm_panel_followers can find the panel. Pass the drm_connector::kdev device to drm_panel allocation for matching. That's only available after drm_sysfs_connector_add(), so we need to postpone the drm_panel allocation until .late_register() hook.
>
>The drm_panel framework is moving towards devm_drm_panel_alloc(). It requires a wrapper struct, and struct intel_panel would be the natural candidate. However, we can't postpone its allocation until .late_register(), so we have to use __devm_drm_panel_alloc() directly for now.
>
>Call the drm_panel_prepare() and drm_panel_unprepare() functions for
>ICL+ DSI, so that followers get notified of the panel power state
>changes. This can later be expanded to VLV+ DSI and eDP.
>
>Cc: Maxime Ripard <mripard@kernel.org>
>Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>Cc: Lee Shawn C <shawn.c.lee@intel.com>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
This patch series was tested on my local device. And panel follower works well on it.
Tested-by: Lee Shawn C <shawn.c.lee@intel.com>
>---
> drivers/gpu/drm/i915/display/icl_dsi.c | 4 +
> .../drm/i915/display/intel_display_types.h | 4 +
> drivers/gpu/drm/i915/display/intel_panel.c | 82 ++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_panel.h | 4 +
> 4 files changed, 93 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
>index 026361354e6f..81410b3aed51 100644
>--- a/drivers/gpu/drm/i915/display/icl_dsi.c
>+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>@@ -1276,6 +1276,8 @@ static void gen11_dsi_enable(struct intel_atomic_state *state,
> intel_backlight_enable(crtc_state, conn_state);
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
>
>+ intel_panel_prepare(crtc_state, conn_state);
>+
> intel_crtc_vblank_on(crtc_state);
> }
>
>@@ -1409,6 +1411,8 @@ static void gen11_dsi_disable(struct intel_atomic_state *state, {
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>
>+ intel_panel_unprepare(old_conn_state);
>+
> /* step1: turn off backlight */
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
> intel_backlight_disable(old_conn_state);
>diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>index ed4d743fc7c5..30c7315fc25e 100644
>--- a/drivers/gpu/drm/i915/display/intel_display_types.h
>+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>@@ -37,6 +37,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_encoder.h>
> #include <drm/drm_framebuffer.h>
>+#include <drm/drm_panel.h>
> #include <drm/drm_rect.h>
> #include <drm/drm_vblank_work.h>
> #include <drm/intel/i915_hdcp_interface.h> @@ -384,6 +385,9 @@ struct intel_vbt_panel_data { };
>
> struct intel_panel {
>+ /* Simple drm_panel */
>+ struct drm_panel *base;
>+
> /* Fixed EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
> const struct drm_edid *fixed_edid;
>
>diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
>index 5ae302bee191..b1d549e6cf53 100644
>--- a/drivers/gpu/drm/i915/display/intel_panel.c
>+++ b/drivers/gpu/drm/i915/display/intel_panel.c
>@@ -463,12 +463,92 @@ void intel_panel_fini(struct intel_connector *connector)
> }
> }
>
>+const struct drm_panel_funcs dummy_panel_funcs = { };
>+
> int intel_panel_register(struct intel_connector *connector) {
>- return intel_backlight_device_register(connector);
>+ struct intel_display *display = to_intel_display(connector);
>+ struct intel_panel *panel = &connector->panel;
>+ int ret;
>+
>+ ret = intel_backlight_device_register(connector);
>+ if (ret)
>+ return ret;
>+
>+ if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) {
>+ struct device *dev = connector->base.kdev;
>+ struct drm_panel *base;
>+
>+ /* Sanity check. */
>+ if (drm_WARN_ON(display->drm, !dev))
>+ goto out;
>+
>+ /*
>+ * We need drm_connector::kdev for allocating the panel, to make
>+ * drm_panel_add_follower() lookups work. The kdev is
>+ * initialized in drm_sysfs_connector_add(), just before the
>+ * connector .late_register() hooks. So we can't allocate the
>+ * panel at connector init time, and can't allocate struct
>+ * intel_panel with a drm_panel sub-struct. For now, use
>+ * __devm_drm_panel_alloc() directly.
>+ *
>+ * The lookups also depend on drm_connector::fwnode being set in
>+ * intel_acpi_assign_connector_fwnodes(). However, if that's
>+ * missing, it will gracefully lead to -EPROBE_DEFER in
>+ * drm_panel_add_follower().
>+ */
>+ base = __devm_drm_panel_alloc(dev, sizeof(*base), 0,
>+ &dummy_panel_funcs,
>+ connector->base.connector_type);
>+ if (IS_ERR(base)) {
>+ ret = PTR_ERR(base);
>+ goto err;
>+ }
>+
>+ panel->base = base;
>+
>+ drm_panel_add(panel->base);
>+
>+ drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Registered panel device '%s', has fwnode: %s\n",
>+ connector->base.base.id, connector->base.name,
>+ dev_name(dev), str_yes_no(dev_fwnode(dev)));
>+ }
>+
>+out:
>+ return 0;
>+
>+err:
>+ intel_backlight_device_unregister(connector);
>+
>+ return ret;
> }
>
> void intel_panel_unregister(struct intel_connector *connector) {
>+ struct intel_panel *panel = &connector->panel;
>+
>+ if (panel->base)
>+ drm_panel_remove(panel->base);
>+
> intel_backlight_device_unregister(connector);
> }
>+
>+/* Notify followers, if any, about power being up. */ void
>+intel_panel_prepare(const struct intel_crtc_state *crtc_state,
>+ const struct drm_connector_state *conn_state) {
>+ struct intel_connector *connector = to_intel_connector(conn_state->connector);
>+ struct intel_panel *panel = &connector->panel;
>+
>+ drm_panel_prepare(panel->base);
>+}
>+
>+/* Notify followers, if any, about power going down. */ void
>+intel_panel_unprepare(const struct drm_connector_state *old_conn_state)
>+{
>+ struct intel_connector *connector = to_intel_connector(old_conn_state->connector);
>+ struct intel_panel *panel = &connector->panel;
>+
>+ drm_panel_unprepare(panel->base);
>+}
>diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
>index 3d592a4404f3..56a6412cf0fb 100644
>--- a/drivers/gpu/drm/i915/display/intel_panel.h
>+++ b/drivers/gpu/drm/i915/display/intel_panel.h
>@@ -53,4 +53,8 @@ void intel_panel_add_vbt_sdvo_fixed_mode(struct intel_connector *connector); void intel_panel_add_encoder_fixed_mode(struct intel_connector *connector,
> struct intel_encoder *encoder);
>
>+void intel_panel_prepare(const struct intel_crtc_state *crtc_state,
>+ const struct drm_connector_state *conn_state); void
>+intel_panel_unprepare(const struct drm_connector_state
>+*old_conn_state);
>+
> #endif /* __INTEL_PANEL_H__ */
>--
>2.39.5
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 4/4] drm/i915/panel: sync panel prepared state at register
2025-06-06 9:05 ` [PATCH 4/4] drm/i915/panel: sync panel prepared state at register Jani Nikula
@ 2025-06-06 15:52 ` Lee, Shawn C
2025-06-10 8:30 ` Murthy, Arun R
2025-06-10 12:02 ` Maxime Ripard
2 siblings, 0 replies; 23+ messages in thread
From: Lee, Shawn C @ 2025-06-06 15:52 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Yang, Simon1, Doug Anderson,
Maxime Ripard
On June 6, 2025, 9:05, Jani Nikula <jani.nikula@intel.com> wrote:
>If the panel is enabled at probe, and we take over the hardware state, the drm_panel prepared state will be out of sync. We'll need to notify drm_panel framework about the state at probe, so it can in turn notify the panel followers.
>
>Cc: Lee Shawn C <shawn.c.lee@intel.com>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
This patch series was tested on my local device. And panel follower works well on it.
Tested-by: Lee Shawn C <shawn.c.lee@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_panel.c | 41 ++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
>index b1d549e6cf53..f956919dc648 100644
>--- a/drivers/gpu/drm/i915/display/intel_panel.c
>+++ b/drivers/gpu/drm/i915/display/intel_panel.c
>@@ -463,6 +463,45 @@ void intel_panel_fini(struct intel_connector *connector)
> }
> }
>
>+/*
>+ * If the panel was already enabled at probe, and we took over the
>+state, the
>+ * panel prepared state is out of sync, and the panel followers won't
>+be
>+ * notified. We need to call drm_panel_prepare() on enabled panels.
>+ *
>+ * It would be natural to handle this e.g. in the connector
>+->sync_state hook at
>+ * intel_modeset_readout_hw_state(), but that's unfortunately too
>+early. We
>+ * don't have drm_connector::kdev at that time. For now, figure out the
>+state at
>+ * ->late_register, and sync there.
>+ */
>+static void intel_panel_sync_state(struct intel_connector *connector) {
>+ struct intel_display *display = to_intel_display(connector);
>+ struct drm_connector_state *conn_state;
>+ struct intel_crtc *crtc;
>+ int ret;
>+
>+ ret = drm_modeset_lock(&display->drm->mode_config.connection_mutex, NULL);
>+ if (ret)
>+ return;
>+
>+ conn_state = connector->base.state;
>+
>+ crtc = to_intel_crtc(conn_state->crtc);
>+ if (crtc) {
>+ struct intel_crtc_state *crtc_state;
>+
>+ crtc_state = to_intel_crtc_state(crtc->base.state);
>+
>+ if (crtc_state->hw.active) {
>+ drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Panel prepare\n",
>+ connector->base.base.id, connector->base.name);
>+ intel_panel_prepare(crtc_state, conn_state);
>+ }
>+ }
>+
>+ drm_modeset_unlock(&display->drm->mode_config.connection_mutex);
>+}
>+
> const struct drm_panel_funcs dummy_panel_funcs = { };
>
>@@ -513,6 +552,8 @@ int intel_panel_register(struct intel_connector *connector)
> drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Registered panel device '%s', has fwnode: %s\n",
> connector->base.base.id, connector->base.name,
> dev_name(dev), str_yes_no(dev_fwnode(dev)));
>+
>+ intel_panel_sync_state(connector);
> }
>
> out:
>--
>2.39.5
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
2025-06-06 15:45 ` Lee, Shawn C
@ 2025-06-10 0:06 ` Doug Anderson
2025-06-10 9:36 ` Jani Nikula
2025-06-10 8:01 ` Murthy, Arun R
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Doug Anderson @ 2025-06-10 0:06 UTC (permalink / raw)
To: Jani Nikula
Cc: dri-devel, intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Maxime Ripard, Neil Armstrong,
Jessica Zhang
Hi,
On Fri, Jun 6, 2025 at 2:06 AM Jani Nikula <jani.nikula@intel.com> wrote:
>
> Use firmware node based lookups for panel followers, to make the code
> independent of OF and device tree, and make it work also for ACPI with
> an appropriate _DSD.
>
> ASL example:
>
> Package (0x02)
> {
> "panel", \_SB.PCI0.GFX0.LCD0
> }
>
> Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_panel.c | 39 +++++++++++++++++++++++++++++--------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index fee65dc65979..3eb0a615f7a9 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -473,17 +473,40 @@ int of_drm_get_panel_orientation(const struct device_node *np,
> EXPORT_SYMBOL(of_drm_get_panel_orientation);
> #endif
>
> -static struct drm_panel *of_find_panel(struct device *follower_dev)
> +/* Find panel by fwnode */
> +static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle *fwnode)
nit: It might be worth adding a comment that says it should be
identical to of_drm_find_panel() since that has a much richer
kerneldoc that talks about the corner cases.
> {
> - struct device_node *panel_np;
> struct drm_panel *panel;
>
> - panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
> - if (!panel_np)
> + if (!fwnode_device_is_available(fwnode))
> return ERR_PTR(-ENODEV);
>
> - panel = of_drm_find_panel(panel_np);
> - of_node_put(panel_np);
> + mutex_lock(&panel_lock);
> +
> + list_for_each_entry(panel, &panel_list, list) {
> + if (dev_fwnode(panel->dev) == fwnode) {
> + mutex_unlock(&panel_lock);
> + return panel;
> + }
> + }
> +
> + mutex_unlock(&panel_lock);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +/* Find panel by follower device */
> +static struct drm_panel *find_panel_by_dev(struct device *follower_dev)
> +{
> + struct fwnode_handle *fwnode;
> + struct drm_panel *panel;
> +
> + fwnode = fwnode_find_reference(dev_fwnode(follower_dev), "panel", 0);
> + if (IS_ERR_OR_NULL(fwnode))
nit: why IS_ERR_OR_NULL() instead of IS_ERR()? The kerneldoc for
fwnode_find_reference() doesn't mention anything about it returning a
NULL value in any cases...
Other than the nits, this looks reasonable to me.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
I no longer have any easy access to hardware where panel-follower is
truly necessary, but I can at least see the panel-follower calls
getting made on sc7180-trogdor-lazor, so the of->fwnode conversion
stuff must be working.
Tested-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
2025-06-06 15:45 ` Lee, Shawn C
2025-06-10 0:06 ` Doug Anderson
@ 2025-06-10 8:01 ` Murthy, Arun R
2025-06-10 9:41 ` [PATCH v2] " Jani Nikula
2025-06-10 12:01 ` [PATCH 1/4] " Maxime Ripard
4 siblings, 0 replies; 23+ messages in thread
From: Murthy, Arun R @ 2025-06-10 8:01 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Lee, Shawn C, Yang, Simon1,
Doug Anderson, Maxime Ripard, Nikula, Jani, Neil Armstrong,
Jessica Zhang
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Friday, June 6, 2025 2:35 PM
> To: dri-devel@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Heikki
> Krogerus <heikki.krogerus@linux.intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; Yang,
> Simon1 <simon1.yang@intel.com>; Doug Anderson <dianders@chromium.org>;
> Maxime Ripard <mripard@kernel.org>; Nikula, Jani <jani.nikula@intel.com>;
> Neil Armstrong <neil.armstrong@linaro.org>; Jessica Zhang
> <jessica.zhang@oss.qualcomm.com>
> Subject: [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers
>
> Use firmware node based lookups for panel followers, to make the code
> independent of OF and device tree, and make it work also for ACPI with an
> appropriate _DSD.
>
> ASL example:
>
> Package (0x02)
> {
> "panel", \_SB.PCI0.GFX0.LCD0
> }
>
> Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_panel.c | 39 +++++++++++++++++++++++++++++--------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index
> fee65dc65979..3eb0a615f7a9 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -473,17 +473,40 @@ int of_drm_get_panel_orientation(const struct
> device_node *np, EXPORT_SYMBOL(of_drm_get_panel_orientation);
> #endif
>
> -static struct drm_panel *of_find_panel(struct device *follower_dev)
> +/* Find panel by fwnode */
> +static struct drm_panel *find_panel_by_fwnode(const struct
> +fwnode_handle *fwnode)
> {
> - struct device_node *panel_np;
> struct drm_panel *panel;
>
> - panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
> - if (!panel_np)
> + if (!fwnode_device_is_available(fwnode))
> return ERR_PTR(-ENODEV);
>
> - panel = of_drm_find_panel(panel_np);
> - of_node_put(panel_np);
> + mutex_lock(&panel_lock);
> +
> + list_for_each_entry(panel, &panel_list, list) {
> + if (dev_fwnode(panel->dev) == fwnode) {
> + mutex_unlock(&panel_lock);
> + return panel;
> + }
> + }
> +
> + mutex_unlock(&panel_lock);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +/* Find panel by follower device */
> +static struct drm_panel *find_panel_by_dev(struct device *follower_dev)
> +{
> + struct fwnode_handle *fwnode;
> + struct drm_panel *panel;
> +
> + fwnode = fwnode_find_reference(dev_fwnode(follower_dev), "panel",
> 0);
> + if (IS_ERR_OR_NULL(fwnode))
> + return ERR_PTR(-ENODEV);
> +
> + panel = find_panel_by_fwnode(fwnode);
> + fwnode_handle_put(fwnode);
>
> return panel;
> }
> @@ -506,7 +529,7 @@ bool drm_is_panel_follower(struct device *dev)
Does the kdoc for this function need updates?
The doc says its supported on only device tree supported systems.
Apart from the above mentioned update, from ACPI perspective looks good to me.
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Thanks and Regards,
Arun R Murthy
--------------------
> * don't bother trying to parse it here. We just need to know if the
> * property is there.
> */
> - return of_property_present(dev->of_node, "panel");
> + return device_property_present(dev, "panel");
> }
> EXPORT_SYMBOL(drm_is_panel_follower);
>
> @@ -536,7 +559,7 @@ int drm_panel_add_follower(struct device
> *follower_dev,
> struct drm_panel *panel;
> int ret;
>
> - panel = of_find_panel(follower_dev);
> + panel = find_panel_by_dev(follower_dev);
> if (IS_ERR(panel))
> return PTR_ERR(panel);
>
> --
> 2.39.5
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 2/4] drm/i915/panel: add panel register/unregister
2025-06-06 9:05 ` [PATCH 2/4] drm/i915/panel: add panel register/unregister Jani Nikula
2025-06-06 15:48 ` Lee, Shawn C
@ 2025-06-10 8:09 ` Murthy, Arun R
2025-06-10 9:00 ` Jani Nikula
2025-06-10 12:01 ` Maxime Ripard
2 siblings, 1 reply; 23+ messages in thread
From: Murthy, Arun R @ 2025-06-10 8:09 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Lee, Shawn C, Yang, Simon1,
Doug Anderson, Maxime Ripard, Nikula, Jani
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Friday, June 6, 2025 2:35 PM
> To: dri-devel@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Heikki
> Krogerus <heikki.krogerus@linux.intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; Yang,
> Simon1 <simon1.yang@intel.com>; Doug Anderson <dianders@chromium.org>;
> Maxime Ripard <mripard@kernel.org>; Nikula, Jani <jani.nikula@intel.com>
> Subject: [PATCH 2/4] drm/i915/panel: add panel register/unregister
>
> Add panel register/unregister functions, and handle backlight
> register/unregister from there. This is in preparation for adding more panel
> specific register/unregister functionality.
>
> Cc: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> .../gpu/drm/i915/display/intel_connector.c | 23 +++++++++----------
> drivers/gpu/drm/i915/display/intel_panel.c | 10 ++++++++
> drivers/gpu/drm/i915/display/intel_panel.h | 2 ++
> 3 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> b/drivers/gpu/drm/i915/display/intel_connector.c
> index 9a61c972dce9..2867d76d1a5e 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -32,7 +32,6 @@
>
> #include "i915_drv.h"
> #include "i915_utils.h"
> -#include "intel_backlight.h"
> #include "intel_connector.h"
> #include "intel_display_core.h"
> #include "intel_display_debugfs.h"
> @@ -153,36 +152,36 @@ void intel_connector_destroy(struct drm_connector
> *connector)
> kfree(connector);
> }
>
> -int intel_connector_register(struct drm_connector *connector)
> +int intel_connector_register(struct drm_connector *_connector)
> {
> - struct intel_connector *intel_connector =
> to_intel_connector(connector);
> - struct drm_i915_private *i915 = to_i915(connector->dev);
> + struct intel_connector *connector = to_intel_connector(_connector);
> + struct drm_i915_private *i915 = to_i915(_connector->dev);
Can intel_display be used over here?
Apart from the above one, patch looks good to me.
Reviewed-by: Arun R Murthy <arun.r.murthy@gmail.com>
Thanks and Regards,
Arun R Murthy
--------------------
> int ret;
>
> - ret = intel_backlight_device_register(intel_connector);
> + ret = intel_panel_register(connector);
> if (ret)
> goto err;
>
> if (i915_inject_probe_failure(i915)) {
> ret = -EFAULT;
> - goto err_backlight;
> + goto err_panel;
> }
>
> - intel_connector_debugfs_add(intel_connector);
> + intel_connector_debugfs_add(connector);
>
> return 0;
>
> -err_backlight:
> - intel_backlight_device_unregister(intel_connector);
> +err_panel:
> + intel_panel_unregister(connector);
> err:
> return ret;
> }
>
> -void intel_connector_unregister(struct drm_connector *connector)
> +void intel_connector_unregister(struct drm_connector *_connector)
> {
> - struct intel_connector *intel_connector =
> to_intel_connector(connector);
> + struct intel_connector *connector = to_intel_connector(_connector);
>
> - intel_backlight_device_unregister(intel_connector);
> + intel_panel_unregister(connector);
> }
>
> void intel_connector_attach_encoder(struct intel_connector *connector, diff --
> git a/drivers/gpu/drm/i915/display/intel_panel.c
> b/drivers/gpu/drm/i915/display/intel_panel.c
> index f5c972880391..5ae302bee191 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -462,3 +462,13 @@ void intel_panel_fini(struct intel_connector
> *connector)
> drm_mode_destroy(connector->base.dev, fixed_mode);
> }
> }
> +
> +int intel_panel_register(struct intel_connector *connector) {
> + return intel_backlight_device_register(connector);
> +}
> +
> +void intel_panel_unregister(struct intel_connector *connector) {
> + intel_backlight_device_unregister(connector);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h
> b/drivers/gpu/drm/i915/display/intel_panel.h
> index b60d12322e5d..3d592a4404f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -23,6 +23,8 @@ void intel_panel_init_alloc(struct intel_connector
> *connector); int intel_panel_init(struct intel_connector *connector,
> const struct drm_edid *fixed_edid); void
> intel_panel_fini(struct intel_connector *connector);
> +int intel_panel_register(struct intel_connector *connector); void
> +intel_panel_unregister(struct intel_connector *connector);
> enum drm_connector_status
> intel_panel_detect(struct drm_connector *connector, bool force); bool
> intel_panel_use_ssc(struct intel_display *display);
> --
> 2.39.5
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI
2025-06-06 9:05 ` [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI Jani Nikula
2025-06-06 15:50 ` Lee, Shawn C
@ 2025-06-10 8:21 ` Murthy, Arun R
2025-06-10 9:08 ` Jani Nikula
2025-06-10 12:01 ` Maxime Ripard
2 siblings, 1 reply; 23+ messages in thread
From: Murthy, Arun R @ 2025-06-10 8:21 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Lee, Shawn C, Yang, Simon1,
Doug Anderson, Maxime Ripard, Nikula, Jani
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Friday, June 6, 2025 2:35 PM
> To: dri-devel@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Heikki
> Krogerus <heikki.krogerus@linux.intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; Yang,
> Simon1 <simon1.yang@intel.com>; Doug Anderson <dianders@chromium.org>;
> Maxime Ripard <mripard@kernel.org>; Nikula, Jani <jani.nikula@intel.com>
> Subject: [PATCH 3/4] drm/i915/panel: register drm_panel and call
> prepare/unprepare for ICL+ DSI
>
> Allocate and register a drm_panel so that drm_panel_followers can find the
> panel. Pass the drm_connector::kdev device to drm_panel allocation for
> matching. That's only available after drm_sysfs_connector_add(), so we need to
> postpone the drm_panel allocation until .late_register() hook.
>
> The drm_panel framework is moving towards devm_drm_panel_alloc(). It
> requires a wrapper struct, and struct intel_panel would be the natural
> candidate. However, we can't postpone its allocation until .late_register(), so we
> have to use __devm_drm_panel_alloc() directly for now.
>
> Call the drm_panel_prepare() and drm_panel_unprepare() functions for
> ICL+ DSI, so that followers get notified of the panel power state
> changes. This can later be expanded to VLV+ DSI and eDP.
>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 4 +
> .../drm/i915/display/intel_display_types.h | 4 +
> drivers/gpu/drm/i915/display/intel_panel.c | 82 ++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_panel.h | 4 +
> 4 files changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 026361354e6f..81410b3aed51 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1276,6 +1276,8 @@ static void gen11_dsi_enable(struct
> intel_atomic_state *state,
> intel_backlight_enable(crtc_state, conn_state);
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
>
> + intel_panel_prepare(crtc_state, conn_state);
> +
Should this be done before the intel_backlight_enable() ?
> intel_crtc_vblank_on(crtc_state);
> }
>
> @@ -1409,6 +1411,8 @@ static void gen11_dsi_disable(struct
> intel_atomic_state *state, {
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>
> + intel_panel_unprepare(old_conn_state);
> +
> /* step1: turn off backlight */
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
> intel_backlight_disable(old_conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index ed4d743fc7c5..30c7315fc25e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -37,6 +37,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_encoder.h>
> #include <drm/drm_framebuffer.h>
> +#include <drm/drm_panel.h>
> #include <drm/drm_rect.h>
> #include <drm/drm_vblank_work.h>
> #include <drm/intel/i915_hdcp_interface.h> @@ -384,6 +385,9 @@ struct
> intel_vbt_panel_data { };
>
> struct intel_panel {
> + /* Simple drm_panel */
> + struct drm_panel *base;
> +
> /* Fixed EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
> const struct drm_edid *fixed_edid;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
> b/drivers/gpu/drm/i915/display/intel_panel.c
> index 5ae302bee191..b1d549e6cf53 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -463,12 +463,92 @@ void intel_panel_fini(struct intel_connector
> *connector)
> }
> }
>
> +const struct drm_panel_funcs dummy_panel_funcs = { };
> +
> int intel_panel_register(struct intel_connector *connector) {
> - return intel_backlight_device_register(connector);
> + struct intel_display *display = to_intel_display(connector);
> + struct intel_panel *panel = &connector->panel;
> + int ret;
> +
> + ret = intel_backlight_device_register(connector);
> + if (ret)
> + return ret;
> +
Do we need to assign the backlight_device created in intel_backlight_device_register() to the element backlight in struct drm_panel, so as to use the drm_panel framework for panel backlight control?
Thanks and Regards,
Arun R Murthy
-------------------
> + if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI)
> {
> + struct device *dev = connector->base.kdev;
> + struct drm_panel *base;
> +
> + /* Sanity check. */
> + if (drm_WARN_ON(display->drm, !dev))
> + goto out;
> +
> + /*
> + * We need drm_connector::kdev for allocating the panel, to
> make
> + * drm_panel_add_follower() lookups work. The kdev is
> + * initialized in drm_sysfs_connector_add(), just before the
> + * connector .late_register() hooks. So we can't allocate the
> + * panel at connector init time, and can't allocate struct
> + * intel_panel with a drm_panel sub-struct. For now, use
> + * __devm_drm_panel_alloc() directly.
> + *
> + * The lookups also depend on drm_connector::fwnode being
> set in
> + * intel_acpi_assign_connector_fwnodes(). However, if that's
> + * missing, it will gracefully lead to -EPROBE_DEFER in
> + * drm_panel_add_follower().
> + */
> + base = __devm_drm_panel_alloc(dev, sizeof(*base), 0,
> + &dummy_panel_funcs,
> + connector->base.connector_type);
> + if (IS_ERR(base)) {
> + ret = PTR_ERR(base);
> + goto err;
> + }
> +
> + panel->base = base;
> +
> + drm_panel_add(panel->base);
> +
> + drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Registered
> panel device '%s', has fwnode: %s\n",
> + connector->base.base.id, connector->base.name,
> + dev_name(dev), str_yes_no(dev_fwnode(dev)));
> + }
> +
> +out:
> + return 0;
> +
> +err:
> + intel_backlight_device_unregister(connector);
> +
> + return ret;
> }
>
> void intel_panel_unregister(struct intel_connector *connector) {
> + struct intel_panel *panel = &connector->panel;
> +
> + if (panel->base)
> + drm_panel_remove(panel->base);
> +
> intel_backlight_device_unregister(connector);
> }
> +
> +/* Notify followers, if any, about power being up. */ void
> +intel_panel_prepare(const struct intel_crtc_state *crtc_state,
> + const struct drm_connector_state *conn_state) {
> + struct intel_connector *connector = to_intel_connector(conn_state-
> >connector);
> + struct intel_panel *panel = &connector->panel;
> +
> + drm_panel_prepare(panel->base);
> +}
> +
> +/* Notify followers, if any, about power going down. */ void
> +intel_panel_unprepare(const struct drm_connector_state *old_conn_state)
> +{
> + struct intel_connector *connector = to_intel_connector(old_conn_state-
> >connector);
> + struct intel_panel *panel = &connector->panel;
> +
> + drm_panel_unprepare(panel->base);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h
> b/drivers/gpu/drm/i915/display/intel_panel.h
> index 3d592a4404f3..56a6412cf0fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -53,4 +53,8 @@ void intel_panel_add_vbt_sdvo_fixed_mode(struct
> intel_connector *connector); void intel_panel_add_encoder_fixed_mode(struct
> intel_connector *connector,
> struct intel_encoder *encoder);
>
> +void intel_panel_prepare(const struct intel_crtc_state *crtc_state,
> + const struct drm_connector_state *conn_state); void
> +intel_panel_unprepare(const struct drm_connector_state
> +*old_conn_state);
> +
> #endif /* __INTEL_PANEL_H__ */
> --
> 2.39.5
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 4/4] drm/i915/panel: sync panel prepared state at register
2025-06-06 9:05 ` [PATCH 4/4] drm/i915/panel: sync panel prepared state at register Jani Nikula
2025-06-06 15:52 ` Lee, Shawn C
@ 2025-06-10 8:30 ` Murthy, Arun R
2025-06-10 12:02 ` Maxime Ripard
2 siblings, 0 replies; 23+ messages in thread
From: Murthy, Arun R @ 2025-06-10 8:30 UTC (permalink / raw)
To: Nikula, Jani, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Lee, Shawn C, Yang, Simon1,
Doug Anderson, Maxime Ripard, Nikula, Jani
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Friday, June 6, 2025 2:35 PM
> To: dri-devel@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Heikki
> Krogerus <heikki.krogerus@linux.intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; Yang,
> Simon1 <simon1.yang@intel.com>; Doug Anderson <dianders@chromium.org>;
> Maxime Ripard <mripard@kernel.org>; Nikula, Jani <jani.nikula@intel.com>
> Subject: [PATCH 4/4] drm/i915/panel: sync panel prepared state at register
>
> If the panel is enabled at probe, and we take over the hardware state, the
> drm_panel prepared state will be out of sync. We'll need to notify drm_panel
> framework about the state at probe, so it can in turn notify the panel followers.
>
> Cc: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Thanks and Regards,
Arun R Murthy
-------------------
> drivers/gpu/drm/i915/display/intel_panel.c | 41 ++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
> b/drivers/gpu/drm/i915/display/intel_panel.c
> index b1d549e6cf53..f956919dc648 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -463,6 +463,45 @@ void intel_panel_fini(struct intel_connector
> *connector)
> }
> }
>
> +/*
> + * If the panel was already enabled at probe, and we took over the
> +state, the
> + * panel prepared state is out of sync, and the panel followers won't
> +be
> + * notified. We need to call drm_panel_prepare() on enabled panels.
> + *
> + * It would be natural to handle this e.g. in the connector
> +->sync_state hook at
> + * intel_modeset_readout_hw_state(), but that's unfortunately too
> +early. We
> + * don't have drm_connector::kdev at that time. For now, figure out the
> +state at
> + * ->late_register, and sync there.
> + */
> +static void intel_panel_sync_state(struct intel_connector *connector) {
> + struct intel_display *display = to_intel_display(connector);
> + struct drm_connector_state *conn_state;
> + struct intel_crtc *crtc;
> + int ret;
> +
> + ret = drm_modeset_lock(&display->drm-
> >mode_config.connection_mutex, NULL);
> + if (ret)
> + return;
> +
> + conn_state = connector->base.state;
> +
> + crtc = to_intel_crtc(conn_state->crtc);
> + if (crtc) {
> + struct intel_crtc_state *crtc_state;
> +
> + crtc_state = to_intel_crtc_state(crtc->base.state);
> +
> + if (crtc_state->hw.active) {
> + drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s]
> Panel prepare\n",
> + connector->base.base.id, connector-
> >base.name);
> + intel_panel_prepare(crtc_state, conn_state);
> + }
> + }
> +
> + drm_modeset_unlock(&display->drm-
> >mode_config.connection_mutex);
> +}
> +
> const struct drm_panel_funcs dummy_panel_funcs = { };
>
> @@ -513,6 +552,8 @@ int intel_panel_register(struct intel_connector
> *connector)
> drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Registered
> panel device '%s', has fwnode: %s\n",
> connector->base.base.id, connector->base.name,
> dev_name(dev), str_yes_no(dev_fwnode(dev)));
> +
> + intel_panel_sync_state(connector);
> }
>
> out:
> --
> 2.39.5
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 2/4] drm/i915/panel: add panel register/unregister
2025-06-10 8:09 ` Murthy, Arun R
@ 2025-06-10 9:00 ` Jani Nikula
0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-10 9:00 UTC (permalink / raw)
To: Murthy, Arun R, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Lee, Shawn C, Yang, Simon1,
Doug Anderson, Maxime Ripard
On Tue, 10 Jun 2025, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
>> Nikula
>> Sent: Friday, June 6, 2025 2:35 PM
>> To: dri-devel@lists.freedesktop.org
>> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Heikki
>> Krogerus <heikki.krogerus@linux.intel.com>; Wysocki, Rafael J
>> <rafael.j.wysocki@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; Yang,
>> Simon1 <simon1.yang@intel.com>; Doug Anderson <dianders@chromium.org>;
>> Maxime Ripard <mripard@kernel.org>; Nikula, Jani <jani.nikula@intel.com>
>> Subject: [PATCH 2/4] drm/i915/panel: add panel register/unregister
>>
>> Add panel register/unregister functions, and handle backlight
>> register/unregister from there. This is in preparation for adding more panel
>> specific register/unregister functionality.
>>
>> Cc: Lee Shawn C <shawn.c.lee@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> .../gpu/drm/i915/display/intel_connector.c | 23 +++++++++----------
>> drivers/gpu/drm/i915/display/intel_panel.c | 10 ++++++++
>> drivers/gpu/drm/i915/display/intel_panel.h | 2 ++
>> 3 files changed, 23 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
>> b/drivers/gpu/drm/i915/display/intel_connector.c
>> index 9a61c972dce9..2867d76d1a5e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_connector.c
>> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
>> @@ -32,7 +32,6 @@
>>
>> #include "i915_drv.h"
>> #include "i915_utils.h"
>> -#include "intel_backlight.h"
>> #include "intel_connector.h"
>> #include "intel_display_core.h"
>> #include "intel_display_debugfs.h"
>> @@ -153,36 +152,36 @@ void intel_connector_destroy(struct drm_connector
>> *connector)
>> kfree(connector);
>> }
>>
>> -int intel_connector_register(struct drm_connector *connector)
>> +int intel_connector_register(struct drm_connector *_connector)
>> {
>> - struct intel_connector *intel_connector =
>> to_intel_connector(connector);
>> - struct drm_i915_private *i915 = to_i915(connector->dev);
>> + struct intel_connector *connector = to_intel_connector(_connector);
>> + struct drm_i915_private *i915 = to_i915(_connector->dev);
> Can intel_display be used over here?
i915 is passed to i915_inject_probe_failure() below, so no.
BR,
Jani.
>
> Apart from the above one, patch looks good to me.
> Reviewed-by: Arun R Murthy <arun.r.murthy@gmail.com>
>
> Thanks and Regards,
> Arun R Murthy
> --------------------
>
>> int ret;
>>
>> - ret = intel_backlight_device_register(intel_connector);
>> + ret = intel_panel_register(connector);
>> if (ret)
>> goto err;
>>
>> if (i915_inject_probe_failure(i915)) {
>> ret = -EFAULT;
>> - goto err_backlight;
>> + goto err_panel;
>> }
>>
>> - intel_connector_debugfs_add(intel_connector);
>> + intel_connector_debugfs_add(connector);
>>
>> return 0;
>>
>> -err_backlight:
>> - intel_backlight_device_unregister(intel_connector);
>> +err_panel:
>> + intel_panel_unregister(connector);
>> err:
>> return ret;
>> }
>>
>> -void intel_connector_unregister(struct drm_connector *connector)
>> +void intel_connector_unregister(struct drm_connector *_connector)
>> {
>> - struct intel_connector *intel_connector =
>> to_intel_connector(connector);
>> + struct intel_connector *connector = to_intel_connector(_connector);
>>
>> - intel_backlight_device_unregister(intel_connector);
>> + intel_panel_unregister(connector);
>> }
>>
>> void intel_connector_attach_encoder(struct intel_connector *connector, diff --
>> git a/drivers/gpu/drm/i915/display/intel_panel.c
>> b/drivers/gpu/drm/i915/display/intel_panel.c
>> index f5c972880391..5ae302bee191 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
>> @@ -462,3 +462,13 @@ void intel_panel_fini(struct intel_connector
>> *connector)
>> drm_mode_destroy(connector->base.dev, fixed_mode);
>> }
>> }
>> +
>> +int intel_panel_register(struct intel_connector *connector) {
>> + return intel_backlight_device_register(connector);
>> +}
>> +
>> +void intel_panel_unregister(struct intel_connector *connector) {
>> + intel_backlight_device_unregister(connector);
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h
>> b/drivers/gpu/drm/i915/display/intel_panel.h
>> index b60d12322e5d..3d592a4404f3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.h
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
>> @@ -23,6 +23,8 @@ void intel_panel_init_alloc(struct intel_connector
>> *connector); int intel_panel_init(struct intel_connector *connector,
>> const struct drm_edid *fixed_edid); void
>> intel_panel_fini(struct intel_connector *connector);
>> +int intel_panel_register(struct intel_connector *connector); void
>> +intel_panel_unregister(struct intel_connector *connector);
>> enum drm_connector_status
>> intel_panel_detect(struct drm_connector *connector, bool force); bool
>> intel_panel_use_ssc(struct intel_display *display);
>> --
>> 2.39.5
>
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI
2025-06-10 8:21 ` Murthy, Arun R
@ 2025-06-10 9:08 ` Jani Nikula
0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-10 9:08 UTC (permalink / raw)
To: Murthy, Arun R, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Heikki Krogerus, Wysocki, Rafael J, Lee, Shawn C, Yang, Simon1,
Doug Anderson, Maxime Ripard
On Tue, 10 Jun 2025, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
>> Nikula
>> Sent: Friday, June 6, 2025 2:35 PM
>> To: dri-devel@lists.freedesktop.org
>> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Heikki
>> Krogerus <heikki.krogerus@linux.intel.com>; Wysocki, Rafael J
>> <rafael.j.wysocki@intel.com>; Lee, Shawn C <shawn.c.lee@intel.com>; Yang,
>> Simon1 <simon1.yang@intel.com>; Doug Anderson <dianders@chromium.org>;
>> Maxime Ripard <mripard@kernel.org>; Nikula, Jani <jani.nikula@intel.com>
>> Subject: [PATCH 3/4] drm/i915/panel: register drm_panel and call
>> prepare/unprepare for ICL+ DSI
>>
>> Allocate and register a drm_panel so that drm_panel_followers can find the
>> panel. Pass the drm_connector::kdev device to drm_panel allocation for
>> matching. That's only available after drm_sysfs_connector_add(), so we need to
>> postpone the drm_panel allocation until .late_register() hook.
>>
>> The drm_panel framework is moving towards devm_drm_panel_alloc(). It
>> requires a wrapper struct, and struct intel_panel would be the natural
>> candidate. However, we can't postpone its allocation until .late_register(), so we
>> have to use __devm_drm_panel_alloc() directly for now.
>>
>> Call the drm_panel_prepare() and drm_panel_unprepare() functions for
>> ICL+ DSI, so that followers get notified of the panel power state
>> changes. This can later be expanded to VLV+ DSI and eDP.
>>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Cc: Lee Shawn C <shawn.c.lee@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/icl_dsi.c | 4 +
>> .../drm/i915/display/intel_display_types.h | 4 +
>> drivers/gpu/drm/i915/display/intel_panel.c | 82 ++++++++++++++++++-
>> drivers/gpu/drm/i915/display/intel_panel.h | 4 +
>> 4 files changed, 93 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>> b/drivers/gpu/drm/i915/display/icl_dsi.c
>> index 026361354e6f..81410b3aed51 100644
>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> @@ -1276,6 +1276,8 @@ static void gen11_dsi_enable(struct
>> intel_atomic_state *state,
>> intel_backlight_enable(crtc_state, conn_state);
>> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
>>
>> + intel_panel_prepare(crtc_state, conn_state);
>> +
> Should this be done before the intel_backlight_enable() ?
I'm just playing it safe and ensuring the panel is fully powered
including backlight before letting followers know we're powered.
>
>> intel_crtc_vblank_on(crtc_state);
>> }
>>
>> @@ -1409,6 +1411,8 @@ static void gen11_dsi_disable(struct
>> intel_atomic_state *state, {
>> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>
>> + intel_panel_unprepare(old_conn_state);
>> +
>> /* step1: turn off backlight */
>> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
>> intel_backlight_disable(old_conn_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index ed4d743fc7c5..30c7315fc25e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -37,6 +37,7 @@
>> #include <drm/drm_crtc.h>
>> #include <drm/drm_encoder.h>
>> #include <drm/drm_framebuffer.h>
>> +#include <drm/drm_panel.h>
>> #include <drm/drm_rect.h>
>> #include <drm/drm_vblank_work.h>
>> #include <drm/intel/i915_hdcp_interface.h> @@ -384,6 +385,9 @@ struct
>> intel_vbt_panel_data { };
>>
>> struct intel_panel {
>> + /* Simple drm_panel */
>> + struct drm_panel *base;
>> +
>> /* Fixed EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
>> const struct drm_edid *fixed_edid;
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
>> b/drivers/gpu/drm/i915/display/intel_panel.c
>> index 5ae302bee191..b1d549e6cf53 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
>> @@ -463,12 +463,92 @@ void intel_panel_fini(struct intel_connector
>> *connector)
>> }
>> }
>>
>> +const struct drm_panel_funcs dummy_panel_funcs = { };
>> +
>> int intel_panel_register(struct intel_connector *connector) {
>> - return intel_backlight_device_register(connector);
>> + struct intel_display *display = to_intel_display(connector);
>> + struct intel_panel *panel = &connector->panel;
>> + int ret;
>> +
>> + ret = intel_backlight_device_register(connector);
>> + if (ret)
>> + return ret;
>> +
> Do we need to assign the backlight_device created in intel_backlight_device_register() to the element backlight in struct drm_panel, so as to use the drm_panel framework for panel backlight control?
For now, we only use drm_panel framework for prepare/unprepare
notifications to followers. Plugging in enable/disable with backlight
doesn't seem trivial, and needs careful thought and refactoring. So
better not mix the two for the time being.
BR,
Jani.
>
> Thanks and Regards,
> Arun R Murthy
> -------------------
>
>> + if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI)
>> {
>> + struct device *dev = connector->base.kdev;
>> + struct drm_panel *base;
>> +
>> + /* Sanity check. */
>> + if (drm_WARN_ON(display->drm, !dev))
>> + goto out;
>> +
>> + /*
>> + * We need drm_connector::kdev for allocating the panel, to
>> make
>> + * drm_panel_add_follower() lookups work. The kdev is
>> + * initialized in drm_sysfs_connector_add(), just before the
>> + * connector .late_register() hooks. So we can't allocate the
>> + * panel at connector init time, and can't allocate struct
>> + * intel_panel with a drm_panel sub-struct. For now, use
>> + * __devm_drm_panel_alloc() directly.
>> + *
>> + * The lookups also depend on drm_connector::fwnode being
>> set in
>> + * intel_acpi_assign_connector_fwnodes(). However, if that's
>> + * missing, it will gracefully lead to -EPROBE_DEFER in
>> + * drm_panel_add_follower().
>> + */
>> + base = __devm_drm_panel_alloc(dev, sizeof(*base), 0,
>> + &dummy_panel_funcs,
>> + connector->base.connector_type);
>> + if (IS_ERR(base)) {
>> + ret = PTR_ERR(base);
>> + goto err;
>> + }
>> +
>> + panel->base = base;
>> +
>> + drm_panel_add(panel->base);
>> +
>> + drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] Registered
>> panel device '%s', has fwnode: %s\n",
>> + connector->base.base.id, connector->base.name,
>> + dev_name(dev), str_yes_no(dev_fwnode(dev)));
>> + }
>> +
>> +out:
>> + return 0;
>> +
>> +err:
>> + intel_backlight_device_unregister(connector);
>> +
>> + return ret;
>> }
>>
>> void intel_panel_unregister(struct intel_connector *connector) {
>> + struct intel_panel *panel = &connector->panel;
>> +
>> + if (panel->base)
>> + drm_panel_remove(panel->base);
>> +
>> intel_backlight_device_unregister(connector);
>> }
>> +
>> +/* Notify followers, if any, about power being up. */ void
>> +intel_panel_prepare(const struct intel_crtc_state *crtc_state,
>> + const struct drm_connector_state *conn_state) {
>> + struct intel_connector *connector = to_intel_connector(conn_state-
>> >connector);
>> + struct intel_panel *panel = &connector->panel;
>> +
>> + drm_panel_prepare(panel->base);
>> +}
>> +
>> +/* Notify followers, if any, about power going down. */ void
>> +intel_panel_unprepare(const struct drm_connector_state *old_conn_state)
>> +{
>> + struct intel_connector *connector = to_intel_connector(old_conn_state-
>> >connector);
>> + struct intel_panel *panel = &connector->panel;
>> +
>> + drm_panel_unprepare(panel->base);
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h
>> b/drivers/gpu/drm/i915/display/intel_panel.h
>> index 3d592a4404f3..56a6412cf0fb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.h
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
>> @@ -53,4 +53,8 @@ void intel_panel_add_vbt_sdvo_fixed_mode(struct
>> intel_connector *connector); void intel_panel_add_encoder_fixed_mode(struct
>> intel_connector *connector,
>> struct intel_encoder *encoder);
>>
>> +void intel_panel_prepare(const struct intel_crtc_state *crtc_state,
>> + const struct drm_connector_state *conn_state); void
>> +intel_panel_unprepare(const struct drm_connector_state
>> +*old_conn_state);
>> +
>> #endif /* __INTEL_PANEL_H__ */
>> --
>> 2.39.5
>
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers
2025-06-10 0:06 ` Doug Anderson
@ 2025-06-10 9:36 ` Jani Nikula
0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-10 9:36 UTC (permalink / raw)
To: Doug Anderson
Cc: dri-devel, intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Maxime Ripard, Neil Armstrong,
Jessica Zhang
On Mon, 09 Jun 2025, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Fri, Jun 6, 2025 at 2:06 AM Jani Nikula <jani.nikula@intel.com> wrote:
>>
>> Use firmware node based lookups for panel followers, to make the code
>> independent of OF and device tree, and make it work also for ACPI with
>> an appropriate _DSD.
>>
>> ASL example:
>>
>> Package (0x02)
>> {
>> "panel", \_SB.PCI0.GFX0.LCD0
>> }
>>
>> Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Cc: Neil Armstrong <neil.armstrong@linaro.org>
>> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Doug Anderson <dianders@chromium.org>
>> Cc: Lee Shawn C <shawn.c.lee@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/drm_panel.c | 39 +++++++++++++++++++++++++++++--------
>> 1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index fee65dc65979..3eb0a615f7a9 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -473,17 +473,40 @@ int of_drm_get_panel_orientation(const struct device_node *np,
>> EXPORT_SYMBOL(of_drm_get_panel_orientation);
>> #endif
>>
>> -static struct drm_panel *of_find_panel(struct device *follower_dev)
>> +/* Find panel by fwnode */
>> +static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle *fwnode)
>
> nit: It might be worth adding a comment that says it should be
> identical to of_drm_find_panel() since that has a much richer
> kerneldoc that talks about the corner cases.
Agreed.
I'm actually wondering if it would be possible to implement
of_drm_find_panel() like this (as a follow-up change):
struct drm_panel *of_drm_find_panel(const struct device_node *np)
{
const struct fwnode_handle *fwnode = of_fwnode_handle(np);
return find_panel_by_fwnode(fwnode);
}
But I'd rather leave that out for now.
>
>> {
>> - struct device_node *panel_np;
>> struct drm_panel *panel;
>>
>> - panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
>> - if (!panel_np)
>> + if (!fwnode_device_is_available(fwnode))
>> return ERR_PTR(-ENODEV);
>>
>> - panel = of_drm_find_panel(panel_np);
>> - of_node_put(panel_np);
>> + mutex_lock(&panel_lock);
>> +
>> + list_for_each_entry(panel, &panel_list, list) {
>> + if (dev_fwnode(panel->dev) == fwnode) {
>> + mutex_unlock(&panel_lock);
>> + return panel;
>> + }
>> + }
>> +
>> + mutex_unlock(&panel_lock);
>> +
>> + return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +/* Find panel by follower device */
>> +static struct drm_panel *find_panel_by_dev(struct device *follower_dev)
>> +{
>> + struct fwnode_handle *fwnode;
>> + struct drm_panel *panel;
>> +
>> + fwnode = fwnode_find_reference(dev_fwnode(follower_dev), "panel", 0);
>> + if (IS_ERR_OR_NULL(fwnode))
>
> nit: why IS_ERR_OR_NULL() instead of IS_ERR()? The kerneldoc for
> fwnode_find_reference() doesn't mention anything about it returning a
> NULL value in any cases...
Will fix.
> Other than the nits, this looks reasonable to me.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I no longer have any easy access to hardware where panel-follower is
> truly necessary, but I can at least see the panel-follower calls
> getting made on sc7180-trogdor-lazor, so the of->fwnode conversion
> stuff must be working.
>
> Tested-by: Douglas Anderson <dianders@chromium.org>
Thanks for the review and testing, much appreciated!
BR,
Jani.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2] drm/panel: use fwnode based lookups for panel followers
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
` (2 preceding siblings ...)
2025-06-10 8:01 ` Murthy, Arun R
@ 2025-06-10 9:41 ` Jani Nikula
2025-06-10 12:01 ` [PATCH 1/4] " Maxime Ripard
4 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-10 9:41 UTC (permalink / raw)
To: Jani Nikula, dri-devel
Cc: intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Doug Anderson, Maxime Ripard,
Neil Armstrong, Jessica Zhang, Arun R Murthy
Use firmware node based lookups for panel followers, to make the code
independent of OF and device tree, and make it work also for ACPI with
an appropriate _DSD.
ASL example:
Package (0x02)
{
"panel", \_SB.PCI0.GFX0.LCD0
}
v2:
- Update comments (Doug, Arun)
- s/IS_ERR_OR_NULL/IS_ERR/ (Doug)
Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Doug Anderson <dianders@chromium.org>
Cc: Lee Shawn C <shawn.c.lee@intel.com>
Tested-by: Lee Shawn C <shawn.c.lee@intel.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_panel.c | 42 ++++++++++++++++++++++++++++---------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index fee65dc65979..805b4151ccef 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -473,17 +473,40 @@ int of_drm_get_panel_orientation(const struct device_node *np,
EXPORT_SYMBOL(of_drm_get_panel_orientation);
#endif
-static struct drm_panel *of_find_panel(struct device *follower_dev)
+/* Find panel by fwnode. This should be identical to of_drm_find_panel(). */
+static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle *fwnode)
{
- struct device_node *panel_np;
struct drm_panel *panel;
- panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
- if (!panel_np)
+ if (!fwnode_device_is_available(fwnode))
return ERR_PTR(-ENODEV);
- panel = of_drm_find_panel(panel_np);
- of_node_put(panel_np);
+ mutex_lock(&panel_lock);
+
+ list_for_each_entry(panel, &panel_list, list) {
+ if (dev_fwnode(panel->dev) == fwnode) {
+ mutex_unlock(&panel_lock);
+ return panel;
+ }
+ }
+
+ mutex_unlock(&panel_lock);
+
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+/* Find panel by follower device */
+static struct drm_panel *find_panel_by_dev(struct device *follower_dev)
+{
+ struct fwnode_handle *fwnode;
+ struct drm_panel *panel;
+
+ fwnode = fwnode_find_reference(dev_fwnode(follower_dev), "panel", 0);
+ if (IS_ERR(fwnode))
+ return ERR_PTR(-ENODEV);
+
+ panel = find_panel_by_fwnode(fwnode);
+ fwnode_handle_put(fwnode);
return panel;
}
@@ -494,7 +517,7 @@ static struct drm_panel *of_find_panel(struct device *follower_dev)
*
* This checks to see if a device needs to be power sequenced together with
* a panel using the panel follower API.
- * At the moment panels can only be followed on device tree enabled systems.
+ *
* The "panel" property of the follower points to the panel to be followed.
*
* Return: true if we should be power sequenced with a panel; false otherwise.
@@ -506,7 +529,7 @@ bool drm_is_panel_follower(struct device *dev)
* don't bother trying to parse it here. We just need to know if the
* property is there.
*/
- return of_property_present(dev->of_node, "panel");
+ return device_property_present(dev, "panel");
}
EXPORT_SYMBOL(drm_is_panel_follower);
@@ -523,7 +546,6 @@ EXPORT_SYMBOL(drm_is_panel_follower);
* If a follower is added to a panel that's already been turned on, the
* follower's prepare callback is called right away.
*
- * At the moment panels can only be followed on device tree enabled systems.
* The "panel" property of the follower points to the panel to be followed.
*
* Return: 0 or an error code. Note that -ENODEV means that we detected that
@@ -536,7 +558,7 @@ int drm_panel_add_follower(struct device *follower_dev,
struct drm_panel *panel;
int ret;
- panel = of_find_panel(follower_dev);
+ panel = find_panel_by_dev(follower_dev);
if (IS_ERR(panel))
return PTR_ERR(panel);
--
2.39.5
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
` (3 preceding siblings ...)
2025-06-10 9:41 ` [PATCH v2] " Jani Nikula
@ 2025-06-10 12:01 ` Maxime Ripard
4 siblings, 0 replies; 23+ messages in thread
From: Maxime Ripard @ 2025-06-10 12:01 UTC (permalink / raw)
To: Jani Nikula
Cc: dri-devel, intel-gfx, intel-xe, simon1.yang, Doug Anderson,
Heikki Krogerus, Jessica Zhang, Lee Shawn C, Maxime Ripard,
Neil Armstrong, Wysocki Rafael J
On Fri, 6 Jun 2025 12:05:09 +0300, Jani Nikula wrote:
> Use firmware node based lookups for panel followers, to make the code
> independent of OF and device tree, and make it work also for ACPI with
> an appropriate _DSD.
>
> ASL example:
>
> [ ... ]
Acked-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] drm/i915/panel: add panel register/unregister
2025-06-06 9:05 ` [PATCH 2/4] drm/i915/panel: add panel register/unregister Jani Nikula
2025-06-06 15:48 ` Lee, Shawn C
2025-06-10 8:09 ` Murthy, Arun R
@ 2025-06-10 12:01 ` Maxime Ripard
2 siblings, 0 replies; 23+ messages in thread
From: Maxime Ripard @ 2025-06-10 12:01 UTC (permalink / raw)
To: Jani Nikula
Cc: dri-devel, intel-gfx, intel-xe, simon1.yang, Doug Anderson,
Heikki Krogerus, Lee Shawn C, Maxime Ripard, Wysocki Rafael J
On Fri, 6 Jun 2025 12:05:10 +0300, Jani Nikula wrote:
> Add panel register/unregister functions, and handle backlight
> register/unregister from there. This is in preparation for adding more
> panel specific register/unregister functionality.
>
> Cc: Lee Shawn C <shawn.c.lee@intel.com>
>
> [ ... ]
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI
2025-06-06 9:05 ` [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI Jani Nikula
2025-06-06 15:50 ` Lee, Shawn C
2025-06-10 8:21 ` Murthy, Arun R
@ 2025-06-10 12:01 ` Maxime Ripard
2 siblings, 0 replies; 23+ messages in thread
From: Maxime Ripard @ 2025-06-10 12:01 UTC (permalink / raw)
To: Jani Nikula
Cc: dri-devel, intel-gfx, intel-xe, simon1.yang, Doug Anderson,
Heikki Krogerus, Lee Shawn C, Maxime Ripard, Wysocki Rafael J
On Fri, 6 Jun 2025 12:05:11 +0300, Jani Nikula wrote:
> Allocate and register a drm_panel so that drm_panel_followers can find
> the panel. Pass the drm_connector::kdev device to drm_panel allocation
> for matching. That's only available after drm_sysfs_connector_add(), so
> we need to postpone the drm_panel allocation until .late_register()
> hook.
>
> [ ... ]
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] drm/i915/panel: sync panel prepared state at register
2025-06-06 9:05 ` [PATCH 4/4] drm/i915/panel: sync panel prepared state at register Jani Nikula
2025-06-06 15:52 ` Lee, Shawn C
2025-06-10 8:30 ` Murthy, Arun R
@ 2025-06-10 12:02 ` Maxime Ripard
2 siblings, 0 replies; 23+ messages in thread
From: Maxime Ripard @ 2025-06-10 12:02 UTC (permalink / raw)
To: Jani Nikula
Cc: dri-devel, intel-gfx, intel-xe, simon1.yang, Doug Anderson,
Heikki Krogerus, Lee Shawn C, Maxime Ripard, Wysocki Rafael J
On Fri, 6 Jun 2025 12:05:12 +0300, Jani Nikula wrote:
> If the panel is enabled at probe, and we take over the hardware state,
> the drm_panel prepared state will be out of sync. We'll need to notify
> drm_panel framework about the state at probe, so it can in turn notify
> the panel followers.
>
>
> [ ... ]
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower
2025-06-06 9:05 [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
` (3 preceding siblings ...)
2025-06-06 9:05 ` [PATCH 4/4] drm/i915/panel: sync panel prepared state at register Jani Nikula
@ 2025-06-11 10:07 ` Jani Nikula
4 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2025-06-11 10:07 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, intel-xe, Heikki Krogerus, Wysocki Rafael J,
Lee Shawn C, simon1.yang, Doug Anderson, Maxime Ripard
On Fri, 06 Jun 2025, Jani Nikula <jani.nikula@intel.com> wrote:
> Hi all -
>
> This series expands panel follower to ACPI, and enables drm_panel on
> i915.
>
> Patch 1 makes panel follower matching independent of DT, making it also
> work with ACPI. Similar to DT, you can use a _DSD "panel" property to
> describe the dependency.
>
> Patches 2-4 add drm_panel support to i915 DSI, making it possible to
> have followers be notified of panel states.
>
> It's not a whole lot of code, but I simply could not have done it
> alone. Thanks to Heikki and Rafael for ACPI insights, Shawn for testing
> and feedback, Simon for providing ASL, Doug and Maxime for drm_panel
> insights. Much appreciated!
>
> Alas that's not all. While this has been tested to work on an ACPI
> system, it has not been tested to not regress on DT systems. I wouldn't
> dream of merging this before that, but we don't have such systems
> handy. Could anyone provide their Tested-by on a DT system with panel
> followers, please?
Thanks for the reviews, acks, and tested-bys, merged to drm-misc-next.
BR,
Jani.
>
>
> BR,
> Jani.
>
>
> Jani Nikula (4):
> drm/panel: use fwnode based lookups for panel followers
> drm/i915/panel: add panel register/unregister
> drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+
> DSI
> drm/i915/panel: sync panel prepared state at register
>
> drivers/gpu/drm/drm_panel.c | 39 ++++--
> drivers/gpu/drm/i915/display/icl_dsi.c | 4 +
> .../gpu/drm/i915/display/intel_connector.c | 23 ++-
> .../drm/i915/display/intel_display_types.h | 4 +
> drivers/gpu/drm/i915/display/intel_panel.c | 131 ++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_panel.h | 6 +
> 6 files changed, 187 insertions(+), 20 deletions(-)
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-06-11 10:07 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-06 9:05 [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
2025-06-06 9:05 ` [PATCH 1/4] drm/panel: use fwnode based lookups for panel followers Jani Nikula
2025-06-06 15:45 ` Lee, Shawn C
2025-06-10 0:06 ` Doug Anderson
2025-06-10 9:36 ` Jani Nikula
2025-06-10 8:01 ` Murthy, Arun R
2025-06-10 9:41 ` [PATCH v2] " Jani Nikula
2025-06-10 12:01 ` [PATCH 1/4] " Maxime Ripard
2025-06-06 9:05 ` [PATCH 2/4] drm/i915/panel: add panel register/unregister Jani Nikula
2025-06-06 15:48 ` Lee, Shawn C
2025-06-10 8:09 ` Murthy, Arun R
2025-06-10 9:00 ` Jani Nikula
2025-06-10 12:01 ` Maxime Ripard
2025-06-06 9:05 ` [PATCH 3/4] drm/i915/panel: register drm_panel and call prepare/unprepare for ICL+ DSI Jani Nikula
2025-06-06 15:50 ` Lee, Shawn C
2025-06-10 8:21 ` Murthy, Arun R
2025-06-10 9:08 ` Jani Nikula
2025-06-10 12:01 ` Maxime Ripard
2025-06-06 9:05 ` [PATCH 4/4] drm/i915/panel: sync panel prepared state at register Jani Nikula
2025-06-06 15:52 ` Lee, Shawn C
2025-06-10 8:30 ` Murthy, Arun R
2025-06-10 12:02 ` Maxime Ripard
2025-06-11 10:07 ` [PATCH 0/4] drm/panel, drm/i915: ACPI support for panel follower Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).