Re: [PATCH 1/3] drm/tinydrm/Kconfig: Remove menuconfig DRM_TINYDRM

From: Hans de Goede <hdegoede@redhat.com>
To: "Noralf Trønnes" <noralf@tronnes.org>, dri-devel@lists.freedesktop.org
Cc: daniel.vetter@ffwll.ch, sam@ravnborg.org
Subject: Re: [PATCH 1/3] drm/tinydrm/Kconfig: Remove menuconfig DRM_TINYDRM
Date: Tue, 30 Jul 2019 15:19:07 +0200	[thread overview]
Message-ID: <fc848de3-93b3-5299-261d-475b7277cb83@redhat.com> (raw)
In-Reply-To: <20190725105132.22545-2-noralf@tronnes.org>

Hi,

On 25-07-19 12:51, Noralf Trønnes wrote:
> This makes the tiny drivers visible by default without having to enable a
> knob.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>   drivers/gpu/drm/Makefile        |  2 +-
>   drivers/gpu/drm/tinydrm/Kconfig | 37 +++++++++++++++++++--------------
>   2 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
> index 42b06f4f8989..f8c9a0e71dde 100644
> --- a/drivers/gpu/drm/tinydrm/Kconfig
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -1,16 +1,9 @@
>   # SPDX-License-Identifier: GPL-2.0-only
> -menuconfig DRM_TINYDRM
> -	tristate "Support for simple displays"
> -	depends on DRM
> -	select DRM_KMS_HELPER
> -	select DRM_KMS_CMA_HELPER
> -	help
> -	  Choose this option if you have a tinydrm supported display.
> -	  If M is selected the module will be called tinydrm.
> -
>   config TINYDRM_HX8357D
>   	tristate "DRM support for HX8357D display panels"
> -	depends on DRM_TINYDRM && SPI
> +	depends on DRM && SPI
> +	select DRM_KMS_HELPER
> +	select DRM_KMS_CMA_HELPER
>   	select DRM_MIPI_DBI
>   	select BACKLIGHT_CLASS_DEVICE
>   	help

<snip>

drivers/gpu/drm/tinydrm/Makefile has:

obj-$(CONFIG_DRM_TINYDRM)               += core/

And AFAIK at least most of the drivers under drivers/gpu/drm/tinydrm
actually need the tinydrm-core.

So instead you should make the config option a hidden one
and select it in all the drivers which need it, otherwise
things will no longer work after a clean build AFAICT.

Note that even though the config option now remains, this change:

 > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 > index 98c732f925c7..0b30afa7524d 100644
 > --- a/drivers/gpu/drm/Makefile
 > +++ b/drivers/gpu/drm/Makefile
 > @@ -112,7 +112,7 @@ obj-$(CONFIG_DRM_ARCPGU)+= arc/
 >   obj-y			+= hisilicon/
 >   obj-$(CONFIG_DRM_ZTE)	+= zte/
 >   obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
 > -obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
 > +obj-y			+= tinydrm/
 >   obj-$(CONFIG_DRM_PL111) += pl111/
 >   obj-$(CONFIG_DRM_TVE200) += tve200/
 >   obj-$(CONFIG_DRM_XEN) += xen/

Is still necessary so that when other drivers which do not
depend on the tinydrm core and thus will not do:
	select DRM_TINYDRM
will still get build.

Otherwise this series looks good to me and you can add my:

Reviewed-by: Hans de Goede <hdegoede@redhat.com> to it once
this is fixed.

Note that drivers/gpu/drm/cirrus is a single .c file tiny
driver now a days too, so it too could be moved to the new
tiny dir. I was actually planning on doing something similar
to this series once I got one more tiny driver upstream :)

Regards,

Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel