On Sat, Jun 03, 2023 at 06:59:43PM +0800, Sui Jingfeng wrote:
> From: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
>
> Loongson CPUs maintain cache coherency by hardware, which means that the
> data in the CPU cache is identical to the data in main system memory. As
> for the peripheral device, most of Loongson chips chose to define the
> peripherals as DMA coherent by default, device drivers do not need to
> maintain the coherency between a processor and an I/O device manually.
>
> There are exceptions, for LS2K1000 SoC, part of peripheral device can be
> configured as dma non-coherent. But there is no released version of such
> firmware exist in the market. Peripherals of older ls2k1000 is also DMA
> non-conherent, but they are nearly outdated. So, those are trivial cases.
s/dma/DMA/
s/non-conherent/non-coherent/
s/ls2k1000/LS2K1000/
I guess when you say these are "trivial cases," you mean you don't
care about supporting those devices?
> Nevertheless, kernel space still need to do probe work, because vivante GPU
> IP has been integrated into various platform. Hence, this patch add runtime
> detection code to probe if a specific gpu is DMA coherent, If the answer is
> yes, we are going to utilize such features. On Loongson platfform, When a
> buffer is accesed by both the GPU and the CPU, The driver should prefer
> ETNA_BO_CACHED over ETNA_BO_WC.
s/gpu/GPU/
s/platfform/platform/
s/accesed/accessed/
I guess the only way to discover this coherency attribute is via the
DT "vivante,gc" property? Seems a little weird but I'm really not a
DT person.
> This patch also add a new parameter: etnaviv_param_gpu_coherent, which
> allow userspace to know if such a feature is available. Because
> write-combined BO is still preferred in some case, especially where don't
> need CPU read, for example, uploading shader bin.
> ...
> +static struct device_node *etnaviv_of_first_available_node(void)
> +{
> + struct device_node *core_node;
> +
> + for_each_compatible_node(core_node, NULL, "vivante,gc") {
> + if (!of_device_is_available(core_node))
> + continue;
> +
> + return core_node;
> + }
> +
> + return NULL;
Seems like this would be simpler as:
for_each_compatible_node(core_node, NULL, "vivante,gc") {
if (of_device_is_available(core_node))
return core_node;
}
return NULL;
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -8,6 +8,7 @@
> #include <linux/delay.h>
> #include <linux/dma-fence.h>
> #include <linux/dma-mapping.h>
> +#include <linux/dma-map-ops.h>
It looks like this #include might not be needed? You're only adding a
new reference to priv->dma_coherent, which looks like it was added to
etnaviv_drv.h.
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> @@ -164,6 +165,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
> *value = gpu->identity.eco_id;
> break;
>
> + case ETNAVIV_PARAM_GPU_COHERENT:
> + *value = priv->dma_coherent;
> + break;
> +
> default:
> DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
> return -EINVAL;
> @@ -1861,7 +1866,7 @@ static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq)
>
> gpu->irq = irq;
>
> - dev_info(dev, "IRQ handler registered, irq = %d\n", irq);
> + dev_info(dev, "irq(%d) handler registered\n", irq);
Looks possibly unnecessary, or at least unrelated to this patch.
> return 0;
> }
