| Message ID | 1565198667-4300-1-git-send-email-jcrouse@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | drm/msm: Make DRM_MSM default to 'm' | expand |
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
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
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
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
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.
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(-)