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.
This is based on v4 of Dmitry's work on multiple DSI interfaces: https://lore.kernel.org/linux-arm-msm/20210717124016.316020-1-dmitry.baryshk...
With that in place add SC8180x DP and eDP controllers.
Bjorn Andersson (5): drm/msm/dp: Remove global g_dp_display variable drm/msm/dp: Modify prototype of encoder based API 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 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 60 ++++---- .../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 8 +- drivers/gpu/drm/msm/dp/dp_display.c | 138 ++++++++++-------- drivers/gpu/drm/msm/msm_drv.h | 2 +- 6 files changed, 132 insertions(+), 101 deletions(-)
As the Qualcomm DisplayPort driver only supports a single instance of the driver the commonly used struct dp_display is kept in a global variable. As we introduce additional instances this obviously doesn't work.
Replace this with a combination of existing references to adjacent objects and drvdata.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org ---
Changes since v1: - Renamed dev_get_dp_display_private() - Drop incorrect checks for !container_of() - Dropped the checks for !NULL checks of dev in the USB api - Use right accessor in dp_display_remove()
drivers/gpu/drm/msm/dp/dp_display.c | 80 ++++++++--------------------- 1 file changed, 21 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 13a3cf692562..2c7de43f655a 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -27,7 +27,6 @@ #include "dp_audio.h" #include "dp_debug.h"
-static struct msm_dp *g_dp_display; #define HPD_STRING_SIZE 30
enum { @@ -121,6 +120,13 @@ static const struct of_device_id dp_dt_match[] = { {} };
+static struct dp_display_private *dev_get_dp_display_private(struct device *dev) +{ + struct msm_dp *dp = dev_get_drvdata(dev); + + return container_of(dp, struct dp_display_private, dp_display); +} + static int dp_add_event(struct dp_display_private *dp_priv, u32 event, u32 data, u32 delay) { @@ -197,15 +203,12 @@ static int dp_display_bind(struct device *dev, struct device *master, void *data) { int rc = 0; - struct dp_display_private *dp; - struct drm_device *drm; + struct dp_display_private *dp = dev_get_dp_display_private(dev); struct msm_drm_private *priv; + struct drm_device *drm;
drm = dev_get_drvdata(master);
- dp = container_of(g_dp_display, - struct dp_display_private, dp_display); - dp->dp_display.drm_dev = drm; priv = drm->dev_private; priv->dp = &(dp->dp_display); @@ -240,13 +243,10 @@ static int dp_display_bind(struct device *dev, struct device *master, static void dp_display_unbind(struct device *dev, struct device *master, void *data) { - struct dp_display_private *dp; + struct dp_display_private *dp = dev_get_dp_display_private(dev); struct drm_device *drm = dev_get_drvdata(master); struct msm_drm_private *priv = drm->dev_private;
- dp = container_of(g_dp_display, - struct dp_display_private, dp_display); - dp_power_client_deinit(dp->power); dp_aux_unregister(dp->aux); priv->dp = NULL; @@ -379,38 +379,17 @@ static void dp_display_host_deinit(struct dp_display_private *dp)
static int dp_display_usbpd_configure_cb(struct device *dev) { - int rc = 0; - struct dp_display_private *dp; - - if (!dev) { - DRM_ERROR("invalid dev\n"); - rc = -EINVAL; - goto end; - } - - dp = container_of(g_dp_display, - struct dp_display_private, dp_display); + struct dp_display_private *dp = dev_get_dp_display_private(dev);
dp_display_host_init(dp, false);
- rc = dp_display_process_hpd_high(dp); -end: - return rc; + return dp_display_process_hpd_high(dp); }
static int dp_display_usbpd_disconnect_cb(struct device *dev) { int rc = 0; - struct dp_display_private *dp; - - if (!dev) { - DRM_ERROR("invalid dev\n"); - rc = -EINVAL; - return rc; - } - - dp = container_of(g_dp_display, - struct dp_display_private, dp_display); + struct dp_display_private *dp = dev_get_dp_display_private(dev);
dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
@@ -472,15 +451,7 @@ static int dp_display_usbpd_attention_cb(struct device *dev) { int rc = 0; u32 sink_request; - struct dp_display_private *dp; - - if (!dev) { - DRM_ERROR("invalid dev\n"); - return -EINVAL; - } - - dp = container_of(g_dp_display, - struct dp_display_private, dp_display); + struct dp_display_private *dp = dev_get_dp_display_private(dev);
/* check for any test request issued by sink */ rc = dp_link_process_request(dp->link); @@ -647,7 +618,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
DRM_DEBUG_DP("hpd_state=%d\n", state); /* signal the disconnect event early to ensure proper teardown */ - dp_display_handle_plugged_change(g_dp_display, false); + dp_display_handle_plugged_change(&dp->dp_display, false);
/* enable HDP plug interrupt to prepare for next plugin */ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true); @@ -842,9 +813,7 @@ static int dp_display_prepare(struct msm_dp *dp) static int dp_display_enable(struct dp_display_private *dp, u32 data) { int rc = 0; - struct msm_dp *dp_display; - - dp_display = g_dp_display; + struct msm_dp *dp_display = &dp->dp_display;
DRM_DEBUG_DP("sink_count=%d\n", dp->link->sink_count); if (dp_display->power_on) { @@ -880,9 +849,7 @@ static int dp_display_post_enable(struct msm_dp *dp_display)
static int dp_display_disable(struct dp_display_private *dp, u32 data) { - struct msm_dp *dp_display; - - dp_display = g_dp_display; + struct msm_dp *dp_display = &dp->dp_display;
if (!dp_display->power_on) return 0; @@ -1237,14 +1204,13 @@ static int dp_display_probe(struct platform_device *pdev) }
mutex_init(&dp->event_mutex); - g_dp_display = &dp->dp_display;
/* Store DP audio handle inside DP display */ - g_dp_display->dp_audio = dp->audio; + dp->dp_display.dp_audio = dp->audio;
init_completion(&dp->audio_comp);
- platform_set_drvdata(pdev, g_dp_display); + platform_set_drvdata(pdev, &dp->dp_display);
rc = component_add(&pdev->dev, &dp_display_comp_ops); if (rc) { @@ -1257,10 +1223,7 @@ static int dp_display_probe(struct platform_device *pdev)
static int dp_display_remove(struct platform_device *pdev) { - struct dp_display_private *dp; - - dp = container_of(g_dp_display, - struct dp_display_private, dp_display); + struct dp_display_private *dp = dev_get_dp_display_private(&pdev->dev);
dp_display_deinit_sub_modules(dp);
@@ -1315,8 +1278,7 @@ static int dp_pm_resume(struct device *dev) else dp->dp_display.is_connected = false;
- dp_display_handle_plugged_change(g_dp_display, - dp->dp_display.is_connected); + dp_display_handle_plugged_change(dp_display, dp->dp_display.is_connected);
DRM_DEBUG_DP("After, sink_count=%d is_connected=%d core_inited=%d power_on=%d\n",
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.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org ---
Changes since v1: - Store msm_dp reference in dpu_encoder_virt instead of carrying a list of them and searching repeatedly
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);
Based on the removal of the g_dp_display and the movement of the priv->dp lookup into the DP code it's now possible to have multiple DP instances.
In line with the other controllers in the MSM driver, introduce a per-compatible list of base addresses which is used to resolve the "instance id" for the given DP controller. This instance id is used as index in the priv->dp[] array.
Then extend the initialization code to initialize struct drm_encoder for each of the registered priv->dp[] and update the logic for associating each struct msm_dp with the struct dpu_encoder_virt.
Lastly, bump the number of struct msm_dp instances carries by priv->dp to 3, the currently known maximum number of controllers found in a Qualcomm SoC.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org ---
Changes since v1: - Update dpu_encoder_setup() to store the reference to the msm_dp in our dpu_enc
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 60 +++++++++++-------- .../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 8 ++- drivers/gpu/drm/msm/dp/dp_display.c | 51 ++++++++++++++-- drivers/gpu/drm/msm/msm_drv.h | 2 +- 5 files changed, 90 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index b7f33da2799c..9cd9539a1504 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2173,7 +2173,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, dpu_encoder_vsync_event_handler, 0); else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) - dpu_enc->dp = priv->dp; + dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work, dpu_encoder_off_work); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index f655adbc2421..a793cc8a007e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -188,6 +188,7 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor) struct dentry *entry; struct drm_device *dev; struct msm_drm_private *priv; + int i;
if (!p) return -EINVAL; @@ -203,8 +204,8 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor) dpu_debugfs_vbif_init(dpu_kms, entry); dpu_debugfs_core_irq_init(dpu_kms, entry);
- if (priv->dp) - msm_dp_debugfs_init(priv->dp, minor); + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) + msm_dp_debugfs_init(priv->dp[i], minor);
return dpu_core_perf_debugfs_init(dpu_kms, entry); } @@ -545,33 +546,40 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev, struct drm_encoder *encoder = NULL; struct msm_display_info info; int rc = 0; + int i;
- if (!priv->dp) - return rc; + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) { + if (!priv->dp[i]) + continue;
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS); - if (IS_ERR(encoder)) { - DPU_ERROR("encoder init failed for dsi display\n"); - return PTR_ERR(encoder); - } + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS); + if (IS_ERR(encoder)) { + DPU_ERROR("encoder init failed for dsi display\n"); + return PTR_ERR(encoder); + }
- memset(&info, 0, sizeof(info)); - rc = msm_dp_modeset_init(priv->dp, dev, encoder); - if (rc) { - DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc); - drm_encoder_cleanup(encoder); - return rc; - } + memset(&info, 0, sizeof(info)); + rc = msm_dp_modeset_init(priv->dp[i], dev, encoder); + if (rc) { + DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc); + drm_encoder_cleanup(encoder); + return rc; + }
- priv->encoders[priv->num_encoders++] = encoder; + priv->encoders[priv->num_encoders++] = encoder; + + info.num_of_h_tiles = 1; + info.h_tile_instance[0] = i; + info.capabilities = MSM_DISPLAY_CAP_VID_MODE; + info.intf_type = encoder->encoder_type; + rc = dpu_encoder_setup(dev, encoder, &info); + if (rc) { + DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n", + encoder->base.id, rc); + return rc; + } + }
- info.num_of_h_tiles = 1; - info.capabilities = MSM_DISPLAY_CAP_VID_MODE; - info.intf_type = encoder->encoder_type; - rc = dpu_encoder_setup(dev, encoder, &info); - if (rc) - DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n", - encoder->base.id, rc); return rc; }
@@ -792,6 +800,7 @@ static int dpu_irq_postinstall(struct msm_kms *kms) { struct msm_drm_private *priv; struct dpu_kms *dpu_kms = to_dpu_kms(kms); + int i;
if (!dpu_kms || !dpu_kms->dev) return -EINVAL; @@ -800,7 +809,8 @@ static int dpu_irq_postinstall(struct msm_kms *kms) if (!priv) return -EINVAL;
- msm_dp_irq_postinstall(priv->dp); + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) + msm_dp_irq_postinstall(priv->dp[i]);
return 0; } diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c index cabe15190ec1..2e1acb1bc390 100644 --- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c +++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c @@ -126,8 +126,12 @@ void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state) priv = drm_dev->dev_private; kms = priv->kms;
- if (priv->dp) - msm_dp_snapshot(disp_state, priv->dp); + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) { + if (!priv->dp[i]) + continue; + + msm_dp_snapshot(disp_state, priv->dp[i]); + }
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { if (!priv->dsi[i]) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 2c7de43f655a..4a6132c18e57 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -78,6 +78,8 @@ struct dp_display_private { char *name; int irq;
+ int id; + /* state variables */ bool core_initialized; bool hpd_irq_on; @@ -115,8 +117,19 @@ struct dp_display_private { struct dp_audio *audio; };
+ +struct msm_dp_config { + phys_addr_t io_start[3]; + size_t num_dp; +}; + +static const struct msm_dp_config sc7180_dp_cfg = { + .io_start = { 0x0ae90000 }, + .num_dp = 1, +}; + static const struct of_device_id dp_dt_match[] = { - {.compatible = "qcom,sc7180-dp"}, + { .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_cfg }, {} };
@@ -211,7 +224,7 @@ static int dp_display_bind(struct device *dev, struct device *master,
dp->dp_display.drm_dev = drm; priv = drm->dev_private; - priv->dp = &(dp->dp_display); + priv->dp[dp->id] = &(dp->dp_display);
rc = dp->parser->parse(dp->parser); if (rc) { @@ -233,8 +246,11 @@ static int dp_display_bind(struct device *dev, struct device *master, }
rc = dp_register_audio_driver(dev, dp->audio); - if (rc) + if (rc) { DRM_ERROR("Audio registration Dp failed\n"); + goto end; + } +
end: return rc; @@ -249,7 +265,7 @@ static void dp_display_unbind(struct device *dev, struct device *master,
dp_power_client_deinit(dp->power); dp_aux_unregister(dp->aux); - priv->dp = NULL; + priv->dp[dp->id] = NULL; }
static const struct component_ops dp_display_comp_ops = { @@ -1180,6 +1196,26 @@ int dp_display_request_irq(struct msm_dp *dp_display) return 0; }
+static int dp_display_get_id(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 -EINVAL; + + for (i = 0; i < cfg->num_dp; i++) { + if (cfg->io_start[i] == res->start) + return i; + } + + dev_err(&pdev->dev, "unknown displayport instance\n"); + return -EINVAL; +} + static int dp_display_probe(struct platform_device *pdev) { int rc = 0; @@ -1194,6 +1230,10 @@ static int dp_display_probe(struct platform_device *pdev) if (!dp) return -ENOMEM;
+ dp->id = dp_display_get_id(pdev); + if (dp->id < 0) + return -EINVAL; + dp->pdev = pdev; dp->name = "drm_dp";
@@ -1388,6 +1428,9 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor) struct device *dev; int rc;
+ if (!dp_display) + return; + dp = container_of(dp_display, struct dp_display_private, dp_display); dev = &dp->pdev->dev;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 8b005d1ac899..2e84dc30e12e 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -161,7 +161,7 @@ struct msm_drm_private { /* DSI is shared by mdp4 and mdp5 */ struct msm_dsi *dsi[2];
- struct msm_dp *dp; + struct msm_dp *dp[3];
/* when we have more than one 'msm_gpu' these need to be an array: */ struct msm_gpu *gpu;
Quoting Bjorn Andersson (2021-08-25 16:42:31)
Does this need the same if (!priv->dp) continue check like the other loops over priv->dp?
Like this one.
This one too? Or maybe those gained NULL pointer checks.
unsigned int?
Nitpick: Drop double newline.
Nitpick: drop braces
Ah ok, it's signed for this error check. Maybe assign dp->id in the function and return 0 instead of assigning it here? dp_display_assign_id()
Alright that's one.
On Thu 26 Aug 00:13 PDT 2021, Stephen Boyd wrote:
Quoting Bjorn Andersson (2021-08-25 16:42:31)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
[..]
[..]
This already has a NULL check, that's why I added one to the adjacent msm_dp_debugfs_init() as well.
[..]
I like the fact that the "getter" doesn't have side effects, but making dp->id unsigned makes sense. So let's pay the price of a local signed variable here.
Thanks for the feedback, Bjorn
Quoting Bjorn Andersson (2021-08-26 09:57:18)
Ok.
Sure. If that's the only change then feel free to add
Reviewed-by: Stephen Boyd swboyd@chromium.org
on the next version.
Quoting Bjorn Andersson (2021-08-25 16:42:31)
Can this be made into another struct, like msm_dp_desc, that also indicates what type of DP connector it is, i.e. eDP vs DP? That would help me understand in modetest and /sys/class/drm what sort of connector is probing. dp_drm_connector_init() would need to pass the type of connector appropriately. Right now, eDP connectors still show up as DP instead of eDP in sysfs.
Hi,
On Thu, Aug 26, 2021 at 10:20 PM Stephen Boyd swboyd@chromium.org wrote:
I'm not convinced that's the right way to do it. I think the right way forward here is to look at whether drm_of_find_panel_or_bridge() finds a panel. If it finds a panel then we're eDP. If it doesn't then we're DP. That matches roughly what Laurent was planning to do for ti-sn65dsi86:
https://lore.kernel.org/all/20210322030128.2283-11-laurent.pinchart+renesas@...
I know technically an eDP and DP controller can have different sets of features but I think what we really are trying to communicate to modetest is whether an internal panel or external display is connected, right?
-Doug
On Fri 01 Oct 06:58 PDT 2021, Doug Anderson wrote:
When we spoke about this earlier I got the impression that there was interest in representing the DP display as a panel at some point in the future. Did I misunderstand you or would we simply update the scheme when that day comes?
I updated the patch based on Stephen's input and it looks nice, but I could certainly respin it again to simply rely on us having an explicit panel or not.
For me Stephen's suggestion resulted in the monitor names in Wayland suddenly making more sense, i.e. it makes more sense to say that the lid on my laptop should control eDP-1, rather than DP-3 on this machine...
Regards, Bjorn
The Qualcomm SC8180x has 2 DP controllers and 1 eDP controller, add compatibles for these to the msm/dp binding.
Reviewed-by: Stephen Boyd swboyd@chromium.org Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org ---
Changes since v1: - Picked up Stephen's R-b
.../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 f915dc080cbc..b36d74c1da7c 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:
The sc8180x has 2 DP and 1 eDP controllers, add support for these to the DP driver.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org ---
Changes since v1: - Squashed DP and eDP data, as there's no reason to keep them separate today
drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 4a6132c18e57..c1307230dc47 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -128,8 +128,15 @@ static const struct msm_dp_config sc7180_dp_cfg = { .num_dp = 1, };
+static const struct msm_dp_config sc8180x_dp_cfg = { + .io_start = { 0xae90000, 0xae98000, 0xae9a000 }, + .num_dp = 3, +}; + static const struct of_device_id dp_dt_match[] = { { .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_cfg }, + { .compatible = "qcom,sc8180x-dp", .data = &sc8180x_dp_cfg }, + { .compatible = "qcom,sc8180x-edp", .data = &sc8180x_dp_cfg }, {} };
dri-devel@lists.freedesktop.org