Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge
@ 2021-04-14 16:39 Rajeev Nandan
  2021-04-14 16:39 ` [v1 1/3] drm/dp: Add DisplayPort aux backlight control support Rajeev Nandan
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Rajeev Nandan @ 2021-04-14 16:39 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm, freedreno, devicetree
  Cc: Rajeev Nandan, linux-kernel, robdclark, dianders, mkrishn,
	kalyan_t, hoegsberg, abhinavk, seanpaul

The backlight level of an eDP panel can be controlled through the AUX
channel using DPCD registers of the panel.

The capability for the Source device to adjust backlight characteristics
within the panel, using the Sink device DPCD registers is indicated by
the TCON_BACKLIGHT_ADJUSTMENT_CAPABLE bit in the EDP_GENERAL_CAPABILITY_1
register (DPCD Address 701h, bit0). In this configuration, the eDP TCON
receives the backlight level information from the host, through the AUX
channel.

The changes in this patch series do the following:
- Add drm_dp_aux_backlight_ APIs to support backlight control using DPCD
  registers on the DisplayPort AUX channel.
  The current version only supports backlight brightness control by the
  EDP_BACKLIGHT_BRIGHTNESS_MSB/LSB registers (DPCD Addresses 722h-723h).
- Add support for backlight control of the eDP panel connected to the
  ti-sn65dsi86 bridge.

Rajeev Nandan (3):
  drm/dp: Add DisplayPort aux backlight control support
  dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight
  drm/bridge: ti-sn65dsi86: Add DisplayPort aux backlight support

 .../bindings/display/bridge/ti,sn65dsi86.yaml      |   8 +
 drivers/gpu/drm/Kconfig                            |   8 +
 drivers/gpu/drm/Makefile                           |   1 +
 drivers/gpu/drm/bridge/Kconfig                     |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c              |  26 +++
 drivers/gpu/drm/drm_dp_aux_backlight.c             | 191 +++++++++++++++++++++
 include/drm/drm_dp_aux_backlight.h                 |  29 ++++
 7 files changed, 264 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_dp_aux_backlight.c
 create mode 100644 include/drm/drm_dp_aux_backlight.h

-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [v1 1/3] drm/dp: Add DisplayPort aux backlight control support
  2021-04-14 16:39 [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Rajeev Nandan
@ 2021-04-14 16:39 ` Rajeev Nandan
  2021-04-14 16:39 ` [v1 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight Rajeev Nandan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Rajeev Nandan @ 2021-04-14 16:39 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm, freedreno, devicetree
  Cc: Rajeev Nandan, linux-kernel, robdclark, dianders, mkrishn,
	kalyan_t, hoegsberg, abhinavk, seanpaul

Add panel backlight control using DPCD registers on the DisplayPort aux
channel.

Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
---
 drivers/gpu/drm/Kconfig                |   8 ++
 drivers/gpu/drm/Makefile               |   1 +
 drivers/gpu/drm/drm_dp_aux_backlight.c | 191 +++++++++++++++++++++++++++++++++
 include/drm/drm_dp_aux_backlight.h     |  29 +++++
 4 files changed, 229 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_dp_aux_backlight.c
 create mode 100644 include/drm/drm_dp_aux_backlight.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 85b79a7f..01f8a48 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -177,6 +177,14 @@ config DRM_DP_CEC
 	  Note: not all adapters support this feature, and even for those
 	  that do support this they often do not hook up the CEC pin.
 
+config DRM_DP_AUX_BACKLIGHT
+	bool "Enable DisplayPort aux backlight control support"
+	depends on DRM
+	select DRM_KMS_HELPER
+	help
+	  Choose this option if you want to use panel backlight control
+	  using DPCD registers on the DisplayPort aux channel.
+
 config DRM_TTM
 	tristate
 	depends on DRM && MMU
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 926adef..e41e40f 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -53,6 +53,7 @@ drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
 drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
+drm_kms_helper-$(CONFIG_DRM_DP_AUX_BACKLIGHT) += drm_dp_aux_backlight.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 obj-$(CONFIG_DRM_DEBUG_SELFTEST) += selftests/
diff --git a/drivers/gpu/drm/drm_dp_aux_backlight.c b/drivers/gpu/drm/drm_dp_aux_backlight.c
new file mode 100644
index 0000000..2fc4ffb
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_aux_backlight.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <drm/drm_dp_aux_backlight.h>
+#include <drm/drm_dp_helper.h>
+#include <drm/drm_print.h>
+
+static int drm_dp_aux_brightness_set(struct backlight_device *bd)
+{
+	struct drm_dp_aux_backlight *pdata = bl_get_data(bd);
+	u16 brightness = bd->props.brightness;
+	u8 val[2] = { 0x0 };
+	int ret = 0;
+
+	if (!pdata->enabled)
+		return 0;
+
+	if (bd->props.power != FB_BLANK_UNBLANK ||
+	    bd->props.fb_blank != FB_BLANK_UNBLANK ||
+	    bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
+		brightness = 0;
+
+	val[0] = brightness >> 8;
+	val[1] = brightness & 0xff;
+	ret = drm_dp_dpcd_write(pdata->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
+				val, sizeof(val));
+	if (ret < 0)
+		return ret;
+
+	return ret;
+}
+
+static int drm_dp_aux_brightness_get(struct backlight_device *bd)
+{
+	struct drm_dp_aux_backlight *pdata = bl_get_data(bd);
+	u8 val[2] = { 0x0 };
+	int ret = 0;
+
+	if (!pdata->enabled)
+		return 0;
+
+	if (bd->props.power != FB_BLANK_UNBLANK ||
+	    bd->props.fb_blank != FB_BLANK_UNBLANK ||
+	    bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
+		return 0;
+
+	ret = drm_dp_dpcd_read(pdata->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
+			       &val, sizeof(val));
+	if (ret < 0)
+		return ret;
+
+	return (val[0] << 8 | val[1]);
+}
+
+static const struct backlight_ops aux_bl_ops = {
+	.update_status = drm_dp_aux_brightness_set,
+	.get_brightness = drm_dp_aux_brightness_get,
+};
+
+/**
+ * drm_dp_aux_backlight_enable() - Enable DP aux backlight
+ * @aux_bl: the DP aux backlight to enable
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_aux_backlight_enable(struct drm_dp_aux_backlight *aux_bl)
+{
+	u8 val = 0;
+	int ret;
+
+	if (!aux_bl)
+		return -EINVAL;
+
+	if (aux_bl->enabled)
+		return 0;
+
+	/* Set backlight control mode */
+	ret = drm_dp_dpcd_readb(aux_bl->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
+				&val);
+	if (ret < 0)
+		return ret;
+
+	val &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
+	val |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
+	ret = drm_dp_dpcd_writeb(aux_bl->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
+				 val);
+	if (ret < 0)
+		return ret;
+
+	/* Enable backlight */
+	ret = drm_dp_dpcd_readb(aux_bl->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
+				&val);
+	if (ret < 0)
+		return ret;
+
+	val |= DP_EDP_BACKLIGHT_ENABLE;
+	ret = drm_dp_dpcd_writeb(aux_bl->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
+				 val);
+	if (ret < 0)
+		return ret;
+
+	ret = backlight_enable(aux_bl->bd);
+	if (ret < 0)
+		DRM_DEV_INFO(aux_bl->dev, "failed to enable backlight: %d\n",
+			     ret);
+
+	aux_bl->enabled = true;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_aux_backlight_enable);
+
+/**
+ * drm_dp_aux_backlight_disable() - Disable DP aux backlight
+ * @aux_bl: the DP aux backlight to disable
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_aux_backlight_disable(struct drm_dp_aux_backlight *aux_bl)
+{
+	u8 val = 0;
+	int ret;
+
+	if (!aux_bl)
+		return -EINVAL;
+
+	if (!aux_bl->enabled)
+		return 0;
+
+	ret = backlight_disable(aux_bl->bd);
+	if (ret < 0)
+		DRM_DEV_INFO(aux_bl->dev, "failed to disable backlight: %d\n",
+			     ret);
+
+	ret = drm_dp_dpcd_readb(aux_bl->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
+				&val);
+	if (ret < 0)
+		return ret;
+
+	val &= ~DP_EDP_BACKLIGHT_ENABLE;
+	ret = drm_dp_dpcd_writeb(aux_bl->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
+				 val);
+	if (ret < 0)
+		return ret;
+
+	aux_bl->enabled = false;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_aux_backlight_disable);
+
+/**
+ * drm_dp_aux_backlight_register() - register a DP aux backlight device
+ * @name: the name of the backlight device
+ * @aux_bl: the DP aux backlight to register
+ *
+ * Creates and registers a new backlight device that uses DPCD registers
+ * on the DisplayPort aux channel to control the brightness of the panel.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_aux_backlight_register(const char *name,
+				struct drm_dp_aux_backlight *aux_bl)
+{
+	struct backlight_properties bl_props = { 0 };
+	int max_brightness;
+	int ret = 0;
+
+	if (!name || !aux_bl || !aux_bl->aux)
+		return -EINVAL;
+
+	max_brightness = 0xffff;
+
+	bl_props.type = BACKLIGHT_RAW;
+	bl_props.brightness = max_brightness;
+	bl_props.max_brightness = max_brightness;
+	aux_bl->bd = devm_backlight_device_register(aux_bl->dev, name,
+						  aux_bl->dev, aux_bl,
+						  &aux_bl_ops, &bl_props);
+	if (IS_ERR(aux_bl->bd)) {
+		ret = PTR_ERR(aux_bl->bd);
+		DRM_DEV_ERROR(aux_bl->dev,
+			      "failed to register backlight (%d)\n", ret);
+		aux_bl->bd = NULL;
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_aux_backlight_register);
diff --git a/include/drm/drm_dp_aux_backlight.h b/include/drm/drm_dp_aux_backlight.h
new file mode 100644
index 0000000..23cc554
--- /dev/null
+++ b/include/drm/drm_dp_aux_backlight.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DRM_DP_AUX_BACKLIGHT_H_
+#define _DRM_DP_AUX_BACKLIGHT_H_
+
+#include <linux/backlight.h>
+#include <drm/drm_dp_helper.h>
+
+/**
+ * struct drm_dp_aux_backlight - DisplayPort aux backlight
+ * @dev: the device to register
+ * @aux: the DisplayPort aux channel
+ * @bd: the backlight device
+ * @enabled: true if backlight is enabled else false.
+ */
+struct drm_dp_aux_backlight {
+	struct device *dev;
+	struct drm_dp_aux *aux;
+	struct backlight_device *bd;
+	bool enabled;
+};
+
+int drm_dp_aux_backlight_enable(struct drm_dp_aux_backlight *aux_bl);
+int drm_dp_aux_backlight_disable(struct drm_dp_aux_backlight *aux_bl);
+
+int drm_dp_aux_backlight_register(const char *name,
+				struct drm_dp_aux_backlight *aux_bl);
+
+#endif /* _DRM_DP_AUX_BACKLIGHT_H_ */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [v1 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight
  2021-04-14 16:39 [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Rajeev Nandan
  2021-04-14 16:39 ` [v1 1/3] drm/dp: Add DisplayPort aux backlight control support Rajeev Nandan
