Re: [Freedreno] [RFC 2/6] drm/msm/dpu: support setting up two independent DSI connectors
On 2021-07-02 10:10, Dmitry Baryshkov wrote:
On 02/07/2021 18:52, abhin...@codeaurora.org wrote:
On 2021-07-02 02:20, Dmitry Baryshkov wrote:
On 02/07/2021 00:12, abhin...@codeaurora.org wrote:
On 2021-06-09 14:17, Dmitry Baryshkov wrote:
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "dual DSI" but also
"two
independent DSI" configurations. In future this would also help
adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
I will have to see Bjorn's changes to check why it was dependent on
this cleanup.
Is the plan to call _dpu_kms_initialize_displayport() twice?
Yes. He needs to initialize several displayport interfaces. With the
current code he has to map ids in the set_encoder_mode, using encoder
ids (to fill up the info.h_tile_instance, which is hardcoded to 0 for
DP in the current code).
But still I am not able to put together where is the dependency on
that series
with this one. Can you please elaborate on that a little bit?
It is possible to support independent outputs with the current code.
I
did that for DSI, Bjorn did for DP. However it results in quite an
ugly code to map received encoder in set_encoder_mode back to the DSI
(DP) instances to fill the h_tiles. If we drop the whole
set_encoder_mode story and call dpu_encoder_setup right from the
_dpu_kms_initialize_dsi() (or _dpu_kms_initialize_displayport()),
supporting multiple outputs becomes an easy task.
Okay got it, I think it will become more clear once he posts.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 89
-
1 file changed, 44 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..b63e1c948ff2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -471,30 +471,55 @@ static int _dpu_kms_initialize_dsi(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
+ struct msm_display_info info;
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
- /*TODO: Support two independent DSI connectors */
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
- if (IS_ERR(encoder)) {
- DPU_ERROR("encoder init failed for dsi display\n");
- return PTR_ERR(encoder);
- }
-
- priv->encoders[priv->num_encoders++] = encoder;
-
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
if (!priv->dsi[i])
continue;
+ if (!encoder) {
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi
display\n");
+ return PTR_ERR(encoder);
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+ info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i])
?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+ }
+
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc =
%d\n",
i, rc);
break;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = i;
+
+ if (!msm_dsi_is_dual_dsi(priv->dsi[i])) {
I would like to clarify the terminology of dual_dsi in the current
DSI driver before the rest of the reviews.
Today IS_DUAL_DSI() means that two DSIs are driving the same display
and the two DSIs are operating in master-slave mode
and are being driven by the same PLL.
Yes
Usually, dual independent DSI means two DSIs driving two separate
panels using two separate PLLs ( DSI0 with PLL0 and DSI1 with PLL1)
Let's stop calling it 'dual'. I'd suggest to continue using what was
there in the source file: 'two independent DSI'.
I assume thats happening due to the foll logic and both DSI PHYs are
operating in STANDALONE mode:
if (!IS_DUAL_DSI()) {
ret = msm_dsi_host_register(msm_dsi->host, true);
if (ret)
return ret;
msm_dsi_phy_set_usecase(msm_dsi->phy,
MSM_DSI_PHY_STANDALONE);
ret = msm_dsi_host_set_src_pll(msm_dsi->host,
msm_dsi->phy);
Yes. If we have two independent DSI outputs, we'd like them to work
in
STANDALONE mode.
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc)
+ DPU_ERROR("failed to setup DPU encoder %d:
rc:%d\n",
+ encoder->base.id, rc);
+ encoder = NULL;
+ }
+ }
+
+ if (encoder) {
We will hit this case only for split-DSI right? ( that is two DSIs
driving the same panel ).
Yes, only in this case.
Even single
Re: [Freedreno] [RFC 2/6] drm/msm/dpu: support setting up two independent DSI connectors
On 02/07/2021 18:52, abhin...@codeaurora.org wrote:
On 2021-07-02 02:20, Dmitry Baryshkov wrote:
On 02/07/2021 00:12, abhin...@codeaurora.org wrote:
On 2021-06-09 14:17, Dmitry Baryshkov wrote:
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "dual DSI" but also "two
independent DSI" configurations. In future this would also help adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
I will have to see Bjorn's changes to check why it was dependent on
this cleanup.
Is the plan to call _dpu_kms_initialize_displayport() twice?
Yes. He needs to initialize several displayport interfaces. With the
current code he has to map ids in the set_encoder_mode, using encoder
ids (to fill up the info.h_tile_instance, which is hardcoded to 0 for
DP in the current code).
But still I am not able to put together where is the dependency on
that series
with this one. Can you please elaborate on that a little bit?
It is possible to support independent outputs with the current code. I
did that for DSI, Bjorn did for DP. However it results in quite an
ugly code to map received encoder in set_encoder_mode back to the DSI
(DP) instances to fill the h_tiles. If we drop the whole
set_encoder_mode story and call dpu_encoder_setup right from the
_dpu_kms_initialize_dsi() (or _dpu_kms_initialize_displayport()),
supporting multiple outputs becomes an easy task.
Okay got it, I think it will become more clear once he posts.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 89 -
1 file changed, 44 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..b63e1c948ff2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -471,30 +471,55 @@ static int _dpu_kms_initialize_dsi(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
+ struct msm_display_info info;
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
- /*TODO: Support two independent DSI connectors */
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
- if (IS_ERR(encoder)) {
- DPU_ERROR("encoder init failed for dsi display\n");
- return PTR_ERR(encoder);
- }
-
- priv->encoders[priv->num_encoders++] = encoder;
-
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
if (!priv->dsi[i])
continue;
+ if (!encoder) {
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return PTR_ERR(encoder);
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+ info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i]) ?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+ }
+
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
i, rc);
break;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = i;
+
+ if (!msm_dsi_is_dual_dsi(priv->dsi[i])) {
I would like to clarify the terminology of dual_dsi in the current
DSI driver before the rest of the reviews.
Today IS_DUAL_DSI() means that two DSIs are driving the same display
and the two DSIs are operating in master-slave mode
and are being driven by the same PLL.
Yes
Usually, dual independent DSI means two DSIs driving two separate
panels using two separate PLLs ( DSI0 with PLL0 and DSI1 with PLL1)
Let's stop calling it 'dual'. I'd suggest to continue using what was
there in the source file: 'two independent DSI'.
I assume thats happening due to the foll logic and both DSI PHYs are
operating in STANDALONE mode:
if (!IS_DUAL_DSI()) {
ret = msm_dsi_host_register(msm_dsi->host, true);
if (ret)
return ret;
msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE);
ret = msm_dsi_host_set_src_pll(msm_dsi->host, msm_dsi->phy);
Yes. If we have two independent DSI outputs, we'd like them to work in
STANDALONE mode.
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc)
+ DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
+ encoder->base.id, rc);
+ encoder = NULL;
+ }
+ }
+
+ if (encoder) {
We will hit this case only for split-DSI right? ( that is two DSIs
driving the same panel ).
Yes, only in this case.
Even single DSI will be created in the above loop now. So this looks
a bit
Re: [Freedreno] [RFC 2/6] drm/msm/dpu: support setting up two independent DSI connectors
On 2021-07-02 02:20, Dmitry Baryshkov wrote:
On 02/07/2021 00:12, abhin...@codeaurora.org wrote:
On 2021-06-09 14:17, Dmitry Baryshkov wrote:
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "dual DSI" but also
"two
independent DSI" configurations. In future this would also help
adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
I will have to see Bjorn's changes to check why it was dependent on
this cleanup.
Is the plan to call _dpu_kms_initialize_displayport() twice?
Yes. He needs to initialize several displayport interfaces. With the
current code he has to map ids in the set_encoder_mode, using encoder
ids (to fill up the info.h_tile_instance, which is hardcoded to 0 for
DP in the current code).
But still I am not able to put together where is the dependency on
that series
with this one. Can you please elaborate on that a little bit?
It is possible to support independent outputs with the current code. I
did that for DSI, Bjorn did for DP. However it results in quite an
ugly code to map received encoder in set_encoder_mode back to the DSI
(DP) instances to fill the h_tiles. If we drop the whole
set_encoder_mode story and call dpu_encoder_setup right from the
_dpu_kms_initialize_dsi() (or _dpu_kms_initialize_displayport()),
supporting multiple outputs becomes an easy task.
Okay got it, I think it will become more clear once he posts.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 89
-
1 file changed, 44 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..b63e1c948ff2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -471,30 +471,55 @@ static int _dpu_kms_initialize_dsi(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
+ struct msm_display_info info;
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
- /*TODO: Support two independent DSI connectors */
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
- if (IS_ERR(encoder)) {
- DPU_ERROR("encoder init failed for dsi display\n");
- return PTR_ERR(encoder);
- }
-
- priv->encoders[priv->num_encoders++] = encoder;
-
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
if (!priv->dsi[i])
continue;
+ if (!encoder) {
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return PTR_ERR(encoder);
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+ info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i]) ?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+ }
+
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
i, rc);
break;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = i;
+
+ if (!msm_dsi_is_dual_dsi(priv->dsi[i])) {
I would like to clarify the terminology of dual_dsi in the current DSI
driver before the rest of the reviews.
Today IS_DUAL_DSI() means that two DSIs are driving the same display
and the two DSIs are operating in master-slave mode
and are being driven by the same PLL.
Yes
Usually, dual independent DSI means two DSIs driving two separate
panels using two separate PLLs ( DSI0 with PLL0 and DSI1 with PLL1)
Let's stop calling it 'dual'. I'd suggest to continue using what was
there in the source file: 'two independent DSI'.
I assume thats happening due to the foll logic and both DSI PHYs are
operating in STANDALONE mode:
if (!IS_DUAL_DSI()) {
ret = msm_dsi_host_register(msm_dsi->host, true);
if (ret)
return ret;
msm_dsi_phy_set_usecase(msm_dsi->phy,
MSM_DSI_PHY_STANDALONE);
ret = msm_dsi_host_set_src_pll(msm_dsi->host, msm_dsi->phy);
Yes. If we have two independent DSI outputs, we'd like them to work in
STANDALONE mode.
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc)
+ DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
+ encoder->base.id, rc);
+ encoder = NULL;
+ }
+ }
+
+ if (encoder) {
We will hit this case only for split-DSI right? ( that is two DSIs
driving the same panel ).
Yes, only in this case.
Even single DSI will be created in the above loop now. So this looks a
bit confusing at the moment.
What is so confusin
Re: [Freedreno] [RFC 2/6] drm/msm/dpu: support setting up two independent DSI connectors
On 02/07/2021 00:12, abhin...@codeaurora.org wrote:
On 2021-06-09 14:17, Dmitry Baryshkov wrote:
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "dual DSI" but also "two
independent DSI" configurations. In future this would also help adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
I will have to see Bjorn's changes to check why it was dependent on this
cleanup.
Is the plan to call _dpu_kms_initialize_displayport() twice?
Yes. He needs to initialize several displayport interfaces. With the
current code he has to map ids in the set_encoder_mode, using encoder
ids (to fill up the info.h_tile_instance, which is hardcoded to 0 for DP
in the current code).
But still I am not able to put together where is the dependency on that
series
with this one. Can you please elaborate on that a little bit?
It is possible to support independent outputs with the current code. I
did that for DSI, Bjorn did for DP. However it results in quite an ugly
code to map received encoder in set_encoder_mode back to the DSI (DP)
instances to fill the h_tiles. If we drop the whole set_encoder_mode
story and call dpu_encoder_setup right from the
_dpu_kms_initialize_dsi() (or _dpu_kms_initialize_displayport()),
supporting multiple outputs becomes an easy task.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 89 -
1 file changed, 44 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..b63e1c948ff2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -471,30 +471,55 @@ static int _dpu_kms_initialize_dsi(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
+ struct msm_display_info info;
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
- /*TODO: Support two independent DSI connectors */
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
- if (IS_ERR(encoder)) {
- DPU_ERROR("encoder init failed for dsi display\n");
- return PTR_ERR(encoder);
- }
-
- priv->encoders[priv->num_encoders++] = encoder;
-
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
if (!priv->dsi[i])
continue;
+ if (!encoder) {
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return PTR_ERR(encoder);
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+ info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i]) ?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+ }
+
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
i, rc);
break;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = i;
+
+ if (!msm_dsi_is_dual_dsi(priv->dsi[i])) {
I would like to clarify the terminology of dual_dsi in the current DSI
driver before the rest of the reviews.
Today IS_DUAL_DSI() means that two DSIs are driving the same display and
the two DSIs are operating in master-slave mode
and are being driven by the same PLL.
Yes
Usually, dual independent DSI means two DSIs driving two separate panels
using two separate PLLs ( DSI0 with PLL0 and DSI1 with PLL1)
Let's stop calling it 'dual'. I'd suggest to continue using what was
there in the source file: 'two independent DSI'.
I assume thats happening due to the foll logic and both DSI PHYs are
operating in STANDALONE mode:
if (!IS_DUAL_DSI()) {
ret = msm_dsi_host_register(msm_dsi->host, true);
if (ret)
return ret;
msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE);
ret = msm_dsi_host_set_src_pll(msm_dsi->host, msm_dsi->phy);
Yes. If we have two independent DSI outputs, we'd like them to work in
STANDALONE mode.
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc)
+ DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
+ encoder->base.id, rc);
+ encoder = NULL;
+ }
+ }
+
+ if (encoder) {
We will hit this case only for split-DSI right? ( that is two DSIs
driving the same panel ).
Yes, only in this case.
Even single DSI will be created in the above loop now. So this looks a
bit confusing at the moment.
What is so confusing? I can probably add a comment there. If the encoder
drivers single DSI output, we setup it after c
Re: [Freedreno] [RFC 2/6] drm/msm/dpu: support setting up two independent DSI connectors
On 2021-06-09 14:17, Dmitry Baryshkov wrote:
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "dual DSI" but also "two
independent DSI" configurations. In future this would also help adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
I will have to see Bjorn's changes to check why it was dependent on this
cleanup.
Is the plan to call _dpu_kms_initialize_displayport() twice?
But still I am not able to put together where is the dependency on that
series
with this one. Can you please elaborate on that a little bit?
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 89 -
1 file changed, 44 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..b63e1c948ff2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -471,30 +471,55 @@ static int _dpu_kms_initialize_dsi(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
+ struct msm_display_info info;
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
- /*TODO: Support two independent DSI connectors */
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
- if (IS_ERR(encoder)) {
- DPU_ERROR("encoder init failed for dsi display\n");
- return PTR_ERR(encoder);
- }
-
- priv->encoders[priv->num_encoders++] = encoder;
-
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
if (!priv->dsi[i])
continue;
+ if (!encoder) {
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi
display\n");
+ return PTR_ERR(encoder);
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+ info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i]) ?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+ }
+
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
i, rc);
break;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = i;
+
+ if (!msm_dsi_is_dual_dsi(priv->dsi[i])) {
I would like to clarify the terminology of dual_dsi in the current DSI
driver before the rest of the reviews.
Today IS_DUAL_DSI() means that two DSIs are driving the same display and
the two DSIs are operating in master-slave mode
and are being driven by the same PLL.
Usually, dual independent DSI means two DSIs driving two separate panels
using two separate PLLs ( DSI0 with PLL0 and DSI1 with PLL1)
I assume thats happening due to the foll logic and both DSI PHYs are
operating in STANDALONE mode:
if (!IS_DUAL_DSI()) {
ret = msm_dsi_host_register(msm_dsi->host, true);
if (ret)
return ret;
msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE);
ret = msm_dsi_host_set_src_pll(msm_dsi->host, msm_dsi->phy);
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc)
+ DPU_ERROR("failed to setup DPU encoder %d:
rc:%d\n",
+ encoder->base.id, rc);
+ encoder = NULL;
+ }
+ }
+
+ if (encoder) {
We will hit this case only for split-DSI right? ( that is two DSIs
driving the same panel ).
Even single DSI will be created in the above loop now. So this looks a
bit confusing at the moment.
I think we need to be more clear on dual-DSI Vs split-DSI to avoid
confusion in the code about which one means what and the one
which we are currently using. So what about having IS_DUAL_DSI() and
IS_SPLIT_DSI() to distinguish the terminologies and chaging
DSI driver accordingly.
+ 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;
@@ -505,6 +530,7 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encode
