[PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC

[PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC

[Hans Verkuil]

Add a missing 'depends on DRM' for the DRM_DP_CEC config
option. Without that enabling DRM_DP_CEC will force CEC_CORE
to =y instead of =m if DRM=m as well.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 1168351267fd..e8e478d6da9c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -163,6 +163,7 @@ config DRM_LOAD_EDID_FIRMWARE

 config DRM_DP_CEC
 	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
+	depends on DRM
 	select CEC_CORE
 	help
 	  Choose this option if you want to enable HDMI CEC support for
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC

[Hans Verkuil]

On 12/6/19 12:26 PM, Hans Verkuil wrote:
> Add a missing 'depends on DRM' for the DRM_DP_CEC config
> option. Without that enabling DRM_DP_CEC will force CEC_CORE
> to =y instead of =m if DRM=m as well.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Daniel, can you review this? It's annoying that the cec core is
compiled as part of the kernel when it can just be a module.

Regards,

	Hans

> ---
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 1168351267fd..e8e478d6da9c 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -163,6 +163,7 @@ config DRM_LOAD_EDID_FIRMWARE
> 
>  config DRM_DP_CEC
>  	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> +	depends on DRM
>  	select CEC_CORE
>  	help
>  	  Choose this option if you want to enable HDMI CEC support for
> 

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

Re: [PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC

[Daniel Vetter]

On Wed, Jan 08, 2020 at 01:08:47PM +0100, Hans Verkuil wrote:
> On 12/6/19 12:26 PM, Hans Verkuil wrote:
> > Add a missing 'depends on DRM' for the DRM_DP_CEC config
> > option. Without that enabling DRM_DP_CEC will force CEC_CORE
> > to =y instead of =m if DRM=m as well.
> > 
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Daniel, can you review this? It's annoying that the cec core is
> compiled as part of the kernel when it can just be a module.

Why did we even make this optional? Really worth it to compile out 4
functions and some change?

Anyway the one you really want here is CONFIG_DRM_KMS_HELPER, but that is
a selected variable, and you can't mix select and depends on because that
breaks Kconfig in hilarious ways. Or so I thought, but other public
symbols like this (e.g. DRM_FBDEV_EMULATION) do the same trick. So I guess

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

But really, is all this complexity?
-Daniel

> 
> Regards,
> 
> 	Hans
> 
> > ---
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 1168351267fd..e8e478d6da9c 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -163,6 +163,7 @@ config DRM_LOAD_EDID_FIRMWARE
> > 
> >  config DRM_DP_CEC
> >  	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> > +	depends on DRM
> >  	select CEC_CORE
> >  	help
> >  	  Choose this option if you want to enable HDMI CEC support for
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC

[Hans Verkuil]

On 1/8/20 6:42 PM, Daniel Vetter wrote:
> On Wed, Jan 08, 2020 at 01:08:47PM +0100, Hans Verkuil wrote:
>> On 12/6/19 12:26 PM, Hans Verkuil wrote:
>>> Add a missing 'depends on DRM' for the DRM_DP_CEC config
>>> option. Without that enabling DRM_DP_CEC will force CEC_CORE
>>> to =y instead of =m if DRM=m as well.
>>>
>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>
>> Daniel, can you review this? It's annoying that the cec core is
>> compiled as part of the kernel when it can just be a module.
> 
> Why did we even make this optional? Really worth it to compile out 4
> functions and some change?

It's not about those few functions, it's because this enables the CEC
framework as well (drivers/media/cec).

If CEC is not needed, then disabling this saves a lot more code than the
few functions in drm_dp_cec.c.

CEC is an optional HDMI feature, so CEC support for HDMI input/output
drivers is optional as well in the kernel config.

Regards,

	Hans

> 
> Anyway the one you really want here is CONFIG_DRM_KMS_HELPER, but that is
> a selected variable, and you can't mix select and depends on because that
> breaks Kconfig in hilarious ways. Or so I thought, but other public
> symbols like this (e.g. DRM_FBDEV_EMULATION) do the same trick. So I guess
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> But really, is all this complexity?
> -Daniel
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>> ---
>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>> index 1168351267fd..e8e478d6da9c 100644
>>> --- a/drivers/gpu/drm/Kconfig
>>> +++ b/drivers/gpu/drm/Kconfig
>>> @@ -163,6 +163,7 @@ config DRM_LOAD_EDID_FIRMWARE
>>>
>>>  config DRM_DP_CEC
>>>  	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
>>> +	depends on DRM
>>>  	select CEC_CORE
>>>  	help
>>>  	  Choose this option if you want to enable HDMI CEC support for
>>>
>>
> 

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

Re: [PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC

[Daniel Vetter]

On Thu, Jan 09, 2020 at 10:11:48AM +0100, Hans Verkuil wrote:
> On 1/8/20 6:42 PM, Daniel Vetter wrote:
> > On Wed, Jan 08, 2020 at 01:08:47PM +0100, Hans Verkuil wrote:
> >> On 12/6/19 12:26 PM, Hans Verkuil wrote:
> >>> Add a missing 'depends on DRM' for the DRM_DP_CEC config
> >>> option. Without that enabling DRM_DP_CEC will force CEC_CORE
> >>> to =y instead of =m if DRM=m as well.
> >>>
> >>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> >>
> >> Daniel, can you review this? It's annoying that the cec core is
> >> compiled as part of the kernel when it can just be a module.
> > 
> > Why did we even make this optional? Really worth it to compile out 4
> > functions and some change?
> 
> It's not about those few functions, it's because this enables the CEC
> framework as well (drivers/media/cec).
> 
> If CEC is not needed, then disabling this saves a lot more code than the
> few functions in drm_dp_cec.c.
> 
> CEC is an optional HDMI feature, so CEC support for HDMI input/output
> drivers is optional as well in the kernel config.

The trouble is that once you have multiple layers of such automatically
optional pieces of code, Kconfig keels over: select isn't recursive. So if
you want to make CEC stuff optional, just compile the DRM stuff only if
both CEC and DRM are enabled, and drivers can then select the overall CEC
stuff. Or alternatively have dummy functions at the CEC library, and just
always compile the DRM CEC stuff in.

You could also fix Kconfig, if you have a life or two to spare :-)

Cheers, Daniel

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Anyway the one you really want here is CONFIG_DRM_KMS_HELPER, but that is
> > a selected variable, and you can't mix select and depends on because that
> > breaks Kconfig in hilarious ways. Or so I thought, but other public
> > symbols like this (e.g. DRM_FBDEV_EMULATION) do the same trick. So I guess
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > But really, is all this complexity?
> > -Daniel
> > 
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >>> ---
> >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> >>> index 1168351267fd..e8e478d6da9c 100644
> >>> --- a/drivers/gpu/drm/Kconfig
> >>> +++ b/drivers/gpu/drm/Kconfig
> >>> @@ -163,6 +163,7 @@ config DRM_LOAD_EDID_FIRMWARE
> >>>
> >>>  config DRM_DP_CEC
> >>>  	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> >>> +	depends on DRM
> >>>  	select CEC_CORE
> >>>  	help
> >>>  	  Choose this option if you want to enable HDMI CEC support for
> >>>
> >>
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel