[PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Suraj Kandpal]

Add new argument to drm_edp_backlight_init which gives the
drm_luminance_range_info struct which will be needed to set the min
and max values for backlight.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
 drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
 include/drm/display/drm_dp_helper.h                   | 1 +
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 99b27e5e3365..3b309ac5190b 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
  * interface.
  * @aux: The DP aux device to use for probing
  * @bl: The &drm_edp_backlight_info struct to fill out with information on the backlight
+ * @lr: The &drm_luminance_range_info struct which is used to get the min max when using
+ *luminance override
  * @driver_pwm_freq_hz: Optional PWM frequency from the driver in hz
  * @edp_dpcd: A cached copy of the eDP DPCD
  * @current_level: Where to store the probed brightness level, if any
@@ -4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
  */
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
+		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
 		       u16 *current_level, u8 *current_mode, bool need_luminance)
 {
@@ -4372,7 +4375,7 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
 
 	bl->aux = aux;
 
-	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+	ret = drm_edp_backlight_init(aux, &bl->info, NULL, 0, edp_dpcd,
 				     &current_level, &current_mode, false);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 6dab8a0c0d74..22af5cf83b1e 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -600,8 +600,9 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
 			    connector->base.base.id, connector->base.name);
 	} else {
 		ret = drm_edp_backlight_init(&intel_dp->aux, &panel->backlight.edp.vesa.info,
-					     panel->vbt.backlight.pwm_freq_hz, intel_dp->edp_dpcd,
-					     &current_level, &current_mode, false);
+					     luminance_range, panel->vbt.backlight.pwm_freq_hz,
+					     intel_dp->edp_dpcd, &current_level, &current_mode,
+					     false);
 		if (ret < 0)
 			return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index b938684a9422..a3681e101d56 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -234,6 +234,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 		    const struct backlight_ops **ops)
 {
 	struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
+	struct drm_luminance_range_info *luminance_range =
+		&nv_conn->base.display_info.luminance_range;
 
 	/*
 	 * Note when this runs the connectors have not been probed yet,
@@ -261,7 +263,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 			NV_DEBUG(drm, "DPCD backlight controls supported on %s\n",
 				 nv_conn->base.name);
 
-			ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info, 0, edp_dpcd,
+			ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info,
+						     luminance_range, 0, edp_dpcd,
 						     &current_level, &current_mode, false);
 			if (ret < 0)
 				return ret;
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index ef0786a0af4a..6f53921f5dce 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -849,6 +849,7 @@ struct drm_edp_backlight_info {
 
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
+		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
 		       u16 *current_level, u8 *current_mode, bool need_luminance);
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-- 
2.34.1

[PATCH 00/13] Modify drm helpers to use luminance

[Suraj Kandpal]

This series modifies drm dp edp helpers so that drivers can now use them
to manipulate brightness using luminance value via the
PANEL_TARGET_LUMINANCE_VALUE register. This feature was
introduced frin eDP 1.5.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>

Suraj Kandpal (13):
  drm/dp: Introduce new member in drm_backlight_info
  drm/dp: Add argument in drm_edp_backlight_init
  drm/dp: Add argument for luminance range info in
    drm_edp_backlight_init
  drm/dp: Move from u16 to u32 for max in drm_edp_backlight_info
  drm/dp: Change current_level argument type to u32
  drm/dp: Modify drm_edp_probe_state
  drm/dp: Change argument type for drm_edp_backlight_set_level
  drm/dp: Modify drm_edp_backlight_set_level
  drm/dp: Change argument type of drm_edp_backlight_enable
  drm/dp: Enable backlight control using luminance
  drm/i915/backlight: Use drm helper to initialize edp backlight
  drm/i915/backlight: Use drm helper to set edp backlight
  drm/i915/backlight: Use drm_edp_backlight_enable

 drivers/gpu/drm/display/drm_dp_helper.c       |  88 ++++++++---
 .../drm/i915/display/intel_dp_aux_backlight.c | 143 ++++++------------
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |   2 +-
 drivers/gpu/drm/nouveau/nouveau_backlight.c   |   9 +-
 include/drm/display/drm_dp_helper.h           |  10 +-
 5 files changed, 126 insertions(+), 126 deletions(-)

-- 
2.34.1

[PATCH 01/13] drm/dp: Introduce new member in drm_backlight_info

[Suraj Kandpal]

Introduce luminance_set flag which indicates if we can manipulate
backlight using luminance value or not which is only possible
after eDP v1.5.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 8 ++++++--
 include/drm/display/drm_dp_helper.h     | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 57828f2b7b5a..41de7a92d76d 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4253,11 +4253,15 @@ drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl
 		bl->aux_set = true;
 	if (edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
 		bl->lsb_reg_used = true;
+	if ((edp_dpcd[0] & DP_EDP_15) && edp_dpcd[3] &
+	    (DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE))
+		bl->luminance_set = true;
 
 	/* Sanity check caps */
-	if (!bl->aux_set && !(edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) {
+	if (!bl->aux_set && !(edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP) &&
+	    !bl->luminance_set) {
 		drm_dbg_kms(aux->drm_dev,
-			    "%s: Panel supports neither AUX or PWM brightness control? Aborting\n",
+			    "%s: Panel does not support AUX, PWM or luminance-based brightness control. Aborting\n",
 			    aux->name);
 		return -EINVAL;
 	}
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index d9614e2c8939..b8fdc09737fc 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -844,6 +844,7 @@ struct drm_edp_backlight_info {
 	bool lsb_reg_used : 1;
 	bool aux_enable : 1;
 	bool aux_set : 1;
+	bool luminance_set : 1;
 };
 
 int
-- 
2.34.1

[PATCH 02/13] drm/dp: Add argument in drm_edp_backlight_init

[Suraj Kandpal]

Add bool argument in drm_edp_backlight init to provide the drivers
option to choose if they want to use luminance values to
manipulate brightness.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c               | 7 ++++---
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_backlight.c           | 2 +-
 include/drm/display/drm_dp_helper.h                   | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 41de7a92d76d..99b27e5e3365 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4231,6 +4231,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
  * @edp_dpcd: A cached copy of the eDP DPCD
  * @current_level: Where to store the probed brightness level, if any
  * @current_mode: Where to store the currently set backlight control mode
+ * @need_luminance: Tells us if a we want to manipulate backlight using luminance values
  *
  * Initializes a &drm_edp_backlight_info struct by probing @aux for it's backlight capabilities,
  * along with also probing the current and maximum supported brightness levels.
@@ -4243,7 +4244,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
-		       u16 *current_level, u8 *current_mode)
+		       u16 *current_level, u8 *current_mode, bool need_luminance)
 {
 	int ret;
 
@@ -4254,7 +4255,7 @@ drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl
 	if (edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
 		bl->lsb_reg_used = true;
 	if ((edp_dpcd[0] & DP_EDP_15) && edp_dpcd[3] &
-	    (DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE))
+	    (DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE) && need_luminance)
 		bl->luminance_set = true;
 
 	/* Sanity check caps */
@@ -4372,7 +4373,7 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
 	bl->aux = aux;
 
 	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
-				     &current_level, &current_mode);
+				     &current_level, &current_mode, false);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 20ab90acb351..d658e77b43d8 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -601,7 +601,7 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
 	} else {
 		ret = drm_edp_backlight_init(&intel_dp->aux, &panel->backlight.edp.vesa.info,
 					     panel->vbt.backlight.pwm_freq_hz, intel_dp->edp_dpcd,
-					     &current_level, &current_mode);
+					     &current_level, &current_mode, false);
 		if (ret < 0)
 			return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index d47442125fa1..b938684a9422 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -262,7 +262,7 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 				 nv_conn->base.name);
 
 			ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info, 0, edp_dpcd,
-						     &current_level, &current_mode);
+						     &current_level, &current_mode, false);
 			if (ret < 0)
 				return ret;
 
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index b8fdc09737fc..ef0786a0af4a 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -850,7 +850,7 @@ struct drm_edp_backlight_info {
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
-		       u16 *current_level, u8 *current_mode);
+		       u16 *current_level, u8 *current_mode, bool need_luminance);
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
 				u16 level);
 int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-- 
2.34.1

[PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Suraj Kandpal]

Add new argument to drm_edp_backlight_init which gives the
drm_luminance_range_info struct which will be needed to set the min
and max values for backlight.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
 drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
 include/drm/display/drm_dp_helper.h                   | 1 +
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 99b27e5e3365..3b309ac5190b 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
  * interface.
  * @aux: The DP aux device to use for probing
  * @bl: The &drm_edp_backlight_info struct to fill out with information on the backlight
+ * @lr: The &drm_luminance_range_info struct which is used to get the min max when using
+ *luminance override
  * @driver_pwm_freq_hz: Optional PWM frequency from the driver in hz
  * @edp_dpcd: A cached copy of the eDP DPCD
  * @current_level: Where to store the probed brightness level, if any
@@ -4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
  */
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
+		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
 		       u16 *current_level, u8 *current_mode, bool need_luminance)
 {
@@ -4372,7 +4375,7 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
 
 	bl->aux = aux;
 
-	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+	ret = drm_edp_backlight_init(aux, &bl->info, NULL, 0, edp_dpcd,
 				     &current_level, &current_mode, false);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index d658e77b43d8..abb5ad4eef5f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -600,8 +600,9 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
 			    connector->base.base.id, connector->base.name);
 	} else {
 		ret = drm_edp_backlight_init(&intel_dp->aux, &panel->backlight.edp.vesa.info,
-					     panel->vbt.backlight.pwm_freq_hz, intel_dp->edp_dpcd,
-					     &current_level, &current_mode, false);
+					     luminance_range, panel->vbt.backlight.pwm_freq_hz,
+					     intel_dp->edp_dpcd, &current_level, &current_mode,
+					     false);
 		if (ret < 0)
 			return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index b938684a9422..a3681e101d56 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -234,6 +234,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 		    const struct backlight_ops **ops)
 {
 	struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
+	struct drm_luminance_range_info *luminance_range =
+		&nv_conn->base.display_info.luminance_range;
 
 	/*
 	 * Note when this runs the connectors have not been probed yet,
@@ -261,7 +263,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 			NV_DEBUG(drm, "DPCD backlight controls supported on %s\n",
 				 nv_conn->base.name);
 
-			ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info, 0, edp_dpcd,
+			ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info,
+						     luminance_range, 0, edp_dpcd,
 						     &current_level, &current_mode, false);
 			if (ret < 0)
 				return ret;
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index ef0786a0af4a..6f53921f5dce 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -849,6 +849,7 @@ struct drm_edp_backlight_info {
 
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
+		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
 		       u16 *current_level, u8 *current_mode, bool need_luminance);
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-- 
2.34.1

[PATCH 04/13] drm/dp: Move from u16 to u32 for max in drm_edp_backlight_info

[Suraj Kandpal]

Use u32 instead of u16 for max variable in drm_edp_backlight_info
since it can now hold max luminance range value which is u32.
We will set this max with max_luminance value when luminance_set is
true.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 10 +++++++---
 include/drm/display/drm_dp_helper.h     |  2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 3b309ac5190b..1322bdfb6c8b 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4270,9 +4270,13 @@ drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl
 		return -EINVAL;
 	}
 
-	ret = drm_edp_backlight_probe_max(aux, bl, driver_pwm_freq_hz, edp_dpcd);
-	if (ret < 0)
-		return ret;
+	if (bl->luminance_set) {
+		bl->max = lr->max_luminance;
+	} else {
+		ret = drm_edp_backlight_probe_max(aux, bl, driver_pwm_freq_hz, edp_dpcd);
+		if (ret < 0)
+			return ret;
+	}
 
 	ret = drm_edp_backlight_probe_state(aux, bl, current_mode);
 	if (ret < 0)
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 6f53921f5dce..39d644495f3e 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -839,7 +839,7 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
 struct drm_edp_backlight_info {
 	u8 pwmgen_bit_count;
 	u8 pwm_freq_pre_divider;
-	u16 max;
+	u32 max;
 
 	bool lsb_reg_used : 1;
 	bool aux_enable : 1;
-- 
2.34.1

[PATCH 05/13] drm/dp: Change current_level argument type to u32

[Suraj Kandpal]

Change the current_level argument type to u32 from u16
since it can now carry the value which it gets from
DP_EDP_PANEL_TARGET_LUMINANCE_VALUE.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c               | 4 ++--
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_backlight.c           | 2 +-
 include/drm/display/drm_dp_helper.h                   | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 1322bdfb6c8b..c58973d8c5f0 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4247,7 +4247,7 @@ int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
 		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
-		       u16 *current_level, u8 *current_mode, bool need_luminance)
+		       u32 *current_level, u8 *current_mode, bool need_luminance)
 {
 	int ret;
 
@@ -4355,7 +4355,7 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
 {
 	struct dp_aux_backlight *bl;
 	struct backlight_properties props = { 0 };
-	u16 current_level;
+	u32 current_level;
 	u8 current_mode;
 	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
 	int ret;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index abb5ad4eef5f..be740fb72ebc 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -581,7 +581,7 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
 		&connector->base.display_info.luminance_range;
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_panel *panel = &connector->panel;
-	u16 current_level;
+	u32 current_level;
 	u8 current_mode;
 	int ret;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index a3681e101d56..a430ee30060e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -247,7 +247,7 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 
 	if (nv_conn->type == DCB_CONNECTOR_eDP) {
 		int ret;
-		u16 current_level;
+		u32 current_level;
 		u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
 		u8 current_mode;
 
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 39d644495f3e..62be80417ded 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -851,7 +851,7 @@ int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
 		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
-		       u16 *current_level, u8 *current_mode, bool need_luminance);
+		       u32 *current_level, u8 *current_mode, bool need_luminance);
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
 				u16 level);
 int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-- 
2.34.1

[PATCH 06/13] drm/dp: Modify drm_edp_probe_state

[Suraj Kandpal]

Modify drm_edp_probe_state to read current level from
DP_EDP_PANEL_TARGET_LUMINANCE_VALUE. We divide it by
1000 since the value in this register is in millinits.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 35 ++++++++++++++++++-------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index c58973d8c5f0..bb1242a1bf6b 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4185,7 +4185,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
 			      u8 *current_mode)
 {
 	int ret;
-	u8 buf[2];
+	u8 buf[3];
 	u8 mode_reg;
 
 	ret = drm_dp_dpcd_read_byte(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &mode_reg);
@@ -4202,17 +4202,32 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
 	if (*current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
 		int size = 1 + bl->lsb_reg_used;
 
-		ret = drm_dp_dpcd_read_data(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, buf, size);
-		if (ret < 0) {
-			drm_dbg_kms(aux->drm_dev, "%s: Failed to read backlight level: %d\n",
-				    aux->name, ret);
-			return ret;
+		if (bl->luminance_set) {
+			ret = drm_dp_dpcd_read_data(aux, DP_EDP_PANEL_TARGET_LUMINANCE_VALUE,
+						    buf, sizeof(buf));
+			if (ret < 0) {
+				drm_dbg_kms(aux->drm_dev,
+					    "%s: Failed to read backlight level: %d\n",
+					    aux->name, ret);
+				return ret;
 		}
 
-		if (bl->lsb_reg_used)
-			return (buf[0] << 8) | buf[1];
-		else
-			return buf[0];
+		return (buf[0] | buf[1] << 8 | buf[2] << 16) / 1000;
+		} else {
+			ret = drm_dp_dpcd_read_data(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
+						    buf, size);
+			if (ret < 0) {
+				drm_dbg_kms(aux->drm_dev,
+					    "%s: Failed to read backlight level: %d\n",
+					    aux->name, ret);
+				return ret;
+			}
+
+			if (bl->lsb_reg_used)
+				return (buf[0] << 8) | buf[1];
+			else
+				return buf[0];
+		}
 	}
 
 	/*
-- 
2.34.1

[PATCH 07/13] drm/dp: Change argument type for drm_edp_backlight_set_level

[Suraj Kandpal]

Use u32 for level variable as one may need to pass value for
DP_EDP_PANEL_TARGET_LUMINANCE_VALUE.

--v2
-Typecase is not needed [Jani]

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 2 +-
 include/drm/display/drm_dp_helper.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index bb1242a1bf6b..c4c52fb37191 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -3933,7 +3933,7 @@ EXPORT_SYMBOL(drm_dp_pcon_convert_rgb_to_ycbcr);
  * Returns: %0 on success, negative error code on failure
  */
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-				u16 level)
+				u32 level)
 {
 	int ret;
 	u8 buf[2] = { 0 };
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 62be80417ded..6bce0176efd3 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -853,7 +853,7 @@ drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
 		       u32 *current_level, u8 *current_mode, bool need_luminance);
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-				u16 level);
+				u32 level);
 int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
 			     u16 level);
 int drm_edp_backlight_disable(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl);
-- 
2.34.1

[PATCH 08/13] drm/dp: Modify drm_edp_backlight_set_level

[Suraj Kandpal]

Modify drm_edp_backlight_set_level to be able to set the value
for register in DP_EDP_PANEL_TARGET_LUMINANCE_VALUE. We multiply
the level with 1000 since we get the value in Nits and the
register accepts it in milliNits.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index c4c52fb37191..dc0bda84d211 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -3936,20 +3936,28 @@ int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_bac
 				u32 level)
 {
 	int ret;
-	u8 buf[2] = { 0 };
+	unsigned int offset = DP_EDP_BACKLIGHT_BRIGHTNESS_MSB;
+	u8 buf[3] = { 0 };
 
 	/* The panel uses the PWM for controlling brightness levels */
-	if (!bl->aux_set)
+	if (!(bl->aux_set || bl->luminance_set))
 		return 0;
 
-	if (bl->lsb_reg_used) {
+	if (bl->luminance_set) {
+		level = level * 1000;
+		level &= 0xffffff;
+		buf[0] = (level & 0x0000ff);
+		buf[1] = (level & 0x00ff00) >> 8;
+		buf[2] = (level & 0xff0000) >> 16;
+		offset = DP_EDP_PANEL_TARGET_LUMINANCE_VALUE;
+	} else if (bl->lsb_reg_used) {
 		buf[0] = (level & 0xff00) >> 8;
 		buf[1] = (level & 0x00ff);
 	} else {
 		buf[0] = level;
 	}
 
-	ret = drm_dp_dpcd_write_data(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, buf, sizeof(buf));
+	ret = drm_dp_dpcd_write_data(aux, offset, buf, sizeof(buf));
 	if (ret < 0) {
 		drm_err(aux->drm_dev,
 			"%s: Failed to write aux backlight level: %d\n",
-- 
2.34.1

[PATCH 09/13] drm/dp: Change argument type of drm_edp_backlight_enable

[Suraj Kandpal]

Change the argument type to u32 for the default level being sent
since it has to now account for luminance value which has to be
set for DP_EDP_PANEL_LUMINANCE_TARGET_VALUE.

--v2
-No need to typecast [Jani]

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 2 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
 include/drm/display/drm_dp_helper.h     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index dc0bda84d211..0421b2ed9bd4 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4020,7 +4020,7 @@ drm_edp_backlight_set_enable(struct drm_dp_aux *aux, const struct drm_edp_backli
  * Returns: %0 on success, negative error code on failure.
  */
 int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-			     const u16 level)
+			     const u32 level)
 {
 	int ret;
 	u8 dpcd_buf;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 725331638a15..e3b8f6f510ef 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1829,7 +1829,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta
 		backlight = nv_connector->backlight;
 		if (backlight && backlight->uses_dpcd)
 			drm_edp_backlight_enable(&nv_connector->aux, &backlight->edp_info,
-						 (u16)backlight->dev->props.brightness);
+						 backlight->dev->props.brightness);
 #endif
 
 		break;
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 6bce0176efd3..b6c03d3ca6c3 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -855,7 +855,7 @@ drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
 				u32 level);
 int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-			     u16 level);
+			     u32 level);
 int drm_edp_backlight_disable(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl);
 
 #if IS_ENABLED(CONFIG_DRM_KMS_HELPER) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
-- 
2.34.1

[PATCH 10/13] drm/dp: Enable backlight control using luminance

[Suraj Kandpal]

Add flag to enable brightness control via luminance value
when enabling edp backlight.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 0421b2ed9bd4..4e2caba8311a 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4030,6 +4030,9 @@ int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct drm_edp_backli
 	else
 		dpcd_buf = DP_EDP_BACKLIGHT_CONTROL_MODE_PWM;
 
+	if (bl->luminance_set)
+		dpcd_buf |= DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE;
+
 	if (bl->pwmgen_bit_count) {
 		ret = drm_dp_dpcd_write_byte(aux, DP_EDP_PWMGEN_BIT_COUNT, bl->pwmgen_bit_count);
 		if (ret < 0)
-- 
2.34.1

[PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight

[Suraj Kandpal]

Now that drm_edp_backlight init has been to be able to setup
brightness manipulation via luminance we can just use that.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 .../drm/i915/display/intel_dp_aux_backlight.c | 100 +++++++++---------
 1 file changed, 49 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index be740fb72ebc..2eff9b545390 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -585,9 +585,36 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
 	u8 current_mode;
 	int ret;
 
-	if (panel->backlight.edp.vesa.luminance_control_support) {
+	ret = drm_edp_backlight_init(&intel_dp->aux, &panel->backlight.edp.vesa.info,
+				     luminance_range, panel->vbt.backlight.pwm_freq_hz,
+				     intel_dp->edp_dpcd, &current_level, &current_mode, true);
+	if (ret < 0)
+		return ret;
+
+	drm_dbg_kms(display->drm,
+		    "[CONNECTOR:%d:%s] AUX VESA backlight enable is controlled through %s\n",
+		    connector->base.base.id, connector->base.name,
+		    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_enable));
+	drm_dbg_kms(display->drm,
+		    "[CONNECTOR:%d:%s] AUX VESA backlight level is controlled through %s\n",
+		    connector->base.base.id, connector->base.name,
+		    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_set));
+
+	if (!panel->backlight.edp.vesa.info.luminance_set ||
+	    !panel->backlight.edp.vesa.info.aux_set ||
+	    !panel->backlight.edp.vesa.info.aux_enable) {
+		ret = panel->backlight.pwm_funcs->setup(connector, pipe);
+		if (ret < 0) {
+			drm_err(display->drm,
+				"[CONNECTOR:%d:%s] Failed to setup PWM backlight controls for eDP backlight: %d\n",
+				connector->base.base.id, connector->base.name, ret);
+			return ret;
+		}
+	}
+
+	if (panel->backlight.edp.vesa.info.luminance_set) {
 		if (luminance_range->max_luminance) {
-			panel->backlight.max = luminance_range->max_luminance;
+			panel->backlight.max = panel->backlight.edp.vesa.info.max;
 			panel->backlight.min = luminance_range->min_luminance;
 		} else {
 			panel->backlight.max = 512;
@@ -596,57 +623,28 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
 		panel->backlight.level = intel_dp_aux_vesa_get_backlight(connector, 0);
 		panel->backlight.enabled = panel->backlight.level != 0;
 		drm_dbg_kms(display->drm,
-			    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level is controlled through DPCD\n",
-			    connector->base.base.id, connector->base.name);
-	} else {
-		ret = drm_edp_backlight_init(&intel_dp->aux, &panel->backlight.edp.vesa.info,
-					     luminance_range, panel->vbt.backlight.pwm_freq_hz,
-					     intel_dp->edp_dpcd, &current_level, &current_mode,
-					     false);
-		if (ret < 0)
-			return ret;
-
-		drm_dbg_kms(display->drm,
-			    "[CONNECTOR:%d:%s] AUX VESA backlight enable is controlled through %s\n",
-			    connector->base.base.id, connector->base.name,
-			    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_enable));
-		drm_dbg_kms(display->drm,
-			    "[CONNECTOR:%d:%s] AUX VESA backlight level is controlled through %s\n",
-			    connector->base.base.id, connector->base.name,
-			    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_set));
-
-		if (!panel->backlight.edp.vesa.info.aux_set ||
-		    !panel->backlight.edp.vesa.info.aux_enable) {
-			ret = panel->backlight.pwm_funcs->setup(connector, pipe);
-			if (ret < 0) {
-				drm_err(display->drm,
-					"[CONNECTOR:%d:%s] Failed to setup PWM backlight controls for eDP backlight: %d\n",
-					connector->base.base.id, connector->base.name, ret);
-				return ret;
-			}
+		    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level is controlled through DPCD\n",
+		    connector->base.base.id, connector->base.name);
+	} else if (panel->backlight.edp.vesa.info.aux_set) {
+		panel->backlight.max = panel->backlight.edp.vesa.info.max;
+		panel->backlight.min = 0;
+		if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
+			panel->backlight.level = current_level;
+			panel->backlight.enabled = panel->backlight.level != 0;
+		} else {
+			panel->backlight.level = panel->backlight.max;
+			panel->backlight.enabled = false;
 		}
