On Mon, Apr 16, 2018 at 11:22:20AM -0700, Jeykumar Sankaran wrote:
Switch DPU from dsi-staging to upstream dsi driver. To make
the switch atomic, this change includes:
- remove dpu connector layers
- clean up dpu connector dependencies in encoder/crtc
- compile out writeback and display port drivers
- compile out dsi-staging driver (separate patch submitted to
remove the driver)
- adapt upstream device hierarchy
Signed-off-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>
---
.../config/arm64/chromiumos-arm64.flavour.config | 4 +-
.../arm64/chromiumos-qualcomm.flavour.config | 4 +-
These files aren't in upstream.
<snip />
+
+struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
+ int drm_enc_mode)
+{
+ struct dpu_encoder_virt *dpu_enc = NULL;
+
+ dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);
You should probably use devm_kzalloc.
+ if (!dpu_enc)
+ return ERR_PTR(ENOMEM);
+
+ drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
+ drm_enc_mode, NULL);
Check the return value?
<snip />
-#ifdef CONFIG_DRM_MSM_DSI_STAGING
static void _dpu_kms_initialize_dsi(struct drm_device *dev,
struct msm_drm_private *priv,
- struct dpu_kms *dpu_kms,
- unsigned max_encoders)
+ struct dpu_kms *dpu_kms)
{
- static const struct dpu_connector_ops dsi_ops = {
- .post_init = dsi_conn_post_init,
- .detect = dsi_conn_detect,
- .get_modes = dsi_connector_get_modes,
- .put_modes = dsi_connector_put_modes,
- .mode_valid = dsi_conn_mode_valid,
- .get_info = dsi_display_get_info,
- .set_backlight = dsi_display_set_backlight,
- .soft_reset = dsi_display_soft_reset,
- .pre_kickoff = dsi_conn_pre_kickoff,
- .clk_ctrl = dsi_display_clk_ctrl,
- .set_power = dsi_display_set_power,
- .get_mode_info = dsi_conn_get_mode_info,
- .get_dst_format = dsi_display_get_dst_format,
- .post_kickoff = dsi_conn_post_kickoff
- };
- struct msm_display_info info;
- struct drm_encoder *encoder;
- void *display, *connector;
+ struct drm_encoder *encoder = NULL;
int i, rc;
- /* dsi */
- for (i = 0; i < dpu_kms->dsi_display_count &&
- priv->num_encoders < max_encoders; ++i) {
- display = dpu_kms->dsi_displays[i];
- encoder = NULL;
+ /*TODO: Support two independent DSI connectors */
+ encoder = dpu_encoder_init(dev, DRM_MODE_CONNECTOR_DSI);
+ if (IS_ERR_OR_NULL(encoder)) {
+ DPU_ERROR("encoder init failed for dsi %d\n", i);
Is i meaningful here? It seems like it's uninitialized...
+ return;
+ }
- memset(&info, 0x0, sizeof(info));
- rc = dsi_display_get_info(&info, display);
- if (rc) {
- DPU_ERROR("dsi get_info %d failed\n", i);
- continue;
- }
+ priv->encoders[priv->num_encoders++] = encoder;
- encoder = dpu_encoder_init(dev, &info);
- if (IS_ERR_OR_NULL(encoder)) {
- DPU_ERROR("encoder init failed for dsi %d\n", i);
- continue;
+ for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+ if (!priv->dsi[i]) {
+ DPU_DEBUG("invalid msm_dsi for ctrl %d\n", i);
+ return;
}
- rc = dsi_display_drm_bridge_init(display, encoder);
+ rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
- DPU_ERROR("dsi bridge %d init failed, %d\n", i,
rc);
- dpu_encoder_destroy(encoder);
+ DPU_ERROR("modeset_init failed for dsi[%d]\n", i);
Printing rc would be nice here, same for above.
continue;
}
-
- connector = dpu_connector_init(dev,
- encoder,
- 0,
- display,
- &dsi_ops,
- DRM_CONNECTOR_POLL_HPD,
- DRM_MODE_CONNECTOR_DSI);
- if (connector) {
- priv->encoders[priv->num_encoders++] = encoder;
- } else {
- DPU_ERROR("dsi %d connector init failed\n", i);
- dsi_display_drm_bridge_deinit(display);
- dpu_encoder_destroy(encoder);
- }
}
}
-#endif
+
#ifdef CONFIG_DRM_MSM_WRITEBACK
static void _dpu_kms_initialize_wb(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms,
- unsigned max_encoders)
+ unsigned int max_encoders)
{
static const struct dpu_connector_ops wb_ops = {
.post_init = dpu_wb_connector_post_init,
@@ -800,7 +710,7 @@ static void _dpu_kms_initialize_wb(struct
drm_device
*dev,
static void _dpu_kms_initialize_dp(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms,
- unsigned max_encoders)
+ unsigned int max_encoders)
These 2 type changes are just diff noise, please remove.
{
static const struct dpu_connector_ops dp_ops = {
.post_init = dp_connector_post_init,
@@ -872,18 +782,7 @@ static void _dpu_kms_setup_displays(struct
drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
{
- unsigned max_encoders;
-
- max_encoders = dpu_kms->dsi_display_count +
dpu_kms->wb_display_count +
- dpu_kms->dp_display_count;
- if (max_encoders > ARRAY_SIZE(priv->encoders)) {
- max_encoders = ARRAY_SIZE(priv->encoders);
- DPU_ERROR("capping number of displays to %d",
max_encoders);
- }
-
-#ifdef CONFIG_DRM_MSM_DSI_STAGING
- _dpu_kms_initialize_dsi(dev, priv, dpu_kms, max_encoders);
-#endif
+ _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
#ifdef CONFIG_DRM_MSM_WRITEBACK
_dpu_kms_initialize_wb(dev, priv, dpu_kms, max_encoders);
@@ -1521,6 +1420,7 @@ static int dpu_kms_atomic_check(struct msm_kms
*kms,
dpu_kms->aspace[domain] : NULL;
}
+#ifdef CONFIG_DRM_MSM_DISPLAYPORT
This seems suspicious. Why different behavior depending on DisplayPort?
The
function loops through all connectors, so this is something that should
work
regardless of whether DP is connected.