Re: [PATCH 07/24] drm/omap: Manage the usable omap_dss_device list within omap_drm_private

Hi Tomi,

Thank you for the patch.

On Monday, 12 February 2018 11:44:37 EET Tomi Valkeinen wrote:

From: Peter Ujfalusi peter.ujfalusi@ti.com

Instead of reaching back to DSS to iterate through the dss_devices every time, use an internal array where we store the available and usable dss_devices.

At the same time remove the omapdss_device_is_connected() check from omap_modeset_init() as it became irrelevant: We are not adding dssdevs if their connect failed.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com

---

drivers/gpu/drm/omapdrm/omap_drv.c | 95 ++++++++++++++++++++--------------- drivers/gpu/drm/omapdrm/omap_drv.h | 3 ++ 2 files changed, 62 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 57d11f9aeead..869a8ab6aa4e 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -149,18 +149,27 @@ static int get_connector_type(struct omap_dss_device *dssdev) } }

-static void omap_disconnect_dssdevs(void) +static void omap_disconnect_dssdevs(struct drm_device *ddev) {

• struct omap_dss_device *dssdev = NULL;

• struct omap_drm_private *priv = ddev->dev_private;
• unsigned int i;
• for (i = 0; i < priv->num_dssdevs; i++) {

• struct omap_dss_device *dssdev = priv->dssdevs[i];
  
  

• for_each_dss_dev(dssdev) dssdev->driver->disconnect(dssdev);

• priv->dssdevs[i] = NULL;
  

• omap_dss_put_device(dssdev);
  

• }
• priv->num_dssdevs = 0;

}

-static int omap_connect_dssdevs(void) +static int omap_connect_dssdevs(struct drm_device *ddev) {

• int r;

• struct omap_drm_private *priv = ddev->dev_private; struct omap_dss_device *dssdev = NULL;

• int r;

  if (!omapdss_stack_is_ready()) return -EPROBE_DEFER;

@@ -173,6 +182,14 @@ static int omap_connect_dssdevs(void) } else if (r) { dev_warn(dssdev->dev, "could not connect display: %s\n", dssdev->name);

• } else {
  

• 	omap_dss_get_device(dssdev);
  

• 	priv->dssdevs[priv->num_dssdevs++] = dssdev;
  

• 	if (priv->num_dssdevs == ARRAY_SIZE(priv->dssdevs)) {
  

• 		/* To balance the 'for_each_dss_dev' loop */
  

• 		omap_dss_put_device(dssdev);
  

• 		break;
  

• 	}
  

  } }

@@ -183,7 +200,7 @@ static int omap_connect_dssdevs(void) * if we are deferring probe, we disconnect the devices we previously * connected */

• omap_disconnect_dssdevs();

• omap_disconnect_dssdevs(ddev);

  return r;

} @@ -208,7 +225,7 @@ static int omap_modeset_init(struct drm_device *dev) int num_ovls = priv->dispc_ops->get_num_ovls(); int num_mgrs = priv->dispc_ops->get_num_mgrs(); int num_crtcs, crtc_idx, plane_idx;

• int ret;

• int ret, i;

i is never negative, you can make it an unsigned int. Same for the other functions below.

Apart from that,

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

u32 plane_crtc_mask;

drm_mode_config_init(dev); @@ -225,11 +242,7 @@ static int omap_modeset_init(struct drm_device *dev) * configuration does not match the expectations or exceeds * the available resources, the configuration is rejected. */

• num_crtcs = 0;
• for_each_dss_dev(dssdev)

• if (omapdss_device_is_connected(dssdev))
  

• 	num_crtcs++;
  

