| Message ID | 20170725180555.3699056-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 26, 2017 at 7:55 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > On Tue, Jul 25, 2017 at 8:05 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> We cannot reference priv->fbdev outside of the #ifdef: >> >> drivers/net/virtio_net.c:1881:12: error: 'virtnet_restore_up' defined but not used [-Werror=unused-function] >> static int virtnet_restore_up(struct virtio_device *vdev) >> drivers/net/virtio_net.c:1859:13: error: 'virtnet_freeze_down' defined but not used [-Werror=unused-function] >> static void virtnet_freeze_down(struct virtio_device *vdev) >> >> As the #ifdef is a bit annoying here, this removes it entirely >> and uses an IS_ENABLED() check in it place where needed. >> >> Fixes: b4dd9f1ffaba ("drm/hisilicon: Remove custom FB helper deferred setup") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > I guess I wasn't clear enough, but you don't even need the IS_ENABLED. > The cma_fini/init functions themselves don't get no-opped out (I guess > we could fix that), but the underlying fb helper functions they call > do, so this is all perfectly fine to call unconditionally. And that's > what all other drivers do. Should I edit while applying, or do you > want to respin? Please just edit as you like then, I think that's quicker. The version I sent was meant to have smaller object code as well, and I didn't think we could rely on drm_fb_cma_helper.c being built without CONFIG_DRM_FBDEV_EMULATION, but I see now that they are independent as you say, so making them unconditional indeed gives the simplest code. Thanks! Arnd
On Wed, Jul 26, 2017 at 08:58:16AM +0200, Arnd Bergmann wrote: > On Wed, Jul 26, 2017 at 7:55 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > On Tue, Jul 25, 2017 at 8:05 PM, Arnd Bergmann <arnd@arndb.de> wrote: > >> We cannot reference priv->fbdev outside of the #ifdef: > >> > >> drivers/net/virtio_net.c:1881:12: error: 'virtnet_restore_up' defined but not used [-Werror=unused-function] > >> static int virtnet_restore_up(struct virtio_device *vdev) > >> drivers/net/virtio_net.c:1859:13: error: 'virtnet_freeze_down' defined but not used [-Werror=unused-function] > >> static void virtnet_freeze_down(struct virtio_device *vdev) > >> > >> As the #ifdef is a bit annoying here, this removes it entirely > >> and uses an IS_ENABLED() check in it place where needed. > >> > >> Fixes: b4dd9f1ffaba ("drm/hisilicon: Remove custom FB helper deferred setup") > >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > > I guess I wasn't clear enough, but you don't even need the IS_ENABLED. > > The cma_fini/init functions themselves don't get no-opped out (I guess > > we could fix that), but the underlying fb helper functions they call > > do, so this is all perfectly fine to call unconditionally. And that's > > what all other drivers do. Should I edit while applying, or do you > > want to respin? > > Please just edit as you like then, I think that's quicker. > > The version I sent was meant to have smaller object code as well, and > I didn't think we could rely on drm_fb_cma_helper.c being built without > CONFIG_DRM_FBDEV_EMULATION, but I see now that they are > independent as you say, so making them unconditional indeed gives > the simplest code. Done, thanks. -Daniel
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c index 1178341c3858..5d2dfe92f62c 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c @@ -34,12 +34,11 @@ static int kirin_drm_kms_cleanup(struct drm_device *dev) { struct kirin_drm_private *priv = dev->dev_private; -#ifdef CONFIG_DRM_FBDEV_EMULATION - if (priv->fbdev) { + if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) && priv->fbdev) { drm_fbdev_cma_fini(priv->fbdev); priv->fbdev = NULL; } -#endif + drm_kms_helper_poll_fini(dev); dc_ops->cleanup(to_platform_device(dev->dev)); drm_mode_config_cleanup(dev); @@ -49,20 +48,17 @@ static int kirin_drm_kms_cleanup(struct drm_device *dev) return 0; } -#ifdef CONFIG_DRM_FBDEV_EMULATION static void kirin_fbdev_output_poll_changed(struct drm_device *dev) { struct kirin_drm_private *priv = dev->dev_private; drm_fbdev_cma_hotplug_event(priv->fbdev); } -#endif static const struct drm_mode_config_funcs kirin_drm_mode_config_funcs = { .fb_create = drm_fb_cma_create, -#ifdef CONFIG_DRM_FBDEV_EMULATION - .output_poll_changed = kirin_fbdev_output_poll_changed, -#endif + .output_poll_changed = IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) ? + kirin_fbdev_output_poll_changed : NULL, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, }; @@ -121,14 +117,15 @@ static int kirin_drm_kms_init(struct drm_device *dev) /* init kms poll for handling hpd */ drm_kms_helper_poll_init(dev); - priv->fbdev = drm_fbdev_cma_init(dev, 32, - dev->mode_config.num_connector); + if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)) + priv->fbdev = drm_fbdev_cma_init(dev, 32, + dev->mode_config.num_connector); + if (IS_ERR(priv->fbdev)) { DRM_ERROR("failed to initialize fbdev.\n"); ret = PTR_ERR(priv->fbdev); goto err_cleanup_poll; } - return 0; err_cleanup_poll: diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h index 7f60c64915d9..56cb62df065c 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h @@ -20,9 +20,7 @@ struct kirin_dc_ops { }; struct kirin_drm_private { -#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_fbdev_cma *fbdev; -#endif }; extern const struct kirin_dc_ops ade_dc_ops;
We cannot reference priv->fbdev outside of the #ifdef: drivers/net/virtio_net.c:1881:12: error: 'virtnet_restore_up' defined but not used [-Werror=unused-function] static int virtnet_restore_up(struct virtio_device *vdev) drivers/net/virtio_net.c:1859:13: error: 'virtnet_freeze_down' defined but not used [-Werror=unused-function] static void virtnet_freeze_down(struct virtio_device *vdev) As the #ifdef is a bit annoying here, this removes it entirely and uses an IS_ENABLED() check in it place where needed. Fixes: b4dd9f1ffaba ("drm/hisilicon: Remove custom FB helper deferred setup") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 19 ++++++++----------- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 2 -- 2 files changed, 8 insertions(+), 13 deletions(-)