On Tue, May 05, 2020 at 11:56:49AM +0200, Thomas Zimmermann wrote: > As it is best practice now, the DRM device instance is now embedded in > struct mga_device. All references to dev_private have been removed. > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > --- > drivers/gpu/drm/mgag200/mgag200_cursor.c | 6 +++--- > drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +- > drivers/gpu/drm/mgag200/mgag200_drv.h | 4 ++-- > drivers/gpu/drm/mgag200/mgag200_main.c | 16 ++++++---------- > drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++-- > drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 ++-- > 6 files changed, 16 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c > index aebc9ce43d551..e3c717c0cffc0 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c > +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c > @@ -15,7 +15,7 @@ static bool warn_palette = true; > static int mgag200_cursor_update(struct mga_device *mdev, void *dst, void *src, > unsigned int width, unsigned int height) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > unsigned int i, row, col; > uint32_t colour_set[16]; > uint32_t *next_space = &colour_set[0]; > @@ -119,7 +119,7 @@ static void mgag200_cursor_set_base(struct mga_device *mdev, u64 address) > static int mgag200_show_cursor(struct mga_device *mdev, void *src, > unsigned int width, unsigned int height) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > struct drm_gem_vram_object *gbo; > void *dst; > s64 off; > @@ -196,7 +196,7 @@ static void mgag200_move_cursor(struct mga_device *mdev, int x, int y) > > int mgag200_cursor_init(struct mga_device *mdev) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > size_t ncursors = ARRAY_SIZE(mdev->cursor.gbo); > size_t size; > int ret; > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c > index ad12c1b7c66cc..fc0775694c097 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.c > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c > @@ -71,7 +71,7 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto err_pci_disable_device; > > - dev = mdev->dev; > + dev = &mdev->base; > > ret = drm_dev_register(dev, ent->driver_data); > if (ret) > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h > index 1ce0386669ffa..fb2797d6ff690 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.h > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h > @@ -96,7 +96,7 @@ > > #define to_mga_crtc(x) container_of(x, struct mga_crtc, base) > #define to_mga_connector(x) container_of(x, struct mga_connector, base) > -#define to_mga_device(x) ((struct mga_device *)(x)->dev_private) > +#define to_mga_device(x) container_of(x, struct mga_device, base) > > struct mga_crtc { > struct drm_crtc base; > @@ -153,7 +153,7 @@ enum mga_type { > #define IS_G200_SE(mdev) (mdev->type == G200_SE_A || mdev->type == G200_SE_B) > > struct mga_device { > - struct drm_device *dev; > + struct drm_device base; > unsigned long flags; > > resource_size_t rmmio_base; > diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c > index 070ff1f433df2..ca3ed463c2d41 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_main.c > +++ b/drivers/gpu/drm/mgag200/mgag200_main.c > @@ -67,7 +67,7 @@ static int mga_probe_vram(struct mga_device *mdev, void __iomem *mem) > /* Map the framebuffer from the card and configure the core */ > static int mga_vram_init(struct mga_device *mdev) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > void __iomem *mem; > > /* BAR 0 is VRAM */ > @@ -100,14 +100,12 @@ static int mga_vram_init(struct mga_device *mdev) > int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv, > struct pci_dev *pdev, unsigned long flags) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > int ret, option; > > - dev = drm_dev_alloc(drv, &pdev->dev); > - if (IS_ERR(dev)) > - return PTR_ERR(dev); > - dev->dev_private = (void *)mdev; > - mdev->dev = dev; > + ret = drm_dev_init(dev, drv, &pdev->dev); You devm_kzalloc this structure in the previous patch. That's kinda less correct than what we have now ... I think this patch and the previous one are needless detour, and straight going to embedding and devm_drm_dev_alloc like I've done e.g. in commit cd8294540776f7986b7cf658a3579d576853610c Author: Daniel Vetter <daniel.vetter@xxxxxxxx> Date: Wed Apr 15 09:40:30 2020 +0200 drm/aspeed: Use devm_drm_dev_alloc Then clean up all the fallout later on (i.e. switch over to mga_device and away from drm_device, heck even the to_mag_device pointer upcasting you can all do afterwards). The intermediate stages all have tricky rules for what exactly can and can't be done, for no real gain, so here a massively split patch series imo just increases the risks you break something somewhere. Cheers, Daniel > + if (ret) > + return ret; > > dev->pdev = pdev; > pci_set_drvdata(pdev, dev); > @@ -185,17 +183,15 @@ int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv, > err_drm_dev_put: > drm_dev_put(dev); > err_mm: > - dev->dev_private = NULL; > return ret; > } > > void mgag200_device_fini(struct mga_device *mdev) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > > mgag200_modeset_fini(mdev); > drm_mode_config_cleanup(dev); > mgag200_cursor_fini(mdev); > mgag200_mm_fini(mdev); > - dev->dev_private = NULL; > } > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c > index aaa73b29b04f0..eb58742026adf 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c > @@ -1433,7 +1433,7 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs = { > /* CRTC setup */ > static void mga_crtc_init(struct mga_device *mdev) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > struct mga_crtc *mga_crtc; > > mga_crtc = kzalloc(sizeof(struct mga_crtc) + > @@ -1618,7 +1618,7 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev) > > int mgag200_modeset_init(struct mga_device *mdev) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > struct drm_encoder *encoder = &mdev->encoder; > struct drm_connector *connector; > int ret; > diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c > index e89657630ea71..86a582490bb67 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c > +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c > @@ -34,7 +34,7 @@ int mgag200_mm_init(struct mga_device *mdev) > { > struct drm_vram_mm *vmm; > int ret; > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > > vmm = drm_vram_helper_alloc_mm(dev, pci_resource_start(dev->pdev, 0), > mdev->mc.vram_size); > @@ -57,7 +57,7 @@ int mgag200_mm_init(struct mga_device *mdev) > > void mgag200_mm_fini(struct mga_device *mdev) > { > - struct drm_device *dev = mdev->dev; > + struct drm_device *dev = &mdev->base; > > mdev->vram_fb_available = 0; > > -- > 2.26.0 > -- 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