• num_crtcs = priv->num_dssdevs; if (num_crtcs > num_mgrs || num_crtcs > num_ovls || num_crtcs > ARRAY_SIZE(priv->crtcs) || num_crtcs > ARRAY_SIZE(priv->planes) ||

@@ -247,15 +260,13 @@ static int omap_modeset_init(struct drm_device *dev)

crtc_idx = 0; plane_idx = 0;

• for_each_dss_dev(dssdev) {

• for (i = 0; i < priv->num_dssdevs; i++) {

• struct omap_dss_device *dssdev = priv->dssdevs[i];
  

  struct drm_connector *connector; struct drm_encoder *encoder; struct drm_plane *plane; struct drm_crtc *crtc;

• if (!omapdss_device_is_connected(dssdev))
  

• 	continue;
  

• encoder = omap_encoder_init(dev, dssdev); if (!encoder) return -ENOMEM;

@@ -329,11 +340,14 @@ static int omap_modeset_init(struct drm_device *dev) /*

• Enable the HPD in external components if supported

*/ -static void omap_modeset_enable_external_hpd(void) +static void omap_modeset_enable_external_hpd(struct drm_device *ddev) {

• struct omap_dss_device *dssdev = NULL;

• struct omap_drm_private *priv = ddev->dev_private;
• int i;
• for (i = 0; i < priv->num_dssdevs; i++) {

• struct omap_dss_device *dssdev = priv->dssdevs[i];
  
  

• for_each_dss_dev(dssdev) { if (dssdev->driver->enable_hpd) dssdev->driver->enable_hpd(dssdev); }

@@ -342,11 +356,14 @@ static void omap_modeset_enable_external_hpd(void) /*

• Disable the HPD in external components if supported

*/ -static void omap_modeset_disable_external_hpd(void) +static void omap_modeset_disable_external_hpd(struct drm_device *ddev) {

• struct omap_dss_device *dssdev = NULL;

• struct omap_drm_private *priv = ddev->dev_private;
• int i;
• for (i = 0; i < priv->num_dssdevs; i++) {

• struct omap_dss_device *dssdev = priv->dssdevs[i];
  
  

• for_each_dss_dev(dssdev) { if (dssdev->driver->disable_hpd) dssdev->driver->disable_hpd(dssdev); }

@@ -543,7 +560,7 @@ static int pdev_probe(struct platform_device *pdev)

omap_crtc_pre_init();

• ret = omap_connect_dssdevs();

• ret = omap_connect_dssdevs(ddev); if (ret) goto err_crtc_uninit;

@@ -583,7 +600,7 @@ static int pdev_probe(struct platform_device *pdev) omap_fbdev_init(ddev);

drm_kms_helper_poll_init(ddev);

• omap_modeset_enable_external_hpd();

• omap_modeset_enable_external_hpd(ddev);

  /*

  • Register the DRM device with the core and the connectors with

@@ -596,7 +613,7 @@ static int pdev_probe(struct platform_device *pdev) return 0;

err_cleanup_helpers:

• omap_modeset_disable_external_hpd();

• omap_modeset_disable_external_hpd(ddev); drm_kms_helper_poll_fini(ddev);

  omap_fbdev_free(ddev);

@@ -606,7 +623,7 @@ static int pdev_probe(struct platform_device *pdev) err_gem_deinit: omap_gem_deinit(ddev); destroy_workqueue(priv->wq);

• omap_disconnect_dssdevs();

• omap_disconnect_dssdevs(ddev);

err_crtc_uninit: omap_crtc_pre_uninit(); drm_dev_unref(ddev); @@ -622,7 +639,7 @@ static int pdev_remove(struct platform_device *pdev)

drm_dev_unregister(ddev);

• omap_modeset_disable_external_hpd();

• omap_modeset_disable_external_hpd(ddev); drm_kms_helper_poll_fini(ddev);

  omap_fbdev_free(ddev);

@@ -636,7 +653,7 @@ static int pdev_remove(struct platform_device *pdev)

destroy_workqueue(priv->wq);

• omap_disconnect_dssdevs();

• omap_disconnect_dssdevs(ddev); omap_crtc_pre_uninit();

  drm_dev_unref(ddev);

@@ -645,11 +662,14 @@ static int pdev_remove(struct platform_device *pdev) }

#ifdef CONFIG_PM_SLEEP -static int omap_drm_suspend_all_displays(void) +static int omap_drm_suspend_all_displays(struct drm_device *ddev) {

• struct omap_dss_device *dssdev = NULL;

• struct omap_drm_private *priv = ddev->dev_private;
• int i;
• for (i = 0; i < priv->num_dssdevs; i++) {

• struct omap_dss_device *dssdev = priv->dssdevs[i];
  
  

• for_each_dss_dev(dssdev) { if (!dssdev->driver) continue;

@@ -664,11 +684,14 @@ static int omap_drm_suspend_all_displays(void) return 0; }

-static int omap_drm_resume_all_displays(void) +static int omap_drm_resume_all_displays(struct drm_device *ddev) {

• struct omap_dss_device *dssdev = NULL;

• struct omap_drm_private *priv = ddev->dev_private;
• int i;
• for (i = 0; i < priv->num_dssdevs; i++) {

• struct omap_dss_device *dssdev = priv->dssdevs[i];
  
  

• for_each_dss_dev(dssdev) { if (!dssdev->driver) continue;

@@ -688,7 +711,7 @@ static int omap_drm_suspend(struct device *dev) drm_kms_helper_poll_disable(drm_dev);

drm_modeset_lock_all(drm_dev);

• omap_drm_suspend_all_displays();

• omap_drm_suspend_all_displays(drm_dev); drm_modeset_unlock_all(drm_dev);

  return 0;

@@ -699,7 +722,7 @@ static int omap_drm_resume(struct device *dev) struct drm_device *drm_dev = dev_get_drvdata(dev);

drm_modeset_lock_all(drm_dev);

• omap_drm_resume_all_displays();

• omap_drm_resume_all_displays(drm_dev); drm_modeset_unlock_all(drm_dev);

  drm_kms_helper_poll_enable(drm_dev);

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h index ba322c519999..c9e433a91cd0 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.h +++ b/drivers/gpu/drm/omapdrm/omap_drv.h @@ -50,6 +50,9 @@ struct omap_drm_private {

const struct dispc_ops *dispc_ops;

• unsigned int num_dssdevs;
• struct omap_dss_device *dssdevs[8];
• unsigned int num_crtcs; struct drm_crtc *crtcs[8];