Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1695992 > unrolled thread

[PATCH v2] drm/hisilicon: fix build error without fbdev emulation

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] drm/hisilicon: fix build error without fbdev emulation Arnd Bergmann <arnd@arndb.de> - 2017-07-25 20:10 +0200
    Re: [PATCH v2] drm/hisilicon: fix build error without fbdev emulation Daniel Vetter <daniel.vetter@ffwll.ch> - 2017-07-26 08:00 +0200
      Re: [PATCH v2] drm/hisilicon: fix build error without fbdev emulation Arnd Bergmann <arnd@arndb.de> - 2017-07-26 09:00 +0200
        Re: [PATCH v2] drm/hisilicon: fix build error without fbdev emulation Daniel Vetter <daniel@ffwll.ch> - 2017-07-26 12:50 +0200

#1695992 — [PATCH v2] drm/hisilicon: fix build error without fbdev emulation

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(-)

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;
-- 
2.9.0

[toc] | [next] | [standalone]

#1696800

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?

Thanks, Daniel

> ---
>  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(-)
>
> 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;
> --
> 2.9.0
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1696826

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

[toc] | [prev] | [next] | [standalone]

#1696980

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web