On Tue, Oct 31, 2017 at 03:40:40PM +0100, Noralf Trønnes wrote:
>
> Den 31.10.2017 11.32, skrev Daniel Vetter:
> > On Mon, Oct 30, 2017 at 04:39:51PM +0100, Noralf Trønnes wrote:
> > > This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
> > >
> > > Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> > > Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > > Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > > ---
> > > drivers/staging/vboxvideo/vbox_drv.c | 2 +-
> > > drivers/staging/vboxvideo/vbox_drv.h | 1 -
> > > drivers/staging/vboxvideo/vbox_main.c | 12 ------------
> > > 3 files changed, 1 insertion(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
> > > index e18642e5027e..a4d8d7898e3d 100644
> > > --- a/drivers/staging/vboxvideo/vbox_drv.c
> > > +++ b/drivers/staging/vboxvideo/vbox_drv.c
> > > @@ -229,7 +229,7 @@ static struct drm_driver driver = {
> > > .load = vbox_driver_load,
> > > .unload = vbox_driver_unload,
> > > - .lastclose = vbox_driver_lastclose,
> > > + .lastclose = drm_fb_helper_lastclose,
> > > .master_set = vbox_master_set,
> > > .master_drop = vbox_master_drop,
> > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h
> > > index 4b9302703b36..7273d7e9bc9b 100644
> > > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > > @@ -128,7 +128,6 @@ struct vbox_private {
> > > int vbox_driver_load(struct drm_device *dev, unsigned long flags);
> > > void vbox_driver_unload(struct drm_device *dev);
> > > -void vbox_driver_lastclose(struct drm_device *dev);
> > > struct vbox_gem_object;
> > > diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
> > > index 80bd039fa08e..c3d756620fd5 100644
> > > --- a/drivers/staging/vboxvideo/vbox_main.c
> > > +++ b/drivers/staging/vboxvideo/vbox_main.c
> > > @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev)
> > > vbox_hw_fini(vbox);
> > > }
> > > -/**
> > > - * @note this is described in the DRM framework documentation. AST does not
> > > - * have it, but we get an oops on driver unload if it is not present.
> > > - */
> > > -void vbox_driver_lastclose(struct drm_device *dev)
> > > -{
> > > - struct vbox_private *vbox = dev->dev_private;
> > > -
> > > - if (vbox->fbdev)
> > Hm, except gma500 all the drivers have this NULL check here. I'm not sure
> > whether that's just cargo-culted or whether that's needed, but I'm wary
> > from slapping an Ack on the entire series as-is.
>
> I think it's cargo-culted because IIRC almost every driver bails out if
> fbdev init fails. So the only time the pointer is NULL is when fbdev
> emulation is compiled out. And in that case the check is not needed.
>
> vboxvideo for instance fails probing if fbdev init fails, and if it
> succeeds vbox->fbdev is always set.
>
> But anyhow, yeah, I think it's better that the driver maintainers verify
> this.
See my other mail, I was blind, I think it's all good.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
