Re: [PATCH v6 8/8] drm/imagination: Enable PowerVR driver for RISC-V

[Michal Wilczynski <m.wilczynski@samsung.com>]

On 6/24/25 15:54, Matt Coster wrote:
> On 23/06/2025 12:42, Michal Wilczynski wrote:
>> Several RISC-V boards feature Imagination GPUs that are compatible with
>> the PowerVR driver. An example is the IMG BXM-4-64 GPU on the Lichee Pi
>> 4A board. This commit adjusts the driver's Kconfig dependencies to allow
>> the PowerVR driver to be compiled on the RISC-V architecture.
>>
>> By enabling compilation on RISC-V, we expand support for these GPUs,
>> providing graphics acceleration capabilities and enhancing hardware
>> compatibility on RISC-V platforms.
>>
>> Add a dependency on MMU to fix a build warning on RISC-V configurations
>> without an MMU.
>>
>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  drivers/gpu/drm/imagination/Kconfig | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
>> index 5f9fff43d6baadc42ebf48d91729bfbf27e06caa..a7da858a5b301e8f088e3e22f5641feb2e078681 100644
>> --- a/drivers/gpu/drm/imagination/Kconfig
>> +++ b/drivers/gpu/drm/imagination/Kconfig
>> @@ -3,9 +3,10 @@
>>  
>>  config DRM_POWERVR
>>  	tristate "Imagination Technologies PowerVR (Series 6 and later) & IMG Graphics"
>> -	depends on ARM64
>> +	depends on (ARM64 || RISCV)
> 
> There were two issues you encountered when enabling COMPILE_TEST in v5,
> both of which are somewhat simple to workaround but expose underlying
> assumptions we made during early development.
> 
> The first [1] is due to us assuming a 64-bit platform, which was never a
> problem with the ARM64 dependency, but may actually be a problem with
> RISCV given this allows for 32-bit as well. You should probably make
> this (RISCV && 64BIT) until the implicit 64-bit dependency can be worked
> out.

Yeah will incude that in next revision.

> 
> Somewhat related, we also assume a little-endian host. Technically ARM64
> can also be big-endian, you just don't encounter that in the wild too
> often so it's never been a "real" issue. I do wonder if swapping out
> (ARM64 || RISCV) for (64BIT && CPU_LITTLE_ENDIAN) entirely would be a
> reasonable change, perhaps for another day though...
> 
> The other [2] is slightly more subtle. To keep things straightforward,
> we currently map CPU pages to GPU pages 1:1, meaning we use the CPU page
> size to define the GPU page size. That GPU page size is configurable,
> but does not support every possible size the CPU could support on any
> architecture. The failing test there was sparc64 with an 8K page size
> causing no GPU page size to be defined. See the #if/#elif ladder at the
> top of pvr_mmu.c for the supported sizes and the doc comment above
> PVR_DEVICE_PAGE_SIZE in pvr_mmu.h for the acknowledgement of the page
> size restrictions.
> 
> The "proper" fix here would be for us to make these two sizes
> independent, but that's not a trivial change. The "quick" fix I suppose
> would be to depend on one of the supported page sizes, so maybe
> (PAGE_SIZE_4KB || PAGE_SIZE_16KB || PAGE_SIZE_64KB || PAGE_SIZE_256KB)
> since the larger page sizes appear unsupported (probably for good
> reason).

Thanks for a great explanation !

> 
>>  	depends on DRM
>>  	depends on PM
>> +	depends on MMU
> 
> Nit: can you keep this alphabetical?
> 
> Cheers,
> Matt
> 
> [1]: https://lore.kernel.org/r/202506191323.zD1fszQb-lkp@intel.com/
> [2]: https://lore.kernel.org/r/202506201103.GX6DA9Gx-lkp@intel.com/
> 
>>  	select DRM_EXEC
>>  	select DRM_GEM_SHMEM_HELPER
>>  	select DRM_SCHED
>>
> 
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv