[PATCH] drm/msm: Make DRM_MSM default to 'm'

[PATCH] drm/msm: Make DRM_MSM default to 'm'

[Jordan Crouse]

Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as
modules but there are some cases where DRM might be built in for whatever
reason and in those situations it is preferable to still keep MSM as a
module by default and let the user decide if they _really_ want to build
it in.

Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure
it doesn't get missed when we need it for a6xx tarets.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 9c37e4d..3b2334b 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -14,11 +14,12 @@ config DRM_MSM
 	select SHMEM
 	select TMPFS
 	select QCOM_SCM if ARCH_QCOM
+	select QCOM_COMMAND_DB if ARCH_QCOM
 	select WANT_DEV_COREDUMP
 	select SND_SOC_HDMI_CODEC if SND_SOC
 	select SYNC_FILE
 	select PM_OPP
-	default y
+	default m
 	help
 	  DRM/KMS driver for MSM/snapdragon.
 
-- 
2.7.4

Re: [PATCH] drm/msm: Make DRM_MSM default to 'm'

[Sam Ravnborg]

Hi Jordan.
On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote:
> Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as
> modules but there are some cases where DRM might be built in for whatever
> reason and in those situations it is preferable to still keep MSM as a
> module by default and let the user decide if they _really_ want to build
> it in.
> 
> Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure
> it doesn't get missed when we need it for a6xx tarets.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
> 
>  drivers/gpu/drm/msm/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 9c37e4d..3b2334b 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -14,11 +14,12 @@ config DRM_MSM
>  	select SHMEM
>  	select TMPFS
>  	select QCOM_SCM if ARCH_QCOM
> +	select QCOM_COMMAND_DB if ARCH_QCOM
>  	select WANT_DEV_COREDUMP
>  	select SND_SOC_HDMI_CODEC if SND_SOC
>  	select SYNC_FILE
>  	select PM_OPP
> -	default y
> +	default m

As a general comment the right thing would be to drop this default.
As it is now the Kconfig says that when DRM is selected then all of the
world would then also get DRM_MSM, which only a small part of this world
you see any benefit in.
So they now have to de-select MSM.

Kconfig has:
    depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)

So maybe not all of the world but all QCOM or IMX5 users. Maybe they are all
interested in MSM. Otherwise the default should rather be dropped.
If there is any good hints then the help text could anyway use some
love, and then add the info there.

The other change with QCOM_COMMAND_DB seems on the other hand to make
sense but then this is another patch.

	Sam

Re: [Freedreno] [PATCH] drm/msm: Make DRM_MSM default to 'm'

[Rob Clark]

On Wed, Aug 7, 2019 at 10:38 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Jordan.
> On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote:
> > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as
> > modules but there are some cases where DRM might be built in for whatever
> > reason and in those situations it is preferable to still keep MSM as a
> > module by default and let the user decide if they _really_ want to build
> > it in.
> >
> > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure
> > it doesn't get missed when we need it for a6xx tarets.
> >
> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >
> >  drivers/gpu/drm/msm/Kconfig | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> > index 9c37e4d..3b2334b 100644
> > --- a/drivers/gpu/drm/msm/Kconfig
> > +++ b/drivers/gpu/drm/msm/Kconfig
> > @@ -14,11 +14,12 @@ config DRM_MSM
> >       select SHMEM
> >       select TMPFS
> >       select QCOM_SCM if ARCH_QCOM
> > +     select QCOM_COMMAND_DB if ARCH_QCOM
> >       select WANT_DEV_COREDUMP
> >       select SND_SOC_HDMI_CODEC if SND_SOC
> >       select SYNC_FILE
> >       select PM_OPP
> > -     default y
> > +     default m
>
> As a general comment the right thing would be to drop this default.
> As it is now the Kconfig says that when DRM is selected then all of the
> world would then also get DRM_MSM, which only a small part of this world
> you see any benefit in.
> So they now have to de-select MSM.

If the default is dropped, it should probably be accompanied by adding
CONFIG_DRM_MSM=m to defconfig's, I think

BR,
-R

> Kconfig has:
>     depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
>
> So maybe not all of the world but all QCOM or IMX5 users. Maybe they are all
> interested in MSM. Otherwise the default should rather be dropped.
> If there is any good hints then the help text could anyway use some
> love, and then add the info there.
>
> The other change with QCOM_COMMAND_DB seems on the other hand to make
> sense but then this is another patch.
>
>         Sam
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm: Make DRM_MSM default to 'm'

[Jordan Crouse]

On Wed, Aug 07, 2019 at 11:08:53AM -0700, Rob Clark wrote:
> On Wed, Aug 7, 2019 at 10:38 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Jordan.
> > On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote:
> > > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as
> > > modules but there are some cases where DRM might be built in for whatever
> > > reason and in those situations it is preferable to still keep MSM as a
> > > module by default and let the user decide if they _really_ want to build
> > > it in.
> > >
> > > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure
> > > it doesn't get missed when we need it for a6xx tarets.
> > >
> > > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > > ---
> > >
> > >  drivers/gpu/drm/msm/Kconfig | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> > > index 9c37e4d..3b2334b 100644
> > > --- a/drivers/gpu/drm/msm/Kconfig
> > > +++ b/drivers/gpu/drm/msm/Kconfig
> > > @@ -14,11 +14,12 @@ config DRM_MSM
> > >       select SHMEM
> > >       select TMPFS
> > >       select QCOM_SCM if ARCH_QCOM
> > > +     select QCOM_COMMAND_DB if ARCH_QCOM
> > >       select WANT_DEV_COREDUMP
> > >       select SND_SOC_HDMI_CODEC if SND_SOC
> > >       select SYNC_FILE
> > >       select PM_OPP
> > > -     default y
> > > +     default m
> >
> > As a general comment the right thing would be to drop this default.
> > As it is now the Kconfig says that when DRM is selected then all of the
> > world would then also get DRM_MSM, which only a small part of this world
> > you see any benefit in.
> > So they now have to de-select MSM.
> 
> If the default is dropped, it should probably be accompanied by adding
> CONFIG_DRM_MSM=m to defconfig's, I think

In general I prefer to not use a default but this is the only GPU driver for
ARCH_QCOM and I think its safe to stay that 99% of ARCH_QCOM users would select
this module and those that wouldn't will omit DRM entirely.

I feel it is net negative if we dropped the default but then had to turn around
and enable it in every defconfig.

Jordan

> BR,
> -R
> 
> > Kconfig has:
> >     depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
> >
> > So maybe not all of the world but all QCOM or IMX5 users. Maybe they are all
> > interested in MSM. Otherwise the default should rather be dropped.
> > If there is any good hints then the help text could anyway use some
> > love, and then add the info there.
> >
> > The other change with QCOM_COMMAND_DB seems on the other hand to make
> > sense but then this is another patch.
> >
> >         Sam
> > _______________________________________________
> > Freedreno mailing list
> > Freedreno@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [Freedreno] [PATCH] drm/msm: Make DRM_MSM default to 'm'

[Sam Ravnborg]

Hi Jordan/Rob.

On Wed, Aug 07, 2019 at 12:46:49PM -0600, Jordan Crouse wrote:
> On Wed, Aug 07, 2019 at 11:08:53AM -0700, Rob Clark wrote:
> > On Wed, Aug 7, 2019 at 10:38 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> > >
> > > Hi Jordan.
> > > On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote:
> > > > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as
> > > > modules but there are some cases where DRM might be built in for whatever
> > > > reason and in those situations it is preferable to still keep MSM as a
> > > > module by default and let the user decide if they _really_ want to build
> > > > it in.
> > > >
> > > > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure
> > > > it doesn't get missed when we need it for a6xx tarets.
> > > >
> > > > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > > > ---
> > > >
> > > >  drivers/gpu/drm/msm/Kconfig | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> > > > index 9c37e4d..3b2334b 100644
> > > > --- a/drivers/gpu/drm/msm/Kconfig
> > > > +++ b/drivers/gpu/drm/msm/Kconfig
> > > > @@ -14,11 +14,12 @@ config DRM_MSM
> > > >       select SHMEM
> > > >       select TMPFS
> > > >       select QCOM_SCM if ARCH_QCOM
> > > > +     select QCOM_COMMAND_DB if ARCH_QCOM
> > > >       select WANT_DEV_COREDUMP
> > > >       select SND_SOC_HDMI_CODEC if SND_SOC
> > > >       select SYNC_FILE
> > > >       select PM_OPP
> > > > -     default y
> > > > +     default m
> > >
> > > As a general comment the right thing would be to drop this default.
> > > As it is now the Kconfig says that when DRM is selected then all of the
> > > world would then also get DRM_MSM, which only a small part of this world
> > > you see any benefit in.
> > > So they now have to de-select MSM.
> > 
> > If the default is dropped, it should probably be accompanied by adding
> > CONFIG_DRM_MSM=m to defconfig's, I think
That would be best. So the defconfigs end up with the same config as
before.

> 
> In general I prefer to not use a default but this is the only GPU driver for
> ARCH_QCOM and I think its safe to stay that 99% of ARCH_QCOM users would select
> this module and those that wouldn't will omit DRM entirely.
> 
> I feel it is net negative if we dropped the default but then had to turn around
> and enable it in every defconfig.
"in every" equals three defconfigs:
$ git grep ARCH_QCOM | grep defconfig
arch/arm/configs/multi_v7_defconfig:CONFIG_ARCH_QCOM=y
arch/arm/configs/qcom_defconfig:CONFIG_ARCH_QCOM=y
arch/arm64/configs/defconfig:CONFIG_ARCH_QCOM=y

	Sam