From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm: don't call ->firstopen for KMS drivers Date: Thu, 11 Jul 2013 12:19:44 +0200 Message-ID: References: <1373458333-5988-14-git-send-email-daniel.vetter@ffwll.ch> <1373480264-10243-1-git-send-email-daniel.vetter@ffwll.ch> <2771673.RHCmxbM7cy@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ie0-f169.google.com (mail-ie0-f169.google.com [209.85.223.169]) by gabe.freedesktop.org (Postfix) with ESMTP id 37E21E64BD for ; Thu, 11 Jul 2013 03:19:45 -0700 (PDT) Received: by mail-ie0-f169.google.com with SMTP id 10so18032789ied.28 for ; Thu, 11 Jul 2013 03:19:44 -0700 (PDT) In-Reply-To: <2771673.RHCmxbM7cy@avalon> 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: Laurent Pinchart Cc: DRI Development List-Id: dri-devel@lists.freedesktop.org On Thu, Jul 11, 2013 at 9:54 AM, Laurent Pinchart wrote: > 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 So if you think we should go overboard I guess it'd be useful to explain what KMS drivers should and shouldn't do in lastclose: - Delayed gpu switching with vga switcheroo. - Restoring of the fbcon, as long as the core is still a bit deficient in getting rid of mouse cursors and stupid sprite setups when X dies untimely and leaves dirt behind. Eventually we should be able to get rid of this though and rely on the magic sysrq to get out of graphics mode and restore fbcon fully. - Nothing else, ever, especially resource cleanups. Can you maybe add a sentence or two to your proposed update for about this? UMS drivers tend to do a lot of nefarious stuff in lastclose, so stressing the difference would be good. Otherwise I like your doc update much more than mine ;-) -Daniel > > 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch