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 | 17 +- 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 | 183 +++++++++++++----- drivers/gpu/drm/msm/msm_drv.h | 33 ++-- 6 files changed, 200 insertions(+), 103 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 --- drivers/gpu/drm/msm/dp/dp_display.c | 78 ++++++++++++++--------------- 1 file changed, 37 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 70b319a8fe83..8696b36d30e4 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 { @@ -122,6 +121,13 @@ static const struct of_device_id dp_dt_match[] = { {} };
+static struct dp_display_private *dev_to_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) { @@ -198,14 +204,16 @@ static int dp_display_bind(struct device *dev, struct device *master, void *data) { int rc = 0; - struct dp_display_private *dp; + struct dp_display_private *dp = dev_to_dp_display_private(dev); struct drm_device *drm; struct msm_drm_private *priv;
drm = dev_get_drvdata(master);
- dp = container_of(g_dp_display, - struct dp_display_private, dp_display); + if (!dp) { + DRM_ERROR("DP driver bind failed. Invalid driver data\n"); + return -EINVAL; + }
dp->dp_display.drm_dev = drm; priv = drm->dev_private; @@ -240,12 +248,14 @@ 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_to_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); + if (!dp) { + DRM_ERROR("Invalid DP driver data\n"); + return; + }
dp_power_client_deinit(dp->power); dp_aux_unregister(dp->aux); @@ -376,17 +386,14 @@ 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; + struct dp_display_private *dp = dev_to_dp_display_private(dev);
- if (!dev) { - DRM_ERROR("invalid dev\n"); - rc = -EINVAL; + if (!dp) { + DRM_ERROR("no driver data found\n"); + rc = -ENODEV; goto end; }
- dp = container_of(g_dp_display, - struct dp_display_private, dp_display); - dp_display_host_init(dp, false);
rc = dp_display_process_hpd_high(dp); @@ -397,17 +404,14 @@ static int dp_display_usbpd_configure_cb(struct device *dev) static int dp_display_usbpd_disconnect_cb(struct device *dev) { int rc = 0; - struct dp_display_private *dp; + struct dp_display_private *dp = dev_to_dp_display_private(dev);
- if (!dev) { - DRM_ERROR("invalid dev\n"); - rc = -EINVAL; + if (!dp) { + DRM_ERROR("no driver data found\n"); + rc = -ENODEV; return rc; }
- dp = container_of(g_dp_display, - struct dp_display_private, dp_display); - dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
return rc; @@ -466,15 +470,15 @@ static int dp_display_usbpd_attention_cb(struct device *dev) { int rc = 0; u32 sink_request; - struct dp_display_private *dp; + struct dp_display_private *dp = dev_to_dp_display_private(dev); + struct dp_usbpd *hpd;
- if (!dev) { - DRM_ERROR("invalid dev\n"); - return -EINVAL; + if (!dp) { + DRM_ERROR("no driver data found\n"); + return -ENODEV; }
- dp = container_of(g_dp_display, - struct dp_display_private, dp_display); + hpd = dp->usbpd;
/* check for any test request issued by sink */ rc = dp_link_process_request(dp->link); @@ -638,7 +642,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, DP_TIMEOUT_5_SECOND);
/* 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); @@ -832,9 +836,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;
if (dp_display->power_on) { DRM_DEBUG_DP("Link already setup, return\n"); @@ -869,9 +871,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; @@ -1229,14 +1229,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) { @@ -1249,10 +1248,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 = platform_get_drvdata(pdev);
dp_display_deinit_sub_modules(dp);
Quoting Bjorn Andersson (2021-07-24 21:24:31)
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
Thanks for removing the global.
drivers/gpu/drm/msm/dp/dp_display.c | 78 ++++++++++++++--------------- 1 file changed, 37 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 70b319a8fe83..8696b36d30e4 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 { @@ -122,6 +121,13 @@ static const struct of_device_id dp_dt_match[] = { {} };
+static struct dp_display_private *dev_to_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) { @@ -198,14 +204,16 @@ static int dp_display_bind(struct device *dev, struct device *master, void *data) { int rc = 0;
struct dp_display_private *dp;
struct dp_display_private *dp = dev_to_dp_display_private(dev); struct drm_device *drm; struct msm_drm_private *priv; drm = dev_get_drvdata(master);
dp = container_of(g_dp_display,struct dp_display_private, dp_display);
if (!dp) {
How can it be NULL? dev_to_dp_display_private() returns container_of() pointer so it doesn't look possible.
DRM_ERROR("DP driver bind failed. Invalid driver data\n");return -EINVAL;} dp->dp_display.drm_dev = drm; priv = drm->dev_private;
On 25/07/2021 07:24, Bjorn Andersson wrote:
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
drivers/gpu/drm/msm/dp/dp_display.c | 78 ++++++++++++++--------------- 1 file changed, 37 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 70b319a8fe83..8696b36d30e4 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 { @@ -122,6 +121,13 @@ static const struct of_device_id dp_dt_match[] = { {} };
+static struct dp_display_private *dev_to_dp_display_private(struct device *dev)
dev_get_dp_display_private() ?
+{
- struct msm_dp *dp = dev_get_drvdata(dev);
- return container_of(dp, struct dp_display_private, dp_display);
+}
As a matter of preference, it might be cleaner to inline dev_get_drvdata and then define msm_dp_get_private to convert from msm_dp to dp_display_private, see below.
static int dp_add_event(struct dp_display_private *dp_priv, u32 event, u32 data, u32 delay) { @@ -198,14 +204,16 @@ static int dp_display_bind(struct device *dev, struct device *master, void *data) { int rc = 0;
- struct dp_display_private *dp;
struct dp_display_private *dp = dev_to_dp_display_private(dev); struct drm_device *drm; struct msm_drm_private *priv;
drm = dev_get_drvdata(master);
- dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
- if (!dp) {
This is not correct, if I'm not mistaken. you wanted to check if dev's private data is NULL (correct check), but ended up checking whether the result of container_of is NULL (incorrect).
DRM_ERROR("DP driver bind failed. Invalid driver data\n");return -EINVAL;}
dp->dp_display.drm_dev = drm; priv = drm->dev_private;
@@ -240,12 +248,14 @@ 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_to_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);
if (!dp) {
DRM_ERROR("Invalid DP driver data\n");return;}
dp_power_client_deinit(dp->power); dp_aux_unregister(dp->aux);
@@ -376,17 +386,14 @@ 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;
- struct dp_display_private *dp = dev_to_dp_display_private(dev);
- if (!dev) {
DRM_ERROR("invalid dev\n");rc = -EINVAL;
- if (!dp) {
DRM_ERROR("no driver data found\n");rc = -ENODEV;
dp = container_of(g_dp_display,
struct dp_display_private, dp_display);dp_display_host_init(dp, false);
rc = dp_display_process_hpd_high(dp);
@@ -397,17 +404,14 @@ static int dp_display_usbpd_configure_cb(struct device *dev) static int dp_display_usbpd_disconnect_cb(struct device *dev) { int rc = 0;
- struct dp_display_private *dp;
- struct dp_display_private *dp = dev_to_dp_display_private(dev);
- if (!dev) {
DRM_ERROR("invalid dev\n");rc = -EINVAL;
`!dev` check should remain in place. And dp should be fetched afterwards. This applies to all the checks in the patch.
- if (!dp) {
DRM_ERROR("no driver data found\n");rc = -ENODEV;
dp = container_of(g_dp_display,
struct dp_display_private, dp_display);dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
return rc;
@@ -466,15 +470,15 @@ static int dp_display_usbpd_attention_cb(struct device *dev) { int rc = 0; u32 sink_request;
- struct dp_display_private *dp;
- struct dp_display_private *dp = dev_to_dp_display_private(dev);
- struct dp_usbpd *hpd;
- if (!dev) {
DRM_ERROR("invalid dev\n");return -EINVAL;
- if (!dp) {
DRM_ERROR("no driver data found\n");return -ENODEV;
- dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
hpd = dp->usbpd;
/* check for any test request issued by sink */ rc = dp_link_process_request(dp->link);
@@ -638,7 +642,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, DP_TIMEOUT_5_SECOND);
/* 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);
@@ -832,9 +836,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;
if (dp_display->power_on) { DRM_DEBUG_DP("Link already setup, return\n");
@@ -869,9 +871,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;
@@ -1229,14 +1229,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) {
@@ -1249,10 +1248,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 = platform_get_drvdata(pdev);
dev_to_dp_display_private() rather than just get_drvdata?
dp_display_deinit_sub_modules(dp);
On 2021-07-24 21:24, Bjorn Andersson wrote:
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
drivers/gpu/drm/msm/dp/dp_display.c | 78 ++++++++++++++--------------- 1 file changed, 37 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 70b319a8fe83..8696b36d30e4 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 { @@ -122,6 +121,13 @@ static const struct of_device_id dp_dt_match[] = { {} };
+static struct dp_display_private *dev_to_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) { @@ -198,14 +204,16 @@ static int dp_display_bind(struct device *dev, struct device *master, void *data) { int rc = 0;
- struct dp_display_private *dp;
struct dp_display_private *dp = dev_to_dp_display_private(dev); struct drm_device *drm; struct msm_drm_private *priv;
drm = dev_get_drvdata(master);
- dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
if (!dp) {
DRM_ERROR("DP driver bind failed. Invalid driver data\n");return -EINVAL;}
dp->dp_display.drm_dev = drm; priv = drm->dev_private;
@@ -240,12 +248,14 @@ 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_to_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);
if (!dp) {
DRM_ERROR("Invalid DP driver data\n");return;}
dp_power_client_deinit(dp->power); dp_aux_unregister(dp->aux);
@@ -376,17 +386,14 @@ 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;
- struct dp_display_private *dp = dev_to_dp_display_private(dev);
- if (!dev) {
DRM_ERROR("invalid dev\n");rc = -EINVAL;
- if (!dp) {
DRM_ERROR("no driver data found\n");rc = -ENODEV;
dp = container_of(g_dp_display,
struct dp_display_private, dp_display);dp_display_host_init(dp, false);
rc = dp_display_process_hpd_high(dp);
@@ -397,17 +404,14 @@ static int dp_display_usbpd_configure_cb(struct device *dev) static int dp_display_usbpd_disconnect_cb(struct device *dev) { int rc = 0;
- struct dp_display_private *dp;
- struct dp_display_private *dp = dev_to_dp_display_private(dev);
- if (!dev) {
DRM_ERROR("invalid dev\n");rc = -EINVAL;
- if (!dp) {
DRM_ERROR("no driver data found\n");rc = -ENODEV;
dp = container_of(g_dp_display,
struct dp_display_private, dp_display);dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
return rc;
@@ -466,15 +470,15 @@ static int dp_display_usbpd_attention_cb(struct device *dev) { int rc = 0; u32 sink_request;
- struct dp_display_private *dp;
- struct dp_display_private *dp = dev_to_dp_display_private(dev);
- struct dp_usbpd *hpd;
- if (!dev) {
DRM_ERROR("invalid dev\n");return -EINVAL;
- if (!dp) {
DRM_ERROR("no driver data found\n");return -ENODEV;
- dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
- hpd = dp->usbpd;
hpd is unused here. It was removed with https://patches.linaro.org/patch/416670/
/* check for any test request issued by sink */ rc = dp_link_process_request(dp->link); @@ -638,7 +642,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, DP_TIMEOUT_5_SECOND);
/* 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); @@ -832,9 +836,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;
if (dp_display->power_on) { DRM_DEBUG_DP("Link already setup, return\n");
@@ -869,9 +871,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;
@@ -1229,14 +1229,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) {
@@ -1249,10 +1248,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 = platform_get_drvdata(pdev);
dp_display_deinit_sub_modules(dp);
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.
The information for doing this lookup is available inside the DP driver, so update the API to take the struct msm_drm_private and the struct drm_encoder and have the DP code figure out which struct msm_dp the operation relates to.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 17 +++++---- drivers/gpu/drm/msm/dp/dp_display.c | 38 +++++++++++++++++---- drivers/gpu/drm/msm/msm_drv.h | 31 +++++++++-------- 3 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1c04b7cce43e..0d64ef0819af 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1002,8 +1002,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(priv, drm_enc, mode, adj_mode);
list_for_each_entry(conn_iter, connector_list, head) if (conn_iter->encoder == drm_enc) @@ -1184,9 +1184,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(priv, drm_enc); if (ret) { DPU_ERROR_ENC(dpu_enc, "dp display enable failed: %d\n", ret); @@ -1226,8 +1225,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(priv, drm_enc)) DPU_ERROR_ENC(dpu_enc, "dp display push idle failed\n"); }
@@ -1255,8 +1254,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(priv, drm_enc)) DPU_ERROR_ENC(dpu_enc, "dp display disable failed\n"); }
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 8696b36d30e4..59ffd6c8f41f 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1432,12 +1432,25 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, return 0; }
-int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) +static struct msm_dp *msm_dp_from_drm_encoder(struct msm_drm_private *priv, + struct drm_encoder *encoder) +{ + if (priv->dp && priv->dp->encoder == encoder) + return priv->dp; + + return NULL; +} + +int msm_dp_display_enable(struct msm_drm_private *priv, struct drm_encoder *encoder) { int rc = 0; struct dp_display_private *dp_display; + struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder); u32 state;
+ if (!dp) + return -EINVAL; + dp_display = container_of(dp, struct dp_display_private, dp_display); if (!dp_display->dp_mode.drm_mode.clock) { DRM_ERROR("invalid params\n"); @@ -1489,9 +1502,13 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) return rc; }
-int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder) +int msm_dp_display_pre_disable(struct msm_drm_private *priv, struct drm_encoder *encoder) { struct dp_display_private *dp_display; + struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder); + + if (!dp) + return 0;
dp_display = container_of(dp, struct dp_display_private, dp_display);
@@ -1500,11 +1517,15 @@ int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder) return 0; }
-int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder) +int msm_dp_display_disable(struct msm_drm_private *priv, struct drm_encoder *encoder) { int rc = 0; u32 state; struct dp_display_private *dp_display; + struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder); + + if (!dp) + return 0;
dp_display = container_of(dp, struct dp_display_private, dp_display);
@@ -1531,11 +1552,16 @@ int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder) return rc; }
-void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) +void msm_dp_display_mode_set(struct msm_drm_private *priv, + struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { struct dp_display_private *dp_display; + struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder); + + if (!dp) + return;
dp_display = container_of(dp, struct dp_display_private, dp_display);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 9bfd37855969..e9232032b266 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -388,12 +388,13 @@ int __init msm_dp_register(void); void __exit msm_dp_unregister(void); int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, struct drm_encoder *encoder); -int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder); -int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder); -int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder); -void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); +int msm_dp_display_enable(struct msm_drm_private *priv, struct drm_encoder *encoder); +int msm_dp_display_disable(struct msm_drm_private *priv, struct drm_encoder *encoder); +int msm_dp_display_pre_disable(struct msm_drm_private *priv, struct drm_encoder *encoder); +void msm_dp_display_mode_set(struct msm_drm_private *priv, + struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode); void msm_dp_irq_postinstall(struct msm_dp *dp_display); void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp_display);
@@ -413,25 +414,25 @@ static inline int msm_dp_modeset_init(struct msm_dp *dp_display, { return -EINVAL; } -static inline int msm_dp_display_enable(struct msm_dp *dp, +static inline int msm_dp_display_enable(struct msm_drm_private *priv, struct drm_encoder *encoder) { return -EINVAL; } -static inline int msm_dp_display_disable(struct msm_dp *dp, - struct drm_encoder *encoder) +static inline int msm_dp_display_disable(struct msm_drm_private *priv, + struct drm_encoder *encoder) { return -EINVAL; } -static inline int msm_dp_display_pre_disable(struct msm_dp *dp, - struct drm_encoder *encoder) +static inline int msm_dp_display_pre_disable(struct msm_drm_private *priv, + struct drm_encoder *encoder) { return -EINVAL; } -static inline void msm_dp_display_mode_set(struct msm_dp *dp, - struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) +static inline void msm_dp_display_mode_set(struct msm_drm_private *priv, + struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { }
Quoting Bjorn Andersson (2021-07-24 21:24:32)
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.
The information for doing this lookup is available inside the DP driver, so update the API to take the struct msm_drm_private and the struct drm_encoder and have the DP code figure out which struct msm_dp the operation relates to.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org
Reviewed-by: Stephen Boyd swboyd@chromium.org
On 25/07/2021 07:24, Bjorn Andersson wrote:
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.
The information for doing this lookup is available inside the DP driver, so update the API to take the struct msm_drm_private and the struct drm_encoder and have the DP code figure out which struct msm_dp the operation relates to.
Initially I thought to propose moving encoder->dp lookup into dpu code by adding msm_dp_display_get_encoder() function. However as I was writing that, I remembered that at some point I had to refactor my own patchset in the way to get rid of calling msm_FOO_get_encoder().
I'd propose simpler solution. In dpu_encoder_setup() you have the DP index and the encoder. So you can store valid msm_dp pointer in the dpu_encoder_virt and remove all the lookups. Then you can replace all priv->dp with bare dpu_enc->dp accesses. Will this work for you?
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 17 +++++---- drivers/gpu/drm/msm/dp/dp_display.c | 38 +++++++++++++++++---- drivers/gpu/drm/msm/msm_drv.h | 31 +++++++++-------- 3 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1c04b7cce43e..0d64ef0819af 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1002,8 +1002,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(priv, drm_enc, mode, adj_mode);list_for_each_entry(conn_iter, connector_list, head) if (conn_iter->encoder == drm_enc)
@@ -1184,9 +1184,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(priv, drm_enc);@@ -1226,8 +1225,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(priv, drm_enc)) DPU_ERROR_ENC(dpu_enc, "dp display push idle failed\n");@@ -1255,8 +1254,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(priv, drm_enc)) DPU_ERROR_ENC(dpu_enc, "dp display disable failed\n");diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 8696b36d30e4..59ffd6c8f41f 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1432,12 +1432,25 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, return 0; }
-int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) +static struct msm_dp *msm_dp_from_drm_encoder(struct msm_drm_private *priv,
struct drm_encoder *encoder)+{
- if (priv->dp && priv->dp->encoder == encoder)
return priv->dp;- return NULL;
+}
+int msm_dp_display_enable(struct msm_drm_private *priv, struct drm_encoder *encoder) { int rc = 0; struct dp_display_private *dp_display;
struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder); u32 state;
if (!dp)
return -EINVAL;dp_display = container_of(dp, struct dp_display_private, dp_display); if (!dp_display->dp_mode.drm_mode.clock) { DRM_ERROR("invalid params\n");
@@ -1489,9 +1502,13 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) return rc; }
-int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder) +int msm_dp_display_pre_disable(struct msm_drm_private *priv, struct drm_encoder *encoder) { struct dp_display_private *dp_display;
struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder);
if (!dp)
return 0;dp_display = container_of(dp, struct dp_display_private, dp_display);
@@ -1500,11 +1517,15 @@ int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder) return 0; }
-int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder) +int msm_dp_display_disable(struct msm_drm_private *priv, struct drm_encoder *encoder) { int rc = 0; u32 state; struct dp_display_private *dp_display;
struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder);
if (!dp)
return 0;dp_display = container_of(dp, struct dp_display_private, dp_display);
@@ -1531,11 +1552,16 @@ int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder) return rc; }
-void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode)+void msm_dp_display_mode_set(struct msm_drm_private *priv,
struct drm_encoder *encoder,struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode){ struct dp_display_private *dp_display;
struct msm_dp *dp = msm_dp_from_drm_encoder(priv, encoder);
if (!dp)
return;dp_display = container_of(dp, struct dp_display_private, dp_display);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 9bfd37855969..e9232032b266 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -388,12 +388,13 @@ int __init msm_dp_register(void); void __exit msm_dp_unregister(void); int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, struct drm_encoder *encoder); -int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder); -int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder); -int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder); -void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode);+int msm_dp_display_enable(struct msm_drm_private *priv, struct drm_encoder *encoder); +int msm_dp_display_disable(struct msm_drm_private *priv, struct drm_encoder *encoder); +int msm_dp_display_pre_disable(struct msm_drm_private *priv, struct drm_encoder *encoder); +void msm_dp_display_mode_set(struct msm_drm_private *priv,
struct drm_encoder *encoder,struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode);@@ -413,25 +414,25 @@ static inline int msm_dp_modeset_init(struct msm_dp *dp_display, { return -EINVAL; } -static inline int msm_dp_display_enable(struct msm_dp *dp, +static inline int msm_dp_display_enable(struct msm_drm_private *priv, struct drm_encoder *encoder) { return -EINVAL; } -static inline int msm_dp_display_disable(struct msm_dp *dp,
struct drm_encoder *encoder)+static inline int msm_dp_display_disable(struct msm_drm_private *priv,
struct drm_encoder *encoder)-static inline int msm_dp_display_pre_disable(struct msm_dp *dp,
struct drm_encoder *encoder)+static inline int msm_dp_display_pre_disable(struct msm_drm_private *priv,
struct drm_encoder *encoder)-static inline void msm_dp_display_mode_set(struct msm_dp *dp,
struct drm_encoder *encoder,struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode)+static inline void msm_dp_display_mode_set(struct msm_drm_private *priv,
struct drm_encoder *encoder,struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode)
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 finding a struct msm_dp from a struct drm_encoder.
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 --- 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 | 59 ++++++++++++++++-- drivers/gpu/drm/msm/msm_drv.h | 2 +- 4 files changed, 95 insertions(+), 34 deletions(-)
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 59ffd6c8f41f..92b7646a1bb7 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -79,6 +79,8 @@ struct dp_display_private { char *name; int irq;
+ int id; + /* state variables */ bool core_initialized; bool hpd_irq_on; @@ -116,8 +118,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 }, {} };
@@ -217,7 +230,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) { @@ -238,8 +251,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; @@ -259,7 +275,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 = { @@ -1205,6 +1221,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; @@ -1219,6 +1255,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";
@@ -1386,6 +1426,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;
@@ -1435,8 +1478,12 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, static struct msm_dp *msm_dp_from_drm_encoder(struct msm_drm_private *priv, struct drm_encoder *encoder) { - if (priv->dp && priv->dp->encoder == encoder) - return priv->dp; + int i; + + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) { + if (priv->dp[i] && priv->dp[i]->encoder == encoder) + return priv->dp[i]; + }
return NULL; } diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index e9232032b266..62d54ef6c2c4 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-07-24 21:24:33)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 59ffd6c8f41f..92b7646a1bb7 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -238,8 +251,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;
This hunk looks useless. Drop it?
@@ -1205,6 +1221,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; @@ -1219,6 +1255,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";diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index e9232032b266..62d54ef6c2c4 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];
It would be nice to either make this dynamically sized (probably little gain), somehow make a BUILD_BUG_ON(), or have a WARN_ON if ARRAY_SIZE(dp) is less than a num_dp so we know somebody messed up.
The sc8180x has 2 DP and 1 eDP controllers, add support for these to the DP driver.
Link: https://lore.kernel.org/linux-arm-msm/20210511042043.592802-5-bjorn.andersso... Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org --- drivers/gpu/drm/msm/dp/dp_display.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 92b7646a1bb7..c26805cfcdd1 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -129,8 +129,20 @@ 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, 0 }, + .num_dp = 3, +}; + +static const struct msm_dp_config sc8180x_edp_cfg = { + .io_start = { 0, 0, 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_edp_cfg }, {} };
The Qualcomm SC8180x has 2 DP controllers and 1 eDP controller, add compatibles for these to the msm/dp binding.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org --- .../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 a6e41be038fc..c6056e0b0845 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:
Quoting Bjorn Andersson (2021-07-24 21:24:35)
The Qualcomm SC8180x has 2 DP controllers and 1 eDP controller, add compatibles for these to the msm/dp binding.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org
Reviewed-by: Stephen Boyd swboyd@chromium.org
The sc8180x has 2 DP and 1 eDP controllers, add support for these to the DP driver.
Link: https://lore.kernel.org/linux-arm-msm/20210511042043.592802-5-bjorn.andersso... Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org --- drivers/gpu/drm/msm/dp/dp_display.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 92b7646a1bb7..c26805cfcdd1 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -129,8 +129,20 @@ 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, 0 }, + .num_dp = 3, +}; + +static const struct msm_dp_config sc8180x_edp_cfg = { + .io_start = { 0, 0, 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_edp_cfg }, {} };
Quoting Bjorn Andersson (2021-07-24 21:24:36)
The sc8180x has 2 DP and 1 eDP controllers, add support for these to the DP driver.
Link: https://lore.kernel.org/linux-arm-msm/20210511042043.592802-5-bjorn.andersso... Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org
drivers/gpu/drm/msm/dp/dp_display.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 92b7646a1bb7..c26805cfcdd1 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -129,8 +129,20 @@ 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, 0 },.num_dp = 3,+};
+static const struct msm_dp_config sc8180x_edp_cfg = {
.io_start = { 0, 0, 0xae9a000 },.num_dp = 3,+};
Can the two structs not be combined into one struct and set as .data for either compatible?
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_edp_cfg }, {}
Hi Bjorn
On 2021-07-24 21:24, Bjorn Andersson wrote:
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.
Thanks for posting the changes.
I dont have major concerns on the series as such apart from minor comments which i will post in a day or two but I will check and get back if this has been validated on sc7280 without any concerns.
One question i had is not directly related to this series but related to multi-DP in general. Does audio work fine across both the DPs when both are connected?
The reason I ask this question is that, I dont know how two hdmi-codec instances are handled today. So we will register twice with hdmi-codec so there should be two audio streams.
But I am not sure if this works correctly in todays design with hdmi-codec.
Any chance you had validated this?
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 | 17 +- 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 | 183 +++++++++++++----- drivers/gpu/drm/msm/msm_drv.h | 33 ++-- 6 files changed, 200 insertions(+), 103 deletions(-)
dri-devel@lists.freedesktop.org
-
abhinavk＠codeaurora.org -
Bjorn Andersson -
Dmitry Baryshkov -
Stephen Boyd