[PATCH] drm/i915/panel: register drm_panel and call prepare/unprepare for eDP
Jani Nikula
jani.nikula at linux.intel.com
Tue Jun 17 08:48:25 UTC 2025
On Tue, 17 Jun 2025, Arun R Murthy <arun.r.murthy at intel.com> wrote:
> Allocate and register drm_panel to allow the panel_follower framework to
> detect the eDP panel and pass drm_connector::kdev device to drm_panel
> allocation for matching.
> Call drm_panel_prepare/unprepare in ddi_enable for eDP to allow the
> followers to get notified of the panel power state changes.
>
> Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> ---
> v1: Initial patch version based on the panel_follower added for DSI by
> Jani.
> ---
> drivers/gpu/drm/i915/display/intel_backlight.h | 1 +
> drivers/gpu/drm/i915/display/intel_ddi.c | 4 ++++
> drivers/gpu/drm/i915/display/intel_dp.c | 6 +++---
> drivers/gpu/drm/i915/display/intel_dp.h | 1 +
> drivers/gpu/drm/i915/display/intel_panel.c | 20 ++++++++++++++++----
> 5 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h b/drivers/gpu/drm/i915/display/intel_backlight.h
> index 339643f6389789a9b52c4c1d74e03d8f15a6efa6..ebeb6448188f0b8cf45edd1093439bd6b6e63111 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.h
> @@ -18,6 +18,7 @@ enum pipe;
>
> void intel_backlight_init_funcs(struct intel_panel *panel);
> int intel_backlight_setup(struct intel_connector *connector, enum pipe pipe);
> +void intel_edp_backlight_setup(struct intel_connector *connector);
> void intel_backlight_destroy(struct intel_panel *panel);
>
> void intel_backlight_enable(const struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index cbd1060e966431e0884aa9df7b5e6d2f27fde6d5..36aa6d14589d37614fe13ba15baf4c7e60014e2c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -73,6 +73,7 @@
> #include "intel_lspcon.h"
> #include "intel_mg_phy_regs.h"
> #include "intel_modeset_lock.h"
> +#include "intel_panel.h"
> #include "intel_pfit.h"
> #include "intel_pps.h"
> #include "intel_psr.h"
> @@ -3355,6 +3356,8 @@ static void intel_ddi_enable_dp(struct intel_atomic_state *state,
> drm_connector_update_privacy_screen(conn_state);
> intel_edp_backlight_on(crtc_state, conn_state);
>
> + intel_panel_prepare(crtc_state, conn_state);
> +
> if (!intel_lspcon_active(dig_port) || intel_dp_has_hdmi_sink(&dig_port->dp))
> intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
>
> @@ -3552,6 +3555,7 @@ static void intel_ddi_disable_dp(struct intel_atomic_state *state,
>
> intel_dp->link.active = false;
>
> + intel_panel_unprepare(old_conn_state);
> intel_psr_disable(intel_dp, old_crtc_state);
> intel_alpm_disable(intel_dp);
> intel_edp_backlight_off(old_conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 277b40b13948ed964f20aa1243f76aa263615360..451b6be6106d6ac244d23fe2d997b4f7b3e9f002 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6464,9 +6464,9 @@ intel_edp_add_properties(struct intel_dp *intel_dp)
> fixed_mode->vdisplay);
> }
>
> -static void intel_edp_backlight_setup(struct intel_dp *intel_dp,
> - struct intel_connector *connector)
> +void intel_edp_backlight_setup(struct intel_connector *connector)
Why?
> {
> + struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct intel_display *display = to_intel_display(intel_dp);
> enum pipe pipe = INVALID_PIPE;
>
> @@ -6627,7 +6627,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>
> intel_panel_init(connector, drm_edid);
>
> - intel_edp_backlight_setup(intel_dp, connector);
> + intel_panel_register(connector);
First, this is *way* too early to register the panel. Second, now it's
being done twice, and you'll have errors at probe.
This change is not needed.
>
> intel_edp_add_properties(intel_dp);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 0657f568119662828344cfddbe876f2acf2596d9..2544783d3d5ad0d8e334fbe3a9ab874e7fee559e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -214,5 +214,6 @@ int intel_dp_compute_min_hblank(struct intel_crtc_state *crtc_state,
>
> int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector);
> void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool force_on_external);
> +void intel_edp_backlight_setup(struct intel_connector *connector);
>
> #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index f956919dc648eab507cdcca149672c1ce0173481..e884d9de3666b17197e3003af7bd9388b2889e6f 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -511,11 +511,21 @@ int intel_panel_register(struct intel_connector *connector)
> struct intel_panel *panel = &connector->panel;
> int ret;
>
> - ret = intel_backlight_device_register(connector);
> - if (ret)
> - return ret;
> + switch (connector->base.connector_type) {
> + case DRM_MODE_CONNECTOR_DSI:
> + ret = intel_backlight_device_register(connector);
> + if (ret)
> + return ret;
> + break;
> + case DRM_MODE_CONNECTOR_eDP:
> + intel_edp_backlight_setup(connector);
> + break;
> + default:
> + break;
> + }
Yeah, this is completely wrong. Setup and register are two different
phases, and now you'll end up setting up backlight twice and not
registering it even once for eDP.
This change is not needed.
>
> - if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) {
> + if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI ||
> + connector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
> struct device *dev = connector->base.kdev;
> struct drm_panel *base;
>
> @@ -561,6 +571,7 @@ int intel_panel_register(struct intel_connector *connector)
>
> err:
> intel_backlight_device_unregister(connector);
> + intel_backlight_destroy(panel);
This is the register failure path, you can't call destroy here.
>
> return ret;
> }
> @@ -573,6 +584,7 @@ void intel_panel_unregister(struct intel_connector *connector)
> drm_panel_remove(panel->base);
>
> intel_backlight_device_unregister(connector);
> + intel_backlight_destroy(panel);
This is the unregister path, you can't call destroy here.
> }
>
> /* Notify followers, if any, about power being up. */
>
> ---
> base-commit: 6f72990d9e0fe084afe257621edd730444a8bc52
> change-id: 20250617-edp_panel-fb90b1c7362a
>
> Best regards,
--
Jani Nikula, Intel
More information about the Intel-xe
mailing list