From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH] drm: don't call ->firstopen for KMS drivers Date: Thu, 11 Jul 2013 09:54:55 +0200 Message-ID: <2771673.RHCmxbM7cy@avalon> References: <1373458333-5988-14-git-send-email-daniel.vetter@ffwll.ch> <1373480264-10243-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [95.142.166.194]) by gabe.freedesktop.org (Postfix) with ESMTP id 9EC01E5D75 for ; Thu, 11 Jul 2013 00:54:38 -0700 (PDT) In-Reply-To: <1373480264-10243-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: DRI Development List-Id: dri-devel@lists.freedesktop.org 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 > Signed-off-by: Daniel Vetter > --- > 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 *); > The firstopen 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 lastclose 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 *); The firstopen method is called by the DRM core - when an application opens a device that has no other opened file handle. - Similarly the lastclose 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 - load and unload - 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 load + method instead. - Note that the lastclose method is also called - at module unload time or, for hot-pluggable devices, when the device is - unplugged. The firstopen and + Similarly the lastclose 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 firstopen and lastclose calls can thus be unbalanced. > 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