Re: [RFC] drm/msm/dp: Allow attaching a drm_panel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/07/2021 02:13, Bjorn Andersson wrote:
eDP panels might need some power sequencing and backlight management,
so make it possible to associate a drm_panel with a DP instance and
prepare and enable the panel accordingly.

Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>

The idea looks good from my point of view. For v1 could you please extend it with the `if (panel)` checks and handling of the error codes.

---

This solves my immediate problem on my 8cx laptops, of indirectly controlling
the backlight during DPMS. But my panel is powered when I boot it and as such I
get the hpd interrupt and I don't actually have to deal with a power on
sequence - so I'm posting this as an RFC, hoping to get some input on these
other aspects.

If this is acceptable I'd be happy to write up an accompanying DT binding
change that marks port 2 of the DP controller's of_graph as a reference to the
attached panel.

  drivers/gpu/drm/msm/dp/dp_display.c | 15 +++++++++++++--
  drivers/gpu/drm/msm/dp/dp_display.h |  1 +
  drivers/gpu/drm/msm/dp/dp_parser.c  | 19 +++++++++++++++++++
  drivers/gpu/drm/msm/dp/dp_parser.h  |  1 +
  4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 206bf7806f51..1db5a3f752d2 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -10,6 +10,7 @@
  #include <linux/component.h>
  #include <linux/of_irq.h>
  #include <linux/delay.h>
+#include <drm/drm_panel.h>
#include "msm_drv.h"
  #include "msm_kms.h"
@@ -252,6 +253,8 @@ static int dp_display_bind(struct device *dev, struct device *master,
  		goto end;
  	}
+ dp->dp_display.drm_panel = dp->parser->drm_panel;
+
  	rc = dp_aux_register(dp->aux, drm);
  	if (rc) {
  		DRM_ERROR("DRM DP AUX register failed\n");
@@ -867,8 +870,10 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
  	return 0;
  }
-static int dp_display_prepare(struct msm_dp *dp)
+static int dp_display_prepare(struct msm_dp *dp_display)
  {
+	drm_panel_prepare(dp_display->drm_panel);
+
  	return 0;
  }
@@ -886,6 +891,8 @@ static int dp_display_enable(struct dp_display_private *dp, u32 data)
  	if (!rc)
  		dp_display->power_on = true;
+ drm_panel_enable(dp_display->drm_panel);
+
  	return rc;
  }
@@ -915,6 +922,8 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data)
  	if (!dp_display->power_on)
  		return 0;
+ drm_panel_disable(dp_display->drm_panel);
+
  	/* wait only if audio was enabled */
  	if (dp_display->audio_enabled) {
  		/* signal the disconnect event */
@@ -939,8 +948,10 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data)
  	return 0;
  }
-static int dp_display_unprepare(struct msm_dp *dp)
+static int dp_display_unprepare(struct msm_dp *dp_display)
  {
+	drm_panel_unprepare(dp_display->drm_panel);
+
  	return 0;
  }
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 8b47cdabb67e..ce337824c95d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -15,6 +15,7 @@ struct msm_dp {
  	struct device *codec_dev;
  	struct drm_connector *connector;
  	struct drm_encoder *encoder;
+	struct drm_panel *drm_panel;
  	bool is_connected;
  	bool audio_enabled;
  	bool power_on;
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index fc8a6452f641..e6a6e9007bfd 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -6,6 +6,7 @@
  #include <linux/of_gpio.h>
  #include <linux/phy/phy.h>
+#include <drm/drm_of.h>
  #include <drm/drm_print.h>
#include "dp_parser.h"
@@ -276,6 +277,20 @@ static int dp_parser_clock(struct dp_parser *parser)
  	return 0;
  }
+static int dp_parser_find_panel(struct dp_parser *parser)
+{
+	struct device_node *np = parser->pdev->dev.of_node;
+	int rc;
+
+	rc = drm_of_find_panel_or_bridge(np, 2, 0, &parser->drm_panel, NULL);
+	if (rc == -ENODEV)
+		rc = 0;
+	else if (rc)
+		DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
+
+	return rc;
+}
+
  static int dp_parser_parse(struct dp_parser *parser)
  {
  	int rc = 0;
@@ -297,6 +312,10 @@ static int dp_parser_parse(struct dp_parser *parser)
  	if (rc)
  		return rc;
+ rc = dp_parser_find_panel(parser);
+	if (rc)
+		return rc;
+
  	/* Map the corresponding regulator information according to
  	 * version. Currently, since we only have one supported platform,
  	 * mapping the regulator directly.
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3266b529c090..994ca9336acd 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -122,6 +122,7 @@ struct dp_parser {
  	struct dp_display_data disp_data;
  	const struct dp_regulator_cfg *regulator_cfg;
  	u32 max_dp_lanes;
+	struct drm_panel *drm_panel;
int (*parse)(struct dp_parser *parser);
  };



--
With best wishes
Dmitry



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux