Hi Tomi, On Friday 10 Mar 2017 14:13:08 Tomi Valkeinen wrote: > On 10/03/17 11:39, Laurent Pinchart wrote: > > The is_cache_coherent() function currently returns true when the mapping > > is not cache-coherent. This isn't a bug as such as the callers interpret > > cache-coherent as meaning that the driver has to handle the coherency > > manually, but it is nonetheless very confusing. Fix it and add a bit > > more documentation to explain how cached buffers are handled. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > --- > > > > drivers/gpu/drm/omapdrm/omap_gem.c | 22 +++++++++++++++------- > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c > > b/drivers/gpu/drm/omapdrm/omap_gem.c index 945bda8e330d..59594ec0d159 > > 100644 > > --- a/drivers/gpu/drm/omapdrm/omap_gem.c > > +++ b/drivers/gpu/drm/omapdrm/omap_gem.c > > @@ -732,16 +732,21 @@ int omap_gem_roll(struct drm_gem_object *obj, > > uint32_t roll)> > > * Memory Management & DMA Sync > > */ > > > > -/** > > - * shmem buffers that are mapped cached can simulate coherency via using > > - * page faulting to keep track of dirty pages > > +/* > > + * shmem buffers that are mapped cached are not coherent. > > + * > > + * We keep track of dirty pages using page faulting to perform cache > > management. + * When a page is mapped to the CPU in read/write mode the > > device can't access + * it and omap_obj->dma_addrs[i] is NULL. When a > > page is mapped to the device + * the omap_obj->dma_addrs[i] is set to the > > DMA address, and the page is + * unmapped from the CPU. > > > > */ > > > > static inline bool is_cached_coherent(struct drm_gem_object *obj) > > { > > > > struct omap_gem_object *omap_obj = to_omap_bo(obj); > > > > - return (omap_obj->flags & OMAP_BO_MEM_SHMEM) && > > - ((omap_obj->flags & OMAP_BO_CACHE_MASK) == OMAP_BO_CACHED); > > + return !((omap_obj->flags & OMAP_BO_MEM_SHMEM) && > > + ((omap_obj->flags & OMAP_BO_CACHE_MASK) == OMAP_BO_CACHED)); > > > > } > > > > /* Sync the buffer for CPU access.. note pages should already be > > > > @@ -752,7 +757,10 @@ void omap_gem_cpu_sync(struct drm_gem_object *obj, > > int pgoff)> > > struct drm_device *dev = obj->dev; > > struct omap_gem_object *omap_obj = to_omap_bo(obj); > > > > - if (is_cached_coherent(obj) && omap_obj->dma_addrs[pgoff]) { > > + if (!is_cached_coherent(obj)) > > + return; > > I think the ! is extra there. If the obj _is_ coherent, we can just return. You're right, my bad. I'll fix that in v2. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel