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: Mon, 22 Jul 2013 11:02:48 +0200 Message-ID: <1733671.WOoQMfelD9@avalon> References: <1373726710-28891-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 6A982E5F4F for ; Mon, 22 Jul 2013 02:02:00 -0700 (PDT) In-Reply-To: <1373726710-28891-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, Thank you for the patch. On Saturday 13 July 2013 16:45:10 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. > > v3: Go with Laurent's slightly more elaborate proposal for the DocBook > update. Add a few words on top of his diff to elaborate a bit on what > KMS drivers should and shouldn't do in lastclose. There was already a > paragraph present talking about restoring properties, I've simply > extended that one. > > Cc: Laurent Pinchart > Signed-off-by: Daniel Vetter Acked-by: Laurent Pinchart > --- > Documentation/DocBook/drm.tmpl | 27 ++++++++++++++++----------- > drivers/gpu/drm/drm_fops.c | 3 ++- > 2 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 4d54ac8..52d5eda 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. > > > @@ -2462,7 +2462,12 @@ void (*postclose) (struct drm_device *, struct > drm_file *); > The lastclose method should restore CRTC > and plane properties to default value, so that a subsequent open of the > - device will not inherit state from the previous user. > + device will not inherit state from the previous user. It can also be > + used to execute delayed power switching state changes, e.g. in > + conjunction with the vga-switcheroo infrastructure. Beyond that KMS > + drivers should not do any further cleanup. Only legacy UMS drivers might > + need to clean up device state so that the vga console or an independent > + fbdev driver could take over. > > > > 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