[PATCH] drm/imagination: Kconfig: add 'PAGE_SIZE=4K' dependency

[PATCH] drm/imagination: Kconfig: add 'PAGE_SIZE=4K' dependency

[Lu Yao]

When 'PAGE_SIZE=64K',the following compilation error occurs:
"
  ../drivers/gpu/drm/imagination/pvr_fw_mips.c: In function
‘pvr_mips_fw_process’:
  ../drivers/gpu/drm/imagination/pvr_fw_mips.c:140:60: error: array
subscript 0 is outside the bounds of an interior zero-length array
‘dma_addr_t[0]’ {aka ‘long long unsigned int[]’}
[-Werror=zero-length-bounds]
  140 |   boot_data->pt_phys_addr[page_nr] =
mips_data->pt_dma_addr[src_page_nr] +
~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
  In file included from ../drivers/gpu/drm/imagination/pvr_fw_mips.c:6:
  ../drivers/gpu/drm/imagination/pvr_fw_mips.h:30:13: note: while
referencing ‘pt_dma_addr’
   30 |  dma_addr_t pt_dma_addr[PVR_MIPS_PT_PAGE_COUNT];
"

This is because 'PVR_MIPS_PT_PAGE_COUNT' is defined as
'(ROGUE_MIPSFW_MAX_NUM_PAGETABLE_PAGES * ROGUE_MIPSFW_PAGE_SIZE_4K) \
>> PAGE_SHIFT', and under the above conditions, 'PAGE_SHIFT' is '16',
'ROGUE_MIPSFW_MAX_NUM_PAGETABLE_PAGES' is '4','ROGUE_MIPSFW_PAGE_SIZE_4K'
is '4096',so 'PVR_MIPS_PT_PAGE_COUNT' is '0' causing the member
'pt_dma_addr' to be incorrectly defined.

Signed-off-by: Lu Yao <yaolu@kylinos.cn>
---
 drivers/gpu/drm/imagination/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
index 3bfa2ac212dc..e585922f634d 100644
--- a/drivers/gpu/drm/imagination/Kconfig
+++ b/drivers/gpu/drm/imagination/Kconfig
@@ -3,7 +3,7 @@
 
 config DRM_POWERVR
 	tristate "Imagination Technologies PowerVR (Series 6 and later) & IMG Graphics"
-	depends on ARM64
+	depends on (ARM64 && ARM64_PAGE_SHIFT=12)
 	depends on DRM
 	depends on PM
 	select DRM_EXEC
-- 
2.25.1

Re: [PATCH] drm/imagination: Kconfig: add 'PAGE_SIZE=4K' dependency

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 2675 bytes --]

Hi, thanks for the patch!

On 28/02/2024 01:23, Lu Yao wrote:
> When 'PAGE_SIZE=64K',the following compilation error occurs:
> "
>   ../drivers/gpu/drm/imagination/pvr_fw_mips.c: In function
> ‘pvr_mips_fw_process’:
>   ../drivers/gpu/drm/imagination/pvr_fw_mips.c:140:60: error: array
> subscript 0 is outside the bounds of an interior zero-length array
> ‘dma_addr_t[0]’ {aka ‘long long unsigned int[]’}
> [-Werror=zero-length-bounds]
>   140 |   boot_data->pt_phys_addr[page_nr] =
> mips_data->pt_dma_addr[src_page_nr] +
> ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
>   In file included from ../drivers/gpu/drm/imagination/pvr_fw_mips.c:6:
>   ../drivers/gpu/drm/imagination/pvr_fw_mips.h:30:13: note: while
> referencing ‘pt_dma_addr’
>    30 |  dma_addr_t pt_dma_addr[PVR_MIPS_PT_PAGE_COUNT];
> "
> 
> This is because 'PVR_MIPS_PT_PAGE_COUNT' is defined as
> '(ROGUE_MIPSFW_MAX_NUM_PAGETABLE_PAGES * ROGUE_MIPSFW_PAGE_SIZE_4K) \
>>> PAGE_SHIFT', and under the above conditions, 'PAGE_SHIFT' is '16',
> 'ROGUE_MIPSFW_MAX_NUM_PAGETABLE_PAGES' is '4','ROGUE_MIPSFW_PAGE_SIZE_4K'
> is '4096',so 'PVR_MIPS_PT_PAGE_COUNT' is '0' causing the member
> 'pt_dma_addr' to be incorrectly defined.

The whole MIPS page table system is supposed to be host page-size
agnostic. In practice, we don’t regularly test on non-4K platforms so
this may well be broken in subtle or not-so-subtle (as in this instance)
ways. There has been at least some testing with 64K host pages – the
configs from TI for the AM62-SK dev boards have that as the default (or
at least they did when we started working with them).

