2021-10-18 03:39:13

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v5 0/5] drm/msm/dp: Support multiple DP instances and add sc8180x

The current implementation supports a single DP instance and the DPU code will
only match it against INTF_DP instance 0. These patches extends this to allow
multiple DP instances and support for matching against DP instances beyond 0.

With that in place add SC8180x DP and eDP controllers.

Bjorn Andersson (7):
drm/msm/dp: Remove global g_dp_display variable
drm/msm/dp: Modify prototype of encoder based API
drm/msm/dp: Allow specifying connector_type per controller
drm/msm/dp: Allow attaching a drm_panel
drm/msm/dp: Support up to 3 DP controllers
dt-bindings: msm/dp: Add SC8180x compatibles
drm/msm/dp: Add sc8180x DP controllers

.../bindings/display/msm/dp-controller.yaml | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 +--
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 8 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 ++++----
.../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 8 +-
drivers/gpu/drm/msm/dp/dp_display.c | 153 ++++++++++--------
drivers/gpu/drm/msm/dp/dp_display.h | 2 +
drivers/gpu/drm/msm/dp/dp_drm.c | 13 +-
drivers/gpu/drm/msm/dp/dp_parser.c | 30 +++-
drivers/gpu/drm/msm/dp/dp_parser.h | 3 +-
drivers/gpu/drm/msm/msm_drv.h | 9 +-
11 files changed, 205 insertions(+), 112 deletions(-)

--
2.29.2


2021-10-18 03:40:08

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v5 4/7] drm/msm/dp: Allow attaching a drm_panel

eDP panels might need some power sequencing and backlight management,
so make it possible to associate a drm_panel with an eDP instance and
prepare and enable the panel accordingly.

Now that we know which hardware instance is DP and which is eDP,
parser->parse() is passed the connector_type and the parser is limited
to only search for a panel in the eDP case.

Signed-off-by: Bjorn Andersson <[email protected]>
---

Changes since v4:
- None

drivers/gpu/drm/msm/dp/dp_display.c | 9 ++++++---
drivers/gpu/drm/msm/dp/dp_display.h | 1 +
drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++++++++++
drivers/gpu/drm/msm/dp/dp_parser.c | 30 ++++++++++++++++++++++++++++-
drivers/gpu/drm/msm/dp/dp_parser.h | 3 ++-
5 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 6913970c8cf9..c663cd619925 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"
@@ -230,12 +231,14 @@ static int dp_display_bind(struct device *dev, struct device *master,
priv = drm->dev_private;
priv->dp = &(dp->dp_display);

- rc = dp->parser->parse(dp->parser);
+ rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
if (rc) {
DRM_ERROR("device tree parsing failed\n");
goto end;
}

+ dp->dp_display.panel_bridge = dp->parser->panel_bridge;
+
dp->aux->drm_dev = drm;
rc = dp_aux_register(dp->aux);
if (rc) {
@@ -822,7 +825,7 @@ 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)
{
return 0;
}
@@ -896,7 +899,7 @@ 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)
{
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 75fcabcfbbdd..8e80e3bac394 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_bridge *panel_bridge;
bool is_connected;
bool audio_enabled;
bool power_on;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index f33e31523f56..76856c4ee1d6 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -5,6 +5,7 @@

#include <drm/drm_atomic_helper.h>
#include <drm/drm_atomic.h>
+#include <drm/drm_bridge.h>
#include <drm/drm_crtc.h>

#include "msm_drv.h"
@@ -160,5 +161,15 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)

drm_connector_attach_encoder(connector, dp_display->encoder);

+ if (dp_display->panel_bridge) {
+ ret = drm_bridge_attach(dp_display->encoder,
+ dp_display->panel_bridge, NULL,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+ if (ret < 0) {
+ DRM_ERROR("failed to attach panel bridge: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+ }
+
return connector;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index 4d6e047f803d..eb6bbfbea484 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"
@@ -263,7 +264,28 @@ static int dp_parser_clock(struct dp_parser *parser)
return 0;
}

-static int dp_parser_parse(struct dp_parser *parser)
+static int dp_parser_find_panel(struct dp_parser *parser)
+{
+ struct device *dev = &parser->pdev->dev;
+ struct drm_panel *panel;
+ int rc;
+
+ rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
+ if (rc) {
+ DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
+ return rc;
+ }
+
+ parser->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+ if (IS_ERR(parser->panel_bridge)) {
+ DRM_ERROR("failed to create panel bridge\n");
+ return PTR_ERR(parser->panel_bridge);
+ }
+
+ return 0;
+}
+
+static int dp_parser_parse(struct dp_parser *parser, int connector_type)
{
int rc = 0;

@@ -284,6 +306,12 @@ static int dp_parser_parse(struct dp_parser *parser)
if (rc)
return rc;

+ if (connector_type == DRM_MODE_CONNECTOR_eDP) {
+ 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 dac10923abde..3172da089421 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -123,8 +123,9 @@ struct dp_parser {
struct dp_display_data disp_data;
const struct dp_regulator_cfg *regulator_cfg;
u32 max_dp_lanes;
+ struct drm_bridge *panel_bridge;

- int (*parse)(struct dp_parser *parser);
+ int (*parse)(struct dp_parser *parser, int connector_type);
};

/**
--
2.29.2

2021-10-18 03:40:14

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v5 6/7] dt-bindings: msm/dp: Add SC8180x compatibles

The Qualcomm SC8180x has 2 DP controllers and 1 eDP controller, add
compatibles for these to the msm/dp binding.

Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---

Changes since v4:
- None

.../devicetree/bindings/display/msm/dp-controller.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index 6bb424c21340..63e585f48789 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -17,6 +17,8 @@ properties:
compatible:
enum:
- qcom,sc7180-dp
+ - qcom,sc8180x-dp
+ - qcom,sc8180x-edp

reg:
items:
--
2.29.2

2021-10-18 03:40:14

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v5 3/7] drm/msm/dp: Allow specifying connector_type per controller

As the following patches introduced support for multiple DP blocks in a
platform and some of those block might be eDP it becomes useful to be
able to specify the connector type per block.

Although there's only a single block at this point, the array of descs
and the search in dp_display_get_desc() are introduced here to simplify
the next patch, that does introduce support for multiple DP blocks.

Signed-off-by: Bjorn Andersson <[email protected]>
---

Changes since v4:
- const the various struct msm_dp_desc instances
- unsigned the connector_type


The references to DRM_MODE_CONNECTOR_DisplayPort in dp_debug.c, that was
highligted in the review of v4 has been removed in a separate patch.

drivers/gpu/drm/msm/dp/dp_display.c | 43 ++++++++++++++++++++++++++++-
drivers/gpu/drm/msm/dp/dp_display.h | 1 +
drivers/gpu/drm/msm/dp/dp_drm.c | 2 +-
3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 5d3ee5ef07c2..6913970c8cf9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -115,8 +115,25 @@ struct dp_display_private {
struct dp_audio *audio;
};

+struct msm_dp_desc {
+ phys_addr_t io_start;
+ unsigned int connector_type;
+};
+
+struct msm_dp_config {
+ const struct msm_dp_desc *descs;
+ size_t num_descs;
+};
+
+static const struct msm_dp_config sc7180_dp_cfg = {
+ .descs = (const struct msm_dp_desc[]) {
+ { .io_start = 0x0ae90000, .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
+ },
+ .num_descs = 1,
+};
+
static const struct of_device_id dp_dt_match[] = {
- {.compatible = "qcom,sc7180-dp"},
+ { .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_cfg },
{}
};

@@ -1180,10 +1197,29 @@ int dp_display_request_irq(struct msm_dp *dp_display)
return 0;
}

+static const struct msm_dp_desc *dp_display_get_desc(struct platform_device *pdev)
+{
+ const struct msm_dp_config *cfg = of_device_get_match_data(&pdev->dev);
+ struct resource *res;
+ int i;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return NULL;
+
+ for (i = 0; i < cfg->num_descs; i++)
+ if (cfg->descs[i].io_start == res->start)
+ return &cfg->descs[i];
+
+ dev_err(&pdev->dev, "unknown displayport instance\n");
+ return NULL;
+}
+
static int dp_display_probe(struct platform_device *pdev)
{
int rc = 0;
struct dp_display_private *dp;
+ const struct msm_dp_desc *desc;

if (!pdev || !pdev->dev.of_node) {
DRM_ERROR("pdev not found\n");
@@ -1194,8 +1230,13 @@ static int dp_display_probe(struct platform_device *pdev)
if (!dp)
return -ENOMEM;

+ desc = dp_display_get_desc(pdev);
+ if (!desc)
+ return -EINVAL;
+
dp->pdev = pdev;
dp->name = "drm_dp";
+ dp->dp_display.connector_type = desc->connector_type;

rc = dp_init_sub_modules(dp);
if (rc) {
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 8b47cdabb67e..75fcabcfbbdd 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -18,6 +18,7 @@ struct msm_dp {
bool is_connected;
bool audio_enabled;
bool power_on;
+ unsigned int connector_type;

hdmi_codec_plugged_cb plugged_cb;

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 764f4b81017e..f33e31523f56 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -147,7 +147,7 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)

ret = drm_connector_init(dp_display->drm_dev, connector,
&dp_connector_funcs,
- DRM_MODE_CONNECTOR_DisplayPort);
+ dp_display->connector_type);
if (ret)
return ERR_PTR(ret);

--
2.29.2

2021-10-18 03:40:42

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v5 3/7] drm/msm/dp: Allow specifying connector_type per controller

Quoting Bjorn Andersson (2021-10-16 15:18:39)
> As the following patches introduced support for multiple DP blocks in a
> platform and some of those block might be eDP it becomes useful to be
> able to specify the connector type per block.
>
> Although there's only a single block at this point, the array of descs
> and the search in dp_display_get_desc() are introduced here to simplify
> the next patch, that does introduce support for multiple DP blocks.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2021-10-18 03:40:43

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] drm/msm/dp: Allow attaching a drm_panel

Quoting Bjorn Andersson (2021-10-16 15:18:40)
> eDP panels might need some power sequencing and backlight management,
> so make it possible to associate a drm_panel with an eDP instance and
> prepare and enable the panel accordingly.
>
> Now that we know which hardware instance is DP and which is eDP,
> parser->parse() is passed the connector_type and the parser is limited
> to only search for a panel in the eDP case.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2021-10-18 03:41:44

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v5 2/7] drm/msm/dp: Modify prototype of encoder based API

Functions in the DisplayPort code that relates to individual instances
(encoders) are passed both the struct msm_dp and the struct drm_encoder.
But in a situation where multiple DP instances would exist this means
that the caller need to resolve which struct msm_dp relates to the
struct drm_encoder at hand.

Store a reference to the struct msm_dp associated with each
dpu_encoder_virt to allow the particular instance to be associate with
the encoder in the following patch.

Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---

Changes since v4:
- None

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 ++++++++++++---------
1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0e9d3fa1544b..b7f33da2799c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -168,6 +168,7 @@ enum dpu_enc_rc_states {
* @vsync_event_work: worker to handle vsync event for autorefresh
* @topology: topology of the display
* @idle_timeout: idle timeout duration in milliseconds
+ * @dp: msm_dp pointer, for DP encoders
*/
struct dpu_encoder_virt {
struct drm_encoder base;
@@ -206,6 +207,8 @@ struct dpu_encoder_virt {
struct msm_display_topology topology;

u32 idle_timeout;
+
+ struct msm_dp *dp;
};

#define to_dpu_encoder_virt(x) container_of(x, struct dpu_encoder_virt, base)
@@ -1000,8 +1003,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,

trace_dpu_enc_mode_set(DRMID(drm_enc));

- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp)
- msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode);
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS)
+ msm_dp_display_mode_set(dpu_enc->dp, drm_enc, mode, adj_mode);

list_for_each_entry(conn_iter, connector_list, head)
if (conn_iter->encoder == drm_enc)
@@ -1182,9 +1185,8 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)

_dpu_encoder_virt_enable_helper(drm_enc);

- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp) {
- ret = msm_dp_display_enable(priv->dp,
- drm_enc);
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) {
+ ret = msm_dp_display_enable(dpu_enc->dp, drm_enc);
if (ret) {
DPU_ERROR_ENC(dpu_enc, "dp display enable failed: %d\n",
ret);
@@ -1224,8 +1226,8 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
/* wait for idle */
dpu_encoder_wait_for_event(drm_enc, MSM_ENC_TX_COMPLETE);

- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp) {
- if (msm_dp_display_pre_disable(priv->dp, drm_enc))
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) {
+ if (msm_dp_display_pre_disable(dpu_enc->dp, drm_enc))
DPU_ERROR_ENC(dpu_enc, "dp display push idle failed\n");
}

@@ -1253,8 +1255,8 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)

DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");

- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp) {
- if (msm_dp_display_disable(priv->dp, drm_enc))
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) {
+ if (msm_dp_display_disable(dpu_enc->dp, drm_enc))
DPU_ERROR_ENC(dpu_enc, "dp display disable failed\n");
}

@@ -2170,7 +2172,8 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
-
+ else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+ dpu_enc->dp = priv->dp;

INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
dpu_encoder_off_work);
--
2.29.2

2021-10-18 23:10:17

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [Freedreno] [PATCH v5 3/7] drm/msm/dp: Allow specifying connector_type per controller

On 2021-10-16 15:18, Bjorn Andersson wrote:
> As the following patches introduced support for multiple DP blocks in a
> platform and some of those block might be eDP it becomes useful to be
> able to specify the connector type per block.
>
> Although there's only a single block at this point, the array of descs
> and the search in dp_display_get_desc() are introduced here to simplify
> the next patch, that does introduce support for multiple DP blocks.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
> ---
>
> Changes since v4:
> - const the various struct msm_dp_desc instances
> - unsigned the connector_type
>
>
> The references to DRM_MODE_CONNECTOR_DisplayPort in dp_debug.c, that
> was
> highligted in the review of v4 has been removed in a separate patch.
>
> drivers/gpu/drm/msm/dp/dp_display.c | 43 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/msm/dp/dp_display.h | 1 +
> drivers/gpu/drm/msm/dp/dp_drm.c | 2 +-
> 3 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 5d3ee5ef07c2..6913970c8cf9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -115,8 +115,25 @@ struct dp_display_private {
> struct dp_audio *audio;
> };
>
> +struct msm_dp_desc {
> + phys_addr_t io_start;
> + unsigned int connector_type;
> +};
> +
> +struct msm_dp_config {
> + const struct msm_dp_desc *descs;
> + size_t num_descs;
> +};
> +
> +static const struct msm_dp_config sc7180_dp_cfg = {
> + .descs = (const struct msm_dp_desc[]) {
> + { .io_start = 0x0ae90000, .connector_type =
> DRM_MODE_CONNECTOR_DisplayPort },
> + },
> + .num_descs = 1,
> +};
> +
> static const struct of_device_id dp_dt_match[] = {
> - {.compatible = "qcom,sc7180-dp"},
> + { .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_cfg },
> {}
> };
>
> @@ -1180,10 +1197,29 @@ int dp_display_request_irq(struct msm_dp
> *dp_display)
> return 0;
> }
>
> +static const struct msm_dp_desc *dp_display_get_desc(struct
> platform_device *pdev)
> +{
> + const struct msm_dp_config *cfg =
> of_device_get_match_data(&pdev->dev);
> + struct resource *res;
> + int i;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return NULL;
> +
> + for (i = 0; i < cfg->num_descs; i++)
> + if (cfg->descs[i].io_start == res->start)
> + return &cfg->descs[i];
> +
> + dev_err(&pdev->dev, "unknown displayport instance\n");
> + return NULL;
> +}
> +
> static int dp_display_probe(struct platform_device *pdev)
> {
> int rc = 0;
> struct dp_display_private *dp;
> + const struct msm_dp_desc *desc;
>
> if (!pdev || !pdev->dev.of_node) {
> DRM_ERROR("pdev not found\n");
> @@ -1194,8 +1230,13 @@ static int dp_display_probe(struct
> platform_device *pdev)
> if (!dp)
> return -ENOMEM;
>
> + desc = dp_display_get_desc(pdev);
> + if (!desc)
> + return -EINVAL;
> +
> dp->pdev = pdev;
> dp->name = "drm_dp";
> + dp->dp_display.connector_type = desc->connector_type;
>
> rc = dp_init_sub_modules(dp);
> if (rc) {
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index 8b47cdabb67e..75fcabcfbbdd 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -18,6 +18,7 @@ struct msm_dp {
> bool is_connected;
> bool audio_enabled;
> bool power_on;
> + unsigned int connector_type;
>
> hdmi_codec_plugged_cb plugged_cb;
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c
> b/drivers/gpu/drm/msm/dp/dp_drm.c
> index 764f4b81017e..f33e31523f56 100644
> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> @@ -147,7 +147,7 @@ struct drm_connector *dp_drm_connector_init(struct
> msm_dp *dp_display)
>
> ret = drm_connector_init(dp_display->drm_dev, connector,
> &dp_connector_funcs,
> - DRM_MODE_CONNECTOR_DisplayPort);
> + dp_display->connector_type);
> if (ret)
> return ERR_PTR(ret);

2021-10-18 23:13:53

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [Freedreno] [PATCH v5 4/7] drm/msm/dp: Allow attaching a drm_panel

On 2021-10-16 15:18, Bjorn Andersson wrote:
> eDP panels might need some power sequencing and backlight management,
> so make it possible to associate a drm_panel with an eDP instance and
> prepare and enable the panel accordingly.
>
> Now that we know which hardware instance is DP and which is eDP,
> parser->parse() is passed the connector_type and the parser is limited
> to only search for a panel in the eDP case.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
> ---
>
> Changes since v4:
> - None
>
> drivers/gpu/drm/msm/dp/dp_display.c | 9 ++++++---
> drivers/gpu/drm/msm/dp/dp_display.h | 1 +
> drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++++++++++
> drivers/gpu/drm/msm/dp/dp_parser.c | 30 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/msm/dp/dp_parser.h | 3 ++-
> 5 files changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 6913970c8cf9..c663cd619925 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"
> @@ -230,12 +231,14 @@ static int dp_display_bind(struct device *dev,
> struct device *master,
> priv = drm->dev_private;
> priv->dp = &(dp->dp_display);
>
> - rc = dp->parser->parse(dp->parser);
> + rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
> if (rc) {
> DRM_ERROR("device tree parsing failed\n");
> goto end;
> }
>
> + dp->dp_display.panel_bridge = dp->parser->panel_bridge;
> +
> dp->aux->drm_dev = drm;
> rc = dp_aux_register(dp->aux);
> if (rc) {
> @@ -822,7 +825,7 @@ 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)
> {
> return 0;
> }
> @@ -896,7 +899,7 @@ 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)
> {
> return 0;
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index 75fcabcfbbdd..8e80e3bac394 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_bridge *panel_bridge;
> bool is_connected;
> bool audio_enabled;
> bool power_on;
> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c
> b/drivers/gpu/drm/msm/dp/dp_drm.c
> index f33e31523f56..76856c4ee1d6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> @@ -5,6 +5,7 @@
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_atomic.h>
> +#include <drm/drm_bridge.h>
> #include <drm/drm_crtc.h>
>
> #include "msm_drv.h"
> @@ -160,5 +161,15 @@ struct drm_connector
> *dp_drm_connector_init(struct msm_dp *dp_display)
>
> drm_connector_attach_encoder(connector, dp_display->encoder);
>
> + if (dp_display->panel_bridge) {
> + ret = drm_bridge_attach(dp_display->encoder,
> + dp_display->panel_bridge, NULL,
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> + if (ret < 0) {
> + DRM_ERROR("failed to attach panel bridge: %d\n", ret);
> + return ERR_PTR(ret);
> + }
> + }
> +
> return connector;
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
> b/drivers/gpu/drm/msm/dp/dp_parser.c
> index 4d6e047f803d..eb6bbfbea484 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"
> @@ -263,7 +264,28 @@ static int dp_parser_clock(struct dp_parser
> *parser)
> return 0;
> }
>
> -static int dp_parser_parse(struct dp_parser *parser)
> +static int dp_parser_find_panel(struct dp_parser *parser)
> +{
> + struct device *dev = &parser->pdev->dev;
> + struct drm_panel *panel;
> + int rc;
> +
> + rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
> + if (rc) {
> + DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
> + return rc;
> + }
> +
> + parser->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> + if (IS_ERR(parser->panel_bridge)) {
> + DRM_ERROR("failed to create panel bridge\n");
> + return PTR_ERR(parser->panel_bridge);
> + }
> +
> + return 0;
> +}
> +
> +static int dp_parser_parse(struct dp_parser *parser, int
> connector_type)
> {
> int rc = 0;
>
> @@ -284,6 +306,12 @@ static int dp_parser_parse(struct dp_parser
> *parser)
> if (rc)
> return rc;
>
> + if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> + 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 dac10923abde..3172da089421 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -123,8 +123,9 @@ struct dp_parser {
> struct dp_display_data disp_data;
> const struct dp_regulator_cfg *regulator_cfg;
> u32 max_dp_lanes;
> + struct drm_bridge *panel_bridge;
>
> - int (*parse)(struct dp_parser *parser);
> + int (*parse)(struct dp_parser *parser, int connector_type);
> };
>
> /**