-
-		if (panel->backlight.edp.vesa.info.aux_set) {
-			panel->backlight.max = panel->backlight.edp.vesa.info.max;
-			panel->backlight.min = 0;
-			if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
-				panel->backlight.level = current_level;
-				panel->backlight.enabled = panel->backlight.level != 0;
-			} else {
-				panel->backlight.level = panel->backlight.max;
-				panel->backlight.enabled = false;
-			}
+	} else {
+		panel->backlight.max = panel->backlight.pwm_level_max;
+		panel->backlight.min = panel->backlight.pwm_level_min;
+		if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
+			panel->backlight.level =
+				panel->backlight.pwm_funcs->get(connector, pipe);
+			panel->backlight.enabled = panel->backlight.pwm_enabled;
 		} else {
-			panel->backlight.max = panel->backlight.pwm_level_max;
-			panel->backlight.min = panel->backlight.pwm_level_min;
-			if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
-				panel->backlight.level =
-					panel->backlight.pwm_funcs->get(connector, pipe);
-				panel->backlight.enabled = panel->backlight.pwm_enabled;
-			} else {
-				panel->backlight.level = panel->backlight.max;
-				panel->backlight.enabled = false;
-			}
+			panel->backlight.level = panel->backlight.max;
+			panel->backlight.enabled = false;
 		}
 	}
 
-- 
2.34.1

[PATCH 12/13] drm/i915/backlight: Use drm helper to set edp backlight

[Suraj Kandpal]

Now that the drm helper sets the backlight using luminance
too we can use that. Remove the obselete function.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 .../drm/i915/display/intel_dp_aux_backlight.c | 34 ++-----------------
 1 file changed, 3 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 2eff9b545390..95b29d9af335 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -476,31 +476,6 @@ static u32 intel_dp_aux_vesa_get_backlight(struct intel_connector *connector, en
 	return connector->panel.backlight.level;
 }
 
-static int
-intel_dp_aux_vesa_set_luminance(struct intel_connector *connector, u32 level)
-{
-	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
-	u8 buf[3];
-	int ret;
-
-	level = level * 1000;
-	level &= 0xffffff;
-	buf[0] = (level & 0x0000ff);
-	buf[1] = (level & 0x00ff00) >> 8;
-	buf[2] = (level & 0xff0000) >> 16;
-
-	ret = drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_PANEL_TARGET_LUMINANCE_VALUE,
-				buf, sizeof(buf));
-	if (ret != sizeof(buf)) {
-		drm_err(intel_dp->aux.drm_dev,
-			"%s: Failed to set VESA Aux Luminance: %d\n",
-			intel_dp->aux.name, ret);
-		return -EINVAL;
-	} else {
-		return 0;
-	}
-}
-
 static void
 intel_dp_aux_vesa_set_backlight(const struct drm_connector_state *conn_state, u32 level)
 {
@@ -508,11 +483,6 @@ intel_dp_aux_vesa_set_backlight(const struct drm_connector_state *conn_state, u3
 	struct intel_panel *panel = &connector->panel;
 	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
 
-	if (panel->backlight.edp.vesa.luminance_control_support) {
-		if (!intel_dp_aux_vesa_set_luminance(connector, level))
-			return;
-	}
-
 	if (!panel->backlight.edp.vesa.info.aux_set) {
 		const u32 pwm_level = intel_backlight_level_to_pwm(connector, level);
 
@@ -538,7 +508,9 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state,
 		if (ret == 1)
 			return;
 
-		if (!intel_dp_aux_vesa_set_luminance(connector, level))
+		if (!drm_edp_backlight_set_level(&intel_dp->aux,
+						 &panel->backlight.edp.vesa.info,
+						 level))
 			return;
 	}
 
-- 
2.34.1

[PATCH 13/13] drm/i915/backlight: Use drm_edp_backlight_enable

[Suraj Kandpal]

Use drm dp helper to enable backlight now that it has been modified
to set PANEL_LUMINANCE_CONTROL_ENABLE bit based on if capability
supports it and the driver wants it. Remove the dead code.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 .../gpu/drm/i915/display/intel_dp_aux_backlight.c  | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 95b29d9af335..b8b0ace9e6fd 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -499,20 +499,6 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state,
 	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	struct intel_panel *panel = &connector->panel;
 	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
-	int ret;
-
-	if (panel->backlight.edp.vesa.luminance_control_support) {
-		ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
-					 DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE);
-
-		if (ret == 1)
-			return;
-
-		if (!drm_edp_backlight_set_level(&intel_dp->aux,
-						 &panel->backlight.edp.vesa.info,
-						 level))
-			return;
-	}
 
 	if (!panel->backlight.edp.vesa.info.aux_enable) {
 		u32 pwm_level;
-- 
2.34.1

Re: [PATCH 06/13] drm/dp: Modify drm_edp_probe_state

[kernel test robot]

Hi Suraj,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20250411]
[cannot apply to linus/master drm-intel/for-linux-next drm-intel/for-linux-next-fixes v6.15-rc2 v6.15-rc1 v6.14 v6.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Suraj-Kandpal/drm-dp-Introduce-new-member-in-drm_backlight_info/20250414-132323
base:   next-20250411
patch link:    https://lore.kernel.org/r/20250414041637.128039-7-suraj.kandpal%40intel.com
patch subject: [PATCH 06/13] drm/dp: Modify drm_edp_probe_state
config: i386-randconfig-141-20250414 (https://download.01.org/0day-ci/archive/20250507/202505071939.IQ3zWvvX-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505071939.IQ3zWvvX-lkp@intel.com/

smatch warnings:
drivers/gpu/drm/display/drm_dp_helper.c:4215 drm_edp_backlight_probe_state() warn: inconsistent indenting

vim +4215 drivers/gpu/drm/display/drm_dp_helper.c

  4182	
  4183	static inline int
  4184	drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
  4185				      u8 *current_mode)
  4186	{
  4187		int ret;
  4188		u8 buf[3];
  4189		u8 mode_reg;
  4190	
  4191		ret = drm_dp_dpcd_read_byte(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &mode_reg);
  4192		if (ret < 0) {
  4193			drm_dbg_kms(aux->drm_dev, "%s: Failed to read backlight mode: %d\n",
  4194				    aux->name, ret);
  4195			return ret < 0 ? ret : -EIO;
  4196		}
  4197	
  4198		*current_mode = (mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK);
  4199		if (!bl->aux_set)
  4200			return 0;
  4201	
  4202		if (*current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
  4203			int size = 1 + bl->lsb_reg_used;
  4204	
  4205			if (bl->luminance_set) {
  4206				ret = drm_dp_dpcd_read_data(aux, DP_EDP_PANEL_TARGET_LUMINANCE_VALUE,
  4207							    buf, sizeof(buf));
  4208				if (ret < 0) {
  4209					drm_dbg_kms(aux->drm_dev,
  4210						    "%s: Failed to read backlight level: %d\n",
  4211						    aux->name, ret);
  4212					return ret;
  4213			}
  4214	
> 4215			return (buf[0] | buf[1] << 8 | buf[2] << 16) / 1000;
  4216			} else {
  4217				ret = drm_dp_dpcd_read_data(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
  4218							    buf, size);
  4219				if (ret < 0) {
  4220					drm_dbg_kms(aux->drm_dev,
  4221						    "%s: Failed to read backlight level: %d\n",
  4222						    aux->name, ret);
  4223					return ret;
  4224				}
  4225	
  4226				if (bl->lsb_reg_used)
  4227					return (buf[0] << 8) | buf[1];
  4228				else
  4229					return buf[0];
  4230			}
  4231		}
  4232	
  4233		/*
  4234		 * If we're not in DPCD control mode yet, the programmed brightness value is meaningless and
  4235		 * the driver should assume max brightness
  4236		 */
  4237		return bl->max;
  4238	}
  4239	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Suraj Kandpal]

Add new argument to drm_edp_backlight_init which gives the
drm_luminance_range_info struct which will be needed to set the min
and max values for backlight.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
 drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
 include/drm/display/drm_dp_helper.h                   | 1 +
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 952b06414641..8d0f8366fd4a 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
  * interface.
  * @aux: The DP aux device to use for probing
  * @bl: The &drm_edp_backlight_info struct to fill out with information on the backlight
+ * @lr: The &drm_luminance_range_info struct which is used to get the min max when using
+ *luminance override
  * @driver_pwm_freq_hz: Optional PWM frequency from the driver in hz
  * @edp_dpcd: A cached copy of the eDP DPCD
  * @current_level: Where to store the probed brightness level, if any
@@ -4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct drm_edp_backlight_i
  */
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
+		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
 		       u16 *current_level, u8 *current_mode, bool need_luminance)
 {
@@ -4372,7 +4375,7 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
 
 	bl->aux = aux;
 
-	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+	ret = drm_edp_backlight_init(aux, &bl->info, NULL, 0, edp_dpcd,
 				     &current_level, &current_mode, false);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index dc6f6680774f..a4fee7c622ee 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -599,8 +599,9 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
 			    connector->base.base.id, connector->base.name);
 	} else {
 		ret = drm_edp_backlight_init(&intel_dp->aux, &panel->backlight.edp.vesa.info,
-					     panel->vbt.backlight.pwm_freq_hz, intel_dp->edp_dpcd,
-					     &current_level, &current_mode, false);
+					     luminance_range, panel->vbt.backlight.pwm_freq_hz,
+					     intel_dp->edp_dpcd, &current_level, &current_mode,
+					     false);
 		if (ret < 0)
 			return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index b938684a9422..a3681e101d56 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -234,6 +234,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 		    const struct backlight_ops **ops)
 {
 	struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
+	struct drm_luminance_range_info *luminance_range =
+		&nv_conn->base.display_info.luminance_range;
 
 	/*
 	 * Note when this runs the connectors have not been probed yet,
@@ -261,7 +263,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
 			NV_DEBUG(drm, "DPCD backlight controls supported on %s\n",
 				 nv_conn->base.name);
 
-			ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info, 0, edp_dpcd,
+			ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info,
+						     luminance_range, 0, edp_dpcd,
 						     &current_level, &current_mode, false);
 			if (ret < 0)
 				return ret;
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index e984e8bc2398..6ec165dc5e56 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -849,6 +849,7 @@ struct drm_edp_backlight_info {
 
 int
 drm_edp_backlight_init(struct drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
+		       struct drm_luminance_range_info *lr,
 		       u16 driver_pwm_freq_hz, const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
 		       u16 *current_level, u8 *current_mode, bool need_luminance);
 int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
-- 
2.34.1

RE: [PATCH 01/13] drm/dp: Introduce new member in drm_backlight_info

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:46 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 01/13] drm/dp: Introduce new member in drm_backlight_info
> 
> Introduce luminance_set flag which indicates if we can manipulate backlight
> using luminance value or not which is only possible after eDP v1.5.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------

>  drivers/gpu/drm/display/drm_dp_helper.c | 8 ++++++--
>  include/drm/display/drm_dp_helper.h     | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 57828f2b7b5a..41de7a92d76d 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4253,11 +4253,15 @@ drm_edp_backlight_init(struct drm_dp_aux *aux,
> struct drm_edp_backlight_info *bl
>  		bl->aux_set = true;
>  	if (edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>  		bl->lsb_reg_used = true;
> +	if ((edp_dpcd[0] & DP_EDP_15) && edp_dpcd[3] &
> +	    (DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE))
> +		bl->luminance_set = true;
> 
>  	/* Sanity check caps */
> -	if (!bl->aux_set && !(edp_dpcd[2] &
> DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) {
> +	if (!bl->aux_set && !(edp_dpcd[2] &
> DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP) &&
> +	    !bl->luminance_set) {
>  		drm_dbg_kms(aux->drm_dev,
> -			    "%s: Panel supports neither AUX or PWM brightness
> control? Aborting\n",
> +			    "%s: Panel does not support AUX, PWM or
> luminance-based
> +brightness control. Aborting\n",
>  			    aux->name);
>  		return -EINVAL;
>  	}
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index d9614e2c8939..b8fdc09737fc 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -844,6 +844,7 @@ struct drm_edp_backlight_info {
>  	bool lsb_reg_used : 1;
>  	bool aux_enable : 1;
>  	bool aux_set : 1;
> +	bool luminance_set : 1;
>  };
> 
>  int
> --
> 2.34.1

RE: [PATCH 02/13] drm/dp: Add argument in drm_edp_backlight_init

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:46 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 02/13] drm/dp: Add argument in drm_edp_backlight_init
> 
> Add bool argument in drm_edp_backlight init to provide the drivers option to
> choose if they want to use luminance values to manipulate brightness.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@gmail.com>

Thanks and Regards,
Arun R Murthy
--------------------

>  drivers/gpu/drm/display/drm_dp_helper.c               | 7 ++++---
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
>  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 2 +-
>  include/drm/display/drm_dp_helper.h                   | 2 +-
>  4 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 41de7a92d76d..99b27e5e3365 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4231,6 +4231,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux
> *aux, struct drm_edp_backlight_i
>   * @edp_dpcd: A cached copy of the eDP DPCD
>   * @current_level: Where to store the probed brightness level, if any
>   * @current_mode: Where to store the currently set backlight control mode
> + * @need_luminance: Tells us if a we want to manipulate backlight using
> + luminance values
>   *
>   * Initializes a &drm_edp_backlight_info struct by probing @aux for it's
> backlight capabilities,
>   * along with also probing the current and maximum supported brightness
> levels.
> @@ -4243,7 +4244,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux
> *aux, struct drm_edp_backlight_i  int  drm_edp_backlight_init(struct
> drm_dp_aux *aux, struct drm_edp_backlight_info *bl,
>  		       u16 driver_pwm_freq_hz, const u8
> edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
> -		       u16 *current_level, u8 *current_mode)
> +		       u16 *current_level, u8 *current_mode, bool
> need_luminance)
>  {
>  	int ret;
> 
> @@ -4254,7 +4255,7 @@ drm_edp_backlight_init(struct drm_dp_aux *aux,
> struct drm_edp_backlight_info *bl
>  	if (edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>  		bl->lsb_reg_used = true;
>  	if ((edp_dpcd[0] & DP_EDP_15) && edp_dpcd[3] &
> -	    (DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE))
> +	    (DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE) &&
> need_luminance)
>  		bl->luminance_set = true;
> 
>  	/* Sanity check caps */
> @@ -4372,7 +4373,7 @@ int drm_panel_dp_aux_backlight(struct drm_panel
> *panel, struct drm_dp_aux *aux)
>  	bl->aux = aux;
> 
>  	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
> -				     &current_level, &current_mode);
> +				     &current_level, &current_mode, false);
>  	if (ret < 0)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 20ab90acb351..d658e77b43d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -601,7 +601,7 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
>  	} else {
>  		ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> >backlight.edp.vesa.info,
>  					     panel->vbt.backlight.pwm_freq_hz,
> intel_dp->edp_dpcd,
> -					     &current_level, &current_mode);
> +					     &current_level, &current_mode,
> false);
>  		if (ret < 0)
>  			return ret;
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index d47442125fa1..b938684a9422 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -262,7 +262,7 @@ nv50_backlight_init(struct nouveau_backlight *bl,
>  				 nv_conn->base.name);
> 
>  			ret = drm_edp_backlight_init(&nv_conn->aux, &bl-
> >edp_info, 0, edp_dpcd,
> -						     &current_level,
> &current_mode);
> +						     &current_level,
> &current_mode, false);
>  			if (ret < 0)
>  				return ret;
> 
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index b8fdc09737fc..ef0786a0af4a 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -850,7 +850,7 @@ struct drm_edp_backlight_info {  int
> drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> drm_edp_backlight_info *bl,
>  		       u16 driver_pwm_freq_hz, const u8
> edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
> -		       u16 *current_level, u8 *current_mode);
> +		       u16 *current_level, u8 *current_mode, bool
> need_luminance);
>  int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
>  				u16 level);
>  int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> --
> 2.34.1

RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:46 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 03/13] drm/dp: Add argument for luminance range info in
> drm_edp_backlight_init
> 
> Add new argument to drm_edp_backlight_init which gives the
> drm_luminance_range_info struct which will be needed to set the min and max
> values for backlight.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
>  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
>  include/drm/display/drm_dp_helper.h                   | 1 +
>  4 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 99b27e5e3365..3b309ac5190b 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct drm_dp_aux
> *aux, struct drm_edp_backlight_i
>   * interface.
>   * @aux: The DP aux device to use for probing
>   * @bl: The &drm_edp_backlight_info struct to fill out with information on the
> backlight
> + * @lr: The &drm_luminance_range_info struct which is used to get the
> + min max when using *luminance override
>   * @driver_pwm_freq_hz: Optional PWM frequency from the driver in hz
>   * @edp_dpcd: A cached copy of the eDP DPCD
>   * @current_level: Where to store the probed brightness level, if any @@ -
> 4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux,
> struct drm_edp_backlight_i
>   */
>  int
>  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> drm_edp_backlight_info *bl,
> +		       struct drm_luminance_range_info *lr,
Would it be better to have this drm_luminance_range_info inside the drm_edp_backlight_info?

Thanks and Regards,
Arun R Murthy
--------------------

>  		       u16 driver_pwm_freq_hz, const u8
> edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
>  		       u16 *current_level, u8 *current_mode, bool
> need_luminance)  { @@ -4372,7 +4375,7 @@ int
> drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux
> *aux)
> 
>  	bl->aux = aux;
> 
> -	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
> +	ret = drm_edp_backlight_init(aux, &bl->info, NULL, 0, edp_dpcd,
>  				     &current_level, &current_mode, false);
>  	if (ret < 0)
>  		return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index d658e77b43d8..abb5ad4eef5f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -600,8 +600,9 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
>  			    connector->base.base.id, connector->base.name);
>  	} else {
>  		ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> >backlight.edp.vesa.info,
> -					     panel->vbt.backlight.pwm_freq_hz,
> intel_dp->edp_dpcd,
> -					     &current_level, &current_mode,
> false);
> +					     luminance_range, panel-
> >vbt.backlight.pwm_freq_hz,
> +					     intel_dp->edp_dpcd,
> &current_level, &current_mode,
> +					     false);
>  		if (ret < 0)
>  			return ret;
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index b938684a9422..a3681e101d56 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -234,6 +234,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
>  		    const struct backlight_ops **ops)  {
>  	struct nouveau_drm *drm = nouveau_drm(nv_encoder-
> >base.base.dev);
> +	struct drm_luminance_range_info *luminance_range =
> +		&nv_conn->base.display_info.luminance_range;
> 
>  	/*
>  	 * Note when this runs the connectors have not been probed yet, @@ -
> 261,7 +263,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
>  			NV_DEBUG(drm, "DPCD backlight controls supported
> on %s\n",
>  				 nv_conn->base.name);
> 
> -			ret = drm_edp_backlight_init(&nv_conn->aux, &bl-
> >edp_info, 0, edp_dpcd,
> +			ret = drm_edp_backlight_init(&nv_conn->aux, &bl-
> >edp_info,
> +						     luminance_range, 0,
> edp_dpcd,
>  						     &current_level,
> &current_mode, false);
>  			if (ret < 0)
>  				return ret;
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index ef0786a0af4a..6f53921f5dce 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -849,6 +849,7 @@ struct drm_edp_backlight_info {
> 
>  int
>  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> drm_edp_backlight_info *bl,
> +		       struct drm_luminance_range_info *lr,
>  		       u16 driver_pwm_freq_hz, const u8
> edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
>  		       u16 *current_level, u8 *current_mode, bool
> need_luminance);  int drm_edp_backlight_set_level(struct drm_dp_aux *aux,
> const struct drm_edp_backlight_info *bl,
> --
> 2.34.1

RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Kandpal, Suraj]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Thursday, June 12, 2025 11:45 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in
> drm_edp_backlight_init
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Sent: Monday, April 14, 2025 9:46 AM
> > To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > intel- xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> > <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> > Subject: [PATCH 03/13] drm/dp: Add argument for luminance range info
> > in drm_edp_backlight_init
> >
> > Add new argument to drm_edp_backlight_init which gives the
> > drm_luminance_range_info struct which will be needed to set the min
> > and max values for backlight.
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
> >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
> >  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
> >  include/drm/display/drm_dp_helper.h                   | 1 +
> >  4 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > b/drivers/gpu/drm/display/drm_dp_helper.c
> > index 99b27e5e3365..3b309ac5190b 100644
> > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > @@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct
> drm_dp_aux
> > *aux, struct drm_edp_backlight_i
> >   * interface.
> >   * @aux: The DP aux device to use for probing
> >   * @bl: The &drm_edp_backlight_info struct to fill out with
> > information on the backlight
> > + * @lr: The &drm_luminance_range_info struct which is used to get the
> > + min max when using *luminance override
> >   * @driver_pwm_freq_hz: Optional PWM frequency from the driver in hz
> >   * @edp_dpcd: A cached copy of the eDP DPCD
> >   * @current_level: Where to store the probed brightness level, if any
> > @@ -
> > 4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux
> > *aux, struct drm_edp_backlight_i
> >   */
> >  int
> >  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> > drm_edp_backlight_info *bl,
> > +		       struct drm_luminance_range_info *lr,
> Would it be better to have this drm_luminance_range_info inside the
> drm_edp_backlight_info?

The thing is we fill drm_edp_backlight_info struct in drm_edp_backlight_init
Which means we would have to pass it anyways. So having a reference of this in
drm_edp_backlight_info didn't make sense.

Regards,
Suraj Kandpal

> 
> Thanks and Regards,
> Arun R Murthy
> --------------------
> 
> >  		       u16 driver_pwm_freq_hz, const u8
> > edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
> >  		       u16 *current_level, u8 *current_mode, bool
> > need_luminance)  { @@ -4372,7 +4375,7 @@ int
> > drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux
> > *aux)
> >
> >  	bl->aux = aux;
> >
> > -	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
> > +	ret = drm_edp_backlight_init(aux, &bl->info, NULL, 0, edp_dpcd,
> >  				     &current_level, &current_mode, false);
> >  	if (ret < 0)
> >  		return ret;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > index d658e77b43d8..abb5ad4eef5f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > @@ -600,8 +600,9 @@ static int
> > intel_dp_aux_vesa_setup_backlight(struct
> > intel_connector *connector,
> >  			    connector->base.base.id, connector->base.name);
> >  	} else {
> >  		ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> > >backlight.edp.vesa.info,
> > -					     panel->vbt.backlight.pwm_freq_hz,
> > intel_dp->edp_dpcd,
> > -					     &current_level, &current_mode,
> > false);
> > +					     luminance_range, panel-
> > >vbt.backlight.pwm_freq_hz,
> > +					     intel_dp->edp_dpcd,
> > &current_level, &current_mode,
> > +					     false);
> >  		if (ret < 0)
> >  			return ret;
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > index b938684a9422..a3681e101d56 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > @@ -234,6 +234,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
> >  		    const struct backlight_ops **ops)  {
> >  	struct nouveau_drm *drm = nouveau_drm(nv_encoder-
> > >base.base.dev);
> > +	struct drm_luminance_range_info *luminance_range =
> > +		&nv_conn->base.display_info.luminance_range;
> >
> >  	/*
> >  	 * Note when this runs the connectors have not been probed yet,
> @@ -
> > 261,7 +263,8 @@ nv50_backlight_init(struct nouveau_backlight *bl,
> >  			NV_DEBUG(drm, "DPCD backlight controls supported
> on %s\n",
> >  				 nv_conn->base.name);
> >
> > -			ret = drm_edp_backlight_init(&nv_conn->aux, &bl-
> > >edp_info, 0, edp_dpcd,
> > +			ret = drm_edp_backlight_init(&nv_conn->aux, &bl-
> > >edp_info,
> > +						     luminance_range, 0,
> > edp_dpcd,
> >  						     &current_level,
> > &current_mode, false);
> >  			if (ret < 0)
> >  				return ret;
> > diff --git a/include/drm/display/drm_dp_helper.h
> > b/include/drm/display/drm_dp_helper.h
> > index ef0786a0af4a..6f53921f5dce 100644
> > --- a/include/drm/display/drm_dp_helper.h
> > +++ b/include/drm/display/drm_dp_helper.h
> > @@ -849,6 +849,7 @@ struct drm_edp_backlight_info {
> >
> >  int
> >  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> > drm_edp_backlight_info *bl,
> > +		       struct drm_luminance_range_info *lr,
> >  		       u16 driver_pwm_freq_hz, const u8
> > edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
> >  		       u16 *current_level, u8 *current_mode, bool
> need_luminance);
> > int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct
> > drm_edp_backlight_info *bl,
> > --
> > 2.34.1

RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Murthy, Arun R]

> > > -----Original Message-----
> > > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > > Sent: Monday, April 14, 2025 9:46 AM
> > > To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > > intel- xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> > > <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> > > Subject: [PATCH 03/13] drm/dp: Add argument for luminance range info
> > > in drm_edp_backlight_init
> > >
> > > Add new argument to drm_edp_backlight_init which gives the
> > > drm_luminance_range_info struct which will be needed to set the min
> > > and max values for backlight.
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > >  drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
> > >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
> > >  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
> > >  include/drm/display/drm_dp_helper.h                   | 1 +
> > >  4 files changed, 12 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > > b/drivers/gpu/drm/display/drm_dp_helper.c
> > > index 99b27e5e3365..3b309ac5190b 100644
> > > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > > @@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct
> > drm_dp_aux
> > > *aux, struct drm_edp_backlight_i
> > >   * interface.
> > >   * @aux: The DP aux device to use for probing
> > >   * @bl: The &drm_edp_backlight_info struct to fill out with
> > > information on the backlight
> > > + * @lr: The &drm_luminance_range_info struct which is used to get
> > > + the min max when using *luminance override
> > >   * @driver_pwm_freq_hz: Optional PWM frequency from the driver in hz
> > >   * @edp_dpcd: A cached copy of the eDP DPCD
> > >   * @current_level: Where to store the probed brightness level, if
> > > any @@ -
> > > 4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux
> > > *aux, struct drm_edp_backlight_i
> > >   */
> > >  int
> > >  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> > > drm_edp_backlight_info *bl,
> > > +		       struct drm_luminance_range_info *lr,
> > Would it be better to have this drm_luminance_range_info inside the
> > drm_edp_backlight_info?
> 
> The thing is we fill drm_edp_backlight_info struct in drm_edp_backlight_init
> Which means we would have to pass it anyways. So having a reference of this in
> drm_edp_backlight_info didn't make sense.
> 
The main intention for this ask is two xx_info struct passed as argument.
Moreover luminance is part of backlight and this new element is _info and there already exists backlight_info. So wondering is luminance can be put inside backlight_info. The caller of this function can fill the luminance part and then make a call.

Thanks and Regards,
Arun R Murthy
--------------------

RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Kandpal, Suraj]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Thursday, June 12, 2025 4:43 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in
> drm_edp_backlight_init
> 
> > > > -----Original Message-----
> > > > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > > > Sent: Monday, April 14, 2025 9:46 AM
> > > > To: nouveau@lists.freedesktop.org;
> > > > dri-devel@lists.freedesktop.org;
> > > > intel- xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > > > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> > > > <arun.r.murthy@intel.com>; Kandpal, Suraj
> > > > <suraj.kandpal@intel.com>
> > > > Subject: [PATCH 03/13] drm/dp: Add argument for luminance range
> > > > info in drm_edp_backlight_init
> > > >
> > > > Add new argument to drm_edp_backlight_init which gives the
> > > > drm_luminance_range_info struct which will be needed to set the
> > > > min and max values for backlight.
> > > >
> > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
> > > >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
> > > >  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
> > > >  include/drm/display/drm_dp_helper.h                   | 1 +
> > > >  4 files changed, 12 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > index 99b27e5e3365..3b309ac5190b 100644
> > > > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > @@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct
> > > drm_dp_aux
> > > > *aux, struct drm_edp_backlight_i
> > > >   * interface.
> > > >   * @aux: The DP aux device to use for probing
> > > >   * @bl: The &drm_edp_backlight_info struct to fill out with
> > > > information on the backlight
> > > > + * @lr: The &drm_luminance_range_info struct which is used to get
> > > > + the min max when using *luminance override
> > > >   * @driver_pwm_freq_hz: Optional PWM frequency from the driver in
> hz
> > > >   * @edp_dpcd: A cached copy of the eDP DPCD
> > > >   * @current_level: Where to store the probed brightness level, if
> > > > any @@ -
> > > > 4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct
> drm_dp_aux
> > > > *aux, struct drm_edp_backlight_i
> > > >   */
> > > >  int
> > > >  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> > > > drm_edp_backlight_info *bl,
> > > > +		       struct drm_luminance_range_info *lr,
> > > Would it be better to have this drm_luminance_range_info inside the
> > > drm_edp_backlight_info?
> >
> > The thing is we fill drm_edp_backlight_info struct in
> > drm_edp_backlight_init Which means we would have to pass it anyways.
> > So having a reference of this in drm_edp_backlight_info didn't make sense.
> >
> The main intention for this ask is two xx_info struct passed as argument.
> Moreover luminance is part of backlight and this new element is _info and
> there already exists backlight_info. So wondering is luminance can be put
> inside backlight_info. The caller of this function can fill the luminance part
> and then make a call.
> 

