Cleanup overly complex omap_modeset_init(). The function is trying to support many unusual configuration, that have never been tested and are not supported by other parts of the dirver.

After cleanup the init function creates exactly one connector, encoder, crtc, and primary plane per each connected dss-device. Each connector->encoder->crtc chain is expected to be separate and each crtc is connect to a single dss-channel. If the configuration does not match the expectations or exceeds the available resources, the configuration is rejected.

Signed-off-by: Jyri Sarha jsarha@ti.com --- drivers/gpu/drm/omapdrm/omap_crtc.c | 23 ++++- drivers/gpu/drm/omapdrm/omap_drv.c | 173 +++++++++-------------------------- drivers/gpu/drm/omapdrm/omap_drv.h | 2 +- drivers/gpu/drm/omapdrm/omap_plane.c | 3 + 4 files changed, 68 insertions(+), 133 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2fe735c..4bafc7b 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -521,6 +521,11 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,

void omap_crtc_pre_init(void) { + int i; + + for (i = 0; i < ARRAY_SIZE(omap_crtcs); i++) + omap_crtcs[i] = NULL; + dss_install_mgr_ops(&mgr_ops); }

@@ -531,17 +536,27 @@ void omap_crtc_pre_uninit(void)

/* initialize crtc */ struct drm_crtc *omap_crtc_init(struct drm_device *dev, - struct drm_plane *plane, enum omap_channel channel, int id) + struct drm_plane *plane, struct omap_dss_device *dssdev) { struct drm_crtc *crtc = NULL; struct omap_crtc *omap_crtc; + enum omap_channel channel; + struct omap_dss_device *out; int ret;

+ out = omapdss_find_output_from_display(dssdev); + channel = out->dispc_channel; + omap_dss_put_device(out); + DBG("%s", channel_names[channel]);

+ /* Multiple displays on same channel is not allowed */ + if (WARN_ON(omap_crtcs[channel] != NULL)) + return ERR_PTR(-EINVAL); + omap_crtc = kzalloc(sizeof(*omap_crtc), GFP_KERNEL); if (!omap_crtc) - return NULL; + return ERR_PTR(-ENOMEM);

crtc = &omap_crtc->base;

@@ -553,8 +568,10 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev, ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL, &omap_crtc_funcs, NULL); if (ret < 0) { + dev_err(dev->dev, "%s(): could not init crtc for: %s\n", + __func__, dssdev->name); kfree(omap_crtc); - return NULL; + return ERR_PTR(ret); }

drm_crtc_helper_add(crtc, &omap_crtc_helper_funcs); diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 17c40aa..7bea6f5 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -219,20 +219,6 @@ static int get_connector_type(struct omap_dss_device *dssdev) } }

-static bool channel_used(struct drm_device *dev, enum omap_channel channel) -{ - struct omap_drm_private *priv = dev->dev_private; - int i; - - for (i = 0; i < priv->num_crtcs; i++) { - struct drm_crtc *crtc = priv->crtcs[i]; - - if (omap_crtc_channel(crtc) == channel) - return true; - } - - return false; -} static void omap_disconnect_dssdevs(void) { struct omap_dss_device *dssdev = NULL; @@ -275,31 +261,6 @@ static int omap_connect_dssdevs(void) return r; }

-static int omap_modeset_create_crtc(struct drm_device *dev, int id, - enum omap_channel channel, - u32 possible_crtcs) -{ - struct omap_drm_private *priv = dev->dev_private; - struct drm_plane *plane; - struct drm_crtc *crtc; - - plane = omap_plane_init(dev, id, DRM_PLANE_TYPE_PRIMARY, - possible_crtcs); - if (IS_ERR(plane)) - return PTR_ERR(plane); - - crtc = omap_crtc_init(dev, plane, channel, id); - - BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs)); - priv->crtcs[id] = crtc; - priv->num_crtcs++; - - priv->planes[id] = plane; - priv->num_planes++; - - return 0; -} - static int omap_modeset_init_properties(struct drm_device *dev) { struct omap_drm_private *priv = dev->dev_private; @@ -318,9 +279,9 @@ static int omap_modeset_init(struct drm_device *dev) int num_ovls = dss_feat_get_num_ovls(); int num_mgrs = dss_feat_get_num_mgrs(); int num_crtcs = 0; - int i, id = 0; + int crtc_idx = 0, plane_idx = 0; int ret; - u32 possible_crtcs; + u32 plane_crtc_mask;

drm_mode_config_init(dev);

@@ -329,134 +290,88 @@ static int omap_modeset_init(struct drm_device *dev) return ret;

/* - * Let's create one CRTC for each connected DSS device if we - * have display managers and overlays (for primary planes) for - * them. + * This function creates exactly one connector, encoder, crtc, + * and primary plane per each connected dss-device. Each + * connector->encoder->crtc chain is expected to be separate + * and each crtc is connect to a single dss-channel. If the + * configuration does not match the expectations or exceeds + * the available resources, the configuration is rejected. */ for_each_dss_dev(dssdev) if (omapdss_device_is_connected(dssdev)) num_crtcs++;

- num_crtcs = min3(num_crtcs, num_mgrs, num_ovls); - possible_crtcs = (1 << num_crtcs) - 1; + if (num_crtcs > num_mgrs || num_crtcs > num_ovls || + num_crtcs > ARRAY_SIZE(priv->crtcs) || + num_crtcs > ARRAY_SIZE(priv->planes) || + num_crtcs > ARRAY_SIZE(priv->encoders) || + num_crtcs > ARRAY_SIZE(priv->connectors)) { + dev_err(dev->dev, "%s(): Too many connected displays\n", + __func__); + return -EINVAL; + } + + /* All planes can be put to any CRTC */ + plane_crtc_mask = (1 << num_crtcs) - 1;

dssdev = NULL;

for_each_dss_dev(dssdev) { struct drm_connector *connector; struct drm_encoder *encoder; - enum omap_channel channel; - struct omap_dss_device *out; + struct drm_plane *plane; + struct drm_crtc *crtc;

if (!omapdss_device_is_connected(dssdev)) continue;

encoder = omap_encoder_init(dev, dssdev); - - if (!encoder) { - dev_err(dev->dev, "could not create encoder: %s\n", - dssdev->name); + if (!encoder) return -ENOMEM; - }

connector = omap_connector_init(dev, get_connector_type(dssdev), dssdev, encoder); - - if (!connector) { - dev_err(dev->dev, "could not create connector: %s\n", - dssdev->name); + if (!connector) return -ENOMEM; - }

- BUG_ON(priv->num_encoders >= ARRAY_SIZE(priv->encoders)); - BUG_ON(priv->num_connectors >= ARRAY_SIZE(priv->connectors)); + plane = omap_plane_init(dev, plane_idx, DRM_PLANE_TYPE_PRIMARY, + plane_crtc_mask); + if (IS_ERR(plane)) + return PTR_ERR(plane);

- priv->encoders[priv->num_encoders++] = encoder; - priv->connectors[priv->num_connectors++] = connector; + crtc = omap_crtc_init(dev, plane, dssdev); + if (IS_ERR(crtc)) + return PTR_ERR(crtc);

drm_mode_connector_attach_encoder(connector, encoder); + encoder->possible_crtcs = (1 << crtc_idx);

- /* - * if we have reached the limit of the crtcs we can - * create, let's not try to create a crtc for this - * panel/encoder and onwards. - */ - if (id == num_crtcs) - continue; + priv->crtcs[priv->num_crtcs++] = crtc; + priv->planes[priv->num_planes++] = plane; + priv->encoders[priv->num_encoders++] = encoder; + priv->connectors[priv->num_connectors++] = connector;

- /* - * get the recommended DISPC channel for this encoder. For now, - * we only try to get create a crtc out of the recommended, the - * other possible channels to which the encoder can connect are - * not considered. - */ - - out = omapdss_find_output_from_display(dssdev); - channel = out->dispc_channel; - omap_dss_put_device(out); - - /* - * if this channel hasn't already been taken by a previously - * allocated crtc, we create a new crtc for it - */ - if (!channel_used(dev, channel)) { - ret = omap_modeset_create_crtc(dev, id, channel, - possible_crtcs); - if (ret < 0) { - dev_err(dev->dev, - "could not create CRTC (channel %u)\n", - channel); - return ret; - } - - id++; - } + plane_idx++; + crtc_idx++; }

/* * Create normal planes for the remaining overlays: */ - for (; id < num_ovls; id++) { + for (; plane_idx < num_ovls; plane_idx++) { struct drm_plane *plane;

- plane = omap_plane_init(dev, id, DRM_PLANE_TYPE_OVERLAY, - possible_crtcs); + if (WARN_ON(priv->num_planes >= ARRAY_SIZE(priv->planes))) + return -EINVAL; + + plane = omap_plane_init(dev, plane_idx, DRM_PLANE_TYPE_OVERLAY, + plane_crtc_mask); if (IS_ERR(plane)) return PTR_ERR(plane);

- BUG_ON(priv->num_planes >= ARRAY_SIZE(priv->planes)); priv->planes[priv->num_planes++] = plane; }

- /* - * populate the the possible_crtcs field for all the encoders - * we created. - */ - for (i = 0; i < priv->num_encoders; i++) { - struct drm_encoder *encoder = priv->encoders[i]; - struct omap_dss_device *dssdev = - omap_encoder_get_dssdev(encoder); - struct omap_dss_device *output; - - output = omapdss_find_output_from_display(dssdev); - - /* figure out which crtc's we can connect the encoder to: */ - encoder->possible_crtcs = 0; - for (id = 0; id < priv->num_crtcs; id++) { - struct drm_crtc *crtc = priv->crtcs[id]; - enum omap_channel crtc_channel; - - crtc_channel = omap_crtc_channel(crtc); - - if (output->dispc_channel == crtc_channel) { - encoder->possible_crtcs |= (1 << id); - break; - } - } - - omap_dss_put_device(output); - } - DBG("registered %d planes, %d crtcs, %d encoders and %d connectors\n", priv->num_planes, priv->num_crtcs, priv->num_encoders, priv->num_connectors); diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h index 9098ea1..1e077a9 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.h +++ b/drivers/gpu/drm/omapdrm/omap_drv.h @@ -135,7 +135,7 @@ static inline void omap_fbdev_free(struct drm_device *dev) void omap_crtc_pre_init(void); void omap_crtc_pre_uninit(void); struct drm_crtc *omap_crtc_init(struct drm_device *dev, - struct drm_plane *plane, enum omap_channel channel, int id); + struct drm_plane *plane, struct omap_dss_device *dssdev); int omap_crtc_wait_pending(struct drm_crtc *crtc); void omap_crtc_error_irq(struct drm_crtc *crtc, uint32_t irqstatus); void omap_crtc_vblank_irq(struct drm_crtc *crtc); diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 386d90a..8fc1f4a 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -371,6 +371,9 @@ struct drm_plane *omap_plane_init(struct drm_device *dev, return plane;

error: + dev_err(dev->dev, "%s(): could not create plane: %s\n", + __func__, plane_names[id]); + kfree(omap_plane); return NULL; }