@ 2021-04-14 16:39 ` Rajeev Nandan
  2021-04-15 22:04   ` Rob Herring
  2021-04-14 16:39 ` [v1 3/3] drm/bridge: ti-sn65dsi86: Add DisplayPort aux backlight support Rajeev Nandan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Rajeev Nandan @ 2021-04-14 16:39 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm, freedreno, devicetree
  Cc: Rajeev Nandan, linux-kernel, robdclark, dianders, mkrishn,
	kalyan_t, hoegsberg, abhinavk, seanpaul

If the panel connected to the bridge supports backlight control
using DPCD registers on the DisplayPort aux channel, setting
"use-aux-backlight" property in the bridge node will enable the
registration of a DP aux backlight device from the bridge driver.

Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
---
 .../devicetree/bindings/display/bridge/ti,sn65dsi86.yaml          | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
index 26932d2..c8d8c00 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
@@ -58,6 +58,12 @@ properties:
   clock-names:
     const: refclk
 
+  use-aux-backlight:
+    type: boolean
+    description:
+      The panel backlight to be controlled using DPCD registers on
+      the DP aux channel.
+
   gpio-controller: true
   '#gpio-cells':
     const: 2
@@ -188,6 +194,8 @@ examples:
 
         no-hpd;
 
+        use-aux-backlight;
+
         ports {
           #address-cells = <1>;
           #size-cells = <0>;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [v1 3/3] drm/bridge: ti-sn65dsi86: Add DisplayPort aux backlight support
  2021-04-14 16:39 [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Rajeev Nandan
  2021-04-14 16:39 ` [v1 1/3] drm/dp: Add DisplayPort aux backlight control support Rajeev Nandan
  2021-04-14 16:39 ` [v1 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight Rajeev Nandan
@ 2021-04-14 16:39 ` Rajeev Nandan
  2021-04-16 23:02 ` [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Doug Anderson
  2021-04-20  8:14 ` Jani Nikula
  4 siblings, 0 replies; 7+ messages in thread
From: Rajeev Nandan @ 2021-04-14 16:39 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm, freedreno, devicetree
  Cc: Rajeev Nandan, linux-kernel, robdclark, dianders, mkrishn,
	kalyan_t, hoegsberg, abhinavk, seanpaul

Add support to control the backlight of the eDP panel connected to
the ti-sn65dsi86 bridge through aux channel.

Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
---
 drivers/gpu/drm/bridge/Kconfig        |  1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index e4110d6..e556ec22 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -239,6 +239,7 @@ config DRM_TI_SN65DSI86
 	select REGMAP_I2C
 	select DRM_PANEL
 	select DRM_MIPI_DSI
+	select DRM_DP_AUX_BACKLIGHT
 	help
 	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
 
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f27306c..813d067 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -22,6 +22,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_dp_aux_backlight.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
@@ -126,6 +127,7 @@
  * @panel:        Our panel.
  * @enable_gpio:  The GPIO we toggle to enable the bridge.
  * @supplies:     Data for bulk enabling/disabling our regulators.
+ * @backlight:    The DP aux backlight device.
  * @dp_lanes:     Count of dp_lanes we're using.
  * @ln_assign:    Value to program to the LN_ASSIGN register.
  * @ln_polrs:     Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
@@ -154,6 +156,7 @@ struct ti_sn_bridge {
 	struct drm_panel		*panel;
 	struct gpio_desc		*enable_gpio;
 	struct regulator_bulk_data	supplies[SN_REGULATOR_SUPPLY_NUM];
+	struct drm_dp_aux_backlight	*backlight;
 	int				dp_lanes;
 	u8				ln_assign;
 	u8				ln_polrs;
@@ -431,6 +434,8 @@ static void ti_sn_bridge_disable(struct drm_bridge *bridge)
 {
 	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
 
+	drm_dp_aux_backlight_disable(pdata->backlight);
+
 	drm_panel_disable(pdata->panel);
 
 	/* disable video stream */
@@ -819,6 +824,8 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 			   VSTREAM_ENABLE);
 
 	drm_panel_enable(pdata->panel);
+
+	drm_dp_aux_backlight_enable(pdata->backlight);
 }
 
 static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
@@ -1215,6 +1222,7 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
 			      const struct i2c_device_id *id)
 {
 	struct ti_sn_bridge *pdata;
+	struct drm_dp_aux_backlight *aux_bl;
 	int ret;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
@@ -1294,9 +1302,27 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
 
 	drm_bridge_add(&pdata->bridge);
 
+	if (of_find_property(pdata->dev->of_node, "use-aux-backlight", NULL)) {
+		aux_bl = devm_kzalloc(pdata->dev, sizeof(*aux_bl), GFP_KERNEL);
+		if (!aux_bl) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		aux_bl->dev = pdata->dev;
+		aux_bl->aux = &pdata->aux;
+		ret = drm_dp_aux_backlight_register("ti-sn-aux-backlight", aux_bl);
+		if (ret)
+			goto out;
+		pdata->backlight = aux_bl;
+	}
+
 	ti_sn_debugfs_init(pdata);
 
 	return 0;
+
+out:
+	pm_runtime_disable(pdata->dev);
+	return ret;
 }
 
 static int ti_sn_bridge_remove(struct i2c_client *client)
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [v1 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight
  2021-04-14 16:39 ` [v1 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight Rajeev Nandan
@ 2021-04-15 22:04   ` Rob Herring
  0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2021-04-15 22:04 UTC (permalink / raw)
  To: Rajeev Nandan
  Cc: dri-devel, linux-arm-msm, freedreno, devicetree, linux-kernel,
	robdclark, dianders, mkrishn, kalyan_t, hoegsberg, abhinavk,
	seanpaul

On Wed, Apr 14, 2021 at 10:09:49PM +0530, Rajeev Nandan wrote:
> If the panel connected to the bridge supports backlight control
> using DPCD registers on the DisplayPort aux channel, setting
> "use-aux-backlight" property in the bridge node will enable the
> registration of a DP aux backlight device from the bridge driver.
> 
> Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
> ---
>  .../devicetree/bindings/display/bridge/ti,sn65dsi86.yaml          | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> index 26932d2..c8d8c00 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> @@ -58,6 +58,12 @@ properties:
>    clock-names:
>      const: refclk
>  
> +  use-aux-backlight:

use-dp-aux-backlight perhaps.

> +    type: boolean
> +    description:
> +      The panel backlight to be controlled using DPCD registers on
> +      the DP aux channel.

Sounds like a property of the panel, not the bridge. So it should be in 
the panel node.

Rob

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge
  2021-04-14 16:39 [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Rajeev Nandan
                   ` (2 preceding siblings ...)
  2021-04-14 16:39 ` [v1 3/3] drm/bridge: ti-sn65dsi86: Add DisplayPort aux backlight support Rajeev Nandan
@ 2021-04-16 23:02 ` Doug Anderson
  2021-04-20  8:14 ` Jani Nikula
  4 siblings, 0 replies; 7+ messages in thread
From: Doug Anderson @ 2021-04-16 23:02 UTC (permalink / raw)
  To: Rajeev Nandan
  Cc: dri-devel, linux-arm-msm, freedreno,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Rob Clark, mkrishn, Kalyan Thota, Kristian H. Kristensen,
	Abhinav Kumar, Sean Paul, Laurent Pinchart, Andrzej Hajda

Hi,

On Wed, Apr 14, 2021 at 9:41 AM Rajeev Nandan <rajeevny@codeaurora.org> wrote:
>
> The backlight level of an eDP panel can be controlled through the AUX
> channel using DPCD registers of the panel.
>
> The capability for the Source device to adjust backlight characteristics
> within the panel, using the Sink device DPCD registers is indicated by
> the TCON_BACKLIGHT_ADJUSTMENT_CAPABLE bit in the EDP_GENERAL_CAPABILITY_1
> register (DPCD Address 701h, bit0). In this configuration, the eDP TCON
> receives the backlight level information from the host, through the AUX
> channel.
>
> The changes in this patch series do the following:
> - Add drm_dp_aux_backlight_ APIs to support backlight control using DPCD
>   registers on the DisplayPort AUX channel.
>   The current version only supports backlight brightness control by the
>   EDP_BACKLIGHT_BRIGHTNESS_MSB/LSB registers (DPCD Addresses 722h-723h).
> - Add support for backlight control of the eDP panel connected to the
>   ti-sn65dsi86 bridge.
>
> Rajeev Nandan (3):
>   drm/dp: Add DisplayPort aux backlight control support
>   dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight
>   drm/bridge: ti-sn65dsi86: Add DisplayPort aux backlight support
>
>  .../bindings/display/bridge/ti,sn65dsi86.yaml      |   8 +
>  drivers/gpu/drm/Kconfig                            |   8 +
>  drivers/gpu/drm/Makefile                           |   1 +
>  drivers/gpu/drm/bridge/Kconfig                     |   1 +
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c              |  26 +++
>  drivers/gpu/drm/drm_dp_aux_backlight.c             | 191 +++++++++++++++++++++
>  include/drm/drm_dp_aux_backlight.h                 |  29 ++++
>  7 files changed, 264 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_dp_aux_backlight.c
>  create mode 100644 include/drm/drm_dp_aux_backlight.h

So I haven't looked in massive detail at this patch series, but the
fact that it's touching "ti-sn65dsi86.c" is a red flag. I know in
out-of-band communications you said you weren't sure how to do better.
...but, perhaps, if folks don't hate my recent series [1] there may be
a way forward.

I wonder if perhaps now that the AUX channel can be registered early
if it gets around the circular dependency problems and now you can put
your code in some combination of the panel code and (maybe?) a new
backlight driver if it's generic enough.

It's possible that you might need to add some code to be able to look
up a "struct drm_dp_aux *" from a device tree node and you might need
to add a new device tree property like "ddc-aux-bus" in order to do
this, but I don't _think_ that would be controversial?

[1] https://lore.kernel.org/r/20210416223950.3586967-1-dianders@chromium.org

-Doug

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge
  2021-04-14 16:39 [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Rajeev Nandan
                   ` (3 preceding siblings ...)
  2021-04-16 23:02 ` [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Doug Anderson
@ 2021-04-20  8:14 ` Jani Nikula
  4 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2021-04-20  8:14 UTC (permalink / raw)
  To: Rajeev Nandan, dri-devel, linux-arm-msm, freedreno, devicetree
  Cc: mkrishn, Rajeev Nandan, linux-kernel, abhinavk, dianders,
	seanpaul, kalyan_t, hoegsberg, Lyude Paul, Lankhorst, Maarten,
	Maxime Ripard, Thomas Zimmermann


Cc: Lyude and drm-misc maintainers

On Wed, 14 Apr 2021, Rajeev Nandan <rajeevny@codeaurora.org> wrote:
> The backlight level of an eDP panel can be controlled through the AUX
> channel using DPCD registers of the panel.
>
> The capability for the Source device to adjust backlight characteristics
> within the panel, using the Sink device DPCD registers is indicated by
> the TCON_BACKLIGHT_ADJUSTMENT_CAPABLE bit in the EDP_GENERAL_CAPABILITY_1
> register (DPCD Address 701h, bit0). In this configuration, the eDP TCON
> receives the backlight level information from the host, through the AUX
> channel.

i915 has had this capability for some years now, and work is in progress
to extract the DP AUX backlight code to drm core as helpers [1]. There's
much more to it than what's proposed here. Adding incompatible DP AUX
code at this point would be a pretty bad outcome.

For example, we can't tie backlight device register to DP AUX backlight,
because there are modes where *both* the eDP PWM pin based backlight
control and DP AUX backlight control are used *simultaneously*. The
backlight device register needs to be in code that is aware of both.

Granted, it was a mistake way back when to add this in i915 only, and it
should've been lifted to drm much earlier. It would've been done by
Lyude by now, but people were not happy about not using drm device based
logging. And that has unfortunately lead to a pretty massive prep series
[2].

Please look into the code added to drm helpers in [1], and see how that
would work for you.


BR,
Jani.


[1] http://lore.kernel.org/r/20210205234515.1216538-1-lyude@redhat.com
[2] http://lore.kernel.org/r/20210419225523.184856-1-lyude@redhat.com


>
> The changes in this patch series do the following:
> - Add drm_dp_aux_backlight_ APIs to support backlight control using DPCD
>   registers on the DisplayPort AUX channel.
>   The current version only supports backlight brightness control by the
>   EDP_BACKLIGHT_BRIGHTNESS_MSB/LSB registers (DPCD Addresses 722h-723h).
> - Add support for backlight control of the eDP panel connected to the
>   ti-sn65dsi86 bridge.
>
> Rajeev Nandan (3):
>   drm/dp: Add DisplayPort aux backlight control support
>   dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight
>   drm/bridge: ti-sn65dsi86: Add DisplayPort aux backlight support
>
>  .../bindings/display/bridge/ti,sn65dsi86.yaml      |   8 +
>  drivers/gpu/drm/Kconfig                            |   8 +
>  drivers/gpu/drm/Makefile                           |   1 +
>  drivers/gpu/drm/bridge/Kconfig                     |   1 +
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c              |  26 +++
>  drivers/gpu/drm/drm_dp_aux_backlight.c             | 191 +++++++++++++++++++++
>  include/drm/drm_dp_aux_backlight.h                 |  29 ++++
>  7 files changed, 264 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_dp_aux_backlight.c
>  create mode 100644 include/drm/drm_dp_aux_backlight.h

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-14 16:39 [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Rajeev Nandan
2021-04-14 16:39 ` [v1 1/3] drm/dp: Add DisplayPort aux backlight control support Rajeev Nandan
2021-04-14 16:39 ` [v1 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Document use-aux-backlight Rajeev Nandan
2021-04-15 22:04   ` Rob Herring
2021-04-14 16:39 ` [v1 3/3] drm/bridge: ti-sn65dsi86: Add DisplayPort aux backlight support Rajeev Nandan
2021-04-16 23:02 ` [v1 0/3] drm: Add support for backlight control of eDP panel on ti-sn65dsi86 bridge Doug Anderson
2021-04-20  8:14 ` Jani Nikula

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git