I see you point but the thing is luminance range is not something we will be using later and is
only used the set the max level of brightness that can be set.
That being said I do get your point on sending two xx_info struct here, I was thinking we send only the
U32 max luminance here since that's the only one we actually use. Drivers can send the max luminance they like.
What do you think?

Regards,
Suraj Kandpal

> Thanks and Regards,
> Arun R Murthy
> --------------------

RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Murthy, Arun R]

> > -----Original Message-----
> > From: Murthy, Arun R <arun.r.murthy@intel.com>
> > Sent: Thursday, June 12, 2025 4:43 PM
> > To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> > nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> > xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > Subject: RE: [PATCH 03/13] drm/dp: Add argument for luminance range
> > info in drm_edp_backlight_init
> >
> > > > > -----Original Message-----
> > > > > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > > > > Sent: Monday, April 14, 2025 9:46 AM
> > > > > To: nouveau@lists.freedesktop.org;
> > > > > dri-devel@lists.freedesktop.org;
> > > > > intel- xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > > > > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun
> > > > > R <arun.r.murthy@intel.com>; Kandpal, Suraj
> > > > > <suraj.kandpal@intel.com>
> > > > > Subject: [PATCH 03/13] drm/dp: Add argument for luminance range
> > > > > info in drm_edp_backlight_init
> > > > >
> > > > > Add new argument to drm_edp_backlight_init which gives the
> > > > > drm_luminance_range_info struct which will be needed to set the
> > > > > min and max values for backlight.
> > > > >
> > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
> > > > >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
> > > > >  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
> > > > >  include/drm/display/drm_dp_helper.h                   | 1 +
> > > > >  4 files changed, 12 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > index 99b27e5e3365..3b309ac5190b 100644
> > > > > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > @@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct
> > > > drm_dp_aux
> > > > > *aux, struct drm_edp_backlight_i
> > > > >   * interface.
> > > > >   * @aux: The DP aux device to use for probing
> > > > >   * @bl: The &drm_edp_backlight_info struct to fill out with
> > > > > information on the backlight
> > > > > + * @lr: The &drm_luminance_range_info struct which is used to
> > > > > + get the min max when using *luminance override
> > > > >   * @driver_pwm_freq_hz: Optional PWM frequency from the driver
> > > > > in
> > hz
> > > > >   * @edp_dpcd: A cached copy of the eDP DPCD
> > > > >   * @current_level: Where to store the probed brightness level,
> > > > > if any @@ -
> > > > > 4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct
> > drm_dp_aux
> > > > > *aux, struct drm_edp_backlight_i
> > > > >   */
> > > > >  int
> > > > >  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> > > > > drm_edp_backlight_info *bl,
> > > > > +		       struct drm_luminance_range_info *lr,
> > > > Would it be better to have this drm_luminance_range_info inside
> > > > the drm_edp_backlight_info?
> > >
> > > The thing is we fill drm_edp_backlight_info struct in
> > > drm_edp_backlight_init Which means we would have to pass it anyways.
> > > So having a reference of this in drm_edp_backlight_info didn't make sense.
> > >
> > The main intention for this ask is two xx_info struct passed as argument.
> > Moreover luminance is part of backlight and this new element is _info
> > and there already exists backlight_info. So wondering is luminance can
> > be put inside backlight_info. The caller of this function can fill the
> > luminance part and then make a call.
> >
> 
> I see you point but the thing is luminance range is not something we will be
> using later and is only used the set the max level of brightness that can be set.
> That being said I do get your point on sending two xx_info struct here, I was
> thinking we send only the
> U32 max luminance here since that's the only one we actually use. Drivers can
> send the max luminance they like.
> What do you think?
> 
That should be better! 4th patch can be squashed with this one.

Thanks and Regards,
Arun R Murthy
--------------------
> Regards,
> Suraj Kandpal
> 
> > Thanks and Regards,
> > Arun R Murthy
> > --------------------

