| Message ID | 20180602114144.18169-1-fdr@pid42.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jun 02, 2018 at 08:41:44AM -0300, Fabio Rafael da Rosa wrote: > The use of load and unload hooks is deprecated. DRM drivers should > use drm_dev_alloc|drm_dev_init and drm_dev_register for initialization > and publishing. > > Signed-off-by: Fabio Rafael da Rosa <fdr@pid42.net> Reviewed-by: Nicholas Mc Guire <der.herr@hofr.at> > --- > drivers/staging/vboxvideo/TODO | 1 - > drivers/staging/vboxvideo/vbox_drv.c | 33 +++++++++++++++++++++++---- > drivers/staging/vboxvideo/vbox_drv.h | 2 +- > drivers/staging/vboxvideo/vbox_main.c | 2 +- > 4 files changed, 31 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO > index bd381d861ab3..468eea856ca6 100644 > --- a/drivers/staging/vboxvideo/TODO > +++ b/drivers/staging/vboxvideo/TODO > @@ -1,6 +1,5 @@ > TODO: > -Move the driver over to the atomic API > --Stop using old load / unload drm_driver hooks > -Get a full review from the drm-maintainers on dri-devel done on this driver > -Extend this TODO with the results of that review > > diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c > index e18642e5027e..a580d184c613 100644 > --- a/drivers/staging/vboxvideo/vbox_drv.c > +++ b/drivers/staging/vboxvideo/vbox_drv.c > @@ -51,14 +51,41 @@ MODULE_DEVICE_TABLE(pci, pciidlist); > > static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > { > - return drm_get_pci_dev(pdev, ent, &driver); > + struct drm_device *dev = NULL; > + int ret = 0; > + > + dev = drm_dev_alloc(&driver, &pdev->dev); > + if (IS_ERR(dev)) { > + ret = PTR_ERR(dev); > + goto err_drv_alloc; > + } > + dev->pdev = pdev; > + pci_set_drvdata(pdev, dev); > + > + ret = vbox_driver_load(dev); > + if (ret) > + goto err_vbox_driver_load; > + > + ret = drm_dev_register(dev, 0); > + if (ret) > + goto err_drv_dev_register; > + > + return ret; > + > + err_drv_dev_register: > + vbox_driver_unload(dev); > + err_vbox_driver_load: > + drm_dev_unref(dev); Documentation to drm_dev_unref() notes: <snip> * This is a compatibility alias for drm_dev_put() and should not be used by new * code. <snip> so probably that should be a drm_dev_put(dev); here (as well as below) Also does dev not need to be freed in the error paths ? atleast I did not see how it would be in the current code (kzalloc´ed in drm_dev_alloc()) thx! hofrat > + err_drv_alloc: > + return ret; > } > > static void vbox_pci_remove(struct pci_dev *pdev) > { > struct drm_device *dev = pci_get_drvdata(pdev); > > - drm_put_dev(dev); > + drm_dev_unregister(dev); > + drm_dev_unref(dev); > } > > static int vbox_drm_freeze(struct drm_device *dev) > @@ -227,8 +254,6 @@ static struct drm_driver driver = { > DRIVER_PRIME, > .dev_priv_size = 0, > > - .load = vbox_driver_load, > - .unload = vbox_driver_unload, > .lastclose = vbox_driver_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 eeac4f0cb2c6..594f84272957 100644 > --- a/drivers/staging/vboxvideo/vbox_drv.h > +++ b/drivers/staging/vboxvideo/vbox_drv.h > @@ -126,7 +126,7 @@ struct vbox_private { > #undef CURSOR_PIXEL_COUNT > #undef CURSOR_DATA_SIZE > > -int vbox_driver_load(struct drm_device *dev, unsigned long flags); > +int vbox_driver_load(struct drm_device *dev); > void vbox_driver_unload(struct drm_device *dev); > void vbox_driver_lastclose(struct drm_device *dev); > > diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c > index 9d2018cd544e..429f6a453619 100644 > --- a/drivers/staging/vboxvideo/vbox_main.c > +++ b/drivers/staging/vboxvideo/vbox_main.c > @@ -350,7 +350,7 @@ static void vbox_hw_fini(struct vbox_private *vbox) > pci_iounmap(vbox->dev->pdev, vbox->guest_heap); > } > > -int vbox_driver_load(struct drm_device *dev, unsigned long flags) > +int vbox_driver_load(struct drm_device *dev) > { > struct vbox_private *vbox; > int ret = 0; > -- > 2.17.0 > > > _______________________________________________ > Kernelnewbies mailing list > Kernelnewbies@kernelnewbies.org > https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
On 2018-06-02 09:15, Nicholas Mc Guire wrote: > On Sat, Jun 02, 2018 at 08:41:44AM -0300, Fabio Rafael da Rosa wrote: >> The use of load and unload hooks is deprecated. DRM drivers should >> use drm_dev_alloc|drm_dev_init and drm_dev_register for initialization >> and publishing. >> >> Signed-off-by: Fabio Rafael da Rosa <fdr@pid42.net> > Reviewed-by: Nicholas Mc Guire <der.herr@hofr.at> >> --- >> drivers/staging/vboxvideo/TODO | 1 - >> drivers/staging/vboxvideo/vbox_drv.c | 33 >> +++++++++++++++++++++++---- >> drivers/staging/vboxvideo/vbox_drv.h | 2 +- >> drivers/staging/vboxvideo/vbox_main.c | 2 +- >> 4 files changed, 31 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/staging/vboxvideo/TODO >> b/drivers/staging/vboxvideo/TODO >> index bd381d861ab3..468eea856ca6 100644 >> --- a/drivers/staging/vboxvideo/TODO >> +++ b/drivers/staging/vboxvideo/TODO >> @@ -1,6 +1,5 @@ >> TODO: >> -Move the driver over to the atomic API >> --Stop using old load / unload drm_driver hooks >> -Get a full review from the drm-maintainers on dri-devel done on this >> driver >> -Extend this TODO with the results of that review >> >> diff --git a/drivers/staging/vboxvideo/vbox_drv.c >> b/drivers/staging/vboxvideo/vbox_drv.c >> index e18642e5027e..a580d184c613 100644 >> --- a/drivers/staging/vboxvideo/vbox_drv.c >> +++ b/drivers/staging/vboxvideo/vbox_drv.c >> @@ -51,14 +51,41 @@ MODULE_DEVICE_TABLE(pci, pciidlist); >> >> static int vbox_pci_probe(struct pci_dev *pdev, const struct >> pci_device_id *ent) >> { >> - return drm_get_pci_dev(pdev, ent, &driver); >> + struct drm_device *dev = NULL; >> + int ret = 0; >> + >> + dev = drm_dev_alloc(&driver, &pdev->dev); >> + if (IS_ERR(dev)) { >> + ret = PTR_ERR(dev); >> + goto err_drv_alloc; >> + } >> + dev->pdev = pdev; >> + pci_set_drvdata(pdev, dev); >> + >> + ret = vbox_driver_load(dev); >> + if (ret) >> + goto err_vbox_driver_load; >> + >> + ret = drm_dev_register(dev, 0); >> + if (ret) >> + goto err_drv_dev_register; >> + >> + return ret; >> + >> + err_drv_dev_register: >> + vbox_driver_unload(dev); >> + err_vbox_driver_load: >> + drm_dev_unref(dev); > > Documentation to drm_dev_unref() notes: > > <snip> > * This is a compatibility alias for drm_dev_put() and should not be > used by new > * code. > <snip> > > so probably that should be a drm_dev_put(dev); here (as well as below) > You are right, it was my mistake while reading the documentation. I'll fix it. > Also does dev not need to be freed in the error paths ? atleast I did > not > see how it would be in the current code (kzalloc´ed in drm_dev_alloc()) > I understood that drm_dev_put would do it (dev is ref counted). > thx! > hofrat > >> + err_drv_alloc: >> + return ret; >> } >> >> static void vbox_pci_remove(struct pci_dev *pdev) >> { >> struct drm_device *dev = pci_get_drvdata(pdev); >> >> - drm_put_dev(dev); >> + drm_dev_unregister(dev); >> + drm_dev_unref(dev); >> } >> >> static int vbox_drm_freeze(struct drm_device *dev) >> @@ -227,8 +254,6 @@ static struct drm_driver driver = { >> DRIVER_PRIME, >> .dev_priv_size = 0, >> >> - .load = vbox_driver_load, >> - .unload = vbox_driver_unload, >> .lastclose = vbox_driver_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 eeac4f0cb2c6..594f84272957 100644 >> --- a/drivers/staging/vboxvideo/vbox_drv.h >> +++ b/drivers/staging/vboxvideo/vbox_drv.h >> @@ -126,7 +126,7 @@ struct vbox_private { >> #undef CURSOR_PIXEL_COUNT >> #undef CURSOR_DATA_SIZE >> >> -int vbox_driver_load(struct drm_device *dev, unsigned long flags); >> +int vbox_driver_load(struct drm_device *dev); >> void vbox_driver_unload(struct drm_device *dev); >> void vbox_driver_lastclose(struct drm_device *dev); >> >> diff --git a/drivers/staging/vboxvideo/vbox_main.c >> b/drivers/staging/vboxvideo/vbox_main.c >> index 9d2018cd544e..429f6a453619 100644 >> --- a/drivers/staging/vboxvideo/vbox_main.c >> +++ b/drivers/staging/vboxvideo/vbox_main.c >> @@ -350,7 +350,7 @@ static void vbox_hw_fini(struct vbox_private >> *vbox) >> pci_iounmap(vbox->dev->pdev, vbox->guest_heap); >> } >> >> -int vbox_driver_load(struct drm_device *dev, unsigned long flags) >> +int vbox_driver_load(struct drm_device *dev) >> { >> struct vbox_private *vbox; >> int ret = 0; >> -- >> 2.17.0 >> >> >> _______________________________________________ >> Kernelnewbies mailing list >> Kernelnewbies@kernelnewbies.org >> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO index bd381d861ab3..468eea856ca6 100644 --- a/drivers/staging/vboxvideo/TODO +++ b/drivers/staging/vboxvideo/TODO @@ -1,6 +1,5 @@ TODO: -Move the driver over to the atomic API --Stop using old load / unload drm_driver hooks -Get a full review from the drm-maintainers on dri-devel done on this driver -Extend this TODO with the results of that review diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c index e18642e5027e..a580d184c613 100644 --- a/drivers/staging/vboxvideo/vbox_drv.c +++ b/drivers/staging/vboxvideo/vbox_drv.c @@ -51,14 +51,41 @@ MODULE_DEVICE_TABLE(pci, pciidlist); static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) { - return drm_get_pci_dev(pdev, ent, &driver); + struct drm_device *dev = NULL; + int ret = 0; + + dev = drm_dev_alloc(&driver, &pdev->dev); + if (IS_ERR(dev)) { + ret = PTR_ERR(dev); + goto err_drv_alloc; + } + dev->pdev = pdev; + pci_set_drvdata(pdev, dev); + + ret = vbox_driver_load(dev); + if (ret) + goto err_vbox_driver_load; + + ret = drm_dev_register(dev, 0); + if (ret) + goto err_drv_dev_register; + + return ret; + + err_drv_dev_register: + vbox_driver_unload(dev); + err_vbox_driver_load: + drm_dev_unref(dev); + err_drv_alloc: + return ret; } static void vbox_pci_remove(struct pci_dev *pdev) { struct drm_device *dev = pci_get_drvdata(pdev); - drm_put_dev(dev); + drm_dev_unregister(dev); + drm_dev_unref(dev); } static int vbox_drm_freeze(struct drm_device *dev) @@ -227,8 +254,6 @@ static struct drm_driver driver = { DRIVER_PRIME, .dev_priv_size = 0, - .load = vbox_driver_load, - .unload = vbox_driver_unload, .lastclose = vbox_driver_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 eeac4f0cb2c6..594f84272957 100644 --- a/drivers/staging/vboxvideo/vbox_drv.h +++ b/drivers/staging/vboxvideo/vbox_drv.h @@ -126,7 +126,7 @@ struct vbox_private { #undef CURSOR_PIXEL_COUNT #undef CURSOR_DATA_SIZE -int vbox_driver_load(struct drm_device *dev, unsigned long flags); +int vbox_driver_load(struct drm_device *dev); void vbox_driver_unload(struct drm_device *dev); void vbox_driver_lastclose(struct drm_device *dev); diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 9d2018cd544e..429f6a453619 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -350,7 +350,7 @@ static void vbox_hw_fini(struct vbox_private *vbox) pci_iounmap(vbox->dev->pdev, vbox->guest_heap); } -int vbox_driver_load(struct drm_device *dev, unsigned long flags) +int vbox_driver_load(struct drm_device *dev) { struct vbox_private *vbox; int ret = 0;
The use of load and unload hooks is deprecated. DRM drivers should use drm_dev_alloc|drm_dev_init and drm_dev_register for initialization and publishing. Signed-off-by: Fabio Rafael da Rosa <fdr@pid42.net> --- drivers/staging/vboxvideo/TODO | 1 - drivers/staging/vboxvideo/vbox_drv.c | 33 +++++++++++++++++++++++---- drivers/staging/vboxvideo/vbox_drv.h | 2 +- drivers/staging/vboxvideo/vbox_main.c | 2 +- 4 files changed, 31 insertions(+), 7 deletions(-)