| Message ID | 489bdaae-9b05-2d70-12e1-4fda7899dfc1@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC | expand |
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 >
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 > > >
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 >>> >> >
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 > >>> > >> > > >
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
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> ---