Re: [PATCH] drm: don't call ->firstopen for KMS drivers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Wednesday 10 July 2013 20:17:44 Daniel Vetter wrote:
> It has way too much potential for driver writers to do stupid things
> like delayed hw setup because the load sequence is somehow racy (e.g.
> the imx driver in staging). So don't call it for modesetting drivers,
> which reduces the complexity of the drm core -> driver interface a
> notch.
> 
> v2: Don't forget to update DocBook.
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/DocBook/drm.tmpl | 2 ++
>  drivers/gpu/drm/drm_fops.c     | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 4d54ac8..0e8a5a3 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2423,6 +2423,8 @@ void (*postclose) (struct drm_device *, struct
> drm_file *);</synopsis> <para>
>          The <methodname>firstopen</methodname> method is called by the DRM
> core when an application opens a device that has no other opened file
> handle.
> +	Not that this callback is only called for legacy ums drm drivers, not
> +	for drm drivers that implement modesetting in the kernel.
>  	Similarly the <methodname>lastclose</methodname> method is called when
>  	the last application holding a file handle opened on the device closes
>  	it. Both methods are mostly used for UMS (User Mode Setting) drivers to

What about

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7d1278e..afa8d40 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2422,18 +2422,18 @@ void (*postclose) (struct drm_device *, struct drm_file *);</synopsis>
       </abstract>
       <para>
         The <methodname>firstopen</methodname> method is called by the DRM core
-	when an application opens a device that has no other opened file handle.
-	Similarly the <methodname>lastclose</methodname> method is called when
-	the last application holding a file handle opened on the device closes
-	it. Both methods are mostly used for UMS (User Mode Setting) drivers to
-	acquire and release device resources which should be done in the
-	<methodname>load</methodname> and <methodname>unload</methodname>
-	methods for KMS drivers.
+	for legacy UMS (User Mode Setting) drivers only when an application
+	opens a device that has no other opened file handle. UMS drivers can
+	implement it to acquire device resources. KMS drivers can't use the
+	method and must acquire resources in the <methodname>load</methodname>
+	method instead.
       </para>
       <para>
-        Note that the <methodname>lastclose</methodname> method is also called
-	at module unload time or, for hot-pluggable devices, when the device is
-	unplugged. The <methodname>firstopen</methodname> and
+	Similarly the <methodname>lastclose</methodname> method is called when
+	the last application holding a file handle opened on the device closes
+	it, for both UMS and KMS drivers. Additionally, the method is also
+	called at module unload time or, for hot-pluggable devices, when the
+	device is unplugged. The <methodname>firstopen</methodname> and
 	<methodname>lastclose</methodname> calls can thus be unbalanced.
       </para>
       <para>

> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 57e3014..fcde7d4 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -51,7 +51,8 @@ static int drm_setup(struct drm_device * dev)
>  	int i;
>  	int ret;
> 
> -	if (dev->driver->firstopen) {
> +	if (dev->driver->firstopen &&
> +	    !drm_core_check_feature(dev, DRIVER_MODESET)) {
>  		ret = dev->driver->firstopen(dev);
>  		if (ret != 0)
>  			return ret;

-- 
Regards,

Laurent Pinchart