RE: [PATCH 04/13] drm/dp: Move from u16 to u32 for max in drm_edp_backlight_info

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:46 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 04/13] drm/dp: Move from u16 to u32 for max in
> drm_edp_backlight_info
> 
> Use u32 instead of u16 for max variable in drm_edp_backlight_info since it can
> now hold max luminance range value which is u32.
> We will set this max with max_luminance value when luminance_set is true.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c | 10 +++++++---
>  include/drm/display/drm_dp_helper.h     |  2 +-
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 3b309ac5190b..1322bdfb6c8b 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4270,9 +4270,13 @@ drm_edp_backlight_init(struct drm_dp_aux *aux,
> struct drm_edp_backlight_info *bl
>  		return -EINVAL;
>  	}
> 
> -	ret = drm_edp_backlight_probe_max(aux, bl, driver_pwm_freq_hz,
> edp_dpcd);
> -	if (ret < 0)
> -		return ret;
> +	if (bl->luminance_set) {
> +		bl->max = lr->max_luminance;
This change may not be required as in 3rd patch the max luminance will directly be copied to the backlight_info.

Other than this change patch looks good.
Thanks and Regards,
Arun R Murthy
--------------------
> +	} else {
> +		ret = drm_edp_backlight_probe_max(aux, bl,
> driver_pwm_freq_hz, edp_dpcd);
> +		if (ret < 0)
> +			return ret;
> +	}
> 
>  	ret = drm_edp_backlight_probe_state(aux, bl, current_mode);
>  	if (ret < 0)
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 6f53921f5dce..39d644495f3e 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -839,7 +839,7 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc,
> enum drm_dp_quirk quirk)  struct drm_edp_backlight_info {
>  	u8 pwmgen_bit_count;
>  	u8 pwm_freq_pre_divider;
> -	u16 max;
> +	u32 max;
> 
>  	bool lsb_reg_used : 1;
>  	bool aux_enable : 1;
> --
> 2.34.1

RE: [PATCH 05/13] drm/dp: Change current_level argument type to u32

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:46 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 05/13] drm/dp: Change current_level argument type to u32
> 
> Change the current_level argument type to u32 from u16 since it can now carry
> the value which it gets from DP_EDP_PANEL_TARGET_LUMINANCE_VALUE.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
Looks good to me!
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------
>  drivers/gpu/drm/display/drm_dp_helper.c               | 4 ++--
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
>  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 2 +-
>  include/drm/display/drm_dp_helper.h                   | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 1322bdfb6c8b..c58973d8c5f0 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4247,7 +4247,7 @@ int
>  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> drm_edp_backlight_info *bl,
>  		       struct drm_luminance_range_info *lr,
>  		       u16 driver_pwm_freq_hz, const u8
> edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
> -		       u16 *current_level, u8 *current_mode, bool
> need_luminance)
> +		       u32 *current_level, u8 *current_mode, bool
> need_luminance)
>  {
>  	int ret;
> 
> @@ -4355,7 +4355,7 @@ int drm_panel_dp_aux_backlight(struct drm_panel
> *panel, struct drm_dp_aux *aux)  {
>  	struct dp_aux_backlight *bl;
>  	struct backlight_properties props = { 0 };
> -	u16 current_level;
> +	u32 current_level;
>  	u8 current_mode;
>  	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
>  	int ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index abb5ad4eef5f..be740fb72ebc 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -581,7 +581,7 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
>  		&connector->base.display_info.luminance_range;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_panel *panel = &connector->panel;
> -	u16 current_level;
> +	u32 current_level;
>  	u8 current_mode;
>  	int ret;
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index a3681e101d56..a430ee30060e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -247,7 +247,7 @@ nv50_backlight_init(struct nouveau_backlight *bl,
> 
>  	if (nv_conn->type == DCB_CONNECTOR_eDP) {
>  		int ret;
> -		u16 current_level;
> +		u32 current_level;
>  		u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
>  		u8 current_mode;
> 
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 39d644495f3e..62be80417ded 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -851,7 +851,7 @@ int
>  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> drm_edp_backlight_info *bl,
>  		       struct drm_luminance_range_info *lr,
>  		       u16 driver_pwm_freq_hz, const u8
> edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
> -		       u16 *current_level, u8 *current_mode, bool
> need_luminance);
> +		       u32 *current_level, u8 *current_mode, bool
> need_luminance);
>  int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
>  				u16 level);
>  int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> --
> 2.34.1

RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Kandpal, Suraj]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Thursday, June 19, 2025 10:47 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in
> drm_edp_backlight_init
> 
> > > -----Original Message-----
> > > From: Murthy, Arun R <arun.r.murthy@intel.com>
> > > Sent: Thursday, June 12, 2025 4:43 PM
> > > To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> > > nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > > intel- xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > > Subject: RE: [PATCH 03/13] drm/dp: Add argument for luminance range
> > > info in drm_edp_backlight_init
> > >
> > > > > > -----Original Message-----
> > > > > > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > > > > > Sent: Monday, April 14, 2025 9:46 AM
> > > > > > To: nouveau@lists.freedesktop.org;
> > > > > > dri-devel@lists.freedesktop.org;
> > > > > > intel- xe@lists.freedesktop.org;
> > > > > > intel-gfx@lists.freedesktop.org
> > > > > > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy,
> > > > > > Arun R <arun.r.murthy@intel.com>; Kandpal, Suraj
> > > > > > <suraj.kandpal@intel.com>
> > > > > > Subject: [PATCH 03/13] drm/dp: Add argument for luminance
> > > > > > range info in drm_edp_backlight_init
> > > > > >
> > > > > > Add new argument to drm_edp_backlight_init which gives the
> > > > > > drm_luminance_range_info struct which will be needed to set
> > > > > > the min and max values for backlight.
> > > > > >
> > > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
> > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
> > > > > >  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
> > > > > >  include/drm/display/drm_dp_helper.h                   | 1 +
> > > > > >  4 files changed, 12 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > > b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > > index 99b27e5e3365..3b309ac5190b 100644
> > > > > > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > > > @@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct
> > > > > drm_dp_aux
> > > > > > *aux, struct drm_edp_backlight_i
> > > > > >   * interface.
> > > > > >   * @aux: The DP aux device to use for probing
> > > > > >   * @bl: The &drm_edp_backlight_info struct to fill out with
> > > > > > information on the backlight
> > > > > > + * @lr: The &drm_luminance_range_info struct which is used to
> > > > > > + get the min max when using *luminance override
> > > > > >   * @driver_pwm_freq_hz: Optional PWM frequency from the
> > > > > > driver in
> > > hz
> > > > > >   * @edp_dpcd: A cached copy of the eDP DPCD
> > > > > >   * @current_level: Where to store the probed brightness
> > > > > > level, if any @@ -
> > > > > > 4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct
> > > drm_dp_aux
> > > > > > *aux, struct drm_edp_backlight_i
> > > > > >   */
> > > > > >  int
> > > > > >  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> > > > > > drm_edp_backlight_info *bl,
> > > > > > +		       struct drm_luminance_range_info *lr,
> > > > > Would it be better to have this drm_luminance_range_info inside
> > > > > the drm_edp_backlight_info?
> > > >
> > > > The thing is we fill drm_edp_backlight_info struct in
> > > > drm_edp_backlight_init Which means we would have to pass it
> anyways.
> > > > So having a reference of this in drm_edp_backlight_info didn't make
> sense.
> > > >
> > > The main intention for this ask is two xx_info struct passed as argument.
> > > Moreover luminance is part of backlight and this new element is
> > > _info and there already exists backlight_info. So wondering is
> > > luminance can be put inside backlight_info. The caller of this
> > > function can fill the luminance part and then make a call.
> > >
> >
> > I see you point but the thing is luminance range is not something we
> > will be using later and is only used the set the max level of brightness that
> can be set.
> > That being said I do get your point on sending two xx_info struct
> > here, I was thinking we send only the
> > U32 max luminance here since that's the only one we actually use.
> > Drivers can send the max luminance they like.
> > What do you think?
> >
> That should be better! 4th patch can be squashed with this one.
> 

Sure will do that

Regards,
Suraj Kandpal

> Thanks and Regards,
> Arun R Murthy
> --------------------
> > Regards,
> > Suraj Kandpal
> >
> > > Thanks and Regards,
> > > Arun R Murthy
> > > --------------------

RE: [PATCH 06/13] drm/dp: Modify drm_edp_probe_state

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 06/13] drm/dp: Modify drm_edp_probe_state
> 
> Modify drm_edp_probe_state to read current level from
> DP_EDP_PANEL_TARGET_LUMINANCE_VALUE. We divide it by
> 1000 since the value in this register is in millinits.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c | 35 ++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index c58973d8c5f0..bb1242a1bf6b 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4185,7 +4185,7 @@ drm_edp_backlight_probe_state(struct drm_dp_aux
> *aux, struct drm_edp_backlight_i
>  			      u8 *current_mode)
>  {
>  	int ret;
> -	u8 buf[2];
> +	u8 buf[3];
>  	u8 mode_reg;
> 
>  	ret = drm_dp_dpcd_read_byte(aux,
> DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &mode_reg); @@ -4202,17
> +4202,32 @@ drm_edp_backlight_probe_state(struct drm_dp_aux *aux, struct
> drm_edp_backlight_i
>  	if (*current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
>  		int size = 1 + bl->lsb_reg_used;
> 
> -		ret = drm_dp_dpcd_read_data(aux,
> DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, buf, size);
> -		if (ret < 0) {
> -			drm_dbg_kms(aux->drm_dev, "%s: Failed to read
> backlight level: %d\n",
> -				    aux->name, ret);
> -			return ret;
> +		if (bl->luminance_set) {
> +			ret = drm_dp_dpcd_read_data(aux,
> DP_EDP_PANEL_TARGET_LUMINANCE_VALUE,
> +						    buf, sizeof(buf));
> +			if (ret < 0) {
> +				drm_dbg_kms(aux->drm_dev,
> +					    "%s: Failed to read backlight level:
> %d\n",
> +					    aux->name, ret);
> +				return ret;
>  		}
> 
> -		if (bl->lsb_reg_used)
> -			return (buf[0] << 8) | buf[1];
> -		else
> -			return buf[0];
> +		return (buf[0] | buf[1] << 8 | buf[2] << 16) / 1000;
Can a comment be added here on the unit?

Upon adding the above comment
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------
> +		} else {
> +			ret = drm_dp_dpcd_read_data(aux,
> DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> +						    buf, size);
> +			if (ret < 0) {
> +				drm_dbg_kms(aux->drm_dev,
> +					    "%s: Failed to read backlight level:
> %d\n",
> +					    aux->name, ret);
> +				return ret;
> +			}
> +
> +			if (bl->lsb_reg_used)
> +				return (buf[0] << 8) | buf[1];
> +			else
> +				return buf[0];
> +		}
>  	}
> 
>  	/*
> --
> 2.34.1

RE: [PATCH 09/13] drm/dp: Change argument type of drm_edp_backlight_enable

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 09/13] drm/dp: Change argument type of
> drm_edp_backlight_enable
> 
> Change the argument type to u32 for the default level being sent since it has to
> now account for luminance value which has to be set for
> DP_EDP_PANEL_LUMINANCE_TARGET_VALUE.
> 
> --v2
> -No need to typecast [Jani]
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------
>  drivers/gpu/drm/display/drm_dp_helper.c | 2 +-
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
>  include/drm/display/drm_dp_helper.h     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index dc0bda84d211..0421b2ed9bd4 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4020,7 +4020,7 @@ drm_edp_backlight_set_enable(struct drm_dp_aux
> *aux, const struct drm_edp_backli
>   * Returns: %0 on success, negative error code on failure.
>   */
>  int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> -			     const u16 level)
> +			     const u32 level)
>  {
>  	int ret;
>  	u8 dpcd_buf;
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 725331638a15..e3b8f6f510ef 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -1829,7 +1829,7 @@ nv50_sor_atomic_enable(struct drm_encoder
> *encoder, struct drm_atomic_state *sta
>  		backlight = nv_connector->backlight;
>  		if (backlight && backlight->uses_dpcd)
>  			drm_edp_backlight_enable(&nv_connector->aux,
> &backlight->edp_info,
> -						 (u16)backlight->dev-
> >props.brightness);
> +						 backlight->dev-
> >props.brightness);
>  #endif
> 
>  		break;
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 6bce0176efd3..b6c03d3ca6c3 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -855,7 +855,7 @@ drm_edp_backlight_init(struct drm_dp_aux *aux,
> struct drm_edp_backlight_info *bl  int drm_edp_backlight_set_level(struct
> drm_dp_aux *aux, const struct drm_edp_backlight_info *bl,
>  				u32 level);
>  int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> -			     u16 level);
> +			     u32 level);
>  int drm_edp_backlight_disable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl);
> 
>  #if IS_ENABLED(CONFIG_DRM_KMS_HELPER) &&
> (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
> --
> 2.34.1

RE: [PATCH 10/13] drm/dp: Enable backlight control using luminance

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 10/13] drm/dp: Enable backlight control using luminance
> 
> Add flag to enable brightness control via luminance value when enabling edp
> backlight.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 0421b2ed9bd4..4e2caba8311a 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4030,6 +4030,9 @@ int drm_edp_backlight_enable(struct drm_dp_aux
> *aux, const struct drm_edp_backli
>  	else
>  		dpcd_buf = DP_EDP_BACKLIGHT_CONTROL_MODE_PWM;
> 
> +	if (bl->luminance_set)
> +		dpcd_buf |= DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE;
Can the backlight control by luminance be used along with PWM or AUX ctl?

Thanks and Regards,
Arun R Murthy
-------------------
> +
>  	if (bl->pwmgen_bit_count) {
>  		ret = drm_dp_dpcd_write_byte(aux,
> DP_EDP_PWMGEN_BIT_COUNT, bl->pwmgen_bit_count);
>  		if (ret < 0)
> --
> 2.34.1

RE: [PATCH 10/13] drm/dp: Enable backlight control using luminance

[Kandpal, Suraj]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Friday, June 20, 2025 10:17 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: RE: [PATCH 10/13] drm/dp: Enable backlight control using luminance
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Sent: Monday, April 14, 2025 9:47 AM
> > To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > intel- xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> > <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> > Subject: [PATCH 10/13] drm/dp: Enable backlight control using
> > luminance
> >
> > Add flag to enable brightness control via luminance value when
> > enabling edp backlight.
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/display/drm_dp_helper.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > b/drivers/gpu/drm/display/drm_dp_helper.c
> > index 0421b2ed9bd4..4e2caba8311a 100644
> > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > @@ -4030,6 +4030,9 @@ int drm_edp_backlight_enable(struct
> drm_dp_aux
> > *aux, const struct drm_edp_backli
> >  	else
> >  		dpcd_buf = DP_EDP_BACKLIGHT_CONTROL_MODE_PWM;
> >
> > +	if (bl->luminance_set)
> > +		dpcd_buf |=
> DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE;
> Can the backlight control by luminance be used along with PWM or AUX ctl?
> 

It does work.

> Thanks and Regards,
> Arun R Murthy
> -------------------
> > +
> >  	if (bl->pwmgen_bit_count) {
> >  		ret = drm_dp_dpcd_write_byte(aux,
> > DP_EDP_PWMGEN_BIT_COUNT, bl->pwmgen_bit_count);
> >  		if (ret < 0)
> > --
> > 2.34.1

RE: [PATCH 10/13] drm/dp: Enable backlight control using luminance

[Murthy, Arun R]

> > > Add flag to enable brightness control via luminance value when
> > > enabling edp backlight.
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > >  drivers/gpu/drm/display/drm_dp_helper.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > > b/drivers/gpu/drm/display/drm_dp_helper.c
> > > index 0421b2ed9bd4..4e2caba8311a 100644
> > > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > > @@ -4030,6 +4030,9 @@ int drm_edp_backlight_enable(struct
> > drm_dp_aux
> > > *aux, const struct drm_edp_backli
> > >  	else
> > >  		dpcd_buf = DP_EDP_BACKLIGHT_CONTROL_MODE_PWM;
> > >
> > > +	if (bl->luminance_set)
> > > +		dpcd_buf |=
> > DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE;
> > Can the backlight control by luminance be used along with PWM or AUX ctl?
> >
> 
> It does work.
> 
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------

RE: [PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp
> backlight
> 
> Now that drm_edp_backlight init has been to be able to setup brightness
Can you reframe this?

> manipulation via luminance we can just use that.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  .../drm/i915/display/intel_dp_aux_backlight.c | 100 +++++++++---------
>  1 file changed, 49 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index be740fb72ebc..2eff9b545390 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -585,9 +585,36 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
>  	u8 current_mode;
>  	int ret;
> 
> -	if (panel->backlight.edp.vesa.luminance_control_support) {
> +	ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> >backlight.edp.vesa.info,
> +				     luminance_range, panel-
> >vbt.backlight.pwm_freq_hz,
> +				     intel_dp->edp_dpcd, &current_level,
> &current_mode, true);
> +	if (ret < 0)
> +		return ret;
> +
> +	drm_dbg_kms(display->drm,
> +		    "[CONNECTOR:%d:%s] AUX VESA backlight enable is
> controlled through %s\n",
> +		    connector->base.base.id, connector->base.name,
> +		    dpcd_vs_pwm_str(panel-
> >backlight.edp.vesa.info.aux_enable));
> +	drm_dbg_kms(display->drm,
> +		    "[CONNECTOR:%d:%s] AUX VESA backlight level is controlled
> through %s\n",
> +		    connector->base.base.id, connector->base.name,
> +		    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_set));
> +
Is two separate debug prints required to convey that backlight is enabled and the level is set with he same parameters?

> +	if (!panel->backlight.edp.vesa.info.luminance_set ||
> +	    !panel->backlight.edp.vesa.info.aux_set ||
> +	    !panel->backlight.edp.vesa.info.aux_enable) {
> +		ret = panel->backlight.pwm_funcs->setup(connector, pipe);
If pwm is used then none of the above conditions will be true so instead can check for the assignment of the function pointer if (pwm_funcs->setup)
In backlight struct enabled and pwm_enabled doesn't make much sense, maybe one element to convey the mode make it easier. This can be taken out of this series.

> +		if (ret < 0) {
> +			drm_err(display->drm,
> +				"[CONNECTOR:%d:%s] Failed to setup PWM
> backlight controls for eDP backlight: %d\n",
> +				connector->base.base.id, connector-
> >base.name, ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (panel->backlight.edp.vesa.info.luminance_set) {
>  		if (luminance_range->max_luminance) {
> -			panel->backlight.max = luminance_range-
> >max_luminance;
> +			panel->backlight.max = panel-
> >backlight.edp.vesa.info.max;
>  			panel->backlight.min = luminance_range-
> >min_luminance;
>  		} else {
>  			panel->backlight.max = 512;
> @@ -596,57 +623,28 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
>  		panel->backlight.level =
> intel_dp_aux_vesa_get_backlight(connector, 0);
>  		panel->backlight.enabled = panel->backlight.level != 0;
>  		drm_dbg_kms(display->drm,
> -			    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level
> is controlled through DPCD\n",
> -			    connector->base.base.id, connector->base.name);
> -	} else {
> -		ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> >backlight.edp.vesa.info,
> -					     luminance_range, panel-
> >vbt.backlight.pwm_freq_hz,
> -					     intel_dp->edp_dpcd,
> &current_level, &current_mode,
> -					     false);
> -		if (ret < 0)
> -			return ret;
> -
> -		drm_dbg_kms(display->drm,
> -			    "[CONNECTOR:%d:%s] AUX VESA backlight enable is
> controlled through %s\n",
> -			    connector->base.base.id, connector->base.name,
> -			    dpcd_vs_pwm_str(panel-
> >backlight.edp.vesa.info.aux_enable));
> -		drm_dbg_kms(display->drm,
> -			    "[CONNECTOR:%d:%s] AUX VESA backlight level is
> controlled through %s\n",
> -			    connector->base.base.id, connector->base.name,
> -			    dpcd_vs_pwm_str(panel-
> >backlight.edp.vesa.info.aux_set));
> -
> -		if (!panel->backlight.edp.vesa.info.aux_set ||
> -		    !panel->backlight.edp.vesa.info.aux_enable) {
> -			ret = panel->backlight.pwm_funcs->setup(connector,
> pipe);
> -			if (ret < 0) {
> -				drm_err(display->drm,
> -					"[CONNECTOR:%d:%s] Failed to setup
> PWM backlight controls for eDP backlight: %d\n",
> -					connector->base.base.id, connector-
> >base.name, ret);
> -				return ret;
> -			}
> +		    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level is
> controlled through DPCD\n",
> +		    connector->base.base.id, connector->base.name);
> +	} else if (panel->backlight.edp.vesa.info.aux_set) {
> +		panel->backlight.max = panel->backlight.edp.vesa.info.max;
> +		panel->backlight.min = 0;
> +		if (current_mode ==
> DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
> +			panel->backlight.level = current_level;
> +			panel->backlight.enabled = panel->backlight.level != 0;
> +		} else {
> +			panel->backlight.level = panel->backlight.max;
> +			panel->backlight.enabled = false;
>  		}
> -
> -		if (panel->backlight.edp.vesa.info.aux_set) {
> -			panel->backlight.max = panel-
> >backlight.edp.vesa.info.max;
> -			panel->backlight.min = 0;
> -			if (current_mode ==
> DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
> -				panel->backlight.level = current_level;
> -				panel->backlight.enabled = panel-
> >backlight.level != 0;
> -			} else {
> -				panel->backlight.level = panel->backlight.max;
> -				panel->backlight.enabled = false;
> -			}
> +	} else {
> +		panel->backlight.max = panel->backlight.pwm_level_max;
> +		panel->backlight.min = panel->backlight.pwm_level_min;
> +		if (current_mode ==
> DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
> +			panel->backlight.level =
> +				panel->backlight.pwm_funcs->get(connector,
> pipe);
> +			panel->backlight.enabled = panel-
> >backlight.pwm_enabled;
>  		} else {
> -			panel->backlight.max = panel-
> >backlight.pwm_level_max;
> -			panel->backlight.min = panel-
> >backlight.pwm_level_min;
> -			if (current_mode ==
> DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
> -				panel->backlight.level =
> -					panel->backlight.pwm_funcs-
> >get(connector, pipe);
> -				panel->backlight.enabled = panel-
> >backlight.pwm_enabled;
> -			} else {
> -				panel->backlight.level = panel->backlight.max;
> -				panel->backlight.enabled = false;
> -			}
> +			panel->backlight.level = panel->backlight.max;
> +			panel->backlight.enabled = false;
Change is simple, adding a new condition for luminance and moving edp_backlight_init out of the if condition so that it will be initialized for all the modes and then initializing backlight level and max based on the mode. But the change looks complex.

Thanks and Regards,
Arun R Murthy
-------------------
>  		}
>  	}
> 
> --
> 2.34.1

RE: [PATCH 12/13] drm/i915/backlight: Use drm helper to set edp backlight

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 12/13] drm/i915/backlight: Use drm helper to set edp
> backlight
> 
> Now that the drm helper sets the backlight using luminance too we can use
> that. Remove the obselete function.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------

>  .../drm/i915/display/intel_dp_aux_backlight.c | 34 ++-----------------
>  1 file changed, 3 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 2eff9b545390..95b29d9af335 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -476,31 +476,6 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
> intel_connector *connector, en
>  	return connector->panel.backlight.level;  }
> 
> -static int
> -intel_dp_aux_vesa_set_luminance(struct intel_connector *connector, u32
> level) -{
> -	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> -	u8 buf[3];
> -	int ret;
> -
> -	level = level * 1000;
> -	level &= 0xffffff;
> -	buf[0] = (level & 0x0000ff);
> -	buf[1] = (level & 0x00ff00) >> 8;
> -	buf[2] = (level & 0xff0000) >> 16;
> -
> -	ret = drm_dp_dpcd_write(&intel_dp->aux,
> DP_EDP_PANEL_TARGET_LUMINANCE_VALUE,
> -				buf, sizeof(buf));
> -	if (ret != sizeof(buf)) {
> -		drm_err(intel_dp->aux.drm_dev,
> -			"%s: Failed to set VESA Aux Luminance: %d\n",
> -			intel_dp->aux.name, ret);
> -		return -EINVAL;
> -	} else {
> -		return 0;
> -	}
> -}
> -
>  static void
>  intel_dp_aux_vesa_set_backlight(const struct drm_connector_state
> *conn_state, u32 level)  { @@ -508,11 +483,6 @@
> intel_dp_aux_vesa_set_backlight(const struct drm_connector_state
> *conn_state, u3
>  	struct intel_panel *panel = &connector->panel;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> 
> -	if (panel->backlight.edp.vesa.luminance_control_support) {
> -		if (!intel_dp_aux_vesa_set_luminance(connector, level))
> -			return;
> -	}
> -
>  	if (!panel->backlight.edp.vesa.info.aux_set) {
>  		const u32 pwm_level =
> intel_backlight_level_to_pwm(connector, level);
> 
> @@ -538,7 +508,9 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
>  		if (ret == 1)
>  			return;
> 
> -		if (!intel_dp_aux_vesa_set_luminance(connector, level))
> +		if (!drm_edp_backlight_set_level(&intel_dp->aux,
> +						 &panel-
> >backlight.edp.vesa.info,
> +						 level))
>  			return;
>  	}
> 
> --
> 2.34.1

RE: [PATCH 13/13] drm/i915/backlight: Use drm_edp_backlight_enable

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 13/13] drm/i915/backlight: Use drm_edp_backlight_enable
> 
> Use drm dp helper to enable backlight now that it has been modified to set
> PANEL_LUMINANCE_CONTROL_ENABLE bit based on if capability supports it
> and the driver wants it. Remove the dead code.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------

>  .../gpu/drm/i915/display/intel_dp_aux_backlight.c  | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 95b29d9af335..b8b0ace9e6fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -499,20 +499,6 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
>  	struct intel_connector *connector = to_intel_connector(conn_state-
> >connector);
>  	struct intel_panel *panel = &connector->panel;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> -	int ret;
> -
> -	if (panel->backlight.edp.vesa.luminance_control_support) {
> -		ret = drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
> -
> DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE);
> -
> -		if (ret == 1)
> -			return;
> -
> -		if (!drm_edp_backlight_set_level(&intel_dp->aux,
> -						 &panel-
> >backlight.edp.vesa.info,
> -						 level))
> -			return;
> -	}
> 
>  	if (!panel->backlight.edp.vesa.info.aux_enable) {
>  		u32 pwm_level;
> --
> 2.34.1

RE: [PATCH 07/13] drm/dp: Change argument type for drm_edp_backlight_set_level

[Murthy, Arun R]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: [PATCH 07/13] drm/dp: Change argument type for
> drm_edp_backlight_set_level
> 
> Use u32 for level variable as one may need to pass value for
> DP_EDP_PANEL_TARGET_LUMINANCE_VALUE.
> 
> --v2
> -Typecase is not needed [Jani]
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------

>  drivers/gpu/drm/display/drm_dp_helper.c | 2 +-
>  include/drm/display/drm_dp_helper.h     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index bb1242a1bf6b..c4c52fb37191 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -3933,7 +3933,7 @@
> EXPORT_SYMBOL(drm_dp_pcon_convert_rgb_to_ycbcr);
>   * Returns: %0 on success, negative error code on failure
>   */
>  int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> -				u16 level)
> +				u32 level)
>  {
>  	int ret;
>  	u8 buf[2] = { 0 };
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 62be80417ded..6bce0176efd3 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -853,7 +853,7 @@ drm_edp_backlight_init(struct drm_dp_aux *aux,
> struct drm_edp_backlight_info *bl
>  		       u16 driver_pwm_freq_hz, const u8
> edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE],
>  		       u32 *current_level, u8 *current_mode, bool
> need_luminance);  int drm_edp_backlight_set_level(struct drm_dp_aux *aux,
> const struct drm_edp_backlight_info *bl,
> -				u16 level);
> +				u32 level);
>  int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
>  			     u16 level);
>  int drm_edp_backlight_disable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl);
> --
> 2.34.1

RE: [PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight

[Kandpal, Suraj]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Friday, June 20, 2025 11:00 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: RE: [PATCH 11/13] drm/i915/backlight: Use drm helper to initialize
> edp backlight
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Sent: Monday, April 14, 2025 9:47 AM
> > To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > intel- xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Murthy, Arun R
> > <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> > Subject: [PATCH 11/13] drm/i915/backlight: Use drm helper to
> > initialize edp backlight
> >
> > Now that drm_edp_backlight init has been to be able to setup
> > brightness
> Can you reframe this?

Sure will reframe this.

> 
> > manipulation via luminance we can just use that.
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  .../drm/i915/display/intel_dp_aux_backlight.c | 100
> > +++++++++---------
> >  1 file changed, 49 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > index be740fb72ebc..2eff9b545390 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > @@ -585,9 +585,36 @@ static int
> > intel_dp_aux_vesa_setup_backlight(struct
> > intel_connector *connector,
> >  	u8 current_mode;
> >  	int ret;
> >
> > -	if (panel->backlight.edp.vesa.luminance_control_support) {
> > +	ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> > >backlight.edp.vesa.info,
> > +				     luminance_range, panel-
> > >vbt.backlight.pwm_freq_hz,
> > +				     intel_dp->edp_dpcd, &current_level,
> > &current_mode, true);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	drm_dbg_kms(display->drm,
> > +		    "[CONNECTOR:%d:%s] AUX VESA backlight enable is
> > controlled through %s\n",
> > +		    connector->base.base.id, connector->base.name,
> > +		    dpcd_vs_pwm_str(panel-
> > >backlight.edp.vesa.info.aux_enable));
> > +	drm_dbg_kms(display->drm,
> > +		    "[CONNECTOR:%d:%s] AUX VESA backlight level is
> controlled
> > through %s\n",
> > +		    connector->base.base.id, connector->base.name,
> > +		    dpcd_vs_pwm_str(panel-
> >backlight.edp.vesa.info.aux_set));
> > +
> Is two separate debug prints required to convey that backlight is enabled and
> the level is set with he same parameters?

Something which existed in legacy code can make a separate patch in case this is not needed
But a patch unrelated to this series after this is done.
Would that be okay.

> 
> > +	if (!panel->backlight.edp.vesa.info.luminance_set ||
> > +	    !panel->backlight.edp.vesa.info.aux_set ||
> > +	    !panel->backlight.edp.vesa.info.aux_enable) {
> > +		ret = panel->backlight.pwm_funcs->setup(connector, pipe);
> If pwm is used then none of the above conditions will be true so instead can
> check for the assignment of the function pointer if (pwm_funcs->setup) In
> backlight struct enabled and pwm_enabled doesn't make much sense, maybe
> one element to convey the mode make it easier. This can be taken out of this
> series.

Sure will look into this and prepare a separate patch from this series .

> 
> > +		if (ret < 0) {
> > +			drm_err(display->drm,
> > +				"[CONNECTOR:%d:%s] Failed to setup PWM
> > backlight controls for eDP backlight: %d\n",
> > +				connector->base.base.id, connector-
> > >base.name, ret);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	if (panel->backlight.edp.vesa.info.luminance_set) {
> >  		if (luminance_range->max_luminance) {
> > -			panel->backlight.max = luminance_range-
> > >max_luminance;
> > +			panel->backlight.max = panel-
> > >backlight.edp.vesa.info.max;
> >  			panel->backlight.min = luminance_range-
> > >min_luminance;
> >  		} else {
> >  			panel->backlight.max = 512;
> > @@ -596,57 +623,28 @@ static int
> > intel_dp_aux_vesa_setup_backlight(struct
> > intel_connector *connector,
> >  		panel->backlight.level =
> > intel_dp_aux_vesa_get_backlight(connector, 0);
> >  		panel->backlight.enabled = panel->backlight.level != 0;
> >  		drm_dbg_kms(display->drm,
> > -			    "[CONNECTOR:%d:%s] AUX VESA Nits backlight
> level
> > is controlled through DPCD\n",
> > -			    connector->base.base.id, connector->base.name);
> > -	} else {
> > -		ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> > >backlight.edp.vesa.info,
> > -					     luminance_range, panel-
> > >vbt.backlight.pwm_freq_hz,
> > -					     intel_dp->edp_dpcd,
> > &current_level, &current_mode,
> > -					     false);
> > -		if (ret < 0)
> > -			return ret;
> > -
> > -		drm_dbg_kms(display->drm,
> > -			    "[CONNECTOR:%d:%s] AUX VESA backlight enable is
> > controlled through %s\n",
> > -			    connector->base.base.id, connector->base.name,
> > -			    dpcd_vs_pwm_str(panel-
> > >backlight.edp.vesa.info.aux_enable));
> > -		drm_dbg_kms(display->drm,
> > -			    "[CONNECTOR:%d:%s] AUX VESA backlight level is
> > controlled through %s\n",
> > -			    connector->base.base.id, connector->base.name,
> > -			    dpcd_vs_pwm_str(panel-
> > >backlight.edp.vesa.info.aux_set));
> > -
> > -		if (!panel->backlight.edp.vesa.info.aux_set ||
> > -		    !panel->backlight.edp.vesa.info.aux_enable) {
> > -			ret = panel->backlight.pwm_funcs->setup(connector,
> > pipe);
> > -			if (ret < 0) {
> > -				drm_err(display->drm,
> > -					"[CONNECTOR:%d:%s] Failed to setup
> > PWM backlight controls for eDP backlight: %d\n",
> > -					connector->base.base.id, connector-
> > >base.name, ret);
> > -				return ret;
> > -			}
> > +		    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level is
> > controlled through DPCD\n",
> > +		    connector->base.base.id, connector->base.name);
> > +	} else if (panel->backlight.edp.vesa.info.aux_set) {
> > +		panel->backlight.max = panel->backlight.edp.vesa.info.max;
> > +		panel->backlight.min = 0;
> > +		if (current_mode ==
> > DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
> > +			panel->backlight.level = current_level;
> > +			panel->backlight.enabled = panel->backlight.level != 0;
> > +		} else {
> > +			panel->backlight.level = panel->backlight.max;
> > +			panel->backlight.enabled = false;
> >  		}
> > -
> > -		if (panel->backlight.edp.vesa.info.aux_set) {
> > -			panel->backlight.max = panel-
> > >backlight.edp.vesa.info.max;
> > -			panel->backlight.min = 0;
> > -			if (current_mode ==
> > DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
> > -				panel->backlight.level = current_level;
> > -				panel->backlight.enabled = panel-
> > >backlight.level != 0;
> > -			} else {
> > -				panel->backlight.level = panel->backlight.max;
> > -				panel->backlight.enabled = false;
> > -			}
> > +	} else {
> > +		panel->backlight.max = panel->backlight.pwm_level_max;
> > +		panel->backlight.min = panel->backlight.pwm_level_min;
> > +		if (current_mode ==
> > DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
> > +			panel->backlight.level =
> > +				panel->backlight.pwm_funcs->get(connector,
> > pipe);
> > +			panel->backlight.enabled = panel-
> > >backlight.pwm_enabled;
> >  		} else {
> > -			panel->backlight.max = panel-
> > >backlight.pwm_level_max;
> > -			panel->backlight.min = panel-
> > >backlight.pwm_level_min;
> > -			if (current_mode ==
> > DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
> > -				panel->backlight.level =
> > -					panel->backlight.pwm_funcs-
> > >get(connector, pipe);
> > -				panel->backlight.enabled = panel-
> > >backlight.pwm_enabled;
> > -			} else {
> > -				panel->backlight.level = panel->backlight.max;
> > -				panel->backlight.enabled = false;
> > -			}
> > +			panel->backlight.level = panel->backlight.max;
> > +			panel->backlight.enabled = false;
> Change is simple, adding a new condition for luminance and moving
> edp_backlight_init out of the if condition so that it will be initialized for all the
> modes and then initializing backlight level and max based on the mode. But
> the change looks complex.

Yes that's true not sure why git ends up making it look like this tried to make it look simpler but always end up with same result

Regards,
Suraj Kandpal

> 
> Thanks and Regards,
> Arun R Murthy
> -------------------
> >  		}
> >  	}
> >
> > --
> > 2.34.1