| Message ID | 20201023122811.2374118-4-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] drm/radeon: Stop changing the drm_driver struct | expand |
On Fri, Oct 23, 2020 at 02:28:10PM +0200, Daniel Vetter wrote: > It's nice if a big function/ioctl table like this is const. Only > downside here is that we need a few more #ifdef to paper over the > differences when CONFIG_DRM_LEGACY is enabled. Maybe provides more > motivation to sunset that horror show :-) > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Maxime Ripard <mripard@kernel.org> Maxime
On Fri, Oct 23, 2020 at 02:28:10PM +0200, Daniel Vetter wrote: > It's nice if a big function/ioctl table like this is const. Only > downside here is that we need a few more #ifdef to paper over the > differences when CONFIG_DRM_LEGACY is enabled. Maybe provides more > motivation to sunset that horror show :-) > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_drv.c | 15 ++++++++++----- > include/drm/drm_device.h | 4 ++++ > include/drm/drm_drv.h | 5 +++-- > 3 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 457ac0f82be2..94d2c41115b8 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -574,7 +574,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) > } > > static int drm_dev_init(struct drm_device *dev, > - struct drm_driver *driver, > + const struct drm_driver *driver, > struct device *parent) > { > int ret; > @@ -589,7 +589,11 @@ static int drm_dev_init(struct drm_device *dev, > > kref_init(&dev->ref); > dev->dev = get_device(parent); > +#ifdef CONFIG_DRM_LEGACY > + dev->driver = (struct drm_driver *) driver; checkpatch will tell you to drop space after closing ')' With this super important fix the patch is: Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > +#else > dev->driver = driver; > +#endif > > INIT_LIST_HEAD(&dev->managed.resources); > spin_lock_init(&dev->managed.lock); > @@ -663,7 +667,7 @@ static void devm_drm_dev_init_release(void *data) > > static int devm_drm_dev_init(struct device *parent, > struct drm_device *dev, > - struct drm_driver *driver) > + const struct drm_driver *driver) > { > int ret; > > @@ -678,7 +682,8 @@ static int devm_drm_dev_init(struct device *parent, > return ret; > } > > -void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, > +void *__devm_drm_dev_alloc(struct device *parent, > + const struct drm_driver *driver, > size_t size, size_t offset) > { > void *container; > @@ -713,7 +718,7 @@ EXPORT_SYMBOL(__devm_drm_dev_alloc); > * RETURNS: > * Pointer to new DRM device, or ERR_PTR on failure. > */ > -struct drm_device *drm_dev_alloc(struct drm_driver *driver, > +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, > struct device *parent) > { > struct drm_device *dev; > @@ -858,7 +863,7 @@ static void remove_compat_control_link(struct drm_device *dev) > */ > int drm_dev_register(struct drm_device *dev, unsigned long flags) > { > - struct drm_driver *driver = dev->driver; > + const struct drm_driver *driver = dev->driver; > int ret; > > if (!driver->load) > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index f4f68e7a9149..2c361964aee7 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -83,7 +83,11 @@ struct drm_device { > } managed; > > /** @driver: DRM driver managing the device */ > +#ifdef CONFIG_DRM_LEGACY > struct drm_driver *driver; > +#else > + const struct drm_driver *driver; > +#endif > > /** > * @dev_private: > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 7af220226a25..cc9da43b6eda 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -516,7 +516,8 @@ struct drm_driver { > #endif > }; > > -void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, > +void *__devm_drm_dev_alloc(struct device *parent, > + const struct drm_driver *driver, > size_t size, size_t offset); > > /** > @@ -549,7 +550,7 @@ void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, > ((type *) __devm_drm_dev_alloc(parent, driver, sizeof(type), \ > offsetof(type, member))) > > -struct drm_device *drm_dev_alloc(struct drm_driver *driver, > +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, > struct device *parent); > int drm_dev_register(struct drm_device *dev, unsigned long flags); > void drm_dev_unregister(struct drm_device *dev); > -- > 2.28.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Am 23.10.20 um 14:28 schrieb Daniel Vetter: > It's nice if a big function/ioctl table like this is const. Only > downside here is that we need a few more #ifdef to paper over the > differences when CONFIG_DRM_LEGACY is enabled. Maybe provides more > motivation to sunset that horror show :-) > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> I very much welcome the change, although the code is *really* ugly. Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_drv.c | 15 ++++++++++----- > include/drm/drm_device.h | 4 ++++ > include/drm/drm_drv.h | 5 +++-- > 3 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 457ac0f82be2..94d2c41115b8 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -574,7 +574,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) > } > > static int drm_dev_init(struct drm_device *dev, > - struct drm_driver *driver, > + const struct drm_driver *driver, > struct device *parent) > { > int ret; > @@ -589,7 +589,11 @@ static int drm_dev_init(struct drm_device *dev, > > kref_init(&dev->ref); > dev->dev = get_device(parent); > +#ifdef CONFIG_DRM_LEGACY > + dev->driver = (struct drm_driver *) driver; > +#else > dev->driver = driver; > +#endif > > INIT_LIST_HEAD(&dev->managed.resources); > spin_lock_init(&dev->managed.lock); > @@ -663,7 +667,7 @@ static void devm_drm_dev_init_release(void *data) > > static int devm_drm_dev_init(struct device *parent, > struct drm_device *dev, > - struct drm_driver *driver) > + const struct drm_driver *driver) > { > int ret; > > @@ -678,7 +682,8 @@ static int devm_drm_dev_init(struct device *parent, > return ret; > } > > -void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, > +void *__devm_drm_dev_alloc(struct device *parent, > + const struct drm_driver *driver, > size_t size, size_t offset) > { > void *container; > @@ -713,7 +718,7 @@ EXPORT_SYMBOL(__devm_drm_dev_alloc); > * RETURNS: > * Pointer to new DRM device, or ERR_PTR on failure. > */ > -struct drm_device *drm_dev_alloc(struct drm_driver *driver, > +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, > struct device *parent) > { > struct drm_device *dev; > @@ -858,7 +863,7 @@ static void remove_compat_control_link(struct drm_device *dev) > */ > int drm_dev_register(struct drm_device *dev, unsigned long flags) > { > - struct drm_driver *driver = dev->driver; > + const struct drm_driver *driver = dev->driver; > int ret; > > if (!driver->load) > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index f4f68e7a9149..2c361964aee7 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -83,7 +83,11 @@ struct drm_device { > } managed; > > /** @driver: DRM driver managing the device */ > +#ifdef CONFIG_DRM_LEGACY > struct drm_driver *driver; > +#else > + const struct drm_driver *driver; > +#endif > > /** > * @dev_private: > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 7af220226a25..cc9da43b6eda 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -516,7 +516,8 @@ struct drm_driver { > #endif > }; > > -void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, > +void *__devm_drm_dev_alloc(struct device *parent, > + const struct drm_driver *driver, > size_t size, size_t offset); > > /** > @@ -549,7 +550,7 @@ void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, > ((type *) __devm_drm_dev_alloc(parent, driver, sizeof(type), \ > offsetof(type, member))) > > -struct drm_device *drm_dev_alloc(struct drm_driver *driver, > +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, > struct device *parent); > int drm_dev_register(struct drm_device *dev, unsigned long flags); > void drm_dev_unregister(struct drm_device *dev); >
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 457ac0f82be2..94d2c41115b8 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -574,7 +574,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) } static int drm_dev_init(struct drm_device *dev, - struct drm_driver *driver, + const struct drm_driver *driver, struct device *parent) { int ret; @@ -589,7 +589,11 @@ static int drm_dev_init(struct drm_device *dev, kref_init(&dev->ref); dev->dev = get_device(parent); +#ifdef CONFIG_DRM_LEGACY + dev->driver = (struct drm_driver *) driver; +#else dev->driver = driver; +#endif INIT_LIST_HEAD(&dev->managed.resources); spin_lock_init(&dev->managed.lock); @@ -663,7 +667,7 @@ static void devm_drm_dev_init_release(void *data) static int devm_drm_dev_init(struct device *parent, struct drm_device *dev, - struct drm_driver *driver) + const struct drm_driver *driver) { int ret; @@ -678,7 +682,8 @@ static int devm_drm_dev_init(struct device *parent, return ret; } -void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, +void *__devm_drm_dev_alloc(struct device *parent, + const struct drm_driver *driver, size_t size, size_t offset) { void *container; @@ -713,7 +718,7 @@ EXPORT_SYMBOL(__devm_drm_dev_alloc); * RETURNS: * Pointer to new DRM device, or ERR_PTR on failure. */ -struct drm_device *drm_dev_alloc(struct drm_driver *driver, +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent) { struct drm_device *dev; @@ -858,7 +863,7 @@ static void remove_compat_control_link(struct drm_device *dev) */ int drm_dev_register(struct drm_device *dev, unsigned long flags) { - struct drm_driver *driver = dev->driver; + const struct drm_driver *driver = dev->driver; int ret; if (!driver->load) diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index f4f68e7a9149..2c361964aee7 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -83,7 +83,11 @@ struct drm_device { } managed; /** @driver: DRM driver managing the device */ +#ifdef CONFIG_DRM_LEGACY struct drm_driver *driver; +#else + const struct drm_driver *driver; +#endif /** * @dev_private: diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 7af220226a25..cc9da43b6eda 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -516,7 +516,8 @@ struct drm_driver { #endif }; -void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, +void *__devm_drm_dev_alloc(struct device *parent, + const struct drm_driver *driver, size_t size, size_t offset); /** @@ -549,7 +550,7 @@ void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver, ((type *) __devm_drm_dev_alloc(parent, driver, sizeof(type), \ offsetof(type, member))) -struct drm_device *drm_dev_alloc(struct drm_driver *driver, +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent); int drm_dev_register(struct drm_device *dev, unsigned long flags); void drm_dev_unregister(struct drm_device *dev);