Skip to contentSkip to conversation

drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

PATCH Applied Rajeev Nandan 5 participants 19 messages Jun 26, 2021 – Jul 12, 2021

Subsystems (6)
Device tree bindings DRM drivers DRM misc Arm Qualcomm mailing list +2 more subsystems
lore
Share & tools
lore

Rajeev Nandan

author
#0 lore raw

v8 cover drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

This series adds the support for the eDP panel that needs the backlight controlling over the DP AUX channel using DPCD registers of the panel as per the VESA's standard.

This series also adds support for the Samsung eDP AMOLED panel that needs DP AUX to control the backlight, and introduces new delays in the @panel_desc.delay to support this panel.

This patch series depends on the following two series:
- Doug's series [1], exposed the DP AUX channel to the panel-simple.
- Lyude's series [2], introduced new drm helper functions for DPCD backlight.

This series is the logical successor to the series [3].

Changes in v1:
- Created dpcd backlight helper with very basic functionality, added backlight registration in the ti-sn65dsi86 bridge driver.

Changes in v2:
- Created a new DisplayPort aux backlight driver and moved the code from drm_dp_aux_backlight.c (v1) to the new driver.

Changes in v3:
- Fixed module compilation (kernel test bot).

Changes in v4:
- Added basic DPCD backlight support in panel-simple.
- Added support for a new Samsung panel ATNA33XC20 that needs DPCD backlight controlling and has a requirement of delays between enable GPIO and regulator.

Changes in v5:
Addressed review suggestions from Douglas:
- Created a new API drm_panel_dp_aux_backlight() in drm_panel.c
- Moved DP AUX backlight functions from panel-simple.c to drm_panel.c
- panel-simple probe() calls drm_panel_dp_aux_backlight() to create backlight when the backlight phandle is not specified in panel DT and DP AUX channel is present.
- Added check for drm_edp_backlight_supported() before registering.
- Removed the @uses_dpcd_backlight flag from panel_desc as this should be auto-detected.
- Updated comments/descriptions.

Changes in v6:
- Rebased
- Updated wanrning messages, fixed word wrapping in comments.
- Fixed ordering of memory allocation

Changes in v7:
- Updated the disable_to_power_off and power_to_enable panel delays as discovered at <https://crrev.com/c/2966167> (Douglas)

Changes in v8:
- Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg)
- Added a new patch #4 to fix the warnings for eDP panel description (Sam Ravnborg)

[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://lore.kernel.org/dri-devel/[email protected]/
[3] https://lore.kernel.org/dri-devel/[email protected]/

Rajeev Nandan (6):
drm/panel: add basic DP AUX backlight support drm/panel-simple: Support DP AUX backlight drm/panel-simple: Support for delays between GPIO & regulator drm/panel-simple: Update validation warnings for eDP panel description
dt-bindings: display: simple: Add Samsung ATNA33XC20 drm/panel-simple: Add Samsung ATNA33XC20

 .../bindings/display/panel/panel-simple.yaml       |   2 +
 drivers/gpu/drm/drm_panel.c                        | 108 +++++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c               |  73 +++++++++++++-
 include/drm/drm_panel.h                            |  15 ++-
 4 files changed, 190 insertions(+), 8 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH v8 1/6 drm/panel: add basic DP AUX backlight support

Some panels support backlight control over DP AUX channel using VESA's standard backlight control interface.
Using new DRM eDP backlight helpers, add support to create and register a backlight for those panels in drm_panel to simplify the panel drivers.

The panel driver with access to "struct drm_dp_aux" can create and register a backlight device using following code snippet in its probe() function:

	err = drm_panel_dp_aux_backlight(panel, aux);
	if (err)
		return err;

Then drm_panel will handle backlight_(enable|disable) calls similar to the case when drm_panel_of_backlight() is used.

Currently, we are not supporting one feature where the source device can combine the backlight brightness levels set through DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not required for the basic backlight controls, it can be added later.

Signed-off-by Rajeev Nandan <[email protected]>
Reviewed-by Douglas Anderson <[email protected]>
Reviewed-by Lyude Paul <[email protected]>

---

Changes in v5:
- New

Changes in v6:
- Fixed ordering of memory allocation (Douglas)
- Updated word wrapping in a comment (Douglas)

Changes in v8:
- Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg)

 drivers/gpu/drm/drm_panel.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_panel.h     |  15 ++++--
 2 files changed, 119 insertions(+), 4 deletions(-)
drivers/gpu/drm/drm_panel.c +108 -0
index f634371..4fa1e3b 100644
@@ -26,12 +26,20 @@
 #include <linux/module.h>
 
 #include <drm/drm_crtc.h>
+#include <drm/drm_dp_helper.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_print.h>
 
 static DEFINE_MUTEX(panel_lock);
 static LIST_HEAD(panel_list);
 
+struct dp_aux_backlight {
+	struct backlight_device *base;
+	struct drm_dp_aux *aux;
+	struct drm_edp_backlight_info info;
+	bool enabled;
+};
+
 /**
  * DOC: drm panel
  *

@@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel)
 	return 0;
 }
 EXPORT_SYMBOL(drm_panel_of_backlight);
+
+static int dp_aux_backlight_update_status(struct backlight_device *bd)
+{
+	struct dp_aux_backlight *bl = bl_get_data(bd);
+	u16 brightness = backlight_get_brightness(bd);
+	int ret = 0;
+
+	if (!backlight_is_blank(bd)) {
+		if (!bl->enabled) {
+			drm_edp_backlight_enable(bl->aux, &bl->info, brightness);
+			bl->enabled = true;
+			return 0;
+		}
+		ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness);
+	} else {
+		if (bl->enabled) {
+			drm_edp_backlight_disable(bl->aux, &bl->info);
+			bl->enabled = false;
+		}
+	}
+
+	return ret;
+}
+
+static const struct backlight_ops dp_aux_bl_ops = {
+	.update_status = dp_aux_backlight_update_status,
+};
+
+/**
+ * drm_panel_dp_aux_backlight - create and use DP AUX backlight
+ * @panel: DRM panel
+ * @aux: The DP AUX channel to use
+ *
+ * Use this function to create and handle backlight if your panel
+ * supports backlight control over DP AUX channel using DPCD
+ * registers as per VESA's standard backlight control interface.
+ *
+ * When the panel is enabled backlight will be enabled after a
+ * successful call to &drm_panel_funcs.enable()
+ *
+ * When the panel is disabled backlight will be disabled before the
+ * call to &drm_panel_funcs.disable().
+ *
+ * A typical implementation for a panel driver supporting backlight
+ * control over DP AUX will call this function at probe time.
+ * Backlight will then be handled transparently without requiring
+ * any intervention from the driver.
+ *
+ * drm_panel_dp_aux_backlight() must be called after the call to drm_panel_init().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+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;
+	u8 current_mode;
+	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+	int ret;
+
+	if (!panel || !panel->dev || !aux)
+		return -EINVAL;
+
+	ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+			       EDP_DISPLAY_CTL_CAP_SIZE);
+	if (ret < 0)
+		return ret;
+
+	if (!drm_edp_backlight_supported(edp_dpcd)) {
+		DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n");
+		return 0;
+	}
+
+	bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL);
+	if (!bl)
+		return -ENOMEM;
+
+	bl->aux = aux;
+
+	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+				     &current_level, &current_mode);
+	if (ret < 0)
+		return ret;
+
+	props.type = BACKLIGHT_RAW;
+	props.brightness = current_level;
+	props.max_brightness = bl->info.max;
+
+	bl->base = devm_backlight_device_register(panel->dev, "dp_aux_backlight",
+						  panel->dev, bl,
+						  &dp_aux_bl_ops, &props);
+	if (IS_ERR(bl->base))
+		return PTR_ERR(bl->base);
+
+	panel->backlight = bl->base;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
 #endif
 
 MODULE_AUTHOR("Thierry Reding <[email protected]>");
include/drm/drm_panel.h +11 -4
index 33605c3..3ebfaa6 100644
@@ -32,6 +32,7 @@ struct backlight_device;
 struct device_node;
 struct drm_connector;
 struct drm_device;
+struct drm_dp_aux;
 struct drm_panel;
 struct display_timing;
 

@@ -64,8 +65,8 @@ enum drm_panel_orientation;
  * the panel. This is the job of the .unprepare() function.
  *
  * Backlight can be handled automatically if configured using
- * drm_panel_of_backlight(). Then the driver does not need to implement the
- * functionality to enable/disable backlight.
+ * drm_panel_of_backlight() or drm_panel_dp_aux_backlight(). Then the driver
+ * does not need to implement the functionality to enable/disable backlight.
  */
 struct drm_panel_funcs {
 	/**

@@ -144,8 +145,8 @@ struct drm_panel {
 	 * Backlight device, used to turn on backlight after the call
 	 * to enable(), and to turn off backlight before the call to
 	 * disable().
-	 * backlight is set by drm_panel_of_backlight() and drivers
-	 * shall not assign it.
+	 * backlight is set by drm_panel_of_backlight() or
+	 * drm_panel_dp_aux_backlight() and drivers shall not assign it.
 	 */
 	struct backlight_device *backlight;
 

@@ -208,11 +209,17 @@ static inline int of_drm_get_panel_orientation(const struct device_node *np,
 #if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
 	(IS_MODULE(CONFIG_DRM) && IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE)))
 int drm_panel_of_backlight(struct drm_panel *panel);
+int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux);
 #else
 static inline int drm_panel_of_backlight(struct drm_panel *panel)
 {
 	return 0;
 }
+static inline int drm_panel_dp_aux_backlight(struct drm_panel *panel,
+					     struct drm_dp_aux *aux)
+{
+	return 0;
+}
 #endif
 
 #endif
Signature · 1 line
2.7.4
maintainer
#2 lore raw

Re: [v8 1/6] drm/panel: add basic DP AUX backlight support

Am 26.06.21 um 18:51 schrieb Rajeev Nandan:

Quoted 126 lines

Some panels support backlight control over DP AUX channel using VESA's standard backlight control interface.
Using new DRM eDP backlight helpers, add support to create and register a backlight for those panels in drm_panel to simplify the panel drivers.

The panel driver with access to "struct drm_dp_aux" can create and register a backlight device using following code snippet in its probe() function:

	err = drm_panel_dp_aux_backlight(panel, aux);
	if (err)
		return err;

Then drm_panel will handle backlight_(enable|disable) calls similar to the case when drm_panel_of_backlight() is used.

Currently, we are not supporting one feature where the source device can combine the backlight brightness levels set through DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not required for the basic backlight controls, it can be added later.

Signed-off-by Rajeev Nandan <[email protected]>
Reviewed-by Douglas Anderson <[email protected]>
Reviewed-by Lyude Paul <[email protected]>

---

Changes in v5:
- New

Changes in v6:
- Fixed ordering of memory allocation (Douglas)
- Updated word wrapping in a comment (Douglas)

Changes in v8:
- Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg)

  drivers/gpu/drm/drm_panel.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
  include/drm/drm_panel.h     |  15 ++++--
  2 files changed, 119 insertions(+), 4 deletions(-)
drivers/gpu/drm/drm_panel.c +64 -0
index f634371..4fa1e3b 100644
@@ -26,12 +26,20 @@
  #include <linux/module.h>
 
  #include <drm/drm_crtc.h>
+#include <drm/drm_dp_helper.h>
  #include <drm/drm_panel.h>
  #include <drm/drm_print.h>
 
  static DEFINE_MUTEX(panel_lock);
  static LIST_HEAD(panel_list);
 
+struct dp_aux_backlight {
+	struct backlight_device *base;
+	struct drm_dp_aux *aux;
+	struct drm_edp_backlight_info info;
+	bool enabled;
+};
+
  /**
   * DOC: drm panel
   *

@@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel)
  	return 0;
  }
  EXPORT_SYMBOL(drm_panel_of_backlight);
+
+static int dp_aux_backlight_update_status(struct backlight_device *bd)
+{
+	struct dp_aux_backlight *bl = bl_get_data(bd);
+	u16 brightness = backlight_get_brightness(bd);
+	int ret = 0;
+
+	if (!backlight_is_blank(bd)) {
+		if (!bl->enabled) {
+			drm_edp_backlight_enable(bl->aux, &bl->info, brightness);
+			bl->enabled = true;
+			return 0;
+		}
+		ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness);
+	} else {
+		if (bl->enabled) {
+			drm_edp_backlight_disable(bl->aux, &bl->info);
+			bl->enabled = false;
+		}
+	}
+
+	return ret;
+}
+
+static const struct backlight_ops dp_aux_bl_ops = {
+	.update_status = dp_aux_backlight_update_status,
+};
+
+/**
+ * drm_panel_dp_aux_backlight - create and use DP AUX backlight
+ * @panel: DRM panel
+ * @aux: The DP AUX channel to use
+ *
+ * Use this function to create and handle backlight if your panel
+ * supports backlight control over DP AUX channel using DPCD
+ * registers as per VESA's standard backlight control interface.
+ *
+ * When the panel is enabled backlight will be enabled after a
+ * successful call to &drm_panel_funcs.enable()
+ *
+ * When the panel is disabled backlight will be disabled before the
+ * call to &drm_panel_funcs.disable().
+ *
+ * A typical implementation for a panel driver supporting backlight
+ * control over DP AUX will call this function at probe time.
+ * Backlight will then be handled transparently without requiring
+ * any intervention from the driver.
+ *
+ * drm_panel_dp_aux_backlight() must be called after the call to drm_panel_init().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+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 };

drivers/gpu/drm/drm_panel.c int drm_panel_of_backlight(struct drm_panel *panel) · line 418

Quoted 10 lines
drivers/gpu/drm/drm_panel.c +10 -0
+	u16 current_level;
+	u8 current_mode;
+	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+	int ret;
+
+	if (!panel || !panel->dev || !aux)
+		return -EINVAL;
+
+	ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+			       EDP_DISPLAY_CTL_CAP_SIZE);

This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. drm_panel.c is in the latter, while drm_dp_dpcd_read() in drm_dp_helper.c is in the former. Please fix.

Best regards
Thomas

Quoted 90 lines
drivers/gpu/drm/drm_panel.c +34 -0
+	if (ret < 0)
+		return ret;
+
+	if (!drm_edp_backlight_supported(edp_dpcd)) {
+		DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n");
+		return 0;
+	}
+
+	bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL);
+	if (!bl)
+		return -ENOMEM;
+
+	bl->aux = aux;
+
+	ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+				     &current_level, &current_mode);
+	if (ret < 0)
+		return ret;
+
+	props.type = BACKLIGHT_RAW;
+	props.brightness = current_level;
+	props.max_brightness = bl->info.max;
+
+	bl->base = devm_backlight_device_register(panel->dev, "dp_aux_backlight",
+						  panel->dev, bl,
+						  &dp_aux_bl_ops, &props);
+	if (IS_ERR(bl->base))
+		return PTR_ERR(bl->base);
+
+	panel->backlight = bl->base;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
  #endif
 
  MODULE_AUTHOR("Thierry Reding <[email protected]>");
include/drm/drm_panel.h +11 -4
index 33605c3..3ebfaa6 100644
@@ -32,6 +32,7 @@ struct backlight_device;
  struct device_node;
  struct drm_connector;
  struct drm_device;
+struct drm_dp_aux;
  struct drm_panel;
  struct display_timing;
 

@@ -64,8 +65,8 @@ enum drm_panel_orientation;
   * the panel. This is the job of the .unprepare() function.
   *
   * Backlight can be handled automatically if configured using
- * drm_panel_of_backlight(). Then the driver does not need to implement the
- * functionality to enable/disable backlight.
+ * drm_panel_of_backlight() or drm_panel_dp_aux_backlight(). Then the driver
+ * does not need to implement the functionality to enable/disable backlight.
   */
  struct drm_panel_funcs {
  	/**

@@ -144,8 +145,8 @@ struct drm_panel {
  	 * Backlight device, used to turn on backlight after the call
  	 * to enable(), and to turn off backlight before the call to
  	 * disable().
-	 * backlight is set by drm_panel_of_backlight() and drivers
-	 * shall not assign it.
+	 * backlight is set by drm_panel_of_backlight() or
+	 * drm_panel_dp_aux_backlight() and drivers shall not assign it.
  	 */
  	struct backlight_device *backlight;
 

@@ -208,11 +209,17 @@ static inline int of_drm_get_panel_orientation(const struct device_node *np,
  #if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
  	(IS_MODULE(CONFIG_DRM) && IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE)))
  int drm_panel_of_backlight(struct drm_panel *panel);
