[PATCH] drm: arm-hdlcd: remove DMA_CMA select

Subsystems: arm mali-dp drm driver, drm drivers, drm drivers and misc gpu patches, the rest

STALE3926d

15 messages, 3 authors, 2016-01-11 · open the first message on its own page

[PATCH] drm: arm-hdlcd: remove DMA_CMA select

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-01 14:09:42

The newly added DRM_HDLCD driver tries to select DMA_CMA, but that is
not necessarily possible, as not all configurations contain HAVE_DMA_CONTIGUOUS:

warning: (DRM_HDLCD) selects DMA_CMA which has unmet direct dependencies (HAVE_DMA_CONTIGUOUS && CMA)
drivers/built-in.o: In function `dma_alloc_from_contiguous':
:(.text+0x1dee00): undefined reference to `cma_alloc'
drivers/built-in.o: In function `dma_release_from_contiguous':
:(.text+0x1dee24): undefined reference to `cma_release'

This removes the 'select' statement. It is not needed because CMA is meant
to transparently change the behavior of dma_alloc_coherent to make it succeed
for larger allocations, but there is no actual build-time dependency, and
the driver can still work without CMA in many cases.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 1561e558334d ("drm: Add support for ARM's HDLCD controller.")
---
Found on ARM randconfig builds with yesterday's linux-next
diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
index 5e8c8a86860b..2f4d3b7fb871 100644
--- a/drivers/gpu/drm/arm/Kconfig
+++ b/drivers/gpu/drm/arm/Kconfig
@@ -10,7 +10,6 @@ config DRM_HDLCD
 	depends on DRM_ARM
 	depends on COMMON_CLK
 	select COMMON_CLK_SCPI
-	select DMA_CMA
 	select DRM_KMS_CMA_HELPER
 	select DRM_GEM_CMA_HELPER
 	help

[PATCH 2/3] drm: arm-hdlcd: remove COMMON_CLK_SCPI select

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-01 14:39:29

The hdlcd driver has no build-time dependency on the SCPI clock
and the bogus 'select' causes a warning when SCPI is disabled:

warning: (DRM_HDLCD) selects COMMON_CLK_SCPI which has unmet direct dependencies (COMMON_CLK && (ARM_SCPI_PROTOCOL || COMPILE_TEST))

This removes the select statement.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Found two more bugs in the same driver, so this is now a patch series
diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
index 2f4d3b7fb871..81ea7802ca50 100644
--- a/drivers/gpu/drm/arm/Kconfig
+++ b/drivers/gpu/drm/arm/Kconfig
@@ -9,7 +9,6 @@ config DRM_HDLCD
 	tristate "ARM HDLCD"
 	depends on DRM_ARM
 	depends on COMMON_CLK
-	select COMMON_CLK_SCPI
 	select DRM_KMS_CMA_HELPER
 	select DRM_GEM_CMA_HELPER
 	help

[PATCH 3/3] drm: arm-hdlcd: fix build warnings for !CONFIG_PM

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-01 14:40:19

The hdlcd_pm_suspend and hdlcd_pm_resume are intentionally unused
when CONFIG_PM is not set in this driver, and we get a compiler warning
for this:

drivers/gpu/drm/arm/hdlcd_drv.c:521:12: warning: 'hdlcd_pm_suspend' defined but not used [-Wunused-function]
drivers/gpu/drm/arm/hdlcd_drv.c:536:12: warning: 'hdlcd_pm_resume' defined but not used [-Wunused-function]

This adds a __maybe_unused annotation for the two functions, to let
the compiler know this is the intended behavior. This is better than
an #ifdef as it improves the compile time coverage of the code and it
looks nicer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 31969efe97d4..6173bb835223 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -518,7 +518,7 @@ static const struct of_device_id  hdlcd_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, hdlcd_of_match);
 
-static int hdlcd_pm_suspend(struct device *dev)
+static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
 	struct drm_crtc *crtc;
@@ -533,7 +533,7 @@ static int hdlcd_pm_suspend(struct device *dev)
 	return 0;
 }
 
-static int hdlcd_pm_resume(struct device *dev)
+static int __maybe_unused hdlcd_pm_resume(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
 	struct drm_crtc *crtc;

[PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-01 22:04:48

CONFIG_DRM_HDLCD is a tristate option that depends on the boolean
CONFIG_DRM_ARM, which in turn depends on the tristate CONFIG_DRM.
The effect of this is that a configuration with CONFIG_DRM=m
and CONFIG_DRM_HDLCD=y can be chosen, but won't link because the
DRM core symbols are not reachable from builtin code:

drivers/built-in.o: In function `hdlcd_drm_unbind':
drivers/gpu/drm/arm/hdlcd_drv.c:445: undefined reference to `drm_fbdev_cma_fini'
drivers/gpu/drm/arm/hdlcd_drv.c:448: undefined reference to `drm_kms_helper_poll_fini'
drivers/gpu/drm/arm/hdlcd_drv.c:450: undefined reference to `drm_vblank_cleanup'
drivers/gpu/drm/arm/hdlcd_drv.c:452: undefined reference to `drm_irq_uninstall'
drivers/gpu/drm/arm/hdlcd_drv.c:460: undefined reference to `drm_mode_config_cleanup'
drivers/gpu/drm/arm/hdlcd_drv.c:461: undefined reference to `drm_dev_unregister'
drivers/gpu/drm/arm/hdlcd_drv.c:462: undefined reference to `drm_dev_unref'
...

This adds another dependency on CONFIG_DRM to enforce that DRM_HDLCD
cannot be builtin if DRM is not.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Ok, I found yet another one after a few hundred extra randconfig builds
on today's next. Let me know if you'd rather have the three Kconfig
changes combined into a single patch, it's starting to get ridiculous.
diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
index 81ea7802ca50..487fb1f0c979 100644
--- a/drivers/gpu/drm/arm/Kconfig
+++ b/drivers/gpu/drm/arm/Kconfig
@@ -7,7 +7,7 @@ config DRM_ARM
 
 config DRM_HDLCD
 	tristate "ARM HDLCD"
-	depends on DRM_ARM
+	depends on DRM_ARM && DRM
 	depends on COMMON_CLK
 	select DRM_KMS_CMA_HELPER
 	select DRM_GEM_CMA_HELPER

Re: [PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Thierry Reding <hidden>
Date: 2016-01-04 08:24:20

On Fri, Jan 01, 2016 at 11:04:07PM +0100, Arnd Bergmann wrote:
CONFIG_DRM_HDLCD is a tristate option that depends on the boolean
CONFIG_DRM_ARM, which in turn depends on the tristate CONFIG_DRM.
The effect of this is that a configuration with CONFIG_DRM=m
and CONFIG_DRM_HDLCD=y can be chosen, but won't link because the
DRM core symbols are not reachable from builtin code:

drivers/built-in.o: In function `hdlcd_drm_unbind':
drivers/gpu/drm/arm/hdlcd_drv.c:445: undefined reference to `drm_fbdev_cma_fini'
drivers/gpu/drm/arm/hdlcd_drv.c:448: undefined reference to `drm_kms_helper_poll_fini'
drivers/gpu/drm/arm/hdlcd_drv.c:450: undefined reference to `drm_vblank_cleanup'
drivers/gpu/drm/arm/hdlcd_drv.c:452: undefined reference to `drm_irq_uninstall'
drivers/gpu/drm/arm/hdlcd_drv.c:460: undefined reference to `drm_mode_config_cleanup'
drivers/gpu/drm/arm/hdlcd_drv.c:461: undefined reference to `drm_dev_unregister'
drivers/gpu/drm/arm/hdlcd_drv.c:462: undefined reference to `drm_dev_unref'
...

This adds another dependency on CONFIG_DRM to enforce that DRM_HDLCD
cannot be builtin if DRM is not.
Ugh... wouldn't it be much simpler to get rid of DRM_ARM? It seems like
a completely superfluous option to me. I don't think we've ever had the
equivalent of "vendor" Kconfig options in DRM, and I don't see why we'd
need to start now. If ARM was going to add another driver it can simply
have a separate Kconfig entry. There should be no need to select the
vendor option first.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160104/2937e3ba/attachment.sig>

Re: [PATCH 3/3] drm: arm-hdlcd: fix build warnings for !CONFIG_PM

From: Thierry Reding <hidden>
Date: 2016-01-04 08:24:36

On Fri, Jan 01, 2016 at 03:39:17PM +0100, Arnd Bergmann wrote:
The hdlcd_pm_suspend and hdlcd_pm_resume are intentionally unused
when CONFIG_PM is not set in this driver, and we get a compiler warning
for this:

drivers/gpu/drm/arm/hdlcd_drv.c:521:12: warning: 'hdlcd_pm_suspend' defined but not used [-Wunused-function]
drivers/gpu/drm/arm/hdlcd_drv.c:536:12: warning: 'hdlcd_pm_resume' defined but not used [-Wunused-function]

This adds a __maybe_unused annotation for the two functions, to let
the compiler know this is the intended behavior. This is better than
an #ifdef as it improves the compile time coverage of the code and it
looks nicer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thierry Reding <redacted>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160104/9e2bae63/attachment.sig>

Re: [PATCH 2/3] drm: arm-hdlcd: remove COMMON_CLK_SCPI select

From: Thierry Reding <hidden>
Date: 2016-01-04 08:26:50

On Fri, Jan 01, 2016 at 03:38:53PM +0100, Arnd Bergmann wrote:
The hdlcd driver has no build-time dependency on the SCPI clock
and the bogus 'select' causes a warning when SCPI is disabled:

warning: (DRM_HDLCD) selects COMMON_CLK_SCPI which has unmet direct dependencies (COMMON_CLK && (ARM_SCPI_PROTOCOL || COMPILE_TEST))

This removes the select statement.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Found two more bugs in the same driver, so this is now a patch series
Reviewed-by: Thierry Reding <redacted>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160104/109467e9/attachment.sig>

Re: [PATCH] drm: arm-hdlcd: remove DMA_CMA select

From: Thierry Reding <hidden>
Date: 2016-01-04 08:27:31

On Fri, Jan 01, 2016 at 03:09:10PM +0100, Arnd Bergmann wrote:
The newly added DRM_HDLCD driver tries to select DMA_CMA, but that is
not necessarily possible, as not all configurations contain HAVE_DMA_CONTIGUOUS:

warning: (DRM_HDLCD) selects DMA_CMA which has unmet direct dependencies (HAVE_DMA_CONTIGUOUS && CMA)
drivers/built-in.o: In function `dma_alloc_from_contiguous':
:(.text+0x1dee00): undefined reference to `cma_alloc'
drivers/built-in.o: In function `dma_release_from_contiguous':
:(.text+0x1dee24): undefined reference to `cma_release'

This removes the 'select' statement. It is not needed because CMA is meant
to transparently change the behavior of dma_alloc_coherent to make it succeed
for larger allocations, but there is no actual build-time dependency, and
the driver can still work without CMA in many cases.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 1561e558334d ("drm: Add support for ARM's HDLCD controller.")
---
Found on ARM randconfig builds with yesterday's linux-next
Reviewed-by: Thierry Reding <redacted>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160104/e10f1e0d/attachment.sig>

Re: [PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-04 08:40:27

On Monday 04 January 2016 09:24:16 Thierry Reding wrote:
Ugh... wouldn't it be much simpler to get rid of DRM_ARM? It seems like
a completely superfluous option to me. I don't think we've ever had the
equivalent of "vendor" Kconfig options in DRM, and I don't see why we'd
need to start now. If ARM was going to add another driver it can simply
have a separate Kconfig entry. There should be no need to select the
vendor option first.
Fine with me too. I vaguely remembered having seen some discussion about
this, so I decided to do a minimal fix, but I agree that would be more
in line with the other drivers.

	Arnd

Re: [PATCH] drm: arm-hdlcd: remove DMA_CMA select

From: Liviu Dudau <liviu.dudau@arm.com>
Date: 2016-01-11 10:37:22

On Fri, Jan 01, 2016 at 03:09:10PM +0100, Arnd Bergmann wrote:
The newly added DRM_HDLCD driver tries to select DMA_CMA, but that is
not necessarily possible, as not all configurations contain HAVE_DMA_CONTIGUOUS:

warning: (DRM_HDLCD) selects DMA_CMA which has unmet direct dependencies (HAVE_DMA_CONTIGUOUS && CMA)
drivers/built-in.o: In function `dma_alloc_from_contiguous':
:(.text+0x1dee00): undefined reference to `cma_alloc'
drivers/built-in.o: In function `dma_release_from_contiguous':
:(.text+0x1dee24): undefined reference to `cma_release'

This removes the 'select' statement. It is not needed because CMA is meant
to transparently change the behavior of dma_alloc_coherent to make it succeed
for larger allocations, but there is no actual build-time dependency, and
the driver can still work without CMA in many cases.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 1561e558334d ("drm: Add support for ARM's HDLCD controller.")
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>

Arnd, are you going to send these corrections yourself or should I pull them
into my tree (possibly squashing them before sending the pull request to
Dave Airlie) ?

Best regards,
Liviu
quoted hunk
---
Found on ARM randconfig builds with yesterday's linux-next
diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
index 5e8c8a86860b..2f4d3b7fb871 100644
--- a/drivers/gpu/drm/arm/Kconfig
+++ b/drivers/gpu/drm/arm/Kconfig
@@ -10,7 +10,6 @@ config DRM_HDLCD
 	depends on DRM_ARM
 	depends on COMMON_CLK
 	select COMMON_CLK_SCPI
-	select DMA_CMA
 	select DRM_KMS_CMA_HELPER
 	select DRM_GEM_CMA_HELPER
 	help
-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

Re: [PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Liviu Dudau <liviu.dudau@arm.com>
Date: 2016-01-11 11:12:59

On Mon, Jan 04, 2016 at 09:39:46AM +0100, Arnd Bergmann wrote:
On Monday 04 January 2016 09:24:16 Thierry Reding wrote:
quoted
Ugh... wouldn't it be much simpler to get rid of DRM_ARM? It seems like
a completely superfluous option to me. I don't think we've ever had the
equivalent of "vendor" Kconfig options in DRM, and I don't see why we'd
need to start now. If ARM was going to add another driver it can simply
have a separate Kconfig entry. There should be no need to select the
vendor option first.
Fine with me too. I vaguely remembered having seen some discussion about
this, so I decided to do a minimal fix, but I agree that would be more
in line with the other drivers.

	Arnd
Arnd,

I'm OK with the whole series of Kconfig clean-up/fixes and I offer my appologies
for adding noise with my patchset. Please let me know how do you prefer to handle
them. I'm OK with merging your changes into my series before sending the pull
request to David Airlie.

Best regards,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

Re: [PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-11 12:19:30

On Monday 11 January 2016 11:12:56 Liviu Dudau wrote:
On Mon, Jan 04, 2016 at 09:39:46AM +0100, Arnd Bergmann wrote:
quoted
On Monday 04 January 2016 09:24:16 Thierry Reding wrote:
quoted
Ugh... wouldn't it be much simpler to get rid of DRM_ARM? It seems like
a completely superfluous option to me. I don't think we've ever had the
equivalent of "vendor" Kconfig options in DRM, and I don't see why we'd
need to start now. If ARM was going to add another driver it can simply
have a separate Kconfig entry. There should be no need to select the
vendor option first.
Fine with me too. I vaguely remembered having seen some discussion about
this, so I decided to do a minimal fix, but I agree that would be more
in line with the other drivers.

      Arnd
Arnd,

I'm OK with the whole series of Kconfig clean-up/fixes and I offer my appologies
for adding noise with my patchset. Please let me know how do you prefer to handle
them. I'm OK with merging your changes into my series before sending the pull
request to David Airlie.
Please merge them into your tree if you have some other changes to send to him.

I just realized that these are in your own git tree at the moment, not
in drm-next. I guess that means you will rebase the whole series after -rc1 and
submit it into linux-4.6, right?

If so, just fold my fixes into your patches when you rebase.

	Arnd

Re: [PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Liviu Dudau <liviu.dudau@arm.com>
Date: 2016-01-11 12:26:48

On Mon, Jan 11, 2016 at 01:18:55PM +0100, Arnd Bergmann wrote:
On Monday 11 January 2016 11:12:56 Liviu Dudau wrote:
quoted
On Mon, Jan 04, 2016 at 09:39:46AM +0100, Arnd Bergmann wrote:
quoted
On Monday 04 January 2016 09:24:16 Thierry Reding wrote:
quoted
Ugh... wouldn't it be much simpler to get rid of DRM_ARM? It seems like
a completely superfluous option to me. I don't think we've ever had the
equivalent of "vendor" Kconfig options in DRM, and I don't see why we'd
need to start now. If ARM was going to add another driver it can simply
have a separate Kconfig entry. There should be no need to select the
vendor option first.
Fine with me too. I vaguely remembered having seen some discussion about
this, so I decided to do a minimal fix, but I agree that would be more
in line with the other drivers.

      Arnd
Arnd,

I'm OK with the whole series of Kconfig clean-up/fixes and I offer my appologies
for adding noise with my patchset. Please let me know how do you prefer to handle
them. I'm OK with merging your changes into my series before sending the pull
request to David Airlie.
Please merge them into your tree if you have some other changes to send to him.

I just realized that these are in your own git tree at the moment, not
in drm-next. I guess that means you will rebase the whole series after -rc1 and
submit it into linux-4.6, right?
Yes, that is the plan.
If so, just fold my fixes into your patches when you rebase.
OK, will do. Repeating the question on another thread: are you OK with me carrying
the Juno .dts changes through drm-next for HDLCD and you picking up Robin Murphy's
patch? AFAIK those are the only changes at the moment until Sudeep re-submits the
Juno r2 dts changes.

Best regards,
Liviu
	Arnd
-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

Re: [PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-01-11 14:00:48

On Monday 11 January 2016 12:26:44 Liviu Dudau wrote:
quoted
If so, just fold my fixes into your patches when you rebase.
OK, will do. Repeating the question on another thread: are you OK with me carrying
the Juno .dts changes through drm-next for HDLCD and you picking up Robin Murphy's
patch? AFAIK those are the only changes at the moment until Sudeep re-submits the
Juno r2 dts changes.
Is there a strong reason for it? Usually the preferred way is to merge all
dts changes through arm-soc, but we can make exceptions if necessary.

	Arnd

Re: [PATCH 4/3] drm: arm-hdlcd: add explictit DRM dependency

From: Liviu Dudau <liviu.dudau@arm.com>
Date: 2016-01-11 14:14:06

On Mon, Jan 11, 2016 at 02:49:10PM +0100, Arnd Bergmann wrote:
On Monday 11 January 2016 12:26:44 Liviu Dudau wrote:
quoted
quoted
If so, just fold my fixes into your patches when you rebase.
OK, will do. Repeating the question on another thread: are you OK with me carrying
the Juno .dts changes through drm-next for HDLCD and you picking up Robin Murphy's
patch? AFAIK those are the only changes at the moment until Sudeep re-submits the
Juno r2 dts changes.
Is there a strong reason for it? Usually the preferred way is to merge all
dts changes through arm-soc, but we can make exceptions if necessary.
No strong reason, just packing the series in a logical way. I'm happy to split the
pull requests. Will send the dts changes to you then.

Best regards,
Liviu
	Arnd
-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help