On 2/3/2022 12:26 AM, Dmitry Baryshkov wrote:
Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess,
which intf type we mean, pass INTF_DSI/INTF_DP directly.
Typically, I am only seeing a 1:1 mapping like
DRM_MODE_ENCODER_DSI means DSI
DRM_MODE_ENCODER_VIRTUAL means WB
So I am not seeing any guessing for the encoder.
The only conflict I am seeing is between DP and EDP as both use
DRM_MODE_ENCODER_TMDS and hence this approach will be useful there.
But that has been marked as a "FIXME" below.
I am suggesting an approach to handle that as well below.
Let me know if you agree with that.
While we are at it, fix the DP audio enablement code which was comparing
intf_type, DRM_MODE_ENCODER_TMDS (= 2) with
DRM_MODE_CONNECTOR_DisplayPort (= 10).
Which would never succeed.
This is a surprising catch for me and left me thinking for a while about
how DP audio is working with this bug because that piece of code was
done to program a register which is needed for DP audio.
This bug happened due to difference in the meaning of intf_type between
upstream and downstream.
After checking more, we found that the register in question has been
deprecated on newer chipsets so I have asked Kuogee to selectively
program it. Here is the change for that:
https://patchwork.freedesktop.org/patch/473869/
Fixes: d13e36d7d222 ("drm/msm/dp: add audio support for Display Port on MSM")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 28 +++++++--------------
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +--
3 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1e648db439f9..e8fc029ad607 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -493,7 +493,7 @@ void dpu_encoder_helper_split_config(
hw_mdptop = phys_enc->hw_mdptop;
disp_info = &dpu_enc->disp_info;
- if (disp_info->intf_type != DRM_MODE_ENCODER_DSI)
+ if (disp_info->intf_type != INTF_DSI)
return;
/**
@@ -555,7 +555,7 @@ static struct msm_display_topology dpu_encoder_get_topology(
else
topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
- if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
+ if (dpu_enc->disp_info.intf_type == INTF_DSI) {
if (dpu_kms->catalog->dspp &&
(dpu_kms->catalog->dspp_count >= topology.num_lm))
topology.num_dspp = topology.num_lm;
@@ -1099,7 +1099,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
}
- if (dpu_enc->disp_info.intf_type == DRM_MODE_CONNECTOR_DisplayPort &&
+ if (dpu_enc->disp_info.intf_type == INTF_DP &&
dpu_enc->cur_master->hw_mdptop &&
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select)
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select(
@@ -1107,7 +1107,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
_dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info);
- if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI &&
+ if (dpu_enc->disp_info.intf_type == INTF_DSI &&
!WARN_ON(dpu_enc->num_phys_encs == 0)) {
unsigned bpc = dpu_enc->phys_encs[0]->connector->display_info.bpc;
for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
@@ -1981,7 +1981,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
{
int ret = 0;
int i = 0;
- enum dpu_intf_type intf_type = INTF_NONE;
struct dpu_enc_phys_init_params phys_params;
if (!dpu_enc) {
@@ -1997,15 +1996,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
phys_params.parent_ops = &dpu_encoder_parent_ops;
phys_params.enc_spinlock = &dpu_enc->enc_spinlock;
- switch (disp_info->intf_type) {
- case DRM_MODE_ENCODER_DSI:
- intf_type = INTF_DSI;
- break;
- case DRM_MODE_ENCODER_TMDS:
- intf_type = INTF_DP;
- break;
- }
-
WARN_ON(disp_info->num_of_h_tiles < 1);
DPU_DEBUG("dsi_info->num_of_h_tiles %d\n", disp_info->num_of_h_tiles);
@@ -2037,11 +2027,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
i, controller_id, phys_params.split_role);
phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
- intf_type,
- controller_id);
+ disp_info->intf_type,
+ controller_id);
if (phys_params.intf_idx == INTF_MAX) {
DPU_ERROR_ENC(dpu_enc, "could not get intf: type %d, id %d\n",
- intf_type, controller_id);
+ disp_info->intf_type, controller_id);
ret = -EINVAL;
}
@@ -2124,11 +2114,11 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
timer_setup(&dpu_enc->frame_done_timer,
dpu_encoder_frame_done_timeout, 0);
- if (disp_info->intf_type == DRM_MODE_ENCODER_DSI)
+ if (disp_info->intf_type == INTF_DSI)
timer_setup(&dpu_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
- else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+ else if (disp_info->intf_type == INTF_DP || disp_info->intf_type == INTF_EDP)
dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index ebe3944355bb..3891bcbbe5a4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -36,7 +36,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
/**
* struct msm_display_info - defines display properties
- * @intf_type: DRM_MODE_ENCODER_ type
+ * @intf_type: INTF_ type
* @capabilities: Bitmask of display flags
* @num_of_h_tiles: Number of horizontal tiles in case of split interface
* @h_tile_instance: Controller instance used per tile. Number of elements is
@@ -45,7 +45,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
* used instead of panel TE in cmd mode panels
*/
struct msm_display_info {
- int intf_type;
+ enum dpu_intf_type intf_type;
uint32_t capabilities;
uint32_t num_of_h_tiles;
uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 47fe11a84a77..f4028be9e2e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -564,7 +564,7 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
priv->encoders[priv->num_encoders++] = encoder;
memset(&info, 0, sizeof(info));
- info.intf_type = encoder->encoder_type;
+ info.intf_type = INTF_DSI;
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
@@ -630,7 +630,7 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
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;
You can query the connector type from the DP driver like this:
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 7cc4d21..0fae0fc 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1457,7 +1457,7 @@ void msm_dp_debugfs_init(struct msm_dp
*dp_display, struct drm_minor *minor)
}
int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
- struct drm_encoder *encoder)
+ struct drm_encoder *encoder, int **connector_type)
{
struct msm_drm_private *priv;
int ret;
@@ -1498,6 +1498,8 @@ int msm_dp_modeset_init(struct msm_dp *dp_display,
struct drm_device *dev,
priv->bridges[priv->num_bridges++] = dp_display->bridge;
+ *connector_type = dp_display->connector_type;
+
return 0;
}
Then you can do something like:
if (connector_type == eDP)
info.intf_type = INTF_eDP;
else if (connector_type == DP)
info.intf_type = INTF_DP;
+ info.intf_type = INTF_DP; /* FIXME: support eDP too */
rc = dpu_encoder_setup(dev, encoder, &info);
if (rc) {
DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
