[PATCH 6/7] drm: omapdrm: Fix incorrect usage of the term 'cache coherency'

[Tomi Valkeinen]

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 at ideasonboard.com>
> ---
>  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.

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170310/23566d99/attachment.sig>