All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Maling list - DRI developers <dri-devel@lists.freedesktop.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: Re: [PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC
Date: Wed, 8 Jan 2020 18:42:36 +0100	[thread overview]
Message-ID: <20200108174236.GH43062@phenom.ffwll.local> (raw)
In-Reply-To: <bbbef09d-6c90-75ba-e480-28365474b1a5@xs4all.nl>

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

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Maling list - DRI developers <dri-devel@lists.freedesktop.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC
Date: Wed, 8 Jan 2020 18:42:36 +0100	[thread overview]
Message-ID: <20200108174236.GH43062@phenom.ffwll.local> (raw)
In-Reply-To: <bbbef09d-6c90-75ba-e480-28365474b1a5@xs4all.nl>

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

  reply	other threads:[~2020-01-08 17:42 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-06 11:26 [PATCH] drm/Kconfig: add missing 'depends on DRM' for DRM_DP_CEC Hans Verkuil
2019-12-06 11:26 ` Hans Verkuil
2020-01-08 12:08 ` Hans Verkuil
2020-01-08 12:08   ` Hans Verkuil
2020-01-08 17:42   ` Daniel Vetter [this message]
2020-01-08 17:42     ` Daniel Vetter
2020-01-09  9:11     ` Hans Verkuil
2020-01-09  9:11       ` Hans Verkuil
2020-01-12 22:43       ` Daniel Vetter
2020-01-12 22:43         ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200108174236.GH43062@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.