linux-kernel - Re: [PATCH v12 1/3] drm: rockchip: Add basic drm driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20141119131758.GN25711@phenom.ffwll.local>
Date:	Wed, 19 Nov 2014 14:17:58 +0100
From:	Daniel Vetter <daniel@...ll.ch>
To:	Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc:	Mark yao <mark.yao@...k-chips.com>,
	Mark Rutland <mark.rutland@....com>, heiko@...ech.de,
	linux-doc@...r.kernel.org, kever.yang@...k-chips.com,
	dri-devel@...ts.freedesktop.org, dianders@...omium.org,
	linux-api@...r.kernel.org, xxm@...k-chips.com,
	linux-rockchip@...ts.infradead.org,
	Grant Likely <grant.likely@...aro.org>,
	huangtao@...k-chips.com, devicetree@...r.kernel.org,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	yxj@...k-chips.com, cf@...k-chips.com,
	Rob Herring <robh+dt@...nel.org>, marcheu@...omium.org,
	xw@...k-chips.com, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Randy Dunlap <rdunlap@...radead.org>,
	linux-kernel@...r.kernel.org, Kumar Gala <galak@...eaurora.org>
Subject: Re: [PATCH v12 1/3] drm: rockchip: Add basic drm driver

On Wed, Nov 19, 2014 at 10:04:13AM +0100, Boris Brezillon wrote:
> AFAIU, the suggestion was to split drm_connector_init and
> drm_connector_register calls:
>  - drm_connector_init call should still be part of the load procedure
>    (this function adds the connector to the connector list which is used
>    by drm_mode_group_init_legacy_group)
>  - drm_connector_register should be called after the device has been
>    registered
> 
> Here what I've done and it seems to work:
> 
> static int atmel_hlcdc_dc_connector_plug_all(struct drm_device *dev)
> {
> 	struct drm_connector *connector, *failed;
> 	int ret;
> 
> 	mutex_lock(&dev->mode_config.mutex);
> 	list_for_each_entry(connector,
> 	&dev->mode_config.connector_list, head) { ret =
> 	drm_connector_register(connector); if (ret) {
> 			failed = connector;
> 			goto err;
> 		}
> 	}
> 	mutex_unlock(&dev->mode_config.mutex);
> 	return 0;
> 
> err:
> 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> 		if (failed == connector)
> 			break;
> 
> 		drm_connector_unregister(connector);
> 	}
> 	mutex_unlock(&dev->mode_config.mutex);
> 
> 	return ret;
> }
> 
> [...]
> 
> static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
> {
> 	struct drm_device *ddev;
> 	int ret;
> 
> 	ddev = drm_dev_alloc(&atmel_hlcdc_dc_driver, &pdev->dev);
> 	if (!ddev)
> 		return -ENOMEM;
> 
> 	ret = drm_dev_set_unique(ddev, dev_name(ddev->dev));
> 	if (ret)
> 		goto err_unref;
> 
> 	ret = atmel_hlcdc_dc_load(ddev);
> 	if (ret)
> 		goto err_unref;
> 
> 	ret = drm_dev_register(ddev, 0);
> 	if (ret)
> 		goto err_unload;
> 
> 	ret = atmel_hlcdc_dc_connector_plug_all(ddev);
> 	if (ret)
> 		goto err_unregister;
> 
> 	return 0;
> 
> err_unregister:
> 	drm_dev_unregister(ddev);
> 
> err_unload:
> 	atmel_hlcdc_dc_unload(ddev);
> 
> err_unref:
> 	drm_dev_unref(ddev);
> 
> 	return ret;
> }
> 
> Daniel, can you confirm that's what you had in mind ?

Yup. To be able to have race-free driver load we need to split object
into an _init step (allocates structs and links to kernel-internal lists)
and _register (makes the object userspace-visible through sysfs and
dev-node kms object lookup idr).

This entire mess is all still fallout from the dark ages of the drm
midlayer and we'll probably have fun with this for another few years ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/