On Sat, Nov 14, 2020 at 10:17:12AM -0500, Jonathan Marek wrote:
> +void msm_gem_sync_cache(struct drm_gem_object *obj, uint32_t flags,
> + size_t range_start, size_t range_end)
> +{
> + struct msm_gem_object *msm_obj = to_msm_bo(obj);
> + struct device *dev = msm_obj->base.dev->dev;
> +
> + /* exit early if get_pages() hasn't been called yet */
> + if (!msm_obj->pages)
> + return;
> +
> + /* TODO: sync only the specified range */
> +
> + if (flags & MSM_GEM_SYNC_FOR_DEVICE) {
> + dma_sync_sg_for_device(dev, msm_obj->sgt->sgl,
> + msm_obj->sgt->nents, DMA_TO_DEVICE);
> + }
> +
> + if (flags & MSM_GEM_SYNC_FOR_CPU) {
> + dma_sync_sg_for_cpu(dev, msm_obj->sgt->sgl,
> + msm_obj->sgt->nents, DMA_FROM_DEVICE);
> + }
Splitting this helper from the only caller is rather strange, epecially
with the two unused arguments. And I think the way this is specified
to take a range, but ignoring it is actively dangerous. User space will
rely on it syncing everything sooner or later and then you are stuck.
So just define a sync all primitive for now, and if you really need a
range sync and have actually implemented it add a new ioctl for that.
