Am Mittwoch, dem 21.06.2023 um 23:30 +0800 schrieb Sui Jingfeng:
Hi,Please explain why you think that this isn't correct.
On 2023/6/21 18:00, Lucas Stach wrote:
I understand you are a profession person in vivante GPU driver domain.dma_sync_sgtable_for_cpu(dev->dev, etnaviv_obj->sgt,Drop this change. Instead etnaviv_gem_new_impl() should do the upgrade
etnaviv_op_to_dma_dir(op));
etnaviv_obj->last_cpu_prep_op = op;
@@ -408,8 +421,9 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
+ struct etnaviv_drm_private *priv = dev->dev_private;
- if (etnaviv_obj->flags & ETNA_BO_CACHED) {
+ if (!priv->dma_coherent && etnaviv_obj->flags & ETNA_BO_CACHED) {
/* fini without a prep is almost certainly a userspace error */
WARN_ON(etnaviv_obj->last_cpu_prep_op == 0);
dma_sync_sgtable_for_device(dev->dev, etnaviv_obj->sgt,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 3524b5811682..754126992264 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -112,11 +112,16 @@ static const struct etnaviv_gem_ops etnaviv_gem_prime_ops = {
struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sgt)
{
+ struct etnaviv_drm_private *priv = dev->dev_private;
struct etnaviv_gem_object *etnaviv_obj;
size_t size = PAGE_ALIGN(attach->dmabuf->size);
+ u32 cache_flags = ETNA_BO_WC;
int ret, npages;
- ret = etnaviv_gem_new_private(dev, size, ETNA_BO_WC,
+ if (priv->dma_coherent)
+ cache_flags = ETNA_BO_CACHED;
+
from WC to CACHED as necessary by adding something like this:
I respect you reviews and instruction.
But, I'm really reluctant to agree with this, is there any space to
negotiate?
/*This is policy, not a mechanism.
* Upgrade WC to CACHED when the device is hardware coherent and the
* platform doesn't allow mixing cached and writecombined mappings to
* the same memory area.
*/
if ((flags & ETNA_BO_CACHE_MASK) == ETNA_BO_WC &&
dev_is_dma_coherent(dev) && !drm_arch_can_wc_memory())
flags = (flags & ~ETNA_BO_CACHE_MASK) & ETNA_BO_CACHED;
Using what cache property is a user-space program's choice.
While you are override the WC with CACHED mapping. This is not correct
in the concept!
It allows
userspace to use WC mappings that would potentially cause loss of
coherency between CPU and GPU, which isn't acceptable.
Regards,
Lucas