| Message ID | 1373726710-28891-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <laurent.pinchart@ideasonboard.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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 *);</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> > @@ -2462,7 +2462,12 @@ void (*postclose) (struct drm_device *, struct > drm_file *);</synopsis> <para> > The <methodname>lastclose</methodname> 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. > </para> > </sect2> > <sect2> > 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;
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 *);</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> @@ -2462,7 +2462,12 @@ void (*postclose) (struct drm_device *, struct drm_file *);</synopsis> <para> The <methodname>lastclose</methodname> 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. </para> </sect2> <sect2> 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;
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 <laurent.pinchart@ideasonboard.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- Documentation/DocBook/drm.tmpl | 27 ++++++++++++++++----------- drivers/gpu/drm/drm_fops.c | 3 ++- 2 files changed, 18 insertions(+), 12 deletions(-)