[PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init()

[PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init()

[Patrik Jakobsson]

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/framebuffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 45df9de22007..2b99c996fdc2 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	int i;
 
-	drm_mode_config_init(dev);
+	if (drmm_mode_config_init(dev))
+		return;
 
 	dev->mode_config.min_width = 0;
 	dev->mode_config.min_height = 0;
@@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev)
 	if (dev_priv->modeset) {
 		drm_kms_helper_poll_fini(dev);
 		psb_fbdev_fini(dev);
-		drm_mode_config_cleanup(dev);
 	}
 }
-- 
2.35.1

[PATCH 2/7] drm/gma500: Uninstall interrupts on driver removal

[Patrik Jakobsson]

Reloading the driver revealed that the interrupt handler never got
uninstalled.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/psb_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 82d51e9821ad..b231fddb8817 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -173,6 +173,8 @@ static void psb_driver_unload(struct drm_device *dev)
 		gma_backlight_exit(dev);
 	psb_modeset_cleanup(dev);
 
+	gma_irq_uninstall(dev);
+
 	if (dev_priv->ops->chip_teardown)
 		dev_priv->ops->chip_teardown(dev);
 
-- 
2.35.1

[PATCH 3/7] drm/gma500: Make use of the drm connector iterator

[Patrik Jakobsson]

This makes sure we're using proper locking when iterating the list of
connectors.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_device.c        | 10 ++++++--
 drivers/gpu/drm/gma500/cdv_intel_display.c |  9 +++++--
 drivers/gpu/drm/gma500/framebuffer.c       |  6 +++--
 drivers/gpu/drm/gma500/gma_display.c       | 16 ++++++++-----
 drivers/gpu/drm/gma500/oaktrail_crtc.c     | 17 ++++++++-----
 drivers/gpu/drm/gma500/oaktrail_lvds.c     | 15 ++++++------
 drivers/gpu/drm/gma500/psb_device.c        | 28 +++++++++++++++-------
 drivers/gpu/drm/gma500/psb_drv.c           | 10 ++++----
 drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++----
 9 files changed, 84 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c
index f854f58bcbb3..dd32b484dd82 100644
--- a/drivers/gpu/drm/gma500/cdv_device.c
+++ b/drivers/gpu/drm/gma500/cdv_device.c
@@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev)
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	struct psb_save_area *regs = &dev_priv->regs;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 
 	dev_dbg(dev->dev, "Saving GPU registers.\n");
@@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev)
 	regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R);
 	regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R);
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter)
 		connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
+	drm_connector_list_iter_end(&conn_iter);
 
 	return 0;
 }
@@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev)
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	struct psb_save_area *regs = &dev_priv->regs;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 	u32 temp;
 
@@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev)
 
 	drm_mode_config_reset(dev);
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter)
 		connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
+	drm_connector_list_iter_end(&conn_iter);
 
 	/* Resume the modeset for every activated CRTC */
 	drm_helper_resume_force_mode(dev);
diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c
index 94ebc48a4349..0c3ddcdc29dc 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_display.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
@@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
 	bool ok;
 	bool is_lvds = false;
 	bool is_dp = false;
-	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 	const struct gma_limit_t *limit;
 	u32 ddi_select = 0;
 	bool is_edp = false;
 
-	list_for_each_entry(connector, &mode_config->connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct gma_encoder *gma_encoder =
 					gma_attached_encoder(connector);
 
@@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
 			is_edp = true;
 			break;
 		default:
+			drm_connector_list_iter_end(&conn_iter);
 			DRM_ERROR("invalid output type.\n");
 			return 0;
 		}
+
+		break;
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	if (dev_priv->dplla_96mhz)
 		/* low-end sku, 96/100 mhz */
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 2b99c996fdc2..0ac6ea5fd3a1 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = {
 static void psb_setup_outputs(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 
 	drm_mode_create_scaling_mode_property(dev);
@@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev)
 							"backlight", 0, 100);
 	dev_priv->ops->output_init(dev);
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list,
-			    head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 		struct drm_encoder *encoder = &gma_encoder->base;
 		int crtc_mask = 0, clone_mask = 0;
@@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev)
 		encoder->possible_clones =
 		    gma_connector_clones(dev, clone_mask);
 	}
+	drm_connector_list_iter_end(&conn_iter);
 }
 
 void psb_modeset_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index 1d7964c339f4..e8157464d9eb 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -27,17 +27,21 @@
 bool gma_pipe_has_type(struct drm_crtc *crtc, int type)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_mode_config *mode_config = &dev->mode_config;
-	struct drm_connector *l_entry;
+	struct drm_connector_list_iter conn_iter;
+	struct drm_connector *connector;
 
-	list_for_each_entry(l_entry, &mode_config->connector_list, head) {
-		if (l_entry->encoder && l_entry->encoder->crtc == crtc) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		if (connector->encoder && connector->encoder->crtc == crtc) {
 			struct gma_encoder *gma_encoder =
-						gma_attached_encoder(l_entry);
-			if (gma_encoder->type == type)
+						gma_attached_encoder(connector);
+			if (gma_encoder->type == type) {
+				drm_connector_list_iter_end(&conn_iter);
 				return true;
+			}
 		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	return false;
 }
diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c
index 36c7c2686c90..873c17cf8fb4 100644
--- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
+++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
@@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
 	bool ok, is_sdvo = false;
 	bool is_lvds = false;
 	bool is_mipi = false;
-	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct gma_encoder *gma_encoder = NULL;
 	uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 	int i;
 	int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0;
@@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
 		adjusted_mode,
 		sizeof(struct drm_display_mode));
 
-	list_for_each_entry(connector, &mode_config->connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (!connector->encoder || connector->encoder->crtc != crtc)
 			continue;
 
@@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
 			is_mipi = true;
 			break;
 		}
+
+		break;
 	}
 
+	if (gma_encoder)
+		drm_object_property_get_value(&connector->base,
+			dev->mode_config.scaling_mode_property, &scalingType);
+
+	drm_connector_list_iter_end(&conn_iter);
+
 	/* Disable the VGA plane that we never use */
 	for (i = 0; i <= need_aux; i++)
 		REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i);
@@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
 					     (mode->crtc_vdisplay - 1), i);
 	}
 
-	if (gma_encoder)
-		drm_object_property_get_value(&connector->base,
-			dev->mode_config.scaling_mode_property, &scalingType);
-
 	if (scalingType == DRM_MODE_SCALE_NO_SCALE) {
 		/* Moorestown doesn't have register support for centering so
 		 * we need to mess with the h/vblank and h/vsync start and
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 28b995ef2844..04852dbc7fb3 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
 	struct drm_device *dev = encoder->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector = NULL;
 	struct drm_crtc *crtc = encoder->crtc;
 	u32 lvds_port;
@@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
 	REG_WRITE(LVDS, lvds_port);
 
 	/* Find the connector we're trying to set up */
-	list_for_each_entry(connector, &mode_config->connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->encoder && connector->encoder->crtc == crtc)
 			break;
 	}
 
-	if (list_entry_is_head(connector, &mode_config->connector_list, head)) {
+	if (!connector) {
+		drm_connector_list_iter_end(&conn_iter);
 		DRM_ERROR("Couldn't find connector when setting mode");
 		gma_power_end(dev);
 		return;
 	}
 
-	drm_object_property_get_value(
-		&connector->base,
-		dev->mode_config.scaling_mode_property,
-		&v);
+	drm_object_property_get_value( &connector->base,
+		dev->mode_config.scaling_mode_property, &v);
+	drm_connector_list_iter_end(&conn_iter);
 
 	if (v == DRM_MODE_SCALE_NO_SCALE)
 		REG_WRITE(PFIT_CONTROL, 0);
diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
index 59f325165667..71534f4ca834 100644
--- a/drivers/gpu/drm/gma500/psb_device.c
+++ b/drivers/gpu/drm/gma500/psb_device.c
@@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev)
 static int psb_save_display_registers(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct gma_connector *gma_connector;
 	struct drm_crtc *crtc;
-	struct gma_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	struct drm_connector *connector;
 	struct psb_state *regs = &dev_priv->regs.psb;
 
 	/* Display arbitration control + watermarks */
@@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev)
 			dev_priv->ops->save_crtc(crtc);
 	}
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
-		if (connector->save)
-			connector->save(&connector->base);
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		gma_connector = to_gma_connector(connector);
+		if (gma_connector->save)
+			gma_connector->save(connector);
+	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	drm_modeset_unlock_all(dev);
 	return 0;
@@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev)
 static int psb_restore_display_registers(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct gma_connector *gma_connector;
 	struct drm_crtc *crtc;
-	struct gma_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	struct drm_connector *connector;
 	struct psb_state *regs = &dev_priv->regs.psb;
 
 	/* Display arbitration + watermarks */
@@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev)
 		if (drm_helper_crtc_in_use(crtc))
 			dev_priv->ops->restore_crtc(crtc);
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
-		if (connector->restore)
-			connector->restore(&connector->base);
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		gma_connector = to_gma_connector(connector);
+		if (gma_connector->restore)
+			gma_connector->restore(connector);
+	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	drm_modeset_unlock_all(dev);
 	return 0;
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index b231fddb8817..bb0e3288e35b 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	unsigned long resource_start, resource_len;
 	unsigned long irqflags;
-	int ret = -ENOMEM;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 	struct gma_encoder *gma_encoder;
 	struct psb_gtt *pg;
+	int ret = -ENOMEM;
 
 	/* initializing driver private data */
 
@@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 	psb_fbdev_init(dev);
 	drm_kms_helper_poll_init(dev);
 
-	/* Only add backlight support if we have LVDS output */
-	list_for_each_entry(connector, &dev->mode_config.connector_list,
-			    head) {
+	/* Only add backlight support if we have LVDS or MIPI output */
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		gma_encoder = gma_attached_encoder(connector);
 
 		switch (gma_encoder->type) {
@@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 			break;
 		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
index a99859b5b13a..fb8234f4d128 100644
--- a/drivers/gpu/drm/gma500/psb_intel_display.c
+++ b/drivers/gpu/drm/gma500/psb_intel_display.c
@@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
 	u32 dpll = 0, fp = 0, dspcntr, pipeconf;
 	bool ok, is_sdvo = false;
 	bool is_lvds = false, is_tv = false;
-	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
 	const struct gma_limit_t *limit;
 
@@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
 		return 0;
 	}
 
-	list_for_each_entry(connector, &mode_config->connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 
 		if (!connector->encoder
@@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
 			break;
 		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	refclk = 96000;
 
@@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)
 
 int gma_connector_clones(struct drm_device *dev, int type_mask)
 {
-	int index_mask = 0;
+	struct drm_connector_list_iter conn_iter;
 	struct drm_connector *connector;
+	int index_mask = 0;
 	int entry = 0;
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list,
-			    head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 		if (type_mask & (1 << gma_encoder->type))
 			index_mask |= (1 << entry);
 		entry++;
 	}
+	drm_connector_list_iter_end(&conn_iter);
+
 	return index_mask;
 }
-- 
2.35.1

[PATCH 4/7] drm/gma500: gma500 don't register non-hotpluggable connectors

[Patrik Jakobsson]

According to docs we should only register connectors that are
hotpluggable. No connectors in gma500 are hotpluggable.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_crt.c  | 3 ---
 drivers/gpu/drm/gma500/cdv_intel_dp.c   | 3 ---
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 2 --
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 --
 drivers/gpu/drm/gma500/oaktrail_hdmi.c  | 1 -
 drivers/gpu/drm/gma500/oaktrail_lvds.c  | 1 -
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 2 --
 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 2 --
 8 files changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_crt.c b/drivers/gpu/drm/gma500/cdv_intel_crt.c
index 4a9bb4994a26..1ae0fbbda0eb 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_crt.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_crt.c
@@ -194,7 +194,6 @@ static void cdv_intel_crt_destroy(struct drm_connector *connector)
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 
 	psb_intel_i2c_destroy(gma_encoder->ddc_bus);
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
 }
@@ -281,8 +280,6 @@ void cdv_intel_crt_init(struct drm_device *dev,
 	drm_connector_helper_add(connector,
 					&cdv_intel_crt_connector_helper_funcs);
 
-	drm_connector_register(connector);
-
 	return;
 failed_ddc:
 	drm_encoder_cleanup(&gma_encoder->base);
diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c
index f562e91337c7..3fba9d4e785d 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
@@ -1866,7 +1866,6 @@ cdv_intel_dp_destroy(struct drm_connector *connector)
 		intel_dp->panel_fixed_mode = NULL;
 	}
 	i2c_del_adapter(&intel_dp->adapter);
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
 }
@@ -1990,8 +1989,6 @@ cdv_intel_dp_init(struct drm_device *dev, struct psb_intel_mode_device *mode_dev
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
 
-	drm_connector_register(connector);
-
 	/* Set up the DDC bus. */
 	switch (output_reg) {
 		case DP_B:
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index e525689f84f0..e0d4c49b3c92 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -245,7 +245,6 @@ static void cdv_hdmi_destroy(struct drm_connector *connector)
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 
 	psb_intel_i2c_destroy(gma_encoder->i2c_bus);
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
 }
@@ -352,7 +351,6 @@ void cdv_hdmi_init(struct drm_device *dev,
 
 	hdmi_priv->hdmi_i2c_adapter = &(gma_encoder->i2c_bus->adapter);
 	hdmi_priv->dev = dev;
-	drm_connector_register(connector);
 	return;
 
 failed_ddc:
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 9e1cdb11023c..851a3cc4653e 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -329,7 +329,6 @@ static void cdv_intel_lvds_destroy(struct drm_connector *connector)
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 
 	psb_intel_i2c_destroy(gma_encoder->i2c_bus);
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
 }
@@ -647,7 +646,6 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 
 out:
 	mutex_unlock(&dev->mode_config.mutex);
-	drm_connector_register(connector);
 	return;
 
 failed_find:
diff --git a/drivers/gpu/drm/gma500/oaktrail_hdmi.c b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
index 6eef60a5ac27..b5946a1cdcd5 100644
--- a/drivers/gpu/drm/gma500/oaktrail_hdmi.c
+++ b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
@@ -654,7 +654,6 @@ void oaktrail_hdmi_init(struct drm_device *dev,
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
-	drm_connector_register(connector);
 	dev_info(dev->dev, "HDMI initialised.\n");
 
 	return;
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 04852dbc7fb3..aed5de8f8245 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -401,7 +401,6 @@ void oaktrail_lvds_init(struct drm_device *dev,
 out:
 	mutex_unlock(&dev->mode_config.mutex);
 
-	drm_connector_register(connector);
 	return;
 
 failed_find:
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index ac97e0d3c7dd..ec8f0b504ccc 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -525,7 +525,6 @@ void psb_intel_lvds_destroy(struct drm_connector *connector)
 	struct psb_intel_lvds_priv *lvds_priv = gma_encoder->dev_priv;
 
 	psb_intel_i2c_destroy(lvds_priv->ddc_bus);
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
 }
@@ -782,7 +781,6 @@ void psb_intel_lvds_init(struct drm_device *dev,
 	 */
 out:
 	mutex_unlock(&dev->mode_config.mutex);
-	drm_connector_register(connector);
 	return;
 
 failed_find:
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 042c4392e676..5b72a759a182 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -1542,7 +1542,6 @@ static int psb_intel_sdvo_get_modes(struct drm_connector *connector)
 
 static void psb_intel_sdvo_destroy(struct drm_connector *connector)
 {
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
 }
@@ -1932,7 +1931,6 @@ psb_intel_sdvo_connector_init(struct psb_intel_sdvo_connector *connector,
 	connector->base.restore = psb_intel_sdvo_restore;
 
 	gma_connector_attach_encoder(&connector->base, &encoder->base);
-	drm_connector_register(&connector->base.base);
 }
 
 static void
-- 
2.35.1

[PATCH 5/7] drm/gma500: Free the correct connector allocation

[Patrik Jakobsson]

The allocation is made for the gma_connector object so we must use the
same address when free()ing the object.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_crt.c  | 3 ++-
 drivers/gpu/drm/gma500/cdv_intel_dp.c   | 3 ++-
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 3 ++-
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 3 ++-
 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 4 +++-
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_crt.c b/drivers/gpu/drm/gma500/cdv_intel_crt.c
index 1ae0fbbda0eb..6bcd18c63c31 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_crt.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_crt.c
@@ -191,11 +191,12 @@ static enum drm_connector_status cdv_intel_crt_detect(
 
 static void cdv_intel_crt_destroy(struct drm_connector *connector)
 {
+	struct gma_connector *gma_connector = to_gma_connector(connector);
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 
 	psb_intel_i2c_destroy(gma_encoder->ddc_bus);
 	drm_connector_cleanup(connector);
-	kfree(connector);
+	kfree(gma_connector);
 }
 
 static int cdv_intel_crt_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c
index 3fba9d4e785d..72b1b2fc3c27 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
@@ -1857,6 +1857,7 @@ cdv_intel_dp_set_property(struct drm_connector *connector,
 static void
 cdv_intel_dp_destroy(struct drm_connector *connector)
 {
+	struct gma_connector *gma_connector = to_gma_connector(connector);
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 	struct cdv_intel_dp *intel_dp = gma_encoder->dev_priv;
 
@@ -1867,7 +1868,7 @@ cdv_intel_dp_destroy(struct drm_connector *connector)
 	}
 	i2c_del_adapter(&intel_dp->adapter);
 	drm_connector_cleanup(connector);
-	kfree(connector);
+	kfree(gma_connector);
 }
 
 static const struct drm_encoder_helper_funcs cdv_intel_dp_helper_funcs = {
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index e0d4c49b3c92..8987e555e113 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -242,11 +242,12 @@ static enum drm_mode_status cdv_hdmi_mode_valid(struct drm_connector *connector,
 
 static void cdv_hdmi_destroy(struct drm_connector *connector)
 {
+	struct gma_connector *gma_connector = to_gma_connector(connector);
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 
 	psb_intel_i2c_destroy(gma_encoder->i2c_bus);
 	drm_connector_cleanup(connector);
-	kfree(connector);
+	kfree(gma_connector);
 }
 
 static const struct drm_encoder_helper_funcs cdv_hdmi_helper_funcs = {
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 851a3cc4653e..98d9f5483a7c 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -326,11 +326,12 @@ static int cdv_intel_lvds_get_modes(struct drm_connector *connector)
  */
 static void cdv_intel_lvds_destroy(struct drm_connector *connector)
 {
+	struct gma_connector *gma_connector = to_gma_connector(connector);
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 
 	psb_intel_i2c_destroy(gma_encoder->i2c_bus);
 	drm_connector_cleanup(connector);
-	kfree(connector);
+	kfree(gma_connector);
 }
 
 static int cdv_intel_lvds_set_property(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index ec8f0b504ccc..cad00380b386 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -521,12 +521,13 @@ static int psb_intel_lvds_get_modes(struct drm_connector *connector)
  */
 void psb_intel_lvds_destroy(struct drm_connector *connector)
 {
+	struct gma_connector *gma_connector = to_gma_connector(connector);
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
 	struct psb_intel_lvds_priv *lvds_priv = gma_encoder->dev_priv;
 
 	psb_intel_i2c_destroy(lvds_priv->ddc_bus);
 	drm_connector_cleanup(connector);
-	kfree(connector);
+	kfree(gma_connector);
 }
 
 int psb_intel_lvds_set_property(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 5b72a759a182..a85aace25548 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -1542,8 +1542,10 @@ static int psb_intel_sdvo_get_modes(struct drm_connector *connector)
 
 static void psb_intel_sdvo_destroy(struct drm_connector *connector)
 {
+	struct gma_connector *gma_connector = to_gma_connector(connector);
+
 	drm_connector_cleanup(connector);
-	kfree(connector);
+	kfree(gma_connector);
 }
 
 static bool psb_intel_sdvo_detect_hdmi_audio(struct drm_connector *connector)
-- 
2.35.1

[PATCH 6/7] drm/gma500: Declare a few functions static

[Patrik Jakobsson]

These functions are not used outside of their file scope so can be
declared as static.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gma_display.c | 15 +++++++--------
 drivers/gpu/drm/gma500/psb_drv.c     |  2 +-
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index e8157464d9eb..369bc1f751cb 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -176,9 +176,9 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
 	}
 }
 
-int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
-		       u32 size,
-		       struct drm_modeset_acquire_ctx *ctx)
+static int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+			      u16 *blue, u32 size,
+			      struct drm_modeset_acquire_ctx *ctx)
 {
 	gma_crtc_load_lut(crtc);
 
@@ -323,10 +323,9 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
 	REG_WRITE(DSPARB, 0x3F3E);
 }
 
-int gma_crtc_cursor_set(struct drm_crtc *crtc,
-			struct drm_file *file_priv,
-			uint32_t handle,
-			uint32_t width, uint32_t height)
+static int gma_crtc_cursor_set(struct drm_crtc *crtc,
+			       struct drm_file *file_priv, uint32_t handle,
+			       uint32_t width, uint32_t height)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -439,7 +438,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
 	return ret;
 }
 
-int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
+static int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 {
 	struct drm_device *dev = crtc->dev;
 	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index bb0e3288e35b..2aff54d505e2 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -100,7 +100,7 @@ static const struct drm_ioctl_desc psb_ioctls[] = {
  *
  *	Soft reset the graphics engine and then reload the necessary registers.
  */
-void psb_spank(struct drm_psb_private *dev_priv)
+static void psb_spank(struct drm_psb_private *dev_priv)
 {
 	PSB_WSGX32(_PSB_CS_RESET_BIF_RESET | _PSB_CS_RESET_DPM_RESET |
 		_PSB_CS_RESET_TA_RESET | _PSB_CS_RESET_USE_RESET |
-- 
2.35.1

[PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions

[Patrik Jakobsson]

These functions operate on a crtc and should be prefixed properly.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gma_display.c | 6 +++---
 drivers/gpu/drm/gma500/psb_irq.c     | 6 +++---
 drivers/gpu/drm/gma500/psb_irq.h     | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index 369bc1f751cb..34ec3fca09ba 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -575,9 +575,9 @@ const struct drm_crtc_funcs gma_crtc_funcs = {
 	.set_config = gma_crtc_set_config,
 	.destroy = gma_crtc_destroy,
 	.page_flip = gma_crtc_page_flip,
-	.enable_vblank = gma_enable_vblank,
-	.disable_vblank = gma_disable_vblank,
-	.get_vblank_counter = gma_get_vblank_counter,
+	.enable_vblank = gma_crtc_enable_vblank,
+	.disable_vblank = gma_crtc_disable_vblank,
+	.get_vblank_counter = gma_crtc_get_vblank_counter,
 };
 
 /*
diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c
index 2e8ded532930..e6e6d61bbeab 100644
--- a/drivers/gpu/drm/gma500/psb_irq.c
+++ b/drivers/gpu/drm/gma500/psb_irq.c
@@ -371,7 +371,7 @@ void gma_irq_uninstall(struct drm_device *dev)
 	free_irq(pdev->irq, dev);
 }
 
-int gma_enable_vblank(struct drm_crtc *crtc)
+int gma_crtc_enable_vblank(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	unsigned int pipe = crtc->index;
@@ -404,7 +404,7 @@ int gma_enable_vblank(struct drm_crtc *crtc)
 	return 0;
 }
 
-void gma_disable_vblank(struct drm_crtc *crtc)
+void gma_crtc_disable_vblank(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	unsigned int pipe = crtc->index;
@@ -428,7 +428,7 @@ void gma_disable_vblank(struct drm_crtc *crtc)
 /* Called from drm generic code, passed a 'crtc', which
  * we use as a pipe index
  */
-u32 gma_get_vblank_counter(struct drm_crtc *crtc)
+u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	unsigned int pipe = crtc->index;
diff --git a/drivers/gpu/drm/gma500/psb_irq.h b/drivers/gpu/drm/gma500/psb_irq.h
index c22878914f5b..b51e395194ff 100644
--- a/drivers/gpu/drm/gma500/psb_irq.h
+++ b/drivers/gpu/drm/gma500/psb_irq.h
@@ -20,9 +20,9 @@ void gma_irq_postinstall(struct drm_device *dev);
 int  gma_irq_install(struct drm_device *dev, unsigned int irq);
 void gma_irq_uninstall(struct drm_device *dev);
 
-int  gma_enable_vblank(struct drm_crtc *crtc);
-void gma_disable_vblank(struct drm_crtc *crtc);
-u32  gma_get_vblank_counter(struct drm_crtc *crtc);
+int  gma_crtc_enable_vblank(struct drm_crtc *crtc);
+void gma_crtc_disable_vblank(struct drm_crtc *crtc);
+u32  gma_crtc_get_vblank_counter(struct drm_crtc *crtc);
 void gma_enable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask);
 void gma_disable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask);
 
-- 
2.35.1

Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator

[Daniel Vetter]

On Tue, Mar 22, 2022 at 02:17:38PM +0100, Patrik Jakobsson wrote:
> This makes sure we're using proper locking when iterating the list of
> connectors.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

Note that this is only needed if your driver deals with hotpluggable
connectors. Since gma500 doesn't, not really a need to convert this over,
but it also doesn't hurt.

If the kerneldoc doesn't explain this, maybe we should add it? Care for a
patch.

Either way on the entire series:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'll leave it up to you whether you want to push this one too or not.
-Daniel

> ---
>  drivers/gpu/drm/gma500/cdv_device.c        | 10 ++++++--
>  drivers/gpu/drm/gma500/cdv_intel_display.c |  9 +++++--
>  drivers/gpu/drm/gma500/framebuffer.c       |  6 +++--
>  drivers/gpu/drm/gma500/gma_display.c       | 16 ++++++++-----
>  drivers/gpu/drm/gma500/oaktrail_crtc.c     | 17 ++++++++-----
>  drivers/gpu/drm/gma500/oaktrail_lvds.c     | 15 ++++++------
>  drivers/gpu/drm/gma500/psb_device.c        | 28 +++++++++++++++-------
>  drivers/gpu/drm/gma500/psb_drv.c           | 10 ++++----
>  drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++----
>  9 files changed, 84 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c
> index f854f58bcbb3..dd32b484dd82 100644
> --- a/drivers/gpu/drm/gma500/cdv_device.c
> +++ b/drivers/gpu/drm/gma500/cdv_device.c
> @@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev)
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>  	struct pci_dev *pdev = to_pci_dev(dev->dev);
>  	struct psb_save_area *regs = &dev_priv->regs;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
>  
>  	dev_dbg(dev->dev, "Saving GPU registers.\n");
> @@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev)
>  	regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R);
>  	regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R);
>  
> -	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter)
>  		connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return 0;
>  }
> @@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev)
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>  	struct pci_dev *pdev = to_pci_dev(dev->dev);
>  	struct psb_save_area *regs = &dev_priv->regs;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
>  	u32 temp;
>  
> @@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev)
>  
>  	drm_mode_config_reset(dev);
>  
> -	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter)
>  		connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	/* Resume the modeset for every activated CRTC */
>  	drm_helper_resume_force_mode(dev);
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c
> index 94ebc48a4349..0c3ddcdc29dc 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_display.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
> @@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
>  	bool ok;
>  	bool is_lvds = false;
>  	bool is_dp = false;
> -	struct drm_mode_config *mode_config = &dev->mode_config;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
>  	const struct gma_limit_t *limit;
>  	u32 ddi_select = 0;
>  	bool is_edp = false;
>  
> -	list_for_each_entry(connector, &mode_config->connector_list, head) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		struct gma_encoder *gma_encoder =
>  					gma_attached_encoder(connector);
>  
> @@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
>  			is_edp = true;
>  			break;
>  		default:
> +			drm_connector_list_iter_end(&conn_iter);
>  			DRM_ERROR("invalid output type.\n");
>  			return 0;
>  		}
> +
> +		break;
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	if (dev_priv->dplla_96mhz)
>  		/* low-end sku, 96/100 mhz */
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
> index 2b99c996fdc2..0ac6ea5fd3a1 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = {
>  static void psb_setup_outputs(struct drm_device *dev)
>  {
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
>  
>  	drm_mode_create_scaling_mode_property(dev);
> @@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev)
>  							"backlight", 0, 100);
>  	dev_priv->ops->output_init(dev);
>  
> -	list_for_each_entry(connector, &dev->mode_config.connector_list,
> -			    head) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
>  		struct drm_encoder *encoder = &gma_encoder->base;
>  		int crtc_mask = 0, clone_mask = 0;
> @@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev)
>  		encoder->possible_clones =
>  		    gma_connector_clones(dev, clone_mask);
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  }
>  
>  void psb_modeset_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> index 1d7964c339f4..e8157464d9eb 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -27,17 +27,21 @@
>  bool gma_pipe_has_type(struct drm_crtc *crtc, int type)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct drm_mode_config *mode_config = &dev->mode_config;
> -	struct drm_connector *l_entry;
> +	struct drm_connector_list_iter conn_iter;
> +	struct drm_connector *connector;
>  
> -	list_for_each_entry(l_entry, &mode_config->connector_list, head) {
> -		if (l_entry->encoder && l_entry->encoder->crtc == crtc) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		if (connector->encoder && connector->encoder->crtc == crtc) {
>  			struct gma_encoder *gma_encoder =
> -						gma_attached_encoder(l_entry);
> -			if (gma_encoder->type == type)
> +						gma_attached_encoder(connector);
> +			if (gma_encoder->type == type) {
> +				drm_connector_list_iter_end(&conn_iter);
>  				return true;
> +			}
>  		}
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return false;
>  }
> diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> index 36c7c2686c90..873c17cf8fb4 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> @@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>  	bool ok, is_sdvo = false;
>  	bool is_lvds = false;
>  	bool is_mipi = false;
> -	struct drm_mode_config *mode_config = &dev->mode_config;
>  	struct gma_encoder *gma_encoder = NULL;
>  	uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
>  	int i;
>  	int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0;
> @@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>  		adjusted_mode,
>  		sizeof(struct drm_display_mode));
>  
> -	list_for_each_entry(connector, &mode_config->connector_list, head) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		if (!connector->encoder || connector->encoder->crtc != crtc)
>  			continue;
>  
> @@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>  			is_mipi = true;
>  			break;
>  		}
> +
> +		break;
>  	}
>  
> +	if (gma_encoder)
> +		drm_object_property_get_value(&connector->base,
> +			dev->mode_config.scaling_mode_property, &scalingType);
> +
> +	drm_connector_list_iter_end(&conn_iter);
> +
>  	/* Disable the VGA plane that we never use */
>  	for (i = 0; i <= need_aux; i++)
>  		REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i);
> @@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>  					     (mode->crtc_vdisplay - 1), i);
>  	}
>  
> -	if (gma_encoder)
> -		drm_object_property_get_value(&connector->base,
> -			dev->mode_config.scaling_mode_property, &scalingType);
> -
>  	if (scalingType == DRM_MODE_SCALE_NO_SCALE) {
>  		/* Moorestown doesn't have register support for centering so
>  		 * we need to mess with the h/vblank and h/vsync start and
> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> index 28b995ef2844..04852dbc7fb3 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> @@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>  	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> -	struct drm_mode_config *mode_config = &dev->mode_config;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector = NULL;
>  	struct drm_crtc *crtc = encoder->crtc;
>  	u32 lvds_port;
> @@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
>  	REG_WRITE(LVDS, lvds_port);
>  
>  	/* Find the connector we're trying to set up */
> -	list_for_each_entry(connector, &mode_config->connector_list, head) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		if (connector->encoder && connector->encoder->crtc == crtc)
>  			break;
>  	}
>  
> -	if (list_entry_is_head(connector, &mode_config->connector_list, head)) {
> +	if (!connector) {
> +		drm_connector_list_iter_end(&conn_iter);
>  		DRM_ERROR("Couldn't find connector when setting mode");
>  		gma_power_end(dev);
>  		return;
>  	}
>  
> -	drm_object_property_get_value(
> -		&connector->base,
> -		dev->mode_config.scaling_mode_property,
> -		&v);
> +	drm_object_property_get_value( &connector->base,
> +		dev->mode_config.scaling_mode_property, &v);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	if (v == DRM_MODE_SCALE_NO_SCALE)
>  		REG_WRITE(PFIT_CONTROL, 0);
> diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
> index 59f325165667..71534f4ca834 100644
> --- a/drivers/gpu/drm/gma500/psb_device.c
> +++ b/drivers/gpu/drm/gma500/psb_device.c
> @@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev)
>  static int psb_save_display_registers(struct drm_device *dev)
>  {
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +	struct gma_connector *gma_connector;
>  	struct drm_crtc *crtc;
> -	struct gma_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +	struct drm_connector *connector;
>  	struct psb_state *regs = &dev_priv->regs.psb;
>  
>  	/* Display arbitration control + watermarks */
> @@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev)
>  			dev_priv->ops->save_crtc(crtc);
>  	}
>  
> -	list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> -		if (connector->save)
> -			connector->save(&connector->base);
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		gma_connector = to_gma_connector(connector);
> +		if (gma_connector->save)
> +			gma_connector->save(connector);
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	drm_modeset_unlock_all(dev);
>  	return 0;
> @@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev)
>  static int psb_restore_display_registers(struct drm_device *dev)
>  {
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +	struct gma_connector *gma_connector;
>  	struct drm_crtc *crtc;
> -	struct gma_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +	struct drm_connector *connector;
>  	struct psb_state *regs = &dev_priv->regs.psb;
>  
>  	/* Display arbitration + watermarks */
> @@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev)
>  		if (drm_helper_crtc_in_use(crtc))
>  			dev_priv->ops->restore_crtc(crtc);
>  
> -	list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> -		if (connector->restore)
> -			connector->restore(&connector->base);
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		gma_connector = to_gma_connector(connector);
> +		if (gma_connector->restore)
> +			gma_connector->restore(connector);
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	drm_modeset_unlock_all(dev);
>  	return 0;
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> index b231fddb8817..bb0e3288e35b 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
>  	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>  	unsigned long resource_start, resource_len;
>  	unsigned long irqflags;
> -	int ret = -ENOMEM;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
>  	struct gma_encoder *gma_encoder;
>  	struct psb_gtt *pg;
> +	int ret = -ENOMEM;
>  
>  	/* initializing driver private data */
>  
> @@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
>  	psb_fbdev_init(dev);
>  	drm_kms_helper_poll_init(dev);
>  
> -	/* Only add backlight support if we have LVDS output */
> -	list_for_each_entry(connector, &dev->mode_config.connector_list,
> -			    head) {
> +	/* Only add backlight support if we have LVDS or MIPI output */
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		gma_encoder = gma_attached_encoder(connector);
>  
>  		switch (gma_encoder->type) {
> @@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
>  			break;
>  		}
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
> index a99859b5b13a..fb8234f4d128 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_display.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
> @@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
>  	u32 dpll = 0, fp = 0, dspcntr, pipeconf;
>  	bool ok, is_sdvo = false;
>  	bool is_lvds = false, is_tv = false;
> -	struct drm_mode_config *mode_config = &dev->mode_config;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
>  	const struct gma_limit_t *limit;
>  
> @@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
>  		return 0;
>  	}
>  
> -	list_for_each_entry(connector, &mode_config->connector_list, head) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
>  
>  		if (!connector->encoder
> @@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
>  			break;
>  		}
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	refclk = 96000;
>  
> @@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)
>  
>  int gma_connector_clones(struct drm_device *dev, int type_mask)
>  {
> -	int index_mask = 0;
> +	struct drm_connector_list_iter conn_iter;
>  	struct drm_connector *connector;
> +	int index_mask = 0;
>  	int entry = 0;
>  
> -	list_for_each_entry(connector, &dev->mode_config.connector_list,
> -			    head) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
>  		if (type_mask & (1 << gma_encoder->type))
>  			index_mask |= (1 << entry);
>  		entry++;
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
> +
>  	return index_mask;
>  }
> -- 
> 2.35.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator

[Patrik Jakobsson]

On Tue, Mar 22, 2022 at 3:39 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Tue, Mar 22, 2022 at 02:17:38PM +0100, Patrik Jakobsson wrote:
> > This makes sure we're using proper locking when iterating the list of
> > connectors.
> >
> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>
> Note that this is only needed if your driver deals with hotpluggable
> connectors. Since gma500 doesn't, not really a need to convert this over,
> but it also doesn't hurt.

Good point. Not sure but I think there is a slight possibility that
Cedarview can support DP MST. If someone adds support for DP MST then
this code would make sense. I was never able to exercise the DP code
because I couldn't find an eDP cable that fits the Intel DN2800MT (my
only device with DP). Do you happen to know what the 40-pin connector
is called? Perhaps Intel has some standard for eDP connectors?

>
> If the kerneldoc doesn't explain this, maybe we should add it? Care for a
> patch.

I didn't see it being mentioned anywhere. I'll send a patch.

>
> Either way on the entire series:
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks for having a look.

>
> I'll leave it up to you whether you want to push this one too or not.
> -Daniel
>
> > ---
> >  drivers/gpu/drm/gma500/cdv_device.c        | 10 ++++++--
> >  drivers/gpu/drm/gma500/cdv_intel_display.c |  9 +++++--
> >  drivers/gpu/drm/gma500/framebuffer.c       |  6 +++--
> >  drivers/gpu/drm/gma500/gma_display.c       | 16 ++++++++-----
> >  drivers/gpu/drm/gma500/oaktrail_crtc.c     | 17 ++++++++-----
> >  drivers/gpu/drm/gma500/oaktrail_lvds.c     | 15 ++++++------
> >  drivers/gpu/drm/gma500/psb_device.c        | 28 +++++++++++++++-------
> >  drivers/gpu/drm/gma500/psb_drv.c           | 10 ++++----
> >  drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++----
> >  9 files changed, 84 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c
> > index f854f58bcbb3..dd32b484dd82 100644
> > --- a/drivers/gpu/drm/gma500/cdv_device.c
> > +++ b/drivers/gpu/drm/gma500/cdv_device.c
> > @@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev)
> >       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> >       struct pci_dev *pdev = to_pci_dev(dev->dev);
> >       struct psb_save_area *regs = &dev_priv->regs;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> >
> >       dev_dbg(dev->dev, "Saving GPU registers.\n");
> > @@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev)
> >       regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R);
> >       regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R);
> >
> > -     list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter)
> >               connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       return 0;
> >  }
> > @@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev)
> >       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> >       struct pci_dev *pdev = to_pci_dev(dev->dev);
> >       struct psb_save_area *regs = &dev_priv->regs;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> >       u32 temp;
> >
> > @@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev)
> >
> >       drm_mode_config_reset(dev);
> >
> > -     list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter)
> >               connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       /* Resume the modeset for every activated CRTC */
> >       drm_helper_resume_force_mode(dev);
> > diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c
> > index 94ebc48a4349..0c3ddcdc29dc 100644
> > --- a/drivers/gpu/drm/gma500/cdv_intel_display.c
> > +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
> > @@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
> >       bool ok;
> >       bool is_lvds = false;
> >       bool is_dp = false;
> > -     struct drm_mode_config *mode_config = &dev->mode_config;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> >       const struct gma_limit_t *limit;
> >       u32 ddi_select = 0;
> >       bool is_edp = false;
> >
> > -     list_for_each_entry(connector, &mode_config->connector_list, head) {
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> >               struct gma_encoder *gma_encoder =
> >                                       gma_attached_encoder(connector);
> >
> > @@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
> >                       is_edp = true;
> >                       break;
> >               default:
> > +                     drm_connector_list_iter_end(&conn_iter);
> >                       DRM_ERROR("invalid output type.\n");
> >                       return 0;
> >               }
> > +
> > +             break;
> >       }
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       if (dev_priv->dplla_96mhz)
> >               /* low-end sku, 96/100 mhz */
> > diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
> > index 2b99c996fdc2..0ac6ea5fd3a1 100644
> > --- a/drivers/gpu/drm/gma500/framebuffer.c
> > +++ b/drivers/gpu/drm/gma500/framebuffer.c
> > @@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = {
> >  static void psb_setup_outputs(struct drm_device *dev)
> >  {
> >       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> >
> >       drm_mode_create_scaling_mode_property(dev);
> > @@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev)
> >                                                       "backlight", 0, 100);
> >       dev_priv->ops->output_init(dev);
> >
> > -     list_for_each_entry(connector, &dev->mode_config.connector_list,
> > -                         head) {
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> >               struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
> >               struct drm_encoder *encoder = &gma_encoder->base;
> >               int crtc_mask = 0, clone_mask = 0;
> > @@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev)
> >               encoder->possible_clones =
> >                   gma_connector_clones(dev, clone_mask);
> >       }
> > +     drm_connector_list_iter_end(&conn_iter);
> >  }
> >
> >  void psb_modeset_init(struct drm_device *dev)
> > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> > index 1d7964c339f4..e8157464d9eb 100644
> > --- a/drivers/gpu/drm/gma500/gma_display.c
> > +++ b/drivers/gpu/drm/gma500/gma_display.c
> > @@ -27,17 +27,21 @@
> >  bool gma_pipe_has_type(struct drm_crtc *crtc, int type)
> >  {
> >       struct drm_device *dev = crtc->dev;
> > -     struct drm_mode_config *mode_config = &dev->mode_config;
> > -     struct drm_connector *l_entry;
> > +     struct drm_connector_list_iter conn_iter;
> > +     struct drm_connector *connector;
> >
> > -     list_for_each_entry(l_entry, &mode_config->connector_list, head) {
> > -             if (l_entry->encoder && l_entry->encoder->crtc == crtc) {
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> > +             if (connector->encoder && connector->encoder->crtc == crtc) {
> >                       struct gma_encoder *gma_encoder =
> > -                                             gma_attached_encoder(l_entry);
> > -                     if (gma_encoder->type == type)
> > +                                             gma_attached_encoder(connector);
> > +                     if (gma_encoder->type == type) {
> > +                             drm_connector_list_iter_end(&conn_iter);
> >                               return true;
> > +                     }
> >               }
> >       }
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       return false;
> >  }
> > diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> > index 36c7c2686c90..873c17cf8fb4 100644
> > --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
> > +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> > @@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
> >       bool ok, is_sdvo = false;
> >       bool is_lvds = false;
> >       bool is_mipi = false;
> > -     struct drm_mode_config *mode_config = &dev->mode_config;
> >       struct gma_encoder *gma_encoder = NULL;
> >       uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> >       int i;
> >       int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0;
> > @@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
> >               adjusted_mode,
> >               sizeof(struct drm_display_mode));
> >
> > -     list_for_each_entry(connector, &mode_config->connector_list, head) {
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> >               if (!connector->encoder || connector->encoder->crtc != crtc)
> >                       continue;
> >
> > @@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
> >                       is_mipi = true;
> >                       break;
> >               }
> > +
> > +             break;
> >       }
> >
> > +     if (gma_encoder)
> > +             drm_object_property_get_value(&connector->base,
> > +                     dev->mode_config.scaling_mode_property, &scalingType);
> > +
> > +     drm_connector_list_iter_end(&conn_iter);
> > +
> >       /* Disable the VGA plane that we never use */
> >       for (i = 0; i <= need_aux; i++)
> >               REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i);
> > @@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
> >                                            (mode->crtc_vdisplay - 1), i);
> >       }
> >
> > -     if (gma_encoder)
> > -             drm_object_property_get_value(&connector->base,
> > -                     dev->mode_config.scaling_mode_property, &scalingType);
> > -
> >       if (scalingType == DRM_MODE_SCALE_NO_SCALE) {
> >               /* Moorestown doesn't have register support for centering so
> >                * we need to mess with the h/vblank and h/vsync start and
> > diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > index 28b995ef2844..04852dbc7fb3 100644
> > --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > @@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
> >       struct drm_device *dev = encoder->dev;
> >       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> >       struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> > -     struct drm_mode_config *mode_config = &dev->mode_config;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector = NULL;
> >       struct drm_crtc *crtc = encoder->crtc;
> >       u32 lvds_port;
> > @@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
> >       REG_WRITE(LVDS, lvds_port);
> >
> >       /* Find the connector we're trying to set up */
> > -     list_for_each_entry(connector, &mode_config->connector_list, head) {
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> >               if (connector->encoder && connector->encoder->crtc == crtc)
> >                       break;
> >       }
> >
> > -     if (list_entry_is_head(connector, &mode_config->connector_list, head)) {
> > +     if (!connector) {
> > +             drm_connector_list_iter_end(&conn_iter);
> >               DRM_ERROR("Couldn't find connector when setting mode");
> >               gma_power_end(dev);
> >               return;
> >       }
> >
> > -     drm_object_property_get_value(
> > -             &connector->base,
> > -             dev->mode_config.scaling_mode_property,
> > -             &v);
> > +     drm_object_property_get_value( &connector->base,
> > +             dev->mode_config.scaling_mode_property, &v);
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       if (v == DRM_MODE_SCALE_NO_SCALE)
> >               REG_WRITE(PFIT_CONTROL, 0);
> > diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
> > index 59f325165667..71534f4ca834 100644
> > --- a/drivers/gpu/drm/gma500/psb_device.c
> > +++ b/drivers/gpu/drm/gma500/psb_device.c
> > @@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev)
> >  static int psb_save_display_registers(struct drm_device *dev)
> >  {
> >       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > +     struct gma_connector *gma_connector;
> >       struct drm_crtc *crtc;
> > -     struct gma_connector *connector;
> > +     struct drm_connector_list_iter conn_iter;
> > +     struct drm_connector *connector;
> >       struct psb_state *regs = &dev_priv->regs.psb;
> >
> >       /* Display arbitration control + watermarks */
> > @@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev)
> >                       dev_priv->ops->save_crtc(crtc);
> >       }
> >
> > -     list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> > -             if (connector->save)
> > -                     connector->save(&connector->base);
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> > +             gma_connector = to_gma_connector(connector);
> > +             if (gma_connector->save)
> > +                     gma_connector->save(connector);
> > +     }
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       drm_modeset_unlock_all(dev);
> >       return 0;
> > @@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev)
> >  static int psb_restore_display_registers(struct drm_device *dev)
> >  {
> >       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > +     struct gma_connector *gma_connector;
> >       struct drm_crtc *crtc;
> > -     struct gma_connector *connector;
> > +     struct drm_connector_list_iter conn_iter;
> > +     struct drm_connector *connector;
> >       struct psb_state *regs = &dev_priv->regs.psb;
> >
> >       /* Display arbitration + watermarks */
> > @@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev)
> >               if (drm_helper_crtc_in_use(crtc))
> >                       dev_priv->ops->restore_crtc(crtc);
> >
> > -     list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> > -             if (connector->restore)
> > -                     connector->restore(&connector->base);
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> > +             gma_connector = to_gma_connector(connector);
> > +             if (gma_connector->restore)
> > +                     gma_connector->restore(connector);
> > +     }
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       drm_modeset_unlock_all(dev);
> >       return 0;
> > diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> > index b231fddb8817..bb0e3288e35b 100644
> > --- a/drivers/gpu/drm/gma500/psb_drv.c
> > +++ b/drivers/gpu/drm/gma500/psb_drv.c
> > @@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
> >       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> >       unsigned long resource_start, resource_len;
> >       unsigned long irqflags;
> > -     int ret = -ENOMEM;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> >       struct gma_encoder *gma_encoder;
> >       struct psb_gtt *pg;
> > +     int ret = -ENOMEM;
> >
> >       /* initializing driver private data */
> >
> > @@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
> >       psb_fbdev_init(dev);
> >       drm_kms_helper_poll_init(dev);
> >
> > -     /* Only add backlight support if we have LVDS output */
> > -     list_for_each_entry(connector, &dev->mode_config.connector_list,
> > -                         head) {
> > +     /* Only add backlight support if we have LVDS or MIPI output */
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> >               gma_encoder = gma_attached_encoder(connector);
> >
> >               switch (gma_encoder->type) {
> > @@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
> >                       break;
> >               }
> >       }
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       if (ret)
> >               return ret;
> > diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
> > index a99859b5b13a..fb8234f4d128 100644
> > --- a/drivers/gpu/drm/gma500/psb_intel_display.c
> > +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
> > @@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
> >       u32 dpll = 0, fp = 0, dspcntr, pipeconf;
> >       bool ok, is_sdvo = false;
> >       bool is_lvds = false, is_tv = false;
> > -     struct drm_mode_config *mode_config = &dev->mode_config;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> >       const struct gma_limit_t *limit;
> >
> > @@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
> >               return 0;
> >       }
> >
> > -     list_for_each_entry(connector, &mode_config->connector_list, head) {
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> >               struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
> >
> >               if (!connector->encoder
> > @@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
> >                       break;
> >               }
> >       }
> > +     drm_connector_list_iter_end(&conn_iter);
> >
> >       refclk = 96000;
> >
> > @@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)
> >
> >  int gma_connector_clones(struct drm_device *dev, int type_mask)
> >  {
> > -     int index_mask = 0;
> > +     struct drm_connector_list_iter conn_iter;
> >       struct drm_connector *connector;
> > +     int index_mask = 0;
> >       int entry = 0;
> >
> > -     list_for_each_entry(connector, &dev->mode_config.connector_list,
> > -                         head) {
> > +     drm_connector_list_iter_begin(dev, &conn_iter);
> > +     drm_for_each_connector_iter(connector, &conn_iter) {
> >               struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
> >               if (type_mask & (1 << gma_encoder->type))
> >                       index_mask |= (1 << entry);
> >               entry++;
> >       }
> > +     drm_connector_list_iter_end(&conn_iter);
> > +
> >       return index_mask;
> >  }
> > --
> > 2.35.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Re: [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init()

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1317 bytes --]

Hi Patrik

Am 22.03.22 um 14:17 schrieb Patrik Jakobsson:
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
>   drivers/gpu/drm/gma500/framebuffer.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
> index 45df9de22007..2b99c996fdc2 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev)
>   	struct pci_dev *pdev = to_pci_dev(dev->dev);
>   	int i;
>   
> -	drm_mode_config_init(dev);
> +	if (drmm_mode_config_init(dev))
> +		return;

This will fail without any notice. I suggest to return an error here or 
at let psb_driver_load() fail.

Best regards
Thomas

>   
>   	dev->mode_config.min_width = 0;
>   	dev->mode_config.min_height = 0;
> @@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev)
>   	if (dev_priv->modeset) {
>   		drm_kms_helper_poll_fini(dev);
>   		psb_fbdev_fini(dev);
> -		drm_mode_config_cleanup(dev);
>   	}
>   }

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init()

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1567 bytes --]



Am 22.03.22 um 20:30 schrieb Thomas Zimmermann:
> Hi Patrik
> 
> Am 22.03.22 um 14:17 schrieb Patrik Jakobsson:
>> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>> ---
>>   drivers/gpu/drm/gma500/framebuffer.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
>> b/drivers/gpu/drm/gma500/framebuffer.c
>> index 45df9de22007..2b99c996fdc2 100644
>> --- a/drivers/gpu/drm/gma500/framebuffer.c
>> +++ b/drivers/gpu/drm/gma500/framebuffer.c
>> @@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev)
>>       struct pci_dev *pdev = to_pci_dev(dev->dev);
>>       int i;
>> -    drm_mode_config_init(dev);
>> +    if (drmm_mode_config_init(dev))
>> +        return;
> 
> This will fail without any notice. I suggest to return an error here or 
> at let psb_driver_load() fail.

'and let psb_driver_load() fail'

> 
> Best regards
> Thomas
> 
>>       dev->mode_config.min_width = 0;
>>       dev->mode_config.min_height = 0;
>> @@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev)
>>       if (dev_priv->modeset) {
>>           drm_kms_helper_poll_fini(dev);
>>           psb_fbdev_fini(dev);
>> -        drm_mode_config_cleanup(dev);
>>       }
>>   }
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 21226 bytes --]

Hi

Am 22.03.22 um 16:46 schrieb Patrik Jakobsson:
> On Tue, Mar 22, 2022 at 3:39 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>>
>> On Tue, Mar 22, 2022 at 02:17:38PM +0100, Patrik Jakobsson wrote:
>>> This makes sure we're using proper locking when iterating the list of
>>> connectors.
>>>
>>> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>>
>> Note that this is only needed if your driver deals with hotpluggable
>> connectors. Since gma500 doesn't, not really a need to convert this over,
>> but it also doesn't hurt.

I'd fix this if only for copy-pasters to not copy incorrect code. :)

> 
> Good point. Not sure but I think there is a slight possibility that
> Cedarview can support DP MST. If someone adds support for DP MST then
> this code would make sense. I was never able to exercise the DP code
> because I couldn't find an eDP cable that fits the Intel DN2800MT (my
> only device with DP). Do you happen to know what the 40-pin connector
> is called? Perhaps Intel has some standard for eDP connectors?
> 
>>
>> If the kerneldoc doesn't explain this, maybe we should add it? Care for a
>> patch.
> 
> I didn't see it being mentioned anywhere. I'll send a patch.
> 
>>
>> Either way on the entire series:
>>
>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Thanks for having a look.
> 
>>
>> I'll leave it up to you whether you want to push this one too or not.
>> -Daniel
>>
>>> ---
>>>   drivers/gpu/drm/gma500/cdv_device.c        | 10 ++++++--
>>>   drivers/gpu/drm/gma500/cdv_intel_display.c |  9 +++++--
>>>   drivers/gpu/drm/gma500/framebuffer.c       |  6 +++--
>>>   drivers/gpu/drm/gma500/gma_display.c       | 16 ++++++++-----
>>>   drivers/gpu/drm/gma500/oaktrail_crtc.c     | 17 ++++++++-----
>>>   drivers/gpu/drm/gma500/oaktrail_lvds.c     | 15 ++++++------
>>>   drivers/gpu/drm/gma500/psb_device.c        | 28 +++++++++++++++-------
>>>   drivers/gpu/drm/gma500/psb_drv.c           | 10 ++++----
>>>   drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++----
>>>   9 files changed, 84 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c
>>> index f854f58bcbb3..dd32b484dd82 100644
>>> --- a/drivers/gpu/drm/gma500/cdv_device.c
>>> +++ b/drivers/gpu/drm/gma500/cdv_device.c
>>> @@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev)
>>>        struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>>        struct pci_dev *pdev = to_pci_dev(dev->dev);
>>>        struct psb_save_area *regs = &dev_priv->regs;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>>
>>>        dev_dbg(dev->dev, "Saving GPU registers.\n");
>>> @@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev)
>>>        regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R);
>>>        regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R);
>>>
>>> -     list_for_each_entry(connector, &dev->mode_config.connector_list, head)
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter)
>>>                connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        return 0;
>>>   }
>>> @@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev)
>>>        struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>>        struct pci_dev *pdev = to_pci_dev(dev->dev);
>>>        struct psb_save_area *regs = &dev_priv->regs;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>>        u32 temp;
>>>
>>> @@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev)
>>>
>>>        drm_mode_config_reset(dev);
>>>
>>> -     list_for_each_entry(connector, &dev->mode_config.connector_list, head)
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter)
>>>                connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        /* Resume the modeset for every activated CRTC */
>>>        drm_helper_resume_force_mode(dev);
>>> diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c
>>> index 94ebc48a4349..0c3ddcdc29dc 100644
>>> --- a/drivers/gpu/drm/gma500/cdv_intel_display.c
>>> +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
>>> @@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
>>>        bool ok;
>>>        bool is_lvds = false;
>>>        bool is_dp = false;
>>> -     struct drm_mode_config *mode_config = &dev->mode_config;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>>        const struct gma_limit_t *limit;
>>>        u32 ddi_select = 0;
>>>        bool is_edp = false;
>>>
>>> -     list_for_each_entry(connector, &mode_config->connector_list, head) {
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>>                struct gma_encoder *gma_encoder =
>>>                                        gma_attached_encoder(connector);
>>>
>>> @@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc,
>>>                        is_edp = true;
>>>                        break;
>>>                default:
>>> +                     drm_connector_list_iter_end(&conn_iter);
>>>                        DRM_ERROR("invalid output type.\n");
>>>                        return 0;
>>>                }
>>> +
>>> +             break;
>>>        }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        if (dev_priv->dplla_96mhz)
>>>                /* low-end sku, 96/100 mhz */
>>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
>>> index 2b99c996fdc2..0ac6ea5fd3a1 100644
>>> --- a/drivers/gpu/drm/gma500/framebuffer.c
>>> +++ b/drivers/gpu/drm/gma500/framebuffer.c
>>> @@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = {
>>>   static void psb_setup_outputs(struct drm_device *dev)
>>>   {
>>>        struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>>
>>>        drm_mode_create_scaling_mode_property(dev);
>>> @@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev)
>>>                                                        "backlight", 0, 100);
>>>        dev_priv->ops->output_init(dev);
>>>
>>> -     list_for_each_entry(connector, &dev->mode_config.connector_list,
>>> -                         head) {
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>>                struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
>>>                struct drm_encoder *encoder = &gma_encoder->base;
>>>                int crtc_mask = 0, clone_mask = 0;
>>> @@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev)
>>>                encoder->possible_clones =
>>>                    gma_connector_clones(dev, clone_mask);
>>>        }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>   }
>>>
>>>   void psb_modeset_init(struct drm_device *dev)
>>> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
>>> index 1d7964c339f4..e8157464d9eb 100644
>>> --- a/drivers/gpu/drm/gma500/gma_display.c
>>> +++ b/drivers/gpu/drm/gma500/gma_display.c
>>> @@ -27,17 +27,21 @@
>>>   bool gma_pipe_has_type(struct drm_crtc *crtc, int type)
>>>   {
>>>        struct drm_device *dev = crtc->dev;
>>> -     struct drm_mode_config *mode_config = &dev->mode_config;
>>> -     struct drm_connector *l_entry;
>>> +     struct drm_connector_list_iter conn_iter;
>>> +     struct drm_connector *connector;
>>>
>>> -     list_for_each_entry(l_entry, &mode_config->connector_list, head) {
>>> -             if (l_entry->encoder && l_entry->encoder->crtc == crtc) {
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>> +             if (connector->encoder && connector->encoder->crtc == crtc) {
>>>                        struct gma_encoder *gma_encoder =
>>> -                                             gma_attached_encoder(l_entry);
>>> -                     if (gma_encoder->type == type)
>>> +                                             gma_attached_encoder(connector);
>>> +                     if (gma_encoder->type == type) {
>>> +                             drm_connector_list_iter_end(&conn_iter);
>>>                                return true;
>>> +                     }
>>>                }
>>>        }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        return false;
>>>   }
>>> diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c
>>> index 36c7c2686c90..873c17cf8fb4 100644
>>> --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
>>> +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
>>> @@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>>>        bool ok, is_sdvo = false;
>>>        bool is_lvds = false;
>>>        bool is_mipi = false;
>>> -     struct drm_mode_config *mode_config = &dev->mode_config;
>>>        struct gma_encoder *gma_encoder = NULL;
>>>        uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>>        int i;
>>>        int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0;
>>> @@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>>>                adjusted_mode,
>>>                sizeof(struct drm_display_mode));
>>>
>>> -     list_for_each_entry(connector, &mode_config->connector_list, head) {
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>>                if (!connector->encoder || connector->encoder->crtc != crtc)
>>>                        continue;
>>>
>>> @@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>>>                        is_mipi = true;
>>>                        break;
>>>                }
>>> +
>>> +             break;
>>>        }
>>>
>>> +     if (gma_encoder)
>>> +             drm_object_property_get_value(&connector->base,
>>> +                     dev->mode_config.scaling_mode_property, &scalingType);
>>> +
>>> +     drm_connector_list_iter_end(&conn_iter);
>>> +
>>>        /* Disable the VGA plane that we never use */
>>>        for (i = 0; i <= need_aux; i++)
>>>                REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i);
>>> @@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc,
>>>                                             (mode->crtc_vdisplay - 1), i);
>>>        }
>>>
>>> -     if (gma_encoder)
>>> -             drm_object_property_get_value(&connector->base,
>>> -                     dev->mode_config.scaling_mode_property, &scalingType);
>>> -
>>>        if (scalingType == DRM_MODE_SCALE_NO_SCALE) {
>>>                /* Moorestown doesn't have register support for centering so
>>>                 * we need to mess with the h/vblank and h/vsync start and
>>> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
>>> index 28b995ef2844..04852dbc7fb3 100644
>>> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
>>> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
>>> @@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
>>>        struct drm_device *dev = encoder->dev;
>>>        struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>>        struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
>>> -     struct drm_mode_config *mode_config = &dev->mode_config;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector = NULL;
>>>        struct drm_crtc *crtc = encoder->crtc;
>>>        u32 lvds_port;
>>> @@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
>>>        REG_WRITE(LVDS, lvds_port);
>>>
>>>        /* Find the connector we're trying to set up */
>>> -     list_for_each_entry(connector, &mode_config->connector_list, head) {
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>>                if (connector->encoder && connector->encoder->crtc == crtc)
>>>                        break;
>>>        }
>>>
>>> -     if (list_entry_is_head(connector, &mode_config->connector_list, head)) {
>>> +     if (!connector) {
>>> +             drm_connector_list_iter_end(&conn_iter);
>>>                DRM_ERROR("Couldn't find connector when setting mode");
>>>                gma_power_end(dev);
>>>                return;
>>>        }
>>>
>>> -     drm_object_property_get_value(
>>> -             &connector->base,
>>> -             dev->mode_config.scaling_mode_property,
>>> -             &v);
>>> +     drm_object_property_get_value( &connector->base,
>>> +             dev->mode_config.scaling_mode_property, &v);
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        if (v == DRM_MODE_SCALE_NO_SCALE)
>>>                REG_WRITE(PFIT_CONTROL, 0);
>>> diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
>>> index 59f325165667..71534f4ca834 100644
>>> --- a/drivers/gpu/drm/gma500/psb_device.c
>>> +++ b/drivers/gpu/drm/gma500/psb_device.c
>>> @@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev)
>>>   static int psb_save_display_registers(struct drm_device *dev)
>>>   {
>>>        struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>> +     struct gma_connector *gma_connector;
>>>        struct drm_crtc *crtc;
>>> -     struct gma_connector *connector;
>>> +     struct drm_connector_list_iter conn_iter;
>>> +     struct drm_connector *connector;
>>>        struct psb_state *regs = &dev_priv->regs.psb;
>>>
>>>        /* Display arbitration control + watermarks */
>>> @@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev)
>>>                        dev_priv->ops->save_crtc(crtc);
>>>        }
>>>
>>> -     list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
>>> -             if (connector->save)
>>> -                     connector->save(&connector->base);
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>> +             gma_connector = to_gma_connector(connector);
>>> +             if (gma_connector->save)
>>> +                     gma_connector->save(connector);
>>> +     }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        drm_modeset_unlock_all(dev);
>>>        return 0;
>>> @@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev)
>>>   static int psb_restore_display_registers(struct drm_device *dev)
>>>   {
>>>        struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>> +     struct gma_connector *gma_connector;
>>>        struct drm_crtc *crtc;
>>> -     struct gma_connector *connector;
>>> +     struct drm_connector_list_iter conn_iter;
>>> +     struct drm_connector *connector;
>>>        struct psb_state *regs = &dev_priv->regs.psb;
>>>
>>>        /* Display arbitration + watermarks */
>>> @@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev)
>>>                if (drm_helper_crtc_in_use(crtc))
>>>                        dev_priv->ops->restore_crtc(crtc);
>>>
>>> -     list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
>>> -             if (connector->restore)
>>> -                     connector->restore(&connector->base);
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>> +             gma_connector = to_gma_connector(connector);
>>> +             if (gma_connector->restore)
>>> +                     gma_connector->restore(connector);
>>> +     }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        drm_modeset_unlock_all(dev);
>>>        return 0;
>>> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
>>> index b231fddb8817..bb0e3288e35b 100644
>>> --- a/drivers/gpu/drm/gma500/psb_drv.c
>>> +++ b/drivers/gpu/drm/gma500/psb_drv.c
>>> @@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
>>>        struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>>        unsigned long resource_start, resource_len;
>>>        unsigned long irqflags;
>>> -     int ret = -ENOMEM;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>>        struct gma_encoder *gma_encoder;
>>>        struct psb_gtt *pg;
>>> +     int ret = -ENOMEM;
>>>
>>>        /* initializing driver private data */
>>>
>>> @@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
>>>        psb_fbdev_init(dev);
>>>        drm_kms_helper_poll_init(dev);
>>>
>>> -     /* Only add backlight support if we have LVDS output */
>>> -     list_for_each_entry(connector, &dev->mode_config.connector_list,
>>> -                         head) {
>>> +     /* Only add backlight support if we have LVDS or MIPI output */
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>>                gma_encoder = gma_attached_encoder(connector);
>>>
>>>                switch (gma_encoder->type) {
>>> @@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
>>>                        break;
>>>                }
>>>        }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        if (ret)
>>>                return ret;
>>> diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
>>> index a99859b5b13a..fb8234f4d128 100644
>>> --- a/drivers/gpu/drm/gma500/psb_intel_display.c
>>> +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
>>> @@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
>>>        u32 dpll = 0, fp = 0, dspcntr, pipeconf;
>>>        bool ok, is_sdvo = false;
>>>        bool is_lvds = false, is_tv = false;
>>> -     struct drm_mode_config *mode_config = &dev->mode_config;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>>        const struct gma_limit_t *limit;
>>>
>>> @@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
>>>                return 0;
>>>        }
>>>
>>> -     list_for_each_entry(connector, &mode_config->connector_list, head) {
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>>                struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
>>>
>>>                if (!connector->encoder
>>> @@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc,
>>>                        break;
>>>                }
>>>        }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>>
>>>        refclk = 96000;
>>>
>>> @@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)
>>>
>>>   int gma_connector_clones(struct drm_device *dev, int type_mask)
>>>   {
>>> -     int index_mask = 0;
>>> +     struct drm_connector_list_iter conn_iter;
>>>        struct drm_connector *connector;
>>> +     int index_mask = 0;
>>>        int entry = 0;
>>>
>>> -     list_for_each_entry(connector, &dev->mode_config.connector_list,
>>> -                         head) {
>>> +     drm_connector_list_iter_begin(dev, &conn_iter);
>>> +     drm_for_each_connector_iter(connector, &conn_iter) {
>>>                struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
>>>                if (type_mask & (1 << gma_encoder->type))
>>>                        index_mask |= (1 << entry);
>>>                entry++;
>>>        }
>>> +     drm_connector_list_iter_end(&conn_iter);
>>> +
>>>        return index_mask;
>>>   }
>>> --
>>> 2.35.1
>>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3651 bytes --]

Hi

Am 22.03.22 um 14:17 schrieb Patrik Jakobsson:
> These functions operate on a crtc and should be prefixed properly.
> 
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

For the whole patchset:

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

If you like, please consider the few comments I had.

Best regards
Thomas


> ---
>   drivers/gpu/drm/gma500/gma_display.c | 6 +++---
>   drivers/gpu/drm/gma500/psb_irq.c     | 6 +++---
>   drivers/gpu/drm/gma500/psb_irq.h     | 6 +++---
>   3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> index 369bc1f751cb..34ec3fca09ba 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -575,9 +575,9 @@ const struct drm_crtc_funcs gma_crtc_funcs = {
>   	.set_config = gma_crtc_set_config,
>   	.destroy = gma_crtc_destroy,
>   	.page_flip = gma_crtc_page_flip,
> -	.enable_vblank = gma_enable_vblank,
> -	.disable_vblank = gma_disable_vblank,
> -	.get_vblank_counter = gma_get_vblank_counter,
> +	.enable_vblank = gma_crtc_enable_vblank,
> +	.disable_vblank = gma_crtc_disable_vblank,
> +	.get_vblank_counter = gma_crtc_get_vblank_counter,
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c
> index 2e8ded532930..e6e6d61bbeab 100644
> --- a/drivers/gpu/drm/gma500/psb_irq.c
> +++ b/drivers/gpu/drm/gma500/psb_irq.c
> @@ -371,7 +371,7 @@ void gma_irq_uninstall(struct drm_device *dev)
>   	free_irq(pdev->irq, dev);
>   }
>   
> -int gma_enable_vblank(struct drm_crtc *crtc)
> +int gma_crtc_enable_vblank(struct drm_crtc *crtc)
>   {
>   	struct drm_device *dev = crtc->dev;
>   	unsigned int pipe = crtc->index;
> @@ -404,7 +404,7 @@ int gma_enable_vblank(struct drm_crtc *crtc)
>   	return 0;
>   }
>   
> -void gma_disable_vblank(struct drm_crtc *crtc)
> +void gma_crtc_disable_vblank(struct drm_crtc *crtc)
>   {
>   	struct drm_device *dev = crtc->dev;
>   	unsigned int pipe = crtc->index;
> @@ -428,7 +428,7 @@ void gma_disable_vblank(struct drm_crtc *crtc)
>   /* Called from drm generic code, passed a 'crtc', which
>    * we use as a pipe index
>    */
> -u32 gma_get_vblank_counter(struct drm_crtc *crtc)
> +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc)
>   {
>   	struct drm_device *dev = crtc->dev;
>   	unsigned int pipe = crtc->index;
> diff --git a/drivers/gpu/drm/gma500/psb_irq.h b/drivers/gpu/drm/gma500/psb_irq.h
> index c22878914f5b..b51e395194ff 100644
> --- a/drivers/gpu/drm/gma500/psb_irq.h
> +++ b/drivers/gpu/drm/gma500/psb_irq.h
> @@ -20,9 +20,9 @@ void gma_irq_postinstall(struct drm_device *dev);
>   int  gma_irq_install(struct drm_device *dev, unsigned int irq);
>   void gma_irq_uninstall(struct drm_device *dev);
>   
> -int  gma_enable_vblank(struct drm_crtc *crtc);
> -void gma_disable_vblank(struct drm_crtc *crtc);
> -u32  gma_get_vblank_counter(struct drm_crtc *crtc);
> +int  gma_crtc_enable_vblank(struct drm_crtc *crtc);
> +void gma_crtc_disable_vblank(struct drm_crtc *crtc);
> +u32  gma_crtc_get_vblank_counter(struct drm_crtc *crtc);
>   void gma_enable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask);
>   void gma_disable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask);
>   

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init()

[Patrik Jakobsson]

On Tue, Mar 22, 2022 at 8:32 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>
>
> Am 22.03.22 um 20:30 schrieb Thomas Zimmermann:
> > Hi Patrik
> >
> > Am 22.03.22 um 14:17 schrieb Patrik Jakobsson:
> >> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> >> ---
> >>   drivers/gpu/drm/gma500/framebuffer.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/gma500/framebuffer.c
> >> b/drivers/gpu/drm/gma500/framebuffer.c
> >> index 45df9de22007..2b99c996fdc2 100644
> >> --- a/drivers/gpu/drm/gma500/framebuffer.c
> >> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> >> @@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev)
> >>       struct pci_dev *pdev = to_pci_dev(dev->dev);
> >>       int i;
> >> -    drm_mode_config_init(dev);
> >> +    if (drmm_mode_config_init(dev))
> >> +        return;
> >
> > This will fail without any notice. I suggest to return an error here or
> > at let psb_driver_load() fail.
>
> 'and let psb_driver_load() fail'

Hi Thomas,
I did consider it but there are more places where psb_driver_load()
can fail so I think it deserves its own patch.

I'll send a follow-up.

-Patrik

>
> >
> > Best regards
> > Thomas
> >
> >>       dev->mode_config.min_width = 0;
> >>       dev->mode_config.min_height = 0;
> >> @@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev)
> >>       if (dev_priv->modeset) {
> >>           drm_kms_helper_poll_fini(dev);
> >>           psb_fbdev_fini(dev);
> >> -        drm_mode_config_cleanup(dev);
> >>       }
> >>   }
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev

Re: [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions

[Patrik Jakobsson]

On Tue, Mar 22, 2022 at 8:36 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 22.03.22 um 14:17 schrieb Patrik Jakobsson:
> > These functions operate on a crtc and should be prefixed properly.
> >
> > Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>
> For the whole patchset:
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> If you like, please consider the few comments I had.

Thanks for the review.

>
> Best regards
> Thomas
>
>
> > ---
> >   drivers/gpu/drm/gma500/gma_display.c | 6 +++---
> >   drivers/gpu/drm/gma500/psb_irq.c     | 6 +++---
> >   drivers/gpu/drm/gma500/psb_irq.h     | 6 +++---
> >   3 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> > index 369bc1f751cb..34ec3fca09ba 100644
> > --- a/drivers/gpu/drm/gma500/gma_display.c
> > +++ b/drivers/gpu/drm/gma500/gma_display.c
> > @@ -575,9 +575,9 @@ const struct drm_crtc_funcs gma_crtc_funcs = {
> >       .set_config = gma_crtc_set_config,
> >       .destroy = gma_crtc_destroy,
> >       .page_flip = gma_crtc_page_flip,
> > -     .enable_vblank = gma_enable_vblank,
> > -     .disable_vblank = gma_disable_vblank,
> > -     .get_vblank_counter = gma_get_vblank_counter,
> > +     .enable_vblank = gma_crtc_enable_vblank,
> > +     .disable_vblank = gma_crtc_disable_vblank,
> > +     .get_vblank_counter = gma_crtc_get_vblank_counter,
> >   };
> >
> >   /*
> > diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c
> > index 2e8ded532930..e6e6d61bbeab 100644
> > --- a/drivers/gpu/drm/gma500/psb_irq.c
> > +++ b/drivers/gpu/drm/gma500/psb_irq.c
> > @@ -371,7 +371,7 @@ void gma_irq_uninstall(struct drm_device *dev)
> >       free_irq(pdev->irq, dev);
> >   }
> >
> > -int gma_enable_vblank(struct drm_crtc *crtc)
> > +int gma_crtc_enable_vblank(struct drm_crtc *crtc)
> >   {
> >       struct drm_device *dev = crtc->dev;
> >       unsigned int pipe = crtc->index;
> > @@ -404,7 +404,7 @@ int gma_enable_vblank(struct drm_crtc *crtc)
> >       return 0;
> >   }
> >
> > -void gma_disable_vblank(struct drm_crtc *crtc)
> > +void gma_crtc_disable_vblank(struct drm_crtc *crtc)
> >   {
> >       struct drm_device *dev = crtc->dev;
> >       unsigned int pipe = crtc->index;
> > @@ -428,7 +428,7 @@ void gma_disable_vblank(struct drm_crtc *crtc)
> >   /* Called from drm generic code, passed a 'crtc', which
> >    * we use as a pipe index
> >    */
> > -u32 gma_get_vblank_counter(struct drm_crtc *crtc)
> > +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc)
> >   {
> >       struct drm_device *dev = crtc->dev;
> >       unsigned int pipe = crtc->index;
> > diff --git a/drivers/gpu/drm/gma500/psb_irq.h b/drivers/gpu/drm/gma500/psb_irq.h
> > index c22878914f5b..b51e395194ff 100644
> > --- a/drivers/gpu/drm/gma500/psb_irq.h
> > +++ b/drivers/gpu/drm/gma500/psb_irq.h
> > @@ -20,9 +20,9 @@ void gma_irq_postinstall(struct drm_device *dev);
> >   int  gma_irq_install(struct drm_device *dev, unsigned int irq);
> >   void gma_irq_uninstall(struct drm_device *dev);
> >
> > -int  gma_enable_vblank(struct drm_crtc *crtc);
> > -void gma_disable_vblank(struct drm_crtc *crtc);
> > -u32  gma_get_vblank_counter(struct drm_crtc *crtc);
> > +int  gma_crtc_enable_vblank(struct drm_crtc *crtc);
> > +void gma_crtc_disable_vblank(struct drm_crtc *crtc);
> > +u32  gma_crtc_get_vblank_counter(struct drm_crtc *crtc);
> >   void gma_enable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask);
> >   void gma_disable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask);
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev