diff mbox

[1/2] drm: fix drm_vm for NOMMU builds

Message ID 20170111133357.3664191-1-arnd@arndb.de (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann Jan. 11, 2017, 1:33 p.m. UTC
When building DRM without an MMU, we run into a compile-time error because
pte_wrprotect() is not defined:

drivers/gpu/drm/drm_vm.c: In function 'drm_mmap_dma':
drivers/gpu/drm/drm_vm.c:496:9: error: implicit declaration of function 'pte_wrprotect' [-Werror=implicit-function-declaration]

The line is not meaningful here, so we can simply add another
compile-time check around it.

Fixes: 62a0d98a188c ("drm: allow to use mmuless SoC")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/drm_vm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Jan. 11, 2017, 4:27 p.m. UTC | #1
On Wed, Jan 11, 2017 at 02:33:34PM +0100, Arnd Bergmann wrote:
> When building DRM without an MMU, we run into a compile-time error because
> pte_wrprotect() is not defined:
> 
> drivers/gpu/drm/drm_vm.c: In function 'drm_mmap_dma':
> drivers/gpu/drm/drm_vm.c:496:9: error: implicit declaration of function 'pte_wrprotect' [-Werror=implicit-function-declaration]
> 
> The line is not meaningful here, so we can simply add another
> compile-time check around it.
> 
> Fixes: 62a0d98a188c ("drm: allow to use mmuless SoC")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

We don't need drm_vm.c on modern drivers, and the idea was to simply not
compile it when not needed. See:

commit 99c48e1e38f0aeaa107ad67c8d91f6c9d9d567a9
Author: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Date:   Wed Jan 4 10:12:56 2017 +0100

    drm: compile drm_vm.c only when needed


How did you manage to enable this stuff?
-Daniel

> ---
>  drivers/gpu/drm/drm_vm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index bd311c77c254..038946588ed7 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -488,7 +488,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma)
>  		vma->vm_flags &= ~(VM_WRITE | VM_MAYWRITE);
>  #if defined(__i386__) || defined(__x86_64__)
>  		pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW;
> -#else
> +#elif IS_ENABLED(CONFIG_MMU)
>  		/* Ye gads this is ugly.  With more thought
>  		   we could move this up higher and use
>  		   `protection_map' instead.  */
> @@ -572,7 +572,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma)
>  		vma->vm_flags &= ~(VM_WRITE | VM_MAYWRITE);
>  #if defined(__i386__) || defined(__x86_64__)
>  		pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW;
> -#else
> +#elif defined (CONFIG_MMU)
>  		/* Ye gads this is ugly.  With more thought
>  		   we could move this up higher and use
>  		   `protection_map' instead.  */
> -- 
> 2.9.0
>
Arnd Bergmann Jan. 11, 2017, 4:33 p.m. UTC | #2
On Wednesday, January 11, 2017 5:27:13 PM CET Daniel Vetter wrote:
> On Wed, Jan 11, 2017 at 02:33:34PM +0100, Arnd Bergmann wrote:
> > When building DRM without an MMU, we run into a compile-time error because
> > pte_wrprotect() is not defined:
> > 
> > drivers/gpu/drm/drm_vm.c: In function 'drm_mmap_dma':
> > drivers/gpu/drm/drm_vm.c:496:9: error: implicit declaration of function 'pte_wrprotect' [-Werror=implicit-function-declaration]
> > 
> > The line is not meaningful here, so we can simply add another
> > compile-time check around it.
> > 
> > Fixes: 62a0d98a188c ("drm: allow to use mmuless SoC")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> We don't need drm_vm.c on modern drivers, and the idea was to simply not
> compile it when not needed. See:
> 
> commit 99c48e1e38f0aeaa107ad67c8d91f6c9d9d567a9
> Author: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Date:   Wed Jan 4 10:12:56 2017 +0100
> 
>     drm: compile drm_vm.c only when needed
> 
> 
> How did you manage to enable this stuff?
> -Daniel

This was a randconfig build, the DRM specific symbols here are

CONFIG_DRM=y
CONFIG_DRM_MIPI_DSI=y
# CONFIG_DRM_DP_AUX_CHARDEV is not set
# CONFIG_DRM_DEBUG_MM is not set
# CONFIG_DRM_DEBUG_MM_SELFTEST is not set
CONFIG_DRM_KMS_HELPER=y
CONFIG_DRM_KMS_FB_HELPER=y
# CONFIG_DRM_FBDEV_EMULATION is not set
CONFIG_DRM_LOAD_EDID_FIRMWARE=y
CONFIG_DRM_GEM_CMA_HELPER=y
CONFIG_DRM_KMS_CMA_HELPER=y
CONFIG_DRM_VM=y

#
# I2C encoder or helper chips
#
# CONFIG_DRM_I2C_CH7006 is not set
CONFIG_DRM_I2C_SIL164=y
# CONFIG_DRM_I2C_NXP_TDA998X is not set
CONFIG_DRM_ARM=y
CONFIG_DRM_HDLCD=y
CONFIG_DRM_HDLCD_SHOW_UNDERRUN=y
# CONFIG_DRM_MALI_DISPLAY is not set

#
# ACP (Audio CoProcessor) Configuration
#
CONFIG_DRM_VGEM=y
CONFIG_DRM_ROCKCHIP=y
CONFIG_ROCKCHIP_ANALOGIX_DP=y
CONFIG_ROCKCHIP_DW_HDMI=m
CONFIG_ROCKCHIP_DW_MIPI_DSI=y
CONFIG_ROCKCHIP_INNO_HDMI=m
# CONFIG_DRM_UDL is not set
CONFIG_DRM_ARMADA=y
CONFIG_DRM_ATMEL_HLCDC=m
CONFIG_DRM_RCAR_DU=y
# CONFIG_DRM_RCAR_HDMI is not set
CONFIG_DRM_RCAR_LVDS=y
# CONFIG_DRM_SHMOBILE is not set
CONFIG_DRM_SUN4I=y
CONFIG_DRM_TILCDC=m
CONFIG_DRM_TILCDC_SLAVE_COMPAT=y
CONFIG_DRM_MSM=y
CONFIG_DRM_MSM_REGISTER_LOGGING=y
CONFIG_DRM_MSM_HDMI_HDCP=y
CONFIG_DRM_MSM_DSI=y
# CONFIG_DRM_MSM_DSI_PLL is not set
# CONFIG_DRM_MSM_DSI_28NM_PHY is not set
# CONFIG_DRM_MSM_DSI_20NM_PHY is not set
CONFIG_DRM_MSM_DSI_28NM_8960_PHY=y
# CONFIG_DRM_FSL_DCU is not set
# CONFIG_DRM_TEGRA is not set
CONFIG_DRM_PANEL=y

#
# Display Panels
#
CONFIG_DRM_PANEL_SIMPLE=m
CONFIG_DRM_PANEL_JDI_LT070ME05000=m
CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00=m
# CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0 is not set
# CONFIG_DRM_PANEL_SHARP_LQ101R1SX01 is not set
# CONFIG_DRM_PANEL_SHARP_LS043T1LE01 is not set
CONFIG_DRM_BRIDGE=y

#
# Display Interface Bridges
#
CONFIG_DRM_ANALOGIX_ANX78XX=y
CONFIG_DRM_DUMB_VGA_DAC=m
CONFIG_DRM_DW_HDMI=m
CONFIG_DRM_DW_HDMI_AHB_AUDIO=m
# CONFIG_DRM_NXP_PTN3460 is not set
CONFIG_DRM_PARADE_PS8622=m
# CONFIG_DRM_SIL_SII8620 is not set
# CONFIG_DRM_SII902X is not set
# CONFIG_DRM_TOSHIBA_TC358767 is not set
CONFIG_DRM_TI_TFP410=y
CONFIG_DRM_ANALOGIX_DP=y
# CONFIG_DRM_I2C_ADV7511 is not set
CONFIG_DRM_VC4=m
CONFIG_DRM_ETNAVIV=m
# CONFIG_DRM_ETNAVIV_REGISTER_LOGGING is not set
CONFIG_DRM_ARCPGU=m
CONFIG_DRM_MXS=y
CONFIG_DRM_MXSFB=m
CONFIG_DRM_MESON=y
CONFIG_DRM_LEGACY=y
# CONFIG_DRM_LIB_RANDOM is not set

No idea what did it in the end.

I also have some older build fixes applied, so it may be something that
happens on my tree but not on plain linux-next.

	Arnd
Daniel Vetter Jan. 11, 2017, 4:36 p.m. UTC | #3
On Wed, Jan 11, 2017 at 5:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday, January 11, 2017 5:27:13 PM CET Daniel Vetter wrote:
>> On Wed, Jan 11, 2017 at 02:33:34PM +0100, Arnd Bergmann wrote:
>> > When building DRM without an MMU, we run into a compile-time error because
>> > pte_wrprotect() is not defined:
>> >
>> > drivers/gpu/drm/drm_vm.c: In function 'drm_mmap_dma':
>> > drivers/gpu/drm/drm_vm.c:496:9: error: implicit declaration of function 'pte_wrprotect' [-Werror=implicit-function-declaration]
>> >
>> > The line is not meaningful here, so we can simply add another
>> > compile-time check around it.
>> >
>> > Fixes: 62a0d98a188c ("drm: allow to use mmuless SoC")
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> We don't need drm_vm.c on modern drivers, and the idea was to simply not
>> compile it when not needed. See:
>>
>> commit 99c48e1e38f0aeaa107ad67c8d91f6c9d9d567a9
>> Author: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Date:   Wed Jan 4 10:12:56 2017 +0100
>>
>>     drm: compile drm_vm.c only when needed
>>
>>
>> How did you manage to enable this stuff?
>> -Daniel
>
> This was a randconfig build, the DRM specific symbols here are
>
> CONFIG_DRM=y
> CONFIG_DRM_MIPI_DSI=y
> # CONFIG_DRM_DP_AUX_CHARDEV is not set
> # CONFIG_DRM_DEBUG_MM is not set
> # CONFIG_DRM_DEBUG_MM_SELFTEST is not set
> CONFIG_DRM_KMS_HELPER=y
> CONFIG_DRM_KMS_FB_HELPER=y
> # CONFIG_DRM_FBDEV_EMULATION is not set
> CONFIG_DRM_LOAD_EDID_FIRMWARE=y
> CONFIG_DRM_GEM_CMA_HELPER=y
> CONFIG_DRM_KMS_CMA_HELPER=y
> CONFIG_DRM_VM=y

Does randconfig just set this for fun, despite that it's a hidden
Kconfig symbol? Should we add a depends !NOMMU to it to make sure it
never gets enabled when it shouldn't be?

tbh I have no idea how Kconfig works, I'm just really good at breaking it :(
-Daniel
Arnd Bergmann Jan. 11, 2017, 4:59 p.m. UTC | #4
On Wed, Jan 11, 2017 at 5:36 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jan 11, 2017 at 5:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> CONFIG_DRM_VM=y
>
> Does randconfig just set this for fun, despite that it's a hidden
> Kconfig symbol? Should we add a depends !NOMMU to it to make sure it
> never gets enabled when it shouldn't be?
>
> tbh I have no idea how Kconfig works, I'm just really good at breaking it :(

no, randconfig won't try to set symbols that a user doesn't see. This
is what 'menuconfig'
says enabled it.

  │   Selected by: DRM_NOUVEAU [=n] && HAS_IOMEM [=y] && DRM [=y] &&
PCI [=n] && MMU [=n] && (BACKLIGHT_CLASS_DEVICE [=m] || !ACPI) ||
DRM_LEGACY [=y] && HAS_IOMEM [=y] && DRM [=y]

It must be the 'DRM_LEGACY' option that is actually user visible and
that contains
'select DRM_VM'.

     Arnd
Benjamin Gaignard Jan. 11, 2017, 5:03 p.m. UTC | #5
2017-01-11 17:59 GMT+01:00 Arnd Bergmann <arnd@arndb.de>:
> On Wed, Jan 11, 2017 at 5:36 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Jan 11, 2017 at 5:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> CONFIG_DRM_VM=y
>>
>> Does randconfig just set this for fun, despite that it's a hidden
>> Kconfig symbol? Should we add a depends !NOMMU to it to make sure it
>> never gets enabled when it shouldn't be?
>>
>> tbh I have no idea how Kconfig works, I'm just really good at breaking it :(
>
> no, randconfig won't try to set symbols that a user doesn't see. This
> is what 'menuconfig'
> says enabled it.
>
>   │   Selected by: DRM_NOUVEAU [=n] && HAS_IOMEM [=y] && DRM [=y] &&
> PCI [=n] && MMU [=n] && (BACKLIGHT_CLASS_DEVICE [=m] || !ACPI) ||
> DRM_LEGACY [=y] && HAS_IOMEM [=y] && DRM [=y]
>
> It must be the 'DRM_LEGACY' option that is actually user visible and
> that contains
> 'select DRM_VM'.

We should add "MMU" on DRM_LEGACY and DRM_VM dependencies
to be sure that they won't be select without MMU support

>
>      Arnd
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index bd311c77c254..038946588ed7 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -488,7 +488,7 @@  static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma)
 		vma->vm_flags &= ~(VM_WRITE | VM_MAYWRITE);
 #if defined(__i386__) || defined(__x86_64__)
 		pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW;
-#else
+#elif IS_ENABLED(CONFIG_MMU)
 		/* Ye gads this is ugly.  With more thought
 		   we could move this up higher and use
 		   `protection_map' instead.  */
@@ -572,7 +572,7 @@  static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma)
 		vma->vm_flags &= ~(VM_WRITE | VM_MAYWRITE);
 #if defined(__i386__) || defined(__x86_64__)
 		pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW;
-#else
+#elif defined (CONFIG_MMU)
 		/* Ye gads this is ugly.  With more thought
 		   we could move this up higher and use
 		   `protection_map' instead.  */