[v8 0/6] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

[v8 0/6] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

[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/20210525000159.3384921-1-dianders@chromium.org/
[2] https://lore.kernel.org/dri-devel/20210514181504.565252-1-lyude@redhat.com/
[3] https://lore.kernel.org/dri-devel/1619416756-3533-1-git-send-email-rajeevny@codeaurora.org/

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(-)

-- 
2.7.4

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

[Rajeev Nandan]

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 <rajeevny@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Lyude Paul <lyude@redhat.com>
---

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(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index f634371..4fa1e3b 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -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 <treding@nvidia.com>");
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 33605c3..3ebfaa6 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -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
-- 
2.7.4

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

[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 <rajeevny@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---

(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(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index df6fbd1..26555ec 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -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;
-- 
2.7.4

Re: [v8 0/6] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

[Doug Anderson]

Hi,

On Sat, Jun 26, 2021 at 9:52 AM Rajeev Nandan <rajeevny@codeaurora.org> wrote:
>
> 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/20210525000159.3384921-1-dianders@chromium.org/
> [2] https://lore.kernel.org/dri-devel/20210514181504.565252-1-lyude@redhat.com/
> [3] https://lore.kernel.org/dri-devel/1619416756-3533-1-git-send-email-rajeevny@codeaurora.org/
>
> 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(-)

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

Re: [v8 0/6] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

[Ville Syrjälä]

On Fri, Jul 09, 2021 at 06:54:05AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Sat, Jun 26, 2021 at 9:52 AM Rajeev Nandan <rajeevny@codeaurora.org> wrote:
> >
> > 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/20210525000159.3384921-1-dianders@chromium.org/
> > [2] https://lore.kernel.org/dri-devel/20210514181504.565252-1-lyude@redhat.com/
> > [3] https://lore.kernel.org/dri-devel/1619416756-3533-1-git-send-email-rajeevny@codeaurora.org/
> >
> > 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(-)
> 
> 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

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

Looks to be due to drm_edp_backlight_enable().

-- 
Ville Syrjälä
Intel

Re: [v8 0/6] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

[Doug Anderson]

Hi,

On Fri, Jul 9, 2021 at 1:41 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Fri, Jul 09, 2021 at 06:54:05AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Sat, Jun 26, 2021 at 9:52 AM Rajeev Nandan <rajeevny@codeaurora.org> wrote:
> > >
> > > 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/20210525000159.3384921-1-dianders@chromium.org/
> > > [2] https://lore.kernel.org/dri-devel/20210514181504.565252-1-lyude@redhat.com/
> > > [3] https://lore.kernel.org/dri-devel/1619416756-3533-1-git-send-email-rajeevny@codeaurora.org/
> > >
> > > 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(-)
> >
> > 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
>
> 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

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

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 8080 bytes --]



Am 26.06.21 um 18:51 schrieb Rajeev Nandan:
> 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 <rajeevny@codeaurora.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> ---
> 
> 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(-)
> 
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index f634371..4fa1e3b 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -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);

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

> +	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 <treding@nvidia.com>");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 33605c3..3ebfaa6 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -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
> 

-- 
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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 5696 bytes --]

Hi

Am 12.07.21 um 11:41 schrieb Thomas Zimmermann:
> 
> 
> Am 26.06.21 um 18:51 schrieb Rajeev Nandan:
>> 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 <rajeevny@codeaurora.org>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>> ---
>>
>> 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(-)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index f634371..4fa1e3b 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -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);
> 
> 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


-- 
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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

[Doug Anderson]

Hi,

On Mon, Jul 12, 2021 at 2:41 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> > +     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

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

[Doug Anderson]

Hi,

On Mon, Jul 12, 2021 at 6:39 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jul 12, 2021 at 2:41 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > > +     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.

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