I’m loathed to accept this patch without at least investigating how deep
the problems go; the true fix here should be to fix the problems causing
this build error rather than just gating off non-4K hosts.

I’ll have a dig this afternoon to see what I can find. Did you try
anything to fix this yourself? It would be nice to not duplicate effort
on this if you have.

Cheers,
Matt

> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> ---
>  drivers/gpu/drm/imagination/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
> index 3bfa2ac212dc..e585922f634d 100644
> --- a/drivers/gpu/drm/imagination/Kconfig
> +++ b/drivers/gpu/drm/imagination/Kconfig
> @@ -3,7 +3,7 @@
>  
>  config DRM_POWERVR
>  	tristate "Imagination Technologies PowerVR (Series 6 and later) & IMG Graphics"
> -	depends on ARM64
> +	depends on (ARM64 && ARM64_PAGE_SHIFT=12)
>  	depends on DRM
>  	depends on PM
>  	select DRM_EXEC

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH] drm/imagination: Kconfig: add 'PAGE_SIZE=4K' dependency

[yaolu]

[-- Attachment #1: Type: text/plain, Size: 3058 bytes --]


在 2024/2/28 18:47, Matt Coster 写道:
> Hi, thanks for the patch!
>
> On 28/02/2024 01:23, Lu Yao wrote:
>> When 'PAGE_SIZE=64K',the following compilation error occurs:
>> "
>>    ../drivers/gpu/drm/imagination/pvr_fw_mips.c: In function
>> ‘pvr_mips_fw_process’:
>>    ../drivers/gpu/drm/imagination/pvr_fw_mips.c:140:60: error: array
>> subscript 0 is outside the bounds of an interior zero-length array
>> ‘dma_addr_t[0]’ {aka ‘long long unsigned int[]’}
>> [-Werror=zero-length-bounds]
>>    140 |   boot_data->pt_phys_addr[page_nr] =
>> mips_data->pt_dma_addr[src_page_nr] +
>> ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
>>    In file included from ../drivers/gpu/drm/imagination/pvr_fw_mips.c:6:
>>    ../drivers/gpu/drm/imagination/pvr_fw_mips.h:30:13: note: while
>> referencing ‘pt_dma_addr’
>>     30 |  dma_addr_t pt_dma_addr[PVR_MIPS_PT_PAGE_COUNT];
>> "
>>
>> This is because 'PVR_MIPS_PT_PAGE_COUNT' is defined as
>> '(ROGUE_MIPSFW_MAX_NUM_PAGETABLE_PAGES * ROGUE_MIPSFW_PAGE_SIZE_4K) \
>>>> PAGE_SHIFT', and under the above conditions, 'PAGE_SHIFT' is '16',
>> 'ROGUE_MIPSFW_MAX_NUM_PAGETABLE_PAGES' is '4','ROGUE_MIPSFW_PAGE_SIZE_4K'
>> is '4096',so 'PVR_MIPS_PT_PAGE_COUNT' is '0' causing the member
>> 'pt_dma_addr' to be incorrectly defined.
> The whole MIPS page table system is supposed to be host page-size
> agnostic. In practice, we don’t regularly test on non-4K platforms so
> this may well be broken in subtle or not-so-subtle (as in this instance)
> ways. There has been at least some testing with 64K host pages – the
> configs from TI for the AM62-SK dev boards have that as the default (or
> at least they did when we started working with them).
>
> I’m loathed to accept this patch without at least investigating how deep
> the problems go; the true fix here should be to fix the problems causing
> this build error rather than just gating off non-4K hosts.
>
> I’ll have a dig this afternoon to see what I can find. Did you try
> anything to fix this yourself? It would be nice to not duplicate effort
> on this if you have.

No, I haven't tried, and I'm currently disabling the DRM_POWERVR driver. Actually,
I don't have any IMG graphics cards. This error was found when  compiling the kernel
randomly.Looks like it's up to you to sort this out reasonably.;)

Regards,
Lu

> Cheers,
> Matt
>
>> Signed-off-by: Lu Yao<yaolu@kylinos.cn>
>> ---
>>   drivers/gpu/drm/imagination/Kconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
>> index 3bfa2ac212dc..e585922f634d 100644
>> --- a/drivers/gpu/drm/imagination/Kconfig
>> +++ b/drivers/gpu/drm/imagination/Kconfig
>> @@ -3,7 +3,7 @@
>>   
>>   config DRM_POWERVR
>>   	tristate "Imagination Technologies PowerVR (Series 6 and later) & IMG Graphics"
>> -	depends on ARM64
>> +	depends on (ARM64 && ARM64_PAGE_SHIFT=12)
>>   	depends on DRM
>>   	depends on PM
>>   	select DRM_EXEC

[-- Attachment #2: Type: text/html, Size: 4010 bytes --]