+int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux);
  #else
  static inline int drm_panel_of_backlight(struct drm_panel *panel)
  {
  	return 0;
  }
+static inline int drm_panel_dp_aux_backlight(struct drm_panel *panel,
+					     struct drm_dp_aux *aux)
+{
+	return 0;
+}
  #endif
 
  #endif
Signature · 6 lines
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
maintainer
#3 lore raw

Re: [v8 1/6] drm/panel: add basic DP AUX backlight support

Hi

Am 12.07.21 um 11:41 schrieb Thomas Zimmermann:

Quoted 135 lines

Am 26.06.21 um 18:51 schrieb Rajeev Nandan:

Quoted 132 lines

Some panels support backlight control over DP AUX channel using VESA's standard backlight control interface.
Using new DRM eDP backlight helpers, add support to create and register a backlight for those panels in drm_panel to simplify the panel drivers.

The panel driver with access to "struct drm_dp_aux" can create and register a backlight device using following code snippet in its probe() function:

    err = drm_panel_dp_aux_backlight(panel, aux);
    if (err)
        return err;

Then drm_panel will handle backlight_(enable|disable) calls similar to the case when drm_panel_of_backlight() is used.

Currently, we are not supporting one feature where the source device can combine the backlight brightness levels set through DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not required for the basic backlight controls, it can be added later.

Signed-off-by Rajeev Nandan <[email protected]>
Reviewed-by Douglas Anderson <[email protected]>
Reviewed-by Lyude Paul <[email protected]>

---

Changes in v5:
- New

Changes in v6:
- Fixed ordering of memory allocation (Douglas)
- Updated word wrapping in a comment (Douglas)

Changes in v8:
- Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg)

  drivers/gpu/drm/drm_panel.c | 108
++++++++++++++++++++++++++++++++++++++++++++
  include/drm/drm_panel.h     |  15 ++++--

  2 files changed, 119 insertions(+), 4 deletions(-)
drivers/gpu/drm/drm_panel.c +57 -0
index f634371..4fa1e3b 100644
@@ -26,12 +26,20 @@
   #include <linux/module.h>
   #include <drm/drm_crtc.h>
+#include <drm/drm_dp_helper.h>
   #include <drm/drm_panel.h>
   #include <drm/drm_print.h>
   static DEFINE_MUTEX(panel_lock);
   static LIST_HEAD(panel_list);
+struct dp_aux_backlight {
+    struct backlight_device *base;
+    struct drm_dp_aux *aux;
+    struct drm_edp_backlight_info info;
+    bool enabled;
+};
+
   /**
    * DOC: drm panel
    *

@@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel)
       return 0;
   }
   EXPORT_SYMBOL(drm_panel_of_backlight);
+
+static int dp_aux_backlight_update_status(struct backlight_device *bd)
+{
+    struct dp_aux_backlight *bl = bl_get_data(bd);
+    u16 brightness = backlight_get_brightness(bd);
+    int ret = 0;
+
+    if (!backlight_is_blank(bd)) {
+        if (!bl->enabled) {
+            drm_edp_backlight_enable(bl->aux, &bl->info, brightness);
+            bl->enabled = true;
+            return 0;
+        }
+        ret = drm_edp_backlight_set_level(bl->aux, &bl->info, 
 brightness);
+    } else {
+        if (bl->enabled) {
+            drm_edp_backlight_disable(bl->aux, &bl->info);
+            bl->enabled = false;
+        }
+    }
+
+    return ret;
+}
+
+static const struct backlight_ops dp_aux_bl_ops = {
+    .update_status = dp_aux_backlight_update_status,
+};
+
+/**
+ * drm_panel_dp_aux_backlight - create and use DP AUX backlight
+ * @panel: DRM panel
+ * @aux: The DP AUX channel to use
+ *
+ * Use this function to create and handle backlight if your panel
+ * supports backlight control over DP AUX channel using DPCD
+ * registers as per VESA's standard backlight control interface.
+ *
+ * When the panel is enabled backlight will be enabled after a
+ * successful call to &drm_panel_funcs.enable()
+ *
+ * When the panel is disabled backlight will be disabled before the
+ * call to &drm_panel_funcs.disable().
+ *
+ * A typical implementation for a panel driver supporting backlight
+ * control over DP AUX will call this function at probe time.
+ * Backlight will then be handled transparently without requiring
+ * any intervention from the driver.
+ *
+ * drm_panel_dp_aux_backlight() must be called after the call to

drm_panel_init().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+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;
+    u8 current_mode;
+    u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+    int ret;

Quoted 8 lines

drivers/gpu/drm/drm_panel.c int drm_panel_of_backlight(struct drm_panel *panel) · line 418

Quoted 6 lines
drivers/gpu/drm/drm_panel.c +6 -0
+
+    if (!panel || !panel->dev || !aux)
+        return -EINVAL;
+
+    ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+                   EDP_DISPLAY_CTL_CAP_SIZE);

This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. drm_panel.c is in the latter, while drm_dp_dpcd_read() in drm_dp_helper.c is in the former. Please fix.

FYI, build DRM as modules and the error shows up during make module_install.

Best regards
Thomas

Signature · 6 lines
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
#4 lore raw

Re: [v8 1/6] drm/panel: add basic DP AUX backlight support

In reply to Thomas Zimmermann

Hi,

Quoted 7 lines · Thomas Zimmermann

drivers/gpu/drm/drm_panel.c int drm_panel_of_backlight(struct drm_panel *panel) · line 418

Quoted 3 lines
drivers/gpu/drm/drm_panel.c +2 -0
+     ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+                            EDP_DISPLAY_CTL_CAP_SIZE);

This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. drm_panel.c is in the latter, while drm_dp_dpcd_read() in drm_dp_helper.c is in the former. Please fix.

Yeah, this was reported Friday and I posted a patch to try to fix it:

https://lore.kernel.org/lkml/20210709152909.1.I23eb4cc5a680341e7b3e791632a635566fa5806a@changeid/

I see Rajeev had some feedback on it. Once I've dug out of my weekend email hole I'll take a look at plan to post a v2 ASAP.

-Doug

#5 lore raw

Re: [v8 1/6] drm/panel: add basic DP AUX backlight support

Hi,

Quoted 12 lines · Doug Anderson

Hi,

Quoted 7 lines · Thomas Zimmermann

drivers/gpu/drm/drm_panel.c int drm_panel_of_backlight(struct drm_panel *panel) · line 418

Quoted 3 lines
drivers/gpu/drm/drm_panel.c +2 -0
+     ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+                            EDP_DISPLAY_CTL_CAP_SIZE);

This creates a cyclic dependency between drm_kms_helper-ko and drm.ko. drm_panel.c is in the latter, while drm_dp_dpcd_read() in drm_dp_helper.c is in the former. Please fix.

Quoted 6 lines

Yeah, this was reported Friday and I posted a patch to try to fix it:

https://lore.kernel.org/lkml/20210709152909.1.I23eb4cc5a680341e7b3e791632a635566fa5806a@changeid/

I see Rajeev had some feedback on it. Once I've dug out of my weekend email hole I'll take a look at plan to post a v2 ASAP.

Breadcrumbs: v2 of the fix is at:

https://lore.kernel.org/lkml/20210712075933.v2.1.I23eb4cc5a680341e7b3e791632a635566fa5806a@changeid/

I'm hoping this looks OK so we can get this resolved before it trips too many people up.

-Doug

author
#6 lore raw

PATCH v8 2/6 drm/panel-simple: Support DP AUX backlight

In reply to Rajeev Nandan

If there is no backlight specified in the device tree and the panel has access to the DP AUX channel then create a DP AUX backlight if supported by the panel.

Signed-off-by Rajeev Nandan <[email protected]>
Reviewed-by Douglas Anderson <[email protected]>

---

(no changes since v5)

This patch depends on the previous patch (2/5) of this series.

Changes in v4:
- New

Changes in v5:
- Address review comments and move backlight functions to drm_panel.c (Douglas)
- Create and register DP AUX backlight if there is no backlight specified in the device tree and panel has the DP AUX channel. (Douglas)
- The new drm_panel_dp_aux_backlight() will do the drm_edp_backlight_supported() check.

 drivers/gpu/drm/panel/panel-simple.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/panel/panel-simple.c +6 -0
index df6fbd1..26555ec 100644
@@ -800,6 +800,12 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
 	if (err)
 		goto disable_pm_runtime;
 
+	if (!panel->base.backlight && panel->aux) {
+		err = drm_panel_dp_aux_backlight(&panel->base, panel->aux);
+		if (err)
+			goto disable_pm_runtime;
+	}
+
 	drm_panel_add(&panel->base);
 
 	return 0;
Signature · 1 line
2.7.4
author
#7 lore raw

PATCH v8 3/6 drm/panel-simple: Support for delays between GPIO & regulator

In reply to Rajeev Nandan

Some panels datasheets may specify a delay between the enable GPIO and the regulator. Support this in panel-simple.

Signed-off-by Rajeev Nandan <[email protected]>
Reviewed-by Douglas Anderson <[email protected]>

---

(no changes since v6)

Changes in v4:
- New

Changes in v5:
- Update description (Douglas)
- Warn if "power_to_enable" or "disable_to_power_off" is non-zero and panel->enable_gpio is NULL (Douglas)

Changes in v6:
- Update warning message to make it more meaningful. (Douglas)

 drivers/gpu/drm/panel/panel-simple.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
drivers/gpu/drm/panel/panel-simple.c +28 -0
index 26555ec..86e5a45 100644
@@ -133,6 +133,22 @@ struct panel_desc {
 		unsigned int prepare_to_enable;
 
 		/**
+		 * @delay.power_to_enable: Time for the power to enable the display on.
+		 *
+		 * The time (in milliseconds) to wait after powering up the display
+		 * before asserting its enable pin.
+		 */
+		unsigned int power_to_enable;
+
+		/**
+		 * @delay.disable_to_power_off: Time for the disable to power the display off.
+		 *
+		 * The time (in milliseconds) to wait before powering off the display
+		 * after deasserting its enable pin.
+		 */
+		unsigned int disable_to_power_off;
+
+		/**
 		 * @delay.enable: Time for the panel to display a valid frame.
 		 *
 		 * The time (in milliseconds) that it takes for the panel to

@@ -347,6 +363,10 @@ static int panel_simple_suspend(struct device *dev)
 	struct panel_simple *p = dev_get_drvdata(dev);
 
 	gpiod_set_value_cansleep(p->enable_gpio, 0);
+
+	if (p->desc->delay.disable_to_power_off)
+		msleep(p->desc->delay.disable_to_power_off);
+
 	regulator_disable(p->supply);
 	p->unprepared_time = ktime_get();
 

@@ -407,6 +427,9 @@ static int panel_simple_prepare_once(struct panel_simple *p)
 		return err;
 	}
 
+	if (p->desc->delay.power_to_enable)
+		msleep(p->desc->delay.power_to_enable);
+
 	gpiod_set_value_cansleep(p->enable_gpio, 1);
 
 	delay = p->desc->delay.prepare;

@@ -782,6 +805,11 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
 		break;
 	}
 
+	if (!panel->enable_gpio && desc->delay.disable_to_power_off)
+		dev_warn(dev, "Need a delay after disabling panel GPIO, but a GPIO wasn't provided\n");
+	if (!panel->enable_gpio && desc->delay.power_to_enable)
+		dev_warn(dev, "Need a delay before enabling panel GPIO, but a GPIO wasn't provided\n");
+
 	dev_set_drvdata(dev, panel);
 
 	/*
Signature · 1 line
2.7.4
author
#8 lore raw

PATCH v8 4/6 drm/panel-simple: Update validation warnings for eDP panel description

In reply to Rajeev Nandan

Do not give a warning for the eDP panels if the "bus_format" is not specified, since most eDP panels can support more than one bus formats and this can be auto-detected.
Also, update the check to include bpc=10 for the eDP panel.

Signed-off-by Rajeev Nandan <[email protected]>

---

Changes in v8:
- New patch, to address the review comments of Sam Ravnborg [1]

[1] https://lore.kernel.org/dri-devel/[email protected]/

 drivers/gpu/drm/panel/panel-simple.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/panel/panel-simple.c +2 -4
index 86e5a45..f966b562 100644
@@ -772,10 +772,8 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
 			desc->bpc != 8);
 		break;
 	case DRM_MODE_CONNECTOR_eDP:
-		if (desc->bus_format == 0)
-			dev_warn(dev, "Specify missing bus_format\n");
-		if (desc->bpc != 6 && desc->bpc != 8)
-			dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
+		if (desc->bpc != 6 && desc->bpc != 8 && desc->bpc != 10)
+			dev_warn(dev, "Expected bpc in {6,8,10} but got: %u\n", desc->bpc);
 		break;
 	case DRM_MODE_CONNECTOR_DSI:
 		if (desc->bpc != 6 && desc->bpc != 8)
Signature · 1 line
2.7.4
#9 lore raw

Re: [v8 4/6] drm/panel-simple: Update validation warnings for eDP panel description

Hi Rajeev,

Quoted 21 lines · Rajeev Nandan

Do not give a warning for the eDP panels if the "bus_format" is not specified, since most eDP panels can support more than one bus formats and this can be auto-detected.
Also, update the check to include bpc=10 for the eDP panel.

Signed-off-by Rajeev Nandan <[email protected]>

---

Changes in v8:
- New patch, to address the review comments of Sam Ravnborg [1]

[1] https://lore.kernel.org/dri-devel/[email protected]/

 drivers/gpu/drm/panel/panel-simple.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/panel/panel-simple.c +0 -0
index 86e5a45..f966b562 100644

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_probe(struct device *dev, const struct panel_desc *desc, · line 776

Quoted 9 lines
drivers/gpu/drm/panel/panel-simple.c +2 -4
@@ -772,10 +772,8 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
 			desc->bpc != 8);
 		break;
 	case DRM_MODE_CONNECTOR_eDP:
-		if (desc->bus_format == 0)
-			dev_warn(dev, "Specify missing bus_format\n");
-		if (desc->bpc != 6 && desc->bpc != 8)
-			dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
+		if (desc->bpc != 6 && desc->bpc != 8 && desc->bpc != 10)
+			dev_warn(dev, "Expected bpc in {6,8,10} but got: %u\n", desc->bpc);

You'll still get a warning is bpc == 0, is that intentional ?

Quoted 3 lines
drivers/gpu/drm/panel/panel-simple.c +0 -0
 		break;
 	case DRM_MODE_CONNECTOR_DSI:
 		if (desc->bpc != 6 && desc->bpc != 8)
Signature · 3 lines
Regards,

Laurent Pinchart
author
#10 lore raw

Re: [v8 4/6] drm/panel-simple: Update validation warnings for eDP panel description

Hi Laurent,

Quoted 29 lines · Laurent Pinchart

Hi Rajeev,

Quoted 26 lines · Rajeev Nandan

Do not give a warning for the eDP panels if the "bus_format" is not specified, since most eDP panels can support more than one bus formats and this can be auto-detected.
Also, update the check to include bpc=10 for the eDP panel.

Signed-off-by Rajeev Nandan <[email protected]>

---

Changes in v8:
- New patch, to address the review comments of Sam Ravnborg [1]

[1] https://lore.kernel.org/dri-devel/[email protected]/

 drivers/gpu/drm/panel/panel-simple.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/panel/panel-simple.c +0 -1
index 86e5a45..f966b562 100644
@@ -772,10 +772,8 @@ static int panel_simple_probe(struct device *dev, 
 const struct panel_desc *desc,
 			desc->bpc != 8);
 		break;
 	case DRM_MODE_CONNECTOR_eDP:
-		if (desc->bus_format == 0)
Quoted 8 lines

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,

Quoted 6 lines
drivers/gpu/drm/panel/panel-simple.c +2 -3
-			dev_warn(dev, "Specify missing bus_format\n");
-		if (desc->bpc != 6 && desc->bpc != 8)
-			dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
+		if (desc->bpc != 6 && desc->bpc != 8 && desc->bpc != 10)
+			dev_warn(dev, "Expected bpc in {6,8,10} but got: %u\n", 
 desc->bpc);

You'll still get a warning is bpc == 0, is that intentional ?

This was not intentional, I missed considering bpc=0 case. As we are removing the warning for bus_format=0 then a similar thing can be done for the bpc=0 also. The bpc value should be a valid one if it is specified. Unlike the bus_format, bpc has few possible values that can be checked here along with 0. Please correct me if I misunderstood the concept. I will fix this.

Thanks,
Rajeev

#11 lore raw

Re: [v8 4/6] drm/panel-simple: Update validation warnings for eDP panel description

Hi Rajeev,

Quoted 38 lines · On Mon, Jun 28, 2021 at 05:46:24PM +0530, [email protected] wrote:
Quoted 28 lines · Laurent Pinchart
Quoted 27 lines · Rajeev Nandan

Do not give a warning for the eDP panels if the "bus_format" is not specified, since most eDP panels can support more than one bus formats and this can be auto-detected.
Also, update the check to include bpc=10 for the eDP panel.

Signed-off-by Rajeev Nandan <[email protected]>

---

Changes in v8:
- New patch, to address the review comments of Sam Ravnborg [1]

[1]
https://lore.kernel.org/dri-devel/[email protected]/

 drivers/gpu/drm/panel/panel-simple.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/panel/panel-simple.c +0 -0
index 86e5a45..f966b562 100644
@@ -772,10 +772,8 @@ static int panel_simple_probe(struct device *dev, 
 const struct panel_desc *desc,
 			desc->bpc != 8);
 		break;
 	case DRM_MODE_CONNECTOR_eDP:
Quoted 8 lines

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_probe(struct device *dev, const struct panel_desc *desc, · line 776

Quoted 6 lines
drivers/gpu/drm/panel/panel-simple.c +2 -4
-		if (desc->bus_format == 0)
-			dev_warn(dev, "Specify missing bus_format\n");
-		if (desc->bpc != 6 && desc->bpc != 8)
-			dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
+		if (desc->bpc != 6 && desc->bpc != 8 && desc->bpc != 10)
+			dev_warn(dev, "Expected bpc in {6,8,10} but got: %u\n", desc->bpc);

You'll still get a warning is bpc == 0, is that intentional ?

Quoted 7 lines

This was not intentional, I missed considering bpc=0 case. As we are removing the warning for bus_format=0 then a similar thing can be done for the bpc=0 also. The bpc value should be a valid one if it is specified. Unlike the bus_format, bpc has few possible values that can be checked here along with 0. Please correct me if I misunderstood the concept.
I will fix this.

What's the point of specifying bpc if it's optional though ? Users of the panel will need to support the case where bpc is set to 0. Have you ensured that they all do ? Can they meaningfully use the bpc value if they need to be ready to support bpc == 0 ?

Signature · 3 lines
Regards,

Laurent Pinchart
Reviewed-by
#12 lore raw

Re: [v8 4/6] drm/panel-simple: Update validation warnings for eDP panel description

Hi,

Quoted 50 lines · Laurent Pinchart

Hi Rajeev,

Quoted 38 lines · On Mon, Jun 28, 2021 at 05:46:24PM +0530, [email protected] wrote:
Quoted 28 lines · Laurent Pinchart
Quoted 27 lines · Rajeev Nandan

Do not give a warning for the eDP panels if the "bus_format" is not specified, since most eDP panels can support more than one bus formats and this can be auto-detected.
Also, update the check to include bpc=10 for the eDP panel.

Signed-off-by Rajeev Nandan <[email protected]>

---

Changes in v8:
- New patch, to address the review comments of Sam Ravnborg [1]

[1]
https://lore.kernel.org/dri-devel/[email protected]/

 drivers/gpu/drm/panel/panel-simple.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/panel/panel-simple.c +0 -0
index 86e5a45..f966b562 100644
@@ -772,10 +772,8 @@ static int panel_simple_probe(struct device *dev,
 const struct panel_desc *desc,
                   desc->bpc != 8);
           break;
   case DRM_MODE_CONNECTOR_eDP:
Quoted 8 lines

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_probe(struct device *dev, const struct panel_desc *desc, · line 776

Quoted 6 lines
drivers/gpu/drm/panel/panel-simple.c +2 -4
-          if (desc->bus_format == 0)
-                  dev_warn(dev, "Specify missing bus_format\n");
-          if (desc->bpc != 6 && desc->bpc != 8)
-                  dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
+          if (desc->bpc != 6 && desc->bpc != 8 && desc->bpc != 10)
+                  dev_warn(dev, "Expected bpc in {6,8,10} but got: %u\n", desc->bpc);

You'll still get a warning is bpc == 0, is that intentional ?

Quoted 7 lines

This was not intentional, I missed considering bpc=0 case. As we are removing the warning for bus_format=0 then a similar thing can be done for the bpc=0 also. The bpc value should be a valid one if it is specified. Unlike the bus_format, bpc has few possible values that can be checked here along with 0. Please correct me if I misunderstood the concept.
I will fix this.

Quoted 4 lines

What's the point of specifying bpc if it's optional though ? Users of the panel will need to support the case where bpc is set to 0. Have you ensured that they all do ? Can they meaningfully use the bpc value if they need to be ready to support bpc == 0 ?

I must be missing something, but to me it seems like Rajeev's patch is fine as-is. From my reading of the code:

* Removes the warning if bus_format == 0. This is correct since I don't think specifying bus format for eDP panels makes lots of sense.

* Removes the warning if bpc == 10. This is correct since we've seen eDP panels with 10bpc.

* Keeps the warning if bpc == 0. IMO we can/should still require panels to specify their BPC. I guess I'm treating this as a "max BPC". I know that we use this field in the sn65dsi86 driver, so if it's OK for this to be 0 then we'll have to change that driver to handle it.

Does that sound right to you Laurent? So since I think Rajeev's patch is OK, I'm happy with:

Reviewed-by Douglas Anderson <[email protected]>

Unless I missed something and this patch needs to change then it feels like Rajeev's patch series is in pretty good shape to land. I'm happy to commit it but since Sam made comments on the previous version I'd plan to wait a bit to make sure he has a chance for another look if he wants to. I've also only got 2 days left before I vanish for 1 week of vacation. ...so my plan is:
* If Sam / Laurent come back before tomorrow and say they're happy then I'll commit.
* If I hear nothing then I'll check back after my vacation. If someone else has committed then I'll be happy. If not and there has just been silence then I'll commit it myself.

Please yell if that's not OK. :-)

-Doug

#13 lore raw

Re: [v8 4/6] drm/panel-simple: Update validation warnings for eDP panel description

Hi Doug,

Quoted 53 lines · Doug Anderson
Quoted 47 lines · Laurent Pinchart
Quoted 38 lines · On Mon, Jun 28, 2021 at 05:46:24PM +0530, [email protected] wrote:
Quoted 28 lines · Laurent Pinchart
Quoted 27 lines · Rajeev Nandan

Do not give a warning for the eDP panels if the "bus_format" is not specified, since most eDP panels can support more than one bus formats and this can be auto-detected.
Also, update the check to include bpc=10 for the eDP panel.

Signed-off-by Rajeev Nandan <[email protected]>

---

Changes in v8:
- New patch, to address the review comments of Sam Ravnborg [1]

[1]
https://lore.kernel.org/dri-devel/[email protected]/

 drivers/gpu/drm/panel/panel-simple.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/panel/panel-simple.c +0 -0
index 86e5a45..f966b562 100644
@@ -772,10 +772,8 @@ static int panel_simple_probe(struct device *dev,
 const struct panel_desc *desc,
                   desc->bpc != 8);
           break;
   case DRM_MODE_CONNECTOR_eDP:
Quoted 8 lines

drivers/gpu/drm/panel/panel-simple.c static int panel_simple_probe(struct device *dev, const struct panel_desc *desc, · line 776

Quoted 6 lines
drivers/gpu/drm/panel/panel-simple.c +2 -4
-          if (desc->bus_format == 0)
-                  dev_warn(dev, "Specify missing bus_format\n");
-          if (desc->bpc != 6 && desc->bpc != 8)
-                  dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
+          if (desc->bpc != 6 && desc->bpc != 8 && desc->bpc != 10)
+                  dev_warn(dev, "Expected bpc in {6,8,10} but got: %u\n", desc->bpc);

You'll still get a warning is bpc == 0, is that intentional ?

Quoted 7 lines

This was not intentional, I missed considering bpc=0 case. As we are removing the warning for bus_format=0 then a similar thing can be done for the bpc=0 also. The bpc value should be a valid one if it is specified. Unlike the bus_format, bpc has few possible values that can be checked here along with 0. Please correct me if I misunderstood the concept.
I will fix this.

Quoted 4 lines

What's the point of specifying bpc if it's optional though ? Users of the panel will need to support the case where bpc is set to 0. Have you ensured that they all do ? Can they meaningfully use the bpc value if they need to be ready to support bpc == 0 ?

Quoted 5 lines

I must be missing something, but to me it seems like Rajeev's patch is fine as-is. From my reading of the code:

* Removes the warning if bus_format == 0. This is correct since I don't think specifying bus format for eDP panels makes lots of sense.

This is embarassing, I've been reading it as desc->bpc == 0 from the beginning :-( My bad. The bpc change is correct.

Quoted 26 lines

* Removes the warning if bpc == 10. This is correct since we've seen eDP panels with 10bpc.

* Keeps the warning if bpc == 0. IMO we can/should still require panels to specify their BPC. I guess I'm treating this as a "max BPC". I know that we use this field in the sn65dsi86 driver, so if it's OK for this to be 0 then we'll have to change that driver to handle it.

Does that sound right to you Laurent? So since I think Rajeev's patch is OK, I'm happy with:

Reviewed-by Douglas Anderson <[email protected]>

Unless I missed something and this patch needs to change then it feels like Rajeev's patch series is in pretty good shape to land. I'm happy to commit it but since Sam made comments on the previous version I'd plan to wait a bit to make sure he has a chance for another look if he wants to. I've also only got 2 days left before I vanish for 1 week of vacation. ...so my plan is:
* If Sam / Laurent come back before tomorrow and say they're happy then I'll commit.
* If I hear nothing then I'll check back after my vacation. If someone else has committed then I'll be happy. If not and there has just been silence then I'll commit it myself.

Please yell if that's not OK. :-)

Signature · 3 lines
Regards,

Laurent Pinchart
author
#14 lore raw

PATCH v8 5/6 dt-bindings: display: simple: Add Samsung ATNA33XC20

In reply to Rajeev Nandan

Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by Rajeev Nandan <[email protected]>
Reviewed-by Douglas Anderson <[email protected]>
Acked-by Rob Herring <[email protected]>

---

(no changes since v4)

Changes in v4:
- New

 Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/display/panel/panel-simple.yaml +2 -0
index 4a0a5e1..f5acfd6 100644
@@ -242,6 +242,8 @@ properties:
       - rocktech,rk101ii01d-ct
         # Rocktech Display Ltd. RK070ER9427 800(RGB)x480 TFT LCD panel
       - rocktech,rk070er9427
+        # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
+      - samsung,atna33xc20
         # Samsung 12.2" (2560x1600 pixels) TFT LCD panel
       - samsung,lsn122dl01-c01
         # Samsung Electronics 10.1" WSVGA TFT LCD panel
Signature · 1 line
2.7.4
author
#15 lore raw

PATCH v8 6/6 drm/panel-simple: Add Samsung ATNA33XC20

In reply to Rajeev Nandan

Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by Rajeev Nandan <[email protected]>
Reviewed-by Douglas Anderson <[email protected]>

---

(No changes since v7)

Changes in v4:
- New

Changes in v5:
- Remove "uses_dpcd_backlight" property, not required now. (Douglas)

Changes in v7:
- Update disable_to_power_off and power_to_enable delays. (Douglas)

 drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
drivers/gpu/drm/panel/panel-simple.c +33 -0
index f966b562..e541257 100644
@@ -3560,6 +3560,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = {
 	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
+static const struct drm_display_mode samsung_atna33xc20_mode = {
+	.clock = 138770,
+	.hdisplay = 1920,
+	.hsync_start = 1920 + 48,
+	.hsync_end = 1920 + 48 + 32,
+	.htotal = 1920 + 48 + 32 + 80,
+	.vdisplay = 1080,
+	.vsync_start = 1080 + 8,
+	.vsync_end = 1080 + 8 + 8,
+	.vtotal = 1080 + 8 + 8 + 16,
+	.flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc samsung_atna33xc20 = {
+	.modes = &samsung_atna33xc20_mode,
+	.num_modes = 1,
+	.bpc = 10,
+	.size = {
+		.width = 294,
+		.height = 165,
+	},
+	.delay = {
+		.disable_to_power_off = 200,
+		.power_to_enable = 400,
+		.hpd_absent_delay = 200,
+		.unprepare = 500,
+	},
+	.connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
 static const struct drm_display_mode samsung_lsn122dl01_c01_mode = {
 	.clock = 271560,
 	.hdisplay = 2560,

@@ -4561,6 +4591,9 @@ static const struct of_device_id platform_of_match[] = {
 		.compatible = "rocktech,rk101ii01d-ct",
 		.data = &rocktech_rk101ii01d_ct,
 	}, {
+		.compatible = "samsung,atna33xc20",
+		.data = &samsung_atna33xc20,
+	}, {
 		.compatible = "samsung,lsn122dl01-c01",
 		.data = &samsung_lsn122dl01_c01,
 	}, {
Signature · 1 line
2.7.4
Applied
#16 lore raw
In reply to Rajeev Nandan

Hi,

Quoted 70 lines · Rajeev Nandan

This series adds the support for the eDP panel that needs the backlight controlling over the DP AUX channel using DPCD registers of the panel as per the VESA's standard.

This series also adds support for the Samsung eDP AMOLED panel that needs DP AUX to control the backlight, and introduces new delays in the @panel_desc.delay to support this panel.

This patch series depends on the following two series:
- Doug's series [1], exposed the DP AUX channel to the panel-simple.
- Lyude's series [2], introduced new drm helper functions for DPCD backlight.

This series is the logical successor to the series [3].

Changes in v1:
- Created dpcd backlight helper with very basic functionality, added backlight registration in the ti-sn65dsi86 bridge driver.

Changes in v2:
- Created a new DisplayPort aux backlight driver and moved the code from drm_dp_aux_backlight.c (v1) to the new driver.

Changes in v3:
- Fixed module compilation (kernel test bot).

Changes in v4:
- Added basic DPCD backlight support in panel-simple.
- Added support for a new Samsung panel ATNA33XC20 that needs DPCD backlight controlling and has a requirement of delays between enable GPIO and regulator.

Changes in v5:
Addressed review suggestions from Douglas:
- Created a new API drm_panel_dp_aux_backlight() in drm_panel.c
- Moved DP AUX backlight functions from panel-simple.c to drm_panel.c
- panel-simple probe() calls drm_panel_dp_aux_backlight() to create backlight when the backlight phandle is not specified in panel DT and DP AUX channel is present.
- Added check for drm_edp_backlight_supported() before registering.
- Removed the @uses_dpcd_backlight flag from panel_desc as this should be auto-detected.
- Updated comments/descriptions.

Changes in v6:
- Rebased
- Updated wanrning messages, fixed word wrapping in comments.
- Fixed ordering of memory allocation

Changes in v7:
- Updated the disable_to_power_off and power_to_enable panel delays as discovered at <https://crrev.com/c/2966167> (Douglas)

Changes in v8:
- Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg)
- Added a new patch #4 to fix the warnings for eDP panel description (Sam Ravnborg)

[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://lore.kernel.org/dri-devel/[email protected]/
[3] https://lore.kernel.org/dri-devel/[email protected]/

Rajeev Nandan (6):
drm/panel: add basic DP AUX backlight support drm/panel-simple: Support DP AUX backlight drm/panel-simple: Support for delays between GPIO & regulator drm/panel-simple: Update validation warnings for eDP panel description
dt-bindings: display: simple: Add Samsung ATNA33XC20 drm/panel-simple: Add Samsung ATNA33XC20

Quoted 5 lines
 .../bindings/display/panel/panel-simple.yaml       |   2 +
 drivers/gpu/drm/drm_panel.c                        | 108 +++++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c               |  73 +++++++++++++-
 include/drm/drm_panel.h                            |  15 ++-
 4 files changed, 190 insertions(+), 8 deletions(-)

Pushed to drm-misc-next.

4bfe6c8f7c23 drm/panel-simple: Add Samsung ATNA33XC20 c20dec193584 dt-bindings: display: simple: Add Samsung ATNA33XC20 13aceea56fd5 drm/panel-simple: Update validation warnings for eDP panel description
18a1488bf1e1 drm/panel-simple: Support for delays between GPIO & regulator bfd451403d70 drm/panel-simple: Support DP AUX backlight 10f7b40e4f30 drm/panel: add basic DP AUX backlight support

-Doug

#17 lore raw
Quoted 81 lines · Doug Anderson

Hi,

Quoted 70 lines · Rajeev Nandan

This series adds the support for the eDP panel that needs the backlight controlling over the DP AUX channel using DPCD registers of the panel as per the VESA's standard.

This series also adds support for the Samsung eDP AMOLED panel that needs DP AUX to control the backlight, and introduces new delays in the @panel_desc.delay to support this panel.

This patch series depends on the following two series:
- Doug's series [1], exposed the DP AUX channel to the panel-simple.
- Lyude's series [2], introduced new drm helper functions for DPCD backlight.

This series is the logical successor to the series [3].

Changes in v1:
- Created dpcd backlight helper with very basic functionality, added backlight registration in the ti-sn65dsi86 bridge driver.

Changes in v2:
- Created a new DisplayPort aux backlight driver and moved the code from drm_dp_aux_backlight.c (v1) to the new driver.

Changes in v3:
- Fixed module compilation (kernel test bot).

Changes in v4:
- Added basic DPCD backlight support in panel-simple.
- Added support for a new Samsung panel ATNA33XC20 that needs DPCD backlight controlling and has a requirement of delays between enable GPIO and regulator.

Changes in v5:
Addressed review suggestions from Douglas:
- Created a new API drm_panel_dp_aux_backlight() in drm_panel.c
- Moved DP AUX backlight functions from panel-simple.c to drm_panel.c
- panel-simple probe() calls drm_panel_dp_aux_backlight() to create backlight when the backlight phandle is not specified in panel DT and DP AUX channel is present.
- Added check for drm_edp_backlight_supported() before registering.
- Removed the @uses_dpcd_backlight flag from panel_desc as this should be auto-detected.
- Updated comments/descriptions.

Changes in v6:
- Rebased
- Updated wanrning messages, fixed word wrapping in comments.
- Fixed ordering of memory allocation

Changes in v7:
- Updated the disable_to_power_off and power_to_enable panel delays as discovered at <https://crrev.com/c/2966167> (Douglas)

Changes in v8:
- Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg)
- Added a new patch #4 to fix the warnings for eDP panel description (Sam Ravnborg)

[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://lore.kernel.org/dri-devel/[email protected]/
[3] https://lore.kernel.org/dri-devel/[email protected]/

Rajeev Nandan (6):
drm/panel: add basic DP AUX backlight support drm/panel-simple: Support DP AUX backlight drm/panel-simple: Support for delays between GPIO & regulator drm/panel-simple: Update validation warnings for eDP panel description
dt-bindings: display: simple: Add Samsung ATNA33XC20 drm/panel-simple: Add Samsung ATNA33XC20

Quoted 5 lines
 .../bindings/display/panel/panel-simple.yaml       |   2 +
 drivers/gpu/drm/drm_panel.c                        | 108 +++++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c               |  73 +++++++++++++-
 include/drm/drm_panel.h                            |  15 ++-
 4 files changed, 190 insertions(+), 8 deletions(-)

Pushed to drm-misc-next.

Quoted 7 lines

4bfe6c8f7c23 drm/panel-simple: Add Samsung ATNA33XC20 c20dec193584 dt-bindings: display: simple: Add Samsung ATNA33XC20 13aceea56fd5 drm/panel-simple: Update validation warnings for eDP panel description
18a1488bf1e1 drm/panel-simple: Support for delays between GPIO & regulator bfd451403d70 drm/panel-simple: Support DP AUX backlight 10f7b40e4f30 drm/panel: add basic DP AUX backlight support

depmod: ERROR: Cycle detected: drm_kms_helper -> drm -> drm_kms_helper

Looks to be due to drm_edp_backlight_enable().

Signature · 2 lines
Ville Syrjälä
Intel
#18 lore raw

Hi,

Quoted 91 lines · Ville Syrjälä
Quoted 81 lines · Doug Anderson

Hi,

Quoted 70 lines · Rajeev Nandan

This series adds the support for the eDP panel that needs the backlight controlling over the DP AUX channel using DPCD registers of the panel as per the VESA's standard.

This series also adds support for the Samsung eDP AMOLED panel that needs DP AUX to control the backlight, and introduces new delays in the @panel_desc.delay to support this panel.

This patch series depends on the following two series:
- Doug's series [1], exposed the DP AUX channel to the panel-simple.
- Lyude's series [2], introduced new drm helper functions for DPCD backlight.

This series is the logical successor to the series [3].

Changes in v1:
- Created dpcd backlight helper with very basic functionality, added backlight registration in the ti-sn65dsi86 bridge driver.

Changes in v2:
- Created a new DisplayPort aux backlight driver and moved the code from drm_dp_aux_backlight.c (v1) to the new driver.

Changes in v3:
- Fixed module compilation (kernel test bot).

Changes in v4:
- Added basic DPCD backlight support in panel-simple.
- Added support for a new Samsung panel ATNA33XC20 that needs DPCD backlight controlling and has a requirement of delays between enable GPIO and regulator.

Changes in v5:
Addressed review suggestions from Douglas:
- Created a new API drm_panel_dp_aux_backlight() in drm_panel.c
- Moved DP AUX backlight functions from panel-simple.c to drm_panel.c
- panel-simple probe() calls drm_panel_dp_aux_backlight() to create backlight when the backlight phandle is not specified in panel DT and DP AUX channel is present.
- Added check for drm_edp_backlight_supported() before registering.
- Removed the @uses_dpcd_backlight flag from panel_desc as this should be auto-detected.
- Updated comments/descriptions.

Changes in v6:
- Rebased
- Updated wanrning messages, fixed word wrapping in comments.
- Fixed ordering of memory allocation

Changes in v7:
- Updated the disable_to_power_off and power_to_enable panel delays as discovered at <https://crrev.com/c/2966167> (Douglas)

Changes in v8:
- Now using backlight_is_blank() to get the backlight blank status (Sam Ravnborg)
- Added a new patch #4 to fix the warnings for eDP panel description (Sam Ravnborg)

[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://lore.kernel.org/dri-devel/[email protected]/
[3] https://lore.kernel.org/dri-devel/[email protected]/

Rajeev Nandan (6):
drm/panel: add basic DP AUX backlight support drm/panel-simple: Support DP AUX backlight drm/panel-simple: Support for delays between GPIO & regulator drm/panel-simple: Update validation warnings for eDP panel description
dt-bindings: display: simple: Add Samsung ATNA33XC20 drm/panel-simple: Add Samsung ATNA33XC20

Quoted 5 lines
 .../bindings/display/panel/panel-simple.yaml       |   2 +
 drivers/gpu/drm/drm_panel.c                        | 108 +++++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c               |  73 +++++++++++++-
 include/drm/drm_panel.h                            |  15 ++-
 4 files changed, 190 insertions(+), 8 deletions(-)

Pushed to drm-misc-next.

Quoted 7 lines

4bfe6c8f7c23 drm/panel-simple: Add Samsung ATNA33XC20 c20dec193584 dt-bindings: display: simple: Add Samsung ATNA33XC20 13aceea56fd5 drm/panel-simple: Update validation warnings for eDP panel description
18a1488bf1e1 drm/panel-simple: Support for delays between GPIO & regulator bfd451403d70 drm/panel-simple: Support DP AUX backlight 10f7b40e4f30 drm/panel: add basic DP AUX backlight support

Quoted 3 lines

depmod: ERROR: Cycle detected: drm_kms_helper -> drm -> drm_kms_helper

Looks to be due to drm_edp_backlight_enable().

Ugh. Thanks for the report! I've taken a schwag at a fix here:

https://lore.kernel.org/lkml/20210709152909.1.I23eb4cc5a680341e7b3e791632a635566fa5806a@changeid/

-Doug