Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <02c16e9b-0eca-caf4-b80c-53f1c7eab4e9@loongson.cn>
Date:   Thu, 22 Jun 2023 01:31:20 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v10 07/11] drm/etnaviv: Add support for the dma coherent
 device
Content-Language: en-US
To:     Lucas Stach <l.stach@pengutronix.de>,
        Sui Jingfeng <18949883232@163.com>,
        Russell King <linux+etnaviv@armlinux.org.uk>,
        Christian Gmeiner <christian.gmeiner@gmail.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>
Cc:     linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        etnaviv@lists.freedesktop.org,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Bjorn Helgaas <bhelgaas@google.com>
References: <20230620094716.2231414-1-18949883232@163.com>
 <20230620094716.2231414-8-18949883232@163.com>
 <8f74f0962c8bab6c832919a5340667c54e1a7ddc.camel@pengutronix.de>
 <2249b895-84b9-adea-531b-bf190e9c866f@loongson.cn>
 <030d44e2753b9b2eea0107cdee6c20e2bc2d3efe.camel@pengutronix.de>
 <3911d448-5613-23a8-cfcb-5ae418677338@loongson.cn>
 <87deb46db35b028da74c94f5496b721e14db4745.camel@pengutronix.de>
From:   Sui Jingfeng <suijingfeng@loongson.cn>
Organization: Loongson
In-Reply-To: <87deb46db35b028da74c94f5496b721e14db4745.camel@pengutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

On 2023/6/22 00:07, Lucas Stach wrote:
> And as the HW guarantees it on your platform, your platform
> implementation makes this function effectively a no-op. Skipping the
> call to this function is breaking the DMA API abstraction, as now the
> driver is second guessing the DMA API implementation. I really see no
> reason to do this.

It is the same reason you chose the word 'effectively', not 'difinitely'.

We don't want waste the CPU's time,


  to running the dma_sync_sg_for_cpu funcion() function


```

void dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
             int nelems, enum dma_data_direction dir)
{
     const struct dma_map_ops *ops = get_dma_ops(dev);

     BUG_ON(!valid_dma_direction(dir));
     if (dma_map_direct(dev, ops))
         dma_direct_sync_sg_for_cpu(dev, sg, nelems, dir);
     else if (ops->sync_sg_for_cpu)
         ops->sync_sg_for_cpu(dev, sg, nelems, dir);
     debug_dma_sync_sg_for_cpu(dev, sg, nelems, dir);
}

```


  to running the this:


```

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 (!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,
etnaviv_op_to_dma_dir(etnaviv_obj->last_cpu_prep_op));
         etnaviv_obj->last_cpu_prep_op = 0;
     }

     return 0;
}

```


But, this is acceptable, because we can kill the GEM_CPU_PREP and 
GEM_CPU_FINI ioctl entirely

at userspace for cached buffer, as this is totally not needed for cached 
mapping on our platform.


Well leave this for the WC mapping only,

OK ?

-- 